From 8d5ae627b87112564b0108d1316b6c2c809aa336 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Sun, 5 Mar 2023 19:45:22 +0000 Subject: [PATCH 01/14] uint256: add definition for constant 'ZERO' --- src/uint256.cpp | 1 + src/uint256.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/uint256.cpp b/src/uint256.cpp index 8c89e896824c..e5d429d64518 100644 --- a/src/uint256.cpp +++ b/src/uint256.cpp @@ -80,5 +80,6 @@ template std::string base_blob<256>::ToString() const; template void base_blob<256>::SetHex(const char*); template void base_blob<256>::SetHex(const std::string&); +const uint256 uint256::ZERO(0); const uint256 uint256::ONE(1); const uint256 uint256::TWO(2); diff --git a/src/uint256.h b/src/uint256.h index b6aee3cf1413..101f617f5ebc 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -127,6 +127,7 @@ class uint256 : public base_blob<256> { constexpr uint256() {} constexpr explicit uint256(uint8_t v) : base_blob<256>(v) {} explicit uint256(const std::vector& vch) : base_blob<256>(vch) {} + static const uint256 ZERO; static const uint256 ONE; static const uint256 TWO; }; From 00802bb21d882886a86c19104722385ea9ff2bb6 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Wed, 5 Apr 2023 21:48:02 +0000 Subject: [PATCH 02/14] partial bitcoin#17938: Disallow automatic conversion between disparate hash types includes: - 0a5ea32ce605984094c5552877cb99bc81654f2c - 3fcc46812334074d2c77a6233e8a961cd0785872 - 2c54217f913967703b404747133be67cf2f4feac - 966a22d859db37b1775e2180e5be032fc4fdf483 - 4d7369125a82214ea42b808a32b71b315a5c3c72 --- src/evo/deterministicmns.cpp | 4 +- src/qt/coincontroldialog.cpp | 2 +- src/rpc/evo.cpp | 6 +-- src/rpc/util.cpp | 2 +- src/script/sign.cpp | 2 +- src/script/standard.cpp | 14 ++++-- src/script/standard.h | 86 +++++++++++++++++++++++++++++----- src/spork.cpp | 2 +- src/wallet/rpcdump.cpp | 4 +- src/wallet/rpcwallet.cpp | 4 +- src/wallet/scriptpubkeyman.cpp | 6 +-- 11 files changed, 102 insertions(+), 30 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index f91214ccea82..1c8e57ed4fa9 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -1416,7 +1416,7 @@ template static bool CheckHashSig(const ProTx& proTx, const PKHash& pkhash, CValidationState& state) { std::string strError; - if (!CHashSigner::VerifyHash(::SerializeHash(proTx), CKeyID(pkhash), proTx.vchSig, strError)) { + if (!CHashSigner::VerifyHash(::SerializeHash(proTx), ToKeyID(pkhash), proTx.vchSig, strError)) { return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-sig"); } return true; @@ -1426,7 +1426,7 @@ template static bool CheckStringSig(const ProTx& proTx, const PKHash& pkhash, CValidationState& state) { std::string strError; - if (!CMessageSigner::VerifyMessage(CKeyID(pkhash), proTx.vchSig, proTx.MakeSignString(), strError)) { + if (!CMessageSigner::VerifyMessage(ToKeyID(pkhash), proTx.vchSig, proTx.MakeSignString(), strError)) { return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-sig"); } return true; diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 88958b875991..4f4dd56a6be7 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -512,7 +512,7 @@ void CoinControlDialog::updateLabels(CCoinControl& m_coin_control, WalletModel * { CPubKey pubkey; PKHash *pkhash = std::get_if(&address); - if (pkhash && model->wallet().getPubKey(out.txout.scriptPubKey, CKeyID(*pkhash), pubkey)) + if (pkhash && model->wallet().getPubKey(out.txout.scriptPubKey, ToKeyID(*pkhash), pubkey)) { nBytesInputs += (pubkey.IsCompressed() ? 148 : 180); } diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 90df3dff897f..58dff88ebc2c 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -178,7 +178,7 @@ static CKeyID ParsePubKeyIDFromAddress(const std::string& strAddress, const std: if (!pkhash) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be a valid P2PKH address, not %s", paramName, strAddress)); } - return CKeyID(*pkhash); + return ToKeyID(*pkhash); } static CBLSPublicKey ParseBLSPubKey(const std::string& hexKey, const std::string& paramName, bool specific_legacy_bls_scheme = false) @@ -773,7 +773,7 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, } CKey key; - if (!spk_man->GetKey(CKeyID(*pkhash), key)) { + if (!spk_man->GetKey(ToKeyID(*pkhash), key)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("collateral key not in wallet: %s", EncodeDestination(txDest))); } SignSpecialTxPayloadByString(tx, ptx, key); @@ -1235,7 +1235,7 @@ static bool CheckWalletOwnsScript(CWallet* pwallet, const CScript& script) { CTxDestination dest; if (ExtractDestination(script, dest)) { - if ((std::get_if(&dest) && spk_man->HaveKey(CKeyID(*std::get_if(&dest)))) || (std::get_if(&dest) && spk_man->HaveCScript(ScriptHash(*std::get_if(&dest))))) { + if ((std::get_if(&dest) && spk_man->HaveKey(ToKeyID(*std::get_if(&dest)))) || (std::get_if(&dest) && spk_man->HaveCScript(CScriptID{ScriptHash(*std::get_if(&dest))}))) { return true; } } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 6b8b48a26aac..e1803542405c 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -197,7 +197,7 @@ CPubKey AddrToPubKey(const FillableSigningProvider& keystore, const std::string& throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("%s does not refer to a key", addr_in)); } CPubKey vchPubKey; - if (!keystore.GetPubKey(CKeyID(*pkhash), vchPubKey)) { + if (!keystore.GetPubKey(ToKeyID(*pkhash), vchPubKey)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("no full public key for address %s", addr_in)); } if (!vchPubKey.IsFullyValid()) { diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 6cbbdfcd8bbc..063c560660d8 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -123,7 +123,7 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator } case TxoutType::SCRIPTHASH: h160 = uint160(vSolutions[0]); - if (GetCScript(provider, sigdata, h160, scriptRet)) { + if (GetCScript(provider, sigdata, CScriptID{h160}, scriptRet)) { ret.push_back(std::vector(scriptRet.begin(), scriptRet.end())); return true; } diff --git a/src/script/standard.cpp b/src/script/standard.cpp index 228a140c4e26..3f7db837d0a4 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -13,11 +13,19 @@ typedef std::vector valtype; bool fAcceptDatacarrier = DEFAULT_ACCEPT_DATACARRIER; unsigned nMaxDatacarrierBytes = MAX_OP_RETURN_RELAY; -CScriptID::CScriptID(const CScript& in) : uint160(Hash160(in.begin(), in.end())) {} +CScriptID::CScriptID(const CScript& in) : BaseHash(Hash160(in.begin(), in.end())) {} +CScriptID::CScriptID(const ScriptHash& in) : BaseHash(static_cast(in)) {} -ScriptHash::ScriptHash(const CScript& in) : uint160(Hash160(in.begin(), in.end())) {} +ScriptHash::ScriptHash(const CScript& in) : BaseHash(Hash160(in.begin(), in.end())) {} +ScriptHash::ScriptHash(const CScriptID& in) : BaseHash(static_cast(in)) {} -PKHash::PKHash(const CPubKey& pubkey) : uint160(pubkey.GetID()) {} +PKHash::PKHash(const CPubKey& pubkey) : BaseHash(pubkey.GetID()) {} +PKHash::PKHash(const CKeyID& pubkey_id) : BaseHash(pubkey_id) {} + +CKeyID ToKeyID(const PKHash& key_hash) +{ + return CKeyID{static_cast(key_hash)}; +} const char* GetTxnOutputType(TxoutType t) { diff --git a/src/script/standard.h b/src/script/standard.h index 71439b45d29f..1fe7f7b0f8bb 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -15,14 +15,77 @@ static const bool DEFAULT_ACCEPT_DATACARRIER = true; class CKeyID; class CScript; +struct ScriptHash; + +template +class BaseHash +{ +protected: + HashType m_hash; + +public: + BaseHash() : m_hash() {} + BaseHash(const HashType& in) : m_hash(in) {} + + unsigned char* begin() + { + return m_hash.begin(); + } + + const unsigned char* begin() const + { + return m_hash.begin(); + } + + unsigned char* end() + { + return m_hash.end(); + } + + const unsigned char* end() const + { + return m_hash.end(); + } + + operator std::vector() const + { + return std::vector{m_hash.begin(), m_hash.end()}; + } + + std::string ToString() const + { + return m_hash.ToString(); + } + + bool operator==(const BaseHash& other) const noexcept + { + return m_hash == other.m_hash; + } + + bool operator!=(const BaseHash& other) const noexcept + { + return !(m_hash == other.m_hash); + } + + bool operator<(const BaseHash& other) const noexcept + { + return m_hash < other.m_hash; + } + + size_t size() const + { + return m_hash.size(); + } +}; /** A reference to a CScript: the Hash160 of its serialization (see script.h) */ -class CScriptID : public uint160 +class CScriptID : public BaseHash { public: - CScriptID() : uint160() {} + CScriptID() : BaseHash() {} explicit CScriptID(const CScript& in); - CScriptID(const uint160& in) : uint160(in) {} + explicit CScriptID(const uint160& in) : BaseHash(in) {} + explicit CScriptID(const ScriptHash& in); }; /** @@ -67,20 +130,21 @@ class CNoDestination { friend bool operator<(const CNoDestination &a, const CNoDestination &b) { return true; } }; -struct PKHash : public uint160 +struct PKHash : public BaseHash { - PKHash() : uint160() {} - explicit PKHash(const uint160& hash) : uint160(hash) {} + PKHash() : BaseHash() {} + explicit PKHash(const uint160& hash) : BaseHash(hash) {} explicit PKHash(const CPubKey& pubkey); - using uint160::uint160; + explicit PKHash(const CKeyID& pubkey_id); }; +CKeyID ToKeyID(const PKHash& key_hash); -struct ScriptHash : public uint160 +struct ScriptHash : public BaseHash { - ScriptHash() : uint160() {} - explicit ScriptHash(const uint160& hash) : uint160(hash) {} + ScriptHash() : BaseHash() {} + explicit ScriptHash(const uint160& hash) : BaseHash(hash) {} explicit ScriptHash(const CScript& script); - using uint160::uint160; + explicit ScriptHash(const CScriptID& script); }; /** diff --git a/src/spork.cpp b/src/spork.cpp index ef27566ea48b..2daae12a01fb 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -286,7 +286,7 @@ bool CSporkManager::SetSporkAddress(const std::string& strAddress) LogPrintf("CSporkManager::SetSporkAddress -- Failed to parse spork address\n"); return false; } - setSporkPubKeyIDs.insert(CKeyID(*pkhash)); + setSporkPubKeyIDs.insert(ToKeyID(*pkhash)); return true; } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 8926e9135bd2..e90071ed0b88 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -145,7 +145,7 @@ UniValue importprivkey(const JSONRPCRequest& request) } // Use timestamp of 1 to scan the whole chain - if (!pwallet->ImportPrivKeys({{CKeyID(vchAddress), key}}, 1)) { + if (!pwallet->ImportPrivKeys({{ToKeyID(vchAddress), key}}, 1)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); } } @@ -829,7 +829,7 @@ UniValue dumpprivkey(const JSONRPCRequest& request) throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to a key"); } CKey vchSecret; - if (!spk_man.GetKey(CKeyID(*pkhash), vchSecret)) { + if (!spk_man.GetKey(ToKeyID(*pkhash), vchSecret)) { throw JSONRPCError(RPC_WALLET_ERROR, "Private key for address " + strAddress + " is not known"); } return EncodeSecret(vchSecret); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 454b0a7fd805..6fab10d8f578 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3564,7 +3564,7 @@ class DescribeWalletAddressVisitor UniValue operator()(const CNoDestination &dest) const { return UniValue(UniValue::VOBJ); } UniValue operator()(const PKHash& pkhash) const { - CKeyID keyID(pkhash); + CKeyID keyID{ToKeyID(pkhash)}; UniValue obj(UniValue::VOBJ); CPubKey vchPubKey; if (provider && provider->GetPubKey(keyID, vchPubKey)) { @@ -3727,7 +3727,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) CHDChain hdChainCurrent; LegacyScriptPubKeyMan* legacy_spk_man = pwallet->GetLegacyScriptPubKeyMan(); if (legacy_spk_man != nullptr) { - if (pkhash && legacy_spk_man->HaveHDKey(CKeyID(*pkhash), hdChainCurrent)) { + if (pkhash && legacy_spk_man->HaveHDKey(ToKeyID(*pkhash), hdChainCurrent)) { ret.pushKV("hdchainid", hdChainCurrent.GetID().GetHex()); } } diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index b947842532f3..bc3b6c7c322f 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -685,7 +685,7 @@ bool LegacyScriptPubKeyMan::SignTransaction(CMutableTransaction& tx, const std:: SigningResult LegacyScriptPubKeyMan::SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const { - CKeyID key_id(pkhash); + CKeyID key_id(ToKeyID(pkhash)); CKey key; if (!GetKey(key_id, key)) { return SigningResult::PRIVATE_KEY_NOT_AVAILABLE; @@ -744,8 +744,8 @@ const CKeyMetadata* LegacyScriptPubKeyMan::GetMetadata(const CTxDestination& des LOCK(cs_KeyStore); const PKHash *pkhash = std::get_if(&dest); - if (pkhash != nullptr && !pkhash->IsNull()) { - auto it = mapKeyMetadata.find(CKeyID(*pkhash)); + if (pkhash != nullptr && !ToKeyID(*pkhash).IsNull()) { + auto it = mapKeyMetadata.find(ToKeyID(*pkhash)); if (it != mapKeyMetadata.end()) { return &it->second; } From 459589552b626c512c18e5b3a5a7f34700a80e4e Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Thu, 6 Apr 2023 00:27:27 +0000 Subject: [PATCH 03/14] merge bitcoin#21523: run VerifyDB on all chainstates --- src/init.cpp | 7 +--- src/rpc/blockchain.cpp | 2 +- src/test/evo_deterministicmns_tests.cpp | 2 +- src/validation.cpp | 49 +++++++++++++++---------- src/validation.h | 10 ++++- 5 files changed, 43 insertions(+), 27 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index a342859805ea..00f8025e4a66 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2190,11 +2190,8 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc if (llmq::utils::IsV19Active(tip)) bls::bls_legacy_scheme.store(false); - // Only verify the DB of the active chainstate. This is fixed in later - // work when we allow VerifyDB to be parameterized by chainstate. - if (&::ChainstateActive() == chainstate && - !CVerifyDB().VerifyDB( - chainparams, *chainstate, &chainstate->CoinsDB(), + if (!CVerifyDB().VerifyDB( + *chainstate, chainparams, chainstate->CoinsDB(), *node.evodb, args.GetArg("-checklevel", DEFAULT_CHECKLEVEL), args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS))) { diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 108f8aa11f11..3a9101bbc537 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1415,7 +1415,7 @@ static UniValue verifychain(const JSONRPCRequest& request) const NodeContext& node_context = EnsureNodeContext(request.context); return CVerifyDB().VerifyDB( - Params(), ::ChainstateActive(), &::ChainstateActive().CoinsTip(), *node_context.evodb, check_level, check_depth); + ::ChainstateActive(), Params(), ::ChainstateActive().CoinsTip(), *node_context.evodb, check_level, check_depth); } /** Implementation of IsSuperMajority with better feedback */ diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index da8fef5f36fe..ba1a293db485 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -655,7 +655,7 @@ void FuncVerifyDB(TestChainSetup& setup) // Verify db consistency LOCK(cs_main); - BOOST_ASSERT(CVerifyDB().VerifyDB(Params(), ::ChainstateActive(), &::ChainstateActive().CoinsTip(), *(setup.m_node.evodb), 4, 2)); + BOOST_ASSERT(CVerifyDB().VerifyDB(::ChainstateActive(), Params(), ::ChainstateActive().CoinsTip(), *(setup.m_node.evodb), 4, 2)); } BOOST_AUTO_TEST_SUITE(evo_dip3_activation_tests) diff --git a/src/validation.cpp b/src/validation.cpp index bb3e5154fc72..d7c0bb06d949 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4738,41 +4738,50 @@ CVerifyDB::~CVerifyDB() uiInterface.ShowProgress("", 100, false); } -bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CChainState& active_chainstate, CCoinsView *coinsview, CEvoDB& evoDb, int nCheckLevel, int nCheckDepth) +bool CVerifyDB::VerifyDB( + CChainState& chainstate, + const CChainParams& chainparams, + CCoinsView& coinsview, + CEvoDB& evoDb, + int nCheckLevel, int nCheckDepth) { AssertLockHeld(cs_main); - assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate)); - if (active_chainstate.m_chain.Tip() == nullptr || active_chainstate.m_chain.Tip()->pprev == nullptr) + assert(std::addressof(::ChainstateActive()) == std::addressof(chainstate)); + if (chainstate.m_chain.Tip() == nullptr || chainstate.m_chain.Tip()->pprev == nullptr) return true; // begin tx and let it rollback auto dbTx = evoDb.BeginTransaction(); // Verify blocks in the best chain - if (nCheckDepth <= 0 || nCheckDepth > active_chainstate.m_chain.Height()) - nCheckDepth = active_chainstate.m_chain.Height(); + if (nCheckDepth <= 0 || nCheckDepth > chainstate.m_chain.Height()) + nCheckDepth = chainstate.m_chain.Height(); nCheckLevel = std::max(0, std::min(4, nCheckLevel)); LogPrintf("Verifying last %i blocks at level %i\n", nCheckDepth, nCheckLevel); - CCoinsViewCache coins(coinsview); + CCoinsViewCache coins(&coinsview); CBlockIndex* pindex; CBlockIndex* pindexFailure = nullptr; int nGoodTransactions = 0; CValidationState state; int reportDone = 0; LogPrintf("[0%%]..."); /* Continued */ - for (pindex = active_chainstate.m_chain.Tip(); pindex && pindex->pprev; pindex = pindex->pprev) { - const int percentageDone = std::max(1, std::min(99, (int)(((double)(active_chainstate.m_chain.Height() - pindex->nHeight)) / (double)nCheckDepth * (nCheckLevel >= 4 ? 50 : 100)))); + + bool is_snapshot_cs = !chainstate.m_from_snapshot_blockhash.IsNull(); + + for (pindex = chainstate.m_chain.Tip(); pindex && pindex->pprev; pindex = pindex->pprev) { + const int percentageDone = std::max(1, std::min(99, (int)(((double)(chainstate.m_chain.Height() - pindex->nHeight)) / (double)nCheckDepth * (nCheckLevel >= 4 ? 50 : 100)))); if (reportDone < percentageDone/10) { // report every 10% step LogPrintf("[%d%%]...", percentageDone); /* Continued */ reportDone = percentageDone/10; } uiInterface.ShowProgress(_("Verifying blocks...").translated, percentageDone, false); - if (pindex->nHeight <= active_chainstate.m_chain.Height()-nCheckDepth) + if (pindex->nHeight <= chainstate.m_chain.Height()-nCheckDepth) break; - if (fPruneMode && !(pindex->nStatus & BLOCK_HAVE_DATA)) { - // If pruning, only go back as far as we have data. + if ((fPruneMode || is_snapshot_cs) && !(pindex->nStatus & BLOCK_HAVE_DATA)) { + // If pruning or running under an assumeutxo snapshot, only go + // back as far as we have data. LogPrintf("VerifyDB(): block verification stopping at height %d (pruning, no data)\n", pindex->nHeight); break; } @@ -4794,9 +4803,11 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CChainState& active_ch } } // check level 3: check for inconsistencies during memory-only disconnect of tip blocks - if (nCheckLevel >= 3 && (coins.DynamicMemoryUsage() + active_chainstate.CoinsTip().DynamicMemoryUsage()) <= active_chainstate.m_coinstip_cache_size_bytes) { + size_t curr_coins_usage = coins.DynamicMemoryUsage() + chainstate.CoinsTip().DynamicMemoryUsage(); + + if (nCheckLevel >= 3 && curr_coins_usage <= chainstate.m_coinstip_cache_size_bytes) { assert(coins.GetBestBlock() == pindex->GetBlockHash()); - DisconnectResult res = active_chainstate.DisconnectBlock(block, pindex, coins); + DisconnectResult res = chainstate.DisconnectBlock(block, pindex, coins); if (res == DISCONNECT_FAILED) { return error("VerifyDB(): *** irrecoverable inconsistency in block data at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); } @@ -4810,26 +4821,26 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CChainState& active_ch if (ShutdownRequested()) return true; } if (pindexFailure) - return error("VerifyDB(): *** coin database inconsistencies found (last %i blocks, %i good transactions before that)\n", active_chainstate.m_chain.Height() - pindexFailure->nHeight + 1, nGoodTransactions); + return error("VerifyDB(): *** coin database inconsistencies found (last %i blocks, %i good transactions before that)\n", chainstate.m_chain.Height() - pindexFailure->nHeight + 1, nGoodTransactions); // store block count as we move pindex at check level >= 4 - int block_count = active_chainstate.m_chain.Height() - pindex->nHeight; + int block_count = chainstate.m_chain.Height() - pindex->nHeight; // check level 4: try reconnecting blocks if (nCheckLevel >= 4) { - while (pindex != active_chainstate.m_chain.Tip()) { - const int percentageDone = std::max(1, std::min(99, 100 - (int)(((double)(active_chainstate.m_chain.Height() - pindex->nHeight)) / (double)nCheckDepth * 50))); + while (pindex != chainstate.m_chain.Tip()) { + const int percentageDone = std::max(1, std::min(99, 100 - (int)(((double)(chainstate.m_chain.Height() - pindex->nHeight)) / (double)nCheckDepth * 50))); if (reportDone < percentageDone/10) { // report every 10% step LogPrintf("[%d%%]...", percentageDone); /* Continued */ reportDone = percentageDone/10; } uiInterface.ShowProgress(_("Verifying blocks...").translated, percentageDone, false); - pindex = active_chainstate.m_chain.Next(pindex); + pindex = chainstate.m_chain.Next(pindex); CBlock block; if (!ReadBlockFromDisk(block, pindex, chainparams.GetConsensus())) return error("VerifyDB(): *** ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); - if (!active_chainstate.ConnectBlock(block, state, pindex, coins, chainparams)) + if (!chainstate.ConnectBlock(block, state, pindex, coins, chainparams)) return error("VerifyDB(): *** found unconnectable block at %d, hash=%s (%s)", pindex->nHeight, pindex->GetBlockHash().ToString(), FormatStateMessage(state)); if (ShutdownRequested()) return true; } diff --git a/src/validation.h b/src/validation.h index 0164b4ba9803..db9fba1f5222 100644 --- a/src/validation.h +++ b/src/validation.h @@ -333,7 +333,13 @@ class CVerifyDB { public: CVerifyDB(); ~CVerifyDB(); - bool VerifyDB(const CChainParams& chainparams, CChainState& active_chainstate, CCoinsView *coinsview, CEvoDB& evoDb, int nCheckLevel, int nCheckDepth) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool VerifyDB( + CChainState& chainstate, + const CChainParams& chainparams, + CCoinsView& coinsview, + CEvoDB& evoDb, + int nCheckLevel, + int nCheckDepth) EXCLUSIVE_LOCKS_REQUIRED(cs_main); }; enum DisconnectResult { @@ -928,6 +934,8 @@ class ChainstateManager return m_blockman.m_prev_block_index; } + //! @returns true if a snapshot-based chainstate is in use. Also implies + //! that a background validation chainstate is also in use. bool IsSnapshotActive() const; std::optional SnapshotBlockhash() const; From a262bb888b5b04eef925e5c8ef6b6e9ecd2ad06f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Sun, 12 Mar 2023 21:09:58 +0000 Subject: [PATCH 04/14] merge bitcoin#17954: Remove calls to Chain::Lock methods --- src/Makefile.test.include | 1 + src/interfaces/chain.cpp | 119 +++++++++++++--------- src/interfaces/chain.h | 75 ++++++++++---- src/interfaces/wallet.cpp | 12 +-- src/qt/test/wallettests.cpp | 2 +- src/sync.h | 2 +- src/test/interfaces_tests.cpp | 157 +++++++++++++++++++++++++++++ src/wallet/rpcdump.cpp | 41 ++++---- src/wallet/rpcwallet.cpp | 49 ++++----- src/wallet/test/coinjoin_tests.cpp | 2 +- src/wallet/test/wallet_tests.cpp | 23 +++-- src/wallet/wallet.cpp | 130 ++++++++++++------------ src/wallet/wallet.h | 8 +- 13 files changed, 419 insertions(+), 202 deletions(-) create mode 100644 src/test/interfaces_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index b8e5f98e5f68..4d1b921ba174 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -105,6 +105,7 @@ BITCOIN_TESTS =\ test/getarg_tests.cpp \ test/governance_validators_tests.cpp \ test/hash_tests.cpp \ + test/interfaces_tests.cpp \ test/key_io_tests.cpp \ test/key_tests.cpp \ test/lcg.h \ diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 4795e5b9d4f1..c80091d28a07 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -36,6 +36,22 @@ namespace interfaces { namespace { + +bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock& lock) +{ + if (!index) return false; + if (block.m_hash) *block.m_hash = index->GetBlockHash(); + if (block.m_height) *block.m_height = index->nHeight; + if (block.m_time) *block.m_time = index->GetBlockTime(); + if (block.m_max_time) *block.m_max_time = index->GetBlockTimeMax(); + if (block.m_mtp_time) *block.m_mtp_time = index->GetMedianTimePast(); + if (block.m_data) { + REVERSE_LOCK(lock); + if (!ReadBlockFromDisk(*block.m_data, index, Params().GetConsensus())) block.m_data->SetNull(); + } + return true; +} + class NotificationsProxy : public CValidationInterface { public: @@ -162,20 +178,6 @@ class ChainImpl : public Chain assert(block != nullptr); return block->GetBlockHash(); } - int64_t getBlockTime(int height) override - { - LOCK(cs_main); - CBlockIndex* block = ::ChainActive()[height]; - assert(block != nullptr); - return block->GetBlockTime(); - } - int64_t getBlockMedianTimePast(int height) override - { - LOCK(cs_main); - CBlockIndex* block = ::ChainActive()[height]; - assert(block != nullptr); - return block->GetMedianTimePast(); - } bool haveBlockOnDisk(int height) override { LOCK(cs_main); @@ -192,20 +194,6 @@ class ChainImpl : public Chain } return std::nullopt; } - std::optional findPruned(int start_height, std::optional stop_height) override - { - LOCK(cs_main); - if (::fPruneMode) { - CBlockIndex* block = stop_height ? ::ChainActive()[*stop_height] : ::ChainActive().Tip(); - while (block && block->nHeight >= start_height) { - if ((block->nStatus & BLOCK_HAVE_DATA) == 0) { - return block->nHeight; - } - block = block->pprev; - } - } - return std::nullopt; - } std::optional findFork(const uint256& hash, std::optional* height) override { LOCK(cs_main); @@ -241,26 +229,48 @@ class ChainImpl : public Chain LOCK(cs_main); return CheckFinalTx(::ChainActive().Tip(), tx); } - bool findBlock(const uint256& hash, CBlock* block, int64_t* time, int64_t* time_max) override + bool findBlock(const uint256& hash, const FoundBlock& block) override { - CBlockIndex* index; - { - LOCK(cs_main); - index = g_chainman.m_blockman.LookupBlockIndex(hash); - if (!index) { - return false; - } - if (time) { - *time = index->GetBlockTime(); - } - if (time_max) { - *time_max = index->GetBlockTimeMax(); + WAIT_LOCK(cs_main, lock); + return FillBlock(g_chainman.m_blockman.LookupBlockIndex(hash), block, lock); + } + bool findFirstBlockWithTimeAndHeight(int64_t min_time, int min_height, const FoundBlock& block) override + { + WAIT_LOCK(cs_main, lock); + return FillBlock(ChainActive().FindEarliestAtLeast(min_time, min_height), block, lock); + } + bool findNextBlock(const uint256& block_hash, int block_height, const FoundBlock& next, bool* reorg) override { + WAIT_LOCK(cs_main, lock); + CBlockIndex* block = ChainActive()[block_height]; + if (block && block->GetBlockHash() != block_hash) block = nullptr; + if (reorg) *reorg = !block; + return FillBlock(block ? ChainActive()[block_height + 1] : nullptr, next, lock); + } + bool findAncestorByHeight(const uint256& block_hash, int ancestor_height, const FoundBlock& ancestor_out) override + { + WAIT_LOCK(cs_main, lock); + if (const CBlockIndex* block = g_chainman.m_blockman.LookupBlockIndex(block_hash)) { + if (const CBlockIndex* ancestor = block->GetAncestor(ancestor_height)) { + return FillBlock(ancestor, ancestor_out, lock); } } - if (block && !ReadBlockFromDisk(*block, index, Params().GetConsensus())) { - block->SetNull(); - } - return true; + return FillBlock(nullptr, ancestor_out, lock); + } + bool findAncestorByHash(const uint256& block_hash, const uint256& ancestor_hash, const FoundBlock& ancestor_out) override + { + WAIT_LOCK(cs_main, lock); + const CBlockIndex* block = g_chainman.m_blockman.LookupBlockIndex(block_hash); + const CBlockIndex* ancestor = g_chainman.m_blockman.LookupBlockIndex(ancestor_hash); + if (block && ancestor && block->GetAncestor(ancestor->nHeight) != ancestor) ancestor = nullptr; + return FillBlock(ancestor, ancestor_out, lock); + } + bool findCommonAncestor(const uint256& block_hash1, const uint256& block_hash2, const FoundBlock& ancestor_out, const FoundBlock& block1_out, const FoundBlock& block2_out) override + { + WAIT_LOCK(cs_main, lock); + const CBlockIndex* block1 = g_chainman.m_blockman.LookupBlockIndex(block_hash1); + const CBlockIndex* block2 = g_chainman.m_blockman.LookupBlockIndex(block_hash2); + const CBlockIndex* ancestor = block1 && block2 ? LastCommonAncestor(block1, block2) : nullptr; + return FillBlock(ancestor, ancestor_out, lock) & FillBlock(block1, block1_out, lock) & FillBlock(block2, block2_out, lock); } void findCoins(std::map& coins) override { return FindCoins(m_node, coins); } double guessVerificationProgress(const uint256& block_hash) override @@ -268,6 +278,25 @@ class ChainImpl : public Chain LOCK(cs_main); return GuessVerificationProgress(Params().TxData(), g_chainman.m_blockman.LookupBlockIndex(block_hash)); } + bool hasBlocks(const uint256& block_hash, int min_height, std::optional max_height) override + { + // hasBlocks returns true if all ancestors of block_hash in specified + // range have block data (are not pruned), false if any ancestors in + // specified range are missing data. + // + // For simplicity and robustness, min_height and max_height are only + // used to limit the range, and passing min_height that's too low or + // max_height that's too high will not crash or change the result. + LOCK(::cs_main); + if (CBlockIndex* block = g_chainman.m_blockman.LookupBlockIndex(block_hash)) { + if (max_height && block->nHeight >= *max_height) block = block->GetAncestor(*max_height); + for (; block->nStatus & BLOCK_HAVE_DATA; block = block->pprev) { + // Check pprev to not segfault if min_height is too low + if (block->nHeight <= min_height || !block->pprev) return true; + } + } + return false; + } bool hasDescendantsInMempool(const uint256& txid) override { if (!m_node.mempool) return false; diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 10282086975d..2f7bcac398db 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -46,6 +46,27 @@ namespace interfaces { class Wallet; class Handler; +//! Helper for findBlock to selectively return pieces of block data. +class FoundBlock +{ +public: + FoundBlock& hash(uint256& hash) { m_hash = &hash; return *this; } + FoundBlock& height(int& height) { m_height = &height; return *this; } + FoundBlock& time(int64_t& time) { m_time = &time; return *this; } + FoundBlock& maxTime(int64_t& max_time) { m_max_time = &max_time; return *this; } + FoundBlock& mtpTime(int64_t& mtp_time) { m_mtp_time = &mtp_time; return *this; } + //! Read block data from disk. If the block exists but doesn't have data + //! (for example due to pruning), the CBlock variable will be set to null. + FoundBlock& data(CBlock& data) { m_data = &data; return *this; } + + uint256* m_hash = nullptr; + int* m_height = nullptr; + int64_t* m_time = nullptr; + int64_t* m_max_time = nullptr; + int64_t* m_mtp_time = nullptr; + CBlock* m_data = nullptr; +}; + //! Interface giving clients (wallet processes, maybe other analysis tools in //! the future) ability to access to the chain state, receive notifications, //! estimate fees, and submit transactions. @@ -88,13 +109,6 @@ class Chain //! Get block hash. Height must be valid or this function will abort. virtual uint256 getBlockHash(int height) = 0; - //! Get block time. Height must be valid or this function will abort. - virtual int64_t getBlockTime(int height) = 0; - - //! Get block median time past. Height must be valid or this function - //! will abort. - virtual int64_t getBlockMedianTimePast(int height) = 0; - //! Check that the block is available on disk (i.e. has not been //! pruned), and contains transactions. virtual bool haveBlockOnDisk(int height) = 0; @@ -106,10 +120,6 @@ class Chain //! (to avoid the cost of a second lookup in case this information is needed.) virtual std::optional findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) = 0; - //! Return height of last block in the specified range which is pruned, or - //! nullopt if no block in the range is pruned. Range is inclusive. - virtual std::optional findPruned(int start_height = 0, std::optional stop_height = std::nullopt) = 0; - //! Return height of the specified block if it is on the chain, otherwise //! return the height of the highest block on chain that's an ancestor //! of the specified block, or nullopt if there is no common ancestor. @@ -131,14 +141,36 @@ class Chain //! Return whether node has the block and optionally return block metadata //! or contents. - //! - //! If a block pointer is provided to retrieve the block contents, and the - //! block exists but doesn't have data (for example due to pruning), the - //! block will be empty and all fields set to null. - virtual bool findBlock(const uint256& hash, - CBlock* block = nullptr, - int64_t* time = nullptr, - int64_t* max_time = nullptr) = 0; + virtual bool findBlock(const uint256& hash, const FoundBlock& block={}) = 0; + + //! Find first block in the chain with timestamp >= the given time + //! and height >= than the given height, return false if there is no block + //! with a high enough timestamp and height. Optionally return block + //! information. + virtual bool findFirstBlockWithTimeAndHeight(int64_t min_time, int min_height, const FoundBlock& block={}) = 0; + + //! Find next block if block is part of current chain. Also flag if + //! there was a reorg and the specified block hash is no longer in the + //! current chain, and optionally return block information. + virtual bool findNextBlock(const uint256& block_hash, int block_height, const FoundBlock& next={}, bool* reorg=nullptr) = 0; + + //! Find ancestor of block at specified height and optionally return + //! ancestor information. + virtual bool findAncestorByHeight(const uint256& block_hash, int ancestor_height, const FoundBlock& ancestor_out={}) = 0; + + //! Return whether block descends from a specified ancestor, and + //! optionally return ancestor information. + virtual bool findAncestorByHash(const uint256& block_hash, + const uint256& ancestor_hash, + const FoundBlock& ancestor_out={}) = 0; + + //! Find most recent common ancestor between two blocks and optionally + //! return block information. + virtual bool findCommonAncestor(const uint256& block_hash1, + const uint256& block_hash2, + const FoundBlock& ancestor_out={}, + const FoundBlock& block1_out={}, + const FoundBlock& block2_out={}) = 0; //! Look up unspent output information. Returns coins in the mempool and in //! the current chain UTXO set. Iterates through all the keys in the map and @@ -149,6 +181,11 @@ class Chain //! the specified block hash are verified. virtual double guessVerificationProgress(const uint256& block_hash) = 0; + //! Return true if data is available for all blocks in the specified range + //! of blocks. This checks all blocks that are ancestors of block_hash in + //! the height range from min_height to max_height, inclusive. + virtual bool hasBlocks(const uint256& block_hash, int min_height = 0, std::optional max_height = {}) = 0; + //! Check if transaction has descendants in mempool. virtual bool hasDescendantsInMempool(const uint256& txid) = 0; diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 815e32946c5f..944b24293823 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -77,7 +78,7 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) WalletTxStatus MakeWalletTxStatus(const CWallet& wallet, const CWalletTx& wtx) { WalletTxStatus result; - result.block_height = wallet.chain().getBlockHeight(wtx.m_confirm.hashBlock).value_or(std::numeric_limits::max()); + result.block_height = wtx.m_confirm.block_height > 0 ? wtx.m_confirm.block_height : std::numeric_limits::max(); result.blocks_to_maturity = wtx.GetBlocksToMaturity(); result.depth_in_main_chain = wtx.GetDepthInMainChain(); result.time_received = wtx.nTimeReceived; @@ -367,11 +368,8 @@ class WalletImpl : public Wallet if (mi == m_wallet->mapWallet.end()) { return false; } - if (std::optional height = m_wallet->chain().getHeight()) { - block_time = m_wallet->chain().getBlockTime(*height); - } else { - block_time = -1; - } + block_time = -1; + CHECK_NONFATAL(m_wallet->chain().findBlock(m_wallet->GetLastBlockHash(), FoundBlock().time(block_time))); tx_status = MakeWalletTxStatus(*m_wallet, mi->second); return true; } @@ -425,8 +423,8 @@ class WalletImpl : public Wallet if (!locked_wallet) { return false; } + num_blocks = m_wallet->GetLastBlockHeight(); balances = getBalances(); - num_blocks = m_wallet->chain().getHeight().value_or(-1); return true; } CAmount getBalance() override diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 470d74b8426d..f944c2c366ea 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -123,7 +123,7 @@ void TestGUI(interfaces::Node& node) { WalletRescanReserver reserver(wallet.get()); reserver.reserve(); - CWallet::ScanResult result = wallet->ScanForWalletTransactions(wallet->chain().getBlockHash(0), {} /* stop_block */, reserver, true /* fUpdate */); + CWallet::ScanResult result = wallet->ScanForWalletTransactions(Params().GetConsensus().hashGenesisBlock, 0 /* start_height */, {} /* max_height */, reserver, true /* fUpdate */); QCOMPARE(result.status, CWallet::ScanResult::SUCCESS); QCOMPARE(result.last_scanned_block, ::ChainActive().Tip()->GetBlockHash()); QVERIFY(result.last_failed_block.IsNull()); diff --git a/src/sync.h b/src/sync.h index 880ce460ca36..a6510f2c2842 100644 --- a/src/sync.h +++ b/src/sync.h @@ -221,7 +221,7 @@ class SCOPED_LOCKABLE UniqueLock : public Base friend class reverse_lock; }; -#define REVERSE_LOCK(g) decltype(g)::reverse_lock PASTE2(revlock, __COUNTER__)(g, #g, __FILE__, __LINE__) +#define REVERSE_LOCK(g) typename std::decay::type::reverse_lock PASTE2(revlock, __COUNTER__)(g, #g, __FILE__, __LINE__) template using DebugLock = UniqueLock::type>::type>; diff --git a/src/test/interfaces_tests.cpp b/src/test/interfaces_tests.cpp new file mode 100644 index 000000000000..de2ad2a580bc --- /dev/null +++ b/src/test/interfaces_tests.cpp @@ -0,0 +1,157 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include