From 4ba4e0716d0867a02dcbd9e8ffab24d722083d21 Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Wed, 30 Jun 2021 02:19:24 -0700 Subject: [PATCH] When validation is waiting for parent chain daemon, "stall". Currently, if -validatepegin is given, and block validation can't proceed because the parent chain is not synced, we mark the block invalid and put it in a queue to be "revalidated" later. Unfortunately, marking a block invalid has downstream consequences, in particular causing descendant blocks to be marked invalid, which are not currently fixed by the queue. Instead, we'll use a different strategy: if the mainchain daemon isn't sufficiently synced to validate a block, we will "stall" connecting that block to the chain, and have ActivateBestChain simply keep the tip at the previous block until we're ready. We can still download and validate (partly) blocks past this point while we're waiting. They will be connected once the parent chain daemon catches up. --- src/init.cpp | 1 - src/mainchainrpc.cpp | 10 ++- src/pegins.cpp | 9 ++- src/pegins.h | 2 +- src/rpc/rawtransaction.cpp | 5 +- src/txdb.cpp | 8 -- src/txdb.h | 2 - src/validation.cpp | 154 ++++++++++++++----------------------- 8 files changed, 78 insertions(+), 113 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 7e847479cd..534f7cce07 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -582,7 +582,6 @@ void SetupServerArgs() gArgs.AddArg("-mainchainrpccookiefile=", "The bitcoind cookie auth path which the daemon will use to connect to the trusted mainchain daemon to validate peg-ins. (default: `/regtest/.cookie`)", false, OptionsCategory::ELEMENTS); gArgs.AddArg("-mainchainrpctimeout=", strprintf("Timeout in seconds during mainchain RPC requests, or 0 for no timeout. (default: %d)", DEFAULT_HTTP_CLIENT_TIMEOUT), false, OptionsCategory::ELEMENTS); gArgs.AddArg("-peginconfirmationdepth=", strprintf("Pegin claims must be this deep to be considered valid. (default: %d)", DEFAULT_PEGIN_CONFIRMATION_DEPTH), false, OptionsCategory::ELEMENTS); - gArgs.AddArg("-recheckpeginblockinterval=", strprintf("The interval in seconds at which a peg-in witness failing block is re-evaluated in case of intermittent peg-in witness failure. 0 means never. (default: %u)", 120), false, OptionsCategory::ELEMENTS); gArgs.AddArg("-parentpubkeyprefix", strprintf("The byte prefix, in decimal, of the parent chain's base58 pubkey address. (default: %d)", 111), false, OptionsCategory::CHAINPARAMS); gArgs.AddArg("-parentscriptprefix", strprintf("The byte prefix, in decimal, of the parent chain's base58 script address. (default: %d)", 196), false, OptionsCategory::CHAINPARAMS); gArgs.AddArg("-parent_bech32_hrp", strprintf("The human-readable part of the parent chain's bech32 encoding. (default: %s)", "bc"), false, OptionsCategory::CHAINPARAMS); diff --git a/src/mainchainrpc.cpp b/src/mainchainrpc.cpp index abaf16cf6a..9d969f55a3 100644 --- a/src/mainchainrpc.cpp +++ b/src/mainchainrpc.cpp @@ -152,18 +152,24 @@ UniValue CallMainChainRPC(const std::string& strMethod, const UniValue& params) bool IsConfirmedBitcoinBlock(const uint256& hash, const int nMinConfirmationDepth, const int nbTxs) { + LogPrintf("Checking for confirmed bitcoin block with hash %s, mindepth %d, nbtxs %d\n", hash.ToString().c_str(), nMinConfirmationDepth, nbTxs); try { UniValue params(UniValue::VARR); params.push_back(hash.GetHex()); UniValue reply = CallMainChainRPC("getblockheader", params); - if (!find_value(reply, "error").isNull()) + if (!find_value(reply, "error").isNull()) { + LogPrintf("ERROR: Got error reply from bitcoind getblockheader.\n"); return false; + } UniValue result = find_value(reply, "result"); - if (!result.isObject()) + if (!result.isObject()) { + LogPrintf("ERROR: bitcoind getblockheader result was malformed (not object).\n"); return false; + } UniValue confirmations = find_value(result.get_obj(), "confirmations"); if (!confirmations.isNum() || confirmations.get_int64() < nMinConfirmationDepth) { + LogPrintf("Insufficient confirmations (got %s).\n", confirmations.write()); return false; } diff --git a/src/pegins.cpp b/src/pegins.cpp index 167d1cd3dd..462444b537 100644 --- a/src/pegins.cpp +++ b/src/pegins.cpp @@ -240,7 +240,11 @@ bool CheckParentProofOfWork(uint256 hash, unsigned int nBits, const Consensus::P return true; } -bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const std::vector>& fedpegscripts, const COutPoint& prevout, std::string& err_msg, bool check_depth) { +bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const std::vector>& fedpegscripts, const COutPoint& prevout, std::string& err_msg, bool check_depth, bool* depth_failed) { + if (depth_failed) { + *depth_failed = false; + } + // 0) Return false if !consensus.has_parent_chain if (!Params().GetConsensus().has_parent_chain) { err_msg = "Parent chain is not enabled on this network."; @@ -372,6 +376,9 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const std::vector< LogPrintf("Required depth: %d\n", required_depth); if (!IsConfirmedBitcoinBlock(block_hash, required_depth, num_txs)) { err_msg = "Needs more confirmations."; + if (depth_failed) { + *depth_failed = true; + } return false; } } diff --git a/src/pegins.h b/src/pegins.h index e3e2f80082..f379306721 100644 --- a/src/pegins.h +++ b/src/pegins.h @@ -19,7 +19,7 @@ bool GetAmountFromParentChainPegin(CAmount& amount, const CTransaction& txBTC, u /** Check whether a parent chain block hash satisfies the proof-of-work requirement specified by nBits */ bool CheckParentProofOfWork(uint256 hash, unsigned int nBits, const Consensus::Params&); /** Checks pegin witness for validity */ -bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const std::vector>& fedpegscripts, const COutPoint& prevout, std::string& err_msg, bool check_depth); +bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const std::vector>& fedpegscripts, const COutPoint& prevout, std::string& err_msg, bool check_depth, bool* depth_failed = nullptr); // Constructs unblinded output to be used in amount and scriptpubkey checks during pegin CTxOut GetPeginOutputFromWitness(const CScriptWitness& pegin_witness); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index b32a7fd505..9ce75016c4 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1060,8 +1060,9 @@ UniValue SignTransaction(interfaces::Chain& chain, CMutableTransaction& mtx, con continue; } // Report warning about immature peg-in though - if(txin.m_is_pegin && !IsValidPeginWitness(txConst.witness.vtxinwit[i].m_pegin_witness, fedpegscripts, txin.prevout, err, true)) { - assert(err == "Needs more confirmations."); + bool depth_failed = false; + if(txin.m_is_pegin && !IsValidPeginWitness(txConst.witness.vtxinwit[i].m_pegin_witness, fedpegscripts, txin.prevout, err, true, &depth_failed)) { + assert(depth_failed); immature_pegin = true; } diff --git a/src/txdb.cpp b/src/txdb.cpp index 9694e5bf66..9942a44e05 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -272,14 +272,6 @@ bool CBlockTreeDB::ReadFlag(const std::string &name, bool &fValue) { return true; } -// ELEMENTS: -bool CBlockTreeDB::ReadInvalidBlockQueue(std::vector &vBlocks) { - return Read(std::make_pair(DB_INVALID_BLOCK_Q, uint256S("0")), vBlocks);//FIXME: why uint 56 and not "" -} -bool CBlockTreeDB::WriteInvalidBlockQueue(const std::vector &vBlocks) { - return Write(std::make_pair(DB_INVALID_BLOCK_Q, uint256S("0")), vBlocks); -} - bool CBlockTreeDB::ReadPAKList(std::vector >& offline_list, std::vector >& online_list, bool& reject) { return Read(std::make_pair(DB_PAK, uint256S("1")), offline_list) && Read(std::make_pair(DB_PAK, uint256S("2")), online_list) && Read(std::make_pair(DB_PAK, uint256S("3")), reject); diff --git a/src/txdb.h b/src/txdb.h index 19eec51587..cb7b9a63ef 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -99,8 +99,6 @@ public: bool ReadFlag(const std::string &name, bool &fValue); bool LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function insertBlockIndex); // ELEMENTS: - bool ReadInvalidBlockQueue(std::vector &vBlocks); - bool WriteInvalidBlockQueue(const std::vector &vBlocks); bool ReadPAKList(std::vector >& offline_list, std::vector >& online_list, bool& reject); bool WritePAKList(const std::vector >& offline_list, const std::vector >& online_list, bool reject); }; diff --git a/src/validation.cpp b/src/validation.cpp index 58da1efe17..edf73805f3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -47,6 +47,7 @@ #include #include +#include #include #include @@ -193,8 +194,8 @@ public: void UnloadBlockIndex(); private: - bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace, bool& fStall) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool, bool& fStall) EXCLUSIVE_LOCKS_REQUIRED(cs_main); CBlockIndex* AddToBlockIndex(const CBlockHeader& block) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Create a new block index entry for a given block hash */ @@ -1936,6 +1937,29 @@ static int64_t nTimeCallbacks = 0; static int64_t nTimeTotal = 0; static int64_t nBlocksTotal = 0; +bool CheckPeginRipeness(const CBlock& block, const std::vector>& fedpegscripts) { + for (unsigned int i = 0; i < block.vtx.size(); i++) { + const CTransaction &tx = *(block.vtx[i]); + + if (!tx.IsCoinBase()) { + for (unsigned int i = 0; i < tx.vin.size(); ++i) { + if (tx.vin[i].m_is_pegin) { + std::string err; + bool depth_failed = false; + if ((tx.witness.vtxinwit.size() <= i) || !IsValidPeginWitness(tx.witness.vtxinwit[i].m_pegin_witness, fedpegscripts, tx.vin[i].prevout, err, true, &depth_failed)) { + if (depth_failed) { + return false; // Pegins not ripe. + } else { + return true; // Some other failure; details later. + } + } + } + } + } + } + return true; +} + /** Apply the effects of this block (with given index) on the UTXO set represented by coins. * Validity checks that depend on the UTXO set are also done; ConnectBlock() * can fail if those validity checks fail (among other reasons). */ @@ -2601,7 +2625,7 @@ public: * * The block is added to connectTrace if connection succeeds. */ -bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) +bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool, bool& fStall) { assert(pindexNew->pprev == chainActive.Tip()); // Read block from disk. @@ -2616,6 +2640,14 @@ bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainp pthisBlock = pblock; } const CBlock& blockConnecting = *pthisBlock; + + const auto& fedpegscripts = GetValidFedpegScripts(pindexNew, chainparams.GetConsensus(), false /* nextblock_validation */); + if (!CheckPeginRipeness(blockConnecting, fedpegscripts)) { + LogPrintf("STALLING further progress in ConnectTip while waiting for parent chain daemon to catch up! Chain will not grow until this is remedied!\n"); + fStall = true; + return true; + } + // Apply the block atomically to the chain state. int64_t nTime2 = GetTimeMicros(); nTimeReadFromDisk += nTime2 - nTime1; int64_t nTime3; @@ -2632,29 +2664,6 @@ bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainp if (!rv) { if (state.IsInvalid()) { InvalidBlockFound(pindexNew, state); - - // ELEMENTS: - // Possibly result of RPC to mainchain bitcoind failure - // or unseen Bitcoin blocks. - // These blocks are later re-evaluated at an interval - // set by `-recheckpeginblockinterval`. - if (state.GetRejectCode() == REJECT_PEGIN) { - //Write queue of invalid blocks that - //must be cleared to continue operation - std::vector vinvalidBlocks; - pblocktree->ReadInvalidBlockQueue(vinvalidBlocks); - bool blockAlreadyInvalid = false; - for (uint256& hash : vinvalidBlocks) { - if (hash == blockConnecting.GetHash()) { - blockAlreadyInvalid = true; - break; - } - } - if (!blockAlreadyInvalid) { - vinvalidBlocks.push_back(blockConnecting.GetHash()); - pblocktree->WriteInvalidBlockQueue(vinvalidBlocks); - } - } } return error("%s: ConnectBlock %s failed, %s", __func__, pindexNew->GetBlockHash().ToString(), FormatStateMessage(state)); } @@ -2762,7 +2771,7 @@ void CChainState::PruneBlockIndexCandidates() { * Try to make some progress towards making pindexMostWork the active block. * pblock is either nullptr or a pointer to a CBlock corresponding to pindexMostWork. */ -bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) +bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace, bool& fStall) { AssertLockHeld(cs_main); @@ -2801,7 +2810,7 @@ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainPar // Connect new blocks. for (CBlockIndex *pindexConnect : reverse_iterate(vpindexToConnect)) { - if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr(), connectTrace, disconnectpool)) { + if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr(), connectTrace, disconnectpool, fStall)) { if (state.IsInvalid()) { // The block violates a consensus rule. if (!state.CorruptionPossible()) { @@ -2819,6 +2828,12 @@ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainPar return false; } } else { + if (fStall) { + // We didn't make progress because the parent chain is not + // synced enough to check pegins. Try again later. + fContinue = false; + break; + } PruneBlockIndexCandidates(); if (!pindexOldTip || chainActive.Tip()->nChainWork > pindexOldTip->nChainWork) { // We're in a better position than we were. Return temporarily to release the lock. @@ -2899,6 +2914,8 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& CBlockIndex *pindexMostWork = nullptr; CBlockIndex *pindexNewTip = nullptr; int nStopAtHeight = gArgs.GetArg("-stopatheight", DEFAULT_STOPATHEIGHT); + bool fStall = false; + do { boost::this_thread::interruption_point(); @@ -2930,7 +2947,7 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& bool fInvalidFound = false; std::shared_ptr nullBlockPtr; - if (!ActivateBestChainStep(state, chainparams, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connectTrace)) + if (!ActivateBestChainStep(state, chainparams, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connectTrace, fStall)) return false; blocks_connected = true; @@ -2944,6 +2961,11 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& assert(trace.pblock && trace.pindex); GetMainSignals().BlockConnected(trace.pblock, trace.pindex, trace.conflictedTxs); } + + if (fStall) { + // Stuck waiting for parent chain daemon, twiddle our thumbs for awhile. + break; + } } while (!chainActive.Tip() || (starting_tip && CBlockIndexWorkComparator()(chainActive.Tip(), starting_tip))); if (!blocks_connected) return true; @@ -2962,6 +2984,11 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& } // When we reach this point, we switched to a new tip (stored in pindexNewTip). + if (fStall) { + // Stuck waiting for parent chain daemon, twiddle our thumbs for awhile. + break; + } + if (nStopAtHeight && pindexNewTip && pindexNewTip->nHeight >= nStopAtHeight) StartShutdown(); // We check shutdown only after giving ActivateBestChainStep a chance to run once so that we @@ -5306,34 +5333,12 @@ public: } instance_of_cmaincleanup; // ELEMENTS: -/* This function has two major purposes: - * 1) Checks that the RPC connection to the parent chain node +/* This function checks that the RPC connection to the parent chain node * can be attained, and is returning back reasonable answers. - * 2) Re-evaluates a list of blocks that have been deemed "bad" - * from the perspective of peg-in witness validation. Blocks are - * added to this queue in ConnectTip based on the error code returned. */ bool MainchainRPCCheck(const bool init) { - // First, we can clear out any blocks thatsomehow are now deemed valid - // eg reconsiderblock rpc call manually - std::vector vblocksToReconsider; - pblocktree->ReadInvalidBlockQueue(vblocksToReconsider); - std::vector vblocksToReconsiderAgain; - for(uint256& blockhash : vblocksToReconsider) { - LOCK(cs_main); - if (mapBlockIndex.count(blockhash)) { - CBlockIndex* pblockindex = mapBlockIndex[blockhash]; - if ((pblockindex->nStatus & BLOCK_FAILED_MASK)) { - vblocksToReconsiderAgain.push_back(blockhash); - } - } - } - vblocksToReconsider = vblocksToReconsiderAgain; - vblocksToReconsiderAgain.clear(); - pblocktree->WriteInvalidBlockQueue(vblocksToReconsider); - - // Next, check for working and valid rpc + // Check for working and valid rpc if (gArgs.GetBoolArg("-validatepegin", Params().GetConsensus().has_parent_chain)) { // During init try until a non-RPC_IN_WARMUP result while (true) { @@ -5384,48 +5389,5 @@ bool MainchainRPCCheck(const bool init) } } - //Sanity startup check won't reconsider queued blocks - if (init) { - return true; - } - - // Getting this far means we either aren't validating pegins(so let's make sure that's why - // it failed previously) or we successfully connected to bitcoind - // Time to reconsider blocks - if (vblocksToReconsider.size() > 0) { - CValidationState state; - for(const uint256& blockhash : vblocksToReconsider) { - { - LOCK(cs_main); - if (mapBlockIndex.count(blockhash) == 0) - continue; - CBlockIndex* pblockindex = mapBlockIndex[blockhash]; - ResetBlockFailureFlags(pblockindex); - } - } - - //All blocks are now being reconsidered - ActivateBestChain(state, Params()); - //This simply checks for DB errors - if (!state.IsValid()) { - //Something scary? - } - - //Now to clear out now-valid blocks - for(const uint256& blockhash : vblocksToReconsider) { - LOCK(cs_main); - if (mapBlockIndex.count(blockhash)) { - CBlockIndex* pblockindex = mapBlockIndex[blockhash]; - - //Marked as invalid still, put back into queue - if((pblockindex->nStatus & BLOCK_FAILED_MASK)) { - vblocksToReconsiderAgain.push_back(blockhash); - } - } - } - - //Write back remaining blocks - pblocktree->WriteInvalidBlockQueue(vblocksToReconsiderAgain); - } return true; }