Merge pull request #5106

1bea2bb Rename ProcessBlock to ProcessNewBlock to indicate change of behaviour, and document it (Luke Dashjr)
d29a291 Rename RPC_TRANSACTION_* errors to RPC_VERIFY_* and use RPC_VERIFY_ERROR for submitblock (Luke Dashjr)
f877aaa Bugfix: submitblock: Use a temporary CValidationState to determine accurately the outcome of ProcessBlock, now that it no longer does the full block validity check (Luke Dashjr)
24e8896 Add CValidationInterface::BlockChecked notification (Luke Dashjr)
This commit is contained in:
Wladimir J. van der Laan 2014-11-03 12:16:12 +01:00
commit 84d26d3a36
No known key found for this signature in database
GPG key ID: 74810B012346C9A6
6 changed files with 71 additions and 18 deletions

View file

@ -152,6 +152,8 @@ struct CMainSignals {
boost::signals2::signal<void (const uint256 &)> Inventory; boost::signals2::signal<void (const uint256 &)> Inventory;
// Tells listeners to broadcast their data. // Tells listeners to broadcast their data.
boost::signals2::signal<void ()> Broadcast; boost::signals2::signal<void ()> Broadcast;
// Notifies listeners of a block validation result
boost::signals2::signal<void (const CBlock&, const CValidationState&)> BlockChecked;
} g_signals; } g_signals;
} // anon namespace } // anon namespace
@ -163,9 +165,11 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) {
g_signals.SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); g_signals.SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1));
g_signals.Inventory.connect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); g_signals.Inventory.connect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1));
g_signals.Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn)); g_signals.Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn));
g_signals.BlockChecked.connect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2));
} }
void UnregisterValidationInterface(CValidationInterface* pwalletIn) { void UnregisterValidationInterface(CValidationInterface* pwalletIn) {
g_signals.BlockChecked.disconnect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2));
g_signals.Broadcast.disconnect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn)); g_signals.Broadcast.disconnect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn));
g_signals.Inventory.disconnect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); g_signals.Inventory.disconnect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1));
g_signals.SetBestChain.disconnect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); g_signals.SetBestChain.disconnect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1));
@ -175,6 +179,7 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) {
} }
void UnregisterAllValidationInterfaces() { void UnregisterAllValidationInterfaces() {
g_signals.BlockChecked.disconnect_all_slots();
g_signals.Broadcast.disconnect_all_slots(); g_signals.Broadcast.disconnect_all_slots();
g_signals.Inventory.disconnect_all_slots(); g_signals.Inventory.disconnect_all_slots();
g_signals.SetBestChain.disconnect_all_slots(); g_signals.SetBestChain.disconnect_all_slots();
@ -1864,7 +1869,9 @@ bool static ConnectTip(CValidationState &state, CBlockIndex *pindexNew, CBlock *
{ {
CCoinsViewCache view(pcoinsTip); CCoinsViewCache view(pcoinsTip);
CInv inv(MSG_BLOCK, pindexNew->GetBlockHash()); CInv inv(MSG_BLOCK, pindexNew->GetBlockHash());
if (!ConnectBlock(*pblock, state, pindexNew, view)) { bool rv = ConnectBlock(*pblock, state, pindexNew, view);
g_signals.BlockChecked(*pblock, state);
if (!rv) {
if (state.IsInvalid()) if (state.IsInvalid())
InvalidBlockFound(pindexNew, state); InvalidBlockFound(pindexNew, state);
return error("ConnectTip() : ConnectBlock %s failed", pindexNew->GetBlockHash().ToString()); return error("ConnectTip() : ConnectBlock %s failed", pindexNew->GetBlockHash().ToString());
@ -2504,7 +2511,7 @@ void CBlockIndex::BuildSkip()
pskip = pprev->GetAncestor(GetSkipHeight(nHeight)); pskip = pprev->GetAncestor(GetSkipHeight(nHeight));
} }
bool ProcessBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, CDiskBlockPos *dbp) bool ProcessNewBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, CDiskBlockPos *dbp)
{ {
// Preliminary checks // Preliminary checks
bool checked = CheckBlock(*pblock, state); bool checked = CheckBlock(*pblock, state);
@ -2513,7 +2520,7 @@ bool ProcessBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, CDiskBl
LOCK(cs_main); LOCK(cs_main);
MarkBlockAsReceived(pblock->GetHash()); MarkBlockAsReceived(pblock->GetHash());
if (!checked) { if (!checked) {
return error("ProcessBlock() : CheckBlock FAILED"); return error("%s : CheckBlock FAILED", __func__);
} }
// Store to disk // Store to disk
@ -2523,11 +2530,11 @@ bool ProcessBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, CDiskBl
mapBlockSource[pindex->GetBlockHash()] = pfrom->GetId(); mapBlockSource[pindex->GetBlockHash()] = pfrom->GetId();
} }
if (!ret) if (!ret)
return error("ProcessBlock() : AcceptBlock FAILED"); return error("%s : AcceptBlock FAILED", __func__);
} }
if (!ActivateBestChain(state, pblock)) if (!ActivateBestChain(state, pblock))
return error("ProcessBlock() : ActivateBestChain failed"); return error("%s : ActivateBestChain failed", __func__);
return true; return true;
} }
@ -3136,7 +3143,7 @@ bool LoadExternalBlockFile(FILE* fileIn, CDiskBlockPos *dbp)
// process in case the block isn't known yet // process in case the block isn't known yet
if (mapBlockIndex.count(hash) == 0) { if (mapBlockIndex.count(hash) == 0) {
CValidationState state; CValidationState state;
if (ProcessBlock(state, NULL, &block, dbp)) if (ProcessNewBlock(state, NULL, &block, dbp))
nLoaded++; nLoaded++;
if (state.IsError()) if (state.IsError())
break; break;
@ -3156,7 +3163,7 @@ bool LoadExternalBlockFile(FILE* fileIn, CDiskBlockPos *dbp)
LogPrintf("%s: Processing out of order child %s of %s\n", __func__, block.GetHash().ToString(), LogPrintf("%s: Processing out of order child %s of %s\n", __func__, block.GetHash().ToString(),
head.ToString()); head.ToString());
CValidationState dummy; CValidationState dummy;
if (ProcessBlock(dummy, NULL, &block, &it->second)) if (ProcessNewBlock(dummy, NULL, &block, &it->second))
{ {
nLoaded++; nLoaded++;
queue.push_back(block.GetHash()); queue.push_back(block.GetHash());
@ -3934,7 +3941,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
pfrom->AddInventoryKnown(inv); pfrom->AddInventoryKnown(inv);
CValidationState state; CValidationState state;
ProcessBlock(state, pfrom, &block); ProcessNewBlock(state, pfrom, &block);
int nDoS; int nDoS;
if (state.IsInvalid(nDoS)) { if (state.IsInvalid(nDoS)) {
pfrom->PushMessage("reject", strCommand, state.GetRejectCode(), pfrom->PushMessage("reject", strCommand, state.GetRejectCode(),

View file

@ -151,8 +151,16 @@ void RegisterNodeSignals(CNodeSignals& nodeSignals);
/** Unregister a network node */ /** Unregister a network node */
void UnregisterNodeSignals(CNodeSignals& nodeSignals); void UnregisterNodeSignals(CNodeSignals& nodeSignals);
/** Process an incoming block */ /** Process an incoming block. This only returns after the best known valid
bool ProcessBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, CDiskBlockPos *dbp = NULL); block is made active. Note that it does not, however, guarantee that the
specific block passed to it has been checked for validity!
@param[out] state This may be set to an Error state if any error occurred processing it, including during validation/connection/etc of otherwise unrelated blocks during reorganisation; or it may be set to an Invalid state iff pblock is itself invalid (but this is not guaranteed even when the block is checked). If you want to *possibly* get feedback on whether pblock is valid, you must also install a CValidationInterface - this will have its BlockChecked method called whenever *any* block completes validation.
@param[in] pfrom The node which we are receiving the block from; it is added to mapBlockSource and may be penalised if the block is invalid.
@param[in] pblock The block we want to process.
@param[out] dbp If pblock is stored to disk (or already there), this will be set to its location.
@return True if state.IsValid()
*/
bool ProcessNewBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, CDiskBlockPos *dbp = NULL);
/** Check whether enough disk space is available for an incoming block */ /** Check whether enough disk space is available for an incoming block */
bool CheckDiskSpace(uint64_t nAdditionalBytes = 0); bool CheckDiskSpace(uint64_t nAdditionalBytes = 0);
/** Open a block file (blk?????.dat) */ /** Open a block file (blk?????.dat) */
@ -651,6 +659,7 @@ protected:
virtual void UpdatedTransaction(const uint256 &hash) {}; virtual void UpdatedTransaction(const uint256 &hash) {};
virtual void Inventory(const uint256 &hash) {}; virtual void Inventory(const uint256 &hash) {};
virtual void ResendWalletTransactions() {}; virtual void ResendWalletTransactions() {};
virtual void BlockChecked(const CBlock&, const CValidationState&) {};
friend void ::RegisterValidationInterface(CValidationInterface*); friend void ::RegisterValidationInterface(CValidationInterface*);
friend void ::UnregisterValidationInterface(CValidationInterface*); friend void ::UnregisterValidationInterface(CValidationInterface*);
friend void ::UnregisterAllValidationInterfaces(); friend void ::UnregisterAllValidationInterfaces();

View file

@ -428,8 +428,8 @@ bool ProcessBlockFound(CBlock* pblock, CWallet& wallet, CReserveKey& reservekey)
// Process this block the same as if we had received it from another node // Process this block the same as if we had received it from another node
CValidationState state; CValidationState state;
if (!ProcessBlock(state, NULL, pblock)) if (!ProcessNewBlock(state, NULL, pblock))
return error("BitcoinMiner : ProcessBlock, block not accepted"); return error("BitcoinMiner : ProcessNewBlock, block not accepted");
return true; return true;
} }

View file

@ -527,6 +527,24 @@ Value getblocktemplate(const Array& params, bool fHelp)
return result; return result;
} }
class submitblock_StateCatcher : public CValidationInterface
{
public:
uint256 hash;
bool found;
CValidationState state;
submitblock_StateCatcher(const uint256 &hashIn) : hash(hashIn), found(false), state() {};
protected:
virtual void BlockChecked(const CBlock& block, const CValidationState& stateIn) {
if (block.GetHash() != hash)
return;
found = true;
state = stateIn;
};
};
Value submitblock(const Array& params, bool fHelp) Value submitblock(const Array& params, bool fHelp)
{ {
if (fHelp || params.size() < 1 || params.size() > 2) if (fHelp || params.size() < 1 || params.size() > 2)
@ -559,8 +577,22 @@ Value submitblock(const Array& params, bool fHelp)
} }
CValidationState state; CValidationState state;
bool fAccepted = ProcessBlock(state, NULL, &pblock); submitblock_StateCatcher sc(pblock.GetHash());
if (!fAccepted) RegisterValidationInterface(&sc);
bool fAccepted = ProcessNewBlock(state, NULL, &pblock);
UnregisterValidationInterface(&sc);
if (fAccepted)
{
if (!sc.found)
return "inconclusive";
state = sc.state;
}
if (state.IsError())
{
std::string strRejectReason = state.GetRejectReason();
throw JSONRPCError(RPC_VERIFY_ERROR, strRejectReason);
}
if (state.IsInvalid())
return "rejected"; // TODO: report validation state return "rejected"; // TODO: report validation state
return Value::null; return Value::null;

View file

@ -49,9 +49,14 @@ enum RPCErrorCode
RPC_INVALID_PARAMETER = -8, // Invalid, missing or duplicate parameter RPC_INVALID_PARAMETER = -8, // Invalid, missing or duplicate parameter
RPC_DATABASE_ERROR = -20, // Database error RPC_DATABASE_ERROR = -20, // Database error
RPC_DESERIALIZATION_ERROR = -22, // Error parsing or validating structure in raw format RPC_DESERIALIZATION_ERROR = -22, // Error parsing or validating structure in raw format
RPC_TRANSACTION_ERROR = -25, // General error during transaction submission RPC_VERIFY_ERROR = -25, // General error during transaction or block submission
RPC_TRANSACTION_REJECTED = -26, // Transaction was rejected by network rules RPC_VERIFY_REJECTED = -26, // Transaction or block was rejected by network rules
RPC_TRANSACTION_ALREADY_IN_CHAIN= -27, // Transaction already in chain RPC_VERIFY_ALREADY_IN_CHAIN = -27, // Transaction already in chain
// Aliases for backward compatibility
RPC_TRANSACTION_ERROR = RPC_VERIFY_ERROR,
RPC_TRANSACTION_REJECTED = RPC_VERIFY_REJECTED,
RPC_TRANSACTION_ALREADY_IN_CHAIN= RPC_VERIFY_ALREADY_IN_CHAIN,
// P2P client errors // P2P client errors
RPC_CLIENT_NOT_CONNECTED = -9, // Bitcoin is not connected RPC_CLIENT_NOT_CONNECTED = -9, // Bitcoin is not connected

View file

@ -79,7 +79,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
pblock->hashMerkleRoot = pblock->BuildMerkleTree(); pblock->hashMerkleRoot = pblock->BuildMerkleTree();
pblock->nNonce = blockinfo[i].nonce; pblock->nNonce = blockinfo[i].nonce;
CValidationState state; CValidationState state;
BOOST_CHECK(ProcessBlock(state, NULL, pblock)); BOOST_CHECK(ProcessNewBlock(state, NULL, pblock));
BOOST_CHECK(state.IsValid()); BOOST_CHECK(state.IsValid());
pblock->hashPrevBlock = pblock->GetHash(); pblock->hashPrevBlock = pblock->GetHash();
} }