diff options
28 files changed, 570 insertions, 533 deletions
diff --git a/contrib/verifybinaries/README.md b/contrib/verifybinaries/README.md index 4209fdb364..c50d4bef71 100644 --- a/contrib/verifybinaries/README.md +++ b/contrib/verifybinaries/README.md @@ -21,21 +21,21 @@ The script returns 0 if everything passes the checks. It returns 1 if either the ```sh -./verify.sh bitcoin-core-0.11.2 -./verify.sh bitcoin-core-0.12.0 -./verify.sh bitcoin-core-0.13.0-rc3 +./verify.py bitcoin-core-0.11.2 +./verify.py bitcoin-core-0.12.0 +./verify.py bitcoin-core-0.13.0-rc3 ``` If you only want to download the binaries of certain platform, add the corresponding suffix, e.g.: ```sh -./verify.sh bitcoin-core-0.11.2-osx -./verify.sh 0.12.0-linux -./verify.sh bitcoin-core-0.13.0-rc3-win64 +./verify.py bitcoin-core-0.11.2-osx +./verify.py 0.12.0-linux +./verify.py bitcoin-core-0.13.0-rc3-win64 ``` If you do not want to keep the downloaded binaries, specify anything as the second parameter. ```sh -./verify.sh bitcoin-core-0.13.0 delete +./verify.py bitcoin-core-0.13.0 delete ``` diff --git a/contrib/verifybinaries/verify.py b/contrib/verifybinaries/verify.py new file mode 100755 index 0000000000..b97ad11b8e --- /dev/null +++ b/contrib/verifybinaries/verify.py @@ -0,0 +1,183 @@ +#!/usr/bin/env python3 +# 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. +"""Script for verifying Bitoin Core release binaries + +This script attempts to download the signature file SHA256SUMS.asc from +bitcoincore.org and bitcoin.org and compares them. +It first checks if the signature passes, and then downloads the files +specified in the file, and checks if the hashes of these files match those +that are specified in the signature file. +The script returns 0 if everything passes the checks. It returns 1 if either +the signature check or the hash check doesn't pass. If an error occurs the +return value is >= 2. +""" +from hashlib import sha256 +import os +import subprocess +import sys +from textwrap import indent + +WORKINGDIR = "/tmp/bitcoin_verify_binaries" +HASHFILE = "hashes.tmp" +HOST1 = "https://bitcoincore.org" +HOST2 = "https://bitcoin.org" +VERSIONPREFIX = "bitcoin-core-" +SIGNATUREFILENAME = "SHA256SUMS.asc" + + +def parse_version_string(version_str): + if version_str.startswith(VERSIONPREFIX): # remove version prefix + version_str = version_str[len(VERSIONPREFIX):] + + parts = version_str.split('-') + version_base = parts[0] + version_rc = "" + version_os = "" + if len(parts) == 2: # "<version>-rcN" or "version-platform" + if "rc" in parts[1]: + version_rc = parts[1] + else: + version_os = parts[1] + elif len(parts) == 3: # "<version>-rcN-platform" + version_rc = parts[1] + version_os = parts[2] + + return version_base, version_rc, version_os + + +def download_with_wget(remote_file, local_file=None): + if local_file: + wget_args = ['wget', '-O', local_file, remote_file] + else: + # use timestamping mechanism if local filename is not explicitely set + wget_args = ['wget', '-N', remote_file] + + result = subprocess.run(wget_args, + stderr=subprocess.STDOUT, stdout=subprocess.PIPE) + return result.returncode == 0, result.stdout.decode().rstrip() + + +def files_are_equal(filename1, filename2): + with open(filename1, 'rb') as file1: + contents1 = file1.read() + with open(filename2, 'rb') as file2: + contents2 = file2.read() + return contents1 == contents2 + + +def verify_with_gpg(signature_filename, output_filename): + result = subprocess.run(['gpg', '--yes', '--decrypt', '--output', + output_filename, signature_filename], + stderr=subprocess.STDOUT, stdout=subprocess.PIPE) + return result.returncode, result.stdout.decode().rstrip() + + +def remove_files(filenames): + for filename in filenames: + os.remove(filename) + + +def main(args): + # sanity check + if len(args) < 1: + print("Error: need to specify a version on the command line") + return 3 + + # determine remote dir dependend on provided version string + version_base, version_rc, os_filter = parse_version_string(args[0]) + remote_dir = f"/bin/{VERSIONPREFIX}{version_base}/" + if version_rc: + remote_dir += f"test.{version_rc}/" + remote_sigfile = remote_dir + SIGNATUREFILENAME + + # create working directory + os.makedirs(WORKINGDIR, exist_ok=True) + os.chdir(WORKINGDIR) + + # fetch first signature file + sigfile1 = SIGNATUREFILENAME + success, output = download_with_wget(HOST1 + remote_sigfile, sigfile1) + if not success: + print("Error: couldn't fetch signature file. " + "Have you specified the version number in the following format?") + print(f"[{VERSIONPREFIX}]<version>[-rc[0-9]][-platform] " + f"(example: {VERSIONPREFIX}0.21.0-rc3-osx)") + print("wget output:") + print(indent(output, '\t')) + return 4 + + # fetch second signature file + sigfile2 = SIGNATUREFILENAME + ".2" + success, output = download_with_wget(HOST2 + remote_sigfile, sigfile2) + if not success: + print("bitcoin.org failed to provide signature file, " + "but bitcoincore.org did?") + print("wget output:") + print(indent(output, '\t')) + remove_files([sigfile1]) + return 5 + + # ensure that both signature files are equal + if not files_are_equal(sigfile1, sigfile2): + print("bitcoin.org and bitcoincore.org signature files were not equal?") + print(f"See files {WORKINGDIR}/{sigfile1} and {WORKINGDIR}/{sigfile2}") + return 6 + + # check signature and extract data into file + retval, output = verify_with_gpg(sigfile1, HASHFILE) + if retval != 0: + if retval == 1: + print("Bad signature.") + elif retval == 2: + print("gpg error. Do you have the Bitcoin Core binary release " + "signing key installed?") + print("gpg output:") + print(indent(output, '\t')) + remove_files([sigfile1, sigfile2, HASHFILE]) + return 1 + + # extract hashes/filenames of binaries to verify from hash file; + # each line has the following format: "<hash> <binary_filename>" + with open(HASHFILE, 'r', encoding='utf8') as hash_file: + hashes_to_verify = [ + line.split()[:2] for line in hash_file if os_filter in line] + remove_files([HASHFILE]) + if not hashes_to_verify: + print("error: no files matched the platform specified") + return 7 + + # download binaries + for _, binary_filename in hashes_to_verify: + print(f"Downloading {binary_filename}") + download_with_wget(HOST1 + remote_dir + binary_filename) + + # verify hashes + offending_files = [] + for hash_expected, binary_filename in hashes_to_verify: + with open(binary_filename, 'rb') as binary_file: + hash_calculated = sha256(binary_file.read()).hexdigest() + if hash_calculated != hash_expected: + offending_files.append(binary_filename) + if offending_files: + print("Hashes don't match.") + print("Offending files:") + print('\n'.join(offending_files)) + return 1 + verified_binaries = [entry[1] for entry in hashes_to_verify] + + # clean up files if desired + if len(args) >= 2: + print("Clean up the binaries") + remove_files([sigfile1, sigfile2] + verified_binaries) + else: + print(f"Keep the binaries in {WORKINGDIR}") + + print("Verified hashes of") + print('\n'.join(verified_binaries)) + return 0 + + +if __name__ == '__main__': + sys.exit(main(sys.argv[1:])) diff --git a/contrib/verifybinaries/verify.sh b/contrib/verifybinaries/verify.sh deleted file mode 100755 index 4296998631..0000000000 --- a/contrib/verifybinaries/verify.sh +++ /dev/null @@ -1,177 +0,0 @@ -#!/usr/bin/env bash -# Copyright (c) 2016-2019 The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. - -### This script attempts to download the signature file SHA256SUMS.asc from -### bitcoincore.org and bitcoin.org and compares them. -### It first checks if the signature passes, and then downloads the files specified in -### the file, and checks if the hashes of these files match those that are specified -### in the signature file. -### The script returns 0 if everything passes the checks. It returns 1 if either the -### signature check or the hash check doesn't pass. If an error occurs the return value is 2 - -export LC_ALL=C -function clean_up { - for file in "$@" - do - rm "$file" 2> /dev/null - done -} - -WORKINGDIR="/tmp/bitcoin_verify_binaries" -TMPFILE="hashes.tmp" - -SIGNATUREFILENAME="SHA256SUMS.asc" -RCSUBDIR="test" -HOST1="https://bitcoincore.org" -HOST2="https://bitcoin.org" -BASEDIR="/bin/" -VERSIONPREFIX="bitcoin-core-" -RCVERSIONSTRING="rc" - -if [ ! -d "$WORKINGDIR" ]; then - mkdir "$WORKINGDIR" -fi - -cd "$WORKINGDIR" || exit 1 - -#test if a version number has been passed as an argument -if [ -n "$1" ]; then - #let's also check if the version number includes the prefix 'bitcoin-', - # and add this prefix if it doesn't - if [[ $1 == "$VERSIONPREFIX"* ]]; then - VERSION="$1" - else - VERSION="$VERSIONPREFIX$1" - fi - - STRIPPEDLAST="${VERSION%-*}" - - #now let's see if the version string contains "rc" or a platform name (e.g. "osx") - if [[ "$STRIPPEDLAST-" == "$VERSIONPREFIX" ]]; then - BASEDIR="$BASEDIR$VERSION/" - else - # let's examine the last part to see if it's rc and/or platform name - STRIPPEDNEXTTOLAST="${STRIPPEDLAST%-*}" - if [[ "$STRIPPEDNEXTTOLAST-" == "$VERSIONPREFIX" ]]; then - - LASTSUFFIX="${VERSION##*-}" - VERSION="$STRIPPEDLAST" - - if [[ $LASTSUFFIX == *"$RCVERSIONSTRING"* ]]; then - RCVERSION="$LASTSUFFIX" - else - PLATFORM="$LASTSUFFIX" - fi - - else - RCVERSION="${STRIPPEDLAST##*-}" - PLATFORM="${VERSION##*-}" - - VERSION="$STRIPPEDNEXTTOLAST" - fi - - BASEDIR="$BASEDIR$VERSION/" - if [[ $RCVERSION == *"$RCVERSIONSTRING"* ]]; then - BASEDIR="$BASEDIR$RCSUBDIR.$RCVERSION/" - fi - fi -else - echo "Error: need to specify a version on the command line" - exit 2 -fi - -if ! WGETOUT=$(wget -N "$HOST1$BASEDIR$SIGNATUREFILENAME" 2>&1); then - echo "Error: couldn't fetch signature file. Have you specified the version number in the following format?" - # shellcheck disable=SC1087 - echo "[$VERSIONPREFIX]<version>-[$RCVERSIONSTRING[0-9]] (example: ${VERSIONPREFIX}0.10.4-${RCVERSIONSTRING}1)" - echo "wget output:" - # shellcheck disable=SC2001 - echo "$WGETOUT"|sed 's/^/\t/g' - exit 2 -fi - -if ! WGETOUT=$(wget -N -O "$SIGNATUREFILENAME.2" "$HOST2$BASEDIR$SIGNATUREFILENAME" 2>&1); then - echo "bitcoin.org failed to provide signature file, but bitcoincore.org did?" - echo "wget output:" - # shellcheck disable=SC2001 - echo "$WGETOUT"|sed 's/^/\t/g' - clean_up $SIGNATUREFILENAME - exit 3 -fi - -SIGFILEDIFFS="$(diff $SIGNATUREFILENAME $SIGNATUREFILENAME.2)" -if [ "$SIGFILEDIFFS" != "" ]; then - echo "bitcoin.org and bitcoincore.org signature files were not equal?" - clean_up $SIGNATUREFILENAME $SIGNATUREFILENAME.2 - exit 4 -fi - -#then we check it -GPGOUT=$(gpg --yes --decrypt --output "$TMPFILE" "$SIGNATUREFILENAME" 2>&1) - -#return value 0: good signature -#return value 1: bad signature -#return value 2: gpg error - -RET="$?" -if [ $RET -ne 0 ]; then - if [ $RET -eq 1 ]; then - #and notify the user if it's bad - echo "Bad signature." - elif [ $RET -eq 2 ]; then - #or if a gpg error has occurred - echo "gpg error. Do you have the Bitcoin Core binary release signing key installed?" - fi - - echo "gpg output:" - # shellcheck disable=SC2001 - echo "$GPGOUT"|sed 's/^/\t/g' - clean_up $SIGNATUREFILENAME $SIGNATUREFILENAME.2 $TMPFILE - exit "$RET" -fi - -if [ -n "$PLATFORM" ]; then - grep $PLATFORM $TMPFILE > "$TMPFILE-plat" - TMPFILESIZE=$(stat -c%s "$TMPFILE-plat") - if [ $TMPFILESIZE -eq 0 ]; then - echo "error: no files matched the platform specified" && exit 3 - fi - mv "$TMPFILE-plat" $TMPFILE -fi - -#here we extract the filenames from the signature file -FILES=$(awk '{print $2}' "$TMPFILE") - -#and download these one by one -for file in $FILES -do - echo "Downloading $file" - wget --quiet -N "$HOST1$BASEDIR$file" -done - -#check hashes -DIFF=$(diff <(sha256sum $FILES) "$TMPFILE") - -if [ $? -eq 1 ]; then - echo "Hashes don't match." - echo "Offending files:" - echo "$DIFF"|grep "^<"|awk '{print "\t"$3}' - exit 1 -elif [ $? -gt 1 ]; then - echo "Error executing 'diff'" - exit 2 -fi - -if [ -n "$2" ]; then - echo "Clean up the binaries" - clean_up $FILES $SIGNATUREFILENAME $SIGNATUREFILENAME.2 $TMPFILE -else - echo "Keep the binaries in $WORKINGDIR" - clean_up $TMPFILE -fi - -echo -e "Verified hashes of \n$FILES" - -exit 0 diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index e50c4476bb..3abfbfd784 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -42,20 +42,19 @@ static void CoinSelection(benchmark::Bench& bench) } addCoin(3 * COIN, wallet, wtxs); - // Create groups - std::vector<OutputGroup> groups; + // Create coins + std::vector<COutput> coins; for (const auto& wtx : wtxs) { - COutput output(wtx.get(), 0 /* iIn */, 6 * 24 /* nDepthIn */, true /* spendable */, true /* solvable */, true /* safe */); - groups.emplace_back(output.GetInputCoin(), 6, false, 0, 0); + coins.emplace_back(wtx.get(), 0 /* iIn */, 6 * 24 /* nDepthIn */, true /* spendable */, true /* solvable */, true /* safe */); } const CoinEligibilityFilter filter_standard(1, 6, 0); - const CoinSelectionParams coin_selection_params(true, 34, 148, CFeeRate(0), 0); + const CoinSelectionParams coin_selection_params(true, 34, 148, CFeeRate(0), 0, false); bench.run([&] { std::set<CInputCoin> setCoinsRet; CAmount nValueRet; bool bnb_used; - bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used); + bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used); assert(success); assert(nValueRet == 1003 * COIN); assert(setCoinsRet.size() == 2); @@ -75,7 +74,8 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector<OutputGroup> tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; std::unique_ptr<CWalletTx> wtx = MakeUnique<CWalletTx>(&testWallet, MakeTransactionRef(std::move(tx))); - set.emplace_back(COutput(wtx.get(), nInput, 0, true, true, true).GetInputCoin(), 0, true, 0, 0); + set.emplace_back(); + set.back().Insert(COutput(wtx.get(), nInput, 0, true, true, true).GetInputCoin(), 0, true, 0, 0, false); wtxn.emplace_back(std::move(wtx)); } // Copied from src/wallet/test/coinselector_tests.cpp diff --git a/src/index/base.cpp b/src/index/base.cpp index e67b813763..3d3dda95b1 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -62,7 +62,7 @@ bool BaseIndex::Init() if (locator.IsNull()) { m_best_block_index = nullptr; } else { - m_best_block_index = FindForkInGlobalIndex(::ChainActive(), locator); + m_best_block_index = g_chainman.m_blockman.FindForkInGlobalIndex(::ChainActive(), locator); } m_synced = m_best_block_index.load() == ::ChainActive().Tip(); return true; @@ -239,7 +239,7 @@ void BaseIndex::ChainStateFlushed(const CBlockLocator& locator) const CBlockIndex* locator_tip_index; { LOCK(cs_main); - locator_tip_index = LookupBlockIndex(locator_tip_hash); + locator_tip_index = g_chainman.m_blockman.LookupBlockIndex(locator_tip_hash); } if (!locator_tip_index) { diff --git a/src/init.cpp b/src/init.cpp index e01d16e133..716c06cd3a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -702,7 +702,7 @@ static void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImp if (!file) break; // This error is logged in OpenBlockFile LogPrintf("Reindexing block file blk%05u.dat...\n", (unsigned int)nFile); - LoadExternalBlockFile(chainparams, file, &pos); + ::ChainstateActive().LoadExternalBlockFile(chainparams, file, &pos); if (ShutdownRequested()) { LogPrintf("Shutdown requested. Exit %s\n", __func__); return; @@ -721,7 +721,7 @@ static void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImp FILE *file = fsbridge::fopen(path, "rb"); if (file) { LogPrintf("Importing blocks file %s...\n", path.string()); - LoadExternalBlockFile(chainparams, file); + ::ChainstateActive().LoadExternalBlockFile(chainparams, file); if (ShutdownRequested()) { LogPrintf("Shutdown requested. Exit %s\n", __func__); return; @@ -1608,7 +1608,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA // If the loaded chain has a wrong genesis, bail out immediately // (we're likely using a testnet datadir, or the other way around). if (!chainman.BlockIndex().empty() && - !LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) { + !g_chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) { return InitError(_("Incorrect or no genesis block found. Wrong datadir for network?")); } diff --git a/src/miner.cpp b/src/miner.cpp index 009ae6b13a..076d43c951 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -45,7 +45,7 @@ void RegenerateCommitments(CBlock& block) tx.vout.erase(tx.vout.begin() + GetWitnessCommitmentIndex(block)); block.vtx.at(0) = MakeTransactionRef(tx); - GenerateCoinbaseCommitment(block, WITH_LOCK(cs_main, return LookupBlockIndex(block.hashPrevBlock)), Params().GetConsensus()); + GenerateCoinbaseCommitment(block, WITH_LOCK(cs_main, return g_chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock)), Params().GetConsensus()); block.hashMerkleRoot = BlockMerkleRoot(block); } @@ -176,7 +176,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc pblocktemplate->vTxSigOpsCost[0] = WITNESS_SCALE_FACTOR * GetLegacySigOpCount(*pblock->vtx[0]); BlockValidationState state; - if (!TestBlockValidity(state, chainparams, *pblock, pindexPrev, false, false)) { + if (!TestBlockValidity(state, chainparams, ::ChainstateActive(), *pblock, pindexPrev, false, false)) { throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s", __func__, state.ToString())); } int64_t nTime2 = GetTimeMicros(); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a4070f5b63..b68453759a 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -676,7 +676,7 @@ static void ProcessBlockAvailability(NodeId nodeid) EXCLUSIVE_LOCKS_REQUIRED(cs_ assert(state != nullptr); if (!state->hashLastUnknownBlock.IsNull()) { - const CBlockIndex* pindex = LookupBlockIndex(state->hashLastUnknownBlock); + const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(state->hashLastUnknownBlock); if (pindex && pindex->nChainWork > 0) { if (state->pindexBestKnownBlock == nullptr || pindex->nChainWork >= state->pindexBestKnownBlock->nChainWork) { state->pindexBestKnownBlock = pindex; @@ -693,7 +693,7 @@ static void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) EXCLUSIV ProcessBlockAvailability(nodeid); - const CBlockIndex* pindex = LookupBlockIndex(hash); + const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(hash); if (pindex && pindex->nChainWork > 0) { // An actually better block was announced. if (state->pindexBestKnownBlock == nullptr || pindex->nChainWork >= state->pindexBestKnownBlock->nChainWork) { @@ -1596,7 +1596,7 @@ bool static AlreadyHaveTx(const GenTxid& gtxid, const CTxMemPool& mempool) EXCLU bool static AlreadyHaveBlock(const uint256& block_hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - return LookupBlockIndex(block_hash) != nullptr; + return g_chainman.m_blockman.LookupBlockIndex(block_hash) != nullptr; } void RelayTransaction(const uint256& txid, const uint256& wtxid, const CConnman& connman) @@ -1686,7 +1686,7 @@ void static ProcessGetBlockData(CNode& pfrom, Peer& peer, const CChainParams& ch bool need_activate_chain = false; { LOCK(cs_main); - const CBlockIndex* pindex = LookupBlockIndex(inv.hash); + const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(inv.hash); if (pindex) { if (pindex->HaveTxsDownloaded() && !pindex->IsValid(BLOCK_VALID_SCRIPTS) && pindex->IsValid(BLOCK_VALID_TREE)) { @@ -1701,13 +1701,13 @@ void static ProcessGetBlockData(CNode& pfrom, Peer& peer, const CChainParams& ch } // release cs_main before calling ActivateBestChain if (need_activate_chain) { BlockValidationState state; - if (!ActivateBestChain(state, chainparams, a_recent_block)) { + if (!::ChainstateActive().ActivateBestChain(state, chainparams, a_recent_block)) { LogPrint(BCLog::NET, "failed to activate chain (%s)\n", state.ToString()); } } LOCK(cs_main); - const CBlockIndex* pindex = LookupBlockIndex(inv.hash); + const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(inv.hash); if (pindex) { send = BlockRequestAllowed(pindex, consensusParams); if (!send) { @@ -1997,7 +1997,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, // don't connect before giving DoS points // - Once a headers message is received that is valid and does connect, // nUnconnectingHeaders gets reset back to 0. - if (!LookupBlockIndex(headers[0].hashPrevBlock) && nCount < MAX_BLOCKS_TO_ANNOUNCE) { + if (!g_chainman.m_blockman.LookupBlockIndex(headers[0].hashPrevBlock) && nCount < MAX_BLOCKS_TO_ANNOUNCE) { nodestate->nUnconnectingHeaders++; m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), uint256())); LogPrint(BCLog::NET, "received header %s: missing prev block %s, sending getheaders (%d) to end (peer=%d, nUnconnectingHeaders=%d)\n", @@ -2027,7 +2027,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, // If we don't have the last header, then they'll have given us // something new (if these headers are valid). - if (!LookupBlockIndex(hashLastBlock)) { + if (!g_chainman.m_blockman.LookupBlockIndex(hashLastBlock)) { received_new_header = true; } } @@ -2279,7 +2279,7 @@ static bool PrepareBlockFilterRequest(CNode& peer, const CChainParams& chain_par { LOCK(cs_main); - stop_index = LookupBlockIndex(stop_hash); + stop_index = g_chainman.m_blockman.LookupBlockIndex(stop_hash); // Check that the stop block exists and the peer would be allowed to fetch it. if (!stop_index || !BlockRequestAllowed(stop_index, chain_params.GetConsensus())) { @@ -2974,7 +2974,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, a_recent_block = most_recent_block; } BlockValidationState state; - if (!ActivateBestChain(state, m_chainparams, a_recent_block)) { + if (!::ChainstateActive().ActivateBestChain(state, m_chainparams, a_recent_block)) { LogPrint(BCLog::NET, "failed to activate chain (%s)\n", state.ToString()); } } @@ -2982,7 +2982,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, LOCK(cs_main); // Find the last block the caller has in the main chain - const CBlockIndex* pindex = FindForkInGlobalIndex(::ChainActive(), locator); + const CBlockIndex* pindex = g_chainman.m_blockman.FindForkInGlobalIndex(::ChainActive(), locator); // Send the rest of the chain if (pindex) @@ -3035,7 +3035,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, { LOCK(cs_main); - const CBlockIndex* pindex = LookupBlockIndex(req.blockhash); + const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(req.blockhash); if (!pindex || !(pindex->nStatus & BLOCK_HAVE_DATA)) { LogPrint(BCLog::NET, "Peer %d sent us a getblocktxn for a block we don't have\n", pfrom.GetId()); return; @@ -3089,7 +3089,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (locator.IsNull()) { // If locator is null, return the hashStop block - pindex = LookupBlockIndex(hashStop); + pindex = g_chainman.m_blockman.LookupBlockIndex(hashStop); if (!pindex) { return; } @@ -3102,7 +3102,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, else { // Find the last block the caller has in the main chain - pindex = FindForkInGlobalIndex(::ChainActive(), locator); + pindex = g_chainman.m_blockman.FindForkInGlobalIndex(::ChainActive(), locator); if (pindex) pindex = ::ChainActive().Next(pindex); } @@ -3366,14 +3366,14 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, { LOCK(cs_main); - if (!LookupBlockIndex(cmpctblock.header.hashPrevBlock)) { + if (!g_chainman.m_blockman.LookupBlockIndex(cmpctblock.header.hashPrevBlock)) { // Doesn't connect (or is genesis), instead of DoSing in AcceptBlockHeader, request deeper headers if (!::ChainstateActive().IsInitialBlockDownload()) m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), uint256())); return; } - if (!LookupBlockIndex(cmpctblock.header.GetHash())) { + if (!g_chainman.m_blockman.LookupBlockIndex(cmpctblock.header.GetHash())) { received_new_header = true; } } @@ -4442,7 +4442,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // then send all headers past that one. If we come across any // headers that aren't on ::ChainActive(), give up. for (const uint256& hash : peer->m_blocks_for_headers_relay) { - const CBlockIndex* pindex = LookupBlockIndex(hash); + const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(hash); assert(pindex); if (::ChainActive()[pindex->nHeight] != pindex) { // Bail out if we reorged away from this block @@ -4534,7 +4534,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // in the past. if (!peer->m_blocks_for_headers_relay.empty()) { const uint256& hashToAnnounce = peer->m_blocks_for_headers_relay.back(); - const CBlockIndex* pindex = LookupBlockIndex(hashToAnnounce); + const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(hashToAnnounce); assert(pindex); // Warn if we're announcing a block that is not on the main chain. diff --git a/src/node/coinstats.cpp b/src/node/coinstats.cpp index 02e50c4dbe..56113cb731 100644 --- a/src/node/coinstats.cpp +++ b/src/node/coinstats.cpp @@ -63,7 +63,7 @@ static bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, const stats.hashBlock = pcursor->GetBestBlock(); { LOCK(cs_main); - stats.nHeight = LookupBlockIndex(stats.hashBlock)->nHeight; + stats.nHeight = g_chainman.m_blockman.LookupBlockIndex(stats.hashBlock)->nHeight; } PrepareHash(hash_obj, stats); diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index e07eaa33d8..b7efd68cfd 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -447,7 +447,7 @@ public: { LOCK(cs_main); const CChain& active = Assert(m_node.chainman)->ActiveChain(); - if (CBlockIndex* fork = FindForkInGlobalIndex(active, locator)) { + if (CBlockIndex* fork = g_chainman.m_blockman.FindForkInGlobalIndex(active, locator)) { return fork->nHeight; } return nullopt; @@ -456,7 +456,7 @@ public: { WAIT_LOCK(cs_main, lock); const CChain& active = Assert(m_node.chainman)->ActiveChain(); - return FillBlock(LookupBlockIndex(hash), block, lock, active); + return FillBlock(g_chainman.m_blockman.LookupBlockIndex(hash), block, lock, active); } bool findFirstBlockWithTimeAndHeight(int64_t min_time, int min_height, const FoundBlock& block) override { @@ -468,7 +468,7 @@ public: { WAIT_LOCK(cs_main, lock); const CChain& active = Assert(m_node.chainman)->ActiveChain(); - if (const CBlockIndex* block = LookupBlockIndex(block_hash)) { + 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, active); } @@ -479,8 +479,8 @@ public: { WAIT_LOCK(cs_main, lock); const CChain& active = Assert(m_node.chainman)->ActiveChain(); - const CBlockIndex* block = LookupBlockIndex(block_hash); - const CBlockIndex* ancestor = LookupBlockIndex(ancestor_hash); + 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, active); } @@ -488,8 +488,8 @@ public: { WAIT_LOCK(cs_main, lock); const CChain& active = Assert(m_node.chainman)->ActiveChain(); - const CBlockIndex* block1 = LookupBlockIndex(block_hash1); - const CBlockIndex* block2 = LookupBlockIndex(block_hash2); + 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; // Using & instead of && below to avoid short circuiting and leaving // output uninitialized. @@ -499,7 +499,7 @@ public: double guessVerificationProgress(const uint256& block_hash) override { LOCK(cs_main); - return GuessVerificationProgress(Params().TxData(), LookupBlockIndex(block_hash)); + return GuessVerificationProgress(Params().TxData(), g_chainman.m_blockman.LookupBlockIndex(block_hash)); } bool hasBlocks(const uint256& block_hash, int min_height, Optional<int> max_height) override { @@ -511,7 +511,7 @@ public: // 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 = LookupBlockIndex(block_hash)) { + 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 diff --git a/src/rest.cpp b/src/rest.cpp index 678ffdb760..f872f6e59d 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -179,7 +179,7 @@ static bool rest_headers(const util::Ref& context, { LOCK(cs_main); tip = ::ChainActive().Tip(); - const CBlockIndex* pindex = LookupBlockIndex(hash); + const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(hash); while (pindex != nullptr && ::ChainActive().Contains(pindex)) { headers.push_back(pindex); if (headers.size() == (unsigned long)count) @@ -247,7 +247,7 @@ static bool rest_block(HTTPRequest* req, { LOCK(cs_main); tip = ::ChainActive().Tip(); - pblockindex = LookupBlockIndex(hash); + pblockindex = g_chainman.m_blockman.LookupBlockIndex(hash); if (!pblockindex) { return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found"); } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index e41a78f917..a83b45ae1d 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -843,7 +843,7 @@ static RPCHelpMan getblockheader() const CBlockIndex* tip; { LOCK(cs_main); - pblockindex = LookupBlockIndex(hash); + pblockindex = g_chainman.m_blockman.LookupBlockIndex(hash); tip = ::ChainActive().Tip(); } @@ -967,7 +967,7 @@ static RPCHelpMan getblock() const CBlockIndex* tip; { LOCK(cs_main); - pblockindex = LookupBlockIndex(hash); + pblockindex = g_chainman.m_blockman.LookupBlockIndex(hash); tip = ::ChainActive().Tip(); if (!pblockindex) { @@ -1164,7 +1164,7 @@ static RPCHelpMan gettxout() } } - const CBlockIndex* pindex = LookupBlockIndex(coins_view->GetBestBlock()); + const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(coins_view->GetBestBlock()); ret.pushKV("bestblock", pindex->GetBlockHash().GetHex()); if (coin.nHeight == MEMPOOL_HEIGHT) { ret.pushKV("confirmations", 0); @@ -1557,7 +1557,7 @@ static RPCHelpMan preciousblock() { LOCK(cs_main); - pblockindex = LookupBlockIndex(hash); + pblockindex = g_chainman.m_blockman.LookupBlockIndex(hash); if (!pblockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); } @@ -1595,7 +1595,7 @@ static RPCHelpMan invalidateblock() CBlockIndex* pblockindex; { LOCK(cs_main); - pblockindex = LookupBlockIndex(hash); + pblockindex = g_chainman.m_blockman.LookupBlockIndex(hash); if (!pblockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); } @@ -1603,7 +1603,7 @@ static RPCHelpMan invalidateblock() InvalidateBlock(state, Params(), pblockindex); if (state.IsValid()) { - ActivateBestChain(state, Params()); + ::ChainstateActive().ActivateBestChain(state, Params()); } if (!state.IsValid()) { @@ -1634,7 +1634,7 @@ static RPCHelpMan reconsiderblock() { LOCK(cs_main); - CBlockIndex* pblockindex = LookupBlockIndex(hash); + CBlockIndex* pblockindex = g_chainman.m_blockman.LookupBlockIndex(hash); if (!pblockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); } @@ -1643,7 +1643,7 @@ static RPCHelpMan reconsiderblock() } BlockValidationState state; - ActivateBestChain(state, Params()); + ::ChainstateActive().ActivateBestChain(state, Params()); if (!state.IsValid()) { throw JSONRPCError(RPC_DATABASE_ERROR, state.ToString()); @@ -1689,7 +1689,7 @@ static RPCHelpMan getchaintxstats() } else { uint256 hash(ParseHashV(request.params[1], "blockhash")); LOCK(cs_main); - pindex = LookupBlockIndex(hash); + pindex = g_chainman.m_blockman.LookupBlockIndex(hash); if (!pindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); } @@ -1867,7 +1867,7 @@ static RPCHelpMan getblockstats() pindex = ::ChainActive()[height]; } else { const uint256 hash(ParseHashV(request.params[0], "hash_or_height")); - pindex = LookupBlockIndex(hash); + pindex = g_chainman.m_blockman.LookupBlockIndex(hash); if (!pindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); } @@ -2330,7 +2330,7 @@ static RPCHelpMan getblockfilter() bool block_was_connected; { LOCK(cs_main); - block_index = LookupBlockIndex(block_hash); + block_index = g_chainman.m_blockman.LookupBlockIndex(block_hash); if (!block_index) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); } @@ -2440,7 +2440,7 @@ static RPCHelpMan dumptxoutset() } pcursor = std::unique_ptr<CCoinsViewCursor>(::ChainstateActive().CoinsDB().Cursor()); - tip = LookupBlockIndex(stats.hashBlock); + tip = g_chainman.m_blockman.LookupBlockIndex(stats.hashBlock); CHECK_NONFATAL(tip); } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index c723f1d7ea..50987a735b 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -375,7 +375,7 @@ static RPCHelpMan generateblock() LOCK(cs_main); BlockValidationState state; - if (!TestBlockValidity(state, chainparams, block, LookupBlockIndex(block.hashPrevBlock), false, false)) { + if (!TestBlockValidity(state, chainparams, ::ChainstateActive(), block, g_chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), false, false)) { throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s", state.ToString())); } } @@ -618,7 +618,7 @@ static RPCHelpMan getblocktemplate() throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Block decode failed"); uint256 hash = block.GetHash(); - const CBlockIndex* pindex = LookupBlockIndex(hash); + const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(hash); if (pindex) { if (pindex->IsValid(BLOCK_VALID_SCRIPTS)) return "duplicate"; @@ -632,7 +632,7 @@ static RPCHelpMan getblocktemplate() if (block.hashPrevBlock != pindexPrev->GetBlockHash()) return "inconclusive-not-best-prevblk"; BlockValidationState state; - TestBlockValidity(state, Params(), block, pindexPrev, false, true); + TestBlockValidity(state, Params(), ::ChainstateActive(), block, pindexPrev, false, true); return BIP22ValidationResult(state); } @@ -966,7 +966,7 @@ static RPCHelpMan submitblock() uint256 hash = block.GetHash(); { LOCK(cs_main); - const CBlockIndex* pindex = LookupBlockIndex(hash); + const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(hash); if (pindex) { if (pindex->IsValid(BLOCK_VALID_SCRIPTS)) { return "duplicate"; @@ -979,7 +979,7 @@ static RPCHelpMan submitblock() { LOCK(cs_main); - const CBlockIndex* pindex = LookupBlockIndex(block.hashPrevBlock); + const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock); if (pindex) { UpdateUncommittedBlockStructures(block, pindex, Params().GetConsensus()); } @@ -1023,7 +1023,7 @@ static RPCHelpMan submitheader() } { LOCK(cs_main); - if (!LookupBlockIndex(h.hashPrevBlock)) { + if (!g_chainman.m_blockman.LookupBlockIndex(h.hashPrevBlock)) { throw JSONRPCError(RPC_VERIFY_ERROR, "Must submit previous header (" + h.hashPrevBlock.GetHex() + ") first"); } } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 031ed31da1..e478f12ab5 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -54,7 +54,7 @@ static void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& LOCK(cs_main); entry.pushKV("blockhash", hashBlock.GetHex()); - CBlockIndex* pindex = LookupBlockIndex(hashBlock); + CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(hashBlock); if (pindex) { if (::ChainActive().Contains(pindex)) { entry.pushKV("confirmations", 1 + ::ChainActive().Height() - pindex->nHeight); @@ -178,7 +178,7 @@ static RPCHelpMan getrawtransaction() LOCK(cs_main); uint256 blockhash = ParseHashV(request.params[2], "parameter 3"); - blockindex = LookupBlockIndex(blockhash); + blockindex = g_chainman.m_blockman.LookupBlockIndex(blockhash); if (!blockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block hash not found"); } @@ -260,7 +260,7 @@ static RPCHelpMan gettxoutproof() if (!request.params[1].isNull()) { LOCK(cs_main); hashBlock = ParseHashV(request.params[1], "blockhash"); - pblockindex = LookupBlockIndex(hashBlock); + pblockindex = g_chainman.m_blockman.LookupBlockIndex(hashBlock); if (!pblockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); } @@ -290,7 +290,7 @@ static RPCHelpMan gettxoutproof() if (!tx || hashBlock.IsNull()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not yet in block"); } - pblockindex = LookupBlockIndex(hashBlock); + pblockindex = g_chainman.m_blockman.LookupBlockIndex(hashBlock); if (!pblockindex) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Transaction index corrupt"); } @@ -350,7 +350,7 @@ static RPCHelpMan verifytxoutproof() LOCK(cs_main); - const CBlockIndex* pindex = LookupBlockIndex(merkleBlock.header.GetHash()); + const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(merkleBlock.header.GetHash()); if (!pindex || !::ChainActive().Contains(pindex) || pindex->nTx == 0) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain"); } @@ -816,10 +816,11 @@ static RPCHelpMan sendrawtransaction() { return RPCHelpMan{"sendrawtransaction", "\nSubmit a raw transaction (serialized, hex-encoded) to local node and network.\n" - "\nNote that the transaction will be sent unconditionally to all peers, so using this\n" + "\nThe transaction will be sent unconditionally to all peers, so using sendrawtransaction\n" "for manual rebroadcast may degrade privacy by leaking the transaction's origin, as\n" "nodes will normally not rebroadcast non-wallet transactions already in their mempool.\n" - "\nAlso see createrawtransaction and signrawtransactionwithkey calls.\n", + "\nA specific exception, RPC_TRANSACTION_ALREADY_IN_CHAIN, may throw if the transaction cannot be added to the mempool.\n" + "\nRelated RPCs: createrawtransaction, signrawtransactionwithkey\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK()), diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index 00c4bdc14e..633a95ce96 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -178,7 +178,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) const CBlockIndex* block_index; { LOCK(cs_main); - block_index = LookupBlockIndex(block->GetHash()); + block_index = g_chainman.m_blockman.LookupBlockIndex(block->GetHash()); } BOOST_CHECK(filter_index.BlockUntilSyncedToCurrentChain()); @@ -196,7 +196,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) const CBlockIndex* block_index; { LOCK(cs_main); - block_index = LookupBlockIndex(block->GetHash()); + block_index = g_chainman.m_blockman.LookupBlockIndex(block->GetHash()); } BOOST_CHECK(filter_index.BlockUntilSyncedToCurrentChain()); @@ -210,7 +210,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) const CBlockIndex* block_index; { LOCK(cs_main); - block_index = LookupBlockIndex(block->GetHash()); + block_index = g_chainman.m_blockman.LookupBlockIndex(block->GetHash()); } BOOST_CHECK(filter_index.BlockUntilSyncedToCurrentChain()); @@ -231,14 +231,14 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) { LOCK(cs_main); - block_index = LookupBlockIndex(chainA[i]->GetHash()); + block_index = g_chainman.m_blockman.LookupBlockIndex(chainA[i]->GetHash()); } BOOST_CHECK(filter_index.BlockUntilSyncedToCurrentChain()); CheckFilterLookups(filter_index, block_index, chainA_last_header); { LOCK(cs_main); - block_index = LookupBlockIndex(chainB[i]->GetHash()); + block_index = g_chainman.m_blockman.LookupBlockIndex(chainB[i]->GetHash()); } BOOST_CHECK(filter_index.BlockUntilSyncedToCurrentChain()); CheckFilterLookups(filter_index, block_index, chainB_last_header); diff --git a/src/test/fuzz/load_external_block_file.cpp b/src/test/fuzz/load_external_block_file.cpp index 207ee586bc..95597bf082 100644 --- a/src/test/fuzz/load_external_block_file.cpp +++ b/src/test/fuzz/load_external_block_file.cpp @@ -27,5 +27,5 @@ FUZZ_TARGET_INIT(load_external_block_file, initialize_load_external_block_file) return; } FlatFilePos flat_file_pos; - LoadExternalBlockFile(Params(), fuzzed_block_file, fuzzed_data_provider.ConsumeBool() ? &flat_file_pos : nullptr); + ::ChainstateActive().LoadExternalBlockFile(Params(), fuzzed_block_file, fuzzed_data_provider.ConsumeBool() ? &flat_file_pos : nullptr); } diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index a12dc9a56d..e48771efa2 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -269,8 +269,8 @@ inline CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcep const Network network = fuzzed_data_provider.PickValueInArray({Network::NET_IPV4, Network::NET_IPV6, Network::NET_INTERNAL, Network::NET_ONION}); CNetAddr net_addr; if (network == Network::NET_IPV4) { - const in_addr v4_addr = { - .s_addr = fuzzed_data_provider.ConsumeIntegral<uint32_t>()}; + in_addr v4_addr = {}; + v4_addr.s_addr = fuzzed_data_provider.ConsumeIntegral<uint32_t>(); net_addr = CNetAddr{v4_addr}; } else if (network == Network::NET_IPV6) { if (fuzzed_data_provider.remaining_bytes() >= 16) { diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index d99d63aac6..b9f3f8c955 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -183,7 +183,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const } BlockValidationState state; - if (!ActivateBestChain(state, chainparams)) { + if (!::ChainstateActive().ActivateBestChain(state, chainparams)) { throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", state.ToString())); } diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index ea17cb50f1..ec45d9a434 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -95,8 +95,8 @@ std::shared_ptr<CBlock> MinerTestingSetup::Block(const uint256& prev_hash) std::shared_ptr<CBlock> MinerTestingSetup::FinalizeBlock(std::shared_ptr<CBlock> pblock) { - LOCK(cs_main); // For LookupBlockIndex - GenerateCoinbaseCommitment(*pblock, LookupBlockIndex(pblock->hashPrevBlock), Params().GetConsensus()); + LOCK(cs_main); // For g_chainman.m_blockman.LookupBlockIndex + GenerateCoinbaseCommitment(*pblock, g_chainman.m_blockman.LookupBlockIndex(pblock->hashPrevBlock), Params().GetConsensus()); pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 470e665844..c370f9e981 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -626,7 +626,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const uint64_t innerUsage = 0; CCoinsViewCache mempoolDuplicate(const_cast<CCoinsViewCache*>(pcoins)); - const int64_t spendheight = GetSpendHeight(mempoolDuplicate); + const int64_t spendheight = g_chainman.m_blockman.GetSpendHeight(mempoolDuplicate); std::list<const CTxMemPoolEntry*> waitingOnDependants; for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { diff --git a/src/validation.cpp b/src/validation.cpp index eeb99f0f0a..38df71b994 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -167,17 +167,19 @@ namespace { std::set<int> setDirtyFileInfo; } // anon namespace -CBlockIndex* LookupBlockIndex(const uint256& hash) +CBlockIndex* BlockManager::LookupBlockIndex(const uint256& hash) { AssertLockHeld(cs_main); - BlockMap::const_iterator it = g_chainman.BlockIndex().find(hash); - return it == g_chainman.BlockIndex().end() ? nullptr : it->second; + assert(std::addressof(g_chainman.BlockIndex()) == std::addressof(m_block_index)); + BlockMap::const_iterator it = m_block_index.find(hash); + return it == m_block_index.end() ? nullptr : it->second; } -CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& locator) +CBlockIndex* BlockManager::FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& locator) { AssertLockHeld(cs_main); + assert(std::addressof(g_chainman.m_blockman) == std::addressof(*this)); // Find the latest block common to locator and chain - we expect that // locator.vHave is sorted descending by height. for (const uint256& hash : locator.vHave) { @@ -682,7 +684,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-BIP68-final"); CAmount nFees = 0; - if (!Consensus::CheckTxInputs(tx, state, m_view, GetSpendHeight(m_view), nFees)) { + if (!Consensus::CheckTxInputs(tx, state, m_view, g_chainman.m_blockman.GetSpendHeight(m_view), nFees)) { return false; // state filled in by CheckTxInputs } @@ -1262,8 +1264,8 @@ void CoinsViews::InitCache() } CChainState::CChainState(CTxMemPool& mempool, BlockManager& blockman, uint256 from_snapshot_blockhash) - : m_blockman(blockman), - m_mempool(mempool), + : m_mempool(mempool), + m_blockman(blockman), m_from_snapshot_blockhash(from_snapshot_blockhash) {} void CChainState::InitCoinsDB( @@ -1410,9 +1412,10 @@ bool CScriptCheck::operator()() { return VerifyScript(scriptSig, m_tx_out.scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, m_tx_out.nValue, cacheStore, *txdata), &error); } -int GetSpendHeight(const CCoinsViewCache& inputs) +int BlockManager::GetSpendHeight(const CCoinsViewCache& inputs) { - LOCK(cs_main); + AssertLockHeld(cs_main); + assert(std::addressof(g_chainman.m_blockman) == std::addressof(*this)); CBlockIndex* pindexPrev = LookupBlockIndex(inputs.GetBestBlock()); return pindexPrev->nHeight + 1; } @@ -2767,7 +2770,7 @@ static SynchronizationState GetSynchronizationState(bool init) return SynchronizationState::INIT_DOWNLOAD; } -static bool NotifyHeaderTip() LOCKS_EXCLUDED(cs_main) { +static bool NotifyHeaderTip(CChainState& chainstate) LOCKS_EXCLUDED(cs_main) { bool fNotify = false; bool fInitialBlockDownload = false; static CBlockIndex* pindexHeaderOld = nullptr; @@ -2778,7 +2781,8 @@ static bool NotifyHeaderTip() LOCKS_EXCLUDED(cs_main) { if (pindexHeader != pindexHeaderOld) { fNotify = true; - fInitialBlockDownload = ::ChainstateActive().IsInitialBlockDownload(); + assert(std::addressof(::ChainstateActive()) == std::addressof(chainstate)); + fInitialBlockDownload = chainstate.IsInitialBlockDownload(); pindexHeaderOld = pindexHeader; } } @@ -2895,10 +2899,6 @@ bool CChainState::ActivateBestChain(BlockValidationState &state, const CChainPar return true; } -bool ActivateBestChain(BlockValidationState &state, const CChainParams& chainparams, std::shared_ptr<const CBlock> pblock) { - return ::ChainstateActive().ActivateBestChain(state, chainparams, std::move(pblock)); -} - bool CChainState::PreciousBlock(BlockValidationState& state, const CChainParams& params, CBlockIndex *pindex) { { @@ -3398,14 +3398,14 @@ std::vector<unsigned char> GenerateCoinbaseCommitment(CBlock& block, const CBloc return commitment; } -//! Returns last CBlockIndex* that is a checkpoint -static CBlockIndex* GetLastCheckpoint(const CCheckpointData& data) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +CBlockIndex* BlockManager::GetLastCheckpoint(const CCheckpointData& data) { const MapCheckpoints& checkpoints = data.mapCheckpoints; for (const MapCheckpoints::value_type& i : reverse_iterate(checkpoints)) { const uint256& hash = i.second; + assert(std::addressof(g_chainman.m_blockman) == std::addressof(*this)); CBlockIndex* pindex = LookupBlockIndex(hash); if (pindex) { return pindex; @@ -3423,7 +3423,7 @@ static CBlockIndex* GetLastCheckpoint(const CCheckpointData& data) EXCLUSIVE_LOC * in ConnectBlock(). * Note that -reindex-chainstate skips the validation that happens here! */ -static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, const CChainParams& params, const CBlockIndex* pindexPrev, int64_t nAdjustedTime) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const CChainParams& params, const CBlockIndex* pindexPrev, int64_t nAdjustedTime) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { assert(pindexPrev != nullptr); const int nHeight = pindexPrev->nHeight + 1; @@ -3438,7 +3438,8 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio // Don't accept any forks from the main chain prior to last checkpoint. // GetLastCheckpoint finds the last checkpoint in MapCheckpoints that's in our // BlockIndex(). - CBlockIndex* pcheckpoint = GetLastCheckpoint(params.Checkpoints()); + assert(std::addressof(g_chainman.m_blockman) == std::addressof(blockman)); + CBlockIndex* pcheckpoint = blockman.GetLastCheckpoint(params.Checkpoints()); if (pcheckpoint && nHeight < pcheckpoint->nHeight) { LogPrintf("ERROR: %s: forked chain older than last checkpoint (height %d)\n", __func__, nHeight); return state.Invalid(BlockValidationResult::BLOCK_CHECKPOINT, "bad-fork-prior-to-checkpoint"); @@ -3558,11 +3559,10 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS // Check for duplicate uint256 hash = block.GetHash(); BlockMap::iterator miSelf = m_block_index.find(hash); - CBlockIndex *pindex = nullptr; if (hash != chainparams.GetConsensus().hashGenesisBlock) { if (miSelf != m_block_index.end()) { // Block header is already known. - pindex = miSelf->second; + CBlockIndex* pindex = miSelf->second; if (ppindex) *ppindex = pindex; if (pindex->nStatus & BLOCK_FAILED_MASK) { @@ -3589,7 +3589,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS LogPrintf("ERROR: %s: prev block invalid\n", __func__); return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, "bad-prevblk"); } - if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime())) + if (!ContextualCheckBlockHeader(block, state, *this, chainparams, pindexPrev, GetAdjustedTime())) return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", __func__, hash.ToString(), state.ToString()); /* Determine if this block descends from any block which has been found @@ -3631,8 +3631,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS } } } - if (pindex == nullptr) - pindex = AddToBlockIndex(block); + CBlockIndex* pindex = AddToBlockIndex(block); if (ppindex) *ppindex = pindex; @@ -3643,6 +3642,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS // Exposed wrapper for AcceptBlockHeader bool ChainstateManager::ProcessNewBlockHeaders(const std::vector<CBlockHeader>& headers, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex) { + assert(std::addressof(::ChainstateActive()) == std::addressof(ActiveChainstate())); AssertLockNotHeld(cs_main); { LOCK(cs_main); @@ -3650,7 +3650,7 @@ bool ChainstateManager::ProcessNewBlockHeaders(const std::vector<CBlockHeader>& CBlockIndex *pindex = nullptr; // Use a temp pindex instead of ppindex to avoid a const_cast bool accepted = m_blockman.AcceptBlockHeader( header, state, chainparams, &pindex); - ::ChainstateActive().CheckBlockIndex(chainparams.GetConsensus()); + ActiveChainstate().CheckBlockIndex(chainparams.GetConsensus()); if (!accepted) { return false; @@ -3660,8 +3660,8 @@ bool ChainstateManager::ProcessNewBlockHeaders(const std::vector<CBlockHeader>& } } } - if (NotifyHeaderTip()) { - if (::ChainstateActive().IsInitialBlockDownload() && ppindex && *ppindex) { + if (NotifyHeaderTip(ActiveChainstate())) { + if (ActiveChainstate().IsInitialBlockDownload() && ppindex && *ppindex) { LogPrintf("Synchronizing blockheaders, height: %d (~%.2f%%)\n", (*ppindex)->nHeight, 100.0/((*ppindex)->nHeight+(GetAdjustedTime() - (*ppindex)->GetBlockTime()) / Params().GetConsensus().nPowTargetSpacing) * (*ppindex)->nHeight); } } @@ -3773,6 +3773,7 @@ bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, Block bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<const CBlock> pblock, bool fForceProcessing, bool* fNewBlock) { AssertLockNotHeld(cs_main); + assert(std::addressof(::ChainstateActive()) == std::addressof(ActiveChainstate())); { CBlockIndex *pindex = nullptr; @@ -3788,7 +3789,7 @@ bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const s bool ret = CheckBlock(*pblock, state, chainparams.GetConsensus()); if (ret) { // Store to disk - ret = ::ChainstateActive().AcceptBlock(pblock, state, chainparams, &pindex, fForceProcessing, nullptr, fNewBlock); + ret = ActiveChainstate().AcceptBlock(pblock, state, chainparams, &pindex, fForceProcessing, nullptr, fNewBlock); } if (!ret) { GetMainSignals().BlockChecked(*pblock, state); @@ -3796,20 +3797,27 @@ bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const s } } - NotifyHeaderTip(); + NotifyHeaderTip(ActiveChainstate()); BlockValidationState state; // Only used to report errors, not invalidity - ignore it - if (!::ChainstateActive().ActivateBestChain(state, chainparams, pblock)) + if (!ActiveChainstate().ActivateBestChain(state, chainparams, pblock)) return error("%s: ActivateBestChain failed (%s)", __func__, state.ToString()); return true; } -bool TestBlockValidity(BlockValidationState& state, const CChainParams& chainparams, const CBlock& block, CBlockIndex* pindexPrev, bool fCheckPOW, bool fCheckMerkleRoot) +bool TestBlockValidity(BlockValidationState& state, + const CChainParams& chainparams, + CChainState& chainstate, + const CBlock& block, + CBlockIndex* pindexPrev, + bool fCheckPOW, + bool fCheckMerkleRoot) { AssertLockHeld(cs_main); - assert(pindexPrev && pindexPrev == ::ChainActive().Tip()); - CCoinsViewCache viewNew(&::ChainstateActive().CoinsTip()); + assert(std::addressof(::ChainstateActive()) == std::addressof(chainstate)); + assert(pindexPrev && pindexPrev == chainstate.m_chain.Tip()); + CCoinsViewCache viewNew(&chainstate.CoinsTip()); uint256 block_hash(block.GetHash()); CBlockIndex indexDummy(block); indexDummy.pprev = pindexPrev; @@ -3817,13 +3825,14 @@ bool TestBlockValidity(BlockValidationState& state, const CChainParams& chainpar indexDummy.phashBlock = &block_hash; // NOTE: CheckBlockHeader is called by CheckBlock - if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime())) + assert(std::addressof(g_chainman.m_blockman) == std::addressof(chainstate.m_blockman)); + if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainparams, pindexPrev, GetAdjustedTime())) return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, state.ToString()); if (!CheckBlock(block, state, chainparams.GetConsensus(), fCheckPOW, fCheckMerkleRoot)) return error("%s: Consensus::CheckBlock: %s", __func__, state.ToString()); if (!ContextualCheckBlock(block, state, chainparams.GetConsensus(), pindexPrev)) return error("%s: Consensus::ContextualCheckBlock: %s", __func__, state.ToString()); - if (!::ChainstateActive().ConnectBlock(block, state, &indexDummy, viewNew, chainparams, true)) + if (!chainstate.ConnectBlock(block, state, &indexDummy, viewNew, chainparams, true)) return false; assert(state.IsValid()); @@ -4169,7 +4178,7 @@ bool CChainState::LoadChainTip(const CChainParams& chainparams) } // Load pointer to end of best chain - CBlockIndex* pindex = LookupBlockIndex(coins_cache.GetBestBlock()); + CBlockIndex* pindex = m_blockman.LookupBlockIndex(coins_cache.GetBestBlock()); if (!pindex) { return false; } @@ -4599,7 +4608,7 @@ bool LoadGenesisBlock(const CChainParams& chainparams) return ::ChainstateActive().LoadGenesisBlock(chainparams); } -void LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFilePos* dbp) +void CChainState::LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFilePos* dbp) { // Map of disk positions for blocks with unknown parent (only used for reindex) static std::multimap<uint256, FlatFilePos> mapBlocksUnknownParent; @@ -4648,7 +4657,8 @@ void LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi { LOCK(cs_main); // detect out of order blocks, and store them for later - if (hash != chainparams.GetConsensus().hashGenesisBlock && !LookupBlockIndex(block.hashPrevBlock)) { + assert(std::addressof(g_chainman.m_blockman) == std::addressof(m_blockman)); + if (hash != chainparams.GetConsensus().hashGenesisBlock && !m_blockman.LookupBlockIndex(block.hashPrevBlock)) { LogPrint(BCLog::REINDEX, "%s: Out of order block %s, parent %s not known\n", __func__, hash.ToString(), block.hashPrevBlock.ToString()); if (dbp) @@ -4657,10 +4667,12 @@ void LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi } // process in case the block isn't known yet - CBlockIndex* pindex = LookupBlockIndex(hash); + assert(std::addressof(g_chainman.m_blockman) == std::addressof(m_blockman)); + CBlockIndex* pindex = m_blockman.LookupBlockIndex(hash); if (!pindex || (pindex->nStatus & BLOCK_HAVE_DATA) == 0) { BlockValidationState state; - if (::ChainstateActive().AcceptBlock(pblock, state, chainparams, nullptr, true, dbp, nullptr)) { + assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); + if (AcceptBlock(pblock, state, chainparams, nullptr, true, dbp, nullptr)) { nLoaded++; } if (state.IsError()) { @@ -4674,12 +4686,14 @@ void LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi // Activate the genesis block so normal node progress can continue if (hash == chainparams.GetConsensus().hashGenesisBlock) { BlockValidationState state; + assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); if (!ActivateBestChain(state, chainparams, nullptr)) { break; } } - NotifyHeaderTip(); + assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); + NotifyHeaderTip(*this); // Recursively process earlier encountered successors of this block std::deque<uint256> queue; @@ -4697,7 +4711,8 @@ void LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi head.ToString()); LOCK(cs_main); BlockValidationState dummy; - if (::ChainstateActive().AcceptBlock(pblockrecursive, dummy, chainparams, nullptr, true, &it->second, nullptr)) + assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); + if (AcceptBlock(pblockrecursive, dummy, chainparams, nullptr, true, &it->second, nullptr)) { nLoaded++; queue.push_back(pblockrecursive->GetHash()); @@ -4705,7 +4720,8 @@ void LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi } range.first++; mapBlocksUnknownParent.erase(it); - NotifyHeaderTip(); + assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); + NotifyHeaderTip(*this); } } } catch (const std::exception& e) { @@ -5143,6 +5159,7 @@ double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pin } Optional<uint256> ChainstateManager::SnapshotBlockhash() const { + LOCK(::cs_main); // for m_active_chainstate access if (m_active_chainstate != nullptr) { // If a snapshot chainstate exists, it will always be our active. return m_active_chainstate->m_from_snapshot_blockhash; @@ -5189,13 +5206,14 @@ CChainState& ChainstateManager::InitializeChainstate(CTxMemPool& mempool, const CChainState& ChainstateManager::ActiveChainstate() const { + LOCK(::cs_main); assert(m_active_chainstate); return *m_active_chainstate; } bool ChainstateManager::IsSnapshotActive() const { - return m_snapshot_chainstate && m_active_chainstate == m_snapshot_chainstate.get(); + return m_snapshot_chainstate && WITH_LOCK(::cs_main, return m_active_chainstate) == m_snapshot_chainstate.get(); } CChainState& ChainstateManager::ValidatedChainstate() const @@ -5226,7 +5244,10 @@ void ChainstateManager::Reset() { m_ibd_chainstate.reset(); m_snapshot_chainstate.reset(); - m_active_chainstate = nullptr; + { + LOCK(::cs_main); + m_active_chainstate = nullptr; + } m_snapshot_validated = false; } diff --git a/src/validation.h b/src/validation.h index d3fbabe1a2..e86b3d3af8 100644 --- a/src/validation.h +++ b/src/validation.h @@ -40,6 +40,7 @@ class CBlockIndex; class CBlockTreeDB; class CBlockUndo; class CChainParams; +class CCheckpointData; class CInv; class CConnman; class CScriptCheck; @@ -143,8 +144,6 @@ extern const std::vector<std::string> CHECKLEVEL_DOC; FILE* OpenBlockFile(const FlatFilePos &pos, bool fReadOnly = false); /** Translation to a filesystem path */ fs::path GetBlockPosFilename(const FlatFilePos &pos); -/** Import blocks from an external file */ -void LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFilePos* dbp = nullptr); /** Ensures we have a genesis block in the block tree, possibly writing one to disk. */ bool LoadGenesisBlock(const CChainParams& chainparams); /** Unload database information */ @@ -166,13 +165,6 @@ void StopScriptCheckWorkerThreads(); * @returns The tx if found, otherwise nullptr */ CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock); -/** - * Find the best known block, and make it the tip of the block chain - * - * May not be called with cs_main held. May not be called in a - * validationinterface callback. - */ -bool ActivateBestChain(BlockValidationState& state, const CChainParams& chainparams, std::shared_ptr<const CBlock> pblock = std::shared_ptr<const CBlock>()); CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams); /** Guess verification progress (as a fraction between 0.0=genesis and 1.0=current tip). */ @@ -291,7 +283,13 @@ bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex); bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW = true, bool fCheckMerkleRoot = true); /** Check a block is completely valid from start to finish (only works on top of our current best block) */ -bool TestBlockValidity(BlockValidationState& state, const CChainParams& chainparams, const CBlock& block, CBlockIndex* pindexPrev, bool fCheckPOW = true, bool fCheckMerkleRoot = true) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +bool TestBlockValidity(BlockValidationState& state, + const CChainParams& chainparams, + CChainState& chainstate, + const CBlock& block, + CBlockIndex* pindexPrev, + bool fCheckPOW = true, + bool fCheckMerkleRoot = true) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Check whether witness commitments are required for a block, and whether to enforce NULLDUMMY (BIP 147) rules. * Note that transaction witness validation rules are always enforced when P2SH is enforced. */ @@ -311,11 +309,6 @@ public: bool VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview, int nCheckLevel, int nCheckDepth); }; -CBlockIndex* LookupBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - -/** Find the last common block between the parameter chain and a locator. */ -CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& locator) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - enum DisconnectResult { DISCONNECT_OK, // All good. @@ -433,6 +426,21 @@ public: const CChainParams& chainparams, CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + CBlockIndex* LookupBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + /** Find the last common block between the parameter chain and a locator. */ + CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& locator) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + //! Returns last CBlockIndex* that is a checkpoint + CBlockIndex* GetLastCheckpoint(const CCheckpointData& data) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + /** + * Return the spend height, which is one more than the inputs.GetBestBlock(). + * While checking, GetBestBlock() refers to the parent block. (protected by cs_main) + * This is also true for mempool checks. + */ + int GetSpendHeight(const CCoinsViewCache& inputs) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + ~BlockManager() { Unload(); } @@ -525,11 +533,6 @@ protected: */ mutable std::atomic<bool> m_cached_finished_ibd{false}; - //! Reference to a BlockManager instance which itself is shared across all - //! CChainState instances. Keeping a local reference allows us to test more - //! easily as opposed to referencing a global. - BlockManager& m_blockman; - //! mempool that is kept in sync with the chain CTxMemPool& m_mempool; @@ -537,6 +540,10 @@ protected: std::unique_ptr<CoinsViews> m_coins_views; public: + //! Reference to a BlockManager instance which itself is shared across all + //! CChainState instances. + BlockManager& m_blockman; + explicit CChainState(CTxMemPool& mempool, BlockManager& blockman, uint256 from_snapshot_blockhash = uint256()); /** @@ -613,6 +620,9 @@ public: bool ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + /** Import blocks from an external file */ + void LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFilePos* dbp = nullptr); + /** * Update the on-disk chain state. * The caches and indexes are flushed depending on the mode we're called with @@ -638,9 +648,10 @@ public: void PruneAndFlush(); /** - * Make the best chain active, in multiple steps. The result is either failure - * or an activated best chain. pblock is either nullptr or a pointer to a block - * that is already loaded (to avoid loading it again from disk). + * Find the best known block, and make it the tip of the block chain. The + * result is either failure or an activated best chain. pblock is either + * nullptr or a pointer to a block that is already loaded (to avoid loading + * it again from disk). * * ActivateBestChain is split into steps (see ActivateBestChainStep) so that * we avoid holding cs_main for an extended period of time; the length of this @@ -654,7 +665,7 @@ public: bool ActivateBestChain( BlockValidationState& state, const CChainParams& chainparams, - std::shared_ptr<const CBlock> pblock) LOCKS_EXCLUDED(cs_main); + std::shared_ptr<const CBlock> pblock = nullptr) LOCKS_EXCLUDED(cs_main); bool AcceptBlock(const std::shared_ptr<const CBlock>& pblock, BlockValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -817,7 +828,7 @@ private: //! This is especially important when, e.g., calling ActivateBestChain() //! on all chainstates because we are not able to hold ::cs_main going into //! that call. - CChainState* m_active_chainstate{nullptr}; + CChainState* m_active_chainstate GUARDED_BY(::cs_main) {nullptr}; //! If true, the assumed-valid chainstate has been fully validated //! by the background validation chainstate. @@ -945,13 +956,6 @@ CChain& ChainActive(); /** Global variable that points to the active block tree (protected by cs_main) */ extern std::unique_ptr<CBlockTreeDB> pblocktree; -/** - * Return the spend height, which is one more than the inputs.GetBestBlock(). - * While checking, GetBestBlock() refers to the parent block. (protected by cs_main) - * This is also true for mempool checks. - */ -int GetSpendHeight(const CCoinsViewCache& inputs); - extern VersionBitsCache versionbitscache; /** diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index a2dea84d17..10f89e3a6f 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -300,8 +300,26 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group ******************************************************************************/ -void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants) { +void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants, bool positive_only) { + // Compute the effective value first + const CAmount coin_fee = output.m_input_bytes < 0 ? 0 : m_effective_feerate.GetFee(output.m_input_bytes); + const CAmount ev = output.txout.nValue - coin_fee; + + // Filter for positive only here before adding the coin + if (positive_only && ev <= 0) return; + m_outputs.push_back(output); + CInputCoin& coin = m_outputs.back(); + + coin.m_fee = coin_fee; + fee += coin.m_fee; + + coin.m_long_term_fee = coin.m_input_bytes < 0 ? 0 : m_long_term_feerate.GetFee(coin.m_input_bytes); + long_term_fee += coin.m_long_term_fee; + + coin.effective_value = ev; + effective_value += coin.effective_value; + m_from_me &= from_me; m_value += output.txout.nValue; m_depth = std::min(m_depth, depth); @@ -312,20 +330,6 @@ void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size // descendants is the count as seen from the top ancestor, not the descendants as seen from the // coin itself; thus, this value is counted as the max, not the sum m_descendants = std::max(m_descendants, descendants); - effective_value += output.effective_value; - fee += output.m_fee; - long_term_fee += output.m_long_term_fee; -} - -std::vector<CInputCoin>::iterator OutputGroup::Discard(const CInputCoin& output) { - auto it = m_outputs.begin(); - while (it != m_outputs.end() && it->outpoint != output.outpoint) ++it; - if (it == m_outputs.end()) return it; - m_value -= output.txout.nValue; - effective_value -= output.effective_value; - fee -= output.m_fee; - long_term_fee -= output.m_long_term_fee; - return m_outputs.erase(it); } bool OutputGroup::EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const @@ -334,35 +338,3 @@ bool OutputGroup::EligibleForSpending(const CoinEligibilityFilter& eligibility_f && m_ancestors <= eligibility_filter.max_ancestors && m_descendants <= eligibility_filter.max_descendants; } - -void OutputGroup::SetFees(const CFeeRate effective_feerate, const CFeeRate long_term_feerate) -{ - fee = 0; - long_term_fee = 0; - effective_value = 0; - for (CInputCoin& coin : m_outputs) { - coin.m_fee = coin.m_input_bytes < 0 ? 0 : effective_feerate.GetFee(coin.m_input_bytes); - fee += coin.m_fee; - - coin.m_long_term_fee = coin.m_input_bytes < 0 ? 0 : long_term_feerate.GetFee(coin.m_input_bytes); - long_term_fee += coin.m_long_term_fee; - - coin.effective_value = coin.txout.nValue - coin.m_fee; - effective_value += coin.effective_value; - } -} - -OutputGroup OutputGroup::GetPositiveOnlyGroup() -{ - OutputGroup group(*this); - for (auto it = group.m_outputs.begin(); it != group.m_outputs.end(); ) { - const CInputCoin& coin = *it; - // Only include outputs that are positive effective value (i.e. not dust) - if (coin.effective_value <= 0) { - it = group.Discard(coin); - } else { - ++it; - } - } - return group; -} diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index cf9768b927..f0e1addaf1 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -6,11 +6,10 @@ #define BITCOIN_WALLET_COINSELECTION_H #include <amount.h> +#include <policy/feerate.h> #include <primitives/transaction.h> #include <random.h> -class CFeeRate; - //! target minimum change amount static constexpr CAmount MIN_CHANGE{COIN / 100}; //! final minimum change amount after paying for fees @@ -63,9 +62,11 @@ struct CoinEligibilityFilter const int conf_theirs; const uint64_t max_ancestors; const uint64_t max_descendants; + const bool m_include_partial_groups{false}; //! Include partial destination groups when avoid_reuse and there are full groups CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors), max_descendants(max_ancestors) {} CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors, uint64_t max_descendants) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors), max_descendants(max_descendants) {} + CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors, uint64_t max_descendants, bool include_partial) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors), max_descendants(max_descendants), m_include_partial_groups(include_partial) {} }; struct OutputGroup @@ -78,27 +79,18 @@ struct OutputGroup size_t m_descendants{0}; CAmount effective_value{0}; CAmount fee{0}; + CFeeRate m_effective_feerate{0}; CAmount long_term_fee{0}; + CFeeRate m_long_term_feerate{0}; OutputGroup() {} - OutputGroup(std::vector<CInputCoin>&& outputs, bool from_me, CAmount value, int depth, size_t ancestors, size_t descendants) - : m_outputs(std::move(outputs)) - , m_from_me(from_me) - , m_value(value) - , m_depth(depth) - , m_ancestors(ancestors) - , m_descendants(descendants) + OutputGroup(const CFeeRate& effective_feerate, const CFeeRate& long_term_feerate) : + m_effective_feerate(effective_feerate), + m_long_term_feerate(long_term_feerate) {} - OutputGroup(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants) : OutputGroup() { - Insert(output, depth, from_me, ancestors, descendants); - } - void Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants); - std::vector<CInputCoin>::iterator Discard(const CInputCoin& output); - bool EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const; - //! Update the OutputGroup's fee, long_term_fee, and effective_value based on the given feerates - void SetFees(const CFeeRate effective_feerate, const CFeeRate long_term_feerate); - OutputGroup GetPositiveOnlyGroup(); + void Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants, bool positive_only); + bool EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const; }; bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& target_value, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret, CAmount not_input_fees); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 202804c9ff..ffac78d752 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -35,7 +35,7 @@ static CAmount balance = 0; CoinEligibilityFilter filter_standard(1, 6, 0); CoinEligibilityFilter filter_confirmed(1, 1, 0); CoinEligibilityFilter filter_standard_extra(6, 6, 0); -CoinSelectionParams coin_selection_params(false, 0, 0, CFeeRate(0), 0); +CoinSelectionParams coin_selection_params(false, 0, 0, CFeeRate(0), 0, false); static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set) { @@ -115,7 +115,10 @@ inline std::vector<OutputGroup>& GroupCoins(const std::vector<CInputCoin>& coins { static std::vector<OutputGroup> static_groups; static_groups.clear(); - for (auto& coin : coins) static_groups.emplace_back(coin, 0, true, 0, 0); + for (auto& coin : coins) { + static_groups.emplace_back(); + static_groups.back().Insert(coin, 0, true, 0, 0, false); + } return static_groups; } @@ -123,7 +126,10 @@ inline std::vector<OutputGroup>& GroupCoins(const std::vector<COutput>& coins) { static std::vector<OutputGroup> static_groups; static_groups.clear(); - for (auto& coin : coins) static_groups.emplace_back(coin.GetInputCoin(), coin.nDepth, coin.tx->m_amounts[CWalletTx::DEBIT].m_cached[ISMINE_SPENDABLE] && coin.tx->m_amounts[CWalletTx::DEBIT].m_value[ISMINE_SPENDABLE] == 1 /* HACK: we can't figure out the is_me flag so we use the conditions defined above; perhaps set safe to false for !fIsFromMe in add_coin() */, 0, 0); + for (auto& coin : coins) { + static_groups.emplace_back(); + static_groups.back().Insert(coin.GetInputCoin(), coin.nDepth, coin.tx->m_amounts[CWalletTx::DEBIT].m_cached[ISMINE_SPENDABLE] && coin.tx->m_amounts[CWalletTx::DEBIT].m_value[ISMINE_SPENDABLE] == 1 /* HACK: we can't figure out the is_me flag so we use the conditions defined above; perhaps set safe to false for !fIsFromMe in add_coin() */, 0, 0, false); + } return static_groups; } @@ -263,22 +269,22 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) } // Make sure that effective value is working in SelectCoinsMinConf when BnB is used - CoinSelectionParams coin_selection_params_bnb(true, 0, 0, CFeeRate(3000), 0); + CoinSelectionParams coin_selection_params_bnb(true, 0, 0, CFeeRate(3000), 0, false); CoinSet setCoinsRet; CAmount nValueRet; bool bnb_used; empty_wallet(); add_coin(1); vCoins.at(0).nInputBytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); // Test fees subtracted from output: empty_wallet(); add_coin(1 * CENT); vCoins.at(0).nInputBytes = 40; - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); coin_selection_params_bnb.m_subtract_fee_outputs = true; - BOOST_CHECK(testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); // Make sure that can use BnB when there are preset inputs @@ -317,24 +323,24 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) empty_wallet(); // with an empty wallet we can't even pay one cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); add_coin(1*CENT, 4); // add a new 1 cent coin // with a new 1 cent coin, we still can't find a mature 1 cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // but we can find a new 1 cent - BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); add_coin(2*CENT); // add a mature 2 cent coin // we can't make 3 cents of mature coins - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 3 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 3 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // we can make 3 cents of new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); add_coin(5*CENT); // add a mature 5 cent coin, @@ -344,33 +350,33 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27. total = 38 // we can't make 38 cents only if we disallow new coins: - BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // we can't even make 37 cents if we don't allow new coins even if they're from us - BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard_extra, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard_extra, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); // and we can make 38 cents if we accept all new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 38 * CENT); // try making 34 cents from 1,2,5,10,20 - we can't do it exactly - BOOST_CHECK( testWallet.SelectCoinsMinConf(34 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(34 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 35 * CENT); // but 35 cents is closest BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) // when we try making 7 cents, the smaller coins (1,2,5) are enough. We should see just 2+5 - BOOST_CHECK( testWallet.SelectCoinsMinConf( 7 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 7 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 7 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. - BOOST_CHECK( testWallet.SelectCoinsMinConf( 8 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 8 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK(nValueRet == 8 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) - BOOST_CHECK( testWallet.SelectCoinsMinConf( 9 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 9 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -384,30 +390,30 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(30*CENT); // now we have 6+7+8+20+30 = 71 cents total // check that we have 71 and not 72 - BOOST_CHECK( testWallet.SelectCoinsMinConf(71 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); - BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(71 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // now try making 16 cents. the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 20 * CENT); // we should get 20 in one coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); add_coin( 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 3 coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); add_coin( 18*CENT); // now we have 5+6+7+8+18+20+30 // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // because in the event of a tie, the biggest coin wins // now try making 11 cents. we should get 5+6 - BOOST_CHECK( testWallet.SelectCoinsMinConf(11 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(11 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -416,11 +422,11 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin( 2*COIN); add_coin( 3*COIN); add_coin( 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents - BOOST_CHECK( testWallet.SelectCoinsMinConf(95 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(95 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * COIN); // we should get 1 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - BOOST_CHECK( testWallet.SelectCoinsMinConf(195 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(195 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -435,14 +441,14 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // try making 1 * MIN_CHANGE from the 1.5 * MIN_CHANGE // we'll get change smaller than MIN_CHANGE whatever happens, so can expect MIN_CHANGE exactly - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // but if we add a bigger coin, small change is avoided add_coin(1111*MIN_CHANGE); // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // if we add more small coins: @@ -450,7 +456,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 7 / 10); // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // run the 'mtgox' test (see https://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) @@ -459,7 +465,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) for (int j = 0; j < 20; j++) add_coin(50000 * COIN); - BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins @@ -472,7 +478,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 6 / 10); add_coin(MIN_CHANGE * 7 / 10); add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -482,7 +488,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 6 / 10); add_coin(MIN_CHANGE * 8 / 10); add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // in two coins 0.4+0.6 @@ -493,12 +499,12 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 100); // trying to make 100.01 from these three coins - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE * 10105 / 100); // we should get all coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); } @@ -512,7 +518,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // We only create the wallet once to save time, but we still run the coin selection RUN_TESTS times. for (int i = 0; i < RUN_TESTS; i++) { - BOOST_CHECK(testWallet.SelectCoinsMinConf(2000, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(2000, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); if (amt - 2000 < MIN_CHANGE) { // needs more than one input: @@ -538,8 +544,8 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) for (int i = 0; i < RUN_TESTS; i++) { // picking 50 from 100 coins doesn't depend on the shuffle, // but does depend on randomness in the stochastic approximation code - BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, GroupCoins(vCoins), setCoinsRet , nValueRet, coin_selection_params, bnb_used)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, GroupCoins(vCoins), setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK(!equal_sets(setCoinsRet, setCoinsRet2)); int fails = 0; @@ -547,8 +553,8 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) { // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time // run the test RANDOM_REPEATS times and only complain if all of them fail - BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, GroupCoins(vCoins), setCoinsRet , nValueRet, coin_selection_params, bnb_used)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, GroupCoins(vCoins), setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } @@ -570,8 +576,8 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) { // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time // run the test RANDOM_REPEATS times and only complain if all of them fail - BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, GroupCoins(vCoins), setCoinsRet , nValueRet, coin_selection_params, bnb_used)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, GroupCoins(vCoins), setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } @@ -598,7 +604,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) add_coin(1000 * COIN); add_coin(3 * COIN); - BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -633,13 +639,13 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) CAmount target = rand.randrange(balance - 1000) + 1000; // Perform selection - CoinSelectionParams coin_selection_params_knapsack(false, 34, 148, CFeeRate(0), 0); - CoinSelectionParams coin_selection_params_bnb(true, 34, 148, CFeeRate(0), 0); + CoinSelectionParams coin_selection_params_knapsack(false, 34, 148, CFeeRate(0), 0, false); + CoinSelectionParams coin_selection_params_bnb(true, 34, 148, CFeeRate(0), 0, false); CoinSet out_set; CAmount out_value = 0; bool bnb_used = false; - BOOST_CHECK(testWallet.SelectCoinsMinConf(target, filter_standard, GroupCoins(vCoins), out_set, out_value, coin_selection_params_bnb, bnb_used) || - testWallet.SelectCoinsMinConf(target, filter_standard, GroupCoins(vCoins), out_set, out_value, coin_selection_params_knapsack, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(target, filter_standard, vCoins, out_set, out_value, coin_selection_params_bnb, bnb_used) || + testWallet.SelectCoinsMinConf(target, filter_standard, vCoins, out_set, out_value, coin_selection_params_knapsack, bnb_used)); BOOST_CHECK_GE(out_value, target); } } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 723552860a..db80745db0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2357,13 +2357,12 @@ const CTxOut& CWallet::FindNonChangeParentOutput(const CTransaction& tx, int out return ptx->vout[n]; } -bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<OutputGroup> groups, +bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<COutput> coins, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const { setCoinsRet.clear(); nValueRet = 0; - std::vector<OutputGroup> utxo_pool; if (coin_selection_params.use_bnb) { // Get long term estimate FeeCalculation feeCalc; @@ -2371,35 +2370,27 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil temp.m_confirm_target = 1008; CFeeRate long_term_feerate = GetMinimumFeeRate(*this, temp, &feeCalc); - // Calculate cost of change - CAmount cost_of_change = GetDiscardRate(*this).GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size); + // Get the feerate for effective value. + // When subtracting the fee from the outputs, we want the effective feerate to be 0 + CFeeRate effective_feerate{0}; + if (!coin_selection_params.m_subtract_fee_outputs) { + effective_feerate = coin_selection_params.effective_fee; + } - // Filter by the min conf specs and add to utxo_pool and calculate effective value - for (OutputGroup& group : groups) { - if (!group.EligibleForSpending(eligibility_filter)) continue; + std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, long_term_feerate, eligibility_filter, true /* positive_only */); - if (coin_selection_params.m_subtract_fee_outputs) { - // Set the effective feerate to 0 as we don't want to use the effective value since the fees will be deducted from the output - group.SetFees(CFeeRate(0) /* effective_feerate */, long_term_feerate); - } else { - group.SetFees(coin_selection_params.effective_fee, long_term_feerate); - } + // Calculate cost of change + CAmount cost_of_change = GetDiscardRate(*this).GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size); - OutputGroup pos_group = group.GetPositiveOnlyGroup(); - if (pos_group.effective_value > 0) utxo_pool.push_back(pos_group); - } // Calculate the fees for things that aren't inputs CAmount not_input_fees = coin_selection_params.effective_fee.GetFee(coin_selection_params.tx_noinputs_size); bnb_used = true; - return SelectCoinsBnB(utxo_pool, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees); + return SelectCoinsBnB(groups, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees); } else { - // Filter by the min conf specs and add to utxo_pool - for (const OutputGroup& group : groups) { - if (!group.EligibleForSpending(eligibility_filter)) continue; - utxo_pool.push_back(group); - } + std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, CFeeRate(0), CFeeRate(0), eligibility_filter, false /* positive_only */); + bnb_used = false; - return KnapsackSolver(nTargetValue, utxo_pool, setCoinsRet, nValueRet); + return KnapsackSolver(nTargetValue, groups, setCoinsRet, nValueRet); } } @@ -2482,16 +2473,14 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm // explicitly shuffling the outputs before processing Shuffle(vCoins.begin(), vCoins.end(), FastRandomContext()); } - std::vector<OutputGroup> groups = GroupOutputs(vCoins, !coin_control.m_avoid_partial_spends, max_ancestors); - bool res = value_to_select <= 0 || - SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 6, 0), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || - SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 1, 0), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || - (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, 2), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || - (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || - (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || - (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || - (m_spend_zero_conf_change && !fRejectLongChains && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max()), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 6, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || + SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 1, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || + (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, 2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1, true /* include_partial_groups */), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (m_spend_zero_conf_change && !fRejectLongChains && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max(), std::numeric_limits<uint64_t>::max(), true /* include_partial_groups */), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // because SelectCoinsMinConf clears the setCoinsRet, we now add the possible inputs to the coinset util::insert(setCoinsRet, setPresetCoins); @@ -2782,6 +2771,7 @@ bool CWallet::CreateTransactionInternal( std::vector<COutput> vAvailableCoins; AvailableCoins(vAvailableCoins, true, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0); CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy + coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends; // Create change script that will be used if we need change // TODO: pass in scriptChange instead of reservedest so @@ -4192,51 +4182,90 @@ bool CWalletTx::IsImmatureCoinBase() const return GetBlocksToMaturity() > 0; } -std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outputs, bool single_coin, const size_t max_ancestors) const { - std::vector<OutputGroup> groups; - std::map<CTxDestination, OutputGroup> gmap; - std::set<CTxDestination> full_groups; +std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outputs, bool separate_coins, const CFeeRate& effective_feerate, const CFeeRate& long_term_feerate, const CoinEligibilityFilter& filter, bool positive_only) const +{ + std::vector<OutputGroup> groups_out; - for (const auto& output : outputs) { - if (output.fSpendable) { - CTxDestination dst; - CInputCoin input_coin = output.GetInputCoin(); + if (separate_coins) { + // Single coin means no grouping. Each COutput gets its own OutputGroup. + for (const COutput& output : outputs) { + // Skip outputs we cannot spend + if (!output.fSpendable) continue; size_t ancestors, descendants; chain().getTransactionAncestry(output.tx->GetHash(), ancestors, descendants); - if (!single_coin && ExtractDestination(output.tx->tx->vout[output.i].scriptPubKey, dst)) { - auto it = gmap.find(dst); - if (it != gmap.end()) { - // Limit output groups to no more than OUTPUT_GROUP_MAX_ENTRIES - // number of entries, to protect against inadvertently creating - // a too-large transaction when using -avoidpartialspends to - // prevent breaking consensus or surprising users with a very - // high amount of fees. - if (it->second.m_outputs.size() >= OUTPUT_GROUP_MAX_ENTRIES) { - groups.push_back(it->second); - it->second = OutputGroup{}; - full_groups.insert(dst); - } - it->second.Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants); - } else { - gmap[dst].Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants); - } - } else { - groups.emplace_back(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants); - } + CInputCoin input_coin = output.GetInputCoin(); + + // Make an OutputGroup containing just this output + OutputGroup group{effective_feerate, long_term_feerate}; + group.Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants, positive_only); + + // Check the OutputGroup's eligibility. Only add the eligible ones. + if (positive_only && group.effective_value <= 0) continue; + if (group.m_outputs.size() > 0 && group.EligibleForSpending(filter)) groups_out.push_back(group); + } + return groups_out; + } + + // We want to combine COutputs that have the same scriptPubKey into single OutputGroups + // except when there are more than OUTPUT_GROUP_MAX_ENTRIES COutputs grouped in an OutputGroup. + // To do this, we maintain a map where the key is the scriptPubKey and the value is a vector of OutputGroups. + // For each COutput, we check if the scriptPubKey is in the map, and if it is, the COutput's CInputCoin is added + // to the last OutputGroup in the vector for the scriptPubKey. When the last OutputGroup has + // OUTPUT_GROUP_MAX_ENTRIES CInputCoins, a new OutputGroup is added to the end of the vector. + std::map<CScript, std::vector<OutputGroup>> spk_to_groups_map; + for (const auto& output : outputs) { + // Skip outputs we cannot spend + if (!output.fSpendable) continue; + + size_t ancestors, descendants; + chain().getTransactionAncestry(output.tx->GetHash(), ancestors, descendants); + CInputCoin input_coin = output.GetInputCoin(); + CScript spk = input_coin.txout.scriptPubKey; + + std::vector<OutputGroup>& groups = spk_to_groups_map[spk]; + + if (groups.size() == 0) { + // No OutputGroups for this scriptPubKey yet, add one + groups.emplace_back(effective_feerate, long_term_feerate); } + + // Get the last OutputGroup in the vector so that we can add the CInputCoin to it + // A pointer is used here so that group can be reassigned later if it is full. + OutputGroup* group = &groups.back(); + + // Check if this OutputGroup is full. We limit to OUTPUT_GROUP_MAX_ENTRIES when using -avoidpartialspends + // to avoid surprising users with very high fees. + if (group->m_outputs.size() >= OUTPUT_GROUP_MAX_ENTRIES) { + // The last output group is full, add a new group to the vector and use that group for the insertion + groups.emplace_back(effective_feerate, long_term_feerate); + group = &groups.back(); + } + + // Add the input_coin to group + group->Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants, positive_only); } - if (!single_coin) { - for (auto& it : gmap) { - auto& group = it.second; - if (full_groups.count(it.first) > 0) { - // Make this unattractive as we want coin selection to avoid it if possible - group.m_ancestors = max_ancestors - 1; + + // Now we go through the entire map and pull out the OutputGroups + for (const auto& spk_and_groups_pair: spk_to_groups_map) { + const std::vector<OutputGroup>& groups_per_spk= spk_and_groups_pair.second; + + // Go through the vector backwards. This allows for the first item we deal with being the partial group. + for (auto group_it = groups_per_spk.rbegin(); group_it != groups_per_spk.rend(); group_it++) { + const OutputGroup& group = *group_it; + + // Don't include partial groups if there are full groups too and we don't want partial groups + if (group_it == groups_per_spk.rbegin() && groups_per_spk.size() > 1 && !filter.m_include_partial_groups) { + continue; } - groups.push_back(group); + + // Check the OutputGroup's eligibility. Only add the eligible ones. + if (positive_only && group.effective_value <= 0) continue; + if (group.m_outputs.size() > 0 && group.EligibleForSpending(filter)) groups_out.push_back(group); } } - return groups; + + return groups_out; } bool CWallet::IsCrypted() const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e3eae1dd95..4fc4466604 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -610,8 +610,16 @@ struct CoinSelectionParams size_t tx_noinputs_size = 0; //! Indicate that we are subtracting the fee from outputs bool m_subtract_fee_outputs = false; - - CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_fee, size_t tx_noinputs_size) : use_bnb(use_bnb), change_output_size(change_output_size), change_spend_size(change_spend_size), effective_fee(effective_fee), tx_noinputs_size(tx_noinputs_size) {} + bool m_avoid_partial_spends = false; + + CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_fee, size_t tx_noinputs_size, bool avoid_partial) : + use_bnb(use_bnb), + change_output_size(change_output_size), + change_spend_size(change_spend_size), + effective_fee(effective_fee), + tx_noinputs_size(tx_noinputs_size), + m_avoid_partial_spends(avoid_partial) + {} CoinSelectionParams() {} }; @@ -818,7 +826,7 @@ public: * completion the coin set and corresponding actual target value is * assembled */ - bool SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<OutputGroup> groups, + bool SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<COutput> coins, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const; bool IsSpent(const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -827,7 +835,7 @@ public: bool IsSpentKey(const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned int n, bool used, std::set<CTxDestination>& tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - std::vector<OutputGroup> GroupOutputs(const std::vector<COutput>& outputs, bool single_coin, const size_t max_ancestors) const; + std::vector<OutputGroup> GroupOutputs(const std::vector<COutput>& outputs, bool separate_coins, const CFeeRate& effective_feerate, const CFeeRate& long_term_feerate, const CoinEligibilityFilter& filter, bool positive_only) const; bool IsLockedCoin(uint256 hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void LockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 7c5317480c..4bda73599d 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -18,6 +18,7 @@ import sys import tempfile import time +from typing import List from .authproxy import JSONRPCException from . import coverage from .p2p import NetworkThread @@ -89,14 +90,11 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): This class also contains various public and private helper methods.""" - chain = None # type: str - setup_clean_chain = None # type: bool - def __init__(self): """Sets test framework defaults. Do not override this method. Instead, override the set_test_params() method""" - self.chain = 'regtest' - self.setup_clean_chain = False - self.nodes = [] + self.chain: str = 'regtest' + self.setup_clean_chain: bool = False + self.nodes: List[TestNode] = [] self.network_thread = None self.rpc_timeout = 60 # Wait for up to 60 seconds for the RPC server to respond self.supports_cli = True |