diff --git a/src/init.cpp b/src/init.cpp index 22d09d28e9..a3ff2d9c8c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -627,7 +627,6 @@ void SetupServerArgs(NodeContext& node) argsman.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`)", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-mainchainrpctimeout=", strprintf("Timeout in seconds during mainchain RPC requests, or 0 for no timeout. (default: %d)", DEFAULT_HTTP_CLIENT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-peginconfirmationdepth=", strprintf("Pegin claims must be this deep to be considered valid. (default: %d)", DEFAULT_PEGIN_CONFIRMATION_DEPTH), ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); - argsman.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), ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-parentpubkeyprefix", strprintf("The byte prefix, in decimal, of the parent chain's base58 pubkey address. (default: %d)", 111), ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-parentscriptprefix", strprintf("The byte prefix, in decimal, of the parent chain's base58 script address. (default: %d)", 196), ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-parent_bech32_hrp", strprintf("The human-readable part of the parent chain's bech32 encoding. (default: %s)", "bc"), ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); diff --git a/src/mainchainrpc.cpp b/src/mainchainrpc.cpp index 1241f02f13..ef7725bbe4 100644 --- a/src/mainchainrpc.cpp +++ b/src/mainchainrpc.cpp @@ -153,18 +153,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 bf262520b3..6bb9229ebe 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 82b8b96be0..3b105eca68 100644 --- a/src/pegins.h +++ b/src/pegins.h @@ -23,7 +23,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); /* Consensus-critical. Matching against telescoped multisig used on Liquid v1: * Pseudo-structure: diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 34792949e4..56cdbf8e00 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -545,8 +545,9 @@ bool ValidateTransactionPeginInputs(const CMutableTransaction& mtx, std::map &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 76b0e56fc3..f78a2acb2a 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -109,8 +109,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 a3271c4ad4..0db428f207 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2083,6 +2083,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). */ @@ -2820,7 +2843,7 @@ public: * * The block is added to connectTrace if connection succeeds. */ -bool CChainState::ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) +bool CChainState::ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool, bool& fStall) { AssertLockHeld(cs_main); AssertLockHeld(m_mempool.cs); @@ -2838,6 +2861,14 @@ bool CChainState::ConnectTip(BlockValidationState& state, const CChainParams& ch 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; @@ -2854,29 +2885,6 @@ bool CChainState::ConnectTip(BlockValidationState& state, const CChainParams& ch 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.GetRejectReason() == "bad-pegin-witness") { - //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(), state.ToString()); } @@ -2988,7 +2996,7 @@ void CChainState::PruneBlockIndexCandidates() { * * @returns true unless a system error occurred */ -bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) +bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace, bool& fStall) { AssertLockHeld(cs_main); AssertLockHeld(m_mempool.cs); @@ -3033,7 +3041,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai // 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.GetResult() != BlockValidationResult::BLOCK_MUTATED) { @@ -3051,6 +3059,12 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai 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 || m_chain.Tip()->nChainWork > pindexOldTip->nChainWork) { // We're in a better position than we were. Return temporarily to release the lock. @@ -3130,6 +3144,8 @@ bool CChainState::ActivateBestChain(BlockValidationState &state, const CChainPar CBlockIndex *pindexMostWork = nullptr; CBlockIndex *pindexNewTip = nullptr; int nStopAtHeight = gArgs.GetArg("-stopatheight", DEFAULT_STOPATHEIGHT); + bool fStall = false; + do { // Block until the validation queue drains. This should largely // never happen in normal operation, however may happen during @@ -3160,7 +3176,7 @@ bool CChainState::ActivateBestChain(BlockValidationState &state, const CChainPar 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)) { // A system error occurred return false; } @@ -3176,6 +3192,11 @@ bool CChainState::ActivateBestChain(BlockValidationState &state, const CChainPar assert(trace.pblock && trace.pindex); GetMainSignals().BlockConnected(trace.pblock, trace.pindex); } + + if (fStall) { + // Stuck waiting for parent chain daemon, twiddle our thumbs for awhile. + break; + } } while (!m_chain.Tip() || (starting_tip && CBlockIndexWorkComparator()(m_chain.Tip(), starting_tip))); if (!blocks_connected) return true; @@ -3194,6 +3215,11 @@ bool CChainState::ActivateBestChain(BlockValidationState &state, const CChainPar } // 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 @@ -5711,35 +5737,12 @@ void ChainstateManager::MaybeRebalanceCaches() } // 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); - ChainstateManager& chainman = g_chainman; - if (chainman.BlockIndex().count(blockhash)) { - CBlockIndex* pblockindex = chainman.BlockIndex()[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) { @@ -5790,50 +5793,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) { - BlockValidationState state; - for(const uint256& blockhash : vblocksToReconsider) { - { - LOCK(cs_main); - ChainstateManager& chainman = g_chainman; - if (chainman.BlockIndex().count(blockhash) == 0) - continue; - CBlockIndex* pblockindex = chainman.BlockIndex()[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); - ChainstateManager& chainman = g_chainman; - if (chainman.BlockIndex().count(blockhash)) { - CBlockIndex* pblockindex = chainman.BlockIndex()[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; } diff --git a/src/validation.h b/src/validation.h index 79f4e24ed0..ba698def55 100644 --- a/src/validation.h +++ b/src/validation.h @@ -719,8 +719,8 @@ public: std::string ToString() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); private: - bool ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs); - bool ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs); + bool ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace, bool& fStall) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs); + bool ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool, bool& fStall) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs); void InvalidBlockFound(CBlockIndex *pindex, const BlockValidationState &state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); CBlockIndex* FindMostWorkChain() EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/test/functional/feature_fedpeg.py b/test/functional/feature_fedpeg.py index 882bd6dfaf..26cdaca74b 100755 --- a/test/functional/feature_fedpeg.py +++ b/test/functional/feature_fedpeg.py @@ -120,7 +120,6 @@ class FedPegTest(BitcoinTestFramework): '-peginconfirmationdepth=10', '-mainchainrpchost=127.0.0.1', '-mainchainrpcport=%s' % rpc_port(n), - '-recheckpeginblockinterval=15', # Long enough to allow failure and repair before timeout '-parentgenesisblockhash=%s' % self.parentgenesisblockhash, '-parentpubkeyprefix=111', '-parentscriptprefix=196',