diff options
Diffstat (limited to 'src')
86 files changed, 1678 insertions, 1224 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index af6bcaf4d7..e14b5ec040 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -606,7 +606,6 @@ libbitcoin_util_a_SOURCES = \ chainparamsbase.cpp \ clientversion.cpp \ compat/glibcxx_sanity.cpp \ - compat/strnlen.cpp \ fs.cpp \ interfaces/echo.cpp \ interfaces/handler.cpp \ @@ -621,6 +620,7 @@ libbitcoin_util_a_SOURCES = \ util/asmap.cpp \ util/bip32.cpp \ util/bytevectorhash.cpp \ + util/check.cpp \ util/error.cpp \ util/fees.cpp \ util/getuniquepath.cpp \ @@ -839,6 +839,7 @@ bitcoin_chainstate_SOURCES = \ uint256.cpp \ util/asmap.cpp \ util/bytevectorhash.cpp \ + util/check.cpp \ util/getuniquepath.cpp \ util/hasher.cpp \ util/moneystr.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index a7505b9bcf..5d25104327 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -176,8 +176,11 @@ if USE_BDB BITCOIN_TESTS += wallet/test/db_tests.cpp endif -if USE_SQLITE FUZZ_WALLET_SRC = \ + wallet/test/fuzz/coinselection.cpp + +if USE_SQLITE +FUZZ_WALLET_SRC += \ wallet/test/fuzz/notifications.cpp endif # USE_SQLITE @@ -385,8 +388,19 @@ univalue_test_object_LDFLAGS = -static $(LIBTOOL_APP_LDFLAGS) endif %.cpp.test: %.cpp - @echo Running tests: `cat $< | grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1` from $< - $(AM_V_at)$(TEST_BINARY) --catch_system_errors=no -l test_suite -t "`cat $< | grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1`" -- DEBUG_LOG_OUT > $<.log 2>&1 || (cat $<.log && false) + @echo Running tests: $$(\ + cat $< | \ + grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | \ + cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1\ + ) from $< + $(AM_V_at)export TEST_LOGFILE=$(abs_builddir)/$$(\ + echo $< | grep -E -o "(wallet/test/.*\.cpp|test/.*\.cpp)" | $(SED) -e s/\.cpp/.log/ \ + ) && \ + $(TEST_BINARY) --catch_system_errors=no -l test_suite -t "$$(\ + cat $< | \ + grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | \ + cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1\ + )" -- DEBUG_LOG_OUT > "$$TEST_LOGFILE" 2>&1 || (cat "$$TEST_LOGFILE" && false) %.json.h: %.json @$(MKDIR_P) $(@D) diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 0fa8f3c3da..1fa2644647 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -185,7 +185,7 @@ void ReadFromStream(AddrMan& addr, CDataStream& ssPeers) std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const ArgsManager& args, std::unique_ptr<AddrMan>& addrman) { auto check_addrman = std::clamp<int32_t>(args.GetIntArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000); - addrman = std::make_unique<AddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); + addrman = std::make_unique<AddrMan>(asmap, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); int64_t nStart = GetTimeMillis(); const auto path_addr{args.GetDataDirNet() / "peers.dat"}; @@ -194,7 +194,7 @@ std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const A LogPrintf("Loaded %i addresses from peers.dat %dms\n", addrman->size(), GetTimeMillis() - nStart); } catch (const DbNotFoundError&) { // Addrman can be in an inconsistent state after failure, reset it - addrman = std::make_unique<AddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); + addrman = std::make_unique<AddrMan>(asmap, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); LogPrintf("Creating peers.dat because the file was not found (%s)\n", fs::quoted(fs::PathToString(path_addr))); DumpPeerAddresses(args, *addrman); } catch (const InvalidAddrManVersionError&) { @@ -203,7 +203,7 @@ std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const A return strprintf(_("Failed to rename invalid peers.dat file. Please move or delete it and try again.")); } // Addrman can be in an inconsistent state after failure, reset it - addrman = std::make_unique<AddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); + addrman = std::make_unique<AddrMan>(asmap, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); LogPrintf("Creating new peers.dat because the file version was not compatible (%s). Original backed up to peers.dat.bak\n", fs::quoted(fs::PathToString(path_addr))); DumpPeerAddresses(args, *addrman); } catch (const std::exception& e) { diff --git a/src/addrman.cpp b/src/addrman.cpp index 2fd8143c1c..2a08d99eef 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -946,16 +946,16 @@ std::optional<AddressPosition> AddrManImpl::FindAddressEntry_(const CAddress& ad if(addr_info->fInTried) { int bucket{addr_info->GetTriedBucket(nKey, m_asmap)}; - return AddressPosition(/*tried=*/true, - /*multiplicity=*/1, - /*bucket=*/bucket, - /*position=*/addr_info->GetBucketPosition(nKey, false, bucket)); + return AddressPosition(/*tried_in=*/true, + /*multiplicity_in=*/1, + /*bucket_in=*/bucket, + /*position_in=*/addr_info->GetBucketPosition(nKey, false, bucket)); } else { int bucket{addr_info->GetNewBucket(nKey, m_asmap)}; - return AddressPosition(/*tried=*/false, - /*multiplicity=*/addr_info->nRefCount, - /*bucket=*/bucket, - /*position=*/addr_info->GetBucketPosition(nKey, true, bucket)); + return AddressPosition(/*tried_in=*/false, + /*multiplicity_in=*/addr_info->nRefCount, + /*bucket_in=*/bucket, + /*position_in=*/addr_info->GetBucketPosition(nKey, true, bucket)); } } diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp index 3ca58b923e..34bc4380dd 100644 --- a/src/bench/addrman.cpp +++ b/src/bench/addrman.cpp @@ -101,7 +101,7 @@ static void AddrManGetAddr(benchmark::Bench& bench) FillAddrMan(addrman); bench.run([&] { - const auto& addresses = addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23, /* network */ std::nullopt); + const auto& addresses = addrman.GetAddr(/*max_addresses=*/2500, /*max_pct=*/23, /*network=*/std::nullopt); assert(addresses.size() > 0); }); } diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index a9f05f05ea..de8ab9807c 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -13,7 +13,7 @@ using node::NodeContext; using wallet::AttemptSelection; -using wallet::CInputCoin; +using wallet::CHANGE_LOWER; using wallet::COutput; using wallet::CWallet; using wallet::CWalletTx; @@ -58,20 +58,21 @@ static void CoinSelection(benchmark::Bench& bench) // Create coins std::vector<COutput> coins; for (const auto& wtx : wtxs) { - coins.emplace_back(wallet, *wtx, 0 /* iIn */, 6 * 24 /* nDepthIn */, true /* spendable */, true /* solvable */, true /* safe */); + coins.emplace_back(COutPoint(wtx->GetHash(), 0), wtx->tx->vout.at(0), /*depth=*/6 * 24, GetTxSpendSize(wallet, *wtx, 0), /*spendable=*/true, /*solvable=*/true, /*safe=*/true, wtx->GetTxTime(), /*from_me=*/true); } const CoinEligibilityFilter filter_standard(1, 6, 0); FastRandomContext rand{}; const CoinSelectionParams coin_selection_params{ rand, - /* change_output_size= */ 34, - /* change_spend_size= */ 148, - /* effective_feerate= */ CFeeRate(0), - /* long_term_feerate= */ CFeeRate(0), - /* discard_feerate= */ CFeeRate(0), - /* tx_noinputs_size= */ 0, - /* avoid_partial= */ false, + /*change_output_size=*/ 34, + /*change_spend_size=*/ 148, + /*min_change_target=*/ CHANGE_LOWER, + /*effective_feerate=*/ CFeeRate(0), + /*long_term_feerate=*/ CFeeRate(0), + /*discard_feerate=*/ CFeeRate(0), + /*tx_noinputs_size=*/ 0, + /*avoid_partial=*/ false, }; bench.run([&] { auto result = AttemptSelection(wallet, 1003 * COIN, filter_standard, coins, coin_selection_params); @@ -81,17 +82,15 @@ static void CoinSelection(benchmark::Bench& bench) }); } -typedef std::set<CInputCoin> CoinSet; - // Copied from src/wallet/test/coinselector_tests.cpp static void add_coin(const CAmount& nValue, int nInput, std::vector<OutputGroup>& set) { CMutableTransaction tx; tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; - CInputCoin coin(MakeTransactionRef(tx), nInput); + COutput output(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 0, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ true); set.emplace_back(); - set.back().Insert(coin, 0, true, 0, 0, false); + set.back().Insert(output, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ false); } // Copied from src/wallet/test/coinselector_tests.cpp static CAmount make_hard_case(int utxos, std::vector<OutputGroup>& utxo_pool) diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp index afa4618e1b..a58658c4f1 100644 --- a/src/bench/mempool_stress.cpp +++ b/src/bench/mempool_stress.cpp @@ -86,7 +86,7 @@ static void ComplexMemPool(benchmark::Bench& bench) if (bench.complexityN() > 1) { childTxs = static_cast<int>(bench.complexityN()); } - std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /* min_ancestors */ 1); + std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /*min_ancestors=*/1); const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN); CTxMemPool pool; LOCK2(cs_main, pool.cs); @@ -103,7 +103,7 @@ static void MempoolCheck(benchmark::Bench& bench) { FastRandomContext det_rand{true}; const int childTxs = bench.complexityN() > 1 ? static_cast<int>(bench.complexityN()) : 2000; - const std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /* min_ancestors */ 5); + const std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /*min_ancestors=*/5); const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN, {"-checkmempool=1"}); CTxMemPool pool; LOCK2(cs_main, pool.cs); @@ -111,7 +111,7 @@ static void MempoolCheck(benchmark::Bench& bench) for (auto& tx : ordered_coins) AddTx(tx, pool); bench.run([&]() NO_THREAD_SAFETY_ANALYSIS { - pool.check(coins_tip, /* spendheight */ 2); + pool.check(coins_tip, /*spendheight=*/2); }); } diff --git a/src/bench/rpc_mempool.cpp b/src/bench/rpc_mempool.cpp index 64e4c46899..6e322ba6aa 100644 --- a/src/bench/rpc_mempool.cpp +++ b/src/bench/rpc_mempool.cpp @@ -29,11 +29,11 @@ static void RpcMempool(benchmark::Bench& bench) tx.vout[0].scriptPubKey = CScript() << OP_1 << OP_EQUAL; tx.vout[0].nValue = i; const CTransactionRef tx_r{MakeTransactionRef(tx)}; - AddTx(tx_r, /* fee */ i, pool); + AddTx(tx_r, /*fee=*/i, pool); } bench.run([&] { - (void)MempoolToJSON(pool, /*verbose*/ true); + (void)MempoolToJSON(pool, /*verbose=*/true); }); } diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index d4b8794c6d..a5dd2f3bb1 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -52,10 +52,10 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b }); } -static void WalletBalanceDirty(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ true, /* add_mine */ true); } -static void WalletBalanceClean(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_mine */ true); } -static void WalletBalanceMine(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_mine */ true); } -static void WalletBalanceWatch(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_mine */ false); } +static void WalletBalanceDirty(benchmark::Bench& bench) { WalletBalance(bench, /*set_dirty=*/true, /*add_mine=*/true); } +static void WalletBalanceClean(benchmark::Bench& bench) { WalletBalance(bench, /*set_dirty=*/false, /*add_mine=*/true); } +static void WalletBalanceMine(benchmark::Bench& bench) { WalletBalance(bench, /*set_dirty=*/false, /*add_mine=*/true); } +static void WalletBalanceWatch(benchmark::Bench& bench) { WalletBalance(bench, /*set_dirty=*/false, /*add_mine=*/false); } BENCHMARK(WalletBalanceDirty); BENCHMARK(WalletBalanceClean); diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 72b8fefcc7..fcbb6aacce 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -192,7 +192,7 @@ int main(int argc, char* argv[]) bool new_block; auto sc = std::make_shared<submitblock_StateCatcher>(block.GetHash()); RegisterSharedValidationInterface(sc); - bool accepted = chainman.ProcessNewBlock(chainparams, blockptr, /* force_processing */ true, /* new_block */ &new_block); + bool accepted = chainman.ProcessNewBlock(chainparams, blockptr, /*force_processing=*/true, /*new_block=*/&new_block); UnregisterSharedValidationInterface(sc); if (!new_block && accepted) { std::cerr << "duplicate" << std::endl; diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index b297081cab..0b40626595 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -750,7 +750,7 @@ static void MutateTx(CMutableTransaction& tx, const std::string& command, static void OutputTxJSON(const CTransaction& tx) { UniValue entry(UniValue::VOBJ); - TxToUniv(tx, uint256(), entry); + TxToUniv(tx, /*block_hash=*/uint256(), entry); std::string jsonOutput = entry.write(4); tfm::format(std::cout, "%s\n", jsonOutput); diff --git a/src/bitcoin-util.cpp b/src/bitcoin-util.cpp index b457e0b354..a2f3fca26c 100644 --- a/src/bitcoin-util.cpp +++ b/src/bitcoin-util.cpp @@ -22,8 +22,6 @@ #include <memory> #include <thread> -#include <boost/algorithm/string.hpp> - static const int CONTINUE_EXECUTION=-1; const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr; diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 93510e925f..c65a816a5f 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -9,6 +9,7 @@ #include <consensus/merkle.h> #include <deploymentinfo.h> #include <hash.h> // for signet block challenge hash +#include <script/interpreter.h> #include <util/system.h> #include <assert.h> @@ -65,7 +66,10 @@ public: consensus.signet_blocks = false; consensus.signet_challenge.clear(); consensus.nSubsidyHalvingInterval = 210000; - consensus.BIP16Exception = uint256S("0x00000000000002dc756eebf4f49723ed8d30cc28a5f108eb94b1ba88ac4f9c22"); + consensus.script_flag_exceptions.emplace( // BIP16 exception + uint256S("0x00000000000002dc756eebf4f49723ed8d30cc28a5f108eb94b1ba88ac4f9c22"), SCRIPT_VERIFY_NONE); + consensus.script_flag_exceptions.emplace( // Taproot exception + uint256S("0x0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad"), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS); consensus.BIP34Height = 227931; consensus.BIP34Hash = uint256S("0x000000000000024b89b42a942fe0d9fea3bb44ab7bd1b19115dd6a759c0808b8"); consensus.BIP65Height = 388381; // 000000000000000004c2b624ed5d7756c508d90fd0da2c7c679febfa6c4735f0 @@ -184,7 +188,8 @@ public: consensus.signet_blocks = false; consensus.signet_challenge.clear(); consensus.nSubsidyHalvingInterval = 210000; - consensus.BIP16Exception = uint256S("0x00000000dd30457c001f4095d208cc1296b0eed002427aa599874af7a432b105"); + consensus.script_flag_exceptions.emplace( // BIP16 exception + uint256S("0x00000000dd30457c001f4095d208cc1296b0eed002427aa599874af7a432b105"), SCRIPT_VERIFY_NONE); consensus.BIP34Height = 21111; consensus.BIP34Hash = uint256S("0x0000000023b3a96d3484e5abb3755c413e7d41500f8e2a5c3f0dd01299cd8ef8"); consensus.BIP65Height = 581885; // 00000000007f6655f22f98e72ed80d8b06dc761d5da09df0fa1dc4be4f861eb6 @@ -323,7 +328,6 @@ public: consensus.signet_blocks = true; consensus.signet_challenge.assign(bin.begin(), bin.end()); consensus.nSubsidyHalvingInterval = 210000; - consensus.BIP16Exception = uint256{}; consensus.BIP34Height = 1; consensus.BIP34Hash = uint256{}; consensus.BIP65Height = 1; @@ -391,7 +395,6 @@ public: consensus.signet_blocks = false; consensus.signet_challenge.clear(); consensus.nSubsidyHalvingInterval = 150; - consensus.BIP16Exception = uint256(); consensus.BIP34Height = 1; // Always active unless overridden consensus.BIP34Hash = uint256(); consensus.BIP65Height = 1; // Always active unless overridden diff --git a/src/compat.h b/src/compat.h index 237b881b11..3ec4ab53fd 100644 --- a/src/compat.h +++ b/src/compat.h @@ -80,10 +80,6 @@ typedef int32_t ssize_t; #endif #endif -#if HAVE_DECL_STRNLEN == 0 -size_t strnlen( const char *start, size_t max_len); -#endif // HAVE_DECL_STRNLEN - #ifndef WIN32 typedef void* sockopt_arg_type; #else diff --git a/src/compat/strnlen.cpp b/src/compat/strnlen.cpp deleted file mode 100644 index 93a034a664..0000000000 --- a/src/compat/strnlen.cpp +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright (c) 2009-2018 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#if defined(HAVE_CONFIG_H) -#include <config/bitcoin-config.h> -#endif - -#include <cstring> - -#if HAVE_DECL_STRNLEN == 0 -size_t strnlen( const char *start, size_t max_len) -{ - const char *end = (const char *)memchr(start, '\0', max_len); - - return end ? (size_t)(end - start) : max_len; -} -#endif // HAVE_DECL_STRNLEN diff --git a/src/consensus/params.h b/src/consensus/params.h index 1ed5ca469f..0881f2e388 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -7,7 +7,9 @@ #define BITCOIN_CONSENSUS_PARAMS_H #include <uint256.h> + #include <limits> +#include <map> namespace Consensus { @@ -70,8 +72,13 @@ struct BIP9Deployment { struct Params { uint256 hashGenesisBlock; int nSubsidyHalvingInterval; - /* Block hash that is excepted from BIP16 enforcement */ - uint256 BIP16Exception; + /** + * Hashes of blocks that + * - are known to be consensus valid, and + * - buried in the chain, and + * - fail if the default script verify flags are applied. + */ + std::map<uint256, uint32_t> script_flag_exceptions; /** Block height and hash at which BIP34 becomes active */ int BIP34Height; uint256 BIP34Hash; diff --git a/src/core_io.h b/src/core_io.h index 69b9ac3ebd..aa1381c374 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -53,8 +53,7 @@ UniValue ValueFromAmount(const CAmount amount); std::string FormatScript(const CScript& script); std::string EncodeHexTx(const CTransaction& tx, const int serializeFlags = 0); std::string SighashToStr(unsigned char sighash_type); -void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool include_hex, bool include_address = true); -void ScriptToUniv(const CScript& script, UniValue& out); -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, int serialize_flags = 0, const CTxUndo* txundo = nullptr, TxVerbosity verbosity = TxVerbosity::SHOW_DETAILS); +void ScriptToUniv(const CScript& script, UniValue& out, bool include_hex = true, bool include_address = false); +void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry, bool include_hex = true, int serialize_flags = 0, const CTxUndo* txundo = nullptr, TxVerbosity verbosity = TxVerbosity::SHOW_DETAILS); #endif // BITCOIN_CORE_IO_H diff --git a/src/core_write.cpp b/src/core_write.cpp index 8ec75880fe..cfd4cb1b49 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -19,6 +19,10 @@ #include <util/strencodings.h> #include <util/system.h> +#include <map> +#include <string> +#include <vector> + UniValue ValueFromAmount(const CAmount amount) { static_assert(COIN > 1); @@ -143,31 +147,28 @@ std::string EncodeHexTx(const CTransaction& tx, const int serializeFlags) return HexStr(ssTx); } -void ScriptToUniv(const CScript& script, UniValue& out) -{ - ScriptPubKeyToUniv(script, out, /* include_hex */ true, /* include_address */ false); -} - -void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool include_hex, bool include_address) +void ScriptToUniv(const CScript& script, UniValue& out, bool include_hex, bool include_address) { CTxDestination address; - out.pushKV("asm", ScriptToAsmStr(scriptPubKey)); + out.pushKV("asm", ScriptToAsmStr(script)); if (include_address) { - out.pushKV("desc", InferDescriptor(scriptPubKey, DUMMY_SIGNING_PROVIDER)->ToString()); + out.pushKV("desc", InferDescriptor(script, DUMMY_SIGNING_PROVIDER)->ToString()); + } + if (include_hex) { + out.pushKV("hex", HexStr(script)); } - if (include_hex) out.pushKV("hex", HexStr(scriptPubKey)); std::vector<std::vector<unsigned char>> solns; - const TxoutType type{Solver(scriptPubKey, solns)}; + const TxoutType type{Solver(script, solns)}; - if (include_address && ExtractDestination(scriptPubKey, address) && type != TxoutType::PUBKEY) { + if (include_address && ExtractDestination(script, address) && type != TxoutType::PUBKEY) { out.pushKV("address", EncodeDestination(address)); } out.pushKV("type", GetTxnOutputType(type)); } -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, int serialize_flags, const CTxUndo* txundo, TxVerbosity verbosity) +void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry, bool include_hex, int serialize_flags, const CTxUndo* txundo, TxVerbosity verbosity) { entry.pushKV("txid", tx.GetHash().GetHex()); entry.pushKV("hash", tx.GetWitnessHash().GetHex()); @@ -215,7 +216,7 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, if (verbosity == TxVerbosity::SHOW_DETAILS_AND_PREVOUT) { UniValue o_script_pub_key(UniValue::VOBJ); - ScriptPubKeyToUniv(prev_txout.scriptPubKey, o_script_pub_key, /*include_hex=*/ true); + ScriptToUniv(prev_txout.scriptPubKey, /*out=*/o_script_pub_key, /*include_hex=*/true, /*include_address=*/true); UniValue p(UniValue::VOBJ); p.pushKV("generated", bool(prev_coin.fCoinBase)); @@ -240,7 +241,7 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, out.pushKV("n", (int64_t)i); UniValue o(UniValue::VOBJ); - ScriptPubKeyToUniv(txout.scriptPubKey, o, true); + ScriptToUniv(txout.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true); out.pushKV("scriptPubKey", o); vout.push_back(out); @@ -256,8 +257,9 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, entry.pushKV("fee", ValueFromAmount(fee)); } - if (!hashBlock.IsNull()) - entry.pushKV("blockhash", hashBlock.GetHex()); + if (!block_hash.IsNull()) { + entry.pushKV("blockhash", block_hash.GetHex()); + } if (include_hex) { entry.pushKV("hex", EncodeHexTx(tx, serialize_flags)); // The hex-encoded transaction. Used the name "hex" to be consistent with the verbose output of "getrawtransaction". diff --git a/src/init.cpp b/src/init.cpp index 9e8d2ac4b7..f934fd751d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -444,7 +444,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-asmap=<file>", strprintf("Specify asn mapping used for bucketing of the peers (default: %s). Relative paths will be prefixed by the net-specific datadir location.", DEFAULT_ASMAP_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-bantime=<n>", strprintf("Default duration (in seconds) of manually configured bans (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-bind=<addr>[:<port>][=onion]", strprintf("Bind to given address and always listen on it (default: 0.0.0.0). Use [host]:port notation for IPv6. Append =onion to tag any incoming connections to that address and port as incoming Tor connections (default: 127.0.0.1:%u=onion, testnet: 127.0.0.1:%u=onion, signet: 127.0.0.1:%u=onion, regtest: 127.0.0.1:%u=onion)", defaultBaseParams->OnionServiceTargetPort(), testnetBaseParams->OnionServiceTargetPort(), signetBaseParams->OnionServiceTargetPort(), regtestBaseParams->OnionServiceTargetPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); - argsman.AddArg("-cjdnsreachable", "If set then this host is configured for CJDNS (connecting to fc00::/8 addresses would lead us to the CJDNS network) (default: 0)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-cjdnsreachable", "If set, then this host is configured for CJDNS (connecting to fc00::/8 addresses would lead us to the CJDNS network, see doc/cjdns.md) (default: 0)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-connect=<ip>", "Connect only to the specified node; -noconnect disables automatic connections (the rules for this peer are the same as for -addnode). This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-discover", "Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-dns", strprintf("Allow DNS lookups for -addnode, -seednode and -connect (default: %u)", DEFAULT_NAME_LOOKUP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -1302,6 +1302,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } for (int n = 0; n < NET_MAX; n++) { enum Network net = (enum Network)n; + assert(IsReachable(net)); if (!nets.count(net)) SetReachable(net, false); } @@ -1540,7 +1541,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 8: start indexers if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) { - if (const auto error{CheckLegacyTxindex(*Assert(chainman.m_blockman.m_block_tree_db))}) { + if (const auto error{WITH_LOCK(cs_main, return CheckLegacyTxindex(*Assert(chainman.m_blockman.m_block_tree_db)))}) { return InitError(*error); } diff --git a/src/net.cpp b/src/net.cpp index 955eec46e3..602d56ab98 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -626,12 +626,6 @@ void CNode::CopyStats(CNodeStats& stats) X(addr); X(addrBind); stats.m_network = ConnectedThroughNetwork(); - if (m_tx_relay != nullptr) { - LOCK(m_tx_relay->cs_filter); - stats.fRelayTxes = m_tx_relay->fRelayTxes; - } else { - stats.fRelayTxes = false; - } X(m_last_send); X(m_last_recv); X(m_last_tx_time); @@ -658,11 +652,6 @@ void CNode::CopyStats(CNodeStats& stats) X(nRecvBytes); } X(m_permissionFlags); - if (m_tx_relay != nullptr) { - stats.minFeeFilter = m_tx_relay->minFeeFilter; - } else { - stats.minFeeFilter = 0; - } X(m_last_ping_time); X(m_min_ping_time); @@ -913,7 +902,7 @@ static bool CompareNodeTXTime(const NodeEvictionCandidate &a, const NodeEviction { // There is a fall-through here because it is common for a node to have more than a few peers that have not yet relayed txn. if (a.m_last_tx_time != b.m_last_tx_time) return a.m_last_tx_time < b.m_last_tx_time; - if (a.fRelayTxes != b.fRelayTxes) return b.fRelayTxes; + if (a.m_relay_txs != b.m_relay_txs) return b.m_relay_txs; if (a.fBloomFilter != b.fBloomFilter) return a.fBloomFilter; return a.m_connected > b.m_connected; } @@ -921,7 +910,7 @@ static bool CompareNodeTXTime(const NodeEvictionCandidate &a, const NodeEviction // Pick out the potential block-relay only peers, and sort them by last block time. static bool CompareNodeBlockRelayOnlyTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) { - if (a.fRelayTxes != b.fRelayTxes) return a.fRelayTxes; + if (a.m_relay_txs != b.m_relay_txs) return a.m_relay_txs; if (a.m_last_block_time != b.m_last_block_time) return a.m_last_block_time < b.m_last_block_time; if (a.fRelevantServices != b.fRelevantServices) return b.fRelevantServices; return a.m_connected > b.m_connected; @@ -1046,7 +1035,7 @@ void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& evicti EraseLastKElements(vEvictionCandidates, CompareNodeTXTime, 4); // Protect up to 8 non-tx-relay peers that have sent us novel blocks. EraseLastKElements(vEvictionCandidates, CompareNodeBlockRelayOnlyTime, 8, - [](const NodeEvictionCandidate& n) { return !n.fRelayTxes && n.fRelevantServices; }); + [](const NodeEvictionCandidate& n) { return !n.m_relay_txs && n.fRelevantServices; }); // Protect 4 nodes that most recently sent us novel blocks. // An attacker cannot manipulate this metric without performing useful work. @@ -1112,18 +1101,11 @@ bool CConnman::AttemptToEvictConnection() continue; if (node->fDisconnect) continue; - bool peer_relay_txes = false; - bool peer_filter_not_null = false; - if (node->m_tx_relay != nullptr) { - LOCK(node->m_tx_relay->cs_filter); - peer_relay_txes = node->m_tx_relay->fRelayTxes; - peer_filter_not_null = node->m_tx_relay->pfilter != nullptr; - } NodeEvictionCandidate candidate = {node->GetId(), node->m_connected, node->m_min_ping_time, node->m_last_block_time, node->m_last_tx_time, HasAllDesirableServiceFlags(node->nServices), - peer_relay_txes, peer_filter_not_null, node->nKeyedNetGroup, - node->m_prefer_evict, node->addr.IsLocal(), + node->m_relays_txs.load(), node->m_bloom_filter_loaded.load(), + node->nKeyedNetGroup, node->m_prefer_evict, node->addr.IsLocal(), node->ConnectedThroughNetwork()}; vEvictionCandidates.push_back(candidate); } @@ -2800,7 +2782,7 @@ std::vector<CAddress> CConnman::GetAddresses(CNode& requestor, size_t max_addres auto r = m_addr_response_caches.emplace(cache_id, CachedAddrResponse{}); CachedAddrResponse& cache_entry = r.first->second; if (cache_entry.m_cache_entry_expiration < current_time) { // If emplace() added new one it has expiration 0. - cache_entry.m_addrs_response_cache = GetAddresses(max_addresses, max_pct, /* network */ std::nullopt); + cache_entry.m_addrs_response_cache = GetAddresses(max_addresses, max_pct, /*network=*/std::nullopt); // Choosing a proper cache lifetime is a trade-off between the privacy leak minimization // and the usefulness of ADDR responses to honest users. // @@ -3031,9 +3013,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, std::shared_ptr<Sock> s nLocalServices(nLocalServicesIn) { if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND); - if (conn_type_in != ConnectionType::BLOCK_RELAY) { - m_tx_relay = std::make_unique<TxRelay>(); - } for (const std::string &msg : getAllNetMessageTypes()) mapRecvBytesPerMsgCmd[msg] = 0; @@ -257,7 +257,6 @@ class CNodeStats public: NodeId nodeid; ServiceFlags nServices; - bool fRelayTxes; std::chrono::seconds m_last_send; std::chrono::seconds m_last_recv; std::chrono::seconds m_last_tx_time; @@ -278,7 +277,6 @@ public: NetPermissionFlags m_permissionFlags; std::chrono::microseconds m_last_ping_time; std::chrono::microseconds m_min_ping_time; - CAmount minFeeFilter; // Our address, as reported by the peer std::string addrLocal; // Address of this peer @@ -555,34 +553,15 @@ public: // Peer selected us as (compact blocks) high-bandwidth peer (BIP152) std::atomic<bool> m_bip152_highbandwidth_from{false}; - struct TxRelay { - mutable RecursiveMutex cs_filter; - // We use fRelayTxes for two purposes - - // a) it allows us to not relay tx invs before receiving the peer's version message - // b) the peer may tell us in its version message that we should not relay tx invs - // unless it loads a bloom filter. - bool fRelayTxes GUARDED_BY(cs_filter){false}; - std::unique_ptr<CBloomFilter> pfilter PT_GUARDED_BY(cs_filter) GUARDED_BY(cs_filter){nullptr}; - - mutable RecursiveMutex cs_tx_inventory; - CRollingBloomFilter filterInventoryKnown GUARDED_BY(cs_tx_inventory){50000, 0.000001}; - // Set of transaction ids we still have to announce. - // They are sorted by the mempool before relay, so the order is not important. - std::set<uint256> setInventoryTxToSend; - // Used for BIP35 mempool sending - bool fSendMempool GUARDED_BY(cs_tx_inventory){false}; - // Last time a "MEMPOOL" request was serviced. - std::atomic<std::chrono::seconds> m_last_mempool_req{0s}; - std::chrono::microseconds nNextInvSend{0}; - - /** Minimum fee rate with which to filter inv's to this node */ - std::atomic<CAmount> minFeeFilter{0}; - CAmount lastSentFeeFilter{0}; - std::chrono::microseconds m_next_send_feefilter{0}; - }; + /** Whether we should relay transactions to this peer (their version + * message did not include fRelay=false and this is not a block-relay-only + * connection). This only changes from false to true. It will never change + * back to false. Used only in inbound eviction logic. */ + std::atomic_bool m_relays_txs{false}; - // m_tx_relay == nullptr if we're not relaying transactions with this peer - std::unique_ptr<TxRelay> m_tx_relay; + /** Whether this peer has loaded a bloom filter. Used only in inbound + * eviction logic. */ + std::atomic_bool m_bloom_filter_loaded{false}; /** UNIX epoch time of the last block received from this peer that we had * not yet seen (e.g. not already received from another peer), that passed @@ -658,23 +637,6 @@ public: nRefCount--; } - void AddKnownTx(const uint256& hash) - { - if (m_tx_relay != nullptr) { - LOCK(m_tx_relay->cs_tx_inventory); - m_tx_relay->filterInventoryKnown.insert(hash); - } - } - - void PushTxInventory(const uint256& hash) - { - if (m_tx_relay == nullptr) return; - LOCK(m_tx_relay->cs_tx_inventory); - if (!m_tx_relay->filterInventoryKnown.contains(hash)) { - m_tx_relay->setInventoryTxToSend.insert(hash); - } - } - void CloseSocketDisconnect(); void CopyStats(CNodeStats& stats); @@ -1308,7 +1270,7 @@ struct NodeEvictionCandidate std::chrono::seconds m_last_block_time; std::chrono::seconds m_last_tx_time; bool fRelevantServices; - bool fRelayTxes; + bool m_relay_txs; bool fBloomFilter; uint64_t nKeyedNetGroup; bool prefer_evict; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 34dd7991be..ba72a11ec9 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -233,6 +233,39 @@ struct Peer { /** Whether a ping has been requested by the user */ std::atomic<bool> m_ping_queued{false}; + /** Whether this peer relays txs via wtxid */ + std::atomic<bool> m_wtxid_relay{false}; + + struct TxRelay { + mutable RecursiveMutex m_bloom_filter_mutex; + // We use m_relay_txs for two purposes - + // a) it allows us to not relay tx invs before receiving the peer's version message + // b) the peer may tell us in its version message that we should not relay tx invs + // unless it loads a bloom filter. + bool m_relay_txs GUARDED_BY(m_bloom_filter_mutex){false}; + std::unique_ptr<CBloomFilter> m_bloom_filter PT_GUARDED_BY(m_bloom_filter_mutex) GUARDED_BY(m_bloom_filter_mutex){nullptr}; + + mutable RecursiveMutex m_tx_inventory_mutex; + CRollingBloomFilter m_tx_inventory_known_filter GUARDED_BY(m_tx_inventory_mutex){50000, 0.000001}; + // Set of transaction ids we still have to announce. + // They are sorted by the mempool before relay, so the order is not important. + std::set<uint256> m_tx_inventory_to_send; + // Used for BIP35 mempool sending + bool m_send_mempool GUARDED_BY(m_tx_inventory_mutex){false}; + // Last time a "MEMPOOL" request was serviced. + std::atomic<std::chrono::seconds> m_last_mempool_req{0s}; + std::chrono::microseconds m_next_inv_send_time{0}; + + /** Minimum fee rate with which to filter inv's to this node */ + std::atomic<CAmount> m_fee_filter_received{0}; + CAmount m_fee_filter_sent{0}; + std::chrono::microseconds m_next_send_feefilter{0}; + }; + + /** Transaction relay data. Will be a nullptr if we're not relaying + * transactions with this peer (e.g. if it's a block-relay-only peer) */ + std::unique_ptr<TxRelay> m_tx_relay; + /** A vector of addresses to send to the peer, limited to MAX_ADDR_TO_SEND. */ std::vector<CAddress> m_addrs_to_send; /** Probabilistic filter to track recent addr messages relayed with this @@ -291,8 +324,9 @@ struct Peer { /** Work queue of items requested by this peer **/ std::deque<CInv> m_getdata_requests GUARDED_BY(m_getdata_requests_mutex); - explicit Peer(NodeId id) + explicit Peer(NodeId id, bool tx_relay) : m_id(id) + , m_tx_relay(tx_relay ? std::make_unique<TxRelay>() : nullptr) {} }; @@ -332,9 +366,6 @@ public: const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) override; private: - void _RelayTransaction(const uint256& txid, const uint256& wtxid) - EXCLUSIVE_LOCKS_REQUIRED(cs_main); - /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */ void ConsiderEviction(CNode& pto, std::chrono::seconds time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -395,7 +426,7 @@ private: EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** Send a version message to a peer */ - void PushNodeVersion(CNode& pnode); + void PushNodeVersion(CNode& pnode, const Peer& peer); /** Send a ping message every PING_INTERVAL or if requested via RPC. May * mark the peer to be disconnected if a ping has timed out. @@ -416,7 +447,7 @@ private: void RelayAddress(NodeId originator, const CAddress& addr, bool fReachable); /** Send `feefilter` message. */ - void MaybeSendFeefilter(CNode& node, std::chrono::microseconds current_time); + void MaybeSendFeefilter(CNode& node, Peer& peer, std::chrono::microseconds current_time); const CChainParams& m_chainparams; CConnman& m_connman; @@ -465,7 +496,7 @@ private: std::map<uint256, std::pair<NodeId, bool>> mapBlockSource GUARDED_BY(cs_main); /** Number of peers with wtxid relay. */ - int m_wtxid_relay_peers GUARDED_BY(cs_main) = 0; + std::atomic<int> m_wtxid_relay_peers{0}; /** Number of outbound peers with m_chain_sync.m_protect. */ int m_outbound_peers_with_protect_from_disconnect GUARDED_BY(cs_main) = 0; @@ -780,9 +811,6 @@ struct CNodeState { //! A rolling bloom filter of all announced tx CInvs to this peer. CRollingBloomFilter m_recently_announced_invs = CRollingBloomFilter{INVENTORY_MAX_RECENT_RELAY, 0.000001}; - //! Whether this peer relays txs via wtxid - bool m_wtxid_relay{false}; - CNodeState(bool is_inbound) : m_is_inbound(is_inbound) {} }; @@ -827,6 +855,14 @@ static void PushAddress(Peer& peer, const CAddress& addr, FastRandomContext& ins } } +static void AddKnownTx(Peer& peer, const uint256& hash) +{ + if (peer.m_tx_relay != nullptr) { + LOCK(peer.m_tx_relay->m_tx_inventory_mutex); + peer.m_tx_relay->m_tx_inventory_known_filter.insert(hash); + } +} + static void UpdatePreferredDownload(const CNode& node, CNodeState* state) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { nPreferredDownload -= state->fPreferredDownload; @@ -1122,7 +1158,7 @@ void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count } // namespace -void PeerManagerImpl::PushNodeVersion(CNode& pnode) +void PeerManagerImpl::PushNodeVersion(CNode& pnode, const Peer& peer) { // Note that pnode->GetLocalServices() is a reflection of the local // services we were offering when the CNode object was created for this @@ -1137,7 +1173,7 @@ void PeerManagerImpl::PushNodeVersion(CNode& pnode) CService addr_you = addr.IsRoutable() && !IsProxy(addr) && addr.IsAddrV1Compatible() ? addr : CService(); uint64_t your_services{addr.nServices}; - const bool tx_relay = !m_ignore_incoming_txs && pnode.m_tx_relay != nullptr && !pnode.IsFeelerConn(); + const bool tx_relay = !m_ignore_incoming_txs && peer.m_tx_relay != nullptr && !pnode.IsFeelerConn(); m_connman.PushMessage(&pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, my_services, nTime, your_services, addr_you, // Together the pre-version-31402 serialization of CAddress "addrYou" (without nTime) my_services, CService(), // Together the pre-version-31402 serialization of CAddress "addrMe" (without nTime) @@ -1194,13 +1230,13 @@ void PeerManagerImpl::InitializeNode(CNode *pnode) mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(pnode->IsInboundConn())); assert(m_txrequest.Count(nodeid) == 0); } + PeerRef peer = std::make_shared<Peer>(nodeid, /*tx_relay=*/ !pnode->IsBlockOnlyConn()); { - PeerRef peer = std::make_shared<Peer>(nodeid); LOCK(m_peer_mutex); - m_peer_map.emplace_hint(m_peer_map.end(), nodeid, std::move(peer)); + m_peer_map.emplace_hint(m_peer_map.end(), nodeid, peer); } if (!pnode->IsInboundConn()) { - PushNodeVersion(*pnode); + PushNodeVersion(*pnode, *peer); } } @@ -1212,8 +1248,7 @@ void PeerManagerImpl::ReattemptInitialBroadcast(CScheduler& scheduler) CTransactionRef tx = m_mempool.get(txid); if (tx != nullptr) { - LOCK(cs_main); - _RelayTransaction(txid, tx->GetWitnessHash()); + RelayTransaction(txid, tx->GetWitnessHash()); } else { m_mempool.RemoveUnbroadcastTx(txid, true); } @@ -1240,6 +1275,8 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) PeerRef peer = RemovePeer(nodeid); assert(peer != nullptr); misbehavior = WITH_LOCK(peer->m_misbehavior_mutex, return peer->m_misbehavior_score); + m_wtxid_relay_peers -= peer->m_wtxid_relay; + assert(m_wtxid_relay_peers >= 0); } CNodeState *state = State(nodeid); assert(state != nullptr); @@ -1257,8 +1294,6 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) assert(m_peers_downloading_from >= 0); m_outbound_peers_with_protect_from_disconnect -= state->m_chain_sync.m_protect; assert(m_outbound_peers_with_protect_from_disconnect >= 0); - m_wtxid_relay_peers -= state->m_wtxid_relay; - assert(m_wtxid_relay_peers >= 0); mapNodeState.erase(nodeid); @@ -1331,6 +1366,14 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) c ping_wait = GetTime<std::chrono::microseconds>() - peer->m_ping_start.load(); } + if (peer->m_tx_relay != nullptr) { + stats.m_relay_txs = WITH_LOCK(peer->m_tx_relay->m_bloom_filter_mutex, return peer->m_tx_relay->m_relay_txs); + stats.m_fee_filter_received = peer->m_tx_relay->m_fee_filter_received.load(); + } else { + stats.m_relay_txs = false; + stats.m_fee_filter_received = 0; + } + stats.m_ping_wait = ping_wait; stats.m_addr_processed = peer->m_addr_processed.load(); stats.m_addr_rate_limited = peer->m_addr_rate_limited.load(); @@ -1746,22 +1789,17 @@ void PeerManagerImpl::SendPings() void PeerManagerImpl::RelayTransaction(const uint256& txid, const uint256& wtxid) { - WITH_LOCK(cs_main, _RelayTransaction(txid, wtxid);); -} - -void PeerManagerImpl::_RelayTransaction(const uint256& txid, const uint256& wtxid) -{ - m_connman.ForEachNode([&txid, &wtxid](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { - AssertLockHeld(::cs_main); + LOCK(m_peer_mutex); + for(auto& it : m_peer_map) { + Peer& peer = *it.second; + if (!peer.m_tx_relay) continue; - CNodeState* state = State(pnode->GetId()); - if (state == nullptr) return; - if (state->m_wtxid_relay) { - pnode->PushTxInventory(wtxid); - } else { - pnode->PushTxInventory(txid); + const uint256& hash{peer.m_wtxid_relay ? wtxid : txid}; + LOCK(peer.m_tx_relay->m_tx_inventory_mutex); + if (!peer.m_tx_relay->m_tx_inventory_known_filter.contains(hash)) { + peer.m_tx_relay->m_tx_inventory_to_send.insert(hash); } - }); + }; } void PeerManagerImpl::RelayAddress(NodeId originator, @@ -1907,11 +1945,11 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } else if (inv.IsMsgFilteredBlk()) { bool sendMerkleBlock = false; CMerkleBlock merkleBlock; - if (pfrom.m_tx_relay != nullptr) { - LOCK(pfrom.m_tx_relay->cs_filter); - if (pfrom.m_tx_relay->pfilter) { + if (peer.m_tx_relay != nullptr) { + LOCK(peer.m_tx_relay->m_bloom_filter_mutex); + if (peer.m_tx_relay->m_bloom_filter) { sendMerkleBlock = true; - merkleBlock = CMerkleBlock(*pblock, *pfrom.m_tx_relay->pfilter); + merkleBlock = CMerkleBlock(*pblock, *peer.m_tx_relay->m_bloom_filter); } } if (sendMerkleBlock) { @@ -2000,7 +2038,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic const auto now{GetTime<std::chrono::seconds>()}; // Get last mempool request time - const auto mempool_req = pfrom.m_tx_relay != nullptr ? pfrom.m_tx_relay->m_last_mempool_req.load() : std::chrono::seconds::min(); + const auto mempool_req = peer.m_tx_relay != nullptr ? peer.m_tx_relay->m_last_mempool_req.load() : std::chrono::seconds::min(); // Process as many TX items from the front of the getdata queue as // possible, since they're common and it's efficient to batch process @@ -2013,7 +2051,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic const CInv &inv = *it++; - if (pfrom.m_tx_relay == nullptr) { + if (peer.m_tx_relay == nullptr) { // Ignore GETDATA requests for transactions from blocks-only peers. continue; } @@ -2041,7 +2079,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic } for (const uint256& parent_txid : parent_ids_to_add) { // Relaying a transaction with a recent but unconfirmed parent. - if (WITH_LOCK(pfrom.m_tx_relay->cs_tx_inventory, return !pfrom.m_tx_relay->filterInventoryKnown.contains(parent_txid))) { + if (WITH_LOCK(peer.m_tx_relay->m_tx_inventory_mutex, return !peer.m_tx_relay->m_tx_inventory_known_filter.contains(parent_txid))) { LOCK(cs_main); State(pfrom.GetId())->m_recently_announced_invs.insert(parent_txid); } @@ -2321,7 +2359,7 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set) if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) { LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString()); - _RelayTransaction(orphanHash, porphanTx->GetWitnessHash()); + RelayTransaction(orphanHash, porphanTx->GetWitnessHash()); m_orphanage.AddChildrenToWorkSet(*porphanTx, orphan_work_set); m_orphanage.EraseTx(orphanHash); for (const CTransactionRef& removedTx : result.m_replaced_transactions.value()) { @@ -2637,7 +2675,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // Inbound peers send us their version message when they connect. // We send our version message in response. if (pfrom.IsInboundConn()) { - PushNodeVersion(pfrom); + PushNodeVersion(pfrom, *peer); } // Change version @@ -2676,9 +2714,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // set nodes not capable of serving the complete blockchain history as "limited nodes" pfrom.m_limited_node = (!(nServices & NODE_NETWORK) && (nServices & NODE_NETWORK_LIMITED)); - if (pfrom.m_tx_relay != nullptr) { - LOCK(pfrom.m_tx_relay->cs_filter); - pfrom.m_tx_relay->fRelayTxes = fRelay; // set to true after we get the first filter* message + if (peer->m_tx_relay != nullptr) { + { + LOCK(peer->m_tx_relay->m_bloom_filter_mutex); + peer->m_tx_relay->m_relay_txs = fRelay; // set to true after we get the first filter* message + } + if (fRelay) pfrom.m_relays_txs = true; } if((nServices & NODE_WITNESS)) @@ -2867,9 +2908,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } if (pfrom.GetCommonVersion() >= WTXID_RELAY_VERSION) { - LOCK(cs_main); - if (!State(pfrom.GetId())->m_wtxid_relay) { - State(pfrom.GetId())->m_wtxid_relay = true; + if (!peer->m_wtxid_relay) { + peer->m_wtxid_relay = true; m_wtxid_relay_peers++; } else { LogPrint(BCLog::NET, "ignoring duplicate wtxidrelay from peer=%d\n", pfrom.GetId()); @@ -3005,7 +3045,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // Reject tx INVs when the -blocksonly setting is enabled, or this is a // block-relay-only peer - bool reject_tx_invs{m_ignore_incoming_txs || (pfrom.m_tx_relay == nullptr)}; + bool reject_tx_invs{m_ignore_incoming_txs || (peer->m_tx_relay == nullptr)}; // Allow peers with relay permission to send data other than blocks in blocks only mode if (pfrom.HasPermission(NetPermissionFlags::Relay)) { @@ -3023,7 +3063,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // Ignore INVs that don't match wtxidrelay setting. // Note that orphan parent fetching always uses MSG_TX GETDATAs regardless of the wtxidrelay setting. // This is fine as no INV messages are involved in that process. - if (State(pfrom.GetId())->m_wtxid_relay) { + if (peer->m_wtxid_relay) { if (inv.IsMsgTx()) continue; } else { if (inv.IsMsgWtx()) continue; @@ -3052,7 +3092,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, const bool fAlreadyHave = AlreadyHaveTx(gtxid); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); - pfrom.AddKnownTx(inv.hash); + AddKnownTx(*peer, inv.hash); if (!fAlreadyHave && !m_chainman.ActiveChainstate().IsInitialBlockDownload()) { AddTxAnnouncement(pfrom, gtxid, current_time); } @@ -3282,8 +3322,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // Stop processing the transaction early if // 1) We are in blocks only mode and peer has no relay permission // 2) This peer is a block-relay-only peer - if ((m_ignore_incoming_txs && !pfrom.HasPermission(NetPermissionFlags::Relay)) || (pfrom.m_tx_relay == nullptr)) - { + if ((m_ignore_incoming_txs && !pfrom.HasPermission(NetPermissionFlags::Relay)) || (peer->m_tx_relay == nullptr)) { LogPrint(BCLog::NET, "transaction sent in violation of protocol peer=%d\n", pfrom.GetId()); pfrom.fDisconnect = true; return; @@ -3301,21 +3340,19 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, const uint256& txid = ptx->GetHash(); const uint256& wtxid = ptx->GetWitnessHash(); - LOCK2(cs_main, g_cs_orphans); - - CNodeState* nodestate = State(pfrom.GetId()); - - const uint256& hash = nodestate->m_wtxid_relay ? wtxid : txid; - pfrom.AddKnownTx(hash); - if (nodestate->m_wtxid_relay && txid != wtxid) { - // Insert txid into filterInventoryKnown, even for + const uint256& hash = peer->m_wtxid_relay ? wtxid : txid; + AddKnownTx(*peer, hash); + if (peer->m_wtxid_relay && txid != wtxid) { + // Insert txid into m_tx_inventory_known_filter, even for // wtxidrelay peers. This prevents re-adding of // unconfirmed parents to the recently_announced // filter, when a child tx is requested. See // ProcessGetData(). - pfrom.AddKnownTx(txid); + AddKnownTx(*peer, txid); } + LOCK2(cs_main, g_cs_orphans); + m_txrequest.ReceivedResponse(pfrom.GetId(), txid); if (tx.HasWitness()) m_txrequest.ReceivedResponse(pfrom.GetId(), wtxid); @@ -3340,7 +3377,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, LogPrintf("Not relaying non-mempool transaction %s from forcerelay peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); } else { LogPrintf("Force relaying tx %s from peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); - _RelayTransaction(tx.GetHash(), tx.GetWitnessHash()); + RelayTransaction(tx.GetHash(), tx.GetWitnessHash()); } } return; @@ -3354,7 +3391,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // requests for it. m_txrequest.ForgetTxHash(tx.GetHash()); m_txrequest.ForgetTxHash(tx.GetWitnessHash()); - _RelayTransaction(tx.GetHash(), tx.GetWitnessHash()); + RelayTransaction(tx.GetHash(), tx.GetWitnessHash()); m_orphanage.AddChildrenToWorkSet(tx, peer->m_orphan_work_set); pfrom.m_last_tx_time = GetTime<std::chrono::seconds>(); @@ -3401,7 +3438,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // Eventually we should replace this with an improved // protocol for getting all unconfirmed parents. const auto gtxid{GenTxid::Txid(parent_txid)}; - pfrom.AddKnownTx(parent_txid); + AddKnownTx(*peer, parent_txid); if (!AlreadyHaveTx(gtxid)) AddTxAnnouncement(pfrom, gtxid, current_time); } @@ -3525,7 +3562,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, BlockValidationState state; if (!m_chainman.ProcessNewBlockHeaders({cmpctblock.header}, state, m_chainparams, &pindex)) { if (state.IsInvalid()) { - MaybePunishNodeForBlock(pfrom.GetId(), state, /*via_compact_block*/ true, "invalid header via cmpctblock"); + MaybePunishNodeForBlock(pfrom.GetId(), state, /*via_compact_block=*/true, "invalid header via cmpctblock"); return; } } @@ -3865,7 +3902,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, peer->m_addrs_to_send.clear(); std::vector<CAddress> vAddr; if (pfrom.HasPermission(NetPermissionFlags::Addr)) { - vAddr = m_connman.GetAddresses(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND, /* network */ std::nullopt); + vAddr = m_connman.GetAddresses(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND, /*network=*/std::nullopt); } else { vAddr = m_connman.GetAddresses(pfrom, MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND); } @@ -3897,9 +3934,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } - if (pfrom.m_tx_relay != nullptr) { - LOCK(pfrom.m_tx_relay->cs_tx_inventory); - pfrom.m_tx_relay->fSendMempool = true; + if (peer->m_tx_relay != nullptr) { + LOCK(peer->m_tx_relay->m_tx_inventory_mutex); + peer->m_tx_relay->m_send_mempool = true; } return; } @@ -3993,11 +4030,15 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // There is no excuse for sending a too-large filter Misbehaving(pfrom.GetId(), 100, "too-large bloom filter"); } - else if (pfrom.m_tx_relay != nullptr) + else if (peer->m_tx_relay != nullptr) { - LOCK(pfrom.m_tx_relay->cs_filter); - pfrom.m_tx_relay->pfilter.reset(new CBloomFilter(filter)); - pfrom.m_tx_relay->fRelayTxes = true; + { + LOCK(peer->m_tx_relay->m_bloom_filter_mutex); + peer->m_tx_relay->m_bloom_filter.reset(new CBloomFilter(filter)); + peer->m_tx_relay->m_relay_txs = true; + } + pfrom.m_bloom_filter_loaded = true; + pfrom.m_relays_txs = true; } return; } @@ -4016,10 +4057,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, bool bad = false; if (vData.size() > MAX_SCRIPT_ELEMENT_SIZE) { bad = true; - } else if (pfrom.m_tx_relay != nullptr) { - LOCK(pfrom.m_tx_relay->cs_filter); - if (pfrom.m_tx_relay->pfilter) { - pfrom.m_tx_relay->pfilter->insert(vData); + } else if (peer->m_tx_relay != nullptr) { + LOCK(peer->m_tx_relay->m_bloom_filter_mutex); + if (peer->m_tx_relay->m_bloom_filter) { + peer->m_tx_relay->m_bloom_filter->insert(vData); } else { bad = true; } @@ -4036,12 +4077,17 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, pfrom.fDisconnect = true; return; } - if (pfrom.m_tx_relay == nullptr) { + if (peer->m_tx_relay == nullptr) { return; } - LOCK(pfrom.m_tx_relay->cs_filter); - pfrom.m_tx_relay->pfilter = nullptr; - pfrom.m_tx_relay->fRelayTxes = true; + + { + LOCK(peer->m_tx_relay->m_bloom_filter_mutex); + peer->m_tx_relay->m_bloom_filter = nullptr; + peer->m_tx_relay->m_relay_txs = true; + } + pfrom.m_bloom_filter_loaded = false; + pfrom.m_relays_txs = true; return; } @@ -4049,8 +4095,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, CAmount newFeeFilter = 0; vRecv >> newFeeFilter; if (MoneyRange(newFeeFilter)) { - if (pfrom.m_tx_relay != nullptr) { - pfrom.m_tx_relay->minFeeFilter = newFeeFilter; + if (peer->m_tx_relay != nullptr) { + peer->m_tx_relay->m_fee_filter_received = newFeeFilter; } LogPrint(BCLog::NET, "received: feefilter of %s from peer=%d\n", CFeeRate(newFeeFilter).ToString(), pfrom.GetId()); } @@ -4508,10 +4554,10 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros } } -void PeerManagerImpl::MaybeSendFeefilter(CNode& pto, std::chrono::microseconds current_time) +void PeerManagerImpl::MaybeSendFeefilter(CNode& pto, Peer& peer, std::chrono::microseconds current_time) { if (m_ignore_incoming_txs) return; - if (!pto.m_tx_relay) return; + if (!peer.m_tx_relay) return; if (pto.GetCommonVersion() < FEEFILTER_VERSION) return; // peers with the forcerelay permission should not filter txs to us if (pto.HasPermission(NetPermissionFlags::ForceRelay)) return; @@ -4525,27 +4571,27 @@ void PeerManagerImpl::MaybeSendFeefilter(CNode& pto, std::chrono::microseconds c currentFilter = MAX_MONEY; } else { static const CAmount MAX_FILTER{g_filter_rounder.round(MAX_MONEY)}; - if (pto.m_tx_relay->lastSentFeeFilter == MAX_FILTER) { + if (peer.m_tx_relay->m_fee_filter_sent == MAX_FILTER) { // Send the current filter if we sent MAX_FILTER previously // and made it out of IBD. - pto.m_tx_relay->m_next_send_feefilter = 0us; + peer.m_tx_relay->m_next_send_feefilter = 0us; } } - if (current_time > pto.m_tx_relay->m_next_send_feefilter) { + if (current_time > peer.m_tx_relay->m_next_send_feefilter) { CAmount filterToSend = g_filter_rounder.round(currentFilter); // We always have a fee filter of at least minRelayTxFee filterToSend = std::max(filterToSend, ::minRelayTxFee.GetFeePerK()); - if (filterToSend != pto.m_tx_relay->lastSentFeeFilter) { + if (filterToSend != peer.m_tx_relay->m_fee_filter_sent) { m_connman.PushMessage(&pto, CNetMsgMaker(pto.GetCommonVersion()).Make(NetMsgType::FEEFILTER, filterToSend)); - pto.m_tx_relay->lastSentFeeFilter = filterToSend; + peer.m_tx_relay->m_fee_filter_sent = filterToSend; } - pto.m_tx_relay->m_next_send_feefilter = GetExponentialRand(current_time, AVG_FEEFILTER_BROADCAST_INTERVAL); + peer.m_tx_relay->m_next_send_feefilter = GetExponentialRand(current_time, AVG_FEEFILTER_BROADCAST_INTERVAL); } // If the fee filter has changed substantially and it's still more than MAX_FEEFILTER_CHANGE_DELAY // until scheduled broadcast, then move the broadcast to within MAX_FEEFILTER_CHANGE_DELAY. - else if (current_time + MAX_FEEFILTER_CHANGE_DELAY < pto.m_tx_relay->m_next_send_feefilter && - (currentFilter < 3 * pto.m_tx_relay->lastSentFeeFilter / 4 || currentFilter > 4 * pto.m_tx_relay->lastSentFeeFilter / 3)) { - pto.m_tx_relay->m_next_send_feefilter = current_time + GetRandomDuration<std::chrono::microseconds>(MAX_FEEFILTER_CHANGE_DELAY); + else if (current_time + MAX_FEEFILTER_CHANGE_DELAY < peer.m_tx_relay->m_next_send_feefilter && + (currentFilter < 3 * peer.m_tx_relay->m_fee_filter_sent / 4 || currentFilter > 4 * peer.m_tx_relay->m_fee_filter_sent / 3)) { + peer.m_tx_relay->m_next_send_feefilter = current_time + GetRandomDuration<std::chrono::microseconds>(MAX_FEEFILTER_CHANGE_DELAY); } } @@ -4812,45 +4858,45 @@ bool PeerManagerImpl::SendMessages(CNode* pto) peer->m_blocks_for_inv_relay.clear(); } - if (pto->m_tx_relay != nullptr) { - LOCK(pto->m_tx_relay->cs_tx_inventory); + if (peer->m_tx_relay != nullptr) { + LOCK(peer->m_tx_relay->m_tx_inventory_mutex); // Check whether periodic sends should happen bool fSendTrickle = pto->HasPermission(NetPermissionFlags::NoBan); - if (pto->m_tx_relay->nNextInvSend < current_time) { + if (peer->m_tx_relay->m_next_inv_send_time < current_time) { fSendTrickle = true; if (pto->IsInboundConn()) { - pto->m_tx_relay->nNextInvSend = NextInvToInbounds(current_time, INBOUND_INVENTORY_BROADCAST_INTERVAL); + peer->m_tx_relay->m_next_inv_send_time = NextInvToInbounds(current_time, INBOUND_INVENTORY_BROADCAST_INTERVAL); } else { - pto->m_tx_relay->nNextInvSend = GetExponentialRand(current_time, OUTBOUND_INVENTORY_BROADCAST_INTERVAL); + peer->m_tx_relay->m_next_inv_send_time = GetExponentialRand(current_time, OUTBOUND_INVENTORY_BROADCAST_INTERVAL); } } // Time to send but the peer has requested we not relay transactions. if (fSendTrickle) { - LOCK(pto->m_tx_relay->cs_filter); - if (!pto->m_tx_relay->fRelayTxes) pto->m_tx_relay->setInventoryTxToSend.clear(); + LOCK(peer->m_tx_relay->m_bloom_filter_mutex); + if (!peer->m_tx_relay->m_relay_txs) peer->m_tx_relay->m_tx_inventory_to_send.clear(); } // Respond to BIP35 mempool requests - if (fSendTrickle && pto->m_tx_relay->fSendMempool) { + if (fSendTrickle && peer->m_tx_relay->m_send_mempool) { auto vtxinfo = m_mempool.infoAll(); - pto->m_tx_relay->fSendMempool = false; - const CFeeRate filterrate{pto->m_tx_relay->minFeeFilter.load()}; + peer->m_tx_relay->m_send_mempool = false; + const CFeeRate filterrate{peer->m_tx_relay->m_fee_filter_received.load()}; - LOCK(pto->m_tx_relay->cs_filter); + LOCK(peer->m_tx_relay->m_bloom_filter_mutex); for (const auto& txinfo : vtxinfo) { - const uint256& hash = state.m_wtxid_relay ? txinfo.tx->GetWitnessHash() : txinfo.tx->GetHash(); - CInv inv(state.m_wtxid_relay ? MSG_WTX : MSG_TX, hash); - pto->m_tx_relay->setInventoryTxToSend.erase(hash); + const uint256& hash = peer->m_wtxid_relay ? txinfo.tx->GetWitnessHash() : txinfo.tx->GetHash(); + CInv inv(peer->m_wtxid_relay ? MSG_WTX : MSG_TX, hash); + peer->m_tx_relay->m_tx_inventory_to_send.erase(hash); // Don't send transactions that peers will not put into their mempool if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) { continue; } - if (pto->m_tx_relay->pfilter) { - if (!pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; + if (peer->m_tx_relay->m_bloom_filter) { + if (!peer->m_tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; } - pto->m_tx_relay->filterInventoryKnown.insert(hash); + peer->m_tx_relay->m_tx_inventory_known_filter.insert(hash); // Responses to MEMPOOL requests bypass the m_recently_announced_invs filter. vInv.push_back(inv); if (vInv.size() == MAX_INV_SZ) { @@ -4858,37 +4904,37 @@ bool PeerManagerImpl::SendMessages(CNode* pto) vInv.clear(); } } - pto->m_tx_relay->m_last_mempool_req = std::chrono::duration_cast<std::chrono::seconds>(current_time); + peer->m_tx_relay->m_last_mempool_req = std::chrono::duration_cast<std::chrono::seconds>(current_time); } // Determine transactions to relay if (fSendTrickle) { // Produce a vector with all candidates for sending std::vector<std::set<uint256>::iterator> vInvTx; - vInvTx.reserve(pto->m_tx_relay->setInventoryTxToSend.size()); - for (std::set<uint256>::iterator it = pto->m_tx_relay->setInventoryTxToSend.begin(); it != pto->m_tx_relay->setInventoryTxToSend.end(); it++) { + vInvTx.reserve(peer->m_tx_relay->m_tx_inventory_to_send.size()); + for (std::set<uint256>::iterator it = peer->m_tx_relay->m_tx_inventory_to_send.begin(); it != peer->m_tx_relay->m_tx_inventory_to_send.end(); it++) { vInvTx.push_back(it); } - const CFeeRate filterrate{pto->m_tx_relay->minFeeFilter.load()}; + const CFeeRate filterrate{peer->m_tx_relay->m_fee_filter_received.load()}; // Topologically and fee-rate sort the inventory we send for privacy and priority reasons. // A heap is used so that not all items need sorting if only a few are being sent. - CompareInvMempoolOrder compareInvMempoolOrder(&m_mempool, state.m_wtxid_relay); + CompareInvMempoolOrder compareInvMempoolOrder(&m_mempool, peer->m_wtxid_relay); std::make_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); // No reason to drain out at many times the network's capacity, // especially since we have many peers and some will draw much shorter delays. unsigned int nRelayedTransactions = 0; - LOCK(pto->m_tx_relay->cs_filter); + LOCK(peer->m_tx_relay->m_bloom_filter_mutex); while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) { // Fetch the top element from the heap std::pop_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); std::set<uint256>::iterator it = vInvTx.back(); vInvTx.pop_back(); uint256 hash = *it; - CInv inv(state.m_wtxid_relay ? MSG_WTX : MSG_TX, hash); + CInv inv(peer->m_wtxid_relay ? MSG_WTX : MSG_TX, hash); // Remove it from the to-be-sent set - pto->m_tx_relay->setInventoryTxToSend.erase(it); + peer->m_tx_relay->m_tx_inventory_to_send.erase(it); // Check if not in the filter already - if (pto->m_tx_relay->filterInventoryKnown.contains(hash)) { + if (peer->m_tx_relay->m_tx_inventory_known_filter.contains(hash)) { continue; } // Not in the mempool anymore? don't bother sending it. @@ -4902,7 +4948,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) { continue; } - if (pto->m_tx_relay->pfilter && !pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; + if (peer->m_tx_relay->m_bloom_filter && !peer->m_tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; // Send State(pto->GetId())->m_recently_announced_invs.insert(hash); vInv.push_back(inv); @@ -4929,14 +4975,14 @@ bool PeerManagerImpl::SendMessages(CNode* pto) m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); vInv.clear(); } - pto->m_tx_relay->filterInventoryKnown.insert(hash); + peer->m_tx_relay->m_tx_inventory_known_filter.insert(hash); if (hash != txid) { - // Insert txid into filterInventoryKnown, even for + // Insert txid into m_tx_inventory_known_filter, even for // wtxidrelay peers. This prevents re-adding of // unconfirmed parents to the recently_announced // filter, when a child tx is requested. See // ProcessGetData(). - pto->m_tx_relay->filterInventoryKnown.insert(txid); + peer->m_tx_relay->m_tx_inventory_known_filter.insert(txid); } } } @@ -5057,6 +5103,6 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (!vGetData.empty()) m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::GETDATA, vGetData)); } // release cs_main - MaybeSendFeefilter(*pto, current_time); + MaybeSendFeefilter(*pto, *peer, current_time); return true; } diff --git a/src/net_processing.h b/src/net_processing.h index e30f9f516c..7dacaee5c1 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -29,6 +29,8 @@ struct CNodeStateStats { int m_starting_height = -1; std::chrono::microseconds m_ping_wait; std::vector<int> vHeightInFlight; + bool m_relay_txs; + CAmount m_fee_filter_received; uint64_t m_addr_processed = 0; uint64_t m_addr_rate_limited = 0; bool m_addr_relay_enabled{false}; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 74d53d2062..d71455bc37 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -590,7 +590,7 @@ public: bool relay, std::string& err_string) override { - const TransactionError err = BroadcastTransaction(m_node, tx, err_string, max_tx_fee, relay, /*wait_callback*/ false); + const TransactionError err = BroadcastTransaction(m_node, tx, err_string, max_tx_fee, relay, /*wait_callback=*/false); // Chain clients only care about failures to accept the tx to the mempool. Disregard non-mempool related failures. // Note: this will need to be updated if BroadcastTransactions() is updated to return other non-mempool failures // that Chain clients do not need to know about. diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 293f5ddea4..85e3c23085 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -853,7 +853,7 @@ void BitcoinGUI::aboutClicked() if(!clientModel) return; - auto dlg = new HelpMessageDialog(this, /* about */ true); + auto dlg = new HelpMessageDialog(this, /*about=*/true); GUIUtil::ShowModalDialogAsynchronously(dlg); } diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index d7a2aaaf19..d3103492a4 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -16,10 +16,11 @@ #include <qt/platformstyle.h> #include <qt/walletmodel.h> -#include <wallet/coincontrol.h> #include <interfaces/node.h> #include <key_io.h> #include <policy/policy.h> +#include <wallet/coincontrol.h> +#include <wallet/coinselection.h> #include <wallet/wallet.h> #include <QApplication> @@ -32,7 +33,6 @@ #include <QTreeWidget> using wallet::CCoinControl; -using wallet::MIN_CHANGE; QList<CAmount> CoinControlDialog::payAmounts; bool CoinControlDialog::fSubtractFeeFromAmount = false; @@ -485,11 +485,10 @@ void CoinControlDialog::updateLabels(CCoinControl& m_coin_control, WalletModel * if (!CoinControlDialog::fSubtractFeeFromAmount) nChange -= nPayFee; - // Never create dust outputs; if we would, just add the dust to the fee. - if (nChange > 0 && nChange < MIN_CHANGE) - { + if (nChange > 0) { // Assumes a p2pkh script size CTxOut txout(nChange, CScript() << std::vector<unsigned char>(24, 0)); + // Never create dust outputs; if we would, just add the dust to the fee. if (IsDust(txout, model->node().getDustRelayFee())) { nPayFee += nChange; diff --git a/src/qt/intro.cpp b/src/qt/intro.cpp index dcde7adec4..63b4055092 100644 --- a/src/qt/intro.cpp +++ b/src/qt/intro.cpp @@ -298,12 +298,12 @@ void Intro::setStatus(int status, const QString &message, quint64 bytesAvailable void Intro::UpdateFreeSpaceLabel() { - QString freeString = tr("%1 GB of space available").arg(m_bytes_available / GB_BYTES); + QString freeString = tr("%n GB of space available", "", m_bytes_available / GB_BYTES); if (m_bytes_available < m_required_space_gb * GB_BYTES) { - freeString += " " + tr("(of %1 GB needed)").arg(m_required_space_gb); + freeString += " " + tr("(of %n GB needed)", "", m_required_space_gb); ui->freeSpace->setStyleSheet("QLabel { color: #800000 }"); } else if (m_bytes_available / GB_BYTES - m_required_space_gb < 10) { - freeString += " " + tr("(%1 GB needed for full chain)").arg(m_required_space_gb); + freeString += " " + tr("(%n GB needed for full chain)", "", m_required_space_gb); ui->freeSpace->setStyleSheet("QLabel { color: #999900 }"); } else { ui->freeSpace->setStyleSheet(""); diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index cd3193a1d2..563bca76e5 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -80,7 +80,7 @@ QVariant PeerTableModel::data(const QModelIndex& index, int role) const //: An Outbound Connection to a Peer. tr("Outbound")); case ConnectionType: - return GUIUtil::ConnectionTypeToQString(rec->nodeStats.m_conn_type, /* prepend_direction */ false); + return GUIUtil::ConnectionTypeToQString(rec->nodeStats.m_conn_type, /*prepend_direction=*/false); case Network: return GUIUtil::NetworkToQString(rec->nodeStats.m_network); case Ping: diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 53e13a475b..dcc4f36aaa 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -848,7 +848,7 @@ void RPCConsole::setFontSize(int newSize) // clear console (reset icon sizes, default stylesheet) and re-add the content float oldPosFactor = 1.0 / ui->messagesWidget->verticalScrollBar()->maximum() * ui->messagesWidget->verticalScrollBar()->value(); - clear(/* keep_prompt */ true); + clear(/*keep_prompt=*/true); ui->messagesWidget->setHtml(str); ui->messagesWidget->verticalScrollBar()->setValue(oldPosFactor * ui->messagesWidget->verticalScrollBar()->maximum()); } @@ -1169,7 +1169,6 @@ void RPCConsole::updateDetailWidget() peerAddrDetails += "<br />" + tr("via %1").arg(QString::fromStdString(stats->nodeStats.addrLocal)); ui->peerHeading->setText(peerAddrDetails); ui->peerServices->setText(GUIUtil::formatServicesStr(stats->nodeStats.nServices)); - ui->peerRelayTxes->setText(stats->nodeStats.fRelayTxes ? ts.yes : ts.no); QString bip152_hb_settings; if (stats->nodeStats.m_bip152_highbandwidth_to) bip152_hb_settings = ts.to; if (stats->nodeStats.m_bip152_highbandwidth_from) bip152_hb_settings += (bip152_hb_settings.isEmpty() ? ts.from : QLatin1Char('/') + ts.from); @@ -1188,7 +1187,7 @@ void RPCConsole::updateDetailWidget() ui->timeoffset->setText(GUIUtil::formatTimeOffset(stats->nodeStats.nTimeOffset)); ui->peerVersion->setText(QString::number(stats->nodeStats.nVersion)); ui->peerSubversion->setText(QString::fromStdString(stats->nodeStats.cleanSubVer)); - ui->peerConnectionType->setText(GUIUtil::ConnectionTypeToQString(stats->nodeStats.m_conn_type, /* prepend_direction */ true)); + ui->peerConnectionType->setText(GUIUtil::ConnectionTypeToQString(stats->nodeStats.m_conn_type, /*prepend_direction=*/true)); ui->peerNetwork->setText(GUIUtil::NetworkToQString(stats->nodeStats.m_network)); if (stats->nodeStats.m_permissionFlags == NetPermissionFlags::None) { ui->peerPermissions->setText(ts.na); @@ -1221,6 +1220,7 @@ void RPCConsole::updateDetailWidget() ui->peerAddrRelayEnabled->setText(stats->nodeStateStats.m_addr_relay_enabled ? ts.yes : ts.no); ui->peerAddrProcessed->setText(QString::number(stats->nodeStateStats.m_addr_processed)); ui->peerAddrRateLimited->setText(QString::number(stats->nodeStateStats.m_addr_rate_limited)); + ui->peerRelayTxes->setText(stats->nodeStateStats.m_relay_txs ? ts.yes : ts.no); } ui->peersTabRightPanel->show(); diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 44b4fee2e7..6b0495f5a8 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -32,11 +32,11 @@ // Amount column is right-aligned it contains numbers static int column_alignments[] = { - Qt::AlignLeft|Qt::AlignVCenter, /* status */ - Qt::AlignLeft|Qt::AlignVCenter, /* watchonly */ - Qt::AlignLeft|Qt::AlignVCenter, /* date */ - Qt::AlignLeft|Qt::AlignVCenter, /* type */ - Qt::AlignLeft|Qt::AlignVCenter, /* address */ + Qt::AlignLeft|Qt::AlignVCenter, /*status=*/ + Qt::AlignLeft|Qt::AlignVCenter, /*watchonly=*/ + Qt::AlignLeft|Qt::AlignVCenter, /*date=*/ + Qt::AlignLeft|Qt::AlignVCenter, /*type=*/ + Qt::AlignLeft|Qt::AlignVCenter, /*address=*/ Qt::AlignRight|Qt::AlignVCenter /* amount */ }; diff --git a/src/rest.cpp b/src/rest.cpp index 4b6bb7ecaf..d59b6d1c13 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -670,7 +670,7 @@ static bool rest_tx(const std::any& context, HTTPRequest* req, const std::string case RetFormat::JSON: { UniValue objTx(UniValue::VOBJ); - TxToUniv(*tx, hashBlock, objTx); + TxToUniv(*tx, /*block_hash=*/hashBlock, /*entry=*/ objTx); std::string strJSON = objTx.write() + "\n"; req->WriteHeader("Content-Type", "application/json"); req->WriteReply(HTTP_OK, strJSON); @@ -855,7 +855,7 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std:: // include the script in a json output UniValue o(UniValue::VOBJ); - ScriptPubKeyToUniv(coin.out.scriptPubKey, o, true); + ScriptToUniv(coin.out.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true); utxo.pushKV("scriptPubKey", o); utxos.push_back(utxo); } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 6eb82bed43..cf72af1012 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -187,7 +187,7 @@ UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIn // coinbase transaction (i.e. i == 0) doesn't have undo data const CTxUndo* txundo = (have_undo && i > 0) ? &blockUndo.vtxundo.at(i - 1) : nullptr; UniValue objTx(UniValue::VOBJ); - TxToUniv(*tx, uint256(), objTx, true, RPCSerializationFlags(), txundo, verbosity); + TxToUniv(*tx, /*block_hash=*/uint256(), /*entry=*/objTx, /*include_hex=*/true, RPCSerializationFlags(), txundo, verbosity); txs.push_back(objTx); } break; @@ -671,7 +671,7 @@ static RPCHelpMan getblock() { {RPCResult::Type::STR, "asm", "The asm"}, {RPCResult::Type::STR, "hex", "The hex"}, - {RPCResult::Type::STR, "address", /* optional */ true, "The Bitcoin address (only if a well-defined address exists)"}, + {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, {RPCResult::Type::STR, "type", "The type (one of: " + GetAllOutputTypes() + ")"}, }}, }}, @@ -1026,7 +1026,7 @@ static RPCHelpMan gettxout() } ret.pushKV("value", ValueFromAmount(coin.out.nValue)); UniValue o(UniValue::VOBJ); - ScriptPubKeyToUniv(coin.out.scriptPubKey, o, true); + ScriptToUniv(coin.out.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true); ret.pushKV("scriptPubKey", o); ret.pushKV("coinbase", (bool)coin.fCoinBase); diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index c480a093a4..23e9d4074c 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -142,6 +142,10 @@ static const CRPCConvertParam vRPCConvertParams[] = { "send", 1, "conf_target" }, { "send", 3, "fee_rate"}, { "send", 4, "options" }, + { "sendall", 0, "recipients" }, + { "sendall", 1, "conf_target" }, + { "sendall", 3, "fee_rate"}, + { "sendall", 4, "options" }, { "importprivkey", 2, "rescan" }, { "importaddress", 2, "rescan" }, { "importaddress", 3, "p2sh" }, diff --git a/src/rpc/external_signer.cpp b/src/rpc/external_signer.cpp index 60ec15e904..82aa6f9516 100644 --- a/src/rpc/external_signer.cpp +++ b/src/rpc/external_signer.cpp @@ -22,7 +22,7 @@ static RPCHelpMan enumeratesigners() RPCResult{ RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::ARR, "signers", /* optional */ false, "", + {RPCResult::Type::ARR, "signers", /*optional=*/false, "", { {RPCResult::Type::OBJ, "", "", { diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index bc7ef0c08e..1caf4ad96c 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -14,9 +14,219 @@ #include <rpc/util.h> #include <txmempool.h> #include <univalue.h> +#include <util/moneystr.h> #include <validation.h> -static std::vector<RPCResult> MempoolEntryDescription() { return { +using node::DEFAULT_MAX_RAW_TX_FEE_RATE; +using node::NodeContext; + +static RPCHelpMan sendrawtransaction() +{ + return RPCHelpMan{"sendrawtransaction", + "\nSubmit a raw transaction (serialized, hex-encoded) to local node and network.\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" + "\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, RPCArg::Default{FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK())}, + "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + + "/kvB.\nSet to 0 to accept any fee rate.\n"}, + }, + RPCResult{ + RPCResult::Type::STR_HEX, "", "The transaction hash in hex" + }, + RPCExamples{ + "\nCreate a transaction\n" + + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\" : \\\"mytxid\\\",\\\"vout\\\":0}]\" \"{\\\"myaddress\\\":0.01}\"") + + "Sign the transaction, and get back the hex\n" + + HelpExampleCli("signrawtransactionwithwallet", "\"myhex\"") + + "\nSend the transaction (signed hex)\n" + + HelpExampleCli("sendrawtransaction", "\"signedhex\"") + + "\nAs a JSON-RPC call\n" + + HelpExampleRpc("sendrawtransaction", "\"signedhex\"") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue + { + RPCTypeCheck(request.params, { + UniValue::VSTR, + UniValueType(), // VNUM or VSTR, checked inside AmountFromValue() + }); + + CMutableTransaction mtx; + if (!DecodeHexTx(mtx, request.params[0].get_str())) { + throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input."); + } + CTransactionRef tx(MakeTransactionRef(std::move(mtx))); + + const CFeeRate max_raw_tx_fee_rate = request.params[1].isNull() ? + DEFAULT_MAX_RAW_TX_FEE_RATE : + CFeeRate(AmountFromValue(request.params[1])); + + int64_t virtual_size = GetVirtualTransactionSize(*tx); + CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size); + + std::string err_string; + AssertLockNotHeld(cs_main); + NodeContext& node = EnsureAnyNodeContext(request.context); + const TransactionError err = BroadcastTransaction(node, tx, err_string, max_raw_tx_fee, /*relay=*/true, /*wait_callback=*/true); + if (TransactionError::OK != err) { + throw JSONRPCTransactionError(err, err_string); + } + + return tx->GetHash().GetHex(); + }, + }; +} + +static RPCHelpMan testmempoolaccept() +{ + return RPCHelpMan{"testmempoolaccept", + "\nReturns result of mempool acceptance tests indicating if raw transaction(s) (serialized, hex-encoded) would be accepted by mempool.\n" + "\nIf multiple transactions are passed in, parents must come before children and package policies apply: the transactions cannot conflict with any mempool transactions or each other.\n" + "\nIf one transaction fails, other transactions may not be fully validated (the 'allowed' key will be blank).\n" + "\nThe maximum number of transactions allowed is " + ToString(MAX_PACKAGE_COUNT) + ".\n" + "\nThis checks if transactions violate the consensus or policy rules.\n" + "\nSee sendrawtransaction call.\n", + { + {"rawtxs", RPCArg::Type::ARR, RPCArg::Optional::NO, "An array of hex strings of raw transactions.", + { + {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""}, + }, + }, + {"maxfeerate", RPCArg::Type::AMOUNT, RPCArg::Default{FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK())}, + "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kvB\n"}, + }, + RPCResult{ + RPCResult::Type::ARR, "", "The result of the mempool acceptance test for each raw transaction in the input array.\n" + "Returns results for each transaction in the same order they were passed in.\n" + "Transactions that cannot be fully validated due to failures in other transactions will not contain an 'allowed' result.\n", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"}, + {RPCResult::Type::STR_HEX, "wtxid", "The transaction witness hash in hex"}, + {RPCResult::Type::STR, "package-error", /*optional=*/true, "Package validation error, if any (only possible if rawtxs had more than 1 transaction)."}, + {RPCResult::Type::BOOL, "allowed", /*optional=*/true, "Whether this tx would be accepted to the mempool and pass client-specified maxfeerate. " + "If not present, the tx was not fully validated due to a failure in another tx in the list."}, + {RPCResult::Type::NUM, "vsize", /*optional=*/true, "Virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted (only present when 'allowed' is true)"}, + {RPCResult::Type::OBJ, "fees", /*optional=*/true, "Transaction fees (only present if 'allowed' is true)", + { + {RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT}, + }}, + {RPCResult::Type::STR, "reject-reason", /*optional=*/true, "Rejection string (only present when 'allowed' is false)"}, + }}, + } + }, + RPCExamples{ + "\nCreate a transaction\n" + + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\" : \\\"mytxid\\\",\\\"vout\\\":0}]\" \"{\\\"myaddress\\\":0.01}\"") + + "Sign the transaction, and get back the hex\n" + + HelpExampleCli("signrawtransactionwithwallet", "\"myhex\"") + + "\nTest acceptance of the transaction (signed hex)\n" + + HelpExampleCli("testmempoolaccept", R"('["signedhex"]')") + + "\nAs a JSON-RPC call\n" + + HelpExampleRpc("testmempoolaccept", "[\"signedhex\"]") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue + { + RPCTypeCheck(request.params, { + UniValue::VARR, + UniValueType(), // VNUM or VSTR, checked inside AmountFromValue() + }); + const UniValue raw_transactions = request.params[0].get_array(); + if (raw_transactions.size() < 1 || raw_transactions.size() > MAX_PACKAGE_COUNT) { + throw JSONRPCError(RPC_INVALID_PARAMETER, + "Array must contain between 1 and " + ToString(MAX_PACKAGE_COUNT) + " transactions."); + } + + const CFeeRate max_raw_tx_fee_rate = request.params[1].isNull() ? + DEFAULT_MAX_RAW_TX_FEE_RATE : + CFeeRate(AmountFromValue(request.params[1])); + + std::vector<CTransactionRef> txns; + txns.reserve(raw_transactions.size()); + for (const auto& rawtx : raw_transactions.getValues()) { + CMutableTransaction mtx; + if (!DecodeHexTx(mtx, rawtx.get_str())) { + throw JSONRPCError(RPC_DESERIALIZATION_ERROR, + "TX decode failed: " + rawtx.get_str() + " Make sure the tx has at least one input."); + } + txns.emplace_back(MakeTransactionRef(std::move(mtx))); + } + + NodeContext& node = EnsureAnyNodeContext(request.context); + CTxMemPool& mempool = EnsureMemPool(node); + ChainstateManager& chainman = EnsureChainman(node); + CChainState& chainstate = chainman.ActiveChainstate(); + const PackageMempoolAcceptResult package_result = [&] { + LOCK(::cs_main); + if (txns.size() > 1) return ProcessNewPackage(chainstate, mempool, txns, /*test_accept=*/true); + return PackageMempoolAcceptResult(txns[0]->GetWitnessHash(), + chainman.ProcessTransaction(txns[0], /*test_accept=*/true)); + }(); + + UniValue rpc_result(UniValue::VARR); + // We will check transaction fees while we iterate through txns in order. If any transaction fee + // exceeds maxfeerate, we will leave the rest of the validation results blank, because it + // doesn't make sense to return a validation result for a transaction if its ancestor(s) would + // not be submitted. + bool exit_early{false}; + for (const auto& tx : txns) { + UniValue result_inner(UniValue::VOBJ); + result_inner.pushKV("txid", tx->GetHash().GetHex()); + result_inner.pushKV("wtxid", tx->GetWitnessHash().GetHex()); + if (package_result.m_state.GetResult() == PackageValidationResult::PCKG_POLICY) { + result_inner.pushKV("package-error", package_result.m_state.GetRejectReason()); + } + auto it = package_result.m_tx_results.find(tx->GetWitnessHash()); + if (exit_early || it == package_result.m_tx_results.end()) { + // Validation unfinished. Just return the txid and wtxid. + rpc_result.push_back(result_inner); + continue; + } + const auto& tx_result = it->second; + // Package testmempoolaccept doesn't allow transactions to already be in the mempool. + CHECK_NONFATAL(tx_result.m_result_type != MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + if (tx_result.m_result_type == MempoolAcceptResult::ResultType::VALID) { + const CAmount fee = tx_result.m_base_fees.value(); + // Check that fee does not exceed maximum fee + const int64_t virtual_size = tx_result.m_vsize.value(); + const CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size); + if (max_raw_tx_fee && fee > max_raw_tx_fee) { + result_inner.pushKV("allowed", false); + result_inner.pushKV("reject-reason", "max-fee-exceeded"); + exit_early = true; + } else { + // Only return the fee and vsize if the transaction would pass ATMP. + // These can be used to calculate the feerate. + result_inner.pushKV("allowed", true); + result_inner.pushKV("vsize", virtual_size); + UniValue fees(UniValue::VOBJ); + fees.pushKV("base", ValueFromAmount(fee)); + result_inner.pushKV("fees", fees); + } + } else { + result_inner.pushKV("allowed", false); + const TxValidationState state = tx_result.m_state; + if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) { + result_inner.pushKV("reject-reason", "missing-inputs"); + } else { + result_inner.pushKV("reject-reason", state.GetRejectReason()); + } + } + rpc_result.push_back(result_inner); + } + return rpc_result; + }, + }; +} + +static std::vector<RPCResult> MempoolEntryDescription() +{ + return { RPCResult{RPCResult::Type::NUM, "vsize", "virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted."}, RPCResult{RPCResult::Type::NUM, "weight", "transaction weight as defined in BIP 141."}, RPCResult{RPCResult::Type::STR_AMOUNT, "fee", /*optional=*/true, @@ -50,7 +260,8 @@ static std::vector<RPCResult> MempoolEntryDescription() { return { {RPCResult{RPCResult::Type::STR_HEX, "transactionid", "child transaction id"}}}, RPCResult{RPCResult::Type::BOOL, "bip125-replaceable", "Whether this transaction could be replaced due to BIP125 (replace-by-fee)"}, RPCResult{RPCResult::Type::BOOL, "unbroadcast", "Whether this transaction is currently unbroadcast (initial broadcast not yet acknowledged by any peers)"}, -};} + }; +} static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPoolEntry& e) EXCLUSIVE_LOCKS_REQUIRED(pool.cs) { @@ -164,7 +375,7 @@ UniValue MempoolToJSON(const CTxMemPool& pool, bool verbose, bool include_mempoo } } -RPCHelpMan getrawmempool() +static RPCHelpMan getrawmempool() { return RPCHelpMan{"getrawmempool", "\nReturns all transaction ids in memory pool as a json array of string transaction ids.\n" @@ -214,7 +425,7 @@ RPCHelpMan getrawmempool() }; } -RPCHelpMan getmempoolancestors() +static RPCHelpMan getmempoolancestors() { return RPCHelpMan{"getmempoolancestors", "\nIf txid is in the mempool, returns all in-mempool ancestors.\n", @@ -278,7 +489,7 @@ RPCHelpMan getmempoolancestors() }; } -RPCHelpMan getmempooldescendants() +static RPCHelpMan getmempooldescendants() { return RPCHelpMan{"getmempooldescendants", "\nIf txid is in the mempool, returns all in-mempool descendants.\n", @@ -343,7 +554,7 @@ RPCHelpMan getmempooldescendants() }; } -RPCHelpMan getmempoolentry() +static RPCHelpMan getmempoolentry() { return RPCHelpMan{"getmempoolentry", "\nReturns mempool data for given transaction\n", @@ -394,7 +605,7 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool) return ret; } -RPCHelpMan getmempoolinfo() +static RPCHelpMan getmempoolinfo() { return RPCHelpMan{"getmempoolinfo", "\nReturns details on the active state of the TX memory pool.\n", @@ -423,7 +634,7 @@ RPCHelpMan getmempoolinfo() }; } -RPCHelpMan savemempool() +static RPCHelpMan savemempool() { return RPCHelpMan{"savemempool", "\nDumps the mempool to disk. It will fail until the previous dump is fully loaded.\n", @@ -463,6 +674,8 @@ void RegisterMempoolRPCCommands(CRPCTable& t) static const CRPCCommand commands[]{ // category actor (function) // -------- ---------------- + {"rawtransactions", &sendrawtransaction}, + {"rawtransactions", &testmempoolaccept}, {"blockchain", &getmempoolancestors}, {"blockchain", &getmempooldescendants}, {"blockchain", &getmempoolentry}, diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 8d7b48d697..89f096981f 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -116,7 +116,7 @@ static RPCHelpMan createmultisig() {RPCResult::Type::STR, "address", "The value of the new multisig address."}, {RPCResult::Type::STR_HEX, "redeemScript", "The string value of the hex-encoded redemption script."}, {RPCResult::Type::STR, "descriptor", "The descriptor for this multisig"}, - {RPCResult::Type::ARR, "warnings", /* optional */ true, "Any warnings resulting from the creation of this multisig", + {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Any warnings resulting from the creation of this multisig", { {RPCResult::Type::STR, "", ""}, }}, diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 1bde4fccbb..225feabf14 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -93,7 +93,7 @@ static RPCHelpMan getpeerinfo() { return RPCHelpMan{ "getpeerinfo", - "\nReturns data about each connected network node as a json array of objects.\n", + "Returns data about each connected network peer as a json array of objects.", {}, RPCResult{ RPCResult::Type::ARR, "", "", @@ -105,7 +105,7 @@ static RPCHelpMan getpeerinfo() {RPCResult::Type::STR, "addr", "(host:port) The IP address and port of the peer"}, {RPCResult::Type::STR, "addrbind", /*optional=*/true, "(ip:port) Bind address of the connection to the peer"}, {RPCResult::Type::STR, "addrlocal", /*optional=*/true, "(ip:port) Local address as reported by the peer"}, - {RPCResult::Type::STR, "network", "Network (" + Join(GetNetworkNames(/* append_unroutable */ true), ", ") + ")"}, + {RPCResult::Type::STR, "network", "Network (" + Join(GetNetworkNames(/*append_unroutable=*/true), ", ") + ")"}, {RPCResult::Type::NUM, "mapped_as", /*optional=*/true, "The AS in the BGP route to the peer used for diversifying\n" "peer selection (only available if the asmap config flag is set)"}, {RPCResult::Type::STR_HEX, "services", "The services offered"}, @@ -113,7 +113,7 @@ static RPCHelpMan getpeerinfo() { {RPCResult::Type::STR, "SERVICE_NAME", "the service name if it is recognised"} }}, - {RPCResult::Type::BOOL, "relaytxes", "Whether peer has asked us to relay transactions to it"}, + {RPCResult::Type::BOOL, "relaytxes", /*optional=*/true, "Whether peer has asked us to relay transactions to it"}, {RPCResult::Type::NUM_TIME, "lastsend", "The " + UNIX_EPOCH_TIME + " of the last send"}, {RPCResult::Type::NUM_TIME, "lastrecv", "The " + UNIX_EPOCH_TIME + " of the last receive"}, {RPCResult::Type::NUM_TIME, "last_transaction", "The " + UNIX_EPOCH_TIME + " of the last valid transaction received from this peer"}, @@ -144,7 +144,7 @@ static RPCHelpMan getpeerinfo() { {RPCResult::Type::STR, "permission_type", Join(NET_PERMISSIONS_DOC, ",\n") + ".\n"}, }}, - {RPCResult::Type::NUM, "minfeefilter", "The minimum fee rate for transactions this peer accepts"}, + {RPCResult::Type::NUM, "minfeefilter", /*optional=*/true, "The minimum fee rate for transactions this peer accepts"}, {RPCResult::Type::OBJ_DYN, "bytessent_per_msg", "", { {RPCResult::Type::NUM, "msg", "The total bytes sent aggregated by message type\n" @@ -197,7 +197,6 @@ static RPCHelpMan getpeerinfo() } obj.pushKV("services", strprintf("%016x", stats.nServices)); obj.pushKV("servicesnames", GetServicesNames(stats.nServices)); - obj.pushKV("relaytxes", stats.fRelayTxes); obj.pushKV("lastsend", count_seconds(stats.m_last_send)); obj.pushKV("lastrecv", count_seconds(stats.m_last_recv)); obj.pushKV("last_transaction", count_seconds(stats.m_last_tx_time)); @@ -232,6 +231,8 @@ static RPCHelpMan getpeerinfo() heights.push_back(height); } obj.pushKV("inflight", heights); + obj.pushKV("relaytxes", statestats.m_relay_txs); + obj.pushKV("minfeefilter", ValueFromAmount(statestats.m_fee_filter_received)); obj.pushKV("addr_relay_enabled", statestats.m_addr_relay_enabled); obj.pushKV("addr_processed", statestats.m_addr_processed); obj.pushKV("addr_rate_limited", statestats.m_addr_rate_limited); @@ -241,7 +242,6 @@ static RPCHelpMan getpeerinfo() permissions.push_back(permission); } obj.pushKV("permissions", permissions); - obj.pushKV("minfeefilter", ValueFromAmount(stats.minFeeFilter)); UniValue sendPerMsgCmd(UniValue::VOBJ); for (const auto& i : stats.mapSendBytesPerMsgCmd) { @@ -888,7 +888,7 @@ static RPCHelpMan getnodeaddresses() } // returns a shuffled list of CAddress - const std::vector<CAddress> vAddr{connman.GetAddresses(count, /* max_pct */ 0, network)}; + const std::vector<CAddress> vAddr{connman.GetAddresses(count, /*max_pct=*/0, network)}; UniValue ret(UniValue::VARR); for (const CAddress& addr : vAddr) { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 8e60e82f35..8e4b396da9 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -33,9 +33,9 @@ #include <script/standard.h> #include <uint256.h> #include <util/bip32.h> -#include <util/moneystr.h> #include <util/strencodings.h> #include <util/string.h> +#include <util/vector.h> #include <validation.h> #include <validationinterface.h> @@ -46,7 +46,6 @@ using node::AnalyzePSBT; using node::BroadcastTransaction; -using node::DEFAULT_MAX_RAW_TX_FEE_RATE; using node::FindCoins; using node::GetTransaction; using node::NodeContext; @@ -60,7 +59,7 @@ static void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& // Blockchain contextual information (confirmations and blocktime) is not // available to code in bitcoin-common, so we query them here and push the // data into the returned UniValue. - TxToUniv(tx, uint256(), entry, true, RPCSerializationFlags()); + TxToUniv(tx, /*block_hash=*/uint256(), entry, /*include_hex=*/true, RPCSerializationFlags()); if (!hashBlock.IsNull()) { LOCK(cs_main); @@ -79,6 +78,54 @@ static void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& } } +static std::vector<RPCResult> DecodeTxDoc(const std::string& txid_field_doc) +{ + return { + {RPCResult::Type::STR_HEX, "txid", txid_field_doc}, + {RPCResult::Type::STR_HEX, "hash", "The transaction hash (differs from txid for witness transactions)"}, + {RPCResult::Type::NUM, "size", "The serialized transaction size"}, + {RPCResult::Type::NUM, "vsize", "The virtual transaction size (differs from size for witness transactions)"}, + {RPCResult::Type::NUM, "weight", "The transaction's weight (between vsize*4-3 and vsize*4)"}, + {RPCResult::Type::NUM, "version", "The version"}, + {RPCResult::Type::NUM_TIME, "locktime", "The lock time"}, + {RPCResult::Type::ARR, "vin", "", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR_HEX, "coinbase", /*optional=*/true, "The coinbase value (only if coinbase transaction)"}, + {RPCResult::Type::STR_HEX, "txid", /*optional=*/true, "The transaction id (if not coinbase transaction)"}, + {RPCResult::Type::NUM, "vout", /*optional=*/true, "The output number (if not coinbase transaction)"}, + {RPCResult::Type::OBJ, "scriptSig", /*optional=*/true, "The script (if not coinbase transaction)", + { + {RPCResult::Type::STR, "asm", "asm"}, + {RPCResult::Type::STR_HEX, "hex", "hex"}, + }}, + {RPCResult::Type::ARR, "txinwitness", /*optional=*/true, "", + { + {RPCResult::Type::STR_HEX, "hex", "hex-encoded witness data (if any)"}, + }}, + {RPCResult::Type::NUM, "sequence", "The script sequence number"}, + }}, + }}, + {RPCResult::Type::ARR, "vout", "", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR_AMOUNT, "value", "The value in " + CURRENCY_UNIT}, + {RPCResult::Type::NUM, "n", "index"}, + {RPCResult::Type::OBJ, "scriptPubKey", "", + { + {RPCResult::Type::STR, "asm", "the asm"}, + {RPCResult::Type::STR, "desc", "Inferred descriptor for the output"}, + {RPCResult::Type::STR_HEX, "hex", "the hex"}, + {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, + {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, + }}, + }}, + }}, + }; +} + static std::vector<RPCArg> CreateTxDoc() { return { @@ -120,7 +167,7 @@ static RPCHelpMan getrawtransaction() { return RPCHelpMan{ "getrawtransaction", - "\nReturn the raw transaction data.\n" + "Return the raw transaction data.\n" "\nBy default, this call only returns a transaction if it is in the mempool. If -txindex is enabled\n" "and no blockhash argument is passed, it will return the transaction if it is in the mempool or any block.\n" @@ -129,7 +176,7 @@ static RPCHelpMan getrawtransaction() "\nHint: Use gettransaction for wallet transactions.\n" "\nIf verbose is 'true', returns an Object with information about 'txid'.\n" - "If verbose is 'false' or omitted, returns a string that is serialized, hex-encoded data for 'txid'.\n", + "If verbose is 'false' or omitted, returns a string that is serialized, hex-encoded data for 'txid'.", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"}, {"verbose", RPCArg::Type::BOOL, RPCArg::Default{false}, "If false, return a string, otherwise return a json object"}, @@ -141,55 +188,16 @@ static RPCHelpMan getrawtransaction() }, RPCResult{"if verbose is set to true", RPCResult::Type::OBJ, "", "", + Cat<std::vector<RPCResult>>( { {RPCResult::Type::BOOL, "in_active_chain", /*optional=*/true, "Whether specified block is in the active chain or not (only present with explicit \"blockhash\" argument)"}, - {RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded data for 'txid'"}, - {RPCResult::Type::STR_HEX, "txid", "The transaction id (same as provided)"}, - {RPCResult::Type::STR_HEX, "hash", "The transaction hash (differs from txid for witness transactions)"}, - {RPCResult::Type::NUM, "size", "The serialized transaction size"}, - {RPCResult::Type::NUM, "vsize", "The virtual transaction size (differs from size for witness transactions)"}, - {RPCResult::Type::NUM, "weight", "The transaction's weight (between vsize*4-3 and vsize*4)"}, - {RPCResult::Type::NUM, "version", "The version"}, - {RPCResult::Type::NUM_TIME, "locktime", "The lock time"}, - {RPCResult::Type::ARR, "vin", "", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR_HEX, "txid", "The transaction id"}, - {RPCResult::Type::NUM, "vout", "The output number"}, - {RPCResult::Type::OBJ, "scriptSig", "The script", - { - {RPCResult::Type::STR, "asm", "asm"}, - {RPCResult::Type::STR_HEX, "hex", "hex"}, - }}, - {RPCResult::Type::NUM, "sequence", "The script sequence number"}, - {RPCResult::Type::ARR, "txinwitness", /*optional=*/true, "", - { - {RPCResult::Type::STR_HEX, "hex", "hex-encoded witness data (if any)"}, - }}, - }}, - }}, - {RPCResult::Type::ARR, "vout", "", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::NUM, "value", "The value in " + CURRENCY_UNIT}, - {RPCResult::Type::NUM, "n", "index"}, - {RPCResult::Type::OBJ, "scriptPubKey", "", - { - {RPCResult::Type::STR, "asm", "the asm"}, - {RPCResult::Type::STR, "desc", "Inferred descriptor for the output"}, - {RPCResult::Type::STR, "hex", "the hex"}, - {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, - {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, - }}, - }}, - }}, {RPCResult::Type::STR_HEX, "blockhash", /*optional=*/true, "the block hash"}, {RPCResult::Type::NUM, "confirmations", /*optional=*/true, "The confirmations"}, {RPCResult::Type::NUM_TIME, "blocktime", /*optional=*/true, "The block time expressed in " + UNIX_EPOCH_TIME}, {RPCResult::Type::NUM, "time", /*optional=*/true, "Same as \"blocktime\""}, - } + {RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded data for 'txid'"}, + }, + DecodeTxDoc(/*txid_field_doc=*/"The transaction id (same as provided)")), }, }, RPCExamples{ @@ -309,7 +317,7 @@ static RPCHelpMan createrawtransaction() static RPCHelpMan decoderawtransaction() { return RPCHelpMan{"decoderawtransaction", - "\nReturn a JSON object representing the serialized, hex-encoded transaction.\n", + "Return a JSON object representing the serialized, hex-encoded transaction.", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction hex string"}, {"iswitness", RPCArg::Type::BOOL, RPCArg::DefaultHint{"depends on heuristic tests"}, "Whether the transaction hex is a serialized witness transaction.\n" @@ -322,50 +330,7 @@ static RPCHelpMan decoderawtransaction() }, RPCResult{ RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR_HEX, "txid", "The transaction id"}, - {RPCResult::Type::STR_HEX, "hash", "The transaction hash (differs from txid for witness transactions)"}, - {RPCResult::Type::NUM, "size", "The transaction size"}, - {RPCResult::Type::NUM, "vsize", "The virtual transaction size (differs from size for witness transactions)"}, - {RPCResult::Type::NUM, "weight", "The transaction's weight (between vsize*4 - 3 and vsize*4)"}, - {RPCResult::Type::NUM, "version", "The version"}, - {RPCResult::Type::NUM_TIME, "locktime", "The lock time"}, - {RPCResult::Type::ARR, "vin", "", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR_HEX, "coinbase", /*optional=*/true, ""}, - {RPCResult::Type::STR_HEX, "txid", /*optional=*/true, "The transaction id"}, - {RPCResult::Type::NUM, "vout", /*optional=*/true, "The output number"}, - {RPCResult::Type::OBJ, "scriptSig", /*optional=*/true, "The script", - { - {RPCResult::Type::STR, "asm", "asm"}, - {RPCResult::Type::STR_HEX, "hex", "hex"}, - }}, - {RPCResult::Type::ARR, "txinwitness", /*optional=*/true, "", - { - {RPCResult::Type::STR_HEX, "hex", "hex-encoded witness data (if any)"}, - }}, - {RPCResult::Type::NUM, "sequence", "The script sequence number"}, - }}, - }}, - {RPCResult::Type::ARR, "vout", "", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::NUM, "value", "The value in " + CURRENCY_UNIT}, - {RPCResult::Type::NUM, "n", "index"}, - {RPCResult::Type::OBJ, "scriptPubKey", "", - { - {RPCResult::Type::STR, "asm", "the asm"}, - {RPCResult::Type::STR, "desc", "Inferred descriptor for the output"}, - {RPCResult::Type::STR_HEX, "hex", "the hex"}, - {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, - {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, - }}, - }}, - }}, - } + DecodeTxDoc(/*txid_field_doc=*/"The transaction id"), }, RPCExamples{ HelpExampleCli("decoderawtransaction", "\"hexstring\"") @@ -385,7 +350,7 @@ static RPCHelpMan decoderawtransaction() } UniValue result(UniValue::VOBJ); - TxToUniv(CTransaction(std::move(mtx)), uint256(), result, false); + TxToUniv(CTransaction(std::move(mtx)), /*block_hash=*/uint256(), /*entry=*/result, /*include_hex=*/false); return result; }, @@ -437,7 +402,7 @@ static RPCHelpMan decodescript() } else { // Empty scripts are valid } - ScriptPubKeyToUniv(script, r, /* include_hex */ false); + ScriptToUniv(script, /*out=*/r, /*include_hex=*/false, /*include_address=*/true); std::vector<std::vector<unsigned char>> solutions_data; const TxoutType which_type{Solver(script, solutions_data)}; @@ -514,7 +479,7 @@ static RPCHelpMan decodescript() // Scripts that are not fit for P2WPKH are encoded as P2WSH. segwitScr = GetScriptForDestination(WitnessV0ScriptHash(script)); } - ScriptPubKeyToUniv(segwitScr, sr, /* include_hex */ true); + ScriptToUniv(segwitScr, /*out=*/sr, /*include_hex=*/true, /*include_address=*/true); sr.pushKV("p2sh-segwit", EncodeDestination(ScriptHash(segwitScr))); r.pushKV("segwit", sr); } @@ -714,210 +679,6 @@ static RPCHelpMan signrawtransactionwithkey() }; } -static RPCHelpMan sendrawtransaction() -{ - return RPCHelpMan{"sendrawtransaction", - "\nSubmit a raw transaction (serialized, hex-encoded) to local node and network.\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" - "\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, RPCArg::Default{FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK())}, - "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + - "/kvB.\nSet to 0 to accept any fee rate.\n"}, - }, - RPCResult{ - RPCResult::Type::STR_HEX, "", "The transaction hash in hex" - }, - RPCExamples{ - "\nCreate a transaction\n" - + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\" : \\\"mytxid\\\",\\\"vout\\\":0}]\" \"{\\\"myaddress\\\":0.01}\"") + - "Sign the transaction, and get back the hex\n" - + HelpExampleCli("signrawtransactionwithwallet", "\"myhex\"") + - "\nSend the transaction (signed hex)\n" - + HelpExampleCli("sendrawtransaction", "\"signedhex\"") + - "\nAs a JSON-RPC call\n" - + HelpExampleRpc("sendrawtransaction", "\"signedhex\"") - }, - [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue -{ - RPCTypeCheck(request.params, { - UniValue::VSTR, - UniValueType(), // VNUM or VSTR, checked inside AmountFromValue() - }); - - CMutableTransaction mtx; - if (!DecodeHexTx(mtx, request.params[0].get_str())) { - throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input."); - } - CTransactionRef tx(MakeTransactionRef(std::move(mtx))); - - const CFeeRate max_raw_tx_fee_rate = request.params[1].isNull() ? - DEFAULT_MAX_RAW_TX_FEE_RATE : - CFeeRate(AmountFromValue(request.params[1])); - - int64_t virtual_size = GetVirtualTransactionSize(*tx); - CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size); - - std::string err_string; - AssertLockNotHeld(cs_main); - NodeContext& node = EnsureAnyNodeContext(request.context); - const TransactionError err = BroadcastTransaction(node, tx, err_string, max_raw_tx_fee, /*relay*/ true, /*wait_callback*/ true); - if (TransactionError::OK != err) { - throw JSONRPCTransactionError(err, err_string); - } - - return tx->GetHash().GetHex(); -}, - }; -} - -static RPCHelpMan testmempoolaccept() -{ - return RPCHelpMan{"testmempoolaccept", - "\nReturns result of mempool acceptance tests indicating if raw transaction(s) (serialized, hex-encoded) would be accepted by mempool.\n" - "\nIf multiple transactions are passed in, parents must come before children and package policies apply: the transactions cannot conflict with any mempool transactions or each other.\n" - "\nIf one transaction fails, other transactions may not be fully validated (the 'allowed' key will be blank).\n" - "\nThe maximum number of transactions allowed is " + ToString(MAX_PACKAGE_COUNT) + ".\n" - "\nThis checks if transactions violate the consensus or policy rules.\n" - "\nSee sendrawtransaction call.\n", - { - {"rawtxs", RPCArg::Type::ARR, RPCArg::Optional::NO, "An array of hex strings of raw transactions.", - { - {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""}, - }, - }, - {"maxfeerate", RPCArg::Type::AMOUNT, RPCArg::Default{FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK())}, - "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kvB\n"}, - }, - RPCResult{ - RPCResult::Type::ARR, "", "The result of the mempool acceptance test for each raw transaction in the input array.\n" - "Returns results for each transaction in the same order they were passed in.\n" - "Transactions that cannot be fully validated due to failures in other transactions will not contain an 'allowed' result.\n", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"}, - {RPCResult::Type::STR_HEX, "wtxid", "The transaction witness hash in hex"}, - {RPCResult::Type::STR, "package-error", /*optional=*/true, "Package validation error, if any (only possible if rawtxs had more than 1 transaction)."}, - {RPCResult::Type::BOOL, "allowed", /*optional=*/true, "Whether this tx would be accepted to the mempool and pass client-specified maxfeerate. " - "If not present, the tx was not fully validated due to a failure in another tx in the list."}, - {RPCResult::Type::NUM, "vsize", /*optional=*/true, "Virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted (only present when 'allowed' is true)"}, - {RPCResult::Type::OBJ, "fees", /*optional=*/true, "Transaction fees (only present if 'allowed' is true)", - { - {RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT}, - }}, - {RPCResult::Type::STR, "reject-reason", /*optional=*/true, "Rejection string (only present when 'allowed' is false)"}, - }}, - } - }, - RPCExamples{ - "\nCreate a transaction\n" - + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\" : \\\"mytxid\\\",\\\"vout\\\":0}]\" \"{\\\"myaddress\\\":0.01}\"") + - "Sign the transaction, and get back the hex\n" - + HelpExampleCli("signrawtransactionwithwallet", "\"myhex\"") + - "\nTest acceptance of the transaction (signed hex)\n" - + HelpExampleCli("testmempoolaccept", R"('["signedhex"]')") + - "\nAs a JSON-RPC call\n" - + HelpExampleRpc("testmempoolaccept", "[\"signedhex\"]") - }, - [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue -{ - RPCTypeCheck(request.params, { - UniValue::VARR, - UniValueType(), // VNUM or VSTR, checked inside AmountFromValue() - }); - const UniValue raw_transactions = request.params[0].get_array(); - if (raw_transactions.size() < 1 || raw_transactions.size() > MAX_PACKAGE_COUNT) { - throw JSONRPCError(RPC_INVALID_PARAMETER, - "Array must contain between 1 and " + ToString(MAX_PACKAGE_COUNT) + " transactions."); - } - - const CFeeRate max_raw_tx_fee_rate = request.params[1].isNull() ? - DEFAULT_MAX_RAW_TX_FEE_RATE : - CFeeRate(AmountFromValue(request.params[1])); - - std::vector<CTransactionRef> txns; - txns.reserve(raw_transactions.size()); - for (const auto& rawtx : raw_transactions.getValues()) { - CMutableTransaction mtx; - if (!DecodeHexTx(mtx, rawtx.get_str())) { - throw JSONRPCError(RPC_DESERIALIZATION_ERROR, - "TX decode failed: " + rawtx.get_str() + " Make sure the tx has at least one input."); - } - txns.emplace_back(MakeTransactionRef(std::move(mtx))); - } - - NodeContext& node = EnsureAnyNodeContext(request.context); - CTxMemPool& mempool = EnsureMemPool(node); - ChainstateManager& chainman = EnsureChainman(node); - CChainState& chainstate = chainman.ActiveChainstate(); - const PackageMempoolAcceptResult package_result = [&] { - LOCK(::cs_main); - if (txns.size() > 1) return ProcessNewPackage(chainstate, mempool, txns, /* test_accept */ true); - return PackageMempoolAcceptResult(txns[0]->GetWitnessHash(), - chainman.ProcessTransaction(txns[0], /*test_accept=*/ true)); - }(); - - UniValue rpc_result(UniValue::VARR); - // We will check transaction fees while we iterate through txns in order. If any transaction fee - // exceeds maxfeerate, we will leave the rest of the validation results blank, because it - // doesn't make sense to return a validation result for a transaction if its ancestor(s) would - // not be submitted. - bool exit_early{false}; - for (const auto& tx : txns) { - UniValue result_inner(UniValue::VOBJ); - result_inner.pushKV("txid", tx->GetHash().GetHex()); - result_inner.pushKV("wtxid", tx->GetWitnessHash().GetHex()); - if (package_result.m_state.GetResult() == PackageValidationResult::PCKG_POLICY) { - result_inner.pushKV("package-error", package_result.m_state.GetRejectReason()); - } - auto it = package_result.m_tx_results.find(tx->GetWitnessHash()); - if (exit_early || it == package_result.m_tx_results.end()) { - // Validation unfinished. Just return the txid and wtxid. - rpc_result.push_back(result_inner); - continue; - } - const auto& tx_result = it->second; - // Package testmempoolaccept doesn't allow transactions to already be in the mempool. - CHECK_NONFATAL(tx_result.m_result_type != MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); - if (tx_result.m_result_type == MempoolAcceptResult::ResultType::VALID) { - const CAmount fee = tx_result.m_base_fees.value(); - // Check that fee does not exceed maximum fee - const int64_t virtual_size = tx_result.m_vsize.value(); - const CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size); - if (max_raw_tx_fee && fee > max_raw_tx_fee) { - result_inner.pushKV("allowed", false); - result_inner.pushKV("reject-reason", "max-fee-exceeded"); - exit_early = true; - } else { - // Only return the fee and vsize if the transaction would pass ATMP. - // These can be used to calculate the feerate. - result_inner.pushKV("allowed", true); - result_inner.pushKV("vsize", virtual_size); - UniValue fees(UniValue::VOBJ); - fees.pushKV("base", ValueFromAmount(fee)); - result_inner.pushKV("fees", fees); - } - } else { - result_inner.pushKV("allowed", false); - const TxValidationState state = tx_result.m_state; - if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) { - result_inner.pushKV("reject-reason", "missing-inputs"); - } else { - result_inner.pushKV("reject-reason", state.GetRejectReason()); - } - } - rpc_result.push_back(result_inner); - } - return rpc_result; -}, - }; -} - static RPCHelpMan decodepsbt() { return RPCHelpMan{ @@ -1106,7 +867,7 @@ static RPCHelpMan decodepsbt() // Add the decoded tx UniValue tx_univ(UniValue::VOBJ); - TxToUniv(CTransaction(*psbtx.tx), uint256(), tx_univ, false); + TxToUniv(CTransaction(*psbtx.tx), /*block_hash=*/uint256(), /*entry=*/tx_univ, /*include_hex=*/false); result.pushKV("tx", tx_univ); // Add the global xpubs @@ -1162,7 +923,7 @@ static RPCHelpMan decodepsbt() txout = input.witness_utxo; UniValue o(UniValue::VOBJ); - ScriptPubKeyToUniv(txout.scriptPubKey, o, /* include_hex */ true); + ScriptToUniv(txout.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true); UniValue out(UniValue::VOBJ); out.pushKV("amount", ValueFromAmount(txout.nValue)); @@ -1176,7 +937,7 @@ static RPCHelpMan decodepsbt() txout = input.non_witness_utxo->vout[psbtx.tx->vin[i].prevout.n]; UniValue non_wit(UniValue::VOBJ); - TxToUniv(*input.non_witness_utxo, uint256(), non_wit, false); + TxToUniv(*input.non_witness_utxo, /*block_hash=*/uint256(), /*entry=*/non_wit, /*include_hex=*/false); in.pushKV("non_witness_utxo", non_wit); have_a_utxo = true; @@ -1209,12 +970,12 @@ static RPCHelpMan decodepsbt() // Redeem script and witness script if (!input.redeem_script.empty()) { UniValue r(UniValue::VOBJ); - ScriptToUniv(input.redeem_script, r); + ScriptToUniv(input.redeem_script, /*out=*/r); in.pushKV("redeem_script", r); } if (!input.witness_script.empty()) { UniValue r(UniValue::VOBJ); - ScriptToUniv(input.witness_script, r); + ScriptToUniv(input.witness_script, /*out=*/r); in.pushKV("witness_script", r); } @@ -1319,12 +1080,12 @@ static RPCHelpMan decodepsbt() // Redeem script and witness script if (!output.redeem_script.empty()) { UniValue r(UniValue::VOBJ); - ScriptToUniv(output.redeem_script, r); + ScriptToUniv(output.redeem_script, /*out=*/r); out.pushKV("redeem_script", r); } if (!output.witness_script.empty()) { UniValue r(UniValue::VOBJ); - ScriptToUniv(output.witness_script, r); + ScriptToUniv(output.witness_script, /*out=*/r); out.pushKV("witness_script", r); } @@ -1928,10 +1689,8 @@ static const CRPCCommand commands[] = { "rawtransactions", &createrawtransaction, }, { "rawtransactions", &decoderawtransaction, }, { "rawtransactions", &decodescript, }, - { "rawtransactions", &sendrawtransaction, }, { "rawtransactions", &combinerawtransaction, }, { "rawtransactions", &signrawtransactionwithkey, }, - { "rawtransactions", &testmempoolaccept, }, { "rawtransactions", &decodepsbt, }, { "rawtransactions", &combinepsbt, }, { "rawtransactions", &finalizepsbt, }, diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index c70236cc1c..333ed6f5da 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -167,7 +167,7 @@ static RPCHelpMan stop() // to the client (intended for testing) "\nRequest a graceful shutdown of " PACKAGE_NAME ".", { - {"wait", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "how long to wait in ms", "", {}, /* hidden */ true}, + {"wait", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "how long to wait in ms", "", {}, /*hidden=*/true}, }, RPCResult{RPCResult::Type::STR, "", "A string with the content '" + RESULT + "'"}, RPCExamples{""}, diff --git a/src/rpc/txoutproof.cpp b/src/rpc/txoutproof.cpp index 2700fb400c..a5443b0329 100644 --- a/src/rpc/txoutproof.cpp +++ b/src/rpc/txoutproof.cpp @@ -87,7 +87,7 @@ static RPCHelpMan gettxoutproof() LOCK(cs_main); if (pblockindex == nullptr) { - const CTransactionRef tx = GetTransaction(/* block_index */ nullptr, /* mempool */ nullptr, *setTxids.begin(), Params().GetConsensus(), hashBlock); + const CTransactionRef tx = GetTransaction(/*block_index=*/nullptr, /*mempool=*/nullptr, *setTxids.begin(), Params().GetConsensus(), hashBlock); if (!tx || hashBlock.IsNull()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not yet in block"); } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 7c859268be..9c9e6e9f11 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -774,7 +774,7 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const // Elements in a JSON structure (dictionary or array) are separated by a comma const std::string maybe_separator{outer_type != OuterType::NONE ? "," : ""}; - // The key name if recursed into an dictionary + // The key name if recursed into a dictionary const std::string maybe_key{ outer_type == OuterType::OBJ ? "\"" + this->m_key_name + "\" : " : @@ -865,10 +865,11 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const bool RPCResult::MatchesType(const UniValue& result) const { - switch (m_type) { - case Type::ELISION: { - return false; + if (m_skip_type_check) { + return true; } + switch (m_type) { + case Type::ELISION: case Type::ANY: { return true; } @@ -889,11 +890,52 @@ bool RPCResult::MatchesType(const UniValue& result) const } case Type::ARR_FIXED: case Type::ARR: { - return UniValue::VARR == result.getType(); + if (UniValue::VARR != result.getType()) return false; + for (size_t i{0}; i < result.get_array().size(); ++i) { + // If there are more results than documented, re-use the last doc_inner. + const RPCResult& doc_inner{m_inner.at(std::min(m_inner.size() - 1, i))}; + if (!doc_inner.MatchesType(result.get_array()[i])) return false; + } + return true; // empty result array is valid } case Type::OBJ_DYN: case Type::OBJ: { - return UniValue::VOBJ == result.getType(); + if (UniValue::VOBJ != result.getType()) return false; + if (!m_inner.empty() && m_inner.at(0).m_type == Type::ELISION) return true; + if (m_type == Type::OBJ_DYN) { + const RPCResult& doc_inner{m_inner.at(0)}; // Assume all types are the same, randomly pick the first + for (size_t i{0}; i < result.get_obj().size(); ++i) { + if (!doc_inner.MatchesType(result.get_obj()[i])) { + return false; + } + } + return true; // empty result obj is valid + } + std::set<std::string> doc_keys; + for (const auto& doc_entry : m_inner) { + doc_keys.insert(doc_entry.m_key_name); + } + std::map<std::string, UniValue> result_obj; + result.getObjMap(result_obj); + for (const auto& result_entry : result_obj) { + if (doc_keys.find(result_entry.first) == doc_keys.end()) { + return false; // missing documentation + } + } + + for (const auto& doc_entry : m_inner) { + const auto result_it{result_obj.find(doc_entry.m_key_name)}; + if (result_it == result_obj.end()) { + if (!doc_entry.m_optional) { + return false; // result is missing a required key + } + continue; + } + if (!doc_entry.MatchesType(result_it->second)) { + return false; // wrong type + } + } + return true; } } // no default case, so the compiler can warn about missing cases CHECK_NONFATAL(false); diff --git a/src/rpc/util.h b/src/rpc/util.h index 89d32d4193..e16fed75bc 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -256,6 +256,7 @@ struct RPCResult { const std::string m_key_name; //!< Only used for dicts const std::vector<RPCResult> m_inner; //!< Only used for arrays or dicts const bool m_optional; + const bool m_skip_type_check; const std::string m_description; const std::string m_cond; @@ -270,6 +271,7 @@ struct RPCResult { m_key_name{std::move(m_key_name)}, m_inner{std::move(inner)}, m_optional{optional}, + m_skip_type_check{false}, m_description{std::move(description)}, m_cond{std::move(cond)} { @@ -290,11 +292,13 @@ struct RPCResult { const std::string m_key_name, const bool optional, const std::string description, - const std::vector<RPCResult> inner = {}) + const std::vector<RPCResult> inner = {}, + bool skip_type_check = false) : m_type{std::move(type)}, m_key_name{std::move(m_key_name)}, m_inner{std::move(inner)}, m_optional{optional}, + m_skip_type_check{skip_type_check}, m_description{std::move(description)}, m_cond{} { @@ -305,8 +309,9 @@ struct RPCResult { const Type type, const std::string m_key_name, const std::string description, - const std::vector<RPCResult> inner = {}) - : RPCResult{type, m_key_name, false, description, inner} {} + const std::vector<RPCResult> inner = {}, + bool skip_type_check = false) + : RPCResult{type, m_key_name, false, description, inner, skip_type_check} {} /** Append the sections of the result. */ void ToSections(Sections& sections, OuterType outer_type = OuterType::NONE, const int current_indent = 0) const; diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 11b1a1c887..23fb9b6226 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -2009,7 +2009,7 @@ bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, const C // The scriptSig must be _exactly_ CScript(), otherwise we reintroduce malleability. return set_error(serror, SCRIPT_ERR_WITNESS_MALLEATED); } - if (!VerifyWitnessProgram(*witness, witnessversion, witnessprogram, flags, checker, serror, /* is_p2sh */ false)) { + if (!VerifyWitnessProgram(*witness, witnessversion, witnessprogram, flags, checker, serror, /*is_p2sh=*/false)) { return false; } // Bypass the cleanstack check at the end. The actual stack is obviously not clean @@ -2054,7 +2054,7 @@ bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, const C // reintroduce malleability. return set_error(serror, SCRIPT_ERR_WITNESS_MALLEATED_P2SH); } - if (!VerifyWitnessProgram(*witness, witnessversion, witnessprogram, flags, checker, serror, /* is_p2sh */ true)) { + if (!VerifyWitnessProgram(*witness, witnessversion, witnessprogram, flags, checker, serror, /*is_p2sh=*/true)) { return false; } // Bypass the cleanstack check at the end. The actual stack is obviously not clean diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 2e5c49e0b6..d77515f16c 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -655,7 +655,7 @@ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, CTxIn& txin = mtx.vin[i]; auto coin = coins.find(txin.prevout); if (coin == coins.end() || coin->second.IsSpent()) { - txdata.Init(txConst, /* spent_outputs */ {}, /* force */ true); + txdata.Init(txConst, /*spent_outputs=*/{}, /*force=*/true); break; } else { spent_outputs.emplace_back(coin->second.out.nValue, coin->second.out.scriptPubKey); diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index 6965f40253..ea1a27c6f6 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -235,12 +235,6 @@ PosixLockedPageAllocator::PosixLockedPageAllocator() #endif } -// Some systems (at least OS X) do not define MAP_ANONYMOUS yet and define -// MAP_ANON which is deprecated -#ifndef MAP_ANONYMOUS -#define MAP_ANONYMOUS MAP_ANON -#endif - void *PosixLockedPageAllocator::AllocateLocked(size_t len, bool *lockingSuccess) { void *addr; diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 404929b690..30add9c16d 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -311,7 +311,7 @@ void DoCheck(const std::string& prv, const std::string& pub, const std::string& spend.vout.resize(1); std::vector<CTxOut> utxos(1); PrecomputedTransactionData txdata; - txdata.Init(spend, std::move(utxos), /* force */ true); + txdata.Init(spend, std::move(utxos), /*force=*/true); MutableTransactionSignatureCreator creator(&spend, 0, CAmount{0}, &txdata, SIGHASH_DEFAULT); SignatureData sigdata; BOOST_CHECK_MESSAGE(ProduceSignature(Merge(keys_priv, script_provider), creator, spks[n], sigdata), prv); diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp index fb11ea36ce..4981287152 100644 --- a/src/test/fuzz/net.cpp +++ b/src/test/fuzz/net.cpp @@ -52,16 +52,6 @@ FUZZ_TARGET_INIT(net, initialize_net) } }, [&] { - const std::optional<CInv> inv_opt = ConsumeDeserializable<CInv>(fuzzed_data_provider); - if (!inv_opt) { - return; - } - node.AddKnownTx(inv_opt->hash); - }, - [&] { - node.PushTxInventory(ConsumeUInt256(fuzzed_data_provider)); - }, - [&] { const std::optional<CService> service_opt = ConsumeDeserializable<CService>(fuzzed_data_provider); if (!service_opt) { return; diff --git a/src/test/fuzz/node_eviction.cpp b/src/test/fuzz/node_eviction.cpp index 2e90085744..6a363f00f7 100644 --- a/src/test/fuzz/node_eviction.cpp +++ b/src/test/fuzz/node_eviction.cpp @@ -26,7 +26,7 @@ FUZZ_TARGET(node_eviction) /*m_last_block_time=*/std::chrono::seconds{fuzzed_data_provider.ConsumeIntegral<int64_t>()}, /*m_last_tx_time=*/std::chrono::seconds{fuzzed_data_provider.ConsumeIntegral<int64_t>()}, /*fRelevantServices=*/fuzzed_data_provider.ConsumeBool(), - /*fRelayTxes=*/fuzzed_data_provider.ConsumeBool(), + /*m_relay_txs=*/fuzzed_data_provider.ConsumeBool(), /*fBloomFilter=*/fuzzed_data_provider.ConsumeBool(), /*nKeyedNetGroup=*/fuzzed_data_provider.ConsumeIntegral<uint64_t>(), /*prefer_evict=*/fuzzed_data_provider.ConsumeBool(), diff --git a/src/test/fuzz/script_format.cpp b/src/test/fuzz/script_format.cpp index 241bdfe666..9186746bcf 100644 --- a/src/test/fuzz/script_format.cpp +++ b/src/test/fuzz/script_format.cpp @@ -29,7 +29,5 @@ FUZZ_TARGET_INIT(script_format, initialize_script_format) (void)ScriptToAsmStr(script, /*fAttemptSighashDecode=*/fuzzed_data_provider.ConsumeBool()); UniValue o1(UniValue::VOBJ); - ScriptPubKeyToUniv(script, o1, /*include_hex=*/fuzzed_data_provider.ConsumeBool()); - UniValue o3(UniValue::VOBJ); - ScriptToUniv(script, o3); + ScriptToUniv(script, /*out=*/o1, /*include_hex=*/fuzzed_data_provider.ConsumeBool(), /*include_address=*/fuzzed_data_provider.ConsumeBool()); } diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index 6dd8a36692..273aa0dc5c 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -102,6 +102,6 @@ FUZZ_TARGET_INIT(transaction, initialize_transaction) (void)IsWitnessStandard(tx, coins_view_cache); UniValue u(UniValue::VOBJ); - TxToUniv(tx, /*hashBlock=*/uint256::ZERO, u); - TxToUniv(tx, /*hashBlock=*/uint256::ONE, u); + TxToUniv(tx, /*block_hash=*/uint256::ZERO, /*entry=*/u); + TxToUniv(tx, /*block_hash=*/uint256::ONE, /*entry=*/u); } diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index f0c1b0d147..6766fbf2d9 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -225,7 +225,7 @@ void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, const ServiceFlags remote_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS); const NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS); const int32_t version = fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max()); - const bool filter_txs = fuzzed_data_provider.ConsumeBool(); + const bool relay_txs{fuzzed_data_provider.ConsumeBool()}; const CNetMsgMaker mm{0}; @@ -241,7 +241,7 @@ void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, uint64_t{1}, // dummy nonce std::string{}, // dummy subver int32_t{}, // dummy starting_height - filter_txs), + relay_txs), }; (void)connman.ReceiveMsgFrom(node, msg_version); @@ -255,10 +255,9 @@ void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, assert(node.nVersion == version); assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION)); assert(node.nServices == remote_services); - if (node.m_tx_relay != nullptr) { - LOCK(node.m_tx_relay->cs_filter); - assert(node.m_tx_relay->fRelayTxes == filter_txs); - } + CNodeStateStats statestats; + assert(peerman.GetNodeStateStats(node.GetId(), statestats)); + assert(statestats.m_relay_txs == (relay_txs && !node.IsBlockOnlyConn())); node.m_permissionFlags = permission_flags; if (successfully_connected) { CSerializedNetMsg msg_verack{mm.Make(NetMsgType::VERACK)}; diff --git a/src/test/net_peer_eviction_tests.cpp b/src/test/net_peer_eviction_tests.cpp index 6ec3fb0c6b..e5ce936519 100644 --- a/src/test/net_peer_eviction_tests.cpp +++ b/src/test/net_peer_eviction_tests.cpp @@ -627,7 +627,7 @@ BOOST_AUTO_TEST_CASE(peer_eviction_test) number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { candidate.m_last_block_time = std::chrono::seconds{number_of_nodes - candidate.id}; if (candidate.id <= 7) { - candidate.fRelayTxes = false; + candidate.m_relay_txs = false; candidate.fRelevantServices = true; } }, @@ -646,7 +646,7 @@ BOOST_AUTO_TEST_CASE(peer_eviction_test) number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) { candidate.m_last_block_time = std::chrono::seconds{number_of_nodes - candidate.id}; if (candidate.id <= 7) { - candidate.fRelayTxes = false; + candidate.m_relay_txs = false; candidate.fRelevantServices = true; } }, diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index fcb1a80765..e7c01bd6d0 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -732,26 +732,31 @@ BOOST_AUTO_TEST_CASE(LimitedAndReachable_Network) BOOST_CHECK(IsReachable(NET_IPV6)); BOOST_CHECK(IsReachable(NET_ONION)); BOOST_CHECK(IsReachable(NET_I2P)); + BOOST_CHECK(IsReachable(NET_CJDNS)); SetReachable(NET_IPV4, false); SetReachable(NET_IPV6, false); SetReachable(NET_ONION, false); SetReachable(NET_I2P, false); + SetReachable(NET_CJDNS, false); BOOST_CHECK(!IsReachable(NET_IPV4)); BOOST_CHECK(!IsReachable(NET_IPV6)); BOOST_CHECK(!IsReachable(NET_ONION)); BOOST_CHECK(!IsReachable(NET_I2P)); + BOOST_CHECK(!IsReachable(NET_CJDNS)); SetReachable(NET_IPV4, true); SetReachable(NET_IPV6, true); SetReachable(NET_ONION, true); SetReachable(NET_I2P, true); + SetReachable(NET_CJDNS, true); BOOST_CHECK(IsReachable(NET_IPV4)); BOOST_CHECK(IsReachable(NET_IPV6)); BOOST_CHECK(IsReachable(NET_ONION)); BOOST_CHECK(IsReachable(NET_I2P)); + BOOST_CHECK(IsReachable(NET_CJDNS)); } BOOST_AUTO_TEST_CASE(LimitedAndReachable_NetworkCaseUnroutableAndInternal) diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 9c6950f11f..3f5353b5a2 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -12,7 +12,16 @@ // For details see https://github.com/bitcoin/bitcoin/pull/22348. #define __kernel_entry #endif +#if defined(__GNUC__) +// Boost 1.78 requires the following workaround. +// See: https://github.com/boostorg/process/issues/235 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wnarrowing" +#endif #include <boost/process.hpp> +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif #endif // ENABLE_EXTERNAL_SIGNER #include <boost/test/unit_test.hpp> diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 6013080dc6..55919b7f95 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -73,19 +73,19 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) CKey parent_key; parent_key.MakeNewKey(true); CScript parent_locking_script = GetScriptForDestination(PKHash(parent_key.GetPubKey())); - auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/ m_coinbase_txns[0], /*input_vout=*/0, - /*input_height=*/ 0, /*input_signing_key=*/coinbaseKey, - /*output_destination=*/ parent_locking_script, - /*output_amount=*/ CAmount(49 * COIN), /*submit=*/false); + auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[0], /*input_vout=*/0, + /*input_height=*/0, /*input_signing_key=*/coinbaseKey, + /*output_destination=*/parent_locking_script, + /*output_amount=*/CAmount(49 * COIN), /*submit=*/false); CTransactionRef tx_parent = MakeTransactionRef(mtx_parent); CKey child_key; child_key.MakeNewKey(true); CScript child_locking_script = GetScriptForDestination(PKHash(child_key.GetPubKey())); - auto mtx_child = CreateValidMempoolTransaction(/*input_transaction=*/ tx_parent, /*input_vout=*/0, - /*input_height=*/ 101, /*input_signing_key=*/parent_key, + auto mtx_child = CreateValidMempoolTransaction(/*input_transaction=*/tx_parent, /*input_vout=*/0, + /*input_height=*/101, /*input_signing_key=*/parent_key, /*output_destination=*/child_locking_script, - /*output_amount=*/ CAmount(48 * COIN), /*submit=*/false); + /*output_amount=*/CAmount(48 * COIN), /*submit=*/false); CTransactionRef tx_child = MakeTransactionRef(mtx_child); const auto result_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {tx_parent, tx_child}, /*test_accept=*/true); BOOST_CHECK_MESSAGE(result_parent_child.m_state.IsValid(), @@ -128,11 +128,11 @@ BOOST_FIXTURE_TEST_CASE(noncontextual_package_tests, TestChain100Setup) // Parent and Child Package { auto mtx_parent = CreateValidMempoolTransaction(m_coinbase_txns[0], 0, 0, coinbaseKey, spk, - CAmount(49 * COIN), /* submit */ false); + CAmount(49 * COIN), /*submit=*/false); CTransactionRef tx_parent = MakeTransactionRef(mtx_parent); auto mtx_child = CreateValidMempoolTransaction(tx_parent, 0, 101, placeholder_key, spk2, - CAmount(48 * COIN), /* submit */ false); + CAmount(48 * COIN), /*submit=*/false); CTransactionRef tx_child = MakeTransactionRef(mtx_child); PackageValidationState state; @@ -218,14 +218,14 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) // Unrelated transactions are not allowed in package submission. Package package_unrelated; for (size_t i{0}; i < 10; ++i) { - auto mtx = CreateValidMempoolTransaction(/* input_transaction */ m_coinbase_txns[i + 25], /* vout */ 0, - /* input_height */ 0, /* input_signing_key */ coinbaseKey, - /* output_destination */ parent_locking_script, - /* output_amount */ CAmount(49 * COIN), /* submit */ false); + auto mtx = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[i + 25], /*input_vout=*/0, + /*input_height=*/0, /*input_signing_key=*/coinbaseKey, + /*output_destination=*/parent_locking_script, + /*output_amount=*/CAmount(49 * COIN), /*submit=*/false); package_unrelated.emplace_back(MakeTransactionRef(mtx)); } auto result_unrelated_submit = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - package_unrelated, /* test_accept */ false); + package_unrelated, /*test_accept=*/false); BOOST_CHECK(result_unrelated_submit.m_state.IsInvalid()); BOOST_CHECK_EQUAL(result_unrelated_submit.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); BOOST_CHECK_EQUAL(result_unrelated_submit.m_state.GetRejectReason(), "package-not-child-with-parents"); @@ -234,10 +234,10 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) // Parent and Child (and Grandchild) Package Package package_parent_child; Package package_3gen; - auto mtx_parent = CreateValidMempoolTransaction(/* input_transaction */ m_coinbase_txns[0], /* vout */ 0, - /* input_height */ 0, /* input_signing_key */ coinbaseKey, - /* output_destination */ parent_locking_script, - /* output_amount */ CAmount(49 * COIN), /* submit */ false); + auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[0], /*input_vout=*/0, + /*input_height=*/0, /*input_signing_key=*/coinbaseKey, + /*output_destination=*/parent_locking_script, + /*output_amount=*/CAmount(49 * COIN), /*submit=*/false); CTransactionRef tx_parent = MakeTransactionRef(mtx_parent); package_parent_child.push_back(tx_parent); package_3gen.push_back(tx_parent); @@ -245,10 +245,10 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) CKey child_key; child_key.MakeNewKey(true); CScript child_locking_script = GetScriptForDestination(PKHash(child_key.GetPubKey())); - auto mtx_child = CreateValidMempoolTransaction(/* input_transaction */ tx_parent, /* vout */ 0, - /* input_height */ 101, /* input_signing_key */ parent_key, - /* output_destination */ child_locking_script, - /* output_amount */ CAmount(48 * COIN), /* submit */ false); + auto mtx_child = CreateValidMempoolTransaction(/*input_transaction=*/tx_parent, /*input_vout=*/0, + /*input_height=*/101, /*input_signing_key=*/parent_key, + /*output_destination=*/child_locking_script, + /*output_amount=*/CAmount(48 * COIN), /*submit=*/false); CTransactionRef tx_child = MakeTransactionRef(mtx_child); package_parent_child.push_back(tx_child); package_3gen.push_back(tx_child); @@ -256,17 +256,17 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) CKey grandchild_key; grandchild_key.MakeNewKey(true); CScript grandchild_locking_script = GetScriptForDestination(PKHash(grandchild_key.GetPubKey())); - auto mtx_grandchild = CreateValidMempoolTransaction(/* input_transaction */ tx_child, /* vout */ 0, - /* input_height */ 101, /* input_signing_key */ child_key, - /* output_destination */ grandchild_locking_script, - /* output_amount */ CAmount(47 * COIN), /* submit */ false); + auto mtx_grandchild = CreateValidMempoolTransaction(/*input_transaction=*/tx_child, /*input_vout=*/0, + /*input_height=*/101, /*input_signing_key=*/child_key, + /*output_destination=*/grandchild_locking_script, + /*output_amount=*/CAmount(47 * COIN), /*submit=*/false); CTransactionRef tx_grandchild = MakeTransactionRef(mtx_grandchild); package_3gen.push_back(tx_grandchild); // 3 Generations is not allowed. { auto result_3gen_submit = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - package_3gen, /* test_accept */ false); + package_3gen, /*test_accept=*/false); BOOST_CHECK(result_3gen_submit.m_state.IsInvalid()); BOOST_CHECK_EQUAL(result_3gen_submit.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); BOOST_CHECK_EQUAL(result_3gen_submit.m_state.GetRejectReason(), "package-not-child-with-parents"); @@ -280,7 +280,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) package_missing_parent.push_back(MakeTransactionRef(mtx_child)); { const auto result_missing_parent = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - package_missing_parent, /* test_accept */ false); + package_missing_parent, /*test_accept=*/false); BOOST_CHECK(result_missing_parent.m_state.IsInvalid()); BOOST_CHECK_EQUAL(result_missing_parent.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); BOOST_CHECK_EQUAL(result_missing_parent.m_state.GetRejectReason(), "package-not-child-with-unconfirmed-parents"); @@ -291,7 +291,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) // Submit package with parent + child. { const auto submit_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - package_parent_child, /* test_accept */ false); + package_parent_child, /*test_accept=*/false); expected_pool_size += 2; BOOST_CHECK_MESSAGE(submit_parent_child.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_parent_child.m_state.GetRejectReason()); @@ -310,7 +310,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) // Already-in-mempool transactions should be detected and de-duplicated. { const auto submit_deduped = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - package_parent_child, /* test_accept */ false); + package_parent_child, /*test_accept=*/false); BOOST_CHECK_MESSAGE(submit_deduped.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_deduped.m_state.GetRejectReason()); auto it_parent_deduped = submit_deduped.m_tx_results.find(tx_parent->GetWitnessHash()); @@ -340,10 +340,10 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) // and the mempool entry's wtxid returned. CScript witnessScript = CScript() << OP_DROP << OP_TRUE; CScript scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(witnessScript)); - auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/ m_coinbase_txns[0], /*vout=*/ 0, - /*input_height=*/ 0, /*input_signing_key=*/ coinbaseKey, - /*output_destination=*/ scriptPubKey, - /*output_amount=*/ CAmount(49 * COIN), /*submit=*/ false); + auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[0], /*input_vout=*/0, + /*input_height=*/0, /*input_signing_key=*/coinbaseKey, + /*output_destination=*/scriptPubKey, + /*output_amount=*/CAmount(49 * COIN), /*submit=*/false); CTransactionRef ptx_parent = MakeTransactionRef(mtx_parent); // Make two children with the same txid but different witnesses. @@ -384,7 +384,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) // same-txid-different-witness. { const auto submit_witness1 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - {ptx_parent, ptx_child1}, /*test_accept=*/ false); + {ptx_parent, ptx_child1}, /*test_accept=*/false); BOOST_CHECK_MESSAGE(submit_witness1.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_witness1.m_state.GetRejectReason()); auto it_parent1 = submit_witness1.m_tx_results.find(ptx_parent->GetWitnessHash()); @@ -400,7 +400,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_child1->GetHash()))); const auto submit_witness2 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - {ptx_parent, ptx_child2}, /*test_accept=*/ false); + {ptx_parent, ptx_child2}, /*test_accept=*/false); BOOST_CHECK_MESSAGE(submit_witness2.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_witness2.m_state.GetRejectReason()); auto it_parent2_deduped = submit_witness2.m_tx_results.find(ptx_parent->GetWitnessHash()); @@ -417,7 +417,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) // Deduplication should work when wtxid != txid. Submit package with the already-in-mempool // transactions again, which should not fail. const auto submit_segwit_dedup = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - {ptx_parent, ptx_child1}, /*test_accept=*/ false); + {ptx_parent, ptx_child1}, /*test_accept=*/false); BOOST_CHECK_MESSAGE(submit_segwit_dedup.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_segwit_dedup.m_state.GetRejectReason()); auto it_parent_dup = submit_segwit_dedup.m_tx_results.find(ptx_parent->GetWitnessHash()); @@ -436,16 +436,16 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) CKey grandchild_key; grandchild_key.MakeNewKey(true); CScript grandchild_locking_script = GetScriptForDestination(WitnessV0KeyHash(grandchild_key.GetPubKey())); - auto mtx_grandchild = CreateValidMempoolTransaction(/*input_transaction=*/ ptx_child2, /* vout=*/ 0, - /*input_height=*/ 0, /*input_signing_key=*/ child_key, - /*output_destination=*/ grandchild_locking_script, - /*output_amount=*/ CAmount(47 * COIN), /*submit=*/ false); + auto mtx_grandchild = CreateValidMempoolTransaction(/*input_transaction=*/ptx_child2, /*input_vout=*/0, + /*input_height=*/0, /*input_signing_key=*/child_key, + /*output_destination=*/grandchild_locking_script, + /*output_amount=*/CAmount(47 * COIN), /*submit=*/false); CTransactionRef ptx_grandchild = MakeTransactionRef(mtx_grandchild); // We already submitted child1 above. { const auto submit_spend_ignored = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - {ptx_child2, ptx_grandchild}, /*test_accept=*/ false); + {ptx_child2, ptx_grandchild}, /*test_accept=*/false); BOOST_CHECK_MESSAGE(submit_spend_ignored.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_spend_ignored.m_state.GetRejectReason()); auto it_child2_ignored = submit_spend_ignored.m_tx_results.find(ptx_child2->GetWitnessHash()); @@ -471,10 +471,10 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) acs_witness.stack.push_back(std::vector<unsigned char>(acs_script.begin(), acs_script.end())); // parent1 will already be in the mempool - auto mtx_parent1 = CreateValidMempoolTransaction(/*input_transaction=*/ m_coinbase_txns[1], /*vout=*/ 0, - /*input_height=*/ 0, /*input_signing_key=*/ coinbaseKey, - /*output_destination=*/ acs_spk, - /*output_amount=*/ CAmount(49 * COIN), /*submit=*/ true); + auto mtx_parent1 = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[1], /*input_vout=*/0, + /*input_height=*/0, /*input_signing_key=*/coinbaseKey, + /*output_destination=*/acs_spk, + /*output_amount=*/CAmount(49 * COIN), /*submit=*/true); CTransactionRef ptx_parent1 = MakeTransactionRef(mtx_parent1); package_mixed.push_back(ptx_parent1); @@ -489,10 +489,10 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) parent2_witness2.stack.push_back(std::vector<unsigned char>(grandparent2_script.begin(), grandparent2_script.end())); // Create grandparent2 creating an output with multiple spending paths. Submit to mempool. - auto mtx_grandparent2 = CreateValidMempoolTransaction(/*input_transaction=*/ m_coinbase_txns[2], /* vout=*/ 0, - /*input_height=*/ 0, /*input_signing_key=*/ coinbaseKey, - /*output_destination=*/ grandparent2_spk, - /*output_amount=*/ CAmount(49 * COIN), /*submit=*/ true); + auto mtx_grandparent2 = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[2], /*input_vout=*/0, + /*input_height=*/0, /*input_signing_key=*/coinbaseKey, + /*output_destination=*/grandparent2_spk, + /*output_amount=*/CAmount(49 * COIN), /*submit=*/true); CTransactionRef ptx_grandparent2 = MakeTransactionRef(mtx_grandparent2); CMutableTransaction mtx_parent2_v1; @@ -517,10 +517,10 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) package_mixed.push_back(ptx_parent2_v1); // parent3 will be a new transaction - auto mtx_parent3 = CreateValidMempoolTransaction(/*input_transaction=*/ m_coinbase_txns[3], /*vout=*/ 0, - /*input_height=*/ 0, /*input_signing_key=*/ coinbaseKey, - /*output_destination=*/ acs_spk, - /*output_amount=*/ CAmount(49 * COIN), /*submit=*/ false); + auto mtx_parent3 = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[3], /*input_vout=*/0, + /*input_height=*/0, /*input_signing_key=*/coinbaseKey, + /*output_destination=*/acs_spk, + /*output_amount=*/CAmount(49 * COIN), /*submit=*/false); CTransactionRef ptx_parent3 = MakeTransactionRef(mtx_parent3); package_mixed.push_back(ptx_parent3); diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index fe3cf52974..62b770753a 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -52,7 +52,7 @@ std::vector<NodeEvictionCandidate> GetRandomNodeEvictionCandidates(int n_candida /*m_last_block_time=*/std::chrono::seconds{random_context.randrange(100)}, /*m_last_tx_time=*/std::chrono::seconds{random_context.randrange(100)}, /*fRelevantServices=*/random_context.randbool(), - /*fRelayTxes=*/random_context.randbool(), + /*m_relay_txs=*/random_context.randbool(), /*fBloomFilter=*/random_context.randbool(), /*nKeyedNetGroup=*/random_context.randrange(100), /*prefer_evict=*/random_context.randbool(), diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 1881573e7a..b5d8411e1d 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -78,6 +78,31 @@ BOOST_AUTO_TEST_CASE(util_datadir) BOOST_CHECK_EQUAL(dd_norm, args.GetDataDirBase()); } +namespace { +class NoCopyOrMove +{ +public: + int i; + explicit NoCopyOrMove(int i) : i{i} { } + + NoCopyOrMove() = delete; + NoCopyOrMove(const NoCopyOrMove&) = delete; + NoCopyOrMove(NoCopyOrMove&&) = delete; + NoCopyOrMove& operator=(const NoCopyOrMove&) = delete; + NoCopyOrMove& operator=(NoCopyOrMove&&) = delete; + + operator bool() const { return i != 0; } + + int get_ip1() { return i + 1; } + bool test() + { + // Check that Assume can be used within a lambda and still call methods + [&]() { Assume(get_ip1()); }(); + return Assume(get_ip1() != 5); + } +}; +} // namespace + BOOST_AUTO_TEST_CASE(util_check) { // Check that Assert can forward @@ -89,6 +114,14 @@ BOOST_AUTO_TEST_CASE(util_check) // Check that Assume can be used as unary expression const bool result{Assume(two == 2)}; Assert(result); + + // Check that Assert doesn't require copy/move + NoCopyOrMove x{9}; + Assert(x).i += 3; + Assert(x).test(); + + // Check nested Asserts + BOOST_CHECK_EQUAL(Assert((Assert(x).test() ? 3 : 0)), 3); } BOOST_AUTO_TEST_CASE(util_criticalsection) diff --git a/src/txdb.cpp b/src/txdb.cpp index 9a39e90ccd..0dafa3b38a 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -76,7 +76,7 @@ void CCoinsViewDB::ResizeCache(size_t new_cache_size) // filesystem lock. m_db.reset(); m_db = std::make_unique<CDBWrapper>( - m_ldb_path, new_cache_size, m_is_memory, /*fWipe*/ false, /*obfuscate*/ true); + m_ldb_path, new_cache_size, m_is_memory, /*fWipe=*/false, /*obfuscate=*/true); } } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 33918cceab..a480eb038d 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -328,7 +328,7 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, staged_ancestors = it->GetMemPoolParentsConst(); } - return CalculateAncestorsAndCheckLimits(entry.GetTxSize(), /* entry_count */ 1, + return CalculateAncestorsAndCheckLimits(entry.GetTxSize(), /*entry_count=*/1, setAncestors, staged_ancestors, limitAncestorCount, limitAncestorSize, limitDescendantCount, limitDescendantSize, errString); diff --git a/src/util/check.cpp b/src/util/check.cpp new file mode 100644 index 0000000000..2a9f885560 --- /dev/null +++ b/src/util/check.cpp @@ -0,0 +1,14 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <util/check.h> + +#include <tinyformat.h> + +void assertion_fail(const char* file, int line, const char* func, const char* assertion) +{ + auto str = strprintf("%s:%s %s: Assertion `%s' failed.\n", file, line, func, assertion); + fwrite(str.data(), 1, str.size(), stderr); + std::abort(); +} diff --git a/src/util/check.h b/src/util/check.h index a443c13cf2..4ee65c8d34 100644 --- a/src/util/check.h +++ b/src/util/check.h @@ -47,14 +47,26 @@ class NonFatalCheckError : public std::runtime_error #endif /** Helper for Assert() */ -template <typename T> -T get_pure_r_value(T&& val) +void assertion_fail(const char* file, int line, const char* func, const char* assertion); + +/** Helper for Assert()/Assume() */ +template <bool IS_ASSERT, typename T> +T&& inline_assertion_check(T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion) { + if constexpr (IS_ASSERT +#ifdef ABORT_ON_FAILED_ASSUME + || true +#endif + ) { + if (!val) { + assertion_fail(file, line, func, assertion); + } + } return std::forward<T>(val); } /** Identity function. Abort if the value compares equal to zero */ -#define Assert(val) ([&]() -> decltype(get_pure_r_value(val)) { auto&& check = (val); assert(#val && check); return std::forward<decltype(get_pure_r_value(val))>(check); }()) +#define Assert(val) inline_assertion_check<true>(val, __FILE__, __LINE__, __func__, #val) /** * Assume is the identity function. @@ -66,10 +78,6 @@ T get_pure_r_value(T&& val) * - For non-fatal errors in interactive sessions (e.g. RPC or command line * interfaces), CHECK_NONFATAL() might be more appropriate. */ -#ifdef ABORT_ON_FAILED_ASSUME -#define Assume(val) Assert(val) -#else -#define Assume(val) ([&]() -> decltype(get_pure_r_value(val)) { auto&& check = (val); return std::forward<decltype(get_pure_r_value(val))>(check); }()) -#endif +#define Assume(val) inline_assertion_check<false>(val, __FILE__, __LINE__, __func__, #val) #endif // BITCOIN_UTIL_CHECK_H diff --git a/src/util/syscall_sandbox.cpp b/src/util/syscall_sandbox.cpp index f2a9cf664d..a05efac602 100644 --- a/src/util/syscall_sandbox.cpp +++ b/src/util/syscall_sandbox.cpp @@ -592,6 +592,8 @@ public: allowed_syscalls.insert(__NR_getcwd); // get current working directory allowed_syscalls.insert(__NR_getdents); // get directory entries allowed_syscalls.insert(__NR_getdents64); // get directory entries + allowed_syscalls.insert(__NR_inotify_rm_watch);// remove an existing watch from an inotify instance + allowed_syscalls.insert(__NR_linkat); // create relative to a directory file descriptor allowed_syscalls.insert(__NR_lstat); // get file status allowed_syscalls.insert(__NR_mkdir); // create a directory allowed_syscalls.insert(__NR_newfstatat); // get file status diff --git a/src/util/system.cpp b/src/util/system.cpp index 8e45453d31..a7e66defcd 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -6,7 +6,16 @@ #include <util/system.h> #ifdef ENABLE_EXTERNAL_SIGNER +#if defined(__GNUC__) +// Boost 1.78 requires the following workaround. +// See: https://github.com/boostorg/process/issues/235 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wnarrowing" +#endif #include <boost/process.hpp> +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif #endif // ENABLE_EXTERNAL_SIGNER #include <chainparamsbase.h> diff --git a/src/util/threadnames.cpp b/src/util/threadnames.cpp index 764fffabd7..a5a86d2598 100644 --- a/src/util/threadnames.cpp +++ b/src/util/threadnames.cpp @@ -6,7 +6,9 @@ #include <config/bitcoin-config.h> #endif +#include <string> #include <thread> +#include <utility> #if (defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__DragonFly__)) #include <pthread.h> @@ -16,7 +18,7 @@ #include <util/threadnames.h> #ifdef HAVE_SYS_PRCTL_H -#include <sys/prctl.h> // For prctl, PR_SET_NAME, PR_GET_NAME +#include <sys/prctl.h> #endif //! Set the thread's name at the process level. Does not affect the diff --git a/src/validation.cpp b/src/validation.cpp index f399acb9fd..6db13f1f70 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -255,7 +255,7 @@ bool CheckSequenceLocksAtTip(CBlockIndex* tip, } // Returns the script flags which should be checked for a given block -static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& chainparams); +static unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const Consensus::Params& chainparams); static void LimitMempoolSize(CTxMemPool& pool, CCoinsViewCache& coins_cache, size_t limit, std::chrono::seconds age) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, pool.cs) @@ -1013,7 +1013,7 @@ bool MemPoolAccept::ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws) // There is a similar check in CreateNewBlock() to prevent creating // invalid blocks (using TestBlockValidity), however allowing such // transactions into the mempool can be exploited as a DoS attack. - unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(m_active_chainstate.m_chain.Tip(), chainparams.GetConsensus()); + unsigned int currentBlockScriptVerifyFlags{GetBlockScriptFlags(*m_active_chainstate.m_chain.Tip(), chainparams.GetConsensus())}; if (!CheckInputsFromMempoolAndCache(tx, state, m_view, m_pool, currentBlockScriptVerifyFlags, ws.m_precomputed_txdata, m_active_chainstate.CoinsTip())) { LogPrintf("BUG! PLEASE REPORT THIS! CheckInputScripts failed against latest-block but not STANDARD flags %s, %s\n", hash.ToString(), state.ToString()); @@ -1854,45 +1854,39 @@ public: static ThresholdConditionCache warningcache[VERSIONBITS_NUM_BITS] GUARDED_BY(cs_main); -static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& consensusparams) +static unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const Consensus::Params& consensusparams) { - unsigned int flags = SCRIPT_VERIFY_NONE; - // BIP16 didn't become active until Apr 1 2012 (on mainnet, and // retroactively applied to testnet) // However, only one historical block violated the P2SH rules (on both - // mainnet and testnet), so for simplicity, always leave P2SH - // on except for the one violating block. - if (consensusparams.BIP16Exception.IsNull() || // no bip16 exception on this chain - pindex->phashBlock == nullptr || // this is a new candidate block, eg from TestBlockValidity() - *pindex->phashBlock != consensusparams.BIP16Exception) // this block isn't the historical exception - { - // Enforce WITNESS rules whenever P2SH is in effect - flags |= SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS; + // mainnet and testnet). + // Similarly, only one historical block violated the TAPROOT rules on + // mainnet. + // For simplicity, always leave P2SH+WITNESS+TAPROOT on except for the two + // violating blocks. + uint32_t flags{SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_TAPROOT}; + const auto it{consensusparams.script_flag_exceptions.find(*Assert(block_index.phashBlock))}; + if (it != consensusparams.script_flag_exceptions.end()) { + flags = it->second; } // Enforce the DERSIG (BIP66) rule - if (DeploymentActiveAt(*pindex, consensusparams, Consensus::DEPLOYMENT_DERSIG)) { + if (DeploymentActiveAt(block_index, consensusparams, Consensus::DEPLOYMENT_DERSIG)) { flags |= SCRIPT_VERIFY_DERSIG; } // Enforce CHECKLOCKTIMEVERIFY (BIP65) - if (DeploymentActiveAt(*pindex, consensusparams, Consensus::DEPLOYMENT_CLTV)) { + if (DeploymentActiveAt(block_index, consensusparams, Consensus::DEPLOYMENT_CLTV)) { flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY; } // Enforce CHECKSEQUENCEVERIFY (BIP112) - if (DeploymentActiveAt(*pindex, consensusparams, Consensus::DEPLOYMENT_CSV)) { + if (DeploymentActiveAt(block_index, consensusparams, Consensus::DEPLOYMENT_CSV)) { flags |= SCRIPT_VERIFY_CHECKSEQUENCEVERIFY; } - // Enforce Taproot (BIP340-BIP342) - if (DeploymentActiveAt(*pindex, consensusparams, Consensus::DEPLOYMENT_TAPROOT)) { - flags |= SCRIPT_VERIFY_TAPROOT; - } - // Enforce BIP147 NULLDUMMY (activated simultaneously with segwit) - if (DeploymentActiveAt(*pindex, consensusparams, Consensus::DEPLOYMENT_SEGWIT)) { + if (DeploymentActiveAt(block_index, consensusparams, Consensus::DEPLOYMENT_SEGWIT)) { flags |= SCRIPT_VERIFY_NULLDUMMY; } @@ -1900,7 +1894,6 @@ static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consens } - static int64_t nTimeCheck = 0; static int64_t nTimeForks = 0; static int64_t nTimeVerify = 0; @@ -2088,7 +2081,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, } // Get the script flags for this block - unsigned int flags = GetBlockScriptFlags(pindex, m_params.GetConsensus()); + unsigned int flags{GetBlockScriptFlags(*pindex, m_params.GetConsensus())}; int64_t nTime2 = GetTimeMicros(); nTimeForks += nTime2 - nTime1; LogPrint(BCLog::BENCH, " - Fork checks: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime2 - nTime1), nTimeForks * MICRO, nTimeForks * MILLI / nBlocksTotal); diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 2df66ac15b..433759e086 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -50,8 +50,8 @@ struct { * The Branch and Bound algorithm is described in detail in Murch's Master Thesis: * https://murch.one/wp-content/uploads/2016/11/erhardt2016coinselection.pdf * - * @param const std::vector<CInputCoin>& utxo_pool The set of UTXOs that we are choosing from. - * These UTXOs will be sorted in descending order by effective value and the CInputCoins' + * @param const std::vector<OutputGroup>& utxo_pool The set of UTXO groups that we are choosing from. + * These UTXO groups will be sorted in descending order by effective value and the OutputGroups' * values are their effective values. * @param const CAmount& selection_target This is the value that we want to select. It is the lower * bound of the range. @@ -187,11 +187,24 @@ std::optional<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& ut return std::nullopt; } -static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::vector<OutputGroup>& groups, const CAmount& nTotalLower, const CAmount& nTargetValue, +/** Find a subset of the OutputGroups that is at least as large as, but as close as possible to, the + * target amount; solve subset sum. + * param@[in] groups OutputGroups to choose from, sorted by value in descending order. + * param@[in] nTotalLower Total (effective) value of the UTXOs in groups. + * param@[in] nTargetValue Subset sum target, not including change. + * param@[out] vfBest Boolean vector representing the subset chosen that is closest to + * nTargetValue, with indices corresponding to groups. If the ith + * entry is true, that means the ith group in groups was selected. + * param@[out] nBest Total amount of subset chosen that is closest to nTargetValue. + * param@[in] iterations Maximum number of tries. + */ +static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::vector<OutputGroup>& groups, + const CAmount& nTotalLower, const CAmount& nTargetValue, std::vector<char>& vfBest, CAmount& nBest, int iterations = 1000) { std::vector<char> vfIncluded; + // Worst case "best" approximation is just all of the groups. vfBest.assign(groups.size(), true); nBest = nTotalLower; @@ -217,6 +230,8 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v if (nTotal >= nTargetValue) { fReachedTarget = true; + // If the total is between nTargetValue and nBest, it's our new best + // approximation. if (nTotal < nBest) { nBest = nTotal; @@ -231,12 +246,15 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v } } -std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue, FastRandomContext& rng) +std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue, + CAmount change_target, FastRandomContext& rng) { SelectionResult result(nTargetValue); // List of values less than target std::optional<OutputGroup> lowest_larger; + // Groups with selection amount smaller than the target and any change we might produce. + // Don't include groups larger than this, because they will only cause us to overshoot. std::vector<OutputGroup> applicable_groups; CAmount nTotalLower = 0; @@ -246,7 +264,7 @@ std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, if (group.GetSelectionAmount() == nTargetValue) { result.AddInput(group); return result; - } else if (group.GetSelectionAmount() < nTargetValue + MIN_CHANGE) { + } else if (group.GetSelectionAmount() < nTargetValue + change_target) { applicable_groups.push_back(group); nTotalLower += group.GetSelectionAmount(); } else if (!lowest_larger || group.GetSelectionAmount() < lowest_larger->GetSelectionAmount()) { @@ -273,14 +291,14 @@ std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, CAmount nBest; ApproximateBestSubset(rng, applicable_groups, nTotalLower, nTargetValue, vfBest, nBest); - if (nBest != nTargetValue && nTotalLower >= nTargetValue + MIN_CHANGE) { - ApproximateBestSubset(rng, applicable_groups, nTotalLower, nTargetValue + MIN_CHANGE, vfBest, nBest); + if (nBest != nTargetValue && nTotalLower >= nTargetValue + change_target) { + ApproximateBestSubset(rng, applicable_groups, nTotalLower, nTargetValue + change_target, vfBest, nBest); } // If we have a bigger coin and (either the stochastic approximation didn't find a good solution, // or the next bigger coin is closer), return the bigger coin if (lowest_larger && - ((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || lowest_larger->GetSelectionAmount() <= nBest)) { + ((nBest != nTargetValue && nBest < nTargetValue + change_target) || lowest_larger->GetSelectionAmount() <= nBest)) { result.AddInput(*lowest_larger); } else { for (unsigned int i = 0; i < applicable_groups.size(); i++) { @@ -309,29 +327,29 @@ std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, ******************************************************************************/ -void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants, bool positive_only) { +void OutputGroup::Insert(const COutput& output, 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 coin_fee = output.input_bytes < 0 ? 0 : m_effective_feerate.GetFee(output.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(); + COutput& coin = m_outputs.back(); - coin.m_fee = coin_fee; - fee += coin.m_fee; + coin.fee = coin_fee; + fee += coin.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.long_term_fee = coin.input_bytes < 0 ? 0 : m_long_term_feerate.GetFee(coin.input_bytes); + long_term_fee += coin.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); + m_from_me &= coin.from_me; + m_value += coin.txout.nValue; + m_depth = std::min(m_depth, coin.depth); // ancestors here express the number of ancestors the new coin will end up having, which is // the sum, rather than the max; this will overestimate in the cases where multiple inputs // have common ancestors @@ -353,7 +371,7 @@ CAmount OutputGroup::GetSelectionAmount() const return m_subtract_fee_outputs ? m_value : effective_value; } -CAmount GetSelectionWaste(const std::set<CInputCoin>& inputs, CAmount change_cost, CAmount target, bool use_effective_value) +CAmount GetSelectionWaste(const std::set<COutput>& inputs, CAmount change_cost, CAmount target, bool use_effective_value) { // This function should not be called with empty inputs as that would mean the selection failed assert(!inputs.empty()); @@ -361,8 +379,8 @@ CAmount GetSelectionWaste(const std::set<CInputCoin>& inputs, CAmount change_cos // Always consider the cost of spending an input now vs in the future. CAmount waste = 0; CAmount selected_effective_value = 0; - for (const CInputCoin& coin : inputs) { - waste += coin.m_fee - coin.m_long_term_fee; + for (const COutput& coin : inputs) { + waste += coin.fee - coin.long_term_fee; selected_effective_value += use_effective_value ? coin.effective_value : coin.txout.nValue; } @@ -380,6 +398,17 @@ CAmount GetSelectionWaste(const std::set<CInputCoin>& inputs, CAmount change_cos return waste; } +CAmount GenerateChangeTarget(CAmount payment_value, FastRandomContext& rng) +{ + if (payment_value <= CHANGE_LOWER / 2) { + return CHANGE_LOWER; + } else { + // random value between 50ksat and min (payment_value * 2, 1milsat) + const auto upper_bound = std::min(payment_value * 2, CHANGE_UPPER); + return rng.randrange(upper_bound - CHANGE_LOWER) + CHANGE_LOWER; + } +} + void SelectionResult::ComputeAndSetWaste(CAmount change_cost) { m_waste = GetSelectionWaste(m_selected_inputs, change_cost, m_target, m_use_effective); @@ -407,14 +436,14 @@ void SelectionResult::AddInput(const OutputGroup& group) m_use_effective = !group.m_subtract_fee_outputs; } -const std::set<CInputCoin>& SelectionResult::GetInputSet() const +const std::set<COutput>& SelectionResult::GetInputSet() const { return m_selected_inputs; } -std::vector<CInputCoin> SelectionResult::GetShuffledInputVector() const +std::vector<COutput> SelectionResult::GetShuffledInputVector() const { - std::vector<CInputCoin> coins(m_selected_inputs.begin(), m_selected_inputs.end()); + std::vector<COutput> coins(m_selected_inputs.begin(), m_selected_inputs.end()); Shuffle(coins.begin(), coins.end(), FastRandomContext()); return coins; } @@ -426,4 +455,9 @@ bool SelectionResult::operator<(SelectionResult other) const // As this operator is only used in std::min_element, we want the result that has more inputs when waste are equal. return *m_waste < *other.m_waste || (*m_waste == *other.m_waste && m_selected_inputs.size() > other.m_selected_inputs.size()); } + +std::string COutput::ToString() const +{ + return strprintf("COutput(%s, %d, %d) [%s]", outpoint.hash.ToString(), outpoint.n, depth, FormatMoney(txout.nValue)); +} } // namespace wallet diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index a0941d32df..784a91e827 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -13,62 +13,75 @@ #include <optional> namespace wallet { -//! target minimum change amount -static constexpr CAmount MIN_CHANGE{COIN / 100}; -//! final minimum change amount after paying for fees -static const CAmount MIN_FINAL_CHANGE = MIN_CHANGE/2; +//! lower bound for randomly-chosen target change amount +static constexpr CAmount CHANGE_LOWER{50000}; +//! upper bound for randomly-chosen target change amount +static constexpr CAmount CHANGE_UPPER{1000000}; /** A UTXO under consideration for use in funding a new transaction. */ -class CInputCoin { -public: - CInputCoin(const CTransactionRef& tx, unsigned int i) - { - if (!tx) - throw std::invalid_argument("tx should not be null"); - if (i >= tx->vout.size()) - throw std::out_of_range("The output index is out of range"); - - outpoint = COutPoint(tx->GetHash(), i); - txout = tx->vout[i]; - effective_value = txout.nValue; - } +struct COutput { + /** The outpoint identifying this UTXO */ + COutPoint outpoint; - CInputCoin(const CTransactionRef& tx, unsigned int i, int input_bytes) : CInputCoin(tx, i) - { - m_input_bytes = input_bytes; - } + /** The output itself */ + CTxOut txout; - CInputCoin(const COutPoint& outpoint_in, const CTxOut& txout_in) - { - outpoint = outpoint_in; - txout = txout_in; - effective_value = txout.nValue; - } + /** + * Depth in block chain. + * If > 0: the tx is on chain and has this many confirmations. + * If = 0: the tx is waiting confirmation. + * If < 0: a conflicting tx is on chain and has this many confirmations. */ + int depth; - CInputCoin(const COutPoint& outpoint_in, const CTxOut& txout_in, int input_bytes) : CInputCoin(outpoint_in, txout_in) - { - m_input_bytes = input_bytes; - } + /** Pre-computed estimated size of this output as a fully-signed input in a transaction. Can be -1 if it could not be calculated */ + int input_bytes; - COutPoint outpoint; - CTxOut txout; + /** Whether we have the private keys to spend this output */ + bool spendable; + + /** Whether we know how to spend this output, ignoring the lack of keys */ + bool solvable; + + /** + * Whether this output is considered safe to spend. Unconfirmed transactions + * from outside keys and unconfirmed replacement transactions are considered + * unsafe and will not be used to fund new spending transactions. + */ + bool safe; + + /** The time of the transaction containing this output as determined by CWalletTx::nTimeSmart */ + int64_t time; + + /** Whether the transaction containing this output is sent from the owning wallet */ + bool from_me; + + /** The output's value minus fees required to spend it. Initialized as the output's absolute value. */ CAmount effective_value; - CAmount m_fee{0}; - CAmount m_long_term_fee{0}; - /** Pre-computed estimated size of this output as a fully-signed input in a transaction. Can be -1 if it could not be calculated */ - int m_input_bytes{-1}; + /** The fee required to spend this output at the transaction's target feerate. */ + CAmount fee{0}; - bool operator<(const CInputCoin& rhs) const { - return outpoint < rhs.outpoint; - } + /** The fee required to spend this output at the consolidation feerate. */ + CAmount long_term_fee{0}; - bool operator!=(const CInputCoin& rhs) const { - return outpoint != rhs.outpoint; - } + COutput(const COutPoint& outpoint, const CTxOut& txout, int depth, int input_bytes, bool spendable, bool solvable, bool safe, int64_t time, bool from_me) + : outpoint{outpoint}, + txout{txout}, + depth{depth}, + input_bytes{input_bytes}, + spendable{spendable}, + solvable{solvable}, + safe{safe}, + time{time}, + from_me{from_me}, + effective_value{txout.nValue} + {} - bool operator==(const CInputCoin& rhs) const { - return outpoint == rhs.outpoint; + std::string ToString() const; + + bool operator<(const COutput& rhs) const + { + return outpoint < rhs.outpoint; } }; @@ -80,8 +93,13 @@ struct CoinSelectionParams { size_t change_output_size = 0; /** Size of the input to spend a change output in virtual bytes. */ size_t change_spend_size = 0; + /** Mininmum change to target in Knapsack solver: select coins to cover the payment and + * at least this value of change. */ + CAmount m_min_change_target{0}; /** Cost of creating the change output. */ CAmount m_change_fee{0}; + /** The pre-determined minimum value to target when funding a change output. */ + CAmount m_change_target{0}; /** Cost of creating the change output + cost of spending the change output in the future. */ CAmount m_cost_of_change{0}; /** The targeted feerate of the transaction being built. */ @@ -101,11 +119,13 @@ struct CoinSelectionParams { * reuse. Dust outputs are not eligible to be added to output groups and thus not considered. */ bool m_avoid_partial_spends = false; - CoinSelectionParams(FastRandomContext& rng_fast, size_t change_output_size, size_t change_spend_size, CFeeRate effective_feerate, + CoinSelectionParams(FastRandomContext& rng_fast, size_t change_output_size, size_t change_spend_size, + CAmount min_change_target, CFeeRate effective_feerate, CFeeRate long_term_feerate, CFeeRate discard_feerate, size_t tx_noinputs_size, bool avoid_partial) : rng_fast{rng_fast}, change_output_size(change_output_size), change_spend_size(change_spend_size), + m_min_change_target(min_change_target), m_effective_feerate(effective_feerate), m_long_term_feerate(long_term_feerate), m_discard_feerate(discard_feerate), @@ -143,7 +163,7 @@ struct CoinEligibilityFilter struct OutputGroup { /** The list of UTXOs contained in this output group. */ - std::vector<CInputCoin> m_outputs; + std::vector<COutput> m_outputs; /** Whether the UTXOs were sent by the wallet to itself. This is relevant because we may want at * least a certain number of confirmations on UTXOs received from outside wallets while trusting * our own UTXOs more. */ @@ -180,7 +200,7 @@ struct OutputGroup m_subtract_fee_outputs(params.m_subtract_fee_outputs) {} - void Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants, bool positive_only); + void Insert(const COutput& output, size_t ancestors, size_t descendants, bool positive_only); bool EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const; CAmount GetSelectionAmount() const; }; @@ -202,13 +222,28 @@ struct OutputGroup * @param[in] use_effective_value Whether to use the input's effective value (when true) or the real value (when false). * @return The waste */ -[[nodiscard]] CAmount GetSelectionWaste(const std::set<CInputCoin>& inputs, CAmount change_cost, CAmount target, bool use_effective_value = true); +[[nodiscard]] CAmount GetSelectionWaste(const std::set<COutput>& inputs, CAmount change_cost, CAmount target, bool use_effective_value = true); + + +/** Chooose a random change target for each transaction to make it harder to fingerprint the Core + * wallet based on the change output values of transactions it creates. + * The random value is between 50ksat and min(2 * payment_value, 1milsat) + * When payment_value <= 25ksat, the value is just 50ksat. + * + * Making change amounts similar to the payment value may help disguise which output(s) are payments + * are which ones are change. Using double the payment value may increase the number of inputs + * needed (and thus be more expensive in fees), but breaks analysis techniques which assume the + * coins selected are just sufficient to cover the payment amount ("unnecessary input" heuristic). + * + * @param[in] payment_value Average payment value of the transaction output(s). + */ +[[nodiscard]] CAmount GenerateChangeTarget(CAmount payment_value, FastRandomContext& rng); struct SelectionResult { private: /** Set of inputs selected by the algorithm to use in the transaction */ - std::set<CInputCoin> m_selected_inputs; + std::set<COutput> m_selected_inputs; /** The target the algorithm selected for. Note that this may not be equal to the recipient amount as it can include non-input fees */ const CAmount m_target; /** Whether the input values for calculations should be the effective value (true) or normal value (false) */ @@ -234,9 +269,9 @@ public: [[nodiscard]] CAmount GetWaste() const; /** Get m_selected_inputs */ - const std::set<CInputCoin>& GetInputSet() const; - /** Get the vector of CInputCoins that will be used to fill in a CTransaction's vin */ - std::vector<CInputCoin> GetShuffledInputVector() const; + const std::set<COutput>& GetInputSet() const; + /** Get the vector of COutputs that will be used to fill in a CTransaction's vin */ + std::vector<COutput> GetShuffledInputVector() const; bool operator<(SelectionResult other) const; }; @@ -253,7 +288,8 @@ std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_poo std::optional<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, FastRandomContext& rng); // Original coin selection algorithm as a fallback -std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue, FastRandomContext& rng); +std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue, + CAmount change_target, FastRandomContext& rng); } // namespace wallet #endif // BITCOIN_WALLET_COINSELECTION_H diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index bc53180c0e..73042424ad 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -135,7 +135,7 @@ static CFeeRate EstimateFeeRate(const CWallet& wallet, const CWalletTx& wtx, con feerate += std::max(node_incremental_relay_fee, wallet_incremental_relay_fee); // Fee rate must also be at least the wallet's GetMinimumFeeRate - CFeeRate min_feerate(GetMinimumFeeRate(wallet, coin_control, /* feeCalc */ nullptr)); + CFeeRate min_feerate(GetMinimumFeeRate(wallet, coin_control, /*feeCalc=*/nullptr)); // Set the required fee rate for the replacement transaction in coin control. return std::max(feerate, min_feerate); diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 1fb8085661..98e843385c 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -111,6 +111,17 @@ WalletTxOut MakeWalletTxOut(const CWallet& wallet, return result; } +WalletTxOut MakeWalletTxOut(const CWallet& wallet, + const COutput& output) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) +{ + WalletTxOut result; + result.txout = output.txout; + result.time = output.time; + result.depth_in_main_chain = output.depth; + result.is_spent = wallet.IsSpent(output.outpoint.hash, output.outpoint.n); + return result; +} + class WalletImpl : public Wallet { public: @@ -419,8 +430,8 @@ public: for (const auto& entry : ListCoins(*m_wallet)) { auto& group = result[entry.first]; for (const auto& coin : entry.second) { - group.emplace_back(COutPoint(coin.tx->GetHash(), coin.i), - MakeWalletTxOut(*m_wallet, *coin.tx, coin.i, coin.nDepth)); + group.emplace_back(coin.outpoint, + MakeWalletTxOut(*m_wallet, coin)); } } return result; diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 98ce95dcd1..c06513588b 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -130,6 +130,8 @@ bool LoadWallets(WalletContext& context) chain.initError(error); return false; } + + NotifyWalletLoaded(context, pwallet); AddWallet(context, pwallet); } return true; diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index 1a6f06213c..cddf94aab2 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -325,8 +325,8 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse) const CWalletTx& wtx = entry.second; const bool is_trusted{CachedTxIsTrusted(wallet, wtx, trusted_parents)}; const int tx_depth{wallet.GetTxDepthInMainChain(wtx)}; - const CAmount tx_credit_mine{CachedTxGetAvailableCredit(wallet, wtx, /* fUseCache */ true, ISMINE_SPENDABLE | reuse_filter)}; - const CAmount tx_credit_watchonly{CachedTxGetAvailableCredit(wallet, wtx, /* fUseCache */ true, ISMINE_WATCH_ONLY | reuse_filter)}; + const CAmount tx_credit_mine{CachedTxGetAvailableCredit(wallet, wtx, /*fUseCache=*/true, ISMINE_SPENDABLE | reuse_filter)}; + const CAmount tx_credit_watchonly{CachedTxGetAvailableCredit(wallet, wtx, /*fUseCache=*/true, ISMINE_WATCH_ONLY | reuse_filter)}; if (is_trusted && tx_depth >= min_depth) { ret.m_mine_trusted += tx_credit_mine; ret.m_watchonly_trusted += tx_credit_watchonly; diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 51587a64a3..d4f6c9d805 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -239,7 +239,7 @@ RPCHelpMan addmultisigaddress() {RPCResult::Type::STR, "address", "The value of the new multisig address"}, {RPCResult::Type::STR_HEX, "redeemScript", "The string value of the hex-encoded redemption script"}, {RPCResult::Type::STR, "descriptor", "The descriptor for this multisig"}, - {RPCResult::Type::ARR, "warnings", /* optional */ true, "Any warnings resulting from the creation of this multisig", + {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Any warnings resulting from the creation of this multisig", { {RPCResult::Type::STR, "", ""}, }}, @@ -597,7 +597,7 @@ RPCHelpMan getaddressinfo() DescriptorScriptPubKeyMan* desc_spk_man = dynamic_cast<DescriptorScriptPubKeyMan*>(spk_man); if (desc_spk_man) { std::string desc_str; - if (desc_spk_man->GetDescriptorString(desc_str, /* priv */ false)) { + if (desc_spk_man->GetDescriptorString(desc_str, /*priv=*/false)) { ret.pushKV("parent_desc", desc_str); } } diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 035541babd..4eccff3969 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -112,7 +112,7 @@ RPCHelpMan getreceivedbyaddress() LOCK(pwallet->cs_wallet); - return ValueFromAmount(GetReceived(*pwallet, request.params, /* by_label */ false)); + return ValueFromAmount(GetReceived(*pwallet, request.params, /*by_label=*/false)); }, }; } @@ -153,7 +153,7 @@ RPCHelpMan getreceivedbylabel() LOCK(pwallet->cs_wallet); - return ValueFromAmount(GetReceived(*pwallet, request.params, /* by_label */ true)); + return ValueFromAmount(GetReceived(*pwallet, request.params, /*by_label=*/true)); }, }; } @@ -648,16 +648,16 @@ RPCHelpMan listunspent() for (const COutput& out : vecOutputs) { CTxDestination address; - const CScript& scriptPubKey = out.tx->tx->vout[out.i].scriptPubKey; + const CScript& scriptPubKey = out.txout.scriptPubKey; bool fValidAddress = ExtractDestination(scriptPubKey, address); - bool reused = avoid_reuse && pwallet->IsSpentKey(out.tx->GetHash(), out.i); + bool reused = avoid_reuse && pwallet->IsSpentKey(out.outpoint.hash, out.outpoint.n); if (destinations.size() && (!fValidAddress || !destinations.count(address))) continue; UniValue entry(UniValue::VOBJ); - entry.pushKV("txid", out.tx->GetHash().GetHex()); - entry.pushKV("vout", out.i); + entry.pushKV("txid", out.outpoint.hash.GetHex()); + entry.pushKV("vout", (int)out.outpoint.n); if (fValidAddress) { entry.pushKV("address", EncodeDestination(address)); @@ -702,21 +702,21 @@ RPCHelpMan listunspent() } entry.pushKV("scriptPubKey", HexStr(scriptPubKey)); - entry.pushKV("amount", ValueFromAmount(out.tx->tx->vout[out.i].nValue)); - entry.pushKV("confirmations", out.nDepth); - if (!out.nDepth) { + entry.pushKV("amount", ValueFromAmount(out.txout.nValue)); + entry.pushKV("confirmations", out.depth); + if (!out.depth) { size_t ancestor_count, descendant_count, ancestor_size; CAmount ancestor_fees; - pwallet->chain().getTransactionAncestry(out.tx->GetHash(), ancestor_count, descendant_count, &ancestor_size, &ancestor_fees); + pwallet->chain().getTransactionAncestry(out.outpoint.hash, ancestor_count, descendant_count, &ancestor_size, &ancestor_fees); if (ancestor_count) { entry.pushKV("ancestorcount", uint64_t(ancestor_count)); entry.pushKV("ancestorsize", uint64_t(ancestor_size)); entry.pushKV("ancestorfees", uint64_t(ancestor_fees)); } } - entry.pushKV("spendable", out.fSpendable); - entry.pushKV("solvable", out.fSolvable); - if (out.fSolvable) { + entry.pushKV("spendable", out.spendable); + entry.pushKV("solvable", out.solvable); + if (out.solvable) { std::unique_ptr<SigningProvider> provider = pwallet->GetSolvingProvider(scriptPubKey); if (provider) { auto descriptor = InferDescriptor(scriptPubKey, *provider); @@ -724,7 +724,7 @@ RPCHelpMan listunspent() } } if (avoid_reuse) entry.pushKV("reused", reused); - entry.pushKV("safe", out.fSafe); + entry.pushKV("safe", out.safe); results.push_back(entry); } diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 072879a42a..07119133b7 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -9,10 +9,12 @@ #include <rpc/rawtransaction_util.h> #include <rpc/util.h> #include <util/fees.h> +#include <util/rbf.h> #include <util/translation.h> #include <util/vector.h> #include <wallet/coincontrol.h> #include <wallet/feebumper.h> +#include <wallet/fees.h> #include <wallet/rpc/util.h> #include <wallet/spend.h> #include <wallet/wallet.h> @@ -21,7 +23,8 @@ namespace wallet { -static void ParseRecipients(const UniValue& address_amounts, const UniValue& subtract_fee_outputs, std::vector<CRecipient> &recipients) { +static void ParseRecipients(const UniValue& address_amounts, const UniValue& subtract_fee_outputs, std::vector<CRecipient>& recipients) +{ std::set<CTxDestination> destinations; int i = 0; for (const std::string& address: address_amounts.getKeys()) { @@ -51,6 +54,93 @@ static void ParseRecipients(const UniValue& address_amounts, const UniValue& sub } } +static void InterpretFeeEstimationInstructions(const UniValue& conf_target, const UniValue& estimate_mode, const UniValue& fee_rate, UniValue& options) +{ + if (options.exists("conf_target") || options.exists("estimate_mode")) { + if (!conf_target.isNull() || !estimate_mode.isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Pass conf_target and estimate_mode either as arguments or in the options object, but not both"); + } + } else { + options.pushKV("conf_target", conf_target); + options.pushKV("estimate_mode", estimate_mode); + } + if (options.exists("fee_rate")) { + if (!fee_rate.isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Pass the fee_rate either as an argument, or in the options object, but not both"); + } + } else { + options.pushKV("fee_rate", fee_rate); + } + if (!options["conf_target"].isNull() && (options["estimate_mode"].isNull() || (options["estimate_mode"].get_str() == "unset"))) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Specify estimate_mode"); + } +} + +static UniValue FinishTransaction(const std::shared_ptr<CWallet> pwallet, const UniValue& options, const CMutableTransaction& rawTx) +{ + // Make a blank psbt + PartiallySignedTransaction psbtx(rawTx); + + // First fill transaction with our data without signing, + // so external signers are not asked sign more than once. + bool complete; + pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, false, true); + const TransactionError err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, true, false)}; + if (err != TransactionError::OK) { + throw JSONRPCTransactionError(err); + } + + CMutableTransaction mtx; + complete = FinalizeAndExtractPSBT(psbtx, mtx); + + UniValue result(UniValue::VOBJ); + + const bool psbt_opt_in{options.exists("psbt") && options["psbt"].get_bool()}; + bool add_to_wallet{options.exists("add_to_wallet") ? options["add_to_wallet"].get_bool() : true}; + if (psbt_opt_in || !complete || !add_to_wallet) { + // Serialize the PSBT + CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); + ssTx << psbtx; + result.pushKV("psbt", EncodeBase64(ssTx.str())); + } + + if (complete) { + std::string hex{EncodeHexTx(CTransaction(mtx))}; + CTransactionRef tx(MakeTransactionRef(std::move(mtx))); + result.pushKV("txid", tx->GetHash().GetHex()); + if (add_to_wallet && !psbt_opt_in) { + pwallet->CommitTransaction(tx, {}, /*orderForm=*/{}); + } else { + result.pushKV("hex", hex); + } + } + result.pushKV("complete", complete); + + return result; +} + +static void PreventOutdatedOptions(const UniValue& options) +{ + if (options.exists("feeRate")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Use fee_rate (" + CURRENCY_ATOM + "/vB) instead of feeRate"); + } + if (options.exists("changeAddress")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Use change_address instead of changeAddress"); + } + if (options.exists("changePosition")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Use change_position instead of changePosition"); + } + if (options.exists("includeWatching")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Use include_watching instead of includeWatching"); + } + if (options.exists("lockUnspents")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Use lock_unspents instead of lockUnspents"); + } + if (options.exists("subtractFeeFromOutputs")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Use subtract_fee_from_outputs instead of subtractFeeFromOutputs"); + } +} + UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vector<CRecipient> &recipients, mapValue_t map_value, bool verbose) { EnsureWalletIsUnlocked(wallet); @@ -108,7 +198,7 @@ static void SetFeeEstimateMode(const CWallet& wallet, CCoinControl& cc, const Un throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and fee_rate"); } // Fee rates in sat/vB cannot represent more than 3 significant digits. - cc.m_feerate = CFeeRate{AmountFromValue(fee_rate, /* decimals */ 3)}; + cc.m_feerate = CFeeRate{AmountFromValue(fee_rate, /*decimals=*/3)}; if (override_min_fee) cc.fOverrideFeeRate = true; // Default RBF to true for explicit fee_rate, if unset. if (!cc.m_signal_bip125_rbf) cc.m_signal_bip125_rbf = true; @@ -203,7 +293,7 @@ RPCHelpMan sendtoaddress() // We also enable partial spend avoidance if reuse avoidance is set. coin_control.m_avoid_partial_spends |= coin_control.m_avoid_address_reuse; - SetFeeEstimateMode(*pwallet, coin_control, /* conf_target */ request.params[6], /* estimate_mode */ request.params[7], /* fee_rate */ request.params[9], /* override_min_fee */ false); + SetFeeEstimateMode(*pwallet, coin_control, /*conf_target=*/request.params[6], /*estimate_mode=*/request.params[7], /*fee_rate=*/request.params[9], /*override_min_fee=*/false); EnsureWalletIsUnlocked(*pwallet); @@ -306,7 +396,7 @@ RPCHelpMan sendmany() coin_control.m_signal_bip125_rbf = request.params[5].get_bool(); } - SetFeeEstimateMode(*pwallet, coin_control, /* conf_target */ request.params[6], /* estimate_mode */ request.params[7], /* fee_rate */ request.params[8], /* override_min_fee */ false); + SetFeeEstimateMode(*pwallet, coin_control, /*conf_target=*/request.params[6], /*estimate_mode=*/request.params[7], /*fee_rate=*/request.params[8], /*override_min_fee=*/false); std::vector<CRecipient> recipients; ParseRecipients(sendTo, subtractFeeFromAmount, recipients); @@ -360,31 +450,43 @@ RPCHelpMan settxfee() // Only includes key documentation where the key is snake_case in all RPC methods. MixedCase keys can be added later. -static std::vector<RPCArg> FundTxDoc() +static std::vector<RPCArg> FundTxDoc(bool solving_data = true) { - return { + std::vector<RPCArg> args = { {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"}, {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, - {"replaceable", RPCArg::Type::BOOL, RPCArg::DefaultHint{"wallet default"}, "Marks this transaction as BIP125-replaceable.\n" - "Allows this transaction to be replaced by a transaction with higher fees"}, - {"solving_data", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "Keys and scripts needed for producing a final transaction with a dummy signature.\n" - "Used for fee estimation during coin selection.", - { - {"pubkeys", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Public keys involved in this transaction.", - { - {"pubkey", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A public key"}, - }}, - {"scripts", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Scripts involved in this transaction.", - { - {"script", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A script"}, - }}, - {"descriptors", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Descriptors that provide solving data for this transaction.", - { - {"descriptor", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A descriptor"}, - }}, - }}, + { + "replaceable", RPCArg::Type::BOOL, RPCArg::DefaultHint{"wallet default"}, "Marks this transaction as BIP125-replaceable.\n" + "Allows this transaction to be replaced by a transaction with higher fees" + }, }; + if (solving_data) { + args.push_back({"solving_data", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "Keys and scripts needed for producing a final transaction with a dummy signature.\n" + "Used for fee estimation during coin selection.", + { + { + "pubkeys", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Public keys involved in this transaction.", + { + {"pubkey", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A public key"}, + } + }, + { + "scripts", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Scripts involved in this transaction.", + { + {"script", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A script"}, + } + }, + { + "descriptors", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Descriptors that provide solving data for this transaction.", + { + {"descriptor", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A descriptor"}, + } + }, + } + }); + } + return args; } void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, const UniValue& options, CCoinControl& coinControl, bool override_min_fee) @@ -736,7 +838,7 @@ RPCHelpMan fundrawtransaction() CCoinControl coin_control; // Automatically select (additional) coins. Can be overridden by options.add_inputs. coin_control.m_add_inputs = true; - FundTransaction(*pwallet, tx, fee, change_position, request.params[1], coin_control, /* override_min_fee */ true); + FundTransaction(*pwallet, tx, fee, change_position, request.params[1], coin_control, /*override_min_fee=*/true); UniValue result(UniValue::VOBJ); result.pushKV("hex", EncodeHexTx(CTransaction(tx))); @@ -941,7 +1043,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) if (options.exists("replaceable")) { coin_control.m_signal_bip125_rbf = options["replaceable"].get_bool(); } - SetFeeEstimateMode(*pwallet, coin_control, conf_target, options["estimate_mode"], options["fee_rate"], /* override_min_fee */ false); + SetFeeEstimateMode(*pwallet, coin_control, conf_target, options["estimate_mode"], options["fee_rate"], /*override_min_fee=*/false); } // Make sure the results are valid at least up to the most recent block @@ -1126,102 +1228,249 @@ RPCHelpMan send() if (!pwallet) return NullUniValue; UniValue options{request.params[4].isNull() ? UniValue::VOBJ : request.params[4]}; - if (options.exists("conf_target") || options.exists("estimate_mode")) { - if (!request.params[1].isNull() || !request.params[2].isNull()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Pass conf_target and estimate_mode either as arguments or in the options object, but not both"); - } - } else { - options.pushKV("conf_target", request.params[1]); - options.pushKV("estimate_mode", request.params[2]); - } - if (options.exists("fee_rate")) { - if (!request.params[3].isNull()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Pass the fee_rate either as an argument, or in the options object, but not both"); - } - } else { - options.pushKV("fee_rate", request.params[3]); - } - if (!options["conf_target"].isNull() && (options["estimate_mode"].isNull() || (options["estimate_mode"].get_str() == "unset"))) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Specify estimate_mode"); - } - if (options.exists("feeRate")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Use fee_rate (" + CURRENCY_ATOM + "/vB) instead of feeRate"); - } - if (options.exists("changeAddress")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Use change_address"); - } - if (options.exists("changePosition")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Use change_position"); - } - if (options.exists("includeWatching")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Use include_watching"); - } - if (options.exists("lockUnspents")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Use lock_unspents"); - } - if (options.exists("subtractFeeFromOutputs")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Use subtract_fee_from_outputs"); - } + InterpretFeeEstimationInstructions(/*conf_target=*/request.params[1], /*estimate_mode=*/request.params[2], /*fee_rate=*/request.params[3], options); + PreventOutdatedOptions(options); - const bool psbt_opt_in = options.exists("psbt") && options["psbt"].get_bool(); CAmount fee; int change_position; - bool rbf = pwallet->m_signal_rbf; - if (options.exists("replaceable")) { - rbf = options["replaceable"].get_bool(); - } + bool rbf{options.exists("replaceable") ? options["replaceable"].get_bool() : pwallet->m_signal_rbf}; CMutableTransaction rawTx = ConstructTransaction(options["inputs"], request.params[0], options["locktime"], rbf); CCoinControl coin_control; // Automatically select coins, unless at least one is manually selected. Can // be overridden by options.add_inputs. coin_control.m_add_inputs = rawTx.vin.size() == 0; SetOptionsInputWeights(options["inputs"], options); - FundTransaction(*pwallet, rawTx, fee, change_position, options, coin_control, /* override_min_fee */ false); + FundTransaction(*pwallet, rawTx, fee, change_position, options, coin_control, /*override_min_fee=*/false); - bool add_to_wallet = true; - if (options.exists("add_to_wallet")) { - add_to_wallet = options["add_to_wallet"].get_bool(); + return FinishTransaction(pwallet, options, rawTx); + } + }; +} + +RPCHelpMan sendall() +{ + return RPCHelpMan{"sendall", + "EXPERIMENTAL warning: this call may be changed in future releases.\n" + "\nSpend the value of all (or specific) confirmed UTXOs in the wallet to one or more recipients.\n" + "Unconfirmed inbound UTXOs and locked UTXOs will not be spent. Sendall will respect the avoid_reuse wallet flag.\n" + "If your wallet contains many small inputs, either because it received tiny payments or as a result of accumulating change, consider using `send_max` to exclude inputs that are worth less than the fees needed to spend them.\n", + { + {"recipients", RPCArg::Type::ARR, RPCArg::Optional::NO, "The sendall destinations. Each address may only appear once.\n" + "Optionally some recipients can be specified with an amount to perform payments, but at least one address must appear without a specified amount.\n", + { + {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "A bitcoin address which receives an equal share of the unspecified amount."}, + {"", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "", + { + {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in " + CURRENCY_UNIT + ""}, + }, + }, + }, + }, + {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"}, + {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, std::string() + "The fee estimate mode, must be one of (case insensitive):\n" + " \"" + FeeModes("\"\n\"") + "\""}, + {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, + { + "options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", + Cat<std::vector<RPCArg>>( + { + {"add_to_wallet", RPCArg::Type::BOOL, RPCArg::Default{true}, "When false, returns the serialized transaction without broadcasting or adding it to the wallet"}, + {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, + {"include_watching", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also select inputs which are watch-only.\n" + "Only solvable inputs can be used. Watch-only destinations are solvable if the public key and/or output script was imported,\n" + "e.g. with 'importpubkey' or 'importmulti' with the 'pubkeys' or 'desc' field."}, + {"inputs", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Use exactly the specified inputs to build the transaction. Specifying inputs is incompatible with send_max. A JSON array of JSON objects", + { + {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"}, + {"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output number"}, + {"sequence", RPCArg::Type::NUM, RPCArg::Optional::NO, "The sequence number"}, + }, + }, + {"locktime", RPCArg::Type::NUM, RPCArg::Default{0}, "Raw locktime. Non-0 value also locktime-activates inputs"}, + {"lock_unspents", RPCArg::Type::BOOL, RPCArg::Default{false}, "Lock selected unspent outputs"}, + {"psbt", RPCArg::Type::BOOL, RPCArg::DefaultHint{"automatic"}, "Always return a PSBT, implies add_to_wallet=false."}, + {"send_max", RPCArg::Type::BOOL, RPCArg::Default{false}, "When true, only use UTXOs that can pay for their own fees to maximize the output amount. When 'false' (default), no UTXO is left behind. send_max is incompatible with providing specific inputs."}, + }, + FundTxDoc() + ), + "options" + }, + }, + RPCResult{ + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::BOOL, "complete", "If the transaction has a complete set of signatures"}, + {RPCResult::Type::STR_HEX, "txid", /*optional=*/true, "The transaction id for the send. Only 1 transaction is created regardless of the number of addresses."}, + {RPCResult::Type::STR_HEX, "hex", /*optional=*/true, "If add_to_wallet is false, the hex-encoded raw transaction with signature(s)"}, + {RPCResult::Type::STR, "psbt", /*optional=*/true, "If more signatures are needed, or if add_to_wallet is false, the base64-encoded (partially) signed transaction"} + } + }, + RPCExamples{"" + "\nSpend all UTXOs from the wallet with a fee rate of 1Â " + CURRENCY_ATOM + "/vB using named arguments\n" + + HelpExampleCli("-named sendall", "recipients='[\"" + EXAMPLE_ADDRESS[0] + "\"]' fee_rate=1\n") + + "Spend all UTXOs with a fee rate of 1.1 " + CURRENCY_ATOM + "/vB using positional arguments\n" + + HelpExampleCli("sendall", "'[\"" + EXAMPLE_ADDRESS[0] + "\"]' null \"unset\" 1.1\n") + + "Spend all UTXOs split into equal amounts to two addresses with a fee rate of 1.5 " + CURRENCY_ATOM + "/vB using the options argument\n" + + HelpExampleCli("sendall", "'[\"" + EXAMPLE_ADDRESS[0] + "\", \"" + EXAMPLE_ADDRESS[1] + "\"]' null \"unset\" null '{\"fee_rate\": 1.5}'\n") + + "Leave dust UTXOs in wallet, spend only UTXOs with positive effective value with a fee rate of 10 " + CURRENCY_ATOM + "/vB using the options argument\n" + + HelpExampleCli("sendall", "'[\"" + EXAMPLE_ADDRESS[0] + "\"]' null \"unset\" null '{\"fee_rate\": 10, \"send_max\": true}'\n") + + "Spend all UTXOs with a fee rate of 1.3 " + CURRENCY_ATOM + "/vB using named arguments and sending a 0.25 " + CURRENCY_UNIT + " to another recipient\n" + + HelpExampleCli("-named sendall", "recipients='[{\"" + EXAMPLE_ADDRESS[1] + "\": 0.25}, \""+ EXAMPLE_ADDRESS[0] + "\"]' fee_rate=1.3\n") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue + { + RPCTypeCheck(request.params, { + UniValue::VARR, // recipients + UniValue::VNUM, // conf_target + UniValue::VSTR, // estimate_mode + UniValueType(), // fee_rate, will be checked by AmountFromValue() in SetFeeEstimateMode() + UniValue::VOBJ, // options + }, true + ); + + std::shared_ptr<CWallet> const pwallet{GetWalletForJSONRPCRequest(request)}; + if (!pwallet) return NullUniValue; + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwallet->BlockUntilSyncedToCurrentChain(); + + UniValue options{request.params[4].isNull() ? UniValue::VOBJ : request.params[4]}; + InterpretFeeEstimationInstructions(/*conf_target=*/request.params[1], /*estimate_mode=*/request.params[2], /*fee_rate=*/request.params[3], options); + PreventOutdatedOptions(options); + + + std::set<std::string> addresses_without_amount; + UniValue recipient_key_value_pairs(UniValue::VARR); + const UniValue& recipients{request.params[0]}; + for (unsigned int i = 0; i < recipients.size(); ++i) { + const UniValue& recipient{recipients[i]}; + if (recipient.isStr()) { + UniValue rkvp(UniValue::VOBJ); + rkvp.pushKV(recipient.get_str(), 0); + recipient_key_value_pairs.push_back(rkvp); + addresses_without_amount.insert(recipient.get_str()); + } else { + recipient_key_value_pairs.push_back(recipient); + } + } + + if (addresses_without_amount.size() == 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Must provide at least one address without a specified amount"); } - // Make a blank psbt - PartiallySignedTransaction psbtx(rawTx); + CCoinControl coin_control; + + SetFeeEstimateMode(*pwallet, coin_control, options["conf_target"], options["estimate_mode"], options["fee_rate"], /*override_min_fee=*/false); - // First fill transaction with our data without signing, - // so external signers are not asked sign more than once. - bool complete; - pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, false, true); - const TransactionError err = pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, true, false); - if (err != TransactionError::OK) { - throw JSONRPCTransactionError(err); + coin_control.fAllowWatchOnly = ParseIncludeWatchonly(options["include_watching"], *pwallet); + + const bool rbf{options.exists("replaceable") ? options["replaceable"].get_bool() : pwallet->m_signal_rbf}; + + FeeCalculation fee_calc_out; + CFeeRate fee_rate{GetMinimumFeeRate(*pwallet, coin_control, &fee_calc_out)}; + // Do not, ever, assume that it's fine to change the fee rate if the user has explicitly + // provided one + if (coin_control.m_feerate && fee_rate > *coin_control.m_feerate) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Fee rate (%s) is lower than the minimum fee rate setting (%s)", coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), fee_rate.ToString(FeeEstimateMode::SAT_VB))); + } + if (fee_calc_out.reason == FeeReason::FALLBACK && !pwallet->m_allow_fallback_fee) { + // eventually allow a fallback fee + throw JSONRPCError(RPC_WALLET_ERROR, "Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee."); } - CMutableTransaction mtx; - complete = FinalizeAndExtractPSBT(psbtx, mtx); + CMutableTransaction rawTx{ConstructTransaction(options["inputs"], recipient_key_value_pairs, options["locktime"], rbf)}; + LOCK(pwallet->cs_wallet); + std::vector<COutput> all_the_utxos; + + CAmount total_input_value(0); + bool send_max{options.exists("send_max") ? options["send_max"].get_bool() : false}; + if (options.exists("inputs") && options.exists("send_max")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot combine send_max with specific inputs."); + } else if (options.exists("inputs")) { + for (const CTxIn& input : rawTx.vin) { + if (pwallet->IsSpent(input.prevout.hash, input.prevout.n)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Input not available. UTXO (%s:%d) was already spent.", input.prevout.hash.ToString(), input.prevout.n)); + } + const CWalletTx* tx{pwallet->GetWalletTx(input.prevout.hash)}; + if (!tx || pwallet->IsMine(tx->tx->vout[input.prevout.n]) != (coin_control.fAllowWatchOnly ? ISMINE_ALL : ISMINE_SPENDABLE)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Input not found. UTXO (%s:%d) is not part of wallet.", input.prevout.hash.ToString(), input.prevout.n)); + } + total_input_value += tx->tx->vout[input.prevout.n].nValue; + } + } else { + AvailableCoins(*pwallet, all_the_utxos, &coin_control, /*nMinimumAmount=*/0); + for (const COutput& output : all_the_utxos) { + CHECK_NONFATAL(output.input_bytes > 0); + if (send_max && fee_rate.GetFee(output.input_bytes) > output.txout.nValue) { + continue; + } + CTxIn input(output.outpoint.hash, output.outpoint.n, CScript(), rbf ? MAX_BIP125_RBF_SEQUENCE : CTxIn::SEQUENCE_FINAL); + rawTx.vin.push_back(input); + total_input_value += output.txout.nValue; + } + } - UniValue result(UniValue::VOBJ); + // estimate final size of tx + const TxSize tx_size{CalculateMaximumSignedTxSize(CTransaction(rawTx), pwallet.get())}; + const CAmount fee_from_size{fee_rate.GetFee(tx_size.vsize)}; + const CAmount effective_value{total_input_value - fee_from_size}; - if (psbt_opt_in || !complete || !add_to_wallet) { - // Serialize the PSBT - CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); - ssTx << psbtx; - result.pushKV("psbt", EncodeBase64(ssTx.str())); + if (effective_value <= 0) { + if (send_max) { + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Total value of UTXO pool too low to pay for transaction, try using lower feerate."); + } else { + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Total value of UTXO pool too low to pay for transaction. Try using lower feerate or excluding uneconomic UTXOs with 'send_max' option."); + } } - if (complete) { - std::string err_string; - std::string hex = EncodeHexTx(CTransaction(mtx)); - CTransactionRef tx(MakeTransactionRef(std::move(mtx))); - result.pushKV("txid", tx->GetHash().GetHex()); - if (add_to_wallet && !psbt_opt_in) { - pwallet->CommitTransaction(tx, {}, {} /* orderForm */); + CAmount output_amounts_claimed{0}; + for (CTxOut out : rawTx.vout) { + output_amounts_claimed += out.nValue; + } + + if (output_amounts_claimed > total_input_value) { + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Assigned more value to outputs than available funds."); + } + + const CAmount remainder{effective_value - output_amounts_claimed}; + if (remainder < 0) { + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Insufficient funds for fees after creating specified outputs."); + } + + const CAmount per_output_without_amount{remainder / (long)addresses_without_amount.size()}; + + bool gave_remaining_to_first{false}; + for (CTxOut& out : rawTx.vout) { + CTxDestination dest; + ExtractDestination(out.scriptPubKey, dest); + std::string addr{EncodeDestination(dest)}; + if (addresses_without_amount.count(addr) > 0) { + out.nValue = per_output_without_amount; + if (!gave_remaining_to_first) { + out.nValue += remainder % addresses_without_amount.size(); + gave_remaining_to_first = true; + } + if (IsDust(out, pwallet->chain().relayDustFee())) { + // Dynamically generated output amount is dust + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Dynamically assigned remainder results in dust output."); + } } else { - result.pushKV("hex", hex); + if (IsDust(out, pwallet->chain().relayDustFee())) { + // Specified output amount is dust + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Specified output amount to %s is below dust threshold.", addr)); + } + } + } + + const bool lock_unspents{options.exists("lock_unspents") ? options["lock_unspents"].get_bool() : false}; + if (lock_unspents) { + for (const CTxIn& txin : rawTx.vin) { + pwallet->LockCoin(txin.prevout); } } - result.pushKV("complete", complete); - return result; + return FinishTransaction(pwallet, options, rawTx); } }; } @@ -1418,7 +1667,7 @@ RPCHelpMan walletcreatefundedpsbt() // be overridden by options.add_inputs. coin_control.m_add_inputs = rawTx.vin.size() == 0; SetOptionsInputWeights(request.params[0], options); - FundTransaction(wallet, rawTx, fee, change_position, options, coin_control, /* override_min_fee */ true); + FundTransaction(wallet, rawTx, fee, change_position, options, coin_control, /*override_min_fee=*/true); // Make a blank psbt PartiallySignedTransaction psbtx(rawTx); diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index ad94ce4b32..c87af2ea30 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -800,7 +800,7 @@ RPCHelpMan gettransaction() if (verbose) { UniValue decoded(UniValue::VOBJ); - TxToUniv(*wtx.tx, uint256(), decoded, false); + TxToUniv(*wtx.tx, /*block_hash=*/uint256(), /*entry=*/decoded, /*include_hex=*/false); entry.pushKV("decoded", decoded); } diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 804331eb5d..4baf16fdcb 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -56,7 +56,7 @@ static RPCHelpMan getwalletinfo() { {RPCResult::Type::NUM, "duration", "elapsed seconds since scan start"}, {RPCResult::Type::NUM, "progress", "scanning progress percentage [0.0, 1.0]"}, - }}, + }, /*skip_type_check=*/true}, {RPCResult::Type::BOOL, "descriptors", "whether this wallet uses descriptors for scriptPubKey management"}, {RPCResult::Type::BOOL, "external_signer", "whether this wallet is configured to use an external signer such as a hardware wallet"}, }}, @@ -644,6 +644,7 @@ RPCHelpMan fundrawtransaction(); RPCHelpMan bumpfee(); RPCHelpMan psbtbumpfee(); RPCHelpMan send(); +RPCHelpMan sendall(); RPCHelpMan walletprocesspsbt(); RPCHelpMan walletcreatefundedpsbt(); RPCHelpMan signrawtransactionwithwallet(); @@ -723,6 +724,7 @@ static const CRPCCommand commands[] = { "wallet", &setwalletflag, }, { "wallet", &signmessage, }, { "wallet", &signrawtransactionwithwallet, }, + { "wallet", &sendall, }, { "wallet", &unloadwallet, }, { "wallet", &upgradewallet, }, { "wallet", &walletcreatefundedpsbt, }, diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 931baeac90..9e508f3a32 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -19,6 +19,8 @@ #include <wallet/transaction.h> #include <wallet/wallet.h> +#include <cmath> + using interfaces::FoundBlock; namespace wallet { @@ -29,11 +31,6 @@ int GetTxSpendSize(const CWallet& wallet, const CWalletTx& wtx, unsigned int out return CalculateMaximumSignedInputSize(wtx.tx->vout[out], &wallet, use_max_sig); } -std::string COutput::ToString() const -{ - return strprintf("COutput(%s, %d, %d) [%s]", tx->GetHash().ToString(), i, nDepth, FormatMoney(tx->tx->vout[i].nValue)); -} - int CalculateMaximumSignedInputSize(const CTxOut& txout, const SigningProvider* provider, bool use_max_sig) { CMutableTransaction txn; @@ -158,6 +155,8 @@ void AvailableCoins(const CWallet& wallet, std::vector<COutput>& vCoins, const C continue; } + bool tx_from_me = CachedTxIsFromMe(wallet, wtx, ISMINE_ALL); + for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) { // Only consider selected coins if add_inputs is false if (coinControl && !coinControl->m_add_inputs && !coinControl->IsSelected(COutPoint(entry.first, i))) { @@ -190,8 +189,9 @@ void AvailableCoins(const CWallet& wallet, std::vector<COutput>& vCoins, const C bool solvable = provider ? IsSolvable(*provider, wtx.tx->vout[i].scriptPubKey) : false; bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable)); + int input_bytes = GetTxSpendSize(wallet, wtx, i, (coinControl && coinControl->fAllowWatchOnly)); - vCoins.push_back(COutput(wallet, wtx, i, nDepth, spendable, solvable, safeTx, (coinControl && coinControl->fAllowWatchOnly))); + vCoins.emplace_back(COutPoint(wtx.GetHash(), i), wtx.tx->vout.at(i), nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me); // Checks the sum amount of all UTXO's. if (nMinimumSumAmount != MAX_MONEY) { @@ -218,8 +218,8 @@ CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinContr std::vector<COutput> vCoins; AvailableCoins(wallet, vCoins, coinControl); for (const COutput& out : vCoins) { - if (out.fSpendable) { - balance += out.tx->tx->vout[out.i].nValue; + if (out.spendable) { + balance += out.txout.nValue; } } return balance; @@ -243,6 +243,12 @@ const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const CTransactio return ptx->vout[n]; } +const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const COutPoint& outpoint) +{ + AssertLockHeld(wallet.cs_wallet); + return FindNonChangeParentOutput(wallet, *wallet.GetWalletTx(outpoint.hash)->tx, outpoint.n); +} + std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet) { AssertLockHeld(wallet.cs_wallet); @@ -254,8 +260,8 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet) for (const COutput& coin : availableCoins) { CTxDestination address; - if ((coin.fSpendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.fSolvable)) && - ExtractDestination(FindNonChangeParentOutput(wallet, *coin.tx->tx, coin.i).scriptPubKey, address)) { + if ((coin.spendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.solvable)) && + ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) { result[address].emplace_back(std::move(coin)); } } @@ -268,14 +274,15 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet) for (const COutPoint& output : lockedCoins) { auto it = wallet.mapWallet.find(output.hash); if (it != wallet.mapWallet.end()) { - int depth = wallet.GetTxDepthInMainChain(it->second); - if (depth >= 0 && output.n < it->second.tx->vout.size() && - wallet.IsMine(it->second.tx->vout[output.n]) == is_mine_filter + const auto& wtx = it->second; + int depth = wallet.GetTxDepthInMainChain(wtx); + if (depth >= 0 && output.n < wtx.tx->vout.size() && + wallet.IsMine(wtx.tx->vout[output.n]) == is_mine_filter ) { CTxDestination address; - if (ExtractDestination(FindNonChangeParentOutput(wallet, *it->second.tx, output.n).scriptPubKey, address)) { + if (ExtractDestination(FindNonChangeParentOutput(wallet, *wtx.tx, output.n).scriptPubKey, address)) { result[address].emplace_back( - wallet, it->second, output.n, depth, true /* spendable */, true /* solvable */, false /* safe */); + COutPoint(wtx.GetHash(), output.n), wtx.tx->vout.at(output.n), depth, GetTxSpendSize(wallet, wtx, output.n), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ false, wtx.GetTxTime(), CachedTxIsFromMe(wallet, wtx, ISMINE_ALL)); } } } @@ -292,15 +299,14 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C // Allowing partial spends means no grouping. Each COutput gets its own OutputGroup. for (const COutput& output : outputs) { // Skip outputs we cannot spend - if (!output.fSpendable) continue; + if (!output.spendable) continue; size_t ancestors, descendants; - wallet.chain().getTransactionAncestry(output.tx->GetHash(), ancestors, descendants); - CInputCoin input_coin = output.GetInputCoin(); + wallet.chain().getTransactionAncestry(output.outpoint.hash, ancestors, descendants); // Make an OutputGroup containing just this output OutputGroup group{coin_sel_params}; - group.Insert(input_coin, output.nDepth, CachedTxIsFromMe(wallet, *output.tx, ISMINE_ALL), ancestors, descendants, positive_only); + group.Insert(output, ancestors, descendants, positive_only); // Check the OutputGroup's eligibility. Only add the eligible ones. if (positive_only && group.GetSelectionAmount() <= 0) continue; @@ -312,18 +318,17 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C // 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 + // For each COutput, we check if the scriptPubKey is in the map, and if it is, the COutput 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. + // OUTPUT_GROUP_MAX_ENTRIES COutputs, 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; + if (!output.spendable) continue; size_t ancestors, descendants; - wallet.chain().getTransactionAncestry(output.tx->GetHash(), ancestors, descendants); - CInputCoin input_coin = output.GetInputCoin(); - CScript spk = input_coin.txout.scriptPubKey; + wallet.chain().getTransactionAncestry(output.outpoint.hash, ancestors, descendants); + CScript spk = output.txout.scriptPubKey; std::vector<OutputGroup>& groups = spk_to_groups_map[spk]; @@ -332,7 +337,7 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C groups.emplace_back(coin_sel_params); } - // Get the last OutputGroup in the vector so that we can add the CInputCoin to it + // Get the last OutputGroup in the vector so that we can add the COutput to it // A pointer is used here so that group can be reassigned later if it is full. OutputGroup* group = &groups.back(); @@ -344,8 +349,8 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C group = &groups.back(); } - // Add the input_coin to group - group->Insert(input_coin, output.nDepth, CachedTxIsFromMe(wallet, *output.tx, ISMINE_ALL), ancestors, descendants, positive_only); + // Add the output to group + group->Insert(output, ancestors, descendants, positive_only); } // Now we go through the entire map and pull out the OutputGroups @@ -386,14 +391,17 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm std::vector<OutputGroup> all_groups = GroupOutputs(wallet, coins, coin_selection_params, eligibility_filter, false /* positive_only */); // While nTargetValue includes the transaction fees for non-input things, it does not include the fee for creating a change output. // So we need to include that for KnapsackSolver as well, as we are expecting to create a change output. - if (auto knapsack_result{KnapsackSolver(all_groups, nTargetValue + coin_selection_params.m_change_fee, coin_selection_params.rng_fast)}) { + if (auto knapsack_result{KnapsackSolver(all_groups, nTargetValue + coin_selection_params.m_change_fee, + coin_selection_params.m_min_change_target, coin_selection_params.rng_fast)}) { knapsack_result->ComputeAndSetWaste(coin_selection_params.m_cost_of_change); results.push_back(*knapsack_result); } - // We include the minimum final change for SRD as we do want to avoid making really small change. - // KnapsackSolver does not need this because it includes MIN_CHANGE internally. - const CAmount srd_target = nTargetValue + coin_selection_params.m_change_fee + MIN_FINAL_CHANGE; + // Include change for SRD as we want to avoid making really small change if the selection just + // barely meets the target. Just use the lower bound change target instead of the randomly + // generated one, since SRD will result in a random change amount anyway; avoid making the + // target needlessly large. + const CAmount srd_target = nTargetValue + coin_selection_params.m_change_fee + CHANGE_LOWER; if (auto srd_result{SelectCoinsSRD(positive_groups, srd_target, coin_selection_params.rng_fast)}) { srd_result->ComputeAndSetWaste(coin_selection_params.m_cost_of_change); results.push_back(*srd_result); @@ -421,11 +429,11 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec if (coin_control.HasSelected() && !coin_control.fAllowOtherInputs) { for (const COutput& out : vCoins) { - if (!out.fSpendable) continue; - /* Set depth, from_me, ancestors, and descendants to 0 or false as these don't matter for preset inputs as no actual selection is being done. + if (!out.spendable) continue; + /* Set ancestors and descendants to 0 as these don't matter for preset inputs as no actual selection is being done. * positive_only is set to false because we want to include all preset inputs, even if they are dust. */ - preset_inputs.Insert(out.GetInputCoin(), 0, false, 0, 0, false); + preset_inputs.Insert(out, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ false); } SelectionResult result(nTargetValue); result.AddInput(preset_inputs); @@ -434,7 +442,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec } // calculate value from preset inputs and store them - std::set<CInputCoin> setPresetCoins; + std::set<COutPoint> preset_coins; std::vector<COutPoint> vPresetInputs; coin_control.ListSelected(vPresetInputs); @@ -455,34 +463,36 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec if (!coin_control.GetExternalOutput(outpoint, txout)) { return std::nullopt; } - input_bytes = CalculateMaximumSignedInputSize(txout, &coin_control.m_external_provider, /* use_max_sig */ true); + input_bytes = CalculateMaximumSignedInputSize(txout, &coin_control.m_external_provider, /*use_max_sig=*/true); } // If available, override calculated size with coin control specified size if (coin_control.HasInputWeight(outpoint)) { input_bytes = GetVirtualTransactionSize(coin_control.GetInputWeight(outpoint), 0, 0); } - CInputCoin coin(outpoint, txout, input_bytes); - if (coin.m_input_bytes == -1) { + if (input_bytes == -1) { return std::nullopt; // Not solvable, can't estimate size for fee } - coin.effective_value = coin.txout.nValue - coin_selection_params.m_effective_feerate.GetFee(coin.m_input_bytes); + + /* Set some defaults for depth, spendable, solvable, safe, time, and from_me as these don't matter for preset inputs since no selection is being done. */ + COutput output(outpoint, txout, /*depth=*/ 0, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false); + output.effective_value = output.txout.nValue - coin_selection_params.m_effective_feerate.GetFee(output.input_bytes); if (coin_selection_params.m_subtract_fee_outputs) { - value_to_select -= coin.txout.nValue; + value_to_select -= output.txout.nValue; } else { - value_to_select -= coin.effective_value; + value_to_select -= output.effective_value; } - setPresetCoins.insert(coin); - /* Set depth, from_me, ancestors, and descendants to 0 or false as don't matter for preset inputs as no actual selection is being done. + preset_coins.insert(outpoint); + /* Set ancestors and descendants to 0 as they don't matter for preset inputs since no actual selection is being done. * positive_only is set to false because we want to include all preset inputs, even if they are dust. */ - preset_inputs.Insert(coin, 0, false, 0, 0, false); + preset_inputs.Insert(output, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ false); } // remove preset inputs from vCoins so that Coin Selection doesn't pick them. for (std::vector<COutput>::iterator it = vCoins.begin(); it != vCoins.end() && coin_control.HasSelected();) { - if (setPresetCoins.count(it->GetInputCoin())) + if (preset_coins.count(it->outpoint)) it = vCoins.erase(it); else ++it; @@ -675,6 +685,7 @@ static bool CreateTransactionInternal( coin_selection_params.m_subtract_fee_outputs = true; } } + coin_selection_params.m_change_target = GenerateChangeTarget(std::floor(recipients_sum / vecSend.size()), rng_fast); // Create change script that will be used if we need change CScript scriptChange; @@ -772,7 +783,7 @@ static bool CreateTransactionInternal( AvailableCoins(wallet, vAvailableCoins, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0); // Choose coins to use - std::optional<SelectionResult> result = SelectCoins(wallet, vAvailableCoins, /* nTargetValue */ selection_target, coin_control, coin_selection_params); + std::optional<SelectionResult> result = SelectCoins(wallet, vAvailableCoins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params); if (!result) { error = _("Insufficient funds"); return false; @@ -798,7 +809,7 @@ static bool CreateTransactionInternal( auto change_position = txNew.vout.insert(txNew.vout.begin() + nChangePosInOut, newTxOut); // Shuffle selected coins and fill in final vin - std::vector<CInputCoin> selected_coins = result->GetShuffledInputVector(); + std::vector<COutput> selected_coins = result->GetShuffledInputVector(); // The sequence number is set to non-maxint so that DiscourageFeeSniping // works. diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 4453fb2762..e43aac5273 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -11,61 +11,11 @@ #include <wallet/wallet.h> namespace wallet { -/** Get the marginal bytes if spending the specified output from this transaction */ +/** Get the marginal bytes if spending the specified output from this transaction. + * use_max_sig indicates whether to use the maximum sized, 72 byte signature when calculating the + * size of the input spend. This should only be set when watch-only outputs are allowed */ int GetTxSpendSize(const CWallet& wallet, const CWalletTx& wtx, unsigned int out, bool use_max_sig = false); -class COutput -{ -public: - const CWalletTx *tx; - - /** Index in tx->vout. */ - int i; - - /** - * Depth in block chain. - * If > 0: the tx is on chain and has this many confirmations. - * If = 0: the tx is waiting confirmation. - * If < 0: a conflicting tx is on chain and has this many confirmations. */ - int nDepth; - - /** Pre-computed estimated size of this output as a fully-signed input in a transaction. Can be -1 if it could not be calculated */ - int nInputBytes; - - /** Whether we have the private keys to spend this output */ - bool fSpendable; - - /** Whether we know how to spend this output, ignoring the lack of keys */ - bool fSolvable; - - /** Whether to use the maximum sized, 72 byte signature when calculating the size of the input spend. This should only be set when watch-only outputs are allowed */ - bool use_max_sig; - - /** - * Whether this output is considered safe to spend. Unconfirmed transactions - * from outside keys and unconfirmed replacement transactions are considered - * unsafe and will not be used to fund new spending transactions. - */ - bool fSafe; - - COutput(const CWallet& wallet, const CWalletTx& wtx, int iIn, int nDepthIn, bool fSpendableIn, bool fSolvableIn, bool fSafeIn, bool use_max_sig_in = false) - { - tx = &wtx; i = iIn; nDepth = nDepthIn; fSpendable = fSpendableIn; fSolvable = fSolvableIn; fSafe = fSafeIn; nInputBytes = -1; use_max_sig = use_max_sig_in; - // If known and signable by the given wallet, compute nInputBytes - // Failure will keep this value -1 - if (fSpendable) { - nInputBytes = GetTxSpendSize(wallet, wtx, i, use_max_sig); - } - } - - std::string ToString() const; - - inline CInputCoin GetInputCoin() const - { - return CInputCoin(tx->tx, i, nInputBytes); - } -}; - //Get the marginal bytes of spending the specified output int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, bool use_max_sig = false); int CalculateMaximumSignedInputSize(const CTxOut& txout, const SigningProvider* pwallet, bool use_max_sig = false); @@ -93,6 +43,7 @@ CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinContr * Find non-change parent output. */ const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const CTransaction& tx, int output) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); +const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const COutPoint& outpoint) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); /** * Return list of available coins and locked coins grouped by non-change output address. diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index f0c799b43e..2a08c8ab57 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -28,20 +28,20 @@ BOOST_FIXTURE_TEST_SUITE(coinselector_tests, WalletTestingSetup) // we repeat those tests this many times and only complain if all iterations of the test fail #define RANDOM_REPEATS 5 -typedef std::set<CInputCoin> CoinSet; +typedef std::set<COutput> CoinSet; static const CoinEligibilityFilter filter_standard(1, 6, 0); static const CoinEligibilityFilter filter_confirmed(1, 1, 0); static const CoinEligibilityFilter filter_standard_extra(6, 6, 0); static int nextLockTime = 0; -static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set) +static void add_coin(const CAmount& nValue, int nInput, std::vector<COutput>& set) { CMutableTransaction tx; tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; tx.nLockTime = nextLockTime++; // so all transactions get different hashes - set.emplace_back(MakeTransactionRef(tx), nInput); + set.emplace_back(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false); } static void add_coin(const CAmount& nValue, int nInput, SelectionResult& result) @@ -50,9 +50,9 @@ static void add_coin(const CAmount& nValue, int nInput, SelectionResult& result) tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; tx.nLockTime = nextLockTime++; // so all transactions get different hashes - CInputCoin coin(MakeTransactionRef(tx), nInput); + COutput output(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false); OutputGroup group; - group.Insert(coin, 1, false, 0, 0, true); + group.Insert(output, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ true); result.AddInput(group); } @@ -62,10 +62,10 @@ static void add_coin(const CAmount& nValue, int nInput, CoinSet& set, CAmount fe tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; tx.nLockTime = nextLockTime++; // so all transactions get different hashes - CInputCoin coin(MakeTransactionRef(tx), nInput); + COutput coin(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false); coin.effective_value = nValue - fee; - coin.m_fee = fee; - coin.m_long_term_fee = long_term_fee; + coin.fee = fee; + coin.long_term_fee = long_term_fee; set.insert(coin); } @@ -82,24 +82,13 @@ static void add_coin(std::vector<COutput>& coins, CWallet& wallet, const CAmount assert(destination_ok); tx.vout[nInput].scriptPubKey = GetScriptForDestination(dest); } - if (fIsFromMe) { - // IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(), - // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() - tx.vin.resize(1); - } uint256 txid = tx.GetHash(); LOCK(wallet.cs_wallet); auto ret = wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(txid), std::forward_as_tuple(MakeTransactionRef(std::move(tx)), TxStateInactive{})); assert(ret.second); CWalletTx& wtx = (*ret.first).second; - if (fIsFromMe) - { - wtx.m_amounts[CWalletTx::DEBIT].Set(ISMINE_SPENDABLE, 1); - wtx.m_is_cache_empty = false; - } - COutput output(wallet, wtx, nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); - coins.push_back(output); + coins.emplace_back(COutPoint(wtx.GetHash(), nInput), wtx.tx->vout.at(nInput), nAge, GetTxSpendSize(wallet, wtx, nInput), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe); } /** Check if SelectionResult a is equivalent to SelectionResult b. @@ -124,11 +113,14 @@ static bool EquivalentResult(const SelectionResult& a, const SelectionResult& b) /** Check if this selection is equal to another one. Equal means same inputs (i.e same value and prevout) */ static bool EqualResult(const SelectionResult& a, const SelectionResult& b) { - std::pair<CoinSet::iterator, CoinSet::iterator> ret = std::mismatch(a.GetInputSet().begin(), a.GetInputSet().end(), b.GetInputSet().begin()); + std::pair<CoinSet::iterator, CoinSet::iterator> ret = std::mismatch(a.GetInputSet().begin(), a.GetInputSet().end(), b.GetInputSet().begin(), + [](const COutput& a, const COutput& b) { + return a.outpoint == b.outpoint; + }); return ret.first == a.GetInputSet().end() && ret.second == b.GetInputSet().end(); } -static CAmount make_hard_case(int utxos, std::vector<CInputCoin>& utxo_pool) +static CAmount make_hard_case(int utxos, std::vector<COutput>& utxo_pool) { utxo_pool.clear(); CAmount target = 0; @@ -140,24 +132,13 @@ static CAmount make_hard_case(int utxos, std::vector<CInputCoin>& utxo_pool) return target; } -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(); - static_groups.back().Insert(coin, 0, true, 0, 0, false); - } - return static_groups; -} - 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(); - 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); + static_groups.back().Insert(coin, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ false); } return static_groups; } @@ -167,13 +148,14 @@ inline std::vector<OutputGroup>& KnapsackGroupOutputs(const std::vector<COutput> FastRandomContext rand{}; CoinSelectionParams coin_selection_params{ rand, - /* change_output_size= */ 0, - /* change_spend_size= */ 0, - /* effective_feerate= */ CFeeRate(0), - /* long_term_feerate= */ CFeeRate(0), - /* discard_feerate= */ CFeeRate(0), - /* tx_noinputs_size= */ 0, - /* avoid_partial= */ false, + /*change_output_size=*/ 0, + /*change_spend_size=*/ 0, + /*min_change_target=*/ CENT, + /*effective_feerate=*/ CFeeRate(0), + /*long_term_feerate=*/ CFeeRate(0), + /*discard_feerate=*/ CFeeRate(0), + /*tx_noinputs_size=*/ 0, + /*avoid_partial=*/ false, }; static std::vector<OutputGroup> static_groups; static_groups = GroupOutputs(wallet, coins, coin_selection_params, filter, /*positive_only=*/false); @@ -185,7 +167,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) { FastRandomContext rand{}; // Setup - std::vector<CInputCoin> utxo_pool; + std::vector<COutput> utxo_pool; SelectionResult expected_result(CAmount(0)); ///////////////////////// @@ -311,13 +293,14 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) // Make sure that effective value is working in AttemptSelection when BnB is used CoinSelectionParams coin_selection_params_bnb{ rand, - /* change_output_size= */ 0, - /* change_spend_size= */ 0, - /* effective_feerate= */ CFeeRate(3000), - /* long_term_feerate= */ CFeeRate(1000), - /* discard_feerate= */ CFeeRate(1000), - /* tx_noinputs_size= */ 0, - /* avoid_partial= */ false, + /*change_output_size=*/ 0, + /*change_spend_size=*/ 0, + /*min_change_target=*/ 0, + /*effective_feerate=*/ CFeeRate(3000), + /*long_term_feerate=*/ CFeeRate(1000), + /*discard_feerate=*/ CFeeRate(1000), + /*tx_noinputs_size=*/ 0, + /*avoid_partial=*/ false, }; { std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); @@ -329,13 +312,13 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) std::vector<COutput> coins; add_coin(coins, *wallet, 1); - coins.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 + coins.at(0).input_bytes = 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(!SelectCoinsBnB(GroupCoins(coins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change)); // Test fees subtracted from output: coins.clear(); add_coin(coins, *wallet, 1 * CENT); - coins.at(0).nInputBytes = 40; + coins.at(0).input_bytes = 40; coin_selection_params_bnb.m_subtract_fee_outputs = true; const auto result9 = SelectCoinsBnB(GroupCoins(coins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change); BOOST_CHECK(result9); @@ -356,7 +339,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) add_coin(coins, *wallet, 2 * CENT, 6 * 24, false, 0, true); CCoinControl coin_control; coin_control.fAllowOtherInputs = true; - coin_control.Select(COutPoint(coins.at(0).tx->GetHash(), coins.at(0).i)); + coin_control.Select(coins.at(0).outpoint); coin_selection_params_bnb.m_effective_feerate = CFeeRate(0); const auto result10 = SelectCoins(*wallet, coins, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(result10); @@ -366,7 +349,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) BOOST_AUTO_TEST_CASE(knapsack_solver_test) { FastRandomContext rand{}; - const auto temp1{[&rand](std::vector<OutputGroup>& g, const CAmount& v) { return KnapsackSolver(g, v, rand); }}; + const auto temp1{[&rand](std::vector<OutputGroup>& g, const CAmount& v, CAmount c) { return KnapsackSolver(g, v, c, rand); }}; const auto KnapsackSolver{temp1}; std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); wallet->LoadWallet(); @@ -382,25 +365,25 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) coins.clear(); // with an empty wallet we can't even pay one cent - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 1 * CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 1 * CENT, CENT)); add_coin(coins, *wallet, 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(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 1 * CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 1 * CENT, CENT)); // but we can find a new 1 cent - const auto result1 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 1 * CENT); + const auto result1 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result1); BOOST_CHECK_EQUAL(result1->GetSelectedValue(), 1 * CENT); add_coin(coins, *wallet, 2*CENT); // add a mature 2 cent coin // we can't make 3 cents of mature coins - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 3 * CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 3 * CENT, CENT)); // we can make 3 cents of new coins - const auto result2 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 3 * CENT); + const auto result2 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 3 * CENT, CENT); BOOST_CHECK(result2); BOOST_CHECK_EQUAL(result2->GetSelectedValue(), 3 * CENT); @@ -411,38 +394,38 @@ 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(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 38 * CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 38 * CENT, CENT)); // we can't even make 37 cents if we don't allow new coins even if they're from us - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard_extra), 38 * CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard_extra), 38 * CENT, CENT)); // but we can make 37 cents if we accept new coins from ourself - const auto result3 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 37 * CENT); + const auto result3 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 37 * CENT, CENT); BOOST_CHECK(result3); BOOST_CHECK_EQUAL(result3->GetSelectedValue(), 37 * CENT); // and we can make 38 cents if we accept all new coins - const auto result4 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 38 * CENT); + const auto result4 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 38 * CENT, CENT); BOOST_CHECK(result4); BOOST_CHECK_EQUAL(result4->GetSelectedValue(), 38 * CENT); // try making 34 cents from 1,2,5,10,20 - we can't do it exactly - const auto result5 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 34 * CENT); + const auto result5 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 34 * CENT, CENT); BOOST_CHECK(result5); BOOST_CHECK_EQUAL(result5->GetSelectedValue(), 35 * CENT); // but 35 cents is closest BOOST_CHECK_EQUAL(result5->GetInputSet().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 - const auto result6 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 7 * CENT); + const auto result6 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 7 * CENT, CENT); BOOST_CHECK(result6); BOOST_CHECK_EQUAL(result6->GetSelectedValue(), 7 * CENT); BOOST_CHECK_EQUAL(result6->GetInputSet().size(), 2U); // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. - const auto result7 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 8 * CENT); + const auto result7 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 8 * CENT, CENT); BOOST_CHECK(result7); BOOST_CHECK(result7->GetSelectedValue() == 8 * CENT); BOOST_CHECK_EQUAL(result7->GetInputSet().size(), 3U); // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) - const auto result8 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 9 * CENT); + const auto result8 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 9 * CENT, CENT); BOOST_CHECK(result8); BOOST_CHECK_EQUAL(result8->GetSelectedValue(), 10 * CENT); BOOST_CHECK_EQUAL(result8->GetInputSet().size(), 1U); @@ -457,12 +440,12 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(coins, *wallet, 30*CENT); // now we have 6+7+8+20+30 = 71 cents total // check that we have 71 and not 72 - const auto result9 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 71 * CENT); + const auto result9 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 71 * CENT, CENT); BOOST_CHECK(result9); - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 72 * CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 72 * CENT, CENT)); // 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 - const auto result10 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 16 * CENT); + const auto result10 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 16 * CENT, CENT); BOOST_CHECK(result10); BOOST_CHECK_EQUAL(result10->GetSelectedValue(), 20 * CENT); // we should get 20 in one coin BOOST_CHECK_EQUAL(result10->GetInputSet().size(), 1U); @@ -470,7 +453,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(coins, *wallet, 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 - const auto result11 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 16 * CENT); + const auto result11 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 16 * CENT, CENT); BOOST_CHECK(result11); BOOST_CHECK_EQUAL(result11->GetSelectedValue(), 18 * CENT); // we should get 18 in 3 coins BOOST_CHECK_EQUAL(result11->GetInputSet().size(), 3U); @@ -478,13 +461,13 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(coins, *wallet, 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 - const auto result12 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 16 * CENT); + const auto result12 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 16 * CENT, CENT); BOOST_CHECK(result12); BOOST_CHECK_EQUAL(result12->GetSelectedValue(), 18 * CENT); // we should get 18 in 1 coin BOOST_CHECK_EQUAL(result12->GetInputSet().size(), 1U); // because in the event of a tie, the biggest coin wins // now try making 11 cents. we should get 5+6 - const auto result13 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 11 * CENT); + const auto result13 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 11 * CENT, CENT); BOOST_CHECK(result13); BOOST_CHECK_EQUAL(result13->GetSelectedValue(), 11 * CENT); BOOST_CHECK_EQUAL(result13->GetInputSet().size(), 2U); @@ -494,12 +477,12 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(coins, *wallet, 2*COIN); add_coin(coins, *wallet, 3*COIN); add_coin(coins, *wallet, 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents - const auto result14 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 95 * CENT); + const auto result14 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 95 * CENT, CENT); BOOST_CHECK(result14); BOOST_CHECK_EQUAL(result14->GetSelectedValue(), 1 * COIN); // we should get 1 BTC in 1 coin BOOST_CHECK_EQUAL(result14->GetInputSet().size(), 1U); - const auto result15 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 195 * CENT); + const auto result15 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 195 * CENT, CENT); BOOST_CHECK(result15); BOOST_CHECK_EQUAL(result15->GetSelectedValue(), 2 * COIN); // we should get 2 BTC in 1 coin BOOST_CHECK_EQUAL(result15->GetInputSet().size(), 1U); @@ -507,34 +490,34 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // empty the wallet and start again, now with fractions of a cent, to test small change avoidance coins.clear(); - add_coin(coins, *wallet, MIN_CHANGE * 1 / 10); - add_coin(coins, *wallet, MIN_CHANGE * 2 / 10); - add_coin(coins, *wallet, MIN_CHANGE * 3 / 10); - add_coin(coins, *wallet, MIN_CHANGE * 4 / 10); - add_coin(coins, *wallet, MIN_CHANGE * 5 / 10); - - // 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 - const auto result16 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), MIN_CHANGE); + add_coin(coins, *wallet, CENT * 1 / 10); + add_coin(coins, *wallet, CENT * 2 / 10); + add_coin(coins, *wallet, CENT * 3 / 10); + add_coin(coins, *wallet, CENT * 4 / 10); + add_coin(coins, *wallet, CENT * 5 / 10); + + // try making 1 * CENT from the 1.5 * CENT + // we'll get change smaller than CENT whatever happens, so can expect CENT exactly + const auto result16 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), CENT, CENT); BOOST_CHECK(result16); - BOOST_CHECK_EQUAL(result16->GetSelectedValue(), MIN_CHANGE); + BOOST_CHECK_EQUAL(result16->GetSelectedValue(), CENT); // but if we add a bigger coin, small change is avoided - add_coin(coins, *wallet, 1111*MIN_CHANGE); + add_coin(coins, *wallet, 1111*CENT); // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 - const auto result17 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 1 * MIN_CHANGE); + const auto result17 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result17); - BOOST_CHECK_EQUAL(result17->GetSelectedValue(), 1 * MIN_CHANGE); // we should get the exact amount + BOOST_CHECK_EQUAL(result17->GetSelectedValue(), 1 * CENT); // we should get the exact amount // if we add more small coins: - add_coin(coins, *wallet, MIN_CHANGE * 6 / 10); - add_coin(coins, *wallet, MIN_CHANGE * 7 / 10); + add_coin(coins, *wallet, CENT * 6 / 10); + add_coin(coins, *wallet, CENT * 7 / 10); - // and try again to make 1.0 * MIN_CHANGE - const auto result18 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 1 * MIN_CHANGE); + // and try again to make 1.0 * CENT + const auto result18 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result18); - BOOST_CHECK_EQUAL(result18->GetSelectedValue(), 1 * MIN_CHANGE); // we should get the exact amount + BOOST_CHECK_EQUAL(result18->GetSelectedValue(), 1 * CENT); // we should get the exact amount // run the 'mtgox' test (see https://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change @@ -542,52 +525,52 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) for (int j = 0; j < 20; j++) add_coin(coins, *wallet, 50000 * COIN); - const auto result19 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 500000 * COIN); + const auto result19 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 500000 * COIN, CENT); BOOST_CHECK(result19); BOOST_CHECK_EQUAL(result19->GetSelectedValue(), 500000 * COIN); // we should get the exact amount BOOST_CHECK_EQUAL(result19->GetInputSet().size(), 10U); // in ten coins - // if there's not enough in the smaller coins to make at least 1 * MIN_CHANGE change (0.5+0.6+0.7 < 1.0+1.0), + // if there's not enough in the smaller coins to make at least 1 * CENT change (0.5+0.6+0.7 < 1.0+1.0), // we need to try finding an exact subset anyway // sometimes it will fail, and so we use the next biggest coin: coins.clear(); - add_coin(coins, *wallet, MIN_CHANGE * 5 / 10); - add_coin(coins, *wallet, MIN_CHANGE * 6 / 10); - add_coin(coins, *wallet, MIN_CHANGE * 7 / 10); - add_coin(coins, *wallet, 1111 * MIN_CHANGE); - const auto result20 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 1 * MIN_CHANGE); + add_coin(coins, *wallet, CENT * 5 / 10); + add_coin(coins, *wallet, CENT * 6 / 10); + add_coin(coins, *wallet, CENT * 7 / 10); + add_coin(coins, *wallet, 1111 * CENT); + const auto result20 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result20); - BOOST_CHECK_EQUAL(result20->GetSelectedValue(), 1111 * MIN_CHANGE); // we get the bigger coin + BOOST_CHECK_EQUAL(result20->GetSelectedValue(), 1111 * CENT); // we get the bigger coin BOOST_CHECK_EQUAL(result20->GetInputSet().size(), 1U); // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) coins.clear(); - add_coin(coins, *wallet, MIN_CHANGE * 4 / 10); - add_coin(coins, *wallet, MIN_CHANGE * 6 / 10); - add_coin(coins, *wallet, MIN_CHANGE * 8 / 10); - add_coin(coins, *wallet, 1111 * MIN_CHANGE); - const auto result21 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), MIN_CHANGE); + add_coin(coins, *wallet, CENT * 4 / 10); + add_coin(coins, *wallet, CENT * 6 / 10); + add_coin(coins, *wallet, CENT * 8 / 10); + add_coin(coins, *wallet, 1111 * CENT); + const auto result21 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), CENT, CENT); BOOST_CHECK(result21); - BOOST_CHECK_EQUAL(result21->GetSelectedValue(), MIN_CHANGE); // we should get the exact amount + BOOST_CHECK_EQUAL(result21->GetSelectedValue(), CENT); // we should get the exact amount BOOST_CHECK_EQUAL(result21->GetInputSet().size(), 2U); // in two coins 0.4+0.6 // test avoiding small change coins.clear(); - add_coin(coins, *wallet, MIN_CHANGE * 5 / 100); - add_coin(coins, *wallet, MIN_CHANGE * 1); - add_coin(coins, *wallet, MIN_CHANGE * 100); + add_coin(coins, *wallet, CENT * 5 / 100); + add_coin(coins, *wallet, CENT * 1); + add_coin(coins, *wallet, CENT * 100); // trying to make 100.01 from these three coins - const auto result22 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), MIN_CHANGE * 10001 / 100); + const auto result22 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), CENT * 10001 / 100, CENT); BOOST_CHECK(result22); - BOOST_CHECK_EQUAL(result22->GetSelectedValue(), MIN_CHANGE * 10105 / 100); // we should get all coins + BOOST_CHECK_EQUAL(result22->GetSelectedValue(), CENT * 10105 / 100); // we should get all coins BOOST_CHECK_EQUAL(result22->GetInputSet().size(), 3U); // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change - const auto result23 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), MIN_CHANGE * 9990 / 100); + const auto result23 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), CENT * 9990 / 100, CENT); BOOST_CHECK(result23); - BOOST_CHECK_EQUAL(result23->GetSelectedValue(), 101 * MIN_CHANGE); + BOOST_CHECK_EQUAL(result23->GetSelectedValue(), 101 * CENT); BOOST_CHECK_EQUAL(result23->GetInputSet().size(), 2U); } @@ -600,12 +583,12 @@ 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++) { - const auto result24 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 2000); + const auto result24 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 2000, CENT); BOOST_CHECK(result24); - if (amt - 2000 < MIN_CHANGE) { + if (amt - 2000 < CENT) { // needs more than one input: - uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); + uint16_t returnSize = std::ceil((2000.0 + CENT)/amt); CAmount returnValue = amt * returnSize; BOOST_CHECK_EQUAL(result24->GetSelectedValue(), returnValue); BOOST_CHECK_EQUAL(result24->GetInputSet().size(), returnSize); @@ -627,9 +610,9 @@ 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 - const auto result25 = KnapsackSolver(GroupCoins(coins), 50 * COIN); + const auto result25 = KnapsackSolver(GroupCoins(coins), 50 * COIN, CENT); BOOST_CHECK(result25); - const auto result26 = KnapsackSolver(GroupCoins(coins), 50 * COIN); + const auto result26 = KnapsackSolver(GroupCoins(coins), 50 * COIN, CENT); BOOST_CHECK(result26); BOOST_CHECK(!EqualResult(*result25, *result26)); @@ -640,9 +623,9 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // When choosing 1 from 100 identical coins, 1% of the time, this test will choose the same coin twice // which will cause it to fail. // To avoid that issue, run the test RANDOM_REPEATS times and only complain if all of them fail - const auto result27 = KnapsackSolver(GroupCoins(coins), COIN); + const auto result27 = KnapsackSolver(GroupCoins(coins), COIN, CENT); BOOST_CHECK(result27); - const auto result28 = KnapsackSolver(GroupCoins(coins), COIN); + const auto result28 = KnapsackSolver(GroupCoins(coins), COIN, CENT); BOOST_CHECK(result28); if (EqualResult(*result27, *result28)) fails++; @@ -663,9 +646,9 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) int fails = 0; for (int j = 0; j < RANDOM_REPEATS; j++) { - const auto result29 = KnapsackSolver(GroupCoins(coins), 90 * CENT); + const auto result29 = KnapsackSolver(GroupCoins(coins), 90 * CENT, CENT); BOOST_CHECK(result29); - const auto result30 = KnapsackSolver(GroupCoins(coins), 90 * CENT); + const auto result30 = KnapsackSolver(GroupCoins(coins), 90 * CENT, CENT); BOOST_CHECK(result30); if (EqualResult(*result29, *result30)) fails++; @@ -691,7 +674,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) add_coin(coins, *wallet, 1000 * COIN); add_coin(coins, *wallet, 3 * COIN); - const auto result = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 1003 * COIN, rand); + const auto result = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 1003 * COIN, CENT, rand); BOOST_CHECK(result); BOOST_CHECK_EQUAL(result->GetSelectedValue(), 1003 * COIN); BOOST_CHECK_EQUAL(result->GetInputSet().size(), 2U); @@ -734,13 +717,14 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) // Perform selection CoinSelectionParams cs_params{ rand, - /* change_output_size= */ 34, - /* change_spend_size= */ 148, - /* effective_feerate= */ CFeeRate(0), - /* long_term_feerate= */ CFeeRate(0), - /* discard_feerate= */ CFeeRate(0), - /* tx_noinputs_size= */ 0, - /* avoid_partial= */ false, + /*change_output_size=*/ 34, + /*change_spend_size=*/ 148, + /*min_change_target=*/ CENT, + /*effective_feerate=*/ CFeeRate(0), + /*long_term_feerate=*/ CFeeRate(0), + /*discard_feerate=*/ CFeeRate(0), + /*tx_noinputs_size=*/ 0, + /*avoid_partial=*/ false, }; CCoinControl cc; const auto result = SelectCoins(*wallet, coins, target, cc, cs_params); diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp new file mode 100644 index 0000000000..2693b68cca --- /dev/null +++ b/src/wallet/test/fuzz/coinselection.cpp @@ -0,0 +1,99 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <policy/feerate.h> +#include <primitives/transaction.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <test/util/setup_common.h> +#include <wallet/coinselection.h> + +#include <vector> + +namespace wallet { + +static void AddCoin(const CAmount& value, int n_input, int n_input_bytes, int locktime, std::vector<COutput>& coins) +{ + CMutableTransaction tx; + tx.vout.resize(n_input + 1); + tx.vout[n_input].nValue = value; + tx.nLockTime = locktime; // all transactions get different hashes + coins.emplace_back(COutPoint(tx.GetHash(), n_input), tx.vout.at(n_input), /*depth=*/0, n_input_bytes, /*spendable=*/true, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/true); +} + +// Randomly distribute coins to instances of OutputGroup +static void GroupCoins(FuzzedDataProvider& fuzzed_data_provider, const std::vector<COutput>& coins, const CoinSelectionParams& coin_params, bool positive_only, std::vector<OutputGroup>& output_groups) +{ + auto output_group = OutputGroup(coin_params); + bool valid_outputgroup{false}; + for (auto& coin : coins) { + output_group.Insert(coin, /*ancestors=*/0, /*descendants=*/0, positive_only); + // If positive_only was specified, nothing may have been inserted, leading to an empty outpout group + // that would be invalid for the BnB algorithm + valid_outputgroup = !positive_only || output_group.GetSelectionAmount() > 0; + if (valid_outputgroup && fuzzed_data_provider.ConsumeBool()) { + output_groups.push_back(output_group); + output_group = OutputGroup(coin_params); + valid_outputgroup = false; + } + } + if (valid_outputgroup) output_groups.push_back(output_group); +} + +FUZZ_TARGET(coinselection) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + std::vector<COutput> utxo_pool; + + const CFeeRate long_term_fee_rate{ConsumeMoney(fuzzed_data_provider, /*max=*/COIN)}; + const CFeeRate effective_fee_rate{ConsumeMoney(fuzzed_data_provider, /*max=*/COIN)}; + const CAmount cost_of_change{ConsumeMoney(fuzzed_data_provider, /*max=*/COIN)}; + const CAmount target{fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(1, MAX_MONEY)}; + const bool subtract_fee_outputs{fuzzed_data_provider.ConsumeBool()}; + + FastRandomContext fast_random_context{ConsumeUInt256(fuzzed_data_provider)}; + CoinSelectionParams coin_params{fast_random_context}; + coin_params.m_subtract_fee_outputs = subtract_fee_outputs; + coin_params.m_long_term_feerate = long_term_fee_rate; + coin_params.m_effective_feerate = effective_fee_rate; + + // Create some coins + CAmount total_balance{0}; + int next_locktime{0}; + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) + { + const int n_input{fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 10)}; + const int n_input_bytes{fuzzed_data_provider.ConsumeIntegralInRange<int>(100, 10000)}; + const CAmount amount{fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(1, MAX_MONEY)}; + if (total_balance + amount >= MAX_MONEY) { + break; + } + AddCoin(amount, n_input, n_input_bytes, ++next_locktime, utxo_pool); + total_balance += amount; + } + + std::vector<OutputGroup> group_pos; + GroupCoins(fuzzed_data_provider, utxo_pool, coin_params, /*positive_only=*/true, group_pos); + std::vector<OutputGroup> group_all; + GroupCoins(fuzzed_data_provider, utxo_pool, coin_params, /*positive_only=*/false, group_all); + + // Run coinselection algorithms + const auto result_bnb = SelectCoinsBnB(group_pos, target, cost_of_change); + + auto result_srd = SelectCoinsSRD(group_pos, target, fast_random_context); + if (result_srd) result_srd->ComputeAndSetWaste(cost_of_change); + + CAmount change_target{GenerateChangeTarget(target, fast_random_context)}; + auto result_knapsack = KnapsackSolver(group_all, target, change_target, fast_random_context); + if (result_knapsack) result_knapsack->ComputeAndSetWaste(cost_of_change); + + // If the total balance is sufficient for the target and we are not using + // effective values, Knapsack should always find a solution. + if (total_balance >= target && subtract_fee_outputs) { + assert(result_knapsack); + } +} + +} // namespace wallet diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index cb006dea3a..38d23b6f91 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -9,6 +9,7 @@ namespace wallet { WalletTestingSetup::WalletTestingSetup(const std::string& chainName) : TestingSetup(chainName), + m_wallet_loader{interfaces::MakeWalletLoader(*m_node.chain, *Assert(m_node.args))}, m_wallet(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()) { m_wallet.LoadWallet(); diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index d4b855b145..e0b38a40e7 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -22,7 +22,7 @@ struct WalletTestingSetup : public TestingSetup { explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); ~WalletTestingSetup(); - std::unique_ptr<interfaces::WalletLoader> m_wallet_loader = interfaces::MakeWalletLoader(*m_node.chain, *Assert(m_node.args)); + std::unique_ptr<interfaces::WalletLoader> m_wallet_loader; CWallet m_wallet; std::unique_ptr<interfaces::Handler> m_chain_notifications_handler; }; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 7d2b769ae6..683f0eb327 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -54,6 +54,7 @@ static const std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context) std::vector<bilingual_str> warnings; auto database = MakeWalletDatabase("", options, status, error); auto wallet = CWallet::Create(context, "", std::move(database), options.create_flags, error, warnings); + NotifyWalletLoaded(context, wallet); if (context.chain) { wallet->postInitProcess(); } @@ -339,7 +340,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) { CWallet wallet(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); - CWalletTx wtx{m_coinbase_txns.back(), TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*position_in_block=*/0}}; + CWalletTx wtx{m_coinbase_txns.back(), TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*index=*/0}}; LOCK(wallet.cs_wallet); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); @@ -373,7 +374,7 @@ static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lock block = &inserted.first->second; block->nTime = blockTime; block->phashBlock = &hash; - state = TxStateConfirmed{hash, block->nHeight, /*position_in_block=*/0}; + state = TxStateConfirmed{hash, block->nHeight, /*index=*/0}; } return wallet.AddToWallet(MakeTransactionRef(tx), state, [&](CWalletTx& wtx, bool /* new_tx */) { // Assign wtx.m_state to simplify test and avoid the need to simulate @@ -540,7 +541,7 @@ public: wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, m_node.chainman->ActiveChain().Tip()->GetBlockHash()); auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); - it->second.m_state = TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*position_in_block=*/1}; + it->second.m_state = TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*index=*/1}; return it->second; } @@ -588,7 +589,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup) for (const auto& group : list) { for (const auto& coin : group.second) { LOCK(wallet->cs_wallet); - wallet->LockCoin(COutPoint(coin.tx->GetHash(), coin.i)); + wallet->LockCoin(coin.outpoint); } } { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index be64b4cdbb..2a0653c719 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -167,6 +167,14 @@ std::unique_ptr<interfaces::Handler> HandleLoadWallet(WalletContext& context, Lo return interfaces::MakeHandler([&context, it] { LOCK(context.wallets_mutex); context.wallet_load_fns.erase(it); }); } +void NotifyWalletLoaded(WalletContext& context, const std::shared_ptr<CWallet>& wallet) +{ + LOCK(context.wallets_mutex); + for (auto& load_wallet : context.wallet_load_fns) { + load_wallet(interfaces::MakeWallet(context, wallet)); + } +} + static Mutex g_loading_wallet_mutex; static Mutex g_wallet_release_mutex; static std::condition_variable g_wallet_release_cv; @@ -232,6 +240,8 @@ std::shared_ptr<CWallet> LoadWalletInternal(WalletContext& context, const std::s status = DatabaseStatus::FAILED_LOAD; return nullptr; } + + NotifyWalletLoaded(context, wallet); AddWallet(context, wallet); wallet->postInitProcess(); @@ -348,6 +358,8 @@ std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string& wallet->Lock(); } } + + NotifyWalletLoaded(context, wallet); AddWallet(context, wallet); wallet->postInitProcess(); @@ -2905,13 +2917,6 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri } { - LOCK(context.wallets_mutex); - for (auto& load_wallet : context.wallet_load_fns) { - load_wallet(interfaces::MakeWallet(context, walletInstance)); - } - } - - { LOCK(walletInstance->cs_wallet); walletInstance->SetBroadcastTransactions(args.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); walletInstance->WalletLogPrintf("setKeyPool.size() = %u\n", walletInstance->GetKeyPoolSize()); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e2c5c69c91..26b7f97b5f 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -20,7 +20,6 @@ #include <util/system.h> #include <util/ui_change_type.h> #include <validationinterface.h> -#include <wallet/coinselection.h> #include <wallet/crypter.h> #include <wallet/scriptpubkeyman.h> #include <wallet/transaction.h> @@ -68,6 +67,7 @@ std::shared_ptr<CWallet> LoadWallet(WalletContext& context, const std::string& n std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string& name, std::optional<bool> load_on_start, DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings); std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& backup_file, const std::string& wallet_name, std::optional<bool> load_on_start, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings); std::unique_ptr<interfaces::Handler> HandleLoadWallet(WalletContext& context, LoadWalletFn load_wallet); +void NotifyWalletLoaded(WalletContext& context, const std::shared_ptr<CWallet>& wallet); std::unique_ptr<WalletDatabase> MakeWalletDatabase(const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error); //! -paytxfee default @@ -95,7 +95,7 @@ static const CAmount WALLET_INCREMENTAL_RELAY_FEE = 5000; //! Default for -spendzeroconfchange static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true; //! Default for -walletrejectlongchains -static const bool DEFAULT_WALLET_REJECT_LONG_CHAINS = false; +static const bool DEFAULT_WALLET_REJECT_LONG_CHAINS{true}; //! -txconfirmtarget default static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 6; //! -walletrbf default @@ -112,7 +112,6 @@ constexpr CAmount HIGH_MAX_TX_FEE{100 * HIGH_TX_FEE_PER_KB}; static constexpr size_t DUMMY_NESTED_P2WPKH_INPUT_SIZE = 91; class CCoinControl; -class COutput; class CWalletTx; class ReserveDestination; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 1251c9f37e..7bbed7973f 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -850,10 +850,10 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // Set the active ScriptPubKeyMans for (auto spk_man_pair : wss.m_active_external_spks) { - pwallet->LoadActiveScriptPubKeyMan(spk_man_pair.second, spk_man_pair.first, /* internal */ false); + pwallet->LoadActiveScriptPubKeyMan(spk_man_pair.second, spk_man_pair.first, /*internal=*/false); } for (auto spk_man_pair : wss.m_active_internal_spks) { - pwallet->LoadActiveScriptPubKeyMan(spk_man_pair.second, spk_man_pair.first, /* internal */ true); + pwallet->LoadActiveScriptPubKeyMan(spk_man_pair.second, spk_man_pair.first, /*internal=*/true); } // Set the descriptor caches |