From fa4fae6227a0a495ab60191e22dc0af8fe7a8484 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 6 May 2025 19:42:59 +0200 Subject: [PATCH 1/8] test: Add NodeClockContext This makes it easier to use mock-time in tests. Also, it resets the global mocktime, so that no state is leaked between test cases. --- src/test/util/time.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/test/util/time.h b/src/test/util/time.h index bcb97eb4e14f..8cd9a1dafcb9 100644 --- a/src/test/util/time.h +++ b/src/test/util/time.h @@ -34,4 +34,32 @@ class SteadyClockContext } }; +/// Helper to initialize the global NodeClock, let a duration elapse, +/// and reset it after use in a test. +class NodeClockContext +{ + NodeSeconds m_t{std::chrono::seconds::max()}; + +public: + /// Initialize with the given time. + explicit NodeClockContext(NodeSeconds init_time) { set(init_time); } + explicit NodeClockContext(std::chrono::seconds init_time) { set(init_time); } + /// Initialize with current time, using the next tick to avoid going back by rounding to seconds. + explicit NodeClockContext() { set(++Now().time_since_epoch()); } + + /// Unset mocktime. + ~NodeClockContext() { set(0s); } + + NodeClockContext(const NodeClockContext&) = delete; + NodeClockContext& operator=(const NodeClockContext&) = delete; + + /// Set mocktime. + void set(NodeSeconds t) { SetMockTime(m_t = t); } + void set(std::chrono::seconds t) { set(NodeSeconds{t}); } + + /// Change mocktime by the given duration delta. + void operator+=(std::chrono::seconds d) { set(m_t += d); } + void operator-=(std::chrono::seconds d) { set(m_t -= d); } +}; + #endif // BITCOIN_TEST_UTIL_TIME_H From eeeeb2a0b902ed69b5cd5523833d3ab5d963c81f Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 6 May 2025 20:12:40 +0200 Subject: [PATCH 2/8] fuzz: Use NodeClockContext This refactor does not change any behavior. However, it is nice to know that no global mocktime leaks from the fuzz init step to the first fuzz input, or from one fuzz input execution to the next. With the clock context, the global is re-set at the end of the context. --- src/test/fuzz/addrman.cpp | 5 +++-- src/test/fuzz/banman.cpp | 5 +++-- src/test/fuzz/block_index_tree.cpp | 3 ++- src/test/fuzz/connman.cpp | 3 ++- src/test/fuzz/headerssync.cpp | 3 ++- src/test/fuzz/i2p.cpp | 3 ++- src/test/fuzz/load_external_block_file.cpp | 3 ++- src/test/fuzz/mini_miner.cpp | 5 +++-- src/test/fuzz/net.cpp | 5 +++-- src/test/fuzz/p2p_handshake.cpp | 8 +++++--- src/test/fuzz/p2p_headers_presync.cpp | 2 +- src/test/fuzz/partially_downloaded_block.cpp | 3 ++- src/test/fuzz/process_message.cpp | 6 +++--- src/test/fuzz/process_messages.cpp | 6 +++--- src/test/fuzz/rbf.cpp | 5 +++-- src/test/fuzz/rpc.cpp | 3 ++- src/test/fuzz/socks5.cpp | 3 ++- src/test/fuzz/txdownloadman.cpp | 7 ++++--- src/test/fuzz/txorphan.cpp | 5 +++-- src/test/fuzz/utxo_snapshot.cpp | 3 ++- src/test/fuzz/utxo_total_supply.cpp | 3 ++- src/test/fuzz/validation_load_mempool.cpp | 3 ++- src/wallet/test/fuzz/fees.cpp | 3 ++- src/wallet/test/fuzz/scriptpubkeyman.cpp | 3 ++- src/wallet/test/fuzz/spend.cpp | 7 ++++--- 25 files changed, 64 insertions(+), 41 deletions(-) diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 88260b272f2e..6b4519c17a01 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -116,7 +117,7 @@ FUZZ_TARGET(addrman, .init = initialize_addrman) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; NetGroupManager netgroupman{ConsumeNetGroupManager(fuzzed_data_provider)}; auto addr_man_ptr = std::make_unique(netgroupman, fuzzed_data_provider, GetCheckRatio()); if (fuzzed_data_provider.ConsumeBool()) { @@ -201,7 +202,7 @@ FUZZ_TARGET(addrman_serdeser, .init = initialize_addrman) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; NetGroupManager netgroupman{ConsumeNetGroupManager(fuzzed_data_provider)}; AddrManDeterministic addr_man1{netgroupman, fuzzed_data_provider, GetCheckRatio()}; diff --git a/src/test/fuzz/banman.cpp b/src/test/fuzz/banman.cpp index e65b4da03abf..aa7e76b90c79 100644 --- a/src/test/fuzz/banman.cpp +++ b/src/test/fuzz/banman.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -44,7 +45,7 @@ FUZZ_TARGET(banman, .init = initialize_banman) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; fs::path banlist_file = gArgs.GetDataDirNet() / "fuzzed_banlist"; const bool start_with_corrupted_banlist{fuzzed_data_provider.ConsumeBool()}; @@ -124,7 +125,7 @@ FUZZ_TARGET(banman, .init = initialize_banman) } if (!force_read_and_write_to_err) { ban_man.DumpBanlist(); - SetMockTime(ConsumeTime(fuzzed_data_provider)); + clock_ctx.set(ConsumeTime(fuzzed_data_provider)); banmap_t banmap; ban_man.GetBanned(banmap); BanMan ban_man_read{banlist_file, /*client_interface=*/nullptr, /*default_ban_time=*/0}; diff --git a/src/test/fuzz/block_index_tree.cpp b/src/test/fuzz/block_index_tree.cpp index 5112a7e0f2ea..0e333cd62881 100644 --- a/src/test/fuzz/block_index_tree.cpp +++ b/src/test/fuzz/block_index_tree.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -41,7 +42,7 @@ void initialize_block_index_tree() FUZZ_TARGET(block_index_tree, .init = initialize_block_index_tree) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; auto& chainman = static_cast(*g_setup->m_node.chainman); auto& blockman = static_cast(chainman.m_blockman); CBlockIndex* genesis = chainman.ActiveChainstate().m_chain[0]; diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index 4a8b7b2155f5..9b7e043c5a4e 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -40,7 +41,7 @@ FUZZ_TARGET(connman, .init = initialize_connman) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; auto netgroupman{ConsumeNetGroupManager(fuzzed_data_provider)}; auto addr_man_ptr{std::make_unique(netgroupman, fuzzed_data_provider, GetCheckRatio())}; if (fuzzed_data_provider.ConsumeBool()) { diff --git a/src/test/fuzz/headerssync.cpp b/src/test/fuzz/headerssync.cpp index f6e574f4044d..f54ea980e4e5 100644 --- a/src/test/fuzz/headerssync.cpp +++ b/src/test/fuzz/headerssync.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -59,7 +60,7 @@ FUZZ_TARGET(headers_sync_state, .init = initialize_headers_sync_state_fuzz) CBlockHeader genesis_header{Params().GenesisBlock()}; CBlockIndex start_index(genesis_header); - SetMockTime(ConsumeTime(fuzzed_data_provider, /*min=*/start_index.GetMedianTimePast())); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider, /*min=*/start_index.GetMedianTimePast())}; const uint256 genesis_hash = genesis_header.GetHash(); start_index.phashBlock = &genesis_hash; diff --git a/src/test/fuzz/i2p.cpp b/src/test/fuzz/i2p.cpp index 9888e7a0b697..613b856dffde 100644 --- a/src/test/fuzz/i2p.cpp +++ b/src/test/fuzz/i2p.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -26,7 +27,7 @@ FUZZ_TARGET(i2p, .init = initialize_i2p) SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; // Mock CreateSock() to create FuzzedSock. auto CreateSockOrig = CreateSock; diff --git a/src/test/fuzz/load_external_block_file.cpp b/src/test/fuzz/load_external_block_file.cpp index 06eccf01bd92..8b86f70dd237 100644 --- a/src/test/fuzz/load_external_block_file.cpp +++ b/src/test/fuzz/load_external_block_file.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -28,7 +29,7 @@ void initialize_load_external_block_file() FUZZ_TARGET(load_external_block_file, .init = initialize_load_external_block_file) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; FuzzedFileProvider fuzzed_file_provider{fuzzed_data_provider}; AutoFile fuzzed_block_file{fuzzed_file_provider.open()}; if (fuzzed_block_file.IsNull()) { diff --git a/src/test/fuzz/mini_miner.cpp b/src/test/fuzz/mini_miner.cpp index 6a5c164985ae..eca0caf9fe61 100644 --- a/src/test/fuzz/mini_miner.cpp +++ b/src/test/fuzz/mini_miner.cpp @@ -6,10 +6,11 @@ #include #include #include +#include #include #include +#include #include -#include #include #include @@ -42,7 +43,7 @@ FUZZ_TARGET(mini_miner, .init = initialize_miner) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; bilingual_str error; CTxMemPool pool{CTxMemPool::Options{}, error}; Assert(error.empty()); diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp index 7b6615bca715..20fbae1594bd 100644 --- a/src/test/fuzz/net.cpp +++ b/src/test/fuzz/net.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -31,7 +32,7 @@ void initialize_net() FUZZ_TARGET(net, .init = initialize_net) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; CNode node{ConsumeNode(fuzzed_data_provider)}; node.SetCommonVersion(fuzzed_data_provider.ConsumeIntegral()); if (const auto service_opt = @@ -80,7 +81,7 @@ FUZZ_TARGET(net, .init = initialize_net) FUZZ_TARGET(local_address, .init = initialize_net) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; CService service{ConsumeService(fuzzed_data_provider)}; CNode node{ConsumeNode(fuzzed_data_provider)}; { diff --git a/src/test/fuzz/p2p_handshake.cpp b/src/test/fuzz/p2p_handshake.cpp index 7ca0a06a8b0e..421aacd86671 100644 --- a/src/test/fuzz/p2p_handshake.cpp +++ b/src/test/fuzz/p2p_handshake.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -44,7 +45,7 @@ FUZZ_TARGET(p2p_handshake, .init = ::initialize) auto& connman = static_cast(*g_setup->m_node.connman); auto& chainman = static_cast(*g_setup->m_node.chainman); - SetMockTime(1610000000); // any time to successfully reset ibd + NodeClockContext clock_ctx{1610000000s}; // any time to successfully reset ibd chainman.ResetIbd(); node::Warnings warnings{}; @@ -80,10 +81,11 @@ FUZZ_TARGET(p2p_handshake, .init = ::initialize) continue; } - SetMockTime(GetTime() + + clock_ctx += std::chrono::seconds{ fuzzed_data_provider.ConsumeIntegralInRange( -std::chrono::seconds{10min}.count(), // Allow mocktime to go backwards slightly - std::chrono::seconds{TIMEOUT_INTERVAL}.count())); + std::chrono::seconds{TIMEOUT_INTERVAL}.count()), + }; CSerializedNetMsg net_msg; net_msg.m_type = PickValue(fuzzed_data_provider, ALL_NET_MESSAGE_TYPES); diff --git a/src/test/fuzz/p2p_headers_presync.cpp b/src/test/fuzz/p2p_headers_presync.cpp index 25a83fef4eaa..704e447fe695 100644 --- a/src/test/fuzz/p2p_headers_presync.cpp +++ b/src/test/fuzz/p2p_headers_presync.cpp @@ -176,7 +176,7 @@ FUZZ_TARGET(p2p_headers_presync, .init = initialize) ChainstateManager& chainman = *g_testing_setup->m_node.chainman; CBlockHeader base{chainman.GetParams().GenesisBlock()}; - SetMockTime(base.nTime); + const NodeClockContext clock_ctx{base.Time()}; LOCK(NetEventsInterface::g_msgproc_mutex); diff --git a/src/test/fuzz/partially_downloaded_block.cpp b/src/test/fuzz/partially_downloaded_block.cpp index 0bbe99915f59..12fdb8f7d2e1 100644 --- a/src/test/fuzz/partially_downloaded_block.cpp +++ b/src/test/fuzz/partially_downloaded_block.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -47,7 +48,7 @@ FUZZ_TARGET(partially_downloaded_block, .init = initialize_pdb) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; auto block{ConsumeDeserializable(fuzzed_data_provider, TX_WITH_WITNESS)}; if (!block || block->vtx.size() == 0 || diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index bd3c4ec9e94b..bca0d3a0d80e 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -75,7 +76,7 @@ FUZZ_TARGET(process_message, .init = initialize_process_message) connman.Reset(); auto& chainman{static_cast(*node.chainman)}; const auto block_index_size{WITH_LOCK(chainman.GetMutex(), return chainman.BlockIndex().size())}; - SetMockTime(1610000000); // any time to successfully reset ibd + NodeClockContext clock_ctx{1610000000s}; // any time to successfully reset ibd chainman.ResetIbd(); chainman.DisableNextWrite(); @@ -105,8 +106,7 @@ FUZZ_TARGET(process_message, .init = initialize_process_message) connman.AddTestNode(p2p_node); FillNode(fuzzed_data_provider, connman, p2p_node); - const auto mock_time = ConsumeTime(fuzzed_data_provider); - SetMockTime(mock_time); + clock_ctx.set(ConsumeTime(fuzzed_data_provider)); CSerializedNetMsg net_msg; net_msg.m_type = random_message_type; diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index 287529733643..d24a1ed5b21d 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -65,7 +66,7 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages) connman.Reset(); auto& chainman{static_cast(*node.chainman)}; const auto block_index_size{WITH_LOCK(chainman.GetMutex(), return chainman.BlockIndex().size())}; - SetMockTime(1610000000); // any time to successfully reset ibd + NodeClockContext clock_ctx{1610000000s}; // any time to successfully reset ibd chainman.ResetIbd(); chainman.DisableNextWrite(); @@ -101,8 +102,7 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages) { const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::MESSAGE_TYPE_SIZE).c_str()}; - const auto mock_time = ConsumeTime(fuzzed_data_provider); - SetMockTime(mock_time); + clock_ctx.set(ConsumeTime(fuzzed_data_provider)); CSerializedNetMsg net_msg; net_msg.m_type = random_message_type; diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp index 023fe7a42b1d..3644881d2269 100644 --- a/src/test/fuzz/rbf.cpp +++ b/src/test/fuzz/rbf.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -53,7 +54,7 @@ FUZZ_TARGET(rbf, .init = initialize_rbf) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; std::optional mtx = ConsumeDeserializable(fuzzed_data_provider, TX_WITH_WITNESS); if (!mtx) { return; @@ -95,7 +96,7 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; // "Real" virtual size is not important for this test since ConsumeTxMemPoolEntry generates its own virtual size values // so we construct small transactions for performance reasons. Child simply needs an input for later to perhaps connect to parent. diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index e8daf9d390ac..cd7f49872b6a 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -370,7 +371,7 @@ FUZZ_TARGET(rpc, .init = initialize_rpc) SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; bool good_data{true}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; const std::string rpc_command = fuzzed_data_provider.ConsumeRandomLengthString(64); if (!g_limit_to_rpc_command.empty() && rpc_command != g_limit_to_rpc_command) { return; diff --git a/src/test/fuzz/socks5.cpp b/src/test/fuzz/socks5.cpp index 72d69310292e..12db69a23687 100644 --- a/src/test/fuzz/socks5.cpp +++ b/src/test/fuzz/socks5.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -30,7 +31,7 @@ void initialize_socks5() FUZZ_TARGET(socks5, .init = initialize_socks5) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; ProxyCredentials proxy_credentials; proxy_credentials.username = fuzzed_data_provider.ConsumeRandomLengthString(512); proxy_credentials.password = fuzzed_data_provider.ConsumeRandomLengthString(512); diff --git a/src/test/fuzz/txdownloadman.cpp b/src/test/fuzz/txdownloadman.cpp index 89a3f060daa2..a2c7fc562c9a 100644 --- a/src/test/fuzz/txdownloadman.cpp +++ b/src/test/fuzz/txdownloadman.cpp @@ -15,11 +15,12 @@ #include #include #include +#include #include +#include #include #include #include -#include #include #include @@ -168,7 +169,7 @@ FUZZ_TARGET(txdownloadman, .init = initialize) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; // Initialize txdownloadman bilingual_str error; @@ -293,7 +294,7 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; // Initialize a TxDownloadManagerImpl bilingual_str error; diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index f6720ffb897a..097989908f3c 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -40,7 +41,7 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage) SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); FastRandomContext orphanage_rng{ConsumeUInt256(fuzzed_data_provider)}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; auto orphanage = node::MakeTxOrphanage(); std::vector outpoints; // Duplicates are tolerated @@ -227,7 +228,7 @@ FUZZ_TARGET(txorphan_protected, .init = initialize_orphanage) SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); FastRandomContext orphanage_rng{ConsumeUInt256(fuzzed_data_provider)}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; // We have num_peers peers. Some subset of them will never exceed their reserved weight or announcement count, and // should therefore never have any orphans evicted. diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index 67290f7d306b..0ed13fcc6ab2 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/src/test/fuzz/utxo_snapshot.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -103,7 +104,7 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - SetMockTime(ConsumeTime(fuzzed_data_provider, /*min=*/1296688602)); // regtest genesis block timestamp + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider, /*min=*/1296688602)}; // regtest genesis block timestamp auto& setup{*g_setup}; bool dirty_chainman{false}; // Reuse the global chainman, but reset it when it is dirty auto& chainman{*setup.m_node.chainman}; diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp index da04e608b051..1275fd45c43c 100644 --- a/src/test/fuzz/utxo_total_supply.cpp +++ b/src/test/fuzz/utxo_total_supply.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -24,7 +25,7 @@ FUZZ_TARGET(utxo_total_supply) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - SetMockTime(ConsumeTime(fuzzed_data_provider, /*min=*/1296688602)); // regtest genesis block timestamp + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider, /*min=*/1296688602)}; // regtest genesis block timestamp /** The testing setup that creates a chainman only (no chainstate) */ ChainTestingSetup test_setup{ ChainType::REGTEST, diff --git a/src/test/fuzz/validation_load_mempool.cpp b/src/test/fuzz/validation_load_mempool.cpp index ed036eb196f6..9180945d0e7a 100644 --- a/src/test/fuzz/validation_load_mempool.cpp +++ b/src/test/fuzz/validation_load_mempool.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include @@ -40,7 +41,7 @@ FUZZ_TARGET(validation_load_mempool, .init = initialize_validation_load_mempool) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; FuzzedFileProvider fuzzed_file_provider{fuzzed_data_provider}; bilingual_str error; diff --git a/src/wallet/test/fuzz/fees.cpp b/src/wallet/test/fuzz/fees.cpp index 5d6c67b74290..0d332c5908ba 100644 --- a/src/wallet/test/fuzz/fees.cpp +++ b/src/wallet/test/fuzz/fees.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -62,7 +63,7 @@ FUZZ_TARGET(wallet_fees, .init = initialize_setup) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; auto& node{g_setup->m_node}; Chainstate* chainstate = &node.chainman->ActiveChainstate(); diff --git a/src/wallet/test/fuzz/scriptpubkeyman.cpp b/src/wallet/test/fuzz/scriptpubkeyman.cpp index 243c9c69114d..77b913897817 100644 --- a/src/wallet/test/fuzz/scriptpubkeyman.cpp +++ b/src/wallet/test/fuzz/scriptpubkeyman.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -78,7 +79,7 @@ FUZZ_TARGET(scriptpubkeyman, .init = initialize_spkm) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; const auto& node{g_setup->m_node}; Chainstate& chainstate{node.chainman->ActiveChainstate()}; std::unique_ptr wallet_ptr{std::make_unique(node.chain.get(), "", CreateMockableWalletDatabase())}; diff --git a/src/wallet/test/fuzz/spend.cpp b/src/wallet/test/fuzz/spend.cpp index 552364a667d0..e9a6bea71ef0 100644 --- a/src/wallet/test/fuzz/spend.cpp +++ b/src/wallet/test/fuzz/spend.cpp @@ -2,20 +2,21 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include #include #include #include +#include #include +#include #include #include #include #include #include -#include -#include using util::ToString; @@ -33,7 +34,7 @@ FUZZ_TARGET(wallet_create_transaction, .init = initialize_setup) { SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - SetMockTime(ConsumeTime(fuzzed_data_provider)); + NodeClockContext clock_ctx{ConsumeTime(fuzzed_data_provider)}; const auto& node = g_setup->m_node; Chainstate& chainstate{node.chainman->ActiveChainstate()}; ArgsManager& args = *node.args; From fa8250e961cd921800f2fd306bef04e39fb566ab Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 11 Mar 2026 09:26:31 +0100 Subject: [PATCH 3/8] refactor: Add and use RPCResultOptions Initially only move skip_type_check there. In the future, more options can be added, without having to touch the constructors. --- src/rpc/blockchain.cpp | 2 +- src/rpc/util.cpp | 2 +- src/rpc/util.h | 23 ++++++++++++++--------- src/wallet/rpc/wallet.cpp | 2 +- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index d97b7c6c15c6..cdbaafe07c7f 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2739,7 +2739,7 @@ static RPCHelpMan getdescriptoractivity() {RPCResult::Type::OBJ, "output_spk", "", ScriptPubKeyDoc()}, }}, // TODO is the skip_type_check avoidable with a heterogeneous ARR? - }, /*skip_type_check=*/true}, + }, {.skip_type_check=true}, }, }, }, RPCExamples{ diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 49569c7be540..75cfa8d3af6a 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -1130,7 +1130,7 @@ static std::optional ExpectedType(RPCResult::Type type) // NOLINTNEXTLINE(misc-no-recursion) UniValue RPCResult::MatchesType(const UniValue& result) const { - if (m_skip_type_check) { + if (m_opts.skip_type_check) { return true; } diff --git a/src/rpc/util.h b/src/rpc/util.h index 37530876d734..a3f959646842 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -292,6 +292,9 @@ struct RPCArg { std::string ToDescriptionString(bool is_named_arg) const; }; +struct RPCResultOptions { + bool skip_type_check{false}; +}; // NOLINTNEXTLINE(misc-no-recursion) struct RPCResult { enum class Type { @@ -314,7 +317,7 @@ struct RPCResult { const std::string m_key_name; //!< Only used for dicts const std::vector m_inner; //!< Only used for arrays or dicts const bool m_optional; - const bool m_skip_type_check; + const RPCResultOptions m_opts; const std::string m_description; const std::string m_cond; @@ -324,12 +327,13 @@ struct RPCResult { std::string m_key_name, bool optional, std::string description, - std::vector inner = {}) + std::vector inner = {}, + RPCResultOptions opts = {}) : m_type{std::move(type)}, m_key_name{std::move(m_key_name)}, m_inner{std::move(inner)}, m_optional{optional}, - m_skip_type_check{false}, + m_opts{std::move(opts)}, m_description{std::move(description)}, m_cond{std::move(cond)} { @@ -342,8 +346,9 @@ struct RPCResult { Type type, std::string m_key_name, std::string description, - std::vector inner = {}) - : RPCResult{std::move(cond), type, std::move(m_key_name), /*optional=*/false, std::move(description), std::move(inner)} {} + std::vector inner = {}, + RPCResultOptions opts = {}) + : RPCResult{std::move(cond), type, std::move(m_key_name), /*optional=*/false, std::move(description), std::move(inner), std::move(opts)} {} RPCResult( Type type, @@ -351,12 +356,12 @@ struct RPCResult { bool optional, std::string description, std::vector inner = {}, - bool skip_type_check = false) + RPCResultOptions opts = {}) : m_type{std::move(type)}, m_key_name{std::move(m_key_name)}, m_inner{std::move(inner)}, m_optional{optional}, - m_skip_type_check{skip_type_check}, + m_opts{std::move(opts)}, m_description{std::move(description)}, m_cond{} { @@ -368,8 +373,8 @@ struct RPCResult { std::string m_key_name, std::string description, std::vector inner = {}, - bool skip_type_check = false) - : RPCResult{type, std::move(m_key_name), /*optional=*/false, std::move(description), std::move(inner), skip_type_check} {} + RPCResultOptions opts = {}) + : RPCResult{type, std::move(m_key_name), /*optional=*/false, std::move(description), std::move(inner), std::move(opts)} {} /** Append the sections of the result. */ void ToSections(Sections& sections, OuterType outer_type = OuterType::NONE, int current_indent = 0) const; diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 9ce274973a6b..c7f03bb35c16 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -53,7 +53,7 @@ static RPCHelpMan getwalletinfo() { {RPCResult::Type::NUM, "duration", "elapsed seconds since scan start"}, {RPCResult::Type::NUM, "progress", "scanning progress percentage [0.0, 1.0]"}, - }, /*skip_type_check=*/true}, + }, {.skip_type_check=true}, }, {RPCResult::Type::BOOL, "descriptors", "whether this wallet uses descriptors for output script management"}, {RPCResult::Type::BOOL, "external_signer", "whether this wallet is configured to use an external signer such as a hardware wallet"}, {RPCResult::Type::BOOL, "blank", "Whether this wallet intentionally does not contain any keys, scripts, or descriptors"}, From fa4d5891b964059a58cb0a9d016cc6791c4fe34b Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 10 Mar 2026 16:35:03 +0100 Subject: [PATCH 4/8] refactor: Introduce TxDocOptions This prepares the function to be more flexible, when more options are passed in the future. --- src/rpc/rawtransaction.cpp | 4 ++-- src/rpc/rawtransaction_util.cpp | 6 +++--- src/rpc/rawtransaction_util.h | 8 +++++++- src/wallet/rpc/transactions.cpp | 2 +- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 5885422be7c2..3632cc49ba4f 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -248,7 +248,7 @@ static RPCHelpMan getrawtransaction() {RPCResult::Type::NUM, "time", /*optional=*/true, "Same as \"blocktime\""}, {RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded data for 'txid'"}, }, - DecodeTxDoc(/*txid_field_doc=*/"The transaction id (same as provided)", /*wallet=*/false)), + TxDoc({.txid_field_doc="The transaction id (same as provided)"})), }, RPCResult{"for verbosity = 2", RPCResult::Type::OBJ, "", "", @@ -422,7 +422,7 @@ static RPCHelpMan decoderawtransaction() }, RPCResult{ RPCResult::Type::OBJ, "", "", - DecodeTxDoc(/*txid_field_doc=*/"The transaction id", /*wallet=*/false), + TxDoc(), }, RPCExamples{ HelpExampleCli("decoderawtransaction", "\"hexstring\"") diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 467289fc14c9..89fc1e271ca9 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -344,10 +344,10 @@ void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const } } -std::vector DecodeTxDoc(const std::string& txid_field_doc, bool wallet) +std::vector TxDoc(const TxDocOptions& opts) { return { - {RPCResult::Type::STR_HEX, "txid", txid_field_doc}, + {RPCResult::Type::STR_HEX, "txid", opts.txid_field_doc}, {RPCResult::Type::STR_HEX, "hash", "The transaction hash (differs from txid for witness transactions)"}, {RPCResult::Type::NUM, "size", "The serialized transaction size"}, {RPCResult::Type::NUM, "vsize", "The virtual transaction size (differs from size for witness transactions)"}, @@ -381,7 +381,7 @@ std::vector DecodeTxDoc(const std::string& txid_field_doc, bool walle {RPCResult::Type::NUM, "n", "index"}, {RPCResult::Type::OBJ, "scriptPubKey", "", ScriptPubKeyDoc()}, }, - wallet ? + opts.wallet ? std::vector{{RPCResult::Type::BOOL, "ischange", /*optional=*/true, "Output script is change (only present if true)"}} : std::vector{} ) diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index 7bd051220a70..ed1efd0b6a53 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -56,7 +56,13 @@ void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in); /** Create a transaction from univalue parameters */ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, std::optional rbf, uint32_t version); +struct TxDocOptions { + /// The description of the txid field + std::string txid_field_doc{"The transaction id"}; + /// Include wallet-related fields (e.g. ischange on outputs) + bool wallet{false}; +}; /** Explain the UniValue "decoded" transaction object, may include extra fields if processed by wallet **/ -std::vector DecodeTxDoc(const std::string& txid_field_doc, bool wallet); +std::vector TxDoc(const TxDocOptions& opts = {}); #endif // BITCOIN_RPC_RAWTRANSACTION_UTIL_H diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index 34ebdea79c73..2160562b90ed 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -705,7 +705,7 @@ RPCHelpMan gettransaction() {RPCResult::Type::STR_HEX, "hex", "Raw data for transaction"}, {RPCResult::Type::OBJ, "decoded", /*optional=*/true, "The decoded transaction (only present when `verbose` is passed)", { - DecodeTxDoc(/*txid_field_doc=*/"The transaction id", /*wallet=*/true), + TxDoc({.wallet = true}), }}, RESULT_LAST_PROCESSED_BLOCK, }) From faea12ecd9fe026876a3278782a9320934214905 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 4 Feb 2026 08:17:05 +0100 Subject: [PATCH 5/8] test: Fixup docs for NodeClockContext and SteadyClockContext --- src/test/fuzz/util/check_globals.cpp | 6 ++++-- src/util/time.h | 10 +++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/test/fuzz/util/check_globals.cpp b/src/test/fuzz/util/check_globals.cpp index f91a965afced..ca3111534d3b 100644 --- a/src/test/fuzz/util/check_globals.cpp +++ b/src/test/fuzz/util/check_globals.cpp @@ -42,8 +42,10 @@ struct CheckGlobalsImpl { std::cerr << "\n\n" "The current fuzz target accessed system time.\n\n" - "This is acceptable, but requires the fuzz target to call \n" - "SetMockTime() at the beginning of processing the fuzz input.\n\n" + "This is acceptable, but requires the fuzz target to use \n" + "a NodeClockContext, SteadyClockContext or call \n" + "SetMockTime() at the \n" "beginning of processing the \n" + "fuzz input.\n\n" "Without setting mock time, time-dependent behavior can lead \n" "to non-reproducible bugs or inefficient fuzzing.\n\n" diff --git a/src/util/time.h b/src/util/time.h index 59abc4d5a7f6..967ea780aafb 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -14,7 +14,8 @@ using namespace std::chrono_literals; -/** Mockable clock in the context of tests, otherwise the system clock */ +/// Version of the system clock that is mockable in the context of tests (via +/// NodeClockContext or ::SetMockTime), otherwise the system clock. struct NodeClock : public std::chrono::system_clock { using time_point = std::chrono::time_point; /** Return current system time or mocked time, if set */ @@ -31,10 +32,9 @@ using SteadyMicroseconds = std::chrono::time_point; From fa9168ffcd6354db6daf759ace123a88fb60cbb2 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Fri, 13 Mar 2026 10:44:47 +0100 Subject: [PATCH 6/8] test: Use asyncio.SelectorEventLoop() over deprecated asyncio.WindowsSelectorEventLoopPolicy() --- test/functional/test_framework/p2p.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index 7a7cce6176c6..bd8a1da75f3a 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -740,9 +740,7 @@ def __init__(self): NetworkThread.listeners = {} NetworkThread.protos = {} - if platform.system() == 'Windows': - asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy()) - NetworkThread.network_event_loop = asyncio.new_event_loop() + NetworkThread.network_event_loop = asyncio.SelectorEventLoop() if platform.system() == "Windows" else asyncio.new_event_loop() def run(self): """Start the network thread.""" From fa050da9805d5388ec74e2e0f92e59e861f11918 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Fri, 13 Mar 2026 10:07:58 +0100 Subject: [PATCH 7/8] test: Move event loop creation to network thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This should fix https://github.com/bitcoin/bitcoin/issues/34367 I am not familiar with Windows sockets thread-safety, but creating the event loop on the main thread, and running it in the network thread could lead to a fast abort in Python on Windows (without any stderr): ``` 77/276 - wallet_txn_clone.py failed, Duration: 1 s stdout: 2025-12-10T08:04:27.500134Z TestFramework (INFO): PRNG seed is: 4018092284830106117 stderr: Combine the logs and print the last 99999999 lines ... ============ Combined log for D:\a\_temp/test_runner_₿_🏃_20251210_075632/wallet_txn_clone_196: ============ test 2025-12-10T08:04:27.500134Z TestFramework (INFO): PRNG seed is: 4018092284830106117 test 2025-12-10T08:04:27.500433Z TestFramework (DEBUG): Setting up network thread ``` Also, I couldn't find any docs that require the loop must be created on the thread that runs them: * https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.new_event_loop * https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.run_forever However, the patch seems trivial to review, harmless, and easy to revert, so it may be a good try to fix the intermittent Windows Python crash. --- test/functional/test_framework/p2p.py | 2 +- test/functional/test_framework/test_framework.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index bd8a1da75f3a..fae4eb94c21a 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -740,10 +740,10 @@ def __init__(self): NetworkThread.listeners = {} NetworkThread.protos = {} - NetworkThread.network_event_loop = asyncio.SelectorEventLoop() if platform.system() == "Windows" else asyncio.new_event_loop() def run(self): """Start the network thread.""" + NetworkThread.network_event_loop = asyncio.SelectorEventLoop() if platform.system() == "Windows" else asyncio.new_event_loop() self.network_event_loop.run_forever() def close(self, *, timeout): diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index fb0fc0af6b8e..95bfc964d60b 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -260,7 +260,7 @@ def setup(self): self.log.debug('Setting up network thread') self.network_thread = NetworkThread() self.network_thread.start() - self.wait_until(lambda: self.network_thread.network_event_loop.is_running()) + self.wait_until(lambda: self.network_thread.network_event_loop is not None and self.network_thread.network_event_loop.is_running()) if self.options.usecli: if not self.supports_cli: From fadf901fd4b4d95bd0dd046b8d44a1154c5ea9a8 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 11 Mar 2026 11:55:41 +0100 Subject: [PATCH 8/8] rpc: Run type check on decodepsbt result For RPCResults, the type may be ELISION, which is confusing and brittle: * The elision should only affect the help output, not the type. * The type should be the real type, so that type checks can be run on it. Fix this issue by introducing a new print_elision option and using it in decodepsbt. This change will ensure that RPCResult::MatchesType is properly run. Also, this clarifies the RPC output minimally: ```diff --- a/decodepsbt +++ b/decodepsbt @@ -35,7 +35,7 @@ Result: "inputs" : [ (json array) { (json object) "non_witness_utxo" : { (json object, optional) Decoded network transaction for non-witness UTXOs - ... + ... The layout is the same as the output of decoderawtransaction. }, "witness_utxo" : { (json object, optional) Transaction output for witness UTXOs "amount" : n, (numeric) The value in BTC ``` --- src/rpc/rawtransaction.cpp | 10 ++++------ src/rpc/rawtransaction_util.cpp | 20 +++++++++++--------- src/rpc/rawtransaction_util.h | 2 ++ src/rpc/util.cpp | 17 ++++++++++++++++- src/rpc/util.h | 9 +++++++++ 5 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 3632cc49ba4f..7a202ccfcc32 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -781,9 +781,8 @@ const RPCResult decodepsbt_inputs{ {RPCResult::Type::OBJ, "", "", { {RPCResult::Type::OBJ, "non_witness_utxo", /*optional=*/true, "Decoded network transaction for non-witness UTXOs", - { - {RPCResult::Type::ELISION, "",""}, - }}, + TxDoc({.elision_description="The layout is the same as the output of decoderawtransaction."}) + }, {RPCResult::Type::OBJ, "witness_utxo", /*optional=*/true, "Transaction output for witness UTXOs", { {RPCResult::Type::NUM, "amount", "The value in " + CURRENCY_UNIT}, @@ -1023,9 +1022,8 @@ static RPCHelpMan decodepsbt() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::OBJ, "tx", "The decoded network-serialized unsigned transaction.", - { - {RPCResult::Type::ELISION, "", "The layout is the same as the output of decoderawtransaction."}, - }}, + TxDoc({.elision_description="The layout is the same as the output of decoderawtransaction."}) + }, {RPCResult::Type::ARR, "global_xpubs", "", { {RPCResult::Type::OBJ, "", "", diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 89fc1e271ca9..d3a1b875807f 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -346,14 +346,16 @@ void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::vector TxDoc(const TxDocOptions& opts) { + std::optional maybe_skip{}; + if (opts.elision_description) maybe_skip.emplace(); return { - {RPCResult::Type::STR_HEX, "txid", opts.txid_field_doc}, - {RPCResult::Type::STR_HEX, "hash", "The transaction hash (differs from txid for witness transactions)"}, - {RPCResult::Type::NUM, "size", "The serialized transaction size"}, - {RPCResult::Type::NUM, "vsize", "The virtual transaction size (differs from size for witness transactions)"}, - {RPCResult::Type::NUM, "weight", "The transaction's weight (between vsize*4-3 and vsize*4)"}, - {RPCResult::Type::NUM, "version", "The version"}, - {RPCResult::Type::NUM_TIME, "locktime", "The lock time"}, + {RPCResult::Type::STR_HEX, "txid", opts.txid_field_doc, {}, {.print_elision=opts.elision_description}}, + {RPCResult::Type::STR_HEX, "hash", "The transaction hash (differs from txid for witness transactions)", {}, {.print_elision=maybe_skip}}, + {RPCResult::Type::NUM, "size", "The serialized transaction size", {}, {.print_elision=maybe_skip}}, + {RPCResult::Type::NUM, "vsize", "The virtual transaction size (differs from size for witness transactions)", {}, {.print_elision=maybe_skip}}, + {RPCResult::Type::NUM, "weight", "The transaction's weight (between vsize*4-3 and vsize*4)", {}, {.print_elision=maybe_skip}}, + {RPCResult::Type::NUM, "version", "The version", {}, {.print_elision=maybe_skip}}, + {RPCResult::Type::NUM_TIME, "locktime", "The lock time", {}, {.print_elision=maybe_skip}}, {RPCResult::Type::ARR, "vin", "", { {RPCResult::Type::OBJ, "", "", @@ -372,7 +374,7 @@ std::vector TxDoc(const TxDocOptions& opts) }}, {RPCResult::Type::NUM, "sequence", "The script sequence number"}, }}, - }}, + }, {.print_elision=maybe_skip}}, {RPCResult::Type::ARR, "vout", "", { {RPCResult::Type::OBJ, "", "", Cat( @@ -386,6 +388,6 @@ std::vector TxDoc(const TxDocOptions& opts) std::vector{} ) }, - }}, + }, {.print_elision=maybe_skip}}, }; } diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index ed1efd0b6a53..5a1bc6047c79 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -61,6 +61,8 @@ struct TxDocOptions { std::string txid_field_doc{"The transaction id"}; /// Include wallet-related fields (e.g. ischange on outputs) bool wallet{false}; + /// Treat this as an elided Result in the help + std::optional elision_description{}; }; /** Explain the UniValue "decoded" transaction object, may include extra fields if processed by wallet **/ std::vector TxDoc(const TxDocOptions& opts = {}); diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 75cfa8d3af6a..cee8a4295bad 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -1015,9 +1015,22 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const (this->m_description.empty() ? "" : " " + this->m_description); }; + // Ensure at least one elision description exists, if there is any elision + const auto elision_has_description{[](const std::vector& inner) { + return std::ranges::none_of(inner, [](const auto& res) { return res.m_opts.print_elision.has_value(); }) || + std::ranges::any_of(inner, [](const auto& res) { return res.m_opts.print_elision.has_value() && !res.m_opts.print_elision->empty(); }); + }}; + + if (m_opts.print_elision) { + if (!m_opts.print_elision->empty()) { + sections.PushSection({indent + "..." + maybe_separator, *m_opts.print_elision}); + } + return; + } + switch (m_type) { case Type::ELISION: { - // If the inner result is empty, use three dots for elision + // Deprecated alias of m_opts.print_elision sections.PushSection({indent + "..." + maybe_separator, m_description}); return; } @@ -1059,6 +1072,7 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const i.ToSections(sections, OuterType::ARR, current_indent + 2); } CHECK_NONFATAL(!m_inner.empty()); + CHECK_NONFATAL(elision_has_description(m_inner)); if (m_type == Type::ARR && m_inner.back().m_type != Type::ELISION) { sections.PushSection({indent_next + "...", ""}); } else { @@ -1074,6 +1088,7 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const sections.PushSection({indent + maybe_key + "{}", Description("empty JSON object")}); return; } + CHECK_NONFATAL(elision_has_description(m_inner)); sections.PushSection({indent + maybe_key + "{", Description("json object")}); for (const auto& i : m_inner) { i.ToSections(sections, OuterType::OBJ, current_indent + 2); diff --git a/src/rpc/util.h b/src/rpc/util.h index a3f959646842..fda71072f3b7 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -294,6 +294,15 @@ struct RPCArg { struct RPCResultOptions { bool skip_type_check{false}; + /// Whether to treat this as elided in the human-readable description, and + /// possibly supply a description for the elision. Normally, there will be + /// one string on any of the elided results, for example `Same output as + /// verbosity = 1`, and all other elided strings will be empty. + /// + /// - If nullopt: normal display. + /// - If empty string: suppress from help. + /// - If non-empty: show "..." with this description. + std::optional print_elision{std::nullopt}; }; // NOLINTNEXTLINE(misc-no-recursion) struct RPCResult {