diff options
Diffstat (limited to 'src')
33 files changed, 707 insertions, 560 deletions
diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 3c24fee60f..f97bf8028a 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -54,12 +54,10 @@ static void CoinSelection(benchmark::Bench& bench) /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), /* tx_noinputs_size= */ 0, /* avoid_partial= */ false); bench.run([&] { - std::set<CInputCoin> setCoinsRet; - CAmount nValueRet; - bool success = AttemptSelection(wallet, 1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params); - assert(success); - assert(nValueRet == 1003 * COIN); - assert(setCoinsRet.size() == 2); + auto result = AttemptSelection(wallet, 1003 * COIN, filter_standard, coins, coin_selection_params); + assert(result); + assert(result->GetSelectedValue() == 1003 * COIN); + assert(result->GetInputSet().size() == 2); }); } @@ -92,17 +90,14 @@ static void BnBExhaustion(benchmark::Bench& bench) { // Setup std::vector<OutputGroup> utxo_pool; - CoinSet selection; - CAmount value_ret = 0; bench.run([&] { // Benchmark CAmount target = make_hard_case(17, utxo_pool); - SelectCoinsBnB(utxo_pool, target, 0, selection, value_ret); // Should exhaust + SelectCoinsBnB(utxo_pool, target, 0); // Should exhaust // Cleanup utxo_pool.clear(); - selection.clear(); }); } diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index afa5b99549..3c8dbeaa21 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -8,14 +8,21 @@ #include <chainparams.h> #include <chainparamsbase.h> +#include <clientversion.h> #include <interfaces/init.h> +#include <key.h> #include <logging.h> +#include <pubkey.h> +#include <tinyformat.h> #include <util/system.h> #include <util/translation.h> #include <util/url.h> #include <wallet/wallettool.h> +#include <exception> #include <functional> +#include <string> +#include <tuple> const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr; UrlDecodeFn* const URL_DECODE = nullptr; diff --git a/src/net.cpp b/src/net.cpp index 2d9e69b6fb..2adccce11e 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -586,8 +586,8 @@ void CNode::CopyStats(CNodeStats& stats) } else { stats.fRelayTxes = false; } - X(nLastSend); - X(nLastRecv); + X(m_last_send); + X(m_last_recv); X(nLastTXTime); X(nLastBlockTime); X(nTimeConnected); @@ -634,7 +634,7 @@ bool CNode::ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete) complete = false; const auto time = GetTime<std::chrono::microseconds>(); LOCK(cs_vRecv); - nLastRecv = std::chrono::duration_cast<std::chrono::seconds>(time).count(); + m_last_recv = std::chrono::duration_cast<std::chrono::seconds>(time); nRecvBytes += msg_bytes.size(); while (msg_bytes.size() > 0) { // absorb network data @@ -805,7 +805,7 @@ size_t CConnman::SocketSendData(CNode& node) const nBytes = send(node.hSocket, reinterpret_cast<const char*>(data.data()) + node.nSendOffset, data.size() - node.nSendOffset, MSG_NOSIGNAL | MSG_DONTWAIT); } if (nBytes > 0) { - node.nLastSend = GetTimeSeconds(); + node.m_last_send = GetTime<std::chrono::seconds>(); node.nSendBytes += nBytes; node.nSendOffset += nBytes; nSentSize += nBytes; @@ -1318,31 +1318,33 @@ void CConnman::NotifyNumConnectionsChanged() } } -bool CConnman::ShouldRunInactivityChecks(const CNode& node, int64_t now) const +bool CConnman::ShouldRunInactivityChecks(const CNode& node, std::chrono::seconds now) const { - return node.nTimeConnected + m_peer_connect_timeout < now; + return std::chrono::seconds{node.nTimeConnected} + m_peer_connect_timeout < now; } bool CConnman::InactivityCheck(const CNode& node) const { - // Use non-mockable system time (otherwise these timers will pop when we - // use setmocktime in the tests). - int64_t now = GetTimeSeconds(); + // Tests that see disconnects after using mocktime can start nodes with a + // large timeout. For example, -peertimeout=999999999. + const auto now{GetTime<std::chrono::seconds>()}; + const auto last_send{node.m_last_send.load()}; + const auto last_recv{node.m_last_recv.load()}; if (!ShouldRunInactivityChecks(node, now)) return false; - if (node.nLastRecv == 0 || node.nLastSend == 0) { - LogPrint(BCLog::NET, "socket no message in first %i seconds, %d %d peer=%d\n", m_peer_connect_timeout, node.nLastRecv != 0, node.nLastSend != 0, node.GetId()); + if (last_recv.count() == 0 || last_send.count() == 0) { + LogPrint(BCLog::NET, "socket no message in first %i seconds, %d %d peer=%d\n", count_seconds(m_peer_connect_timeout), last_recv.count() != 0, last_send.count() != 0, node.GetId()); return true; } - if (now > node.nLastSend + TIMEOUT_INTERVAL) { - LogPrint(BCLog::NET, "socket sending timeout: %is peer=%d\n", now - node.nLastSend, node.GetId()); + if (now > last_send + TIMEOUT_INTERVAL) { + LogPrint(BCLog::NET, "socket sending timeout: %is peer=%d\n", count_seconds(now - last_send), node.GetId()); return true; } - if (now > node.nLastRecv + TIMEOUT_INTERVAL) { - LogPrint(BCLog::NET, "socket receive timeout: %is peer=%d\n", now - node.nLastRecv, node.GetId()); + if (now > last_recv + TIMEOUT_INTERVAL) { + LogPrint(BCLog::NET, "socket receive timeout: %is peer=%d\n", count_seconds(now - last_recv), node.GetId()); return true; } @@ -48,7 +48,7 @@ static const bool DEFAULT_WHITELISTRELAY = true; static const bool DEFAULT_WHITELISTFORCERELAY = false; /** Time after which to disconnect, after waiting for a ping response (or inactivity). */ -static const int TIMEOUT_INTERVAL = 20 * 60; +static constexpr std::chrono::minutes TIMEOUT_INTERVAL{20}; /** Run the feeler connection loop once every 2 minutes. **/ static constexpr auto FEELER_INTERVAL = 2min; /** Run the extra block-relay-only connection loop once every 5 minutes. **/ @@ -241,8 +241,8 @@ public: NodeId nodeid; ServiceFlags nServices; bool fRelayTxes; - int64_t nLastSend; - int64_t nLastRecv; + std::chrono::seconds m_last_send; + std::chrono::seconds m_last_recv; int64_t nLastTXTime; int64_t nLastBlockTime; int64_t nTimeConnected; @@ -420,8 +420,8 @@ public: uint64_t nRecvBytes GUARDED_BY(cs_vRecv){0}; - std::atomic<int64_t> nLastSend{0}; - std::atomic<int64_t> nLastRecv{0}; + std::atomic<std::chrono::seconds> m_last_send{0s}; + std::atomic<std::chrono::seconds> m_last_recv{0s}; //! Unix epoch time at peer connection, in seconds. const int64_t nTimeConnected; std::atomic<int64_t> nTimeOffset{0}; @@ -784,7 +784,7 @@ public: m_msgproc = connOptions.m_msgproc; nSendBufferMaxSize = connOptions.nSendBufferMaxSize; nReceiveFloodSize = connOptions.nReceiveFloodSize; - m_peer_connect_timeout = connOptions.m_peer_connect_timeout; + m_peer_connect_timeout = std::chrono::seconds{connOptions.m_peer_connect_timeout}; { LOCK(cs_totalBytesSent); nMaxOutboundLimit = connOptions.nMaxOutboundLimit; @@ -942,7 +942,7 @@ public: std::chrono::microseconds PoissonNextSendInbound(std::chrono::microseconds now, std::chrono::seconds average_interval); /** Return true if we should disconnect the peer for failing an inactivity check. */ - bool ShouldRunInactivityChecks(const CNode& node, int64_t secs_now) const; + bool ShouldRunInactivityChecks(const CNode& node, std::chrono::seconds now) const; private: struct ListenSocket { @@ -1084,7 +1084,7 @@ private: uint64_t nMaxOutboundLimit GUARDED_BY(cs_totalBytesSent); // P2P timeout in seconds - int64_t m_peer_connect_timeout; + std::chrono::seconds m_peer_connect_timeout; // Whitelisted ranges. Any node connecting from these is automatically // whitelisted (as well as those connecting to whitelisted binds). diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e4b8e9b6a9..809fb8efde 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4357,9 +4357,10 @@ void PeerManagerImpl::CheckForStaleTipAndEvictPeers() void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::microseconds now) { - if (m_connman.ShouldRunInactivityChecks(node_to, std::chrono::duration_cast<std::chrono::seconds>(now).count()) && + if (m_connman.ShouldRunInactivityChecks(node_to, std::chrono::duration_cast<std::chrono::seconds>(now)) && peer.m_ping_nonce_sent && - now > peer.m_ping_start.load() + std::chrono::seconds{TIMEOUT_INTERVAL}) { + now > peer.m_ping_start.load() + TIMEOUT_INTERVAL) + { // The ping timeout is using mocktime. To disable the check during // testing, increase -peertimeout. LogPrint(BCLog::NET, "ping timeout: %fs peer=%d\n", 0.000001 * count_microseconds(now - peer.m_ping_start.load()), peer.m_id); diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 4262866f32..35e4ab4ecd 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -20,6 +20,7 @@ #include <script/script.h> #include <script/standard.h> #include <util/system.h> +#include <util/time.h> #ifdef WIN32 #ifndef NOMINMAX @@ -705,8 +706,9 @@ QString ConnectionTypeToQString(ConnectionType conn_type, bool prepend_direction assert(false); } -QString formatDurationStr(int secs) +QString formatDurationStr(std::chrono::seconds dur) { + const auto secs = count_seconds(dur); QStringList strList; int days = secs / 86400; int hours = (secs % 86400) / 3600; diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index 364962ee9a..fb0dbba88b 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -221,7 +221,7 @@ namespace GUIUtil QString ConnectionTypeToQString(ConnectionType conn_type, bool prepend_direction); /** Convert seconds into a QString with days, hours, mins, secs */ - QString formatDurationStr(int secs); + QString formatDurationStr(std::chrono::seconds dur); /** Format CNodeStats.nServices bitmask into a user-readable string */ QString formatServicesStr(quint64 mask); diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 1c19fb4b27..5d08aff6b9 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1141,7 +1141,7 @@ void RPCConsole::on_sldGraphRange_valueChanged(int value) void RPCConsole::setTrafficGraphRange(int mins) { ui->trafficGraph->setGraphRangeMins(mins); - ui->lblGraphRange->setText(GUIUtil::formatDurationStr(mins * 60)); + ui->lblGraphRange->setText(GUIUtil::formatDurationStr(std::chrono::minutes{mins})); } void RPCConsole::updateTrafficStats(quint64 totalBytesIn, quint64 totalBytesOut) @@ -1172,12 +1172,12 @@ void RPCConsole::updateDetailWidget() if (stats->nodeStats.m_bip152_highbandwidth_from) bip152_hb_settings += (bip152_hb_settings.isEmpty() ? ts.from : QLatin1Char('/') + ts.from); if (bip152_hb_settings.isEmpty()) bip152_hb_settings = ts.no; ui->peerHighBandwidth->setText(bip152_hb_settings); - const int64_t time_now{GetTimeSeconds()}; - ui->peerConnTime->setText(GUIUtil::formatDurationStr(time_now - stats->nodeStats.nTimeConnected)); - ui->peerLastBlock->setText(TimeDurationField(time_now, stats->nodeStats.nLastBlockTime)); - ui->peerLastTx->setText(TimeDurationField(time_now, stats->nodeStats.nLastTXTime)); - ui->peerLastSend->setText(TimeDurationField(time_now, stats->nodeStats.nLastSend)); - ui->peerLastRecv->setText(TimeDurationField(time_now, stats->nodeStats.nLastRecv)); + const auto time_now{GetTime<std::chrono::seconds>()}; + ui->peerConnTime->setText(GUIUtil::formatDurationStr(time_now - std::chrono::seconds{stats->nodeStats.nTimeConnected})); + ui->peerLastBlock->setText(TimeDurationField(time_now, std::chrono::seconds{stats->nodeStats.nLastBlockTime})); + ui->peerLastTx->setText(TimeDurationField(time_now, std::chrono::seconds{stats->nodeStats.nLastTXTime})); + ui->peerLastSend->setText(TimeDurationField(time_now, stats->nodeStats.m_last_send)); + ui->peerLastRecv->setText(TimeDurationField(time_now, stats->nodeStats.m_last_recv)); ui->peerBytesSent->setText(GUIUtil::formatBytes(stats->nodeStats.nSendBytes)); ui->peerBytesRecv->setText(GUIUtil::formatBytes(stats->nodeStats.nRecvBytes)); ui->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.m_last_ping_time)); diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index 2412ae543c..fc6e85b41a 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -175,8 +175,9 @@ private: void updateNetworkState(); /** Helper for the output of a time duration field. Inputs are UNIX epoch times. */ - QString TimeDurationField(uint64_t time_now, uint64_t time_at_event) const { - return time_at_event ? GUIUtil::formatDurationStr(time_now - time_at_event) : tr("Never"); + QString TimeDurationField(std::chrono::seconds time_now, std::chrono::seconds time_at_event) const + { + return time_at_event.count() ? GUIUtil::formatDurationStr(time_now - time_at_event) : tr("Never"); } private Q_SLOTS: diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 22455058bd..827b3a7da3 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -883,8 +883,8 @@ static RPCHelpMan getblockheader() {RPCResult::Type::NUM, "difficulty", "The difficulty"}, {RPCResult::Type::STR_HEX, "chainwork", "Expected number of hashes required to produce the current chain"}, {RPCResult::Type::NUM, "nTx", "The number of transactions in the block"}, - {RPCResult::Type::STR_HEX, "previousblockhash", /* optional */ true, "The hash of the previous block (if available)"}, - {RPCResult::Type::STR_HEX, "nextblockhash", /* optional */ true, "The hash of the next block (if available)"}, + {RPCResult::Type::STR_HEX, "previousblockhash", /*optional=*/true, "The hash of the previous block (if available)"}, + {RPCResult::Type::STR_HEX, "nextblockhash", /*optional=*/true, "The hash of the next block (if available)"}, }}, RPCResult{"for verbose=false", RPCResult::Type::STR_HEX, "", "A string that is serialized, hex-encoded data for block 'hash'"}, @@ -993,8 +993,8 @@ static RPCHelpMan getblock() {RPCResult::Type::NUM, "difficulty", "The difficulty"}, {RPCResult::Type::STR_HEX, "chainwork", "Expected number of hashes required to produce the chain up to this block (in hex)"}, {RPCResult::Type::NUM, "nTx", "The number of transactions in the block"}, - {RPCResult::Type::STR_HEX, "previousblockhash", /* optional */ true, "The hash of the previous block (if available)"}, - {RPCResult::Type::STR_HEX, "nextblockhash", /* optional */ true, "The hash of the next block (if available)"}, + {RPCResult::Type::STR_HEX, "previousblockhash", /*optional=*/true, "The hash of the previous block (if available)"}, + {RPCResult::Type::STR_HEX, "nextblockhash", /*optional=*/true, "The hash of the next block (if available)"}, }}, RPCResult{"for verbosity = 2", RPCResult::Type::OBJ, "", "", @@ -1155,13 +1155,13 @@ static RPCHelpMan gettxoutsetinfo() {RPCResult::Type::STR_HEX, "bestblock", "The hash of the block at which these statistics are calculated"}, {RPCResult::Type::NUM, "txouts", "The number of unspent transaction outputs"}, {RPCResult::Type::NUM, "bogosize", "Database-independent, meaningless metric indicating the UTXO set size"}, - {RPCResult::Type::STR_HEX, "hash_serialized_2", /* optional */ true, "The serialized hash (only present if 'hash_serialized_2' hash_type is chosen)"}, - {RPCResult::Type::STR_HEX, "muhash", /* optional */ true, "The serialized hash (only present if 'muhash' hash_type is chosen)"}, - {RPCResult::Type::NUM, "transactions", /* optional */ true, "The number of transactions with unspent outputs (not available when coinstatsindex is used)"}, - {RPCResult::Type::NUM, "disk_size", /* optional */ true, "The estimated size of the chainstate on disk (not available when coinstatsindex is used)"}, + {RPCResult::Type::STR_HEX, "hash_serialized_2", /*optional=*/true, "The serialized hash (only present if 'hash_serialized_2' hash_type is chosen)"}, + {RPCResult::Type::STR_HEX, "muhash", /*optional=*/true, "The serialized hash (only present if 'muhash' hash_type is chosen)"}, + {RPCResult::Type::NUM, "transactions", /*optional=*/true, "The number of transactions with unspent outputs (not available when coinstatsindex is used)"}, + {RPCResult::Type::NUM, "disk_size", /*optional=*/true, "The estimated size of the chainstate on disk (not available when coinstatsindex is used)"}, {RPCResult::Type::STR_AMOUNT, "total_amount", "The total amount of coins in the UTXO set"}, - {RPCResult::Type::STR_AMOUNT, "total_unspendable_amount", /* optional */ true, "The total amount of coins permanently excluded from the UTXO set (only available if coinstatsindex is used)"}, - {RPCResult::Type::OBJ, "block_info", /* optional */ true, "Info on amounts in the block at this block height (only available if coinstatsindex is used)", + {RPCResult::Type::STR_AMOUNT, "total_unspendable_amount", /*optional=*/true, "The total amount of coins permanently excluded from the UTXO set (only available if coinstatsindex is used)"}, + {RPCResult::Type::OBJ, "block_info", /*optional=*/true, "Info on amounts in the block at this block height (only available if coinstatsindex is used)", { {RPCResult::Type::STR_AMOUNT, "prevout_spent", "Total amount of all prevouts spent in this block"}, {RPCResult::Type::STR_AMOUNT, "coinbase", "Coinbase subsidy amount of this block"}, @@ -1299,7 +1299,7 @@ static RPCHelpMan gettxout() {RPCResult::Type::STR, "asm", ""}, {RPCResult::Type::STR_HEX, "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, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, }}, {RPCResult::Type::BOOL, "coinbase", "Coinbase or not"}, }}, @@ -1475,32 +1475,32 @@ RPCHelpMan getblockchaininfo() {RPCResult::Type::STR_HEX, "chainwork", "total amount of work in active chain, in hexadecimal"}, {RPCResult::Type::NUM, "size_on_disk", "the estimated size of the block and undo files on disk"}, {RPCResult::Type::BOOL, "pruned", "if the blocks are subject to pruning"}, - {RPCResult::Type::NUM, "pruneheight", /* optional */ true, "lowest-height complete block stored (only present if pruning is enabled)"}, - {RPCResult::Type::BOOL, "automatic_pruning", /* optional */ true, "whether automatic pruning is enabled (only present if pruning is enabled)"}, - {RPCResult::Type::NUM, "prune_target_size", /* optional */ true, "the target size used by pruning (only present if automatic pruning is enabled)"}, + {RPCResult::Type::NUM, "pruneheight", /*optional=*/true, "lowest-height complete block stored (only present if pruning is enabled)"}, + {RPCResult::Type::BOOL, "automatic_pruning", /*optional=*/true, "whether automatic pruning is enabled (only present if pruning is enabled)"}, + {RPCResult::Type::NUM, "prune_target_size", /*optional=*/true, "the target size used by pruning (only present if automatic pruning is enabled)"}, {RPCResult::Type::OBJ_DYN, "softforks", "status of softforks", { {RPCResult::Type::OBJ, "xxxx", "name of the softfork", { {RPCResult::Type::STR, "type", "one of \"buried\", \"bip9\""}, - {RPCResult::Type::OBJ, "bip9", /* optional */ true, "status of bip9 softforks (only for \"bip9\" type)", + {RPCResult::Type::OBJ, "bip9", /*optional=*/true, "status of bip9 softforks (only for \"bip9\" type)", { {RPCResult::Type::STR, "status", "one of \"defined\", \"started\", \"locked_in\", \"active\", \"failed\""}, - {RPCResult::Type::NUM, "bit", /* optional */ true, "the bit (0-28) in the block version field used to signal this softfork (only for \"started\" and \"locked_in\" status)"}, + {RPCResult::Type::NUM, "bit", /*optional=*/true, "the bit (0-28) in the block version field used to signal this softfork (only for \"started\" and \"locked_in\" status)"}, {RPCResult::Type::NUM_TIME, "start_time", "the minimum median time past of a block at which the bit gains its meaning"}, {RPCResult::Type::NUM_TIME, "timeout", "the median time past of a block at which the deployment is considered failed if not yet locked in"}, {RPCResult::Type::NUM, "since", "height of the first block to which the status applies"}, {RPCResult::Type::NUM, "min_activation_height", "minimum height of blocks for which the rules may be enforced"}, - {RPCResult::Type::OBJ, "statistics", /* optional */ true, "numeric statistics about signalling for a softfork (only for \"started\" and \"locked_in\" status)", + {RPCResult::Type::OBJ, "statistics", /*optional=*/true, "numeric statistics about signalling for a softfork (only for \"started\" and \"locked_in\" status)", { {RPCResult::Type::NUM, "period", "the length in blocks of the signalling period"}, - {RPCResult::Type::NUM, "threshold", /* optional */ true, "the number of blocks with the version bit set required to activate the feature (only for \"started\" status)"}, + {RPCResult::Type::NUM, "threshold", /*optional=*/true, "the number of blocks with the version bit set required to activate the feature (only for \"started\" status)"}, {RPCResult::Type::NUM, "elapsed", "the number of blocks elapsed since the beginning of the current period"}, {RPCResult::Type::NUM, "count", "the number of blocks with the version bit set in the current period"}, - {RPCResult::Type::BOOL, "possible", /* optional */ true, "returns false if there are not enough blocks left in this period to pass activation threshold (only for \"started\" status)"}, + {RPCResult::Type::BOOL, "possible", /*optional=*/true, "returns false if there are not enough blocks left in this period to pass activation threshold (only for \"started\" status)"}, }}, }}, - {RPCResult::Type::NUM, "height", /* optional */ true, "height of the first block which the rules are or will be enforced (only for \"buried\" type, or \"bip9\" type with \"active\" status)"}, + {RPCResult::Type::NUM, "height", /*optional=*/true, "height of the first block which the rules are or will be enforced (only for \"buried\" type, or \"bip9\" type with \"active\" status)"}, {RPCResult::Type::BOOL, "active", "true if the rules are enforced for the mempool and the next block"}, }}, }}, @@ -1863,9 +1863,9 @@ static RPCHelpMan getchaintxstats() {RPCResult::Type::STR_HEX, "window_final_block_hash", "The hash of the final block in the window"}, {RPCResult::Type::NUM, "window_final_block_height", "The height of the final block in the window."}, {RPCResult::Type::NUM, "window_block_count", "Size of the window in number of blocks"}, - {RPCResult::Type::NUM, "window_tx_count", /* optional */ true, "The number of transactions in the window. Only returned if \"window_block_count\" is > 0"}, - {RPCResult::Type::NUM, "window_interval", /* optional */ true, "The elapsed time in the window in seconds. Only returned if \"window_block_count\" is > 0"}, - {RPCResult::Type::NUM, "txrate", /* optional */ true, "The average rate of transactions per second in the window. Only returned if \"window_interval\" is > 0"}, + {RPCResult::Type::NUM, "window_tx_count", /*optional=*/true, "The number of transactions in the window. Only returned if \"window_block_count\" is > 0"}, + {RPCResult::Type::NUM, "window_interval", /*optional=*/true, "The elapsed time in the window in seconds. Only returned if \"window_block_count\" is > 0"}, + {RPCResult::Type::NUM, "txrate", /*optional=*/true, "The average rate of transactions per second in the window. Only returned if \"window_interval\" is > 0"}, }}, RPCExamples{ HelpExampleCli("getchaintxstats", "") @@ -2000,11 +2000,11 @@ static RPCHelpMan getblockstats() RPCResult{ RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::NUM, "avgfee", /* optional */ true, "Average fee in the block"}, - {RPCResult::Type::NUM, "avgfeerate", /* optional */ true, "Average feerate (in satoshis per virtual byte)"}, - {RPCResult::Type::NUM, "avgtxsize", /* optional */ true, "Average transaction size"}, - {RPCResult::Type::STR_HEX, "blockhash", /* optional */ true, "The block hash (to check for potential reorgs)"}, - {RPCResult::Type::ARR_FIXED, "feerate_percentiles", /* optional */ true, "Feerates at the 10th, 25th, 50th, 75th, and 90th percentile weight unit (in satoshis per virtual byte)", + {RPCResult::Type::NUM, "avgfee", /*optional=*/true, "Average fee in the block"}, + {RPCResult::Type::NUM, "avgfeerate", /*optional=*/true, "Average feerate (in satoshis per virtual byte)"}, + {RPCResult::Type::NUM, "avgtxsize", /*optional=*/true, "Average transaction size"}, + {RPCResult::Type::STR_HEX, "blockhash", /*optional=*/true, "The block hash (to check for potential reorgs)"}, + {RPCResult::Type::ARR_FIXED, "feerate_percentiles", /*optional=*/true, "Feerates at the 10th, 25th, 50th, 75th, and 90th percentile weight unit (in satoshis per virtual byte)", { {RPCResult::Type::NUM, "10th_percentile_feerate", "The 10th percentile feerate"}, {RPCResult::Type::NUM, "25th_percentile_feerate", "The 25th percentile feerate"}, @@ -2012,30 +2012,30 @@ static RPCHelpMan getblockstats() {RPCResult::Type::NUM, "75th_percentile_feerate", "The 75th percentile feerate"}, {RPCResult::Type::NUM, "90th_percentile_feerate", "The 90th percentile feerate"}, }}, - {RPCResult::Type::NUM, "height", /* optional */ true, "The height of the block"}, - {RPCResult::Type::NUM, "ins", /* optional */ true, "The number of inputs (excluding coinbase)"}, - {RPCResult::Type::NUM, "maxfee", /* optional */ true, "Maximum fee in the block"}, - {RPCResult::Type::NUM, "maxfeerate", /* optional */ true, "Maximum feerate (in satoshis per virtual byte)"}, - {RPCResult::Type::NUM, "maxtxsize", /* optional */ true, "Maximum transaction size"}, - {RPCResult::Type::NUM, "medianfee", /* optional */ true, "Truncated median fee in the block"}, - {RPCResult::Type::NUM, "mediantime", /* optional */ true, "The block median time past"}, - {RPCResult::Type::NUM, "mediantxsize", /* optional */ true, "Truncated median transaction size"}, - {RPCResult::Type::NUM, "minfee", /* optional */ true, "Minimum fee in the block"}, - {RPCResult::Type::NUM, "minfeerate", /* optional */ true, "Minimum feerate (in satoshis per virtual byte)"}, - {RPCResult::Type::NUM, "mintxsize", /* optional */ true, "Minimum transaction size"}, - {RPCResult::Type::NUM, "outs", /* optional */ true, "The number of outputs"}, - {RPCResult::Type::NUM, "subsidy", /* optional */ true, "The block subsidy"}, - {RPCResult::Type::NUM, "swtotal_size", /* optional */ true, "Total size of all segwit transactions"}, - {RPCResult::Type::NUM, "swtotal_weight", /* optional */ true, "Total weight of all segwit transactions"}, - {RPCResult::Type::NUM, "swtxs", /* optional */ true, "The number of segwit transactions"}, - {RPCResult::Type::NUM, "time", /* optional */ true, "The block time"}, - {RPCResult::Type::NUM, "total_out", /* optional */ true, "Total amount in all outputs (excluding coinbase and thus reward [ie subsidy + totalfee])"}, - {RPCResult::Type::NUM, "total_size", /* optional */ true, "Total size of all non-coinbase transactions"}, - {RPCResult::Type::NUM, "total_weight", /* optional */ true, "Total weight of all non-coinbase transactions"}, - {RPCResult::Type::NUM, "totalfee", /* optional */ true, "The fee total"}, - {RPCResult::Type::NUM, "txs", /* optional */ true, "The number of transactions (including coinbase)"}, - {RPCResult::Type::NUM, "utxo_increase", /* optional */ true, "The increase/decrease in the number of unspent outputs"}, - {RPCResult::Type::NUM, "utxo_size_inc", /* optional */ true, "The increase/decrease in size for the utxo index (not discounting op_return and similar)"}, + {RPCResult::Type::NUM, "height", /*optional=*/true, "The height of the block"}, + {RPCResult::Type::NUM, "ins", /*optional=*/true, "The number of inputs (excluding coinbase)"}, + {RPCResult::Type::NUM, "maxfee", /*optional=*/true, "Maximum fee in the block"}, + {RPCResult::Type::NUM, "maxfeerate", /*optional=*/true, "Maximum feerate (in satoshis per virtual byte)"}, + {RPCResult::Type::NUM, "maxtxsize", /*optional=*/true, "Maximum transaction size"}, + {RPCResult::Type::NUM, "medianfee", /*optional=*/true, "Truncated median fee in the block"}, + {RPCResult::Type::NUM, "mediantime", /*optional=*/true, "The block median time past"}, + {RPCResult::Type::NUM, "mediantxsize", /*optional=*/true, "Truncated median transaction size"}, + {RPCResult::Type::NUM, "minfee", /*optional=*/true, "Minimum fee in the block"}, + {RPCResult::Type::NUM, "minfeerate", /*optional=*/true, "Minimum feerate (in satoshis per virtual byte)"}, + {RPCResult::Type::NUM, "mintxsize", /*optional=*/true, "Minimum transaction size"}, + {RPCResult::Type::NUM, "outs", /*optional=*/true, "The number of outputs"}, + {RPCResult::Type::NUM, "subsidy", /*optional=*/true, "The block subsidy"}, + {RPCResult::Type::NUM, "swtotal_size", /*optional=*/true, "Total size of all segwit transactions"}, + {RPCResult::Type::NUM, "swtotal_weight", /*optional=*/true, "Total weight of all segwit transactions"}, + {RPCResult::Type::NUM, "swtxs", /*optional=*/true, "The number of segwit transactions"}, + {RPCResult::Type::NUM, "time", /*optional=*/true, "The block time"}, + {RPCResult::Type::NUM, "total_out", /*optional=*/true, "Total amount in all outputs (excluding coinbase and thus reward [ie subsidy + totalfee])"}, + {RPCResult::Type::NUM, "total_size", /*optional=*/true, "Total size of all non-coinbase transactions"}, + {RPCResult::Type::NUM, "total_weight", /*optional=*/true, "Total weight of all non-coinbase transactions"}, + {RPCResult::Type::NUM, "totalfee", /*optional=*/true, "The fee total"}, + {RPCResult::Type::NUM, "txs", /*optional=*/true, "The number of transactions (including coinbase)"}, + {RPCResult::Type::NUM, "utxo_increase", /*optional=*/true, "The increase/decrease in the number of unspent outputs"}, + {RPCResult::Type::NUM, "utxo_size_inc", /*optional=*/true, "The increase/decrease in size for the utxo index (not discounting op_return and similar)"}, }}, RPCExamples{ HelpExampleCli("getblockstats", R"('"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09"' '["minfeerate","avgfeerate"]')") + diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 371de8050d..66e0df906c 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -412,8 +412,8 @@ static RPCHelpMan getmininginfo() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::NUM, "blocks", "The current block"}, - {RPCResult::Type::NUM, "currentblockweight", /* optional */ true, "The block weight of the last assembled block (only present if a block was ever assembled)"}, - {RPCResult::Type::NUM, "currentblocktx", /* optional */ true, "The number of block transactions of the last assembled block (only present if a block was ever assembled)"}, + {RPCResult::Type::NUM, "currentblockweight", /*optional=*/true, "The block weight of the last assembled block (only present if a block was ever assembled)"}, + {RPCResult::Type::NUM, "currentblocktx", /*optional=*/true, "The number of block transactions of the last assembled block (only present if a block was ever assembled)"}, {RPCResult::Type::NUM, "difficulty", "The current difficulty"}, {RPCResult::Type::NUM, "networkhashps", "The network hashes per second"}, {RPCResult::Type::NUM, "pooledtx", "The size of the mempool"}, @@ -590,12 +590,12 @@ static RPCHelpMan getblocktemplate() {RPCResult::Type::STR_HEX, "noncerange", "A range of valid nonces"}, {RPCResult::Type::NUM, "sigoplimit", "limit of sigops in blocks"}, {RPCResult::Type::NUM, "sizelimit", "limit of block size"}, - {RPCResult::Type::NUM, "weightlimit", /* optional */ true, "limit of block weight"}, + {RPCResult::Type::NUM, "weightlimit", /*optional=*/true, "limit of block weight"}, {RPCResult::Type::NUM_TIME, "curtime", "current timestamp in " + UNIX_EPOCH_TIME}, {RPCResult::Type::STR, "bits", "compressed target of next block"}, {RPCResult::Type::NUM, "height", "The height of the next block"}, - {RPCResult::Type::STR_HEX, "signet_challenge", /* optional */ true, "Only on signet"}, - {RPCResult::Type::STR_HEX, "default_witness_commitment", /* optional */ true, "a valid witness commitment for the unmodified block template"}, + {RPCResult::Type::STR_HEX, "signet_challenge", /*optional=*/true, "Only on signet"}, + {RPCResult::Type::STR_HEX, "default_witness_commitment", /*optional=*/true, "a valid witness commitment for the unmodified block template"}, }}, }, RPCExamples{ @@ -1082,8 +1082,8 @@ static RPCHelpMan estimatesmartfee() RPCResult{ RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::NUM, "feerate", /* optional */ true, "estimate fee rate in " + CURRENCY_UNIT + "/kvB (only present if no errors were encountered)"}, - {RPCResult::Type::ARR, "errors", /* optional */ true, "Errors encountered during processing (if there are any)", + {RPCResult::Type::NUM, "feerate", /*optional=*/true, "estimate fee rate in " + CURRENCY_UNIT + "/kvB (only present if no errors were encountered)"}, + {RPCResult::Type::ARR, "errors", /*optional=*/true, "Errors encountered during processing (if there are any)", { {RPCResult::Type::STR, "", "error"}, }}, @@ -1155,12 +1155,12 @@ static RPCHelpMan estimaterawfee() RPCResult{ RPCResult::Type::OBJ, "", "Results are returned for any horizon which tracks blocks up to the confirmation target", { - {RPCResult::Type::OBJ, "short", /* optional */ true, "estimate for short time horizon", + {RPCResult::Type::OBJ, "short", /*optional=*/true, "estimate for short time horizon", { - {RPCResult::Type::NUM, "feerate", /* optional */ true, "estimate fee rate in " + CURRENCY_UNIT + "/kvB"}, + {RPCResult::Type::NUM, "feerate", /*optional=*/true, "estimate fee rate in " + CURRENCY_UNIT + "/kvB"}, {RPCResult::Type::NUM, "decay", "exponential decay (per block) for historical moving average of confirmation data"}, {RPCResult::Type::NUM, "scale", "The resolution of confirmation targets at this time horizon"}, - {RPCResult::Type::OBJ, "pass", /* optional */ true, "information about the lowest range of feerates to succeed in meeting the threshold", + {RPCResult::Type::OBJ, "pass", /*optional=*/true, "information about the lowest range of feerates to succeed in meeting the threshold", { {RPCResult::Type::NUM, "startrange", "start of feerate range"}, {RPCResult::Type::NUM, "endrange", "end of feerate range"}, @@ -1169,20 +1169,20 @@ static RPCHelpMan estimaterawfee() {RPCResult::Type::NUM, "inmempool", "current number of txs in mempool in the feerate range unconfirmed for at least target blocks"}, {RPCResult::Type::NUM, "leftmempool", "number of txs over history horizon in the feerate range that left mempool unconfirmed after target"}, }}, - {RPCResult::Type::OBJ, "fail", /* optional */ true, "information about the highest range of feerates to fail to meet the threshold", + {RPCResult::Type::OBJ, "fail", /*optional=*/true, "information about the highest range of feerates to fail to meet the threshold", { {RPCResult::Type::ELISION, "", ""}, }}, - {RPCResult::Type::ARR, "errors", /* optional */ true, "Errors encountered during processing (if there are any)", + {RPCResult::Type::ARR, "errors", /*optional=*/true, "Errors encountered during processing (if there are any)", { {RPCResult::Type::STR, "error", ""}, }}, }}, - {RPCResult::Type::OBJ, "medium", /* optional */ true, "estimate for medium time horizon", + {RPCResult::Type::OBJ, "medium", /*optional=*/true, "estimate for medium time horizon", { {RPCResult::Type::ELISION, "", ""}, }}, - {RPCResult::Type::OBJ, "long", /* optional */ true, "estimate for long time horizon", + {RPCResult::Type::OBJ, "long", /*optional=*/true, "estimate for long time horizon", { {RPCResult::Type::ELISION, "", ""}, }}, diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 3716d58d4b..a5ee538f67 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -47,13 +47,13 @@ static RPCHelpMan validateaddress() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::BOOL, "isvalid", "If the address is valid or not"}, - {RPCResult::Type::STR, "address", /* optional */ true, "The bitcoin address validated"}, - {RPCResult::Type::STR_HEX, "scriptPubKey", /* optional */ true, "The hex-encoded scriptPubKey generated by the address"}, - {RPCResult::Type::BOOL, "isscript", /* optional */ true, "If the key is a script"}, - {RPCResult::Type::BOOL, "iswitness", /* optional */ true, "If the address is a witness address"}, - {RPCResult::Type::NUM, "witness_version", /* optional */ true, "The version number of the witness program"}, - {RPCResult::Type::STR_HEX, "witness_program", /* optional */ true, "The hex value of the witness program"}, - {RPCResult::Type::STR, "error", /* optional */ true, "Error message, if any"}, + {RPCResult::Type::STR, "address", /*optional=*/true, "The bitcoin address validated"}, + {RPCResult::Type::STR_HEX, "scriptPubKey", /*optional=*/true, "The hex-encoded scriptPubKey generated by the address"}, + {RPCResult::Type::BOOL, "isscript", /*optional=*/true, "If the key is a script"}, + {RPCResult::Type::BOOL, "iswitness", /*optional=*/true, "If the address is a witness address"}, + {RPCResult::Type::NUM, "witness_version", /*optional=*/true, "The version number of the witness program"}, + {RPCResult::Type::STR_HEX, "witness_program", /*optional=*/true, "The hex value of the witness program"}, + {RPCResult::Type::STR, "error", /*optional=*/true, "Error message, if any"}, {RPCResult::Type::ARR, "error_locations", /*optional=*/true, "Indices of likely error locations in address, if known (e.g. Bech32 errors)", { {RPCResult::Type::NUM, "index", "index of a potential error"}, @@ -301,7 +301,7 @@ static RPCHelpMan deriveaddresses() static RPCHelpMan verifymessage() { return RPCHelpMan{"verifymessage", - "\nVerify a signed message\n", + "Verify a signed message.", { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address to use for the signature."}, {"signature", RPCArg::Type::STR, RPCArg::Optional::NO, "The signature provided by the signer in base 64 encoding (see signmessage)."}, diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index bdcb339e14..756f6860f6 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -101,10 +101,10 @@ static RPCHelpMan getpeerinfo() { {RPCResult::Type::NUM, "id", "Peer index"}, {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, "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::NUM, "mapped_as", /* optional */ true, "The AS in the BGP route to the peer used for diversifying\n" + {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"}, {RPCResult::Type::ARR, "servicesnames", "the services offered, in human-readable form", @@ -120,9 +120,9 @@ static RPCHelpMan getpeerinfo() {RPCResult::Type::NUM, "bytesrecv", "The total bytes received"}, {RPCResult::Type::NUM_TIME, "conntime", "The " + UNIX_EPOCH_TIME + " of the connection"}, {RPCResult::Type::NUM, "timeoffset", "The time offset in seconds"}, - {RPCResult::Type::NUM, "pingtime", /* optional */ true, "ping time (if available)"}, - {RPCResult::Type::NUM, "minping", /* optional */ true, "minimum observed ping time (if any at all)"}, - {RPCResult::Type::NUM, "pingwait", /* optional */ true, "ping wait (if non-zero)"}, + {RPCResult::Type::NUM, "pingtime", /*optional=*/true, "ping time (if available)"}, + {RPCResult::Type::NUM, "minping", /*optional=*/true, "minimum observed ping time (if any at all)"}, + {RPCResult::Type::NUM, "pingwait", /*optional=*/true, "ping wait (if non-zero)"}, {RPCResult::Type::NUM, "version", "The peer version, such as 70001"}, {RPCResult::Type::STR, "subver", "The string version"}, {RPCResult::Type::BOOL, "inbound", "Inbound (true) or Outbound (false)"}, @@ -196,8 +196,8 @@ static RPCHelpMan getpeerinfo() obj.pushKV("services", strprintf("%016x", stats.nServices)); obj.pushKV("servicesnames", GetServicesNames(stats.nServices)); obj.pushKV("relaytxes", stats.fRelayTxes); - obj.pushKV("lastsend", stats.nLastSend); - obj.pushKV("lastrecv", stats.nLastRecv); + obj.pushKV("lastsend", count_seconds(stats.m_last_send)); + obj.pushKV("lastrecv", count_seconds(stats.m_last_recv)); obj.pushKV("last_transaction", stats.nLastTXTime); obj.pushKV("last_block", stats.nLastBlockTime); obj.pushKV("bytessent", stats.nSendBytes); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 5c161c7299..78c1596219 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -133,7 +133,7 @@ static RPCHelpMan getrawtransaction() RPCResult{"if verbose is set to true", RPCResult::Type::OBJ, "", "", { - {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::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)"}, @@ -154,7 +154,7 @@ static RPCHelpMan getrawtransaction() {RPCResult::Type::STR_HEX, "hex", "hex"}, }}, {RPCResult::Type::NUM, "sequence", "The script sequence number"}, - {RPCResult::Type::ARR, "txinwitness", /* optional */ true, "", + {RPCResult::Type::ARR, "txinwitness", /*optional=*/true, "", { {RPCResult::Type::STR_HEX, "hex", "hex-encoded witness data (if any)"}, }}, @@ -171,14 +171,14 @@ static RPCHelpMan getrawtransaction() {RPCResult::Type::STR, "asm", "the asm"}, {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, "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, "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\""}, } }, }, @@ -472,15 +472,15 @@ static RPCHelpMan decoderawtransaction() { {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_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::ARR, "txinwitness", /*optional=*/true, "", { {RPCResult::Type::STR_HEX, "hex", "hex-encoded witness data (if any)"}, }}, @@ -498,7 +498,7 @@ static RPCHelpMan decoderawtransaction() {RPCResult::Type::STR, "asm", "the asm"}, {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)"}, + {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, }}, }}, }}, @@ -552,7 +552,7 @@ static RPCHelpMan decodescript() { {RPCResult::Type::STR, "asm", "Script public key"}, {RPCResult::Type::STR, "type", "The output type (e.g. " + GetAllOutputTypes() + ")"}, - {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, "p2sh", /*optional=*/true, "address of P2SH script wrapping this redeem script (not returned for types that should not be wrapped)"}, {RPCResult::Type::OBJ, "segwit", /*optional=*/true, @@ -561,7 +561,7 @@ static RPCHelpMan decodescript() {RPCResult::Type::STR, "asm", "String representation of the script public key"}, {RPCResult::Type::STR_HEX, "hex", "Hex string of the script public key"}, {RPCResult::Type::STR, "type", "The type of the script public key (e.g. witness_v0_keyhash or witness_v0_scripthash)"}, - {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, "p2sh-segwit", "address of the P2SH script wrapping this witness redeem script"}, }}, }, @@ -800,7 +800,7 @@ static RPCHelpMan signrawtransactionwithkey() { {RPCResult::Type::STR_HEX, "hex", "The hex-encoded raw transaction with signature(s)"}, {RPCResult::Type::BOOL, "complete", "If the transaction has a complete set of signatures"}, - {RPCResult::Type::ARR, "errors", /* optional */ true, "Script verification errors (if there are any)", + {RPCResult::Type::ARR, "errors", /*optional=*/true, "Script verification errors (if there are any)", { {RPCResult::Type::OBJ, "", "", { @@ -947,15 +947,15 @@ static RPCHelpMan testmempoolaccept() { {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. " + {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::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)"}, + {RPCResult::Type::STR, "reject-reason", /*optional=*/true, "Rejection string (only present when 'allowed' is false)"}, }}, } }, @@ -1083,11 +1083,11 @@ static RPCHelpMan decodepsbt() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::OBJ, "non_witness_utxo", /* optional */ true, "Decoded network transaction for non-witness UTXOs", + {RPCResult::Type::OBJ, "non_witness_utxo", /*optional=*/true, "Decoded network transaction for non-witness UTXOs", { {RPCResult::Type::ELISION, "",""}, }}, - {RPCResult::Type::OBJ, "witness_utxo", /* optional */ true, "Transaction output for witness UTXOs", + {RPCResult::Type::OBJ, "witness_utxo", /*optional=*/true, "Transaction output for witness UTXOs", { {RPCResult::Type::NUM, "amount", "The value in " + CURRENCY_UNIT}, {RPCResult::Type::OBJ, "scriptPubKey", "", @@ -1095,27 +1095,27 @@ static RPCHelpMan decodepsbt() {RPCResult::Type::STR, "asm", "The asm"}, {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)"}, + {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, }}, }}, - {RPCResult::Type::OBJ_DYN, "partial_signatures", /* optional */ true, "", + {RPCResult::Type::OBJ_DYN, "partial_signatures", /*optional=*/true, "", { {RPCResult::Type::STR, "pubkey", "The public key and signature that corresponds to it."}, }}, - {RPCResult::Type::STR, "sighash", /* optional */ true, "The sighash type to be used"}, - {RPCResult::Type::OBJ, "redeem_script", /* optional */ true, "", + {RPCResult::Type::STR, "sighash", /*optional=*/true, "The sighash type to be used"}, + {RPCResult::Type::OBJ, "redeem_script", /*optional=*/true, "", { {RPCResult::Type::STR, "asm", "The asm"}, {RPCResult::Type::STR_HEX, "hex", "The hex"}, {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, }}, - {RPCResult::Type::OBJ, "witness_script", /* optional */ true, "", + {RPCResult::Type::OBJ, "witness_script", /*optional=*/true, "", { {RPCResult::Type::STR, "asm", "The asm"}, {RPCResult::Type::STR_HEX, "hex", "The hex"}, {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, }}, - {RPCResult::Type::ARR, "bip32_derivs", /* optional */ true, "", + {RPCResult::Type::ARR, "bip32_derivs", /*optional=*/true, "", { {RPCResult::Type::OBJ, "", "", { @@ -1124,16 +1124,16 @@ static RPCHelpMan decodepsbt() {RPCResult::Type::STR, "path", "The path"}, }}, }}, - {RPCResult::Type::OBJ, "final_scriptSig", /* optional */ true, "", + {RPCResult::Type::OBJ, "final_scriptSig", /*optional=*/true, "", { {RPCResult::Type::STR, "asm", "The asm"}, {RPCResult::Type::STR, "hex", "The hex"}, }}, - {RPCResult::Type::ARR, "final_scriptwitness", /* optional */ true, "", + {RPCResult::Type::ARR, "final_scriptwitness", /*optional=*/true, "", { {RPCResult::Type::STR_HEX, "", "hex-encoded witness data (if any)"}, }}, - {RPCResult::Type::OBJ_DYN, "unknown", /* optional */ true, "The unknown global fields", + {RPCResult::Type::OBJ_DYN, "unknown", /*optional=*/true, "The unknown global fields", { {RPCResult::Type::STR_HEX, "key", "(key-value pair) An unknown key-value pair"}, }}, @@ -1143,19 +1143,19 @@ static RPCHelpMan decodepsbt() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::OBJ, "redeem_script", /* optional */ true, "", + {RPCResult::Type::OBJ, "redeem_script", /*optional=*/true, "", { {RPCResult::Type::STR, "asm", "The asm"}, {RPCResult::Type::STR_HEX, "hex", "The hex"}, {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, }}, - {RPCResult::Type::OBJ, "witness_script", /* optional */ true, "", + {RPCResult::Type::OBJ, "witness_script", /*optional=*/true, "", { {RPCResult::Type::STR, "asm", "The asm"}, {RPCResult::Type::STR_HEX, "hex", "The hex"}, {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, }}, - {RPCResult::Type::ARR, "bip32_derivs", /* optional */ true, "", + {RPCResult::Type::ARR, "bip32_derivs", /*optional=*/true, "", { {RPCResult::Type::OBJ, "", "", { @@ -1164,13 +1164,13 @@ static RPCHelpMan decodepsbt() {RPCResult::Type::STR, "path", "The path"}, }}, }}, - {RPCResult::Type::OBJ_DYN, "unknown", /* optional */ true, "The unknown global fields", + {RPCResult::Type::OBJ_DYN, "unknown", /*optional=*/true, "The unknown global fields", { {RPCResult::Type::STR_HEX, "key", "(key-value pair) An unknown key-value pair"}, }}, }}, }}, - {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The transaction fee paid if all UTXOs slots in the PSBT have been filled."}, + {RPCResult::Type::STR_AMOUNT, "fee", /*optional=*/true, "The transaction fee paid if all UTXOs slots in the PSBT have been filled."}, } }, RPCExamples{ @@ -1438,8 +1438,8 @@ static RPCHelpMan finalizepsbt() RPCResult{ RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::STR, "psbt", /* optional */ true, "The base64-encoded partially signed transaction if not extracted"}, - {RPCResult::Type::STR_HEX, "hex", /* optional */ true, "The hex-encoded network transaction if extracted"}, + {RPCResult::Type::STR, "psbt", /*optional=*/true, "The base64-encoded partially signed transaction if not extracted"}, + {RPCResult::Type::STR_HEX, "hex", /*optional=*/true, "The hex-encoded network transaction if extracted"}, {RPCResult::Type::BOOL, "complete", "If the transaction has a complete set of signatures"}, } }, @@ -1799,33 +1799,33 @@ static RPCHelpMan analyzepsbt() RPCResult { RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::ARR, "inputs", /* optional */ true, "", + {RPCResult::Type::ARR, "inputs", /*optional=*/true, "", { {RPCResult::Type::OBJ, "", "", { {RPCResult::Type::BOOL, "has_utxo", "Whether a UTXO is provided"}, {RPCResult::Type::BOOL, "is_final", "Whether the input is finalized"}, - {RPCResult::Type::OBJ, "missing", /* optional */ true, "Things that are missing that are required to complete this input", + {RPCResult::Type::OBJ, "missing", /*optional=*/true, "Things that are missing that are required to complete this input", { - {RPCResult::Type::ARR, "pubkeys", /* optional */ true, "", + {RPCResult::Type::ARR, "pubkeys", /*optional=*/true, "", { {RPCResult::Type::STR_HEX, "keyid", "Public key ID, hash160 of the public key, of a public key whose BIP 32 derivation path is missing"}, }}, - {RPCResult::Type::ARR, "signatures", /* optional */ true, "", + {RPCResult::Type::ARR, "signatures", /*optional=*/true, "", { {RPCResult::Type::STR_HEX, "keyid", "Public key ID, hash160 of the public key, of a public key whose signature is missing"}, }}, - {RPCResult::Type::STR_HEX, "redeemscript", /* optional */ true, "Hash160 of the redeemScript that is missing"}, - {RPCResult::Type::STR_HEX, "witnessscript", /* optional */ true, "SHA256 of the witnessScript that is missing"}, + {RPCResult::Type::STR_HEX, "redeemscript", /*optional=*/true, "Hash160 of the redeemScript that is missing"}, + {RPCResult::Type::STR_HEX, "witnessscript", /*optional=*/true, "SHA256 of the witnessScript that is missing"}, }}, - {RPCResult::Type::STR, "next", /* optional */ true, "Role of the next person that this input needs to go to"}, + {RPCResult::Type::STR, "next", /*optional=*/true, "Role of the next person that this input needs to go to"}, }}, }}, - {RPCResult::Type::NUM, "estimated_vsize", /* optional */ true, "Estimated vsize of the final signed transaction"}, - {RPCResult::Type::STR_AMOUNT, "estimated_feerate", /* optional */ true, "Estimated feerate of the final signed transaction in " + CURRENCY_UNIT + "/kvB. Shown only if all UTXO slots in the PSBT have been filled"}, - {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The transaction fee paid. Shown only if all UTXO slots in the PSBT have been filled"}, + {RPCResult::Type::NUM, "estimated_vsize", /*optional=*/true, "Estimated vsize of the final signed transaction"}, + {RPCResult::Type::STR_AMOUNT, "estimated_feerate", /*optional=*/true, "Estimated feerate of the final signed transaction in " + CURRENCY_UNIT + "/kvB. Shown only if all UTXO slots in the PSBT have been filled"}, + {RPCResult::Type::STR_AMOUNT, "fee", /*optional=*/true, "The transaction fee paid. Shown only if all UTXO slots in the PSBT have been filled"}, {RPCResult::Type::STR, "next", "Role of the next person that this psbt needs to go to"}, - {RPCResult::Type::STR, "error", /* optional */ true, "Error message (if there is one)"}, + {RPCResult::Type::STR, "error", /*optional=*/true, "Error message (if there is one)"}, } }, RPCExamples { diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 683cf88af8..4a5cd0a4be 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -620,10 +620,9 @@ std::string RPCHelpMan::ToString() const ret += arg.ToString(/* oneline */ true); } if (was_optional) ret += " )"; - ret += "\n"; // Description - ret += m_description; + ret += "\n\n" + TrimString(m_description) + "\n"; // Arguments Sections sections; diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index ef18ed1385..f1abd5183a 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -53,7 +53,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) const CChainParams& chainparams = Params(); auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman); // Disable inactivity checks for this test to avoid interference - static_cast<ConnmanTestMsg*>(connman.get())->SetPeerConnectTimeout(99999); + static_cast<ConnmanTestMsg*>(connman.get())->SetPeerConnectTimeout(99999s); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, false); diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 44b98f7852..b6ecf1c492 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -360,7 +360,9 @@ FUZZ_TARGET_INIT(rpc, initialize_rpc) rpc_testing_setup->CallRPC(rpc_command, arguments); } catch (const UniValue& json_rpc_error) { const std::string error_msg{find_value(json_rpc_error, "message").get_str()}; - if (error_msg.find("Internal bug detected") != std::string::npos) { + // Once c++20 is allowed, starts_with can be used. + // if (error_msg.starts_with("Internal bug detected")) { + if (0 == error_msg.rfind("Internal bug detected", 0)) { // Only allow the intentional internal bug assert(error_msg.find("trigger_internal_bug") != std::string::npos); } diff --git a/src/test/util/net.h b/src/test/util/net.h index 2de6e712a2..ad1061a451 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -18,7 +18,7 @@ struct ConnmanTestMsg : public CConnman { using CConnman::CConnman; - void SetPeerConnectTimeout(int64_t timeout) + void SetPeerConnectTimeout(std::chrono::seconds timeout) { m_peer_connect_timeout = timeout; } diff --git a/src/util/check.h b/src/util/check.h index e60088a2c6..b12527546c 100644 --- a/src/util/check.h +++ b/src/util/check.h @@ -33,11 +33,11 @@ class NonFatalCheckError : public std::runtime_error do { \ if (!(condition)) { \ throw NonFatalCheckError( \ - strprintf("%s:%d (%s)\n" \ - "Internal bug detected: '%s'\n" \ + strprintf("Internal bug detected: '%s'\n" \ + "%s:%d (%s)\n" \ "You may report this issue here: %s\n", \ - __FILE__, __LINE__, __func__, \ (#condition), \ + __FILE__, __LINE__, __func__, \ PACKAGE_BUGREPORT)); \ } \ } while (false) diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index c0ca931364..ddf6a8b829 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -56,17 +56,14 @@ struct { * bound of the range. * @param const CAmount& cost_of_change This is the cost of creating and spending a change output. * This plus selection_target is the upper bound of the range. - * @param std::set<CInputCoin>& out_set -> This is an output parameter for the set of CInputCoins - * that have been selected. - * @param CAmount& value_ret -> This is an output parameter for the total value of the CInputCoins - * that were selected. + * @returns The result of this coin selection algorithm, or std::nullopt */ static const size_t TOTAL_TRIES = 100000; -bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret) +std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change) { - out_set.clear(); + SelectionResult result(selection_target); CAmount curr_value = 0; std::vector<bool> curr_selection; // select the utxo at this index @@ -80,7 +77,7 @@ bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selectio curr_available_value += utxo.GetSelectionAmount(); } if (curr_available_value < selection_target) { - return false; + return std::nullopt; } // Sort the utxo_pool @@ -156,25 +153,22 @@ bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selectio // Check for solution if (best_selection.empty()) { - return false; + return std::nullopt; } // Set output set - value_ret = 0; for (size_t i = 0; i < best_selection.size(); ++i) { if (best_selection.at(i)) { - util::insert(out_set, utxo_pool.at(i).m_outputs); - value_ret += utxo_pool.at(i).m_value; + result.AddInput(utxo_pool.at(i)); } } - return true; + return result; } -std::optional<std::pair<std::set<CInputCoin>, CAmount>> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value) +std::optional<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value) { - std::set<CInputCoin> out_set; - CAmount value_ret = 0; + SelectionResult result(target_value); std::vector<size_t> indexes; indexes.resize(utxo_pool.size()); @@ -186,10 +180,9 @@ std::optional<std::pair<std::set<CInputCoin>, CAmount>> SelectCoinsSRD(const std const OutputGroup& group = utxo_pool.at(i); Assume(group.GetSelectionAmount() > 0); selected_eff_value += group.GetSelectionAmount(); - value_ret += group.m_value; - util::insert(out_set, group.m_outputs); + result.AddInput(group); if (selected_eff_value >= target_value) { - return std::make_pair(out_set, value_ret); + return result; } } return std::nullopt; @@ -241,10 +234,9 @@ static void ApproximateBestSubset(const std::vector<OutputGroup>& groups, const } } -bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& groups, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet) +std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue) { - setCoinsRet.clear(); - nValueRet = 0; + SelectionResult result(nTargetValue); // List of values less than target std::optional<OutputGroup> lowest_larger; @@ -255,9 +247,8 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group for (const OutputGroup& group : groups) { if (group.GetSelectionAmount() == nTargetValue) { - util::insert(setCoinsRet, group.m_outputs); - nValueRet += group.m_value; - return true; + result.AddInput(group); + return result; } else if (group.GetSelectionAmount() < nTargetValue + MIN_CHANGE) { applicable_groups.push_back(group); nTotalLower += group.GetSelectionAmount(); @@ -268,17 +259,15 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group if (nTotalLower == nTargetValue) { for (const auto& group : applicable_groups) { - util::insert(setCoinsRet, group.m_outputs); - nValueRet += group.m_value; + result.AddInput(group); } - return true; + return result; } if (nTotalLower < nTargetValue) { - if (!lowest_larger) return false; - util::insert(setCoinsRet, lowest_larger->m_outputs); - nValueRet += lowest_larger->m_value; - return true; + if (!lowest_larger) return std::nullopt; + result.AddInput(*lowest_larger); + return result; } // Solve subset sum by stochastic approximation @@ -295,13 +284,11 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group // or the next bigger coin is closer), return the bigger coin if (lowest_larger && ((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || lowest_larger->GetSelectionAmount() <= nBest)) { - util::insert(setCoinsRet, lowest_larger->m_outputs); - nValueRet += lowest_larger->m_value; + result.AddInput(*lowest_larger); } else { for (unsigned int i = 0; i < applicable_groups.size(); i++) { if (vfBest[i]) { - util::insert(setCoinsRet, applicable_groups[i].m_outputs); - nValueRet += applicable_groups[i].m_value; + result.AddInput(applicable_groups[i]); } } @@ -316,7 +303,7 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group } } - return true; + return result; } /****************************************************************************** @@ -395,3 +382,51 @@ CAmount GetSelectionWaste(const std::set<CInputCoin>& inputs, CAmount change_cos return waste; } + +void SelectionResult::ComputeAndSetWaste(CAmount change_cost) +{ + m_waste = GetSelectionWaste(m_selected_inputs, change_cost, m_target, m_use_effective); +} + +CAmount SelectionResult::GetWaste() const +{ + Assume(m_waste != std::nullopt); + return *m_waste; +} + +CAmount SelectionResult::GetSelectedValue() const +{ + return std::accumulate(m_selected_inputs.cbegin(), m_selected_inputs.cend(), CAmount{0}, [](CAmount sum, const auto& coin) { return sum + coin.txout.nValue; }); +} + +void SelectionResult::Clear() +{ + m_selected_inputs.clear(); + m_waste.reset(); +} + +void SelectionResult::AddInput(const OutputGroup& group) +{ + util::insert(m_selected_inputs, group.m_outputs); + m_use_effective = !group.m_subtract_fee_outputs; +} + +const std::set<CInputCoin>& SelectionResult::GetInputSet() const +{ + return m_selected_inputs; +} + +std::vector<CInputCoin> SelectionResult::GetShuffledInputVector() const +{ + std::vector<CInputCoin> coins(m_selected_inputs.begin(), m_selected_inputs.end()); + Shuffle(coins.begin(), coins.end(), FastRandomContext()); + return coins; +} + +bool SelectionResult::operator<(SelectionResult other) const +{ + Assume(m_waste != std::nullopt); + Assume(other.m_waste != std::nullopt); + // 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()); +} diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index e7d467660f..637afcdb2b 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -187,6 +187,8 @@ struct OutputGroup * where excess = selected_effective_value - target * change_cost = effective_feerate * change_output_size + long_term_feerate * change_spend_size * + * Note this function is separate from SelectionResult for the tests. + * * @param[in] inputs The selected inputs * @param[in] change_cost The cost of creating change and spending it in the future. * Only used if there is change, in which case it must be positive. @@ -197,18 +199,55 @@ struct OutputGroup */ [[nodiscard]] CAmount GetSelectionWaste(const std::set<CInputCoin>& inputs, CAmount change_cost, CAmount target, bool use_effective_value = true); -bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret); +struct SelectionResult +{ +private: + /** Set of inputs selected by the algorithm to use in the transaction */ + std::set<CInputCoin> 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) */ + bool m_use_effective{false}; + /** The computed waste */ + std::optional<CAmount> m_waste; + +public: + explicit SelectionResult(const CAmount target) + : m_target(target) {} + + SelectionResult() = delete; + + /** Get the sum of the input values */ + [[nodiscard]] CAmount GetSelectedValue() const; + + void Clear(); + + void AddInput(const OutputGroup& group); + + /** Calculates and stores the waste for this selection via GetSelectionWaste */ + void ComputeAndSetWaste(CAmount change_cost); + [[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; + + bool operator<(SelectionResult other) const; +}; + +std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change); /** Select coins by Single Random Draw. OutputGroups are selected randomly from the eligible * outputs until the target is satisfied * * @param[in] utxo_pool The positive effective value OutputGroups eligible for selection * @param[in] target_value The target value to select for - * @returns If successful, a pair of set of outputs and total selected value, otherwise, std::nullopt + * @returns If successful, a SelectionResult, otherwise, std::nullopt */ -std::optional<std::pair<std::set<CInputCoin>, CAmount>> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value); +std::optional<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value); // Original coin selection algorithm as a fallback -bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& groups, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet); +std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue); #endif // BITCOIN_WALLET_COINSELECTION_H diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index edc4c26eeb..e570c18099 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -170,7 +170,7 @@ RPCHelpMan listaddressgroupings() { {RPCResult::Type::STR, "address", "The bitcoin address"}, {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT}, - {RPCResult::Type::STR, "label", /* optional */ true, "The label"}, + {RPCResult::Type::STR, "label", /*optional=*/true, "The label"}, }}, }}, } @@ -491,33 +491,33 @@ RPCHelpMan getaddressinfo() {RPCResult::Type::BOOL, "ismine", "If the address is yours."}, {RPCResult::Type::BOOL, "iswatchonly", "If the address is watchonly."}, {RPCResult::Type::BOOL, "solvable", "If we know how to spend coins sent to this address, ignoring the possible lack of private keys."}, - {RPCResult::Type::STR, "desc", /* optional */ true, "A descriptor for spending coins sent to this address (only when solvable)."}, - {RPCResult::Type::STR, "parent_desc", /* optional */ true, "The descriptor used to derive this address if this is a descriptor wallet"}, + {RPCResult::Type::STR, "desc", /*optional=*/true, "A descriptor for spending coins sent to this address (only when solvable)."}, + {RPCResult::Type::STR, "parent_desc", /*optional=*/true, "The descriptor used to derive this address if this is a descriptor wallet"}, {RPCResult::Type::BOOL, "isscript", "If the key is a script."}, {RPCResult::Type::BOOL, "ischange", "If the address was used for change output."}, {RPCResult::Type::BOOL, "iswitness", "If the address is a witness address."}, - {RPCResult::Type::NUM, "witness_version", /* optional */ true, "The version number of the witness program."}, - {RPCResult::Type::STR_HEX, "witness_program", /* optional */ true, "The hex value of the witness program."}, - {RPCResult::Type::STR, "script", /* optional */ true, "The output script type. Only if isscript is true and the redeemscript is known. Possible\n" + {RPCResult::Type::NUM, "witness_version", /*optional=*/true, "The version number of the witness program."}, + {RPCResult::Type::STR_HEX, "witness_program", /*optional=*/true, "The hex value of the witness program."}, + {RPCResult::Type::STR, "script", /*optional=*/true, "The output script type. Only if isscript is true and the redeemscript is known. Possible\n" "types: nonstandard, pubkey, pubkeyhash, scripthash, multisig, nulldata, witness_v0_keyhash,\n" "witness_v0_scripthash, witness_unknown."}, - {RPCResult::Type::STR_HEX, "hex", /* optional */ true, "The redeemscript for the p2sh address."}, - {RPCResult::Type::ARR, "pubkeys", /* optional */ true, "Array of pubkeys associated with the known redeemscript (only if script is multisig).", + {RPCResult::Type::STR_HEX, "hex", /*optional=*/true, "The redeemscript for the p2sh address."}, + {RPCResult::Type::ARR, "pubkeys", /*optional=*/true, "Array of pubkeys associated with the known redeemscript (only if script is multisig).", { {RPCResult::Type::STR, "pubkey", ""}, }}, - {RPCResult::Type::NUM, "sigsrequired", /* optional */ true, "The number of signatures required to spend multisig output (only if script is multisig)."}, - {RPCResult::Type::STR_HEX, "pubkey", /* optional */ true, "The hex value of the raw public key for single-key addresses (possibly embedded in P2SH or P2WSH)."}, - {RPCResult::Type::OBJ, "embedded", /* optional */ true, "Information about the address embedded in P2SH or P2WSH, if relevant and known.", + {RPCResult::Type::NUM, "sigsrequired", /*optional=*/true, "The number of signatures required to spend multisig output (only if script is multisig)."}, + {RPCResult::Type::STR_HEX, "pubkey", /*optional=*/true, "The hex value of the raw public key for single-key addresses (possibly embedded in P2SH or P2WSH)."}, + {RPCResult::Type::OBJ, "embedded", /*optional=*/true, "Information about the address embedded in P2SH or P2WSH, if relevant and known.", { {RPCResult::Type::ELISION, "", "Includes all getaddressinfo output fields for the embedded address, excluding metadata (timestamp, hdkeypath, hdseedid)\n" "and relation to the wallet (ismine, iswatchonly)."}, }}, - {RPCResult::Type::BOOL, "iscompressed", /* optional */ true, "If the pubkey is compressed."}, - {RPCResult::Type::NUM_TIME, "timestamp", /* optional */ true, "The creation time of the key, if available, expressed in " + UNIX_EPOCH_TIME + "."}, - {RPCResult::Type::STR, "hdkeypath", /* optional */ true, "The HD keypath, if the key is HD and available."}, - {RPCResult::Type::STR_HEX, "hdseedid", /* optional */ true, "The Hash160 of the HD seed."}, - {RPCResult::Type::STR_HEX, "hdmasterfingerprint", /* optional */ true, "The fingerprint of the master key."}, + {RPCResult::Type::BOOL, "iscompressed", /*optional=*/true, "If the pubkey is compressed."}, + {RPCResult::Type::NUM_TIME, "timestamp", /*optional=*/true, "The creation time of the key, if available, expressed in " + UNIX_EPOCH_TIME + "."}, + {RPCResult::Type::STR, "hdkeypath", /*optional=*/true, "The HD keypath, if the key is HD and available."}, + {RPCResult::Type::STR_HEX, "hdseedid", /*optional=*/true, "The Hash160 of the HD seed."}, + {RPCResult::Type::STR_HEX, "hdmasterfingerprint", /*optional=*/true, "The fingerprint of the master key."}, {RPCResult::Type::ARR, "labels", "Array of labels associated with the address. Currently limited to one label but returned\n" "as an array to keep the API stable if multiple labels are enabled in the future.", { diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 135c3a15f2..e3daae9cea 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1299,11 +1299,11 @@ RPCHelpMan importmulti() {RPCResult::Type::OBJ, "", "", { {RPCResult::Type::BOOL, "success", ""}, - {RPCResult::Type::ARR, "warnings", /* optional */ true, "", + {RPCResult::Type::ARR, "warnings", /*optional=*/true, "", { {RPCResult::Type::STR, "", ""}, }}, - {RPCResult::Type::OBJ, "error", /* optional */ true, "", + {RPCResult::Type::OBJ, "error", /*optional=*/true, "", { {RPCResult::Type::ELISION, "", "JSONRPC error"}, }}, @@ -1607,11 +1607,11 @@ RPCHelpMan importdescriptors() {RPCResult::Type::OBJ, "", "", { {RPCResult::Type::BOOL, "success", ""}, - {RPCResult::Type::ARR, "warnings", /* optional */ true, "", + {RPCResult::Type::ARR, "warnings", /*optional=*/true, "", { {RPCResult::Type::STR, "", ""}, }}, - {RPCResult::Type::OBJ, "error", /* optional */ true, "", + {RPCResult::Type::OBJ, "error", /*optional=*/true, "", { {RPCResult::Type::ELISION, "", "JSONRPC error"}, }}, diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 326f0f088b..f3294b4570 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -443,9 +443,9 @@ RPCHelpMan getbalances() {RPCResult::Type::STR_AMOUNT, "trusted", "trusted balance (outputs created by the wallet or confirmed outputs)"}, {RPCResult::Type::STR_AMOUNT, "untrusted_pending", "untrusted pending balance (outputs created by others that are in the mempool)"}, {RPCResult::Type::STR_AMOUNT, "immature", "balance from immature coinbase outputs"}, - {RPCResult::Type::STR_AMOUNT, "used", /* optional */ true, "(only present if avoid_reuse is set) balance from coins sent to addresses that were previously spent from (potentially privacy violating)"}, + {RPCResult::Type::STR_AMOUNT, "used", /*optional=*/true, "(only present if avoid_reuse is set) balance from coins sent to addresses that were previously spent from (potentially privacy violating)"}, }}, - {RPCResult::Type::OBJ, "watchonly", /* optional */ true, "watchonly balances (not present if wallet does not watch anything)", + {RPCResult::Type::OBJ, "watchonly", /*optional=*/true, "watchonly balances (not present if wallet does not watch anything)", { {RPCResult::Type::STR_AMOUNT, "trusted", "trusted balance (outputs created by the wallet or confirmed outputs)"}, {RPCResult::Type::STR_AMOUNT, "untrusted_pending", "untrusted pending balance (outputs created by others that are in the mempool)"}, @@ -529,20 +529,20 @@ RPCHelpMan listunspent() { {RPCResult::Type::STR_HEX, "txid", "the transaction id"}, {RPCResult::Type::NUM, "vout", "the vout value"}, - {RPCResult::Type::STR, "address", /* optional */ true, "the bitcoin address"}, - {RPCResult::Type::STR, "label", /* optional */ true, "The associated label, or \"\" for the default label"}, + {RPCResult::Type::STR, "address", /*optional=*/true, "the bitcoin address"}, + {RPCResult::Type::STR, "label", /*optional=*/true, "The associated label, or \"\" for the default label"}, {RPCResult::Type::STR, "scriptPubKey", "the script key"}, {RPCResult::Type::STR_AMOUNT, "amount", "the transaction output amount in " + CURRENCY_UNIT}, {RPCResult::Type::NUM, "confirmations", "The number of confirmations"}, - {RPCResult::Type::NUM, "ancestorcount", /* optional */ true, "The number of in-mempool ancestor transactions, including this one (if transaction is in the mempool)"}, - {RPCResult::Type::NUM, "ancestorsize", /* optional */ true, "The virtual transaction size of in-mempool ancestors, including this one (if transaction is in the mempool)"}, - {RPCResult::Type::STR_AMOUNT, "ancestorfees", /* optional */ true, "The total fees of in-mempool ancestors (including this one) with fee deltas used for mining priority in " + CURRENCY_ATOM + " (if transaction is in the mempool)"}, - {RPCResult::Type::STR_HEX, "redeemScript", /* optional */ true, "The redeemScript if scriptPubKey is P2SH"}, - {RPCResult::Type::STR, "witnessScript", /* optional */ true, "witnessScript if the scriptPubKey is P2WSH or P2SH-P2WSH"}, + {RPCResult::Type::NUM, "ancestorcount", /*optional=*/true, "The number of in-mempool ancestor transactions, including this one (if transaction is in the mempool)"}, + {RPCResult::Type::NUM, "ancestorsize", /*optional=*/true, "The virtual transaction size of in-mempool ancestors, including this one (if transaction is in the mempool)"}, + {RPCResult::Type::STR_AMOUNT, "ancestorfees", /*optional=*/true, "The total fees of in-mempool ancestors (including this one) with fee deltas used for mining priority in " + CURRENCY_ATOM + " (if transaction is in the mempool)"}, + {RPCResult::Type::STR_HEX, "redeemScript", /*optional=*/true, "The redeemScript if scriptPubKey is P2SH"}, + {RPCResult::Type::STR, "witnessScript", /*optional=*/true, "witnessScript if the scriptPubKey is P2WSH or P2SH-P2WSH"}, {RPCResult::Type::BOOL, "spendable", "Whether we have the private keys to spend this output"}, {RPCResult::Type::BOOL, "solvable", "Whether we know how to spend this output, ignoring the lack of keys"}, - {RPCResult::Type::BOOL, "reused", /* optional */ true, "(only present if avoid_reuse is set) Whether this output is reused/dirty (sent to an address that was previously spent from)"}, - {RPCResult::Type::STR, "desc", /* optional */ true, "(only when solvable) A descriptor for spending this output"}, + {RPCResult::Type::BOOL, "reused", /*optional=*/true, "(only present if avoid_reuse is set) Whether this output is reused/dirty (sent to an address that was previously spent from)"}, + {RPCResult::Type::STR, "desc", /*optional=*/true, "(only when solvable) A descriptor for spending this output"}, {RPCResult::Type::BOOL, "safe", "Whether this output is considered safe to spend. Unconfirmed transactions\n" "from outside keys and unconfirmed replacement transactions are considered unsafe\n" "and are not eligible for spending by fundrawtransaction and sendtoaddress."}, diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 62b0a839c1..978174b340 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -723,7 +723,7 @@ RPCHelpMan signrawtransactionwithwallet() { {RPCResult::Type::STR_HEX, "hex", "The hex-encoded raw transaction with signature(s)"}, {RPCResult::Type::BOOL, "complete", "If the transaction has a complete set of signatures"}, - {RPCResult::Type::ARR, "errors", /* optional */ true, "Script verification errors (if there are any)", + {RPCResult::Type::ARR, "errors", /*optional=*/true, "Script verification errors (if there are any)", { {RPCResult::Type::OBJ, "", "", { @@ -1027,9 +1027,9 @@ RPCHelpMan send() 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"} + {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{"" diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index cf828d6339..8a1c0885ac 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -240,7 +240,7 @@ RPCHelpMan listreceivedbyaddress() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction"}, + {RPCResult::Type::BOOL, "involvesWatchonly", /*optional=*/true, "Only returns true if imported addresses were involved in transaction"}, {RPCResult::Type::STR, "address", "The receiving address"}, {RPCResult::Type::STR_AMOUNT, "amount", "The total amount in " + CURRENCY_UNIT + " received by the address"}, {RPCResult::Type::NUM, "confirmations", "The number of confirmations of the most recent transaction included"}, @@ -292,7 +292,7 @@ RPCHelpMan listreceivedbylabel() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction"}, + {RPCResult::Type::BOOL, "involvesWatchonly", /*optional=*/true, "Only returns true if imported addresses were involved in transaction"}, {RPCResult::Type::STR_AMOUNT, "amount", "The total amount received by addresses with this label"}, {RPCResult::Type::NUM, "confirmations", "The number of confirmations of the most recent transaction included"}, {RPCResult::Type::STR, "label", "The label of the receiving address. The default label is \"\""}, @@ -422,25 +422,25 @@ static const std::vector<RPCResult> TransactionDescriptionString() { return{{RPCResult::Type::NUM, "confirmations", "The number of confirmations for the transaction. Negative confirmations means the\n" "transaction conflicted that many blocks ago."}, - {RPCResult::Type::BOOL, "generated", /* optional */ true, "Only present if the transaction's only input is a coinbase one."}, - {RPCResult::Type::BOOL, "trusted", /* optional */ true, "Whether we consider the transaction to be trusted and safe to spend from.\n" + {RPCResult::Type::BOOL, "generated", /*optional=*/true, "Only present if the transaction's only input is a coinbase one."}, + {RPCResult::Type::BOOL, "trusted", /*optional=*/true, "Whether we consider the transaction to be trusted and safe to spend from.\n" "Only present when the transaction has 0 confirmations (or negative confirmations, if conflicted)."}, - {RPCResult::Type::STR_HEX, "blockhash", /* optional */ true, "The block hash containing the transaction."}, - {RPCResult::Type::NUM, "blockheight", /* optional */ true, "The block height containing the transaction."}, - {RPCResult::Type::NUM, "blockindex", /* optional */ true, "The index of the transaction in the block that includes it."}, - {RPCResult::Type::NUM_TIME, "blocktime", /* optional */ true, "The block time expressed in " + UNIX_EPOCH_TIME + "."}, + {RPCResult::Type::STR_HEX, "blockhash", /*optional=*/true, "The block hash containing the transaction."}, + {RPCResult::Type::NUM, "blockheight", /*optional=*/true, "The block height containing the transaction."}, + {RPCResult::Type::NUM, "blockindex", /*optional=*/true, "The index of the transaction in the block that includes it."}, + {RPCResult::Type::NUM_TIME, "blocktime", /*optional=*/true, "The block time expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::STR_HEX, "txid", "The transaction id."}, {RPCResult::Type::ARR, "walletconflicts", "Conflicting transaction ids.", { {RPCResult::Type::STR_HEX, "txid", "The transaction id."}, }}, - {RPCResult::Type::STR_HEX, "replaced_by_txid", /* optional */ true, "The txid if this tx was replaced."}, - {RPCResult::Type::STR_HEX, "replaces_txid", /* optional */ true, "The txid if the tx replaces one."}, - {RPCResult::Type::STR, "comment", /* optional */ true, ""}, - {RPCResult::Type::STR, "to", /* optional */ true, "If a comment to is associated with the transaction."}, + {RPCResult::Type::STR_HEX, "replaced_by_txid", /*optional=*/true, "The txid if this tx was replaced."}, + {RPCResult::Type::STR_HEX, "replaces_txid", /*optional=*/true, "The txid if the tx replaces one."}, + {RPCResult::Type::STR, "comment", /*optional=*/true, ""}, + {RPCResult::Type::STR, "to", /*optional=*/true, "If a comment to is associated with the transaction."}, {RPCResult::Type::NUM_TIME, "time", "The transaction time expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::NUM_TIME, "timereceived", "The time received expressed in " + UNIX_EPOCH_TIME + "."}, - {RPCResult::Type::STR, "comment", /* optional */ true, "If a comment is associated with the transaction, only present if not empty."}, + {RPCResult::Type::STR, "comment", /*optional=*/true, "If a comment is associated with the transaction, only present if not empty."}, {RPCResult::Type::STR, "bip125-replaceable", "(\"yes|no|unknown\") Whether this transaction could be replaced due to BIP125 (replace-by-fee);\n" "may be unknown for unconfirmed transactions not in the mempool."}}; } @@ -462,7 +462,7 @@ RPCHelpMan listtransactions() { {RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>( { - {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction."}, + {RPCResult::Type::BOOL, "involvesWatchonly", /*optional=*/true, "Only returns true if imported addresses were involved in transaction."}, {RPCResult::Type::STR, "address", "The bitcoin address of the transaction."}, {RPCResult::Type::STR, "category", "The transaction category.\n" "\"send\" Transactions sent.\n" @@ -472,14 +472,14 @@ RPCHelpMan listtransactions() "\"orphan\" Orphaned coinbase transactions received."}, {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT + ". This is negative for the 'send' category, and is positive\n" "for all other categories"}, - {RPCResult::Type::STR, "label", /* optional */ true, "A comment for the address/transaction, if any"}, + {RPCResult::Type::STR, "label", /*optional=*/true, "A comment for the address/transaction, if any"}, {RPCResult::Type::NUM, "vout", "the vout value"}, - {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" + {RPCResult::Type::STR_AMOUNT, "fee", /*optional=*/true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" "'send' category of transactions."}, }, TransactionDescriptionString()), { - {RPCResult::Type::BOOL, "abandoned", /* optional */ true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" + {RPCResult::Type::BOOL, "abandoned", /*optional=*/true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" "'send' category of transactions."}, })}, } @@ -576,7 +576,7 @@ RPCHelpMan listsinceblock() { {RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>( { - {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction."}, + {RPCResult::Type::BOOL, "involvesWatchonly", /*optional=*/true, "Only returns true if imported addresses were involved in transaction."}, {RPCResult::Type::STR, "address", "The bitcoin address of the transaction."}, {RPCResult::Type::STR, "category", "The transaction category.\n" "\"send\" Transactions sent.\n" @@ -587,17 +587,17 @@ RPCHelpMan listsinceblock() {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT + ". This is negative for the 'send' category, and is positive\n" "for all other categories"}, {RPCResult::Type::NUM, "vout", "the vout value"}, - {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" + {RPCResult::Type::STR_AMOUNT, "fee", /*optional=*/true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" "'send' category of transactions."}, }, TransactionDescriptionString()), { - {RPCResult::Type::BOOL, "abandoned", /* optional */ true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" + {RPCResult::Type::BOOL, "abandoned", /*optional=*/true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" "'send' category of transactions."}, - {RPCResult::Type::STR, "label", /* optional */ true, "A comment for the address/transaction, if any"}, + {RPCResult::Type::STR, "label", /*optional=*/true, "A comment for the address/transaction, if any"}, })}, }}, - {RPCResult::Type::ARR, "removed", /* optional */ true, "<structure is the same as \"transactions\" above, only present if include_removed=true>\n" + {RPCResult::Type::ARR, "removed", /*optional=*/true, "<structure is the same as \"transactions\" above, only present if include_removed=true>\n" "Note: transactions that were re-added in the active chain will appear as-is in this array, and may thus have a positive confirmation count." , {{RPCResult::Type::ELISION, "", ""},}}, {RPCResult::Type::STR_HEX, "lastblock", "The hash of the block (target_confirmations-1) from the best block on the main chain, or the genesis hash if the referenced block does not exist yet. This is typically used to feed back into listsinceblock the next time you call it. So you would generally use a target_confirmations of say 6, so you will be continually re-notified of transactions until they've reached 6 confirmations plus any new ones"}, @@ -710,7 +710,7 @@ RPCHelpMan gettransaction() RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>( { {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT}, - {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" + {RPCResult::Type::STR_AMOUNT, "fee", /*optional=*/true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the\n" "'send' category of transactions."}, }, TransactionDescriptionString()), @@ -719,8 +719,8 @@ RPCHelpMan gettransaction() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "involvesWatchonly", /* optional */ true, "Only returns true if imported addresses were involved in transaction."}, - {RPCResult::Type::STR, "address", /* optional */ true, "The bitcoin address involved in the transaction."}, + {RPCResult::Type::BOOL, "involvesWatchonly", /*optional=*/true, "Only returns true if imported addresses were involved in transaction."}, + {RPCResult::Type::STR, "address", /*optional=*/true, "The bitcoin address involved in the transaction."}, {RPCResult::Type::STR, "category", "The transaction category.\n" "\"send\" Transactions sent.\n" "\"receive\" Non-coinbase transactions received.\n" @@ -728,16 +728,16 @@ RPCHelpMan gettransaction() "\"immature\" Coinbase transactions received with 100 or fewer confirmations.\n" "\"orphan\" Orphaned coinbase transactions received."}, {RPCResult::Type::STR_AMOUNT, "amount", "The amount in " + CURRENCY_UNIT}, - {RPCResult::Type::STR, "label", /* optional */ true, "A comment for the address/transaction, if any"}, + {RPCResult::Type::STR, "label", /*optional=*/true, "A comment for the address/transaction, if any"}, {RPCResult::Type::NUM, "vout", "the vout value"}, - {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n" + {RPCResult::Type::STR_AMOUNT, "fee", /*optional=*/true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n" "'send' category of transactions."}, - {RPCResult::Type::BOOL, "abandoned", /* optional */ true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" + {RPCResult::Type::BOOL, "abandoned", /*optional=*/true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" "'send' category of transactions."}, }}, }}, {RPCResult::Type::STR_HEX, "hex", "Raw data for transaction"}, - {RPCResult::Type::OBJ, "decoded", /* optional */ true, "The decoded transaction (only present when `verbose` is passed)", + {RPCResult::Type::OBJ, "decoded", /*optional=*/true, "The decoded transaction (only present when `verbose` is passed)", { {RPCResult::Type::ELISION, "", "Equivalent to the RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed."}, }}, diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 99ffe5d964..09f50137c6 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -42,12 +42,12 @@ static RPCHelpMan getwalletinfo() {RPCResult::Type::STR_AMOUNT, "unconfirmed_balance", "DEPRECATED. Identical to getbalances().mine.untrusted_pending"}, {RPCResult::Type::STR_AMOUNT, "immature_balance", "DEPRECATED. Identical to getbalances().mine.immature"}, {RPCResult::Type::NUM, "txcount", "the total number of transactions in the wallet"}, - {RPCResult::Type::NUM_TIME, "keypoololdest", /* optional */ true, "the " + UNIX_EPOCH_TIME + " of the oldest pre-generated key in the key pool. Legacy wallets only."}, + {RPCResult::Type::NUM_TIME, "keypoololdest", /*optional=*/true, "the " + UNIX_EPOCH_TIME + " of the oldest pre-generated key in the key pool. Legacy wallets only."}, {RPCResult::Type::NUM, "keypoolsize", "how many new keys are pre-generated (only counts external keys)"}, - {RPCResult::Type::NUM, "keypoolsize_hd_internal", /* optional */ true, "how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)"}, - {RPCResult::Type::NUM_TIME, "unlocked_until", /* optional */ true, "the " + UNIX_EPOCH_TIME + " until which the wallet is unlocked for transfers, or 0 if the wallet is locked (only present for passphrase-encrypted wallets)"}, + {RPCResult::Type::NUM, "keypoolsize_hd_internal", /*optional=*/true, "how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)"}, + {RPCResult::Type::NUM_TIME, "unlocked_until", /*optional=*/true, "the " + UNIX_EPOCH_TIME + " until which the wallet is unlocked for transfers, or 0 if the wallet is locked (only present for passphrase-encrypted wallets)"}, {RPCResult::Type::STR_AMOUNT, "paytxfee", "the transaction fee configuration, set in " + CURRENCY_UNIT + "/kvB"}, - {RPCResult::Type::STR_HEX, "hdseedid", /* optional */ true, "the Hash160 of the HD seed (only present when HD is enabled)"}, + {RPCResult::Type::STR_HEX, "hdseedid", /*optional=*/true, "the Hash160 of the HD seed (only present when HD is enabled)"}, {RPCResult::Type::BOOL, "private_keys_enabled", "false if privatekeys are disabled for this wallet (enforced watch-only wallet)"}, {RPCResult::Type::BOOL, "avoid_reuse", "whether this wallet tracks clean/dirty coins in terms of reuse"}, {RPCResult::Type::OBJ, "scanning", "current scanning details, or false if no scan is in progress", @@ -524,8 +524,8 @@ static RPCHelpMan upgradewallet() {RPCResult::Type::STR, "wallet_name", "Name of wallet this operation was performed on"}, {RPCResult::Type::NUM, "previous_version", "Version of wallet before this operation"}, {RPCResult::Type::NUM, "current_version", "Version of wallet after this operation"}, - {RPCResult::Type::STR, "result", /* optional */ true, "Description of result, if no error"}, - {RPCResult::Type::STR, "error", /* optional */ true, "Error message (if there is one)"} + {RPCResult::Type::STR, "result", /*optional=*/true, "Description of result, if no error"}, + {RPCResult::Type::STR, "error", /*optional=*/true, "Error message (if there is one)"} }, }, RPCExamples{ diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 5470177440..5d9cc7bf6b 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -373,81 +373,72 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C return groups_out; } -bool AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<COutput> coins, - std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params) +std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<COutput> coins, + const CoinSelectionParams& coin_selection_params) { - setCoinsRet.clear(); - nValueRet = 0; - // Vector of results for use with waste calculation - // In order: calculated waste, selected inputs, selected input value (sum of input values) - // TODO: Use a struct representing the selection result - std::vector<std::tuple<CAmount, std::set<CInputCoin>, CAmount>> results; + // Vector of results. We will choose the best one based on waste. + std::vector<SelectionResult> results; // Note that unlike KnapsackSolver, we do not include the fee for creating a change output as BnB will not create a change output. std::vector<OutputGroup> positive_groups = GroupOutputs(wallet, coins, coin_selection_params, eligibility_filter, true /* positive_only */); - std::set<CInputCoin> bnb_coins; - CAmount bnb_value; - if (SelectCoinsBnB(positive_groups, nTargetValue, coin_selection_params.m_cost_of_change, bnb_coins, bnb_value)) { - const auto waste = GetSelectionWaste(bnb_coins, /* cost of change */ CAmount(0), nTargetValue, !coin_selection_params.m_subtract_fee_outputs); - results.emplace_back(std::make_tuple(waste, std::move(bnb_coins), bnb_value)); + if (auto bnb_result{SelectCoinsBnB(positive_groups, nTargetValue, coin_selection_params.m_cost_of_change)}) { + bnb_result->ComputeAndSetWaste(CAmount(0)); + results.push_back(*bnb_result); } // The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here. 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. - std::set<CInputCoin> knapsack_coins; - CAmount knapsack_value; - if (KnapsackSolver(nTargetValue + coin_selection_params.m_change_fee, all_groups, knapsack_coins, knapsack_value)) { - const auto waste = GetSelectionWaste(knapsack_coins, coin_selection_params.m_cost_of_change, nTargetValue + coin_selection_params.m_change_fee, !coin_selection_params.m_subtract_fee_outputs); - results.emplace_back(std::make_tuple(waste, std::move(knapsack_coins), knapsack_value)); + if (auto knapsack_result{KnapsackSolver(all_groups, nTargetValue + coin_selection_params.m_change_fee)}) { + 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; - auto srd_result = SelectCoinsSRD(positive_groups, srd_target); - if (srd_result != std::nullopt) { - const auto waste = GetSelectionWaste(srd_result->first, coin_selection_params.m_cost_of_change, srd_target, !coin_selection_params.m_subtract_fee_outputs); - results.emplace_back(std::make_tuple(waste, std::move(srd_result->first), srd_result->second)); + if (auto srd_result{SelectCoinsSRD(positive_groups, srd_target)}) { + srd_result->ComputeAndSetWaste(coin_selection_params.m_cost_of_change); + results.push_back(*srd_result); } if (results.size() == 0) { // No solution found - return false; + return std::nullopt; } // Choose the result with the least waste // If the waste is the same, choose the one which spends more inputs. - const auto& best_result = std::min_element(results.begin(), results.end(), [](const auto& a, const auto& b) { - return std::get<0>(a) < std::get<0>(b) || (std::get<0>(a) == std::get<0>(b) && std::get<1>(a).size() > std::get<1>(b).size()); - }); - setCoinsRet = std::get<1>(*best_result); - nValueRet = std::get<2>(*best_result); - return true; + auto& best_result = *std::min_element(results.begin(), results.end()); + return best_result; } -bool SelectCoins(const CWallet& wallet, const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CCoinControl& coin_control, CoinSelectionParams& coin_selection_params) +std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) { std::vector<COutput> vCoins(vAvailableCoins); CAmount value_to_select = nTargetValue; + OutputGroup preset_inputs(coin_selection_params); + // coin control -> return all selected outputs (we want all selected to go into the transaction for sure) if (coin_control.HasSelected() && !coin_control.fAllowOtherInputs) { - for (const COutput& out : vCoins) - { - if (!out.fSpendable) - continue; - nValueRet += out.tx->tx->vout[out.i].nValue; - setCoinsRet.insert(out.GetInputCoin()); + 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. + * 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); } - return (nValueRet >= nTargetValue); + SelectionResult result(nTargetValue); + result.AddInput(preset_inputs); + if (result.GetSelectedValue() < nTargetValue) return std::nullopt; + return result; } // calculate value from preset inputs and store them std::set<CInputCoin> setPresetCoins; - CAmount nValueFromPresetInputs = 0; std::vector<COutPoint> vPresetInputs; coin_control.ListSelected(vPresetInputs); @@ -459,7 +450,7 @@ bool SelectCoins(const CWallet& wallet, const std::vector<COutput>& vAvailableCo const CWalletTx& wtx = it->second; // Clearly invalid input, fail if (wtx.tx->vout.size() <= outpoint.n) { - return false; + return std::nullopt; } input_bytes = GetTxSpendSize(wallet, wtx, outpoint.n, false); txout = wtx.tx->vout.at(outpoint.n); @@ -468,15 +459,14 @@ bool SelectCoins(const CWallet& wallet, const std::vector<COutput>& vAvailableCo // The input is external. We either did not find the tx in mapWallet, or we did but couldn't compute the input size with wallet data if (!coin_control.GetExternalOutput(outpoint, txout)) { // Not ours, and we don't have solving data. - return false; + return std::nullopt; } input_bytes = CalculateMaximumSignedInputSize(txout, &coin_control.m_external_provider, /* use_max_sig */ true); } CInputCoin coin(outpoint, txout, input_bytes); - nValueFromPresetInputs += coin.txout.nValue; if (coin.m_input_bytes == -1) { - return false; // Not solvable, can't estimate size for fee + 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); if (coin_selection_params.m_subtract_fee_outputs) { @@ -485,6 +475,10 @@ bool SelectCoins(const CWallet& wallet, const std::vector<COutput>& vAvailableCo value_to_select -= coin.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. + * 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); } // remove preset inputs from vCoins so that Coin Selection doesn't pick them. @@ -515,60 +509,62 @@ bool SelectCoins(const CWallet& wallet, const std::vector<COutput>& vAvailableCo // Coin Selection attempts to select inputs from a pool of eligible UTXOs to fund the // transaction at a target feerate. If an attempt fails, more attempts may be made using a more // permissive CoinEligibilityFilter. - const bool res = [&] { + std::optional<SelectionResult> res = [&] { // Pre-selected inputs already cover the target amount. - if (value_to_select <= 0) return true; + if (value_to_select <= 0) return std::make_optional(SelectionResult(nTargetValue)); // If possible, fund the transaction with confirmed UTXOs only. Prefer at least six // confirmations on outputs received from other wallets and only spend confirmed change. - if (AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 6, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params)) return true; - if (AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 1, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params)) return true; + if (auto r1{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 6, 0), vCoins, coin_selection_params)}) return r1; + if (auto r2{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 1, 0), vCoins, coin_selection_params)}) return r2; // Fall back to using zero confirmation change (but with as few ancestors in the mempool as // possible) if we cannot fund the transaction otherwise. if (wallet.m_spend_zero_conf_change) { - if (AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, 2), vCoins, setCoinsRet, nValueRet, coin_selection_params)) return true; - if (AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)), - vCoins, setCoinsRet, nValueRet, coin_selection_params)) { - return true; + if (auto r3{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, 2), vCoins, coin_selection_params)}) return r3; + if (auto r4{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)), + vCoins, coin_selection_params)}) { + return r4; } - if (AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2), - vCoins, setCoinsRet, nValueRet, coin_selection_params)) { - return true; + if (auto r5{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2), + vCoins, coin_selection_params)}) { + return r5; } // If partial groups are allowed, relax the requirement of spending OutputGroups (groups // of UTXOs sent to the same address, which are obviously controlled by a single wallet) // in their entirety. - if (AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1, true /* include_partial_groups */), - vCoins, setCoinsRet, nValueRet, coin_selection_params)) { - return true; + if (auto r6{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1, true /* include_partial_groups */), + vCoins, coin_selection_params)}) { + return r6; } // Try with unsafe inputs if they are allowed. This may spend unconfirmed outputs // received from other wallets. - if (coin_control.m_include_unsafe_inputs - && AttemptSelection(wallet, value_to_select, + if (coin_control.m_include_unsafe_inputs) { + if (auto r7{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0 /* conf_mine */, 0 /* conf_theirs */, max_ancestors-1, max_descendants-1, true /* include_partial_groups */), - vCoins, setCoinsRet, nValueRet, coin_selection_params)) { - return true; + vCoins, coin_selection_params)}) { + return r7; + } } // Try with unlimited ancestors/descendants. The transaction will still need to meet // mempool ancestor/descendant policy to be accepted to mempool and broadcasted, but // OutputGroups use heuristics that may overestimate ancestor/descendant counts. - if (!fRejectLongChains && AttemptSelection(wallet, value_to_select, + if (!fRejectLongChains) { + if (auto r8{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max(), std::numeric_limits<uint64_t>::max(), true /* include_partial_groups */), - vCoins, setCoinsRet, nValueRet, coin_selection_params)) { - return true; + vCoins, coin_selection_params)}) { + return r8; + } } } // Coin Selection failed. - return false; + return std::optional<SelectionResult>(); }(); - // AttemptSelection clears setCoinsRet, so add the preset inputs from coin_control to the coinset - util::insert(setCoinsRet, setPresetCoins); + if (!res) return std::nullopt; - // add preset inputs to the total value selected - nValueRet += nValueFromPresetInputs; + // Add preset inputs to result + res->AddInput(preset_inputs); return res; } @@ -766,17 +762,15 @@ static bool CreateTransactionInternal( AvailableCoins(wallet, vAvailableCoins, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0); // Choose coins to use - CAmount inputs_sum = 0; - std::set<CInputCoin> setCoins; - if (!SelectCoins(wallet, vAvailableCoins, /* nTargetValue */ selection_target, setCoins, inputs_sum, 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; } // Always make a change output // We will reduce the fee from this change output later, and remove the output if it is too small. - const CAmount change_and_fee = inputs_sum - recipients_sum; + const CAmount change_and_fee = result->GetSelectedValue() - recipients_sum; assert(change_and_fee >= 0); CTxOut newTxOut(change_and_fee, scriptChange); @@ -795,8 +789,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(setCoins.begin(), setCoins.end()); - Shuffle(selected_coins.begin(), selected_coins.end(), FastRandomContext()); + std::vector<CInputCoin> selected_coins = result->GetShuffledInputVector(); // Note how the sequence number is set to non-maxint so that // the nLockTime set above actually works. diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 7467dd9fa3..268bcfc033 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -101,29 +101,34 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet) std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<COutput>& outputs, const CoinSelectionParams& coin_sel_params, const CoinEligibilityFilter& filter, bool positive_only); /** - * Shuffle and select coins until nTargetValue is reached while avoiding - * small change; This method is stochastic for some inputs and upon - * completion the coin set and corresponding actual target value is - * assembled - * param@[in] coins Set of UTXOs to consider. These will be categorized into - * OutputGroups and filtered using eligibility_filter before - * selecting coins. - * param@[out] setCoinsRet Populated with the coins selected if successful. - * param@[out] nValueRet Used to return the total value of selected coins. + * Attempt to find a valid input set that meets the provided eligibility filter and target. + * Multiple coin selection algorithms will be run and the input set that produces the least waste + * (according to the waste metric) will be chosen. + * + * param@[in] wallet The wallet which provides solving data for the coins + * param@[in] nTargetValue The target value + * param@[in] eligilibity_filter A filter containing rules for which coins are allowed to be included in this selection + * param@[in] coins The vector of coins available for selection prior to filtering + * param@[in] coin_selection_params Parameters for the coin selection + * returns If successful, a SelectionResult containing the input set + * If failed, a nullopt */ -bool AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<COutput> coins, - std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params); +std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<COutput> coins, + const CoinSelectionParams& coin_selection_params); /** - * Select a set of coins such that nValueRet >= nTargetValue and at least + * Select a set of coins such that nTargetValue is met and at least * all coins from coin_control are selected; never select unconfirmed coins if they are not ours - * param@[out] setCoinsRet Populated with inputs including pre-selected inputs from - * coin_control and Coin Selection if successful. - * param@[out] nValueRet Total value of selected coins including pre-selected ones - * from coin_control and Coin Selection if successful. + * param@[in] wallet The wallet which provides data necessary to spend the selected coins + * param@[in] vAvailableCoins The vector of coins available to be spent + * param@[in] nTargetValue The target value + * param@[in] coin_selection_params Parameters for this coin selection such as feerates, whether to avoid partial spends, + * and whether to subtract the fee from the outputs. + * returns If successful, a SelectionResult containing the selected coins + * If failed, a nullopt. */ -bool SelectCoins(const CWallet& wallet, const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, - const CCoinControl& coin_control, CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); +std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, const CCoinControl& coin_control, + const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); /** * Create a new transaction paying the recipients with a set of coins diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 0acd8a811f..a1fa871ccb 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -14,6 +14,7 @@ #include <wallet/test/wallet_test_fixture.h> #include <wallet/wallet.h> +#include <algorithm> #include <boost/test/unit_test.hpp> #include <random> @@ -31,20 +32,35 @@ typedef std::set<CInputCoin> 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) { 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); } +static void add_coin(const CAmount& nValue, int nInput, SelectionResult& result) +{ + CMutableTransaction tx; + tx.vout.resize(nInput + 1); + tx.vout[nInput].nValue = nValue; + tx.nLockTime = nextLockTime++; // so all transactions get different hashes + CInputCoin coin(MakeTransactionRef(tx), nInput); + OutputGroup group; + group.Insert(coin, 1, false, 0, 0, true); + result.AddInput(group); +} + static void add_coin(const CAmount& nValue, int nInput, CoinSet& set, CAmount fee = 0, CAmount long_term_fee = 0) { CMutableTransaction tx; tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; + tx.nLockTime = nextLockTime++; // so all transactions get different hashes CInputCoin coin(MakeTransactionRef(tx), nInput); coin.effective_value = nValue - fee; coin.m_fee = fee; @@ -54,7 +70,6 @@ static void add_coin(const CAmount& nValue, int nInput, CoinSet& set, CAmount fe static void add_coin(std::vector<COutput>& coins, CWallet& wallet, const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0, bool spendable = false) { - static int nextLockTime = 0; CMutableTransaction tx; tx.nLockTime = nextLockTime++; // so all transactions get different hashes tx.vout.resize(nInput + 1); @@ -86,10 +101,30 @@ static void add_coin(std::vector<COutput>& coins, CWallet& wallet, const CAmount coins.push_back(output); } -static bool equal_sets(CoinSet a, CoinSet b) +/** Check if SelectionResult a is equivalent to SelectionResult b. + * Equivalent means same input values, but maybe different inputs (i.e. same value, different prevout) */ +static bool EquivalentResult(const SelectionResult& a, const SelectionResult& b) +{ + std::vector<CAmount> a_amts; + std::vector<CAmount> b_amts; + for (const auto& coin : a.GetInputSet()) { + a_amts.push_back(coin.txout.nValue); + } + for (const auto& coin : b.GetInputSet()) { + b_amts.push_back(coin.txout.nValue); + } + std::sort(a_amts.begin(), a_amts.end()); + std::sort(b_amts.begin(), b_amts.end()); + + std::pair<std::vector<CAmount>::iterator, std::vector<CAmount>::iterator> ret = std::mismatch(a_amts.begin(), a_amts.end(), b_amts.begin()); + return ret.first == a_amts.end() && ret.second == b_amts.end(); +} + +/** 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 = mismatch(a.begin(), a.end(), b.begin()); - return ret.first == a.end() && ret.second == b.end(); + std::pair<CoinSet::iterator, CoinSet::iterator> ret = std::mismatch(a.GetInputSet().begin(), a.GetInputSet().end(), b.GetInputSet().begin()); + return ret.first == a.GetInputSet().end() && ret.second == b.GetInputSet().end(); } static CAmount make_hard_case(int utxos, std::vector<CInputCoin>& utxo_pool) @@ -142,17 +177,14 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) { // Setup std::vector<CInputCoin> utxo_pool; - CoinSet selection; - CoinSet actual_selection; - CAmount value_ret = 0; + SelectionResult expected_result(CAmount(0)); ///////////////////////// // Known Outcome tests // ///////////////////////// // Empty utxo pool - BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 0.5 * CENT, selection, value_ret)); - selection.clear(); + BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 0.5 * CENT)); // Add utxos add_coin(1 * CENT, 1, utxo_pool); @@ -161,87 +193,86 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) add_coin(4 * CENT, 4, utxo_pool); // Select 1 Cent - add_coin(1 * CENT, 1, actual_selection); - BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 0.5 * CENT, selection, value_ret)); - BOOST_CHECK(equal_sets(selection, actual_selection)); - BOOST_CHECK_EQUAL(value_ret, 1 * CENT); - actual_selection.clear(); - selection.clear(); + add_coin(1 * CENT, 1, expected_result); + const auto result1 = SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 0.5 * CENT); + BOOST_CHECK(result1); + BOOST_CHECK(EquivalentResult(expected_result, *result1)); + BOOST_CHECK_EQUAL(result1->GetSelectedValue(), 1 * CENT); + expected_result.Clear(); // Select 2 Cent - add_coin(2 * CENT, 2, actual_selection); - BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 2 * CENT, 0.5 * CENT, selection, value_ret)); - BOOST_CHECK(equal_sets(selection, actual_selection)); - BOOST_CHECK_EQUAL(value_ret, 2 * CENT); - actual_selection.clear(); - selection.clear(); + add_coin(2 * CENT, 2, expected_result); + const auto result2 = SelectCoinsBnB(GroupCoins(utxo_pool), 2 * CENT, 0.5 * CENT); + BOOST_CHECK(result2); + BOOST_CHECK(EquivalentResult(expected_result, *result2)); + BOOST_CHECK_EQUAL(result2->GetSelectedValue(), 2 * CENT); + expected_result.Clear(); // Select 5 Cent - add_coin(4 * CENT, 4, actual_selection); - add_coin(1 * CENT, 1, actual_selection); - BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 5 * CENT, 0.5 * CENT, selection, value_ret)); - BOOST_CHECK(equal_sets(selection, actual_selection)); - BOOST_CHECK_EQUAL(value_ret, 5 * CENT); - actual_selection.clear(); - selection.clear(); + add_coin(4 * CENT, 4, expected_result); + add_coin(1 * CENT, 1, expected_result); + const auto result3 = SelectCoinsBnB(GroupCoins(utxo_pool), 5 * CENT, 0.5 * CENT); + BOOST_CHECK(result3); + BOOST_CHECK(EquivalentResult(expected_result, *result3)); + BOOST_CHECK_EQUAL(result3->GetSelectedValue(), 5 * CENT); + expected_result.Clear(); // Select 11 Cent, not possible - BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 11 * CENT, 0.5 * CENT, selection, value_ret)); - actual_selection.clear(); - selection.clear(); + BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 11 * CENT, 0.5 * CENT)); + expected_result.Clear(); // Cost of change is greater than the difference between target value and utxo sum - add_coin(1 * CENT, 1, actual_selection); - BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 0.9 * CENT, 0.5 * CENT, selection, value_ret)); - BOOST_CHECK_EQUAL(value_ret, 1 * CENT); - BOOST_CHECK(equal_sets(selection, actual_selection)); - actual_selection.clear(); - selection.clear(); + add_coin(1 * CENT, 1, expected_result); + const auto result4 = SelectCoinsBnB(GroupCoins(utxo_pool), 0.9 * CENT, 0.5 * CENT); + BOOST_CHECK(result4); + BOOST_CHECK_EQUAL(result4->GetSelectedValue(), 1 * CENT); + BOOST_CHECK(EquivalentResult(expected_result, *result4)); + expected_result.Clear(); // Cost of change is less than the difference between target value and utxo sum - BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.9 * CENT, 0, selection, value_ret)); - actual_selection.clear(); - selection.clear(); + BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.9 * CENT, 0)); + expected_result.Clear(); // Select 10 Cent add_coin(5 * CENT, 5, utxo_pool); - add_coin(5 * CENT, 5, actual_selection); - add_coin(4 * CENT, 4, actual_selection); - add_coin(1 * CENT, 1, actual_selection); - BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 0.5 * CENT, selection, value_ret)); - BOOST_CHECK(equal_sets(selection, actual_selection)); - BOOST_CHECK_EQUAL(value_ret, 10 * CENT); - actual_selection.clear(); - selection.clear(); + add_coin(5 * CENT, 5, expected_result); + add_coin(4 * CENT, 4, expected_result); + add_coin(1 * CENT, 1, expected_result); + const auto result5 = SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 0.5 * CENT); + BOOST_CHECK(result5); + BOOST_CHECK(EquivalentResult(expected_result, *result5)); + BOOST_CHECK_EQUAL(result5->GetSelectedValue(), 10 * CENT); + expected_result.Clear(); // Negative effective value // Select 10 Cent but have 1 Cent not be possible because too small - add_coin(5 * CENT, 5, actual_selection); - add_coin(3 * CENT, 3, actual_selection); - add_coin(2 * CENT, 2, actual_selection); - BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 5000, selection, value_ret)); - BOOST_CHECK_EQUAL(value_ret, 10 * CENT); + add_coin(5 * CENT, 5, expected_result); + add_coin(3 * CENT, 3, expected_result); + add_coin(2 * CENT, 2, expected_result); + const auto result6 = SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 5000); + BOOST_CHECK(result6); + BOOST_CHECK_EQUAL(result6->GetSelectedValue(), 10 * CENT); // FIXME: this test is redundant with the above, because 1 Cent is selected, not "too small" - // BOOST_CHECK(equal_sets(selection, actual_selection)); + // BOOST_CHECK(EquivalentResult(expected_result, *result)); // Select 0.25 Cent, not possible - BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.25 * CENT, 0.5 * CENT, selection, value_ret)); - actual_selection.clear(); - selection.clear(); + BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.25 * CENT, 0.5 * CENT)); + expected_result.Clear(); // Iteration exhaustion test CAmount target = make_hard_case(17, utxo_pool); - BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), target, 0, selection, value_ret)); // Should exhaust + BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), target, 0)); // Should exhaust target = make_hard_case(14, utxo_pool); - BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), target, 0, selection, value_ret)); // Should not exhaust + const auto result7 = SelectCoinsBnB(GroupCoins(utxo_pool), target, 0); // Should not exhaust + BOOST_CHECK(result7); // Test same value early bailout optimization utxo_pool.clear(); - add_coin(7 * CENT, 7, actual_selection); - add_coin(7 * CENT, 7, actual_selection); - add_coin(7 * CENT, 7, actual_selection); - add_coin(7 * CENT, 7, actual_selection); - add_coin(2 * CENT, 7, actual_selection); + add_coin(7 * CENT, 7, expected_result); + add_coin(7 * CENT, 7, expected_result); + add_coin(7 * CENT, 7, expected_result); + add_coin(7 * CENT, 7, expected_result); + add_coin(2 * CENT, 7, expected_result); add_coin(7 * CENT, 7, utxo_pool); add_coin(7 * CENT, 7, utxo_pool); add_coin(7 * CENT, 7, utxo_pool); @@ -250,9 +281,10 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) for (int i = 0; i < 50000; ++i) { add_coin(5 * CENT, 7, utxo_pool); } - BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 30 * CENT, 5000, selection, value_ret)); - BOOST_CHECK_EQUAL(value_ret, 30 * CENT); - BOOST_CHECK(equal_sets(selection, actual_selection)); + const auto result8 = SelectCoinsBnB(GroupCoins(utxo_pool), 30 * CENT, 5000); + BOOST_CHECK(result8); + BOOST_CHECK_EQUAL(result8->GetSelectedValue(), 30 * CENT); + BOOST_CHECK(EquivalentResult(expected_result, *result8)); //////////////////// // Behavior tests // @@ -264,7 +296,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) } // Run 100 times, to make sure it is never finding a solution for (int i = 0; i < 100; ++i) { - BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 2 * CENT, selection, value_ret)); + BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 2 * CENT)); } // Make sure that effective value is working in AttemptSelection when BnB is used @@ -280,20 +312,19 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) wallet->SetupDescriptorScriptPubKeyMans(); std::vector<COutput> coins; - CoinSet setCoinsRet; - CAmount nValueRet; 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 - BOOST_CHECK(!SelectCoinsBnB(GroupCoins(coins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change, setCoinsRet, nValueRet)); + 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; coin_selection_params_bnb.m_subtract_fee_outputs = true; - BOOST_CHECK(SelectCoinsBnB(GroupCoins(coins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); + const auto result9 = SelectCoinsBnB(GroupCoins(coins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change); + BOOST_CHECK(result9); + BOOST_CHECK_EQUAL(result9->GetSelectedValue(), 1 * CENT); } { @@ -304,8 +335,6 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) wallet->SetupDescriptorScriptPubKeyMans(); std::vector<COutput> coins; - CoinSet setCoinsRet; - CAmount nValueRet; add_coin(coins, *wallet, 5 * CENT, 6 * 24, false, 0, true); add_coin(coins, *wallet, 3 * CENT, 6 * 24, false, 0, true); @@ -314,7 +343,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) coin_control.fAllowOtherInputs = true; coin_control.Select(COutPoint(coins.at(0).tx->GetHash(), coins.at(0).i)); coin_selection_params_bnb.m_effective_feerate = CFeeRate(0); - BOOST_CHECK(SelectCoins(*wallet, coins, 10 * CENT, setCoinsRet, nValueRet, coin_control, coin_selection_params_bnb)); + const auto result10 = SelectCoins(*wallet, coins, 10 * CENT, coin_control, coin_selection_params_bnb); + BOOST_CHECK(result10); } } @@ -326,8 +356,6 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet->SetupDescriptorScriptPubKeyMans(); - CoinSet setCoinsRet, setCoinsRet2; - CAmount nValueRet; std::vector<COutput> coins; // test multiple times to allow for differences in the shuffle order @@ -336,25 +364,27 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) coins.clear(); // with an empty wallet we can't even pay one cent - BOOST_CHECK(!KnapsackSolver(1 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_standard), setCoinsRet, nValueRet)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 1 * 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(1 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_standard), setCoinsRet, nValueRet)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 1 * CENT)); // but we can find a new 1 cent - BOOST_CHECK(KnapsackSolver(1 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); + const auto result1 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 1 * 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(3 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_standard), setCoinsRet, nValueRet)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 3 * CENT)); // we can make 3 cents of new coins - BOOST_CHECK(KnapsackSolver(3 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); + const auto result2 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 3 * CENT); + BOOST_CHECK(result2); + BOOST_CHECK_EQUAL(result2->GetSelectedValue(), 3 * CENT); add_coin(coins, *wallet, 5*CENT); // add a mature 5 cent coin, add_coin(coins, *wallet, 10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses @@ -363,35 +393,41 @@ 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(38 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_standard), setCoinsRet, nValueRet)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 38 * CENT)); // we can't even make 37 cents if we don't allow new coins even if they're from us - BOOST_CHECK(!KnapsackSolver(38 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_standard_extra), setCoinsRet, nValueRet)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard_extra), 38 * CENT)); // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK(KnapsackSolver(37 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_standard), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); + const auto result3 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 37 * CENT); + BOOST_CHECK(result3); + BOOST_CHECK_EQUAL(result3->GetSelectedValue(), 37 * CENT); // and we can make 38 cents if we accept all new coins - BOOST_CHECK(KnapsackSolver(38 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 38 * CENT); + const auto result4 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 38 * 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 - BOOST_CHECK(KnapsackSolver(34 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 35 * CENT); // but 35 cents is closest - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) + const auto result5 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 34 * 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 - BOOST_CHECK(KnapsackSolver(7 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 7 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + const auto result6 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 7 * 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. - BOOST_CHECK(KnapsackSolver(8 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK(nValueRet == 8 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); + const auto result7 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 8 * 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) - BOOST_CHECK(KnapsackSolver(9 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + const auto result8 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 9 * CENT); + BOOST_CHECK(result8); + BOOST_CHECK_EQUAL(result8->GetSelectedValue(), 10 * CENT); + BOOST_CHECK_EQUAL(result8->GetInputSet().size(), 1U); // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin coins.clear(); @@ -403,45 +439,52 @@ 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 - BOOST_CHECK(KnapsackSolver(71 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK(!KnapsackSolver(72 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); + const auto result9 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 71 * CENT); + BOOST_CHECK(result9); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 72 * 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 - BOOST_CHECK(KnapsackSolver(16 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 20 * CENT); // we should get 20 in one coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + const auto result10 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 16 * 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); 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 - BOOST_CHECK(KnapsackSolver(16 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 3 coins - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); + const auto result11 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 16 * 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); 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 - BOOST_CHECK(KnapsackSolver(16 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // because in the event of a tie, the biggest coin wins + const auto result12 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 16 * 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 - BOOST_CHECK(KnapsackSolver(11 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + const auto result13 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 11 * CENT); + BOOST_CHECK(result13); + BOOST_CHECK_EQUAL(result13->GetSelectedValue(), 11 * CENT); + BOOST_CHECK_EQUAL(result13->GetInputSet().size(), 2U); // check that the smallest bigger coin is used add_coin(coins, *wallet, 1*COIN); 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 - BOOST_CHECK(KnapsackSolver(95 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * COIN); // we should get 1 BTC in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + const auto result14 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 95 * 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); - BOOST_CHECK(KnapsackSolver(195 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + const auto result15 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 195 * 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); // empty the wallet and start again, now with fractions of a cent, to test small change avoidance @@ -454,23 +497,26 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // try making 1 * MIN_CHANGE from the 1.5 * MIN_CHANGE // we'll get change smaller than MIN_CHANGE whatever happens, so can expect MIN_CHANGE exactly - BOOST_CHECK(KnapsackSolver(MIN_CHANGE, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); + const auto result16 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), MIN_CHANGE); + BOOST_CHECK(result16); + BOOST_CHECK_EQUAL(result16->GetSelectedValue(), MIN_CHANGE); // but if we add a bigger coin, small change is avoided add_coin(coins, *wallet, 1111*MIN_CHANGE); // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 - BOOST_CHECK(KnapsackSolver(1 * MIN_CHANGE, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount + const auto result17 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 1 * MIN_CHANGE); + BOOST_CHECK(result17); + BOOST_CHECK_EQUAL(result17->GetSelectedValue(), 1 * MIN_CHANGE); // 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); // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK(KnapsackSolver(1 * MIN_CHANGE, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount + const auto result18 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 1 * MIN_CHANGE); + BOOST_CHECK(result18); + BOOST_CHECK_EQUAL(result18->GetSelectedValue(), 1 * MIN_CHANGE); // 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 @@ -478,9 +524,10 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) for (int j = 0; j < 20; j++) add_coin(coins, *wallet, 50000 * COIN); - BOOST_CHECK(KnapsackSolver(500000 * COIN, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount - BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins + const auto result19 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 500000 * COIN); + 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), // we need to try finding an exact subset anyway @@ -491,9 +538,10 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(coins, *wallet, MIN_CHANGE * 6 / 10); add_coin(coins, *wallet, MIN_CHANGE * 7 / 10); add_coin(coins, *wallet, 1111 * MIN_CHANGE); - BOOST_CHECK(KnapsackSolver(1 * MIN_CHANGE, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + const auto result20 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 1 * MIN_CHANGE); + BOOST_CHECK(result20); + BOOST_CHECK_EQUAL(result20->GetSelectedValue(), 1111 * MIN_CHANGE); // 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(); @@ -501,9 +549,10 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(coins, *wallet, MIN_CHANGE * 6 / 10); add_coin(coins, *wallet, MIN_CHANGE * 8 / 10); add_coin(coins, *wallet, 1111 * MIN_CHANGE); - BOOST_CHECK(KnapsackSolver(MIN_CHANGE, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // we should get the exact amount - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // in two coins 0.4+0.6 + const auto result21 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), MIN_CHANGE); + BOOST_CHECK(result21); + BOOST_CHECK_EQUAL(result21->GetSelectedValue(), MIN_CHANGE); // 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(); @@ -512,14 +561,16 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(coins, *wallet, MIN_CHANGE * 100); // trying to make 100.01 from these three coins - BOOST_CHECK(KnapsackSolver(MIN_CHANGE * 10001 / 100, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE * 10105 / 100); // we should get all coins - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); + const auto result22 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), MIN_CHANGE * 10001 / 100); + BOOST_CHECK(result22); + BOOST_CHECK_EQUAL(result22->GetSelectedValue(), MIN_CHANGE * 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 - BOOST_CHECK(KnapsackSolver(MIN_CHANGE * 9990 / 100, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + const auto result23 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), MIN_CHANGE * 9990 / 100); + BOOST_CHECK(result23); + BOOST_CHECK_EQUAL(result23->GetSelectedValue(), 101 * MIN_CHANGE); + BOOST_CHECK_EQUAL(result23->GetInputSet().size(), 2U); } // test with many inputs @@ -531,18 +582,19 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // We only create the wallet once to save time, but we still run the coin selection RUN_TESTS times. for (int i = 0; i < RUN_TESTS; i++) { - BOOST_CHECK(KnapsackSolver(2000, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); + const auto result24 = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_confirmed), 2000); + BOOST_CHECK(result24); if (amt - 2000 < MIN_CHANGE) { // needs more than one input: uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); CAmount returnValue = amt * returnSize; - BOOST_CHECK_EQUAL(nValueRet, returnValue); - BOOST_CHECK_EQUAL(setCoinsRet.size(), returnSize); + BOOST_CHECK_EQUAL(result24->GetSelectedValue(), returnValue); + BOOST_CHECK_EQUAL(result24->GetInputSet().size(), returnSize); } else { // one input is sufficient: - BOOST_CHECK_EQUAL(nValueRet, amt); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + BOOST_CHECK_EQUAL(result24->GetSelectedValue(), amt); + BOOST_CHECK_EQUAL(result24->GetInputSet().size(), 1U); } } } @@ -557,9 +609,11 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) for (int i = 0; i < RUN_TESTS; i++) { // picking 50 from 100 coins doesn't depend on the shuffle, // but does depend on randomness in the stochastic approximation code - BOOST_CHECK(KnapsackSolver(50 * COIN, GroupCoins(coins), setCoinsRet, nValueRet)); - BOOST_CHECK(KnapsackSolver(50 * COIN, GroupCoins(coins), setCoinsRet2, nValueRet)); - BOOST_CHECK(!equal_sets(setCoinsRet, setCoinsRet2)); + const auto result25 = KnapsackSolver(GroupCoins(coins), 50 * COIN); + BOOST_CHECK(result25); + const auto result26 = KnapsackSolver(GroupCoins(coins), 50 * COIN); + BOOST_CHECK(result26); + BOOST_CHECK(!EqualResult(*result25, *result26)); int fails = 0; for (int j = 0; j < RANDOM_REPEATS; j++) @@ -568,9 +622,11 @@ 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 - BOOST_CHECK(KnapsackSolver(COIN, GroupCoins(coins), setCoinsRet, nValueRet)); - BOOST_CHECK(KnapsackSolver(COIN, GroupCoins(coins), setCoinsRet2, nValueRet)); - if (equal_sets(setCoinsRet, setCoinsRet2)) + const auto result27 = KnapsackSolver(GroupCoins(coins), COIN); + BOOST_CHECK(result27); + const auto result28 = KnapsackSolver(GroupCoins(coins), COIN); + BOOST_CHECK(result28); + if (EqualResult(*result27, *result28)) fails++; } BOOST_CHECK_NE(fails, RANDOM_REPEATS); @@ -589,9 +645,11 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) int fails = 0; for (int j = 0; j < RANDOM_REPEATS; j++) { - BOOST_CHECK(KnapsackSolver(90*CENT, GroupCoins(coins), setCoinsRet, nValueRet)); - BOOST_CHECK(KnapsackSolver(90*CENT, GroupCoins(coins), setCoinsRet2, nValueRet)); - if (equal_sets(setCoinsRet, setCoinsRet2)) + const auto result29 = KnapsackSolver(GroupCoins(coins), 90 * CENT); + BOOST_CHECK(result29); + const auto result30 = KnapsackSolver(GroupCoins(coins), 90 * CENT); + BOOST_CHECK(result30); + if (EqualResult(*result29, *result30)) fails++; } BOOST_CHECK_NE(fails, RANDOM_REPEATS); @@ -607,8 +665,6 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet->SetupDescriptorScriptPubKeyMans(); - CoinSet setCoinsRet; - CAmount nValueRet; std::vector<COutput> coins; // Test vValue sort order @@ -616,9 +672,10 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) add_coin(coins, *wallet, 1000 * COIN); add_coin(coins, *wallet, 3 * COIN); - BOOST_CHECK(KnapsackSolver(1003 * COIN, KnapsackGroupOutputs(coins, *wallet, filter_standard), setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + const auto result = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 1003 * COIN); + BOOST_CHECK(result); + BOOST_CHECK_EQUAL(result->GetSelectedValue(), 1003 * COIN); + BOOST_CHECK_EQUAL(result->GetInputSet().size(), 2U); } // Tests that with the ideal conditions, the coin selector will always be able to find a solution that can pay the target value @@ -660,11 +717,10 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) /* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0), /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), /* tx_noinputs_size= */ 0, /* avoid_partial= */ false); - CoinSet out_set; - CAmount out_value = 0; CCoinControl cc; - BOOST_CHECK(SelectCoins(*wallet, coins, target, out_set, out_value, cc, cs_params)); - BOOST_CHECK_GE(out_value, target); + const auto result = SelectCoins(*wallet, coins, target, cc, cs_params); + BOOST_CHECK(result); + BOOST_CHECK_GE(result->GetSelectedValue(), target); } } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index cfe550cb3f..c79e917c69 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3208,8 +3208,11 @@ void CWallet::SetupDescriptorScriptPubKeyMans() for (const UniValue& desc_val : descriptor_vals.get_array().getValues()) { std::string desc_str = desc_val.getValStr(); FlatSigningProvider keys; - std::string dummy_error; - std::unique_ptr<Descriptor> desc = Parse(desc_str, keys, dummy_error, false); + std::string desc_error; + std::unique_ptr<Descriptor> desc = Parse(desc_str, keys, desc_error, false); + if (desc == nullptr) { + throw std::runtime_error(std::string(__func__) + ": Invalid descriptor \"" + desc_str + "\" (" + desc_error + ")"); + } if (!desc->GetOutputType()) { continue; } diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index d6717ebbca..d3207fe460 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -2,6 +2,12 @@ // 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 <wallet/wallettool.h> + #include <fs.h> #include <util/system.h> #include <util/translation.h> diff --git a/src/wallet/wallettool.h b/src/wallet/wallettool.h index f4516bb5bc..09be1dac2c 100644 --- a/src/wallet/wallettool.h +++ b/src/wallet/wallettool.h @@ -5,11 +5,12 @@ #ifndef BITCOIN_WALLET_WALLETTOOL_H #define BITCOIN_WALLET_WALLETTOOL_H -#include <wallet/wallet.h> +#include <string> + +class ArgsManager; namespace WalletTool { -void WalletShowInfo(CWallet* wallet_instance); bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command); } // namespace WalletTool |