From 33381ea530ad79ac1e04c37f5707e93d3e0509ca Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Thu, 12 Sep 2024 15:55:12 +0200 Subject: [PATCH 01/36] scripted-diff: Modernize nLocalServices to m_local_services -BEGIN VERIFY SCRIPT- sed -i 's/nLocalServices/m_local_services/g' src/net.h src/net.cpp sed -i 's/connOptions.nLocalServices/connOptions.m_local_services/g' src/init.cpp sed -i 's/nLocalServices/g_local_services/g' src/init.cpp -END VERIFY SCRIPT- --- src/init.cpp | 12 ++++++------ src/net.cpp | 4 ++-- src/net.h | 10 +++++----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index f67835d7da..b01a493640 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -840,7 +840,7 @@ namespace { // Variables internal to initialization process only int nMaxConnections; int available_fds; -ServiceFlags nLocalServices = ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS); +ServiceFlags g_local_services = ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS); int64_t peer_connect_timeout; std::set g_enabled_filter_types; @@ -955,7 +955,7 @@ bool AppInitParameterInteraction(const ArgsManager& args) // Signal NODE_P2P_V2 if BIP324 v2 transport is enabled. if (args.GetBoolArg("-v2transport", DEFAULT_V2_TRANSPORT)) { - nLocalServices = ServiceFlags(nLocalServices | NODE_P2P_V2); + g_local_services = ServiceFlags(g_local_services | NODE_P2P_V2); } // Signal NODE_COMPACT_FILTERS if peerblockfilters and basic filters index are both enabled. @@ -964,7 +964,7 @@ bool AppInitParameterInteraction(const ArgsManager& args) return InitError(_("Cannot set -peerblockfilters without -blockfilterindex.")); } - nLocalServices = ServiceFlags(nLocalServices | NODE_COMPACT_FILTERS); + g_local_services = ServiceFlags(g_local_services | NODE_COMPACT_FILTERS); } if (args.GetIntArg("-prune", 0)) { @@ -1049,7 +1049,7 @@ bool AppInitParameterInteraction(const ArgsManager& args) SetMockTime(args.GetIntArg("-mocktime", 0)); // SetMockTime(0) is a no-op if (args.GetBoolArg("-peerbloomfilters", DEFAULT_PEERBLOOMFILTERS)) - nLocalServices = ServiceFlags(nLocalServices | NODE_BLOOM); + g_local_services = ServiceFlags(g_local_services | NODE_BLOOM); if (args.IsArgSet("-test")) { if (chainparams.GetChainType() != ChainType::REGTEST) { @@ -1724,7 +1724,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // Prior to setting NODE_NETWORK, check if we can provide historical blocks. if (!WITH_LOCK(chainman.GetMutex(), return chainman.BackgroundSyncInProgress())) { LogPrintf("Setting NODE_NETWORK on non-prune mode\n"); - nLocalServices = ServiceFlags(nLocalServices | NODE_NETWORK); + g_local_services = ServiceFlags(g_local_services | NODE_NETWORK); } else { LogPrintf("Running node in NODE_NETWORK_LIMITED mode until snapshot background sync completes\n"); } @@ -1856,7 +1856,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) StartMapPort(args.GetBoolArg("-upnp", DEFAULT_UPNP), args.GetBoolArg("-natpmp", DEFAULT_NATPMP)); CConnman::Options connOptions; - connOptions.nLocalServices = nLocalServices; + connOptions.m_local_services = g_local_services; connOptions.m_max_automatic_connections = nMaxConnections; connOptions.uiInterface = &uiInterface; connOptions.m_banman = node.banman.get(); diff --git a/src/net.cpp b/src/net.cpp index 257f67ed7b..5cfaec39b8 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2945,7 +2945,7 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai return; pnode->grantOutbound = std::move(grant_outbound); - m_msgproc->InitializeNode(*pnode, nLocalServices); + m_msgproc->InitializeNode(*pnode, m_local_services); { LOCK(m_nodes_mutex); m_nodes.push_back(pnode); @@ -3742,7 +3742,7 @@ uint64_t CConnman::GetTotalBytesSent() const ServiceFlags CConnman::GetLocalServices() const { - return nLocalServices; + return m_local_services; } static std::unique_ptr MakeTransport(NodeId id, bool use_v2transport, bool inbound) noexcept diff --git a/src/net.h b/src/net.h index 2f65a01ce1..686dded775 100644 --- a/src/net.h +++ b/src/net.h @@ -1035,7 +1035,7 @@ class CConnman struct Options { - ServiceFlags nLocalServices = NODE_NONE; + ServiceFlags m_local_services = NODE_NONE; int m_max_automatic_connections = 0; CClientUIInterface* uiInterface = nullptr; NetEventsInterface* m_msgproc = nullptr; @@ -1065,7 +1065,7 @@ class CConnman { AssertLockNotHeld(m_total_bytes_sent_mutex); - nLocalServices = connOptions.nLocalServices; + m_local_services = connOptions.m_local_services; m_max_automatic_connections = connOptions.m_max_automatic_connections; m_max_outbound_full_relay = std::min(MAX_OUTBOUND_FULL_RELAY_CONNECTIONS, m_max_automatic_connections); m_max_outbound_block_relay = std::min(MAX_BLOCK_RELAY_ONLY_CONNECTIONS, m_max_automatic_connections - m_max_outbound_full_relay); @@ -1223,8 +1223,8 @@ class CConnman //! Updates the local services that this node advertises to other peers //! during connection handshake. - void AddLocalServices(ServiceFlags services) { nLocalServices = ServiceFlags(nLocalServices | services); }; - void RemoveLocalServices(ServiceFlags services) { nLocalServices = ServiceFlags(nLocalServices & ~services); } + void AddLocalServices(ServiceFlags services) { m_local_services = ServiceFlags(m_local_services | services); }; + void RemoveLocalServices(ServiceFlags services) { m_local_services = ServiceFlags(m_local_services & ~services); } uint64_t GetMaxOutboundTarget() const EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); std::chrono::seconds GetMaxOutboundTimeframe() const; @@ -1470,7 +1470,7 @@ class CConnman * * \sa Peer::our_services */ - std::atomic nLocalServices; + std::atomic m_local_services; std::unique_ptr semOutbound; std::unique_ptr semAddnode; From fae44c83da982095661b034bdd01afe8ac2fb0a6 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 18 Sep 2024 09:00:20 +0200 Subject: [PATCH 02/36] test: Remove 0.16.3 test from wallet_backwards_compatibility.py --- .../wallet_backwards_compatibility.py | 27 +++++-------------- test/functional/wallet_upgradewallet.py | 1 + 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/test/functional/wallet_backwards_compatibility.py b/test/functional/wallet_backwards_compatibility.py index e71283b928..775786fbb1 100755 --- a/test/functional/wallet_backwards_compatibility.py +++ b/test/functional/wallet_backwards_compatibility.py @@ -33,7 +33,7 @@ def add_options(self, parser): def set_test_params(self): self.setup_clean_chain = True - self.num_nodes = 12 + self.num_nodes = 11 # Add new version after each release: self.extra_args = [ ["-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # Pre-release: use to mine blocks. noban for immediate tx relay @@ -47,7 +47,6 @@ def set_test_params(self): ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v0.19.1 ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=127.0.0.1"], # v0.18.1 ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=127.0.0.1"], # v0.17.2 - ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=127.0.0.1", "-wallet=wallet.dat"], # v0.16.3 ] self.wallet_names = [self.default_wallet_name] @@ -68,7 +67,6 @@ def setup_nodes(self): 190100, 180100, 170200, - 160300, ]) self.start_nodes() @@ -133,18 +131,17 @@ def test_v19_addmultisigaddress(self): def run_test(self): node_miner = self.nodes[0] node_master = self.nodes[1] - node_v21 = self.nodes[self.num_nodes - 6] - node_v17 = self.nodes[self.num_nodes - 2] - node_v16 = self.nodes[self.num_nodes - 1] + node_v21 = self.nodes[self.num_nodes - 5] + node_v17 = self.nodes[self.num_nodes - 1] legacy_nodes = self.nodes[2:] # Nodes that support legacy wallets - legacy_only_nodes = self.nodes[-5:] # Nodes that only support legacy wallets - descriptors_nodes = self.nodes[2:-5] # Nodes that support descriptor wallets + legacy_only_nodes = self.nodes[-4:] # Nodes that only support legacy wallets + descriptors_nodes = self.nodes[2:-4] # Nodes that support descriptor wallets self.generatetoaddress(node_miner, COINBASE_MATURITY + 1, node_miner.getnewaddress()) # Sanity check the test framework: - res = node_v16.getblockchaininfo() + res = node_v17.getblockchaininfo() assert_equal(res['blocks'], COINBASE_MATURITY + 1) self.log.info("Test wallet backwards compatibility...") @@ -215,9 +212,6 @@ def run_test(self): # In descriptors wallet mode, run this test on the nodes that support descriptor wallets # In legacy wallets mode, run this test on the nodes that support legacy wallets for node in descriptors_nodes if self.options.descriptors else legacy_nodes: - if self.major_version_less_than(node, 17): - # loadwallet was introduced in v0.17.0 - continue self.log.info(f"- {node.version}") for wallet_name in ["w1", "w2", "w3"]: if self.major_version_less_than(node, 18) and wallet_name == "w3": @@ -290,15 +284,6 @@ def run_test(self): node_v17.assert_start_raises_init_error(["-wallet=w3"], "Error: Error loading w3: Wallet requires newer version of Bitcoin Core") self.start_node(node_v17.index) - # No wallet created in master can be opened in 0.16 - self.log.info("Test that wallets created in master are too new for 0.16") - self.stop_node(node_v16.index) - for wallet_name in ["w1", "w2", "w3"]: - if self.options.descriptors: - node_v16.assert_start_raises_init_error([f"-wallet={wallet_name}"], f"Error: {wallet_name} corrupt, salvage failed") - else: - node_v16.assert_start_raises_init_error([f"-wallet={wallet_name}"], f"Error: Error loading {wallet_name}: Wallet requires newer version of Bitcoin Core") - # When descriptors are enabled, w1 cannot be opened by 0.21 since it contains a taproot descriptor if self.options.descriptors: self.log.info("Test that 0.21 cannot open wallet containing tr() descriptors") diff --git a/test/functional/wallet_upgradewallet.py b/test/functional/wallet_upgradewallet.py index ef3f925ee8..c909336a25 100755 --- a/test/functional/wallet_upgradewallet.py +++ b/test/functional/wallet_upgradewallet.py @@ -185,6 +185,7 @@ def copy_split_hd(): self.restart_node(0) copy_v16() wallet = node_master.get_wallet_rpc(self.default_wallet_name) + assert_equal(wallet.getbalance(), v16_3_balance) self.log.info("Test upgradewallet without a version argument") self.test_upgradewallet(wallet, previous_version=159900, expected_version=169900) # wallet should still contain the same balance From ccd10fdb97f9b8268a5cd60d7461967cfe536f16 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Wed, 25 Sep 2024 16:03:22 +0000 Subject: [PATCH 03/36] build: Add missing USDT header dependency to kernel --- src/kernel/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/kernel/CMakeLists.txt b/src/kernel/CMakeLists.txt index 65d753b3af..7bf8efc516 100644 --- a/src/kernel/CMakeLists.txt +++ b/src/kernel/CMakeLists.txt @@ -84,6 +84,7 @@ target_link_libraries(bitcoinkernel bitcoin_crypto leveldb secp256k1 + $ PUBLIC Boost::headers ) From 63d6ad7c89cd682f6e779968c4861ea085058356 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 13 Aug 2024 12:49:08 +0200 Subject: [PATCH 04/36] Move BlockMerkleBranch back to merkle.{h,cpp} The Mining interface uses this function in the next commit to calculate the coinbase merkle path. Stratum v2 uses this to send a compact work template. This partially undoes the change in 4defdfab94504018f822dc34a313ad26cedc8255, but is not a revert, because the implementation changed in the meantime. This commit also documents the function. --- src/consensus/merkle.cpp | 103 +++++++++++++++++++++++++++++++++++++ src/consensus/merkle.h | 10 ++++ src/test/merkle_tests.cpp | 104 -------------------------------------- 3 files changed, 113 insertions(+), 104 deletions(-) diff --git a/src/consensus/merkle.cpp b/src/consensus/merkle.cpp index af01902c92..dc32f0ab80 100644 --- a/src/consensus/merkle.cpp +++ b/src/consensus/merkle.cpp @@ -83,3 +83,106 @@ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated) return ComputeMerkleRoot(std::move(leaves), mutated); } +/* This implements a constant-space merkle root/path calculator, limited to 2^32 leaves. */ +static void MerkleComputation(const std::vector& leaves, uint256* proot, bool* pmutated, uint32_t branchpos, std::vector* pbranch) { + if (pbranch) pbranch->clear(); + if (leaves.size() == 0) { + if (pmutated) *pmutated = false; + if (proot) *proot = uint256(); + return; + } + bool mutated = false; + // count is the number of leaves processed so far. + uint32_t count = 0; + // inner is an array of eagerly computed subtree hashes, indexed by tree + // level (0 being the leaves). + // For example, when count is 25 (11001 in binary), inner[4] is the hash of + // the first 16 leaves, inner[3] of the next 8 leaves, and inner[0] equal to + // the last leaf. The other inner entries are undefined. + uint256 inner[32]; + // Which position in inner is a hash that depends on the matching leaf. + int matchlevel = -1; + // First process all leaves into 'inner' values. + while (count < leaves.size()) { + uint256 h = leaves[count]; + bool matchh = count == branchpos; + count++; + int level; + // For each of the lower bits in count that are 0, do 1 step. Each + // corresponds to an inner value that existed before processing the + // current leaf, and each needs a hash to combine it. + for (level = 0; !(count & ((uint32_t{1}) << level)); level++) { + if (pbranch) { + if (matchh) { + pbranch->push_back(inner[level]); + } else if (matchlevel == level) { + pbranch->push_back(h); + matchh = true; + } + } + mutated |= (inner[level] == h); + h = Hash(inner[level], h); + } + // Store the resulting hash at inner position level. + inner[level] = h; + if (matchh) { + matchlevel = level; + } + } + // Do a final 'sweep' over the rightmost branch of the tree to process + // odd levels, and reduce everything to a single top value. + // Level is the level (counted from the bottom) up to which we've sweeped. + int level = 0; + // As long as bit number level in count is zero, skip it. It means there + // is nothing left at this level. + while (!(count & ((uint32_t{1}) << level))) { + level++; + } + uint256 h = inner[level]; + bool matchh = matchlevel == level; + while (count != ((uint32_t{1}) << level)) { + // If we reach this point, h is an inner value that is not the top. + // We combine it with itself (Bitcoin's special rule for odd levels in + // the tree) to produce a higher level one. + if (pbranch && matchh) { + pbranch->push_back(h); + } + h = Hash(h, h); + // Increment count to the value it would have if two entries at this + // level had existed. + count += ((uint32_t{1}) << level); + level++; + // And propagate the result upwards accordingly. + while (!(count & ((uint32_t{1}) << level))) { + if (pbranch) { + if (matchh) { + pbranch->push_back(inner[level]); + } else if (matchlevel == level) { + pbranch->push_back(h); + matchh = true; + } + } + h = Hash(inner[level], h); + level++; + } + } + // Return result. + if (pmutated) *pmutated = mutated; + if (proot) *proot = h; +} + +static std::vector ComputeMerkleBranch(const std::vector& leaves, uint32_t position) { + std::vector ret; + MerkleComputation(leaves, nullptr, nullptr, position, &ret); + return ret; +} + +std::vector BlockMerkleBranch(const CBlock& block, uint32_t position) +{ + std::vector leaves; + leaves.resize(block.vtx.size()); + for (size_t s = 0; s < block.vtx.size(); s++) { + leaves[s] = block.vtx[s]->GetHash(); + } + return ComputeMerkleBranch(leaves, position); +} diff --git a/src/consensus/merkle.h b/src/consensus/merkle.h index 4ae5a5b897..363f68039c 100644 --- a/src/consensus/merkle.h +++ b/src/consensus/merkle.h @@ -24,4 +24,14 @@ uint256 BlockMerkleRoot(const CBlock& block, bool* mutated = nullptr); */ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated = nullptr); +/** + * Compute merkle path to the specified transaction + * + * @param[in] block the block + * @param[in] position transaction for which to calculate the merkle path, defaults to coinbase + * + * @return merkle path ordered from the deepest + */ +std::vector BlockMerkleBranch(const CBlock& block, uint32_t position = 0); + #endif // BITCOIN_CONSENSUS_MERKLE_H diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp index 70308cb29a..2b1cf8595d 100644 --- a/src/test/merkle_tests.cpp +++ b/src/test/merkle_tests.cpp @@ -23,110 +23,6 @@ static uint256 ComputeMerkleRootFromBranch(const uint256& leaf, const std::vecto return hash; } -/* This implements a constant-space merkle root/path calculator, limited to 2^32 leaves. */ -static void MerkleComputation(const std::vector& leaves, uint256* proot, bool* pmutated, uint32_t branchpos, std::vector* pbranch) { - if (pbranch) pbranch->clear(); - if (leaves.size() == 0) { - if (pmutated) *pmutated = false; - if (proot) *proot = uint256(); - return; - } - bool mutated = false; - // count is the number of leaves processed so far. - uint32_t count = 0; - // inner is an array of eagerly computed subtree hashes, indexed by tree - // level (0 being the leaves). - // For example, when count is 25 (11001 in binary), inner[4] is the hash of - // the first 16 leaves, inner[3] of the next 8 leaves, and inner[0] equal to - // the last leaf. The other inner entries are undefined. - uint256 inner[32]; - // Which position in inner is a hash that depends on the matching leaf. - int matchlevel = -1; - // First process all leaves into 'inner' values. - while (count < leaves.size()) { - uint256 h = leaves[count]; - bool matchh = count == branchpos; - count++; - int level; - // For each of the lower bits in count that are 0, do 1 step. Each - // corresponds to an inner value that existed before processing the - // current leaf, and each needs a hash to combine it. - for (level = 0; !(count & ((uint32_t{1}) << level)); level++) { - if (pbranch) { - if (matchh) { - pbranch->push_back(inner[level]); - } else if (matchlevel == level) { - pbranch->push_back(h); - matchh = true; - } - } - mutated |= (inner[level] == h); - h = Hash(inner[level], h); - } - // Store the resulting hash at inner position level. - inner[level] = h; - if (matchh) { - matchlevel = level; - } - } - // Do a final 'sweep' over the rightmost branch of the tree to process - // odd levels, and reduce everything to a single top value. - // Level is the level (counted from the bottom) up to which we've sweeped. - int level = 0; - // As long as bit number level in count is zero, skip it. It means there - // is nothing left at this level. - while (!(count & ((uint32_t{1}) << level))) { - level++; - } - uint256 h = inner[level]; - bool matchh = matchlevel == level; - while (count != ((uint32_t{1}) << level)) { - // If we reach this point, h is an inner value that is not the top. - // We combine it with itself (Bitcoin's special rule for odd levels in - // the tree) to produce a higher level one. - if (pbranch && matchh) { - pbranch->push_back(h); - } - h = Hash(h, h); - // Increment count to the value it would have if two entries at this - // level had existed. - count += ((uint32_t{1}) << level); - level++; - // And propagate the result upwards accordingly. - while (!(count & ((uint32_t{1}) << level))) { - if (pbranch) { - if (matchh) { - pbranch->push_back(inner[level]); - } else if (matchlevel == level) { - pbranch->push_back(h); - matchh = true; - } - } - h = Hash(inner[level], h); - level++; - } - } - // Return result. - if (pmutated) *pmutated = mutated; - if (proot) *proot = h; -} - -static std::vector ComputeMerkleBranch(const std::vector& leaves, uint32_t position) { - std::vector ret; - MerkleComputation(leaves, nullptr, nullptr, position, &ret); - return ret; -} - -static std::vector BlockMerkleBranch(const CBlock& block, uint32_t position) -{ - std::vector leaves; - leaves.resize(block.vtx.size()); - for (size_t s = 0; s < block.vtx.size(); s++) { - leaves[s] = block.vtx[s]->GetHash(); - } - return ComputeMerkleBranch(leaves, position); -} - // Older version of the merkle root computation code, for comparison. static uint256 BlockBuildMerkleTree(const CBlock& block, bool* fMutated, std::vector& vMerkleTree) { From 47b4875ef050c4a41bd04398021af8d605415cab Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 12 Jul 2024 16:08:38 +0200 Subject: [PATCH 05/36] Add getCoinbaseMerklePath() to Mining interface --- src/interfaces/mining.h | 7 +++++++ src/ipc/capnp/mining.capnp | 1 + src/node/interfaces.cpp | 6 ++++++ 3 files changed, 14 insertions(+) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 421c5ba762..209b629aaf 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -42,6 +42,13 @@ class BlockTemplate virtual CTransactionRef getCoinbaseTx() = 0; virtual std::vector getCoinbaseCommitment() = 0; virtual int getWitnessCommitmentIndex() = 0; + + /** + * Compute merkle path to the coinbase transaction + * + * @return merkle path ordered from the deepest + */ + virtual std::vector getCoinbaseMerklePath() = 0; }; //! Interface giving clients (RPC, Stratum v2 Template Provider in the future) diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp index 4ea69d16c9..fe7b125efa 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -31,6 +31,7 @@ interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") { getCoinbaseTx @4 (context: Proxy.Context) -> (result: Data); getCoinbaseCommitment @5 (context: Proxy.Context) -> (result: Data); getWitnessCommitmentIndex @6 (context: Proxy.Context) -> (result: Int32); + getCoinbaseMerklePath @7 (context: Proxy.Context) -> (result: List(Data)); } struct BlockCreateOptions $Proxy.wrap("node::BlockCreateOptions") { diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 1fedcd6cfa..8ed06bd00d 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -910,6 +911,11 @@ class BlockTemplateImpl : public BlockTemplate return GetWitnessCommitmentIndex(m_block_template->block); } + std::vector getCoinbaseMerklePath() override + { + return BlockMerkleBranch(m_block_template->block); + } + const std::unique_ptr m_block_template; }; From 525e9dcba0b8c6744bcd3725864f39786afc8ed5 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 15 Jul 2024 15:31:35 +0200 Subject: [PATCH 06/36] Add submitSolution to BlockTemplate interface --- src/interfaces/mining.h | 7 +++++++ src/ipc/capnp/mining.capnp | 1 + src/node/interfaces.cpp | 29 +++++++++++++++++++++++++++-- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 209b629aaf..2b6c001667 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -49,6 +49,13 @@ class BlockTemplate * @return merkle path ordered from the deepest */ virtual std::vector getCoinbaseMerklePath() = 0; + + /** + * Construct and broadcast the block. + * + * @returns if the block was processed, independent of block validity + */ + virtual bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CMutableTransaction coinbase) = 0; }; //! Interface giving clients (RPC, Stratum v2 Template Provider in the future) diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp index fe7b125efa..5e0216acea 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -32,6 +32,7 @@ interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") { getCoinbaseCommitment @5 (context: Proxy.Context) -> (result: Data); getWitnessCommitmentIndex @6 (context: Proxy.Context) -> (result: Int32); getCoinbaseMerklePath @7 (context: Proxy.Context) -> (result: List(Data)); + submitSolution@8 (context: Proxy.Context, version: UInt32, timestamp: UInt32, nonce: UInt32, coinbase :Data) -> (result: Bool); } struct BlockCreateOptions $Proxy.wrap("node::BlockCreateOptions") { diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 8ed06bd00d..67ebc24242 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -871,7 +871,7 @@ class ChainImpl : public Chain class BlockTemplateImpl : public BlockTemplate { public: - explicit BlockTemplateImpl(std::unique_ptr block_template) : m_block_template(std::move(block_template)) + explicit BlockTemplateImpl(std::unique_ptr block_template, NodeContext& node) : m_block_template(std::move(block_template)), m_node(node) { assert(m_block_template); } @@ -916,7 +916,32 @@ class BlockTemplateImpl : public BlockTemplate return BlockMerkleBranch(m_block_template->block); } + bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CMutableTransaction coinbase) override + { + CBlock block{m_block_template->block}; + + auto cb = MakeTransactionRef(std::move(coinbase)); + + if (block.vtx.size() == 0) { + block.vtx.push_back(cb); + } else { + block.vtx[0] = cb; + } + + block.nVersion = version; + block.nTime = timestamp; + block.nNonce = nonce; + + block.hashMerkleRoot = BlockMerkleRoot(block); + + auto block_ptr = std::make_shared(block); + return chainman().ProcessNewBlock(block_ptr, /*force_processing=*/true, /*min_pow_checked=*/true, /*new_block=*/nullptr); + } + const std::unique_ptr m_block_template; + + ChainstateManager& chainman() { return *Assert(m_node.chainman); } + NodeContext& m_node; }; class MinerImpl : public Mining @@ -990,7 +1015,7 @@ class MinerImpl : public Mining { BlockAssembler::Options assemble_options{options}; ApplyArgsManOptions(*Assert(m_node.args), assemble_options); - return std::make_unique(BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), assemble_options}.CreateNewBlock(script_pub_key)); + return std::make_unique(BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), assemble_options}.CreateNewBlock(script_pub_key), m_node); } NodeContext* context() override { return &m_node; } From fa4c0750331f36121ba92bbc2f22c615b7934e52 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 25 Sep 2024 09:59:05 +0200 Subject: [PATCH 07/36] doc: Clarify waitTipChanged docs It should be obvious that a wait is not needed if the tip does not match. Also, remove a comment that the blockTip notification was only meant for the "UI". (It is used by other stuff for a long time) --- src/interfaces/mining.h | 3 +-- src/validation.cpp | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 421c5ba762..2977811aa6 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -64,8 +64,7 @@ class Mining * Waits for the tip to change * * @param[in] current_tip block hash of the current chain tip. Function waits - * for the chain tip to change if this matches, otherwise - * it returns right away. + * for the chain tip to differ from this. * @param[in] timeout how long to wait for a new tip * @returns Hash and height of the current chain tip after this call. */ diff --git a/src/validation.cpp b/src/validation.cpp index 5da3387ad2..d4ec42015b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3533,7 +3533,6 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< m_chainman.m_options.signals->UpdatedBlockTip(pindexNewTip, pindexFork, still_in_ibd); } - // Always notify the UI if a new block tip was connected if (kernel::IsInterrupted(m_chainman.GetNotifications().blockTip(GetSynchronizationState(still_in_ibd, m_chainman.m_blockman.m_blockfiles_indexed), *pindexNewTip))) { // Just breaking and returning success for now. This could // be changed to bubble up the kernel::Interrupted value to From fa18586c29d4faba3d3f478eaabf9c4cbc1a16e2 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 25 Sep 2024 10:35:15 +0200 Subject: [PATCH 08/36] refactor: Add missing GUARDED_BY(m_tip_block_mutex) Found by Cory Fields in https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1774001261 --- src/node/kernel_notifications.h | 2 +- src/test/validation_chainstate_tests.cpp | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/node/kernel_notifications.h b/src/node/kernel_notifications.h index 9ff980ea56..71947e5f98 100644 --- a/src/node/kernel_notifications.h +++ b/src/node/kernel_notifications.h @@ -60,7 +60,7 @@ class KernelNotifications : public kernel::Notifications Mutex m_tip_block_mutex; std::condition_variable m_tip_block_cv; //! The block for which the last blockTip notification was received for. - uint256 m_tip_block; + uint256 m_tip_block GUARDED_BY(m_tip_block_mutex); private: util::SignalInterrupt& m_shutdown; diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 702af6578d..c9cca8af04 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -70,14 +70,18 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) { ChainstateManager& chainman = *Assert(m_node.chainman); - uint256 curr_tip = m_node.notifications->m_tip_block; + const auto get_notify_tip{[&]() { + LOCK(m_node.notifications->m_tip_block_mutex); + return m_node.notifications->m_tip_block; + }}; + uint256 curr_tip = get_notify_tip(); // Mine 10 more blocks, putting at us height 110 where a valid assumeutxo value can // be found. mineBlocks(10); // After adding some blocks to the tip, best block should have changed. - BOOST_CHECK(m_node.notifications->m_tip_block != curr_tip); + BOOST_CHECK(get_notify_tip() != curr_tip); // Grab block 1 from disk; we'll add it to the background chain later. std::shared_ptr pblockone = std::make_shared(); @@ -92,15 +96,15 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) // Ensure our active chain is the snapshot chainstate. BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.IsSnapshotActive())); - curr_tip = m_node.notifications->m_tip_block; + curr_tip = get_notify_tip(); // Mine a new block on top of the activated snapshot chainstate. mineBlocks(1); // Defined in TestChain100Setup. // After adding some blocks to the snapshot tip, best block should have changed. - BOOST_CHECK(m_node.notifications->m_tip_block != curr_tip); + BOOST_CHECK(get_notify_tip() != curr_tip); - curr_tip = m_node.notifications->m_tip_block; + curr_tip = get_notify_tip(); BOOST_CHECK_EQUAL(chainman.GetAll().size(), 2); @@ -136,10 +140,10 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) // Ensure tip is as expected BOOST_CHECK_EQUAL(background_cs.m_chain.Tip()->GetBlockHash(), pblockone->GetHash()); - // g_best_block should be unchanged after adding a block to the background + // get_notify_tip() should be unchanged after adding a block to the background // validation chain. BOOST_CHECK(block_added); - BOOST_CHECK_EQUAL(curr_tip, m_node.notifications->m_tip_block); + BOOST_CHECK_EQUAL(curr_tip, get_notify_tip()); } BOOST_AUTO_TEST_SUITE_END() From fad8e7fba7bf2c523a191309d392cab79668264b Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 30 Sep 2024 11:53:05 +0200 Subject: [PATCH 09/36] bugfix: Mark m_tip_block_cv as guarded by m_tip_block_mutex This is not strictly required, but all places using m_tip_block_cv (except shutdown) already take the lock. The annotation makes it easier to catch potential deadlocks before review. Adding the missing lock to the shutdown sequence is a bugfix. An alternative would be to take the lock and release it before notifying, see https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1778899716 --- src/node/kernel_notifications.h | 2 +- src/rpc/server.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node/kernel_notifications.h b/src/node/kernel_notifications.h index 71947e5f98..12017f2b2c 100644 --- a/src/node/kernel_notifications.h +++ b/src/node/kernel_notifications.h @@ -58,7 +58,7 @@ class KernelNotifications : public kernel::Notifications bool m_shutdown_on_fatal_error{true}; Mutex m_tip_block_mutex; - std::condition_variable m_tip_block_cv; + std::condition_variable m_tip_block_cv GUARDED_BY(m_tip_block_mutex); //! The block for which the last blockTip notification was received for. uint256 m_tip_block GUARDED_BY(m_tip_block_mutex); diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 086f609ac3..ab2135a919 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -305,7 +305,7 @@ void StopRPC(const std::any& context) DeleteAuthCookie(); node::NodeContext& node = EnsureAnyNodeContext(context); // The notifications interface doesn't exist between initialization step 4a and 7. - if (node.notifications) node.notifications->m_tip_block_cv.notify_all(); + if (node.notifications) WITH_LOCK(node.notifications->m_tip_block_mutex, node.notifications->m_tip_block_cv.notify_all()); LogDebug(BCLog::RPC, "RPC stopped.\n"); }); } From 5ca28ef28bcca1775ff49921fc2528d9439b71ab Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 25 Sep 2024 10:45:34 +0200 Subject: [PATCH 10/36] refactor: Split up NodeContext shutdown_signal and shutdown_request Instead of having a single NodeContext::shutdown member that is used both to request shutdowns and check if they have been requested, use separate members for each. Benefits of this change: 1. Should make code a little clearer and easier to search because it is easier to see which parts of code are triggering shutdowns and which parts are just checking to see if they were triggered. 2. Makes it possible for init.cpp to specify additional code to run when a shutdown is requested, like signalling the m_tip_block_cv condition variable. Motivation for this change was to remove hacky NodeContext argument and m_tip_block_cv access from the StopRPC function, so StopRPC can just be concerned with RPC functionality, not other node functionality. --- src/bitcoind.cpp | 2 +- src/index/base.cpp | 2 +- src/init.cpp | 25 ++++++++++++------- src/node/abort.cpp | 4 +-- src/node/abort.h | 7 ++---- src/node/context.h | 4 ++- src/node/interfaces.cpp | 6 +++-- src/node/kernel_notifications.cpp | 6 ++--- src/node/kernel_notifications.h | 11 +++----- src/rpc/server.cpp | 7 ++---- src/rpc/server.h | 3 +-- src/test/blockfilter_index_tests.cpp | 2 +- src/test/blockmanager_tests.cpp | 8 +++--- src/test/coinstatsindex_tests.cpp | 4 +-- src/test/txindex_tests.cpp | 2 +- src/test/util/setup_common.cpp | 7 +++--- .../validation_chainstatemanager_tests.cpp | 4 +-- 17 files changed, 53 insertions(+), 51 deletions(-) diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 1e27924dfd..9d08c42558 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -274,7 +274,7 @@ MAIN_FUNCTION if (ProcessInitCommands(args)) return EXIT_SUCCESS; // Start application - if (!AppInit(node) || !Assert(node.shutdown)->wait()) { + if (!AppInit(node) || !Assert(node.shutdown_signal)->wait()) { node.exit_status = EXIT_FAILURE; } Interrupt(node); diff --git a/src/index/base.cpp b/src/index/base.cpp index 810358394a..1a7eb9cd5e 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -31,7 +31,7 @@ template void BaseIndex::FatalErrorf(util::ConstevalFormatString fmt, const Args&... args) { auto message = tfm::format(fmt, args...); - node::AbortNode(m_chain->context()->shutdown, m_chain->context()->exit_status, Untranslated(message), m_chain->context()->warnings.get()); + node::AbortNode(m_chain->context()->shutdown_request, m_chain->context()->exit_status, Untranslated(message), m_chain->context()->warnings.get()); } CBlockLocator GetLocator(interfaces::Chain& chain, const uint256& block_hash) diff --git a/src/init.cpp b/src/init.cpp index 3b8474dd46..433a2d0748 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -207,7 +207,14 @@ void InitContext(NodeContext& node) g_shutdown.emplace(); node.args = &gArgs; - node.shutdown = &*g_shutdown; + node.shutdown_signal = &*g_shutdown; + node.shutdown_request = [&node] { + assert(node.shutdown_signal); + if (!(*node.shutdown_signal)()) return false; + // Wake any threads that may be waiting for the tip to change. + if (node.notifications) WITH_LOCK(node.notifications->m_tip_block_mutex, node.notifications->m_tip_block_cv.notify_all()); + return true; + }; } ////////////////////////////////////////////////////////////////////////////// @@ -235,7 +242,7 @@ void InitContext(NodeContext& node) bool ShutdownRequested(node::NodeContext& node) { - return bool{*Assert(node.shutdown)}; + return bool{*Assert(node.shutdown_signal)}; } #if HAVE_SYSTEM @@ -286,7 +293,7 @@ void Shutdown(NodeContext& node) StopHTTPRPC(); StopREST(); - StopRPC(&node); + StopRPC(); StopHTTPServer(); for (const auto& client : node.chain_clients) { client->flush(); @@ -707,7 +714,7 @@ static void StartupNotify(const ArgsManager& args) static bool AppInitServers(NodeContext& node) { const ArgsManager& args = *Assert(node.args); - if (!InitHTTPServer(*Assert(node.shutdown))) { + if (!InitHTTPServer(*Assert(node.shutdown_signal))) { return false; } StartRPC(); @@ -1216,7 +1223,7 @@ static ChainstateLoadResult InitAndLoadChainstate( }; Assert(ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction try { - node.chainman = std::make_unique(*Assert(node.shutdown), chainman_opts, blockman_opts); + node.chainman = std::make_unique(*Assert(node.shutdown_signal), chainman_opts, blockman_opts); } catch (std::exception& e) { return {ChainstateLoadStatus::FAILURE_FATAL, strprintf(Untranslated("Failed to initialize ChainstateManager: %s"), e.what())}; } @@ -1327,7 +1334,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) constexpr uint64_t min_disk_space = 50 << 20; // 50 MB if (!CheckDiskSpace(args.GetBlocksDirPath(), min_disk_space)) { LogError("Shutting down due to lack of disk space!\n"); - if (!(*Assert(node.shutdown))()) { + if (!(Assert(node.shutdown_request))()) { LogError("Failed to send shutdown signal after disk space check\n"); } } @@ -1608,7 +1615,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 7: load block chain - node.notifications = std::make_unique(*Assert(node.shutdown), node.exit_status, *Assert(node.warnings)); + node.notifications = std::make_unique(Assert(node.shutdown_request), node.exit_status, *Assert(node.warnings)); ReadNotificationArgs(args, *node.notifications); // cache size calculations @@ -1649,7 +1656,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return false; } do_reindex = true; - if (!Assert(node.shutdown)->reset()) { + if (!Assert(node.shutdown_signal)->reset()) { LogError("Internal error: failed to reset shutdown signal.\n"); } std::tie(status, error) = InitAndLoadChainstate( @@ -1794,7 +1801,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) ImportBlocks(chainman, vImportFiles); if (args.GetBoolArg("-stopafterblockimport", DEFAULT_STOPAFTERBLOCKIMPORT)) { LogPrintf("Stopping after block import\n"); - if (!(*Assert(node.shutdown))()) { + if (!(Assert(node.shutdown_request))()) { LogError("Failed to send shutdown signal after finishing block import\n"); } return; diff --git a/src/node/abort.cpp b/src/node/abort.cpp index 8a17c41fd2..c15bf047c8 100644 --- a/src/node/abort.cpp +++ b/src/node/abort.cpp @@ -15,12 +15,12 @@ namespace node { -void AbortNode(util::SignalInterrupt* shutdown, std::atomic& exit_status, const bilingual_str& message, node::Warnings* warnings) +void AbortNode(const std::function& shutdown_request, std::atomic& exit_status, const bilingual_str& message, node::Warnings* warnings) { if (warnings) warnings->Set(Warning::FATAL_INTERNAL_ERROR, message); InitError(_("A fatal internal error occurred, see debug.log for details: ") + message); exit_status.store(EXIT_FAILURE); - if (shutdown && !(*shutdown)()) { + if (shutdown_request && !shutdown_request()) { LogError("Failed to send shutdown signal\n"); }; } diff --git a/src/node/abort.h b/src/node/abort.h index c881af4634..c8514628bc 100644 --- a/src/node/abort.h +++ b/src/node/abort.h @@ -6,16 +6,13 @@ #define BITCOIN_NODE_ABORT_H #include +#include struct bilingual_str; -namespace util { -class SignalInterrupt; -} // namespace util - namespace node { class Warnings; -void AbortNode(util::SignalInterrupt* shutdown, std::atomic& exit_status, const bilingual_str& message, node::Warnings* warnings); +void AbortNode(const std::function& shutdown_request, std::atomic& exit_status, const bilingual_str& message, node::Warnings* warnings); } // namespace node #endif // BITCOIN_NODE_ABORT_H diff --git a/src/node/context.h b/src/node/context.h index 398089ff3c..debc122120 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -59,8 +59,10 @@ struct NodeContext { std::unique_ptr ecc_context; //! Init interface for initializing current process and connecting to other processes. interfaces::Init* init{nullptr}; + //! Function to request a shutdown. + std::function shutdown_request; //! Interrupt object used to track whether node shutdown was requested. - util::SignalInterrupt* shutdown{nullptr}; + util::SignalInterrupt* shutdown_signal{nullptr}; std::unique_ptr addrman; std::unique_ptr connman; std::unique_ptr mempool; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index febd968313..25a9029f06 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -134,13 +134,15 @@ class NodeImpl : public Node } void startShutdown() override { - if (!(*Assert(Assert(m_context)->shutdown))()) { + NodeContext& ctx{*Assert(m_context)}; + if (!(Assert(ctx.shutdown_request))()) { LogError("Failed to send shutdown signal\n"); } + // Stop RPC for clean shutdown if any of waitfor* commands is executed. if (args().GetBoolArg("-server", false)) { InterruptRPC(); - StopRPC(m_context); + StopRPC(); } } bool shutdownRequested() override { return ShutdownRequested(*Assert(m_context)); }; diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp index 40d45c8c2b..05fe2f2248 100644 --- a/src/node/kernel_notifications.cpp +++ b/src/node/kernel_notifications.cpp @@ -58,7 +58,7 @@ kernel::InterruptResult KernelNotifications::blockTip(SynchronizationState state uiInterface.NotifyBlockTip(state, &index); if (m_stop_at_height && index.nHeight >= m_stop_at_height) { - if (!m_shutdown()) { + if (!m_shutdown_request()) { LogError("Failed to send shutdown signal after reaching stop height\n"); } return kernel::Interrupted{}; @@ -90,12 +90,12 @@ void KernelNotifications::warningUnset(kernel::Warning id) void KernelNotifications::flushError(const bilingual_str& message) { - AbortNode(&m_shutdown, m_exit_status, message, &m_warnings); + AbortNode(m_shutdown_request, m_exit_status, message, &m_warnings); } void KernelNotifications::fatalError(const bilingual_str& message) { - node::AbortNode(m_shutdown_on_fatal_error ? &m_shutdown : nullptr, + node::AbortNode(m_shutdown_on_fatal_error ? m_shutdown_request : nullptr, m_exit_status, message, &m_warnings); } diff --git a/src/node/kernel_notifications.h b/src/node/kernel_notifications.h index 12017f2b2c..a39a20e2e2 100644 --- a/src/node/kernel_notifications.h +++ b/src/node/kernel_notifications.h @@ -13,6 +13,7 @@ #include #include +#include class ArgsManager; class CBlockIndex; @@ -23,10 +24,6 @@ namespace kernel { enum class Warning; } // namespace kernel -namespace util { -class SignalInterrupt; -} // namespace util - namespace node { class Warnings; @@ -35,8 +32,8 @@ static constexpr int DEFAULT_STOPATHEIGHT{0}; class KernelNotifications : public kernel::Notifications { public: - KernelNotifications(util::SignalInterrupt& shutdown, std::atomic& exit_status, node::Warnings& warnings) - : m_shutdown(shutdown), m_exit_status{exit_status}, m_warnings{warnings} {} + KernelNotifications(const std::function& shutdown_request, std::atomic& exit_status, node::Warnings& warnings) + : m_shutdown_request(shutdown_request), m_exit_status{exit_status}, m_warnings{warnings} {} [[nodiscard]] kernel::InterruptResult blockTip(SynchronizationState state, CBlockIndex& index) override EXCLUSIVE_LOCKS_REQUIRED(!m_tip_block_mutex); @@ -63,7 +60,7 @@ class KernelNotifications : public kernel::Notifications uint256 m_tip_block GUARDED_BY(m_tip_block_mutex); private: - util::SignalInterrupt& m_shutdown; + const std::function& m_shutdown_request; std::atomic& m_exit_status; node::Warnings& m_warnings; }; diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index ab2135a919..6c844ffc8f 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -169,7 +169,7 @@ static RPCHelpMan stop() { // Event loop will exit after current HTTP requests have been handled, so // this reply will get back to the client. - CHECK_NONFATAL((*CHECK_NONFATAL(EnsureAnyNodeContext(jsonRequest.context).shutdown))()); + CHECK_NONFATAL((CHECK_NONFATAL(EnsureAnyNodeContext(jsonRequest.context).shutdown_request))()); if (jsonRequest.params[0].isNum()) { UninterruptibleSleep(std::chrono::milliseconds{jsonRequest.params[0].getInt()}); } @@ -294,7 +294,7 @@ void InterruptRPC() }); } -void StopRPC(const std::any& context) +void StopRPC() { static std::once_flag g_rpc_stop_flag; // This function could be called twice if the GUI has been started with -server=1. @@ -303,9 +303,6 @@ void StopRPC(const std::any& context) LogDebug(BCLog::RPC, "Stopping RPC\n"); WITH_LOCK(g_deadline_timers_mutex, deadlineTimers.clear()); DeleteAuthCookie(); - node::NodeContext& node = EnsureAnyNodeContext(context); - // The notifications interface doesn't exist between initialization step 4a and 7. - if (node.notifications) WITH_LOCK(node.notifications->m_tip_block_mutex, node.notifications->m_tip_block_cv.notify_all()); LogDebug(BCLog::RPC, "RPC stopped.\n"); }); } diff --git a/src/rpc/server.h b/src/rpc/server.h index a8896d4247..5a22279a58 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -9,7 +9,6 @@ #include #include -#include #include #include #include @@ -173,7 +172,7 @@ extern CRPCTable tableRPC; void StartRPC(); void InterruptRPC(); -void StopRPC(const std::any& context); +void StopRPC(); UniValue JSONRPCExec(const JSONRPCRequest& jreq, bool catch_errors); #endif // BITCOIN_RPC_SERVER_H diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index f6024f1ef3..48ae874fcd 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -144,7 +144,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) BOOST_REQUIRE(filter_index.StartBackgroundSync()); // Allow filter index to catch up with the block index. - IndexWaitSynced(filter_index, *Assert(m_node.shutdown)); + IndexWaitSynced(filter_index, *Assert(m_node.shutdown_signal)); // Check that filter index has all blocks that were in the chain before it started. { diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index 121f00bd25..c2b95dd861 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -28,13 +28,13 @@ BOOST_FIXTURE_TEST_SUITE(blockmanager_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) { const auto params {CreateChainParams(ArgsManager{}, ChainType::MAIN)}; - KernelNotifications notifications{*Assert(m_node.shutdown), m_node.exit_status, *Assert(m_node.warnings)}; + KernelNotifications notifications{Assert(m_node.shutdown_request), m_node.exit_status, *Assert(m_node.warnings)}; const BlockManager::Options blockman_opts{ .chainparams = *params, .blocks_dir = m_args.GetBlocksDirPath(), .notifications = notifications, }; - BlockManager blockman{*Assert(m_node.shutdown), blockman_opts}; + BlockManager blockman{*Assert(m_node.shutdown_signal), blockman_opts}; // simulate adding a genesis block normally BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); // simulate what happens during reindex @@ -135,13 +135,13 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup) BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) { - KernelNotifications notifications{*Assert(m_node.shutdown), m_node.exit_status, *Assert(m_node.warnings)}; + KernelNotifications notifications{Assert(m_node.shutdown_request), m_node.exit_status, *Assert(m_node.warnings)}; node::BlockManager::Options blockman_opts{ .chainparams = Params(), .blocks_dir = m_args.GetBlocksDirPath(), .notifications = notifications, }; - BlockManager blockman{*Assert(m_node.shutdown), blockman_opts}; + BlockManager blockman{*Assert(m_node.shutdown_signal), blockman_opts}; // Test blocks with no transactions, not even a coinbase CBlock block1; diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp index 08814c1499..e09aad05e9 100644 --- a/src/test/coinstatsindex_tests.cpp +++ b/src/test/coinstatsindex_tests.cpp @@ -35,7 +35,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup) BOOST_REQUIRE(coin_stats_index.StartBackgroundSync()); - IndexWaitSynced(coin_stats_index, *Assert(m_node.shutdown)); + IndexWaitSynced(coin_stats_index, *Assert(m_node.shutdown_signal)); // Check that CoinStatsIndex works for genesis block. const CBlockIndex* genesis_block_index; @@ -86,7 +86,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup) CoinStatsIndex index{interfaces::MakeChain(m_node), 1 << 20}; BOOST_REQUIRE(index.Init()); BOOST_REQUIRE(index.StartBackgroundSync()); - IndexWaitSynced(index, *Assert(m_node.shutdown)); + IndexWaitSynced(index, *Assert(m_node.shutdown_signal)); std::shared_ptr new_block; CBlockIndex* new_block_index = nullptr; { diff --git a/src/test/txindex_tests.cpp b/src/test/txindex_tests.cpp index 5a32b02ad9..9ee5387830 100644 --- a/src/test/txindex_tests.cpp +++ b/src/test/txindex_tests.cpp @@ -33,7 +33,7 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup) BOOST_REQUIRE(txindex.StartBackgroundSync()); // Allow tx index to catch up with the block index. - IndexWaitSynced(txindex, *Assert(m_node.shutdown)); + IndexWaitSynced(txindex, *Assert(m_node.shutdown_signal)); // Check that txindex excludes genesis block transactions. const CBlock& genesis_block = Params().GenesisBlock(); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index fa89ceb332..7465846356 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -104,7 +104,8 @@ static void ExitFailure(std::string_view str_err) BasicTestingSetup::BasicTestingSetup(const ChainType chainType, TestOpts opts) : m_args{} { - m_node.shutdown = &m_interrupt; + m_node.shutdown_signal = &m_interrupt; + m_node.shutdown_request = [this]{ return m_interrupt(); }; m_node.args = &gArgs; std::vector arguments = Cat( { @@ -224,7 +225,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts) m_cache_sizes = CalculateCacheSizes(m_args); - m_node.notifications = std::make_unique(*Assert(m_node.shutdown), m_node.exit_status, *Assert(m_node.warnings)); + m_node.notifications = std::make_unique(Assert(m_node.shutdown_request), m_node.exit_status, *Assert(m_node.warnings)); m_make_chainman = [this, &chainparams, opts] { Assert(!m_node.chainman); @@ -245,7 +246,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts) .blocks_dir = m_args.GetBlocksDirPath(), .notifications = chainman_opts.notifications, }; - m_node.chainman = std::make_unique(*Assert(m_node.shutdown), chainman_opts, blockman_opts); + m_node.chainman = std::make_unique(*Assert(m_node.shutdown_signal), chainman_opts, blockman_opts); LOCK(m_node.chainman->GetMutex()); m_node.chainman->m_blockman.m_block_tree_db = std::make_unique(DBParams{ .path = m_args.GetDataDirNet() / "blocks" / "index", diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index b4fcfbd853..6c2a825e64 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -382,7 +382,7 @@ struct SnapshotTestSetup : TestChain100Setup { LOCK(::cs_main); chainman.ResetChainstates(); BOOST_CHECK_EQUAL(chainman.GetAll().size(), 0); - m_node.notifications = std::make_unique(*Assert(m_node.shutdown), m_node.exit_status, *Assert(m_node.warnings)); + m_node.notifications = std::make_unique(Assert(m_node.shutdown_request), m_node.exit_status, *Assert(m_node.warnings)); const ChainstateManager::Options chainman_opts{ .chainparams = ::Params(), .datadir = chainman.m_options.datadir, @@ -397,7 +397,7 @@ struct SnapshotTestSetup : TestChain100Setup { // For robustness, ensure the old manager is destroyed before creating a // new one. m_node.chainman.reset(); - m_node.chainman = std::make_unique(*Assert(m_node.shutdown), chainman_opts, blockman_opts); + m_node.chainman = std::make_unique(*Assert(m_node.shutdown_signal), chainman_opts, blockman_opts); } return *Assert(m_node.chainman); } From fa7f52af1a47ebe3d1bc00e3bac70bf9f6fa769b Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 25 Sep 2024 10:08:22 +0200 Subject: [PATCH 11/36] refactor: Use wait_for predicate to check for interrupt Also use uint256::ZERO where appropriate for self-documenting code. --- src/interfaces/mining.h | 3 ++- src/node/interfaces.cpp | 15 ++++----------- src/node/kernel_notifications.h | 4 +++- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 2977811aa6..f71f7d7251 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -61,7 +61,8 @@ class Mining virtual std::optional getTip() = 0; /** - * Waits for the tip to change + * Waits for the connected tip to change. If the tip was not connected on + * startup, this will wait. * * @param[in] current_tip block hash of the current chain tip. Function waits * for the chain tip to differ from this. diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 25a9029f06..ec4777ba7f 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -940,19 +940,12 @@ class MinerImpl : public Mining BlockRef waitTipChanged(uint256 current_tip, MillisecondsDouble timeout) override { - // Interrupt check interval - const MillisecondsDouble tick{1000}; - auto now{std::chrono::steady_clock::now()}; - auto deadline = now + timeout; - // std::chrono does not check against overflow - if (deadline < now) deadline = std::chrono::steady_clock::time_point::max(); + if (timeout > std::chrono::years{100}) timeout = std::chrono::years{100}; // Upper bound to avoid UB in std::chrono { WAIT_LOCK(notifications().m_tip_block_mutex, lock); - while ((notifications().m_tip_block == uint256() || notifications().m_tip_block == current_tip) && !chainman().m_interrupt) { - now = std::chrono::steady_clock::now(); - if (now >= deadline) break; - notifications().m_tip_block_cv.wait_until(lock, std::min(deadline, now + tick)); - } + notifications().m_tip_block_cv.wait_for(lock, timeout, [&]() EXCLUSIVE_LOCKS_REQUIRED(notifications().m_tip_block_mutex) { + return (notifications().m_tip_block != current_tip && notifications().m_tip_block != uint256::ZERO) || chainman().m_interrupt; + }); } // Must release m_tip_block_mutex before locking cs_main, to avoid deadlocks. LOCK(::cs_main); diff --git a/src/node/kernel_notifications.h b/src/node/kernel_notifications.h index a39a20e2e2..296b9c426d 100644 --- a/src/node/kernel_notifications.h +++ b/src/node/kernel_notifications.h @@ -57,7 +57,9 @@ class KernelNotifications : public kernel::Notifications Mutex m_tip_block_mutex; std::condition_variable m_tip_block_cv GUARDED_BY(m_tip_block_mutex); //! The block for which the last blockTip notification was received for. - uint256 m_tip_block GUARDED_BY(m_tip_block_mutex); + //! The initial ZERO means that no block has been connected yet, which may + //! be true even long after startup, until shutdown. + uint256 m_tip_block GUARDED_BY(m_tip_block_mutex){uint256::ZERO}; private: const std::function& m_shutdown_request; From fa2e4439652172df27f14508eef078bd0ee2fca5 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 25 Sep 2024 11:57:17 +0200 Subject: [PATCH 12/36] refactor: Replace g_genesis_wait_cv with m_tip_block_cv They achieve the same, but the extra indirection over boost::signals2 is not needed. --- src/init.cpp | 41 +++++++---------------------------------- 1 file changed, 7 insertions(+), 34 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 433a2d0748..307a339aee 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -685,21 +685,6 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) argsman.AddHiddenArgs(hidden_args); } -static bool fHaveGenesis = false; -static GlobalMutex g_genesis_wait_mutex; -static std::condition_variable g_genesis_wait_cv; - -static void BlockNotifyGenesisWait(const CBlockIndex* pBlockIndex) -{ - if (pBlockIndex != nullptr) { - { - LOCK(g_genesis_wait_mutex); - fHaveGenesis = true; - } - g_genesis_wait_cv.notify_all(); - } -} - #if HAVE_SYSTEM static void StartupNotify(const ArgsManager& args) { @@ -1616,7 +1601,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 7: load block chain node.notifications = std::make_unique(Assert(node.shutdown_request), node.exit_status, *Assert(node.warnings)); - ReadNotificationArgs(args, *node.notifications); + auto& kernel_notifications{*node.notifications}; + ReadNotificationArgs(args, kernel_notifications); // cache size calculations CacheSizes cache_sizes = CalculateCacheSizes(args, g_enabled_filter_types.size()); @@ -1768,15 +1754,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } } - // Either install a handler to notify us when genesis activates, or set fHaveGenesis directly. - // No locking, as this happens before any background thread is started. - boost::signals2::connection block_notify_genesis_wait_connection; - if (WITH_LOCK(chainman.GetMutex(), return chainman.ActiveChain().Tip() == nullptr)) { - block_notify_genesis_wait_connection = uiInterface.NotifyBlockTip_connect(std::bind(BlockNotifyGenesisWait, std::placeholders::_2)); - } else { - fHaveGenesis = true; - } - #if HAVE_SYSTEM const std::string block_notify = args.GetArg("-blocknotify", ""); if (!block_notify.empty()) { @@ -1821,15 +1798,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) }); // Wait for genesis block to be processed - { - WAIT_LOCK(g_genesis_wait_mutex, lock); - // We previously could hang here if shutdown was requested prior to - // ImportBlocks getting started, so instead we just wait on a timer to - // check ShutdownRequested() regularly. - while (!fHaveGenesis && !ShutdownRequested(node)) { - g_genesis_wait_cv.wait_for(lock, std::chrono::milliseconds(500)); - } - block_notify_genesis_wait_connection.disconnect(); + if (WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip() == nullptr)) { + WAIT_LOCK(kernel_notifications.m_tip_block_mutex, lock); + kernel_notifications.m_tip_block_cv.wait(lock, [&]() EXCLUSIVE_LOCKS_REQUIRED(kernel_notifications.m_tip_block_mutex) { + return !kernel_notifications.m_tip_block.IsNull() || ShutdownRequested(node); + }); } if (ShutdownRequested(node)) { From fa22e5c430acaef9713d9a4b4b97bb3f4876f816 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 25 Sep 2024 15:42:51 +0200 Subject: [PATCH 13/36] refactor: Remove dead code that assumed tip == nullptr The tip is set after waiting for the genesis block. --- src/init.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 307a339aee..bf268d418c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1811,17 +1811,17 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 12: start node - //// debug print int64_t best_block_time{}; { - LOCK(cs_main); + LOCK(chainman.GetMutex()); + const auto& tip{*Assert(chainman.ActiveTip())}; LogPrintf("block tree size = %u\n", chainman.BlockIndex().size()); - chain_active_height = chainman.ActiveChain().Height(); - best_block_time = chainman.ActiveChain().Tip() ? chainman.ActiveChain().Tip()->GetBlockTime() : chainman.GetParams().GenesisBlock().GetBlockTime(); + chain_active_height = tip.nHeight; + best_block_time = tip.GetBlockTime(); if (tip_info) { tip_info->block_height = chain_active_height; tip_info->block_time = best_block_time; - tip_info->verification_progress = GuessVerificationProgress(chainman.GetParams().TxData(), chainman.ActiveChain().Tip()); + tip_info->verification_progress = GuessVerificationProgress(chainman.GetParams().TxData(), &tip); } if (tip_info && chainman.m_best_header) { tip_info->header_height = chainman.m_best_header->nHeight; From 605926da0ab4ac7ae4e9aed55a059f37c31c15b5 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 1 Oct 2024 13:03:35 +0100 Subject: [PATCH 14/36] depends: Print ready-to-use `--toolchain` option for CMake invocation --- depends/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/depends/Makefile b/depends/Makefile index f492559307..f1dc300b7a 100644 --- a/depends/Makefile +++ b/depends/Makefile @@ -189,6 +189,7 @@ $(host_prefix)/.stamp_$(final_build_id): $(native_packages) $(packages) echo copying packages: $^ echo to: $(@D) cd $(@D); $(foreach package,$^, $(build_TAR) xf $($(package)_cached); ) + echo To build Bitcoin Core with these packages, pass \'--toolchain $(@D)/toolchain.cmake\' to the first CMake invocation. touch $@ ifeq ($(host),$(build)) From deacf3c7cd68dd4ee973526740e45c7015b6433a Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 1 Oct 2024 13:43:30 +0100 Subject: [PATCH 15/36] cmake: Avoid hardcoding Qt's major version in Find module This change facilitates future migration to Qt 6. --- CMakeLists.txt | 2 +- cmake/module/{FindQt5.cmake => FindQt.cmake} | 24 ++++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) rename cmake/module/{FindQt5.cmake => FindQt.cmake} (76%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 56ac7f4df5..8b687641f2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -177,7 +177,7 @@ if(BUILD_GUI) if(BUILD_GUI_TESTS) list(APPEND qt_components Test) endif() - find_package(Qt5 5.11.3 MODULE REQUIRED + find_package(Qt 5.11.3 MODULE REQUIRED COMPONENTS ${qt_components} ) unset(qt_components) diff --git a/cmake/module/FindQt5.cmake b/cmake/module/FindQt.cmake similarity index 76% rename from cmake/module/FindQt5.cmake rename to cmake/module/FindQt.cmake index f39ee53d5b..2e43294a99 100644 --- a/cmake/module/FindQt5.cmake +++ b/cmake/module/FindQt.cmake @@ -3,10 +3,10 @@ # file COPYING or https://opensource.org/license/mit/. #[=======================================================================[ -FindQt5 -------- +FindQt +------ -Finds the Qt 5 headers and libraries. +Finds the Qt headers and libraries. This is a wrapper around find_package() command that: - facilitates searching in various build environments @@ -19,7 +19,7 @@ if(CMAKE_HOST_APPLE) find_program(HOMEBREW_EXECUTABLE brew) if(HOMEBREW_EXECUTABLE) execute_process( - COMMAND ${HOMEBREW_EXECUTABLE} --prefix qt@5 + COMMAND ${HOMEBREW_EXECUTABLE} --prefix qt@${Qt_FIND_VERSION_MAJOR} OUTPUT_VARIABLE _qt_homebrew_prefix ERROR_QUIET OUTPUT_STRIP_TRAILING_WHITESPACE @@ -40,10 +40,10 @@ endif() # /usr/x86_64-w64-mingw32/lib/libm.a or /usr/arm-linux-gnueabihf/lib/libm.a. set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY BOTH) -find_package(Qt5 ${Qt5_FIND_VERSION} - COMPONENTS ${Qt5_FIND_COMPONENTS} +find_package(Qt${Qt_FIND_VERSION_MAJOR} ${Qt_FIND_VERSION} + COMPONENTS ${Qt_FIND_COMPONENTS} HINTS ${_qt_homebrew_prefix} - PATH_SUFFIXES Qt5 # Required on OpenBSD systems. + PATH_SUFFIXES Qt${Qt_FIND_VERSION_MAJOR} # Required on OpenBSD systems. ) unset(_qt_homebrew_prefix) @@ -56,11 +56,11 @@ else() endif() include(FindPackageHandleStandardArgs) -find_package_handle_standard_args(Qt5 - REQUIRED_VARS Qt5_DIR - VERSION_VAR Qt5_VERSION +find_package_handle_standard_args(Qt + REQUIRED_VARS Qt${Qt_FIND_VERSION_MAJOR}_DIR + VERSION_VAR Qt${Qt_FIND_VERSION_MAJOR}_VERSION ) -foreach(component IN LISTS Qt5_FIND_COMPONENTS ITEMS "") - mark_as_advanced(Qt5${component}_DIR) +foreach(component IN LISTS Qt_FIND_COMPONENTS ITEMS "") + mark_as_advanced(Qt${Qt_FIND_VERSION_MAJOR}${component}_DIR) endforeach() From ae56b3230b287eef5a5657d3089abebffde51484 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Tue, 1 Oct 2024 19:04:24 +0200 Subject: [PATCH 16/36] depends: For mingw cross compile use -gcc-posix to prevent library conflict CMake parses some paths from the spec of the C compiler, assuming it will be the linker, resulting in the link to end up with `-L/usr/lib/gcc/x86_64-w64-mingw32/12-win32` on debian bookworm if both -win32 and -posix variants are installed, and -win32 is the default alternative. This results in the wrong C++ library being linked, missing std::threads::hardware_concurrency and other threading functions. To fix this, use the -posix variant of gcc as well when available. This fixes a regression compared to autotools, where this scenario worked. --- depends/hosts/mingw32.mk | 3 +++ 1 file changed, 3 insertions(+) diff --git a/depends/hosts/mingw32.mk b/depends/hosts/mingw32.mk index c09f7b1e3a..755d7aebe4 100644 --- a/depends/hosts/mingw32.mk +++ b/depends/hosts/mingw32.mk @@ -1,3 +1,6 @@ +ifneq ($(shell $(SHELL) $(.SHELLFLAGS) "command -v $(host)-gcc-posix"),) +mingw32_CC := $(host)-gcc-posix +endif ifneq ($(shell $(SHELL) $(.SHELLFLAGS) "command -v $(host)-g++-posix"),) mingw32_CXX := $(host)-g++-posix endif From fa6d14eacb2a8c1c3243e3075254dfdebaa9290e Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 2 Oct 2024 15:13:55 +0200 Subject: [PATCH 17/36] test: Treat exclude list warning as failure in CI --- test/functional/test_runner.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 301f62d7b0..e783408837 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -446,8 +446,8 @@ def main(): help="Leave bitcoinds and test.* datadir on exit or error") parser.add_argument('--resultsfile', '-r', help='store test results (as CSV) to the provided file') - args, unknown_args = parser.parse_known_args() + fail_on_warn = args.ci if not args.ansi: global DEFAULT, BOLD, GREEN, RED DEFAULT = ("", "") @@ -524,8 +524,12 @@ def main(): # Remove the test cases that the user has explicitly asked to exclude. # The user can specify a test case with or without the .py extension. if args.exclude: + def print_warning_missing_test(test_name): - print("{}WARNING!{} Test '{}' not found in current test list.".format(BOLD[1], BOLD[0], test_name)) + print("{}WARNING!{} Test '{}' not found in current test list. Check the --exclude list.".format(BOLD[1], BOLD[0], test_name)) + if fail_on_warn: + sys.exit(1) + def remove_tests(exclude_list): if not exclude_list: print_warning_missing_test(exclude_test) @@ -562,7 +566,7 @@ def remove_tests(exclude_list): f"A minimum of {MIN_NO_CLEANUP_SPACE // (1024 * 1024 * 1024)} GB of free space is required.") passon_args.append("--nocleanup") - check_script_list(src_dir=config["environment"]["SRCDIR"], fail_on_warn=args.ci) + check_script_list(src_dir=config["environment"]["SRCDIR"], fail_on_warn=fail_on_warn) check_script_prefixes() if not args.keepcache: @@ -871,7 +875,6 @@ def check_script_list(*, src_dir, fail_on_warn): if len(missed_tests) != 0: print("%sWARNING!%s The following scripts are not being run: %s. Check the test lists in test_runner.py." % (BOLD[1], BOLD[0], str(missed_tests))) if fail_on_warn: - # On CI this warning is an error to prevent merging incomplete commits into master sys.exit(1) From 36a6d4b0078ebb39ed082c866bf49214a2a01241 Mon Sep 17 00:00:00 2001 From: Mackain Date: Fri, 27 Sep 2024 22:58:54 +0200 Subject: [PATCH 18/36] doc: update IBD requirements in doc/README.md --- doc/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/README.md b/doc/README.md index 7f5db1b5bf..79ca53ce76 100644 --- a/doc/README.md +++ b/doc/README.md @@ -3,7 +3,7 @@ Bitcoin Core Setup --------------------- -Bitcoin Core is the original Bitcoin client and it builds the backbone of the network. It downloads and, by default, stores the entire history of Bitcoin transactions, which requires a few hundred gigabytes of disk space. Depending on the speed of your computer and network connection, the synchronization process can take anywhere from a few hours to a day or more. +Bitcoin Core is the original Bitcoin client and it builds the backbone of the network. It downloads and, by default, stores the entire history of Bitcoin transactions, which requires several hundred gigabytes or more of disk space. Depending on the speed of your computer and network connection, the synchronization process can take anywhere from a few hours to several days or more. To download Bitcoin Core, visit [bitcoincore.org](https://bitcoincore.org/en/download/). From 80761afced12c774a1768fb27a3975d456981ae0 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 4 Oct 2024 12:58:04 +0100 Subject: [PATCH 19/36] qt6: Handle different signatures of `QANEF::nativeEventFilter` This change ensures compatibility across all supported Qt versions. --- src/qt/winshutdownmonitor.cpp | 4 ++++ src/qt/winshutdownmonitor.h | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/qt/winshutdownmonitor.cpp b/src/qt/winshutdownmonitor.cpp index 0b5278c192..9ccd7028da 100644 --- a/src/qt/winshutdownmonitor.cpp +++ b/src/qt/winshutdownmonitor.cpp @@ -12,7 +12,11 @@ // If we don't want a message to be processed by Qt, return true and set result to // the value that the window procedure should return. Otherwise return false. +#if (QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)) +bool WinShutdownMonitor::nativeEventFilter(const QByteArray &eventType, void *pMessage, qintptr *pnResult) +#else bool WinShutdownMonitor::nativeEventFilter(const QByteArray &eventType, void *pMessage, long *pnResult) +#endif { Q_UNUSED(eventType); diff --git a/src/qt/winshutdownmonitor.h b/src/qt/winshutdownmonitor.h index 060d8546e3..a8b626065d 100644 --- a/src/qt/winshutdownmonitor.h +++ b/src/qt/winshutdownmonitor.h @@ -20,7 +20,11 @@ class WinShutdownMonitor : public QAbstractNativeEventFilter WinShutdownMonitor(std::function shutdown_fn) : m_shutdown_fn{std::move(shutdown_fn)} {} /** Implements QAbstractNativeEventFilter interface for processing Windows messages */ +#if (QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)) + bool nativeEventFilter(const QByteArray &eventType, void *pMessage, qintptr *pnResult) override; +#else bool nativeEventFilter(const QByteArray &eventType, void *pMessage, long *pnResult) override; +#endif /** Register the reason for blocking shutdown on Windows to allow clean client exit */ static void registerShutdownBlockReason(const QString& strReason, const HWND& mainWinId); From f50557f5d36f568b7fe65ff5e242303b16b9b258 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Sat, 5 Oct 2024 23:56:47 +0200 Subject: [PATCH 20/36] test: Fix copy-paste in db_tests ostream operator --- src/wallet/test/db_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index 2fac356263..0e35b59d7c 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -28,7 +28,7 @@ inline std::ostream& operator<<(std::ostream& os, const std::pair(key.data()), key.size()} << "\", \"" - << std::string_view{reinterpret_cast(key.data()), key.size()} << "\")"; + << std::string_view{reinterpret_cast(value.data()), value.size()} << "\")"; return os; } From 56aad83307e46983a397236bd0959e634207f83e Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 3 Oct 2024 10:28:06 +0100 Subject: [PATCH 21/36] ci: set a ctest timeout of 1200 (20 minutes) This should be long enough (with headroom) for our longest running tests, which even under MSAN, TSAN, Valgrind, etc max out at about 800s. i.e under Valgrind I see the longer runtimes as: ```bash 135/136 Test #8: bench_sanity_check_high_priority ..... Passed 371.19 sec 136/136 Test #122: coinselector_tests ................... Passed 343.39 sec ``` In the CI `tests` under TSAN: ```bash tests ................................ Passed 795.20 sec ``` and MSAN: ```bash tests ................................ Passed 658.48 sec ``` This will also prevent the current issue we are seeing of `ctest` running until it reaches the CI timeout, see #30969. However, we still need to figure out what underlying issue is causing the tests to (sometimes) run for so long, but in the mean time, this will stop `ctest` wasting our CI CPU. --- ci/test/03_test_script.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh index 1c1b5fa545..9cc0e8e864 100755 --- a/ci/test/03_test_script.sh +++ b/ci/test/03_test_script.sh @@ -146,7 +146,7 @@ if [ "$RUN_CHECK_DEPS" = "true" ]; then fi if [ "$RUN_UNIT_TESTS" = "true" ]; then - DIR_UNIT_TEST_DATA="${DIR_UNIT_TEST_DATA}" LD_LIBRARY_PATH="${DEPENDS_DIR}/${HOST}/lib" CTEST_OUTPUT_ON_FAILURE=ON ctest "${MAKEJOBS}" + DIR_UNIT_TEST_DATA="${DIR_UNIT_TEST_DATA}" LD_LIBRARY_PATH="${DEPENDS_DIR}/${HOST}/lib" CTEST_OUTPUT_ON_FAILURE=ON ctest "${MAKEJOBS}" --timeout $((TEST_RUNNER_TIMEOUT_FACTOR * 30 )) fi if [ "$RUN_UNIT_TESTS_SEQUENTIAL" = "true" ]; then From faf7a2bccc747f2d509710bd7eeb315e75b6c649 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 7 Oct 2024 12:29:35 +0200 Subject: [PATCH 22/36] ci: Add missing -DWERROR=ON to test-each-commit --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 645268ecf7..2d541af81b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -72,7 +72,7 @@ jobs: run: | # Run tests on commits after the last merge commit and before the PR head commit # Use clang++, because it is a bit faster and uses less memory than g++ - git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && CC=clang CXX=clang++ cmake -B build -DWITH_ZMQ=ON -DBUILD_GUI=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DWITH_BDB=ON -DWITH_MINIUPNPC=ON -DWITH_USDT=ON && cmake --build build -j $(nproc) && ctest --test-dir build -j $(nproc) && ./build/test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }} + git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && CC=clang CXX=clang++ cmake -B build -DWERROR=ON -DWITH_ZMQ=ON -DBUILD_GUI=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DWITH_BDB=ON -DWITH_MINIUPNPC=ON -DWITH_USDT=ON && cmake --build build -j $(nproc) && ctest --test-dir build -j $(nproc) && ./build/test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }} macos-native-arm64: name: 'macOS 14 native, arm64, no depends, sqlite only, gui' From fa1cffacae61264ac89e30a069ba093495cb3216 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Fri, 20 Sep 2024 11:32:40 +0200 Subject: [PATCH 23/36] ci: Install missing nproc in macos task This avoids special-casing macos --- .github/workflows/ci.yml | 2 +- ci/test/03_test_script.sh | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2d541af81b..439d02cc8b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -105,7 +105,7 @@ jobs: run: | # A workaround for "The `brew link` step did not complete successfully" error. brew install --quiet python@3 || brew link --overwrite python@3 - brew install --quiet ninja pkg-config gnu-getopt ccache boost libevent miniupnpc zeromq qt@5 qrencode + brew install --quiet coreutils ninja pkg-config gnu-getopt ccache boost libevent miniupnpc zeromq qt@5 qrencode - name: Set Ccache directory run: echo "CCACHE_DIR=${RUNNER_TEMP}/ccache_dir" >> "$GITHUB_ENV" diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh index 1c1b5fa545..a1d3f7e58d 100755 --- a/ci/test/03_test_script.sh +++ b/ci/test/03_test_script.sh @@ -13,12 +13,11 @@ export LSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/l export TSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/tsan:halt_on_error=1" export UBSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" +echo "Number of available processing units: $(nproc)" if [ "$CI_OS_NAME" == "macos" ]; then top -l 1 -s 0 | awk ' /PhysMem/ {print}' - echo "Number of CPUs: $(sysctl -n hw.logicalcpu)" else free -m -h - echo "Number of CPUs (nproc): $(nproc)" echo "System info: $(uname --kernel-name --kernel-release)" lscpu fi From e0287bc4b2d83cefb7af48aef457153d0729bcd4 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 7 Oct 2024 16:24:53 +0100 Subject: [PATCH 24/36] test: remove unused code from script_tests This has been unused since #29648. Noticed while running a newer version of clang-tidy (19.1.1): ```bash [127/391][6.2s] /opt/homebrew/opt/llvm/bin/clang-tidy -p=build -quiet --config-file=/bitcoin/src/.clang-tidy /bitcoin/src/test/script_tests.cpp bitcoin/src/test/script_tests.cpp:126:25: error: local copy 'tx2' of the variable 'tx' is never modified and never used; consider removing the statement [performance-unnecessary-copy-initialization,-warnings-as-errors] 126 | CMutableTransaction tx2 = tx; | ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~ 127 | BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, &scriptWitness, flags, MutableTransactionSignatureChecker(&tx, 0, txCredit.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err) == expect, message); 512 warnings generated. ``` --- src/test/script_tests.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 8c3806d22f..59eb90bd27 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -123,7 +123,6 @@ void DoTest(const CScript& scriptPubKey, const CScript& scriptSig, const CScript ScriptError err; const CTransaction txCredit{BuildCreditingTransaction(scriptPubKey, nValue)}; CMutableTransaction tx = BuildSpendingTransaction(scriptSig, scriptWitness, txCredit); - CMutableTransaction tx2 = tx; BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, &scriptWitness, flags, MutableTransactionSignatureChecker(&tx, 0, txCredit.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err) == expect, message); BOOST_CHECK_MESSAGE(err == scriptError, FormatScriptError(err) + " where " + FormatScriptError((ScriptError_t)scriptError) + " expected: " + message); From 5901cf7100a75c8131223e23b6c90e0e93611eae Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 6 Sep 2024 15:32:48 -0400 Subject: [PATCH 25/36] clusterlin: abstract out DepGraph::GetReduced{Parents,Children} A fuzz test already relies on these operations, and a future commit will need the same logic too. Therefore, abstract them out into proper member functions, with proper testing. --- src/cluster_linearize.h | 42 ++++++++++++++++++++++++++ src/test/fuzz/cluster_linearize.cpp | 16 ++-------- src/test/util/cluster_linearize.h | 47 ++++++++++++++++++++++++++++- 3 files changed, 90 insertions(+), 15 deletions(-) diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h index e964849f22..8467f3f08b 100644 --- a/src/cluster_linearize.h +++ b/src/cluster_linearize.h @@ -184,6 +184,48 @@ class DepGraph } } + /** Compute the (reduced) set of parents of node i in this graph. + * + * This returns the minimal subset of the parents of i whose ancestors together equal all of + * i's ancestors (unless i is part of a cycle of dependencies). Note that DepGraph does not + * store the set of parents; this information is inferred from the ancestor sets. + * + * Complexity: O(N) where N=Ancestors(i).Count() (which is bounded by TxCount()). + */ + SetType GetReducedParents(ClusterIndex i) const noexcept + { + SetType parents = Ancestors(i); + parents.Reset(i); + for (auto parent : parents) { + if (parents[parent]) { + parents -= Ancestors(parent); + parents.Set(parent); + } + } + return parents; + } + + /** Compute the (reduced) set of children of node i in this graph. + * + * This returns the minimal subset of the children of i whose descendants together equal all of + * i's descendants (unless i is part of a cycle of dependencies). Note that DepGraph does not + * store the set of children; this information is inferred from the descendant sets. + * + * Complexity: O(N) where N=Descendants(i).Count() (which is bounded by TxCount()). + */ + SetType GetReducedChildren(ClusterIndex i) const noexcept + { + SetType children = Descendants(i); + children.Reset(i); + for (auto child : children) { + if (children[child]) { + children -= Descendants(child); + children.Set(child); + } + } + return children; + } + /** Compute the aggregate feerate of a set of nodes in this graph. * * Complexity: O(N) where N=elems.Count(). diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp index d91f85d867..4976afbaad 100644 --- a/src/test/fuzz/cluster_linearize.cpp +++ b/src/test/fuzz/cluster_linearize.cpp @@ -896,24 +896,12 @@ FUZZ_TARGET(clusterlin_postlinearize_tree) } if (direction & 1) { for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { - auto children = depgraph_gen.Descendants(i) - TestBitSet::Singleton(i); - // Remove descendants that are children of other descendants. - for (auto j : children) { - if (!children[j]) continue; - children -= depgraph_gen.Descendants(j); - children.Set(j); - } + auto children = depgraph_gen.GetReducedChildren(i); if (children.Any()) depgraph_tree.AddDependency(i, children.First()); } } else { for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { - auto parents = depgraph_gen.Ancestors(i) - TestBitSet::Singleton(i); - // Remove ancestors that are parents of other ancestors. - for (auto j : parents) { - if (!parents[j]) continue; - parents -= depgraph_gen.Ancestors(j); - parents.Set(j); - } + auto parents = depgraph_gen.GetReducedParents(i); if (parents.Any()) depgraph_tree.AddDependency(parents.First(), i); } } diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index b86ebcd78b..377bfa19fb 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -264,9 +264,23 @@ void SanityCheck(const DepGraph& depgraph) for (ClusterIndex j = 0; j < depgraph.TxCount(); ++j) { assert(depgraph.Ancestors(i)[j] == depgraph.Descendants(j)[i]); } + // No transaction is a parent or child of itself. + auto parents = depgraph.GetReducedParents(i); + auto children = depgraph.GetReducedChildren(i); + assert(!parents[i]); + assert(!children[i]); + // Parents of a transaction do not have ancestors inside those parents (except itself). + // Note that even the transaction itself may be missing (if it is part of a cycle). + for (auto parent : parents) { + assert((depgraph.Ancestors(parent) & parents).IsSubsetOf(SetType::Singleton(parent))); + } + // Similar for children and descendants. + for (auto child : children) { + assert((depgraph.Descendants(child) & children).IsSubsetOf(SetType::Singleton(child))); + } } - // If DepGraph is acyclic, serialize + deserialize must roundtrip. if (IsAcyclic(depgraph)) { + // If DepGraph is acyclic, serialize + deserialize must roundtrip. std::vector ser; VectorWriter writer(ser, 0); writer << Using(depgraph); @@ -284,6 +298,37 @@ void SanityCheck(const DepGraph& depgraph) reader >> Using(decoded_depgraph); assert(depgraph == decoded_depgraph); assert(reader.empty()); + + // In acyclic graphs, the union of parents with parents of parents etc. yields the + // full ancestor set (and similar for children and descendants). + std::vector parents, children; + for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + parents.push_back(depgraph.GetReducedParents(i)); + children.push_back(depgraph.GetReducedChildren(i)); + } + for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + // Initialize the set of ancestors with just the current transaction itself. + SetType ancestors = SetType::Singleton(i); + // Iteratively add parents of all transactions in the ancestor set to itself. + while (true) { + const auto old_ancestors = ancestors; + for (auto j : ancestors) ancestors |= parents[j]; + // Stop when no more changes are being made. + if (old_ancestors == ancestors) break; + } + assert(ancestors == depgraph.Ancestors(i)); + + // Initialize the set of descendants with just the current transaction itself. + SetType descendants = SetType::Singleton(i); + // Iteratively add children of all transactions in the descendant set to itself. + while (true) { + const auto old_descendants = descendants; + for (auto j : descendants) descendants |= children[j]; + // Stop when no more changes are being made. + if (old_descendants == descendants) break; + } + assert(descendants == depgraph.Descendants(i)); + } } } From eaab55ffc8102140086297877747b7fa07b419eb Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 9 Sep 2024 14:51:08 -0400 Subject: [PATCH 26/36] clusterlin: rework DepGraphFormatter::Unser This commit does not change the serialization format. Its purpose is making a few changes already in order to reduce the diff size of the later commit that introduces support for holes in DepGraph. The previous approach was to immediately construct a transaction as soon as its feerate was known in a preliminary position, and then undo that, and place it in the correct position once the position information is known (such that a deserialization error in between would not result in an inconsistent state). The new approach is to delay the actual transaction creation until all its information is known, avoiding the need to undo and redo. This requires a different means of determining whether dependencies are redundant, but that has the advantage that a later commit can apply all dependencies at once, reducing the complexity of deserialization. --- src/test/util/cluster_linearize.h | 52 ++++++++++++++++++------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index 377bfa19fb..1ee0f7c2a7 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -189,10 +189,17 @@ struct DepGraphFormatter /** Mapping from serialization order to cluster order, used later to reconstruct the * cluster order. */ std::vector reordering; + /** How big the entries vector in the reconstructed depgraph will be (before the + * introduction of holes in a further commit, this always equals reordering.size()). */ + ClusterIndex total_size{0}; // Read transactions in topological order. - try { - while (true) { + while (true) { + FeeFrac new_feerate; //!< The new transaction's fee and size. + SetType new_ancestors; //!< The new transaction's ancestors (excluding itself). + uint64_t diff{0}; //!< How many potential parents/insertions we have to skip. + bool read_error{false}; + try { // Read size. Size 0 signifies the end of the DepGraph. int32_t size; s >> VARINT_MODE(size, VarIntMode::NONNEGATIVE_SIGNED); @@ -204,21 +211,18 @@ struct DepGraphFormatter s >> VARINT(coded_fee); coded_fee &= 0xFFFFFFFFFFFFF; // Enough for fee between -21M...21M BTC. static_assert(0xFFFFFFFFFFFFF > uint64_t{2} * 21000000 * 100000000); - auto fee = UnsignedToSigned(coded_fee); - // Extend topo_depgraph with the new transaction (preliminarily at the end). - auto topo_idx = topo_depgraph.AddTransaction({fee, size}); - reordering.push_back(reordering.size()); + new_feerate = {UnsignedToSigned(coded_fee), size}; // Read dependency information. - uint64_t diff = 0; //!< How many potential parents we have to skip. + auto topo_idx = reordering.size(); s >> VARINT(diff); for (ClusterIndex dep_dist = 0; dep_dist < topo_idx; ++dep_dist) { /** Which topo_depgraph index we are currently considering as parent of topo_idx. */ ClusterIndex dep_topo_idx = topo_idx - 1 - dep_dist; // Ignore transactions which are already known ancestors of topo_idx. - if (topo_depgraph.Descendants(dep_topo_idx)[topo_idx]) continue; + if (new_ancestors[dep_topo_idx]) continue; if (diff == 0) { // When the skip counter has reached 0, add an actual dependency. - topo_depgraph.AddDependency(dep_topo_idx, topo_idx); + new_ancestors |= topo_depgraph.Ancestors(dep_topo_idx); // And read the number of skips after it. s >> VARINT(diff); } else { @@ -226,20 +230,24 @@ struct DepGraphFormatter --diff; } } - // If we reach this point, we can interpret the remaining skip value as how far - // from the end of reordering the new transaction should be placed (wrapping - // around), so remove the preliminary position it was put in above (which was to - // make sure that if a deserialization exception occurs, the new transaction still - // has some entry in reordering). - reordering.pop_back(); - ClusterIndex insert_distance = diff % (reordering.size() + 1); - // And then update reordering to reflect this new transaction's insertion. - for (auto& pos : reordering) { - pos += (pos >= reordering.size() - insert_distance); - } - reordering.push_back(reordering.size() - insert_distance); + } catch (const std::ios_base::failure&) { + // Continue even if a read error was encountered. + read_error = true; } - } catch (const std::ios_base::failure&) {} + // Construct a new transaction whenever we made it past the new_feerate construction. + if (new_feerate.IsEmpty()) break; + assert(reordering.size() < SetType::Size()); + auto topo_idx = topo_depgraph.AddTransaction(new_feerate); + for (auto parent : new_ancestors) topo_depgraph.AddDependency(parent, topo_idx); + diff %= total_size + 1; + // Insert the new transaction at distance diff back from the end. + for (auto& pos : reordering) { + pos += (pos >= total_size - diff); + } + reordering.push_back(total_size++ - diff); + // Stop if a read error was encountered during deserialization. + if (read_error) break; + } // Construct the original cluster order depgraph. depgraph = DepGraph(topo_depgraph, reordering); From abf50649d13018bf40c5803730a03053737efeee Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 8 Sep 2024 17:42:25 -0400 Subject: [PATCH 27/36] clusterlin: simplify DepGraphFormatter::Ser This does not change the serialization format. It turns out that it is unnecessary to keep track of the order of transactions in the so-far reconstructed DepGraph to decide how far from the end to insert a new transaction. --- src/test/util/cluster_linearize.h | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index 1ee0f7c2a7..a9ae4ff12a 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -134,9 +134,8 @@ struct DepGraphFormatter }); /** Which transactions the deserializer already knows when it has deserialized what has - * been serialized here so far, and in what order. */ - std::vector rebuilt_order; - rebuilt_order.reserve(depgraph.TxCount()); + * been serialized here so far. */ + SetType done; // Loop over the transactions in topological order. for (ClusterIndex topo_idx = 0; topo_idx < topo_order.size(); ++topo_idx) { @@ -166,14 +165,11 @@ struct DepGraphFormatter } } // Write position information. - ClusterIndex insert_distance = 0; - while (insert_distance < rebuilt_order.size()) { - // Loop to find how far from the end in rebuilt_order to insert. - if (idx > *(rebuilt_order.end() - 1 - insert_distance)) break; - ++insert_distance; - } - rebuilt_order.insert(rebuilt_order.end() - insert_distance, idx); - s << VARINT(diff + insert_distance); + // The new transaction is to be inserted N positions back from the end of the cluster. + // Emit N to indicate that that many insertion choices are skipped. + auto skips = (done - SetType::Fill(idx)).Count(); + s << VARINT(diff + skips); + done.Set(idx); } // Output a final 0 to denote the end of the graph. From 75b5d42419ed1286fc9557bc97ec5bac3f9be837 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 4 Sep 2024 15:14:54 -0400 Subject: [PATCH 28/36] clusterlin: make DepGraph::AddDependency support multiple dependencies at once This changes DepGraph::AddDependency into DepGraph::AddDependencies, which takes in a single child, but a set of parent transactions, making them all dependencies at once. This is important for performance. N transactions can have O(N^2) parents combined, so constructing a full DepGraph using just AddDependency (which is O(N) on its own) could take O(N^3) time, while doing the same with AddDependencies (also O(N) on its own) only takes O(N^2). Notably, this matters for DepGraphFormatter::Unser, which goes from O(N^3) to O(N^2). Co-Authored-By: Greg Sanders --- src/bench/cluster_linearize.cpp | 24 ++++----- src/cluster_linearize.h | 70 +++++++++---------------- src/test/fuzz/cluster_linearize.cpp | 80 +++++++++++++++++++++-------- src/test/util/cluster_linearize.h | 2 +- 4 files changed, 96 insertions(+), 80 deletions(-) diff --git a/src/bench/cluster_linearize.cpp b/src/bench/cluster_linearize.cpp index cf071dda2d..7d011975dd 100644 --- a/src/bench/cluster_linearize.cpp +++ b/src/bench/cluster_linearize.cpp @@ -28,7 +28,7 @@ DepGraph MakeLinearGraph(ClusterIndex ntx) DepGraph depgraph; for (ClusterIndex i = 0; i < ntx; ++i) { depgraph.AddTransaction({-int32_t(i), 1}); - if (i > 0) depgraph.AddDependency(i - 1, i); + if (i > 0) depgraph.AddDependencies(SetType::Singleton(i - 1), i); } return depgraph; } @@ -43,7 +43,7 @@ DepGraph MakeWideGraph(ClusterIndex ntx) DepGraph depgraph; for (ClusterIndex i = 0; i < ntx; ++i) { depgraph.AddTransaction({int32_t(i) + 1, 1}); - if (i > 0) depgraph.AddDependency(0, i); + if (i > 0) depgraph.AddDependencies(SetType::Singleton(0), i); } return depgraph; } @@ -70,19 +70,19 @@ DepGraph MakeHardGraph(ClusterIndex ntx) depgraph.AddTransaction({1, 2}); } else if (i == 1) { depgraph.AddTransaction({14, 2}); - depgraph.AddDependency(0, 1); + depgraph.AddDependencies(SetType::Singleton(0), 1); } else if (i == 2) { depgraph.AddTransaction({6, 1}); - depgraph.AddDependency(2, 1); + depgraph.AddDependencies(SetType::Singleton(2), 1); } else if (i == 3) { depgraph.AddTransaction({5, 1}); - depgraph.AddDependency(2, 3); + depgraph.AddDependencies(SetType::Singleton(2), 3); } else if ((i & 1) == 0) { depgraph.AddTransaction({7, 1}); - depgraph.AddDependency(i - 1, i); + depgraph.AddDependencies(SetType::Singleton(i - 1), i); } else { depgraph.AddTransaction({5, 1}); - depgraph.AddDependency(i, 4); + depgraph.AddDependencies(SetType::Singleton(i), 4); } } else { // Even cluster size. @@ -98,16 +98,16 @@ DepGraph MakeHardGraph(ClusterIndex ntx) depgraph.AddTransaction({1, 1}); } else if (i == 1) { depgraph.AddTransaction({3, 1}); - depgraph.AddDependency(0, 1); + depgraph.AddDependencies(SetType::Singleton(0), 1); } else if (i == 2) { depgraph.AddTransaction({1, 1}); - depgraph.AddDependency(0, 2); + depgraph.AddDependencies(SetType::Singleton(0), 2); } else if (i & 1) { depgraph.AddTransaction({4, 1}); - depgraph.AddDependency(i - 1, i); + depgraph.AddDependencies(SetType::Singleton(i - 1), i); } else { depgraph.AddTransaction({0, 1}); - depgraph.AddDependency(i, 3); + depgraph.AddDependencies(SetType::Singleton(i), 3); } } } @@ -195,7 +195,7 @@ void BenchMergeLinearizationsWorstCase(ClusterIndex ntx, benchmark::Bench& bench DepGraph depgraph; for (ClusterIndex i = 0; i < ntx; ++i) { depgraph.AddTransaction({i, 1}); - if (i) depgraph.AddDependency(0, i); + if (i) depgraph.AddDependencies(SetType::Singleton(0), i); } std::vector lin1; std::vector lin2; diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h index 8467f3f08b..bcc455874d 100644 --- a/src/cluster_linearize.h +++ b/src/cluster_linearize.h @@ -86,35 +86,13 @@ class DepGraph * * Complexity: O(N^2) where N=cluster.size(). */ - explicit DepGraph(const Cluster& cluster) noexcept : entries(cluster.size()) + explicit DepGraph(const Cluster& cluster) noexcept : DepGraph(cluster.size()) { for (ClusterIndex i = 0; i < cluster.size(); ++i) { // Fill in fee and size. entries[i].feerate = cluster[i].first; - // Fill in direct parents as ancestors. - entries[i].ancestors = cluster[i].second; - // Make sure transactions are ancestors of themselves. - entries[i].ancestors.Set(i); - } - - // Propagate ancestor information. - for (ClusterIndex i = 0; i < entries.size(); ++i) { - // At this point, entries[a].ancestors[b] is true iff b is an ancestor of a and there - // is a path from a to b through the subgraph consisting of {a, b} union - // {0, 1, ..., (i-1)}. - SetType to_merge = entries[i].ancestors; - for (ClusterIndex j = 0; j < entries.size(); ++j) { - if (entries[j].ancestors[i]) { - entries[j].ancestors |= to_merge; - } - } - } - - // Fill in descendant information by transposing the ancestor information. - for (ClusterIndex i = 0; i < entries.size(); ++i) { - for (auto j : entries[i].ancestors) { - entries[j].descendants.Set(i); - } + // Fill in dependencies. + AddDependencies(cluster[i].second, i); } } @@ -122,21 +100,16 @@ class DepGraph * * Complexity: O(N^2) where N=depgraph.TxCount(). */ - DepGraph(const DepGraph& depgraph, Span mapping) noexcept : entries(depgraph.TxCount()) + DepGraph(const DepGraph& depgraph, Span mapping) noexcept : DepGraph(depgraph.TxCount()) { Assert(mapping.size() == depgraph.TxCount()); - // Fill in fee, size, ancestors. for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { - const auto& input = depgraph.entries[i]; - auto& output = entries[mapping[i]]; - output.feerate = input.feerate; - for (auto j : input.ancestors) output.ancestors.Set(mapping[j]); - } - // Fill in descendant information. - for (ClusterIndex i = 0; i < entries.size(); ++i) { - for (auto j : entries[i].ancestors) { - entries[j].descendants.Set(i); - } + // Fill in fee and size. + entries[mapping[i]].feerate = depgraph.entries[i].feerate; + // Fill in dependencies by mapping direct parents. + SetType parents; + for (auto j : depgraph.GetReducedParents(i)) parents.Set(mapping[j]); + AddDependencies(parents, mapping[i]); } } @@ -164,21 +137,26 @@ class DepGraph return new_idx; } - /** Modify this transaction graph, adding a dependency between a specified parent and child. + /** Modify this transaction graph, adding multiple parents to a specified child. * * Complexity: O(N) where N=TxCount(). - **/ - void AddDependency(ClusterIndex parent, ClusterIndex child) noexcept + */ + void AddDependencies(const SetType& parents, ClusterIndex child) noexcept { - // Bail out if dependency is already implied. - if (entries[child].ancestors[parent]) return; - // To each ancestor of the parent, add as descendants the descendants of the child. + // Compute the ancestors of parents that are not already ancestors of child. + SetType par_anc; + for (auto par : parents - Ancestors(child)) { + par_anc |= Ancestors(par); + } + par_anc -= Ancestors(child); + // Bail out if there are no such ancestors. + if (par_anc.None()) return; + // To each such ancestor, add as descendants the descendants of the child. const auto& chl_des = entries[child].descendants; - for (auto anc_of_par : Ancestors(parent)) { + for (auto anc_of_par : par_anc) { entries[anc_of_par].descendants |= chl_des; } - // To each descendant of the child, add as ancestors the ancestors of the parent. - const auto& par_anc = entries[parent].ancestors; + // To each descendant of the child, add those ancestors. for (auto dec_of_chl : Descendants(child)) { entries[dec_of_chl].ancestors |= par_anc; } diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp index 4976afbaad..bf7db4482c 100644 --- a/src/test/fuzz/cluster_linearize.cpp +++ b/src/test/fuzz/cluster_linearize.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -176,7 +177,7 @@ void MakeConnected(DepGraph& depgraph) while (todo.Any()) { auto nextcomp = depgraph.FindConnectedComponent(todo); Assume(depgraph.IsConnected(nextcomp)); - depgraph.AddDependency(comp.Last(), nextcomp.First()); + depgraph.AddDependencies(BS::Singleton(comp.Last()), nextcomp.First()); todo -= nextcomp; comp = nextcomp; } @@ -240,32 +241,65 @@ std::vector ReadLinearization(const DepGraph& depgraph, SpanRe } // namespace -FUZZ_TARGET(clusterlin_add_dependency) +FUZZ_TARGET(clusterlin_add_dependencies) { - // Verify that computing a DepGraph from a cluster, or building it step by step using AddDependency - // have the same effect. + // Verify that computing a DepGraph from a cluster, or building it step by step using + // AddDependencies has the same effect. - // Construct a cluster of a certain length, with no dependencies. FuzzedDataProvider provider(buffer.data(), buffer.size()); - auto num_tx = provider.ConsumeIntegralInRange(2, 32); + auto rng_seed = provider.ConsumeIntegral(); + InsecureRandomContext rng(rng_seed); + + // Construct a cluster of a certain length, with no dependencies. + auto num_tx = provider.ConsumeIntegralInRange(2, TestBitSet::Size()); Cluster cluster(num_tx, std::pair{FeeFrac{0, 1}, TestBitSet{}}); // Construct the corresponding DepGraph object (also no dependencies). - DepGraph depgraph(cluster); - SanityCheck(depgraph); - // Read (parent, child) pairs, and add them to the cluster and depgraph. - LIMITED_WHILE(provider.remaining_bytes() > 0, TestBitSet::Size() * TestBitSet::Size()) { - auto parent = provider.ConsumeIntegralInRange(0, num_tx - 1); - auto child = provider.ConsumeIntegralInRange(0, num_tx - 2); - child += (child >= parent); - cluster[child].second.Set(parent); - depgraph.AddDependency(parent, child); - assert(depgraph.Ancestors(child)[parent]); - assert(depgraph.Descendants(parent)[child]); + DepGraph depgraph_batch(cluster); + SanityCheck(depgraph_batch); + // Read (parents, child) pairs, and add the dependencies to the cluster and depgraph. + std::vector> deps_list; + LIMITED_WHILE(provider.remaining_bytes() > 0, TestBitSet::Size()) { + const auto parents_mask = provider.ConsumeIntegralInRange(0, (uint64_t{1} << num_tx) - 1); + auto child = provider.ConsumeIntegralInRange(0, num_tx - 1); + + auto parents_mask_shifted = parents_mask; + TestBitSet deps; + for (ClusterIndex i = 0; i < num_tx; ++i) { + if (parents_mask_shifted & 1) { + deps.Set(i); + cluster[child].second.Set(i); + } + parents_mask_shifted >>= 1; + } + assert(parents_mask_shifted == 0); + depgraph_batch.AddDependencies(deps, child); + for (auto i : deps) { + deps_list.emplace_back(i, child); + assert(depgraph_batch.Ancestors(child)[i]); + assert(depgraph_batch.Descendants(i)[child]); + } } // Sanity check the result. - SanityCheck(depgraph); + SanityCheck(depgraph_batch); // Verify that the resulting DepGraph matches one recomputed from the cluster. - assert(DepGraph(cluster) == depgraph); + assert(DepGraph(cluster) == depgraph_batch); + + DepGraph depgraph_individual; + // Add all transactions to depgraph_individual. + for (const auto& [feerate, parents] : cluster) { + depgraph_individual.AddTransaction(feerate); + } + SanityCheck(depgraph_individual); + // Add all individual dependencies to depgraph_individual in randomized order. + std::shuffle(deps_list.begin(), deps_list.end(), rng); + for (auto [parent, child] : deps_list) { + depgraph_individual.AddDependencies(TestBitSet::Singleton(parent), child); + assert(depgraph_individual.Ancestors(child)[parent]); + assert(depgraph_individual.Descendants(parent)[child]); + } + // Sanity check and compare again the batched version. + SanityCheck(depgraph_individual); + assert(depgraph_individual == depgraph_batch); } FUZZ_TARGET(clusterlin_cluster_serialization) @@ -897,12 +931,16 @@ FUZZ_TARGET(clusterlin_postlinearize_tree) if (direction & 1) { for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { auto children = depgraph_gen.GetReducedChildren(i); - if (children.Any()) depgraph_tree.AddDependency(i, children.First()); + if (children.Any()) { + depgraph_tree.AddDependencies(TestBitSet::Singleton(i), children.First()); + } } } else { for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { auto parents = depgraph_gen.GetReducedParents(i); - if (parents.Any()) depgraph_tree.AddDependency(parents.First(), i); + if (parents.Any()) { + depgraph_tree.AddDependencies(TestBitSet::Singleton(parents.First()), i); + } } } diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index a9ae4ff12a..bec4d41d38 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -234,7 +234,7 @@ struct DepGraphFormatter if (new_feerate.IsEmpty()) break; assert(reordering.size() < SetType::Size()); auto topo_idx = topo_depgraph.AddTransaction(new_feerate); - for (auto parent : new_ancestors) topo_depgraph.AddDependency(parent, topo_idx); + topo_depgraph.AddDependencies(new_ancestors, topo_idx); diff %= total_size + 1; // Insert the new transaction at distance diff back from the end. for (auto& pos : reordering) { From 0606e66fdbb914f984433d8950f0c32b5fb871f3 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 9 Sep 2024 15:12:15 -0400 Subject: [PATCH 29/36] clusterlin: add DepGraph::RemoveTransactions and support for holes in DepGraph This commits introduces support in DepGraph for the transaction positions to be non-continuous. Specifically, it adds: * DepGraph::RemoveTransactions which removes 0 or more positions from a DepGraph. * DepGraph::Positions() to get a set of which positions are in use. * DepGraph::PositionRange() to get the highest used position in a DepGraph + 1. In addition, it extends the DepGraphFormatter format to support holes in a compatible way (it serializes non-holey DepGraphs identically to the old code, and deserializes them the same way) --- src/cluster_linearize.h | 120 +++++++++++++++++++++----- src/test/cluster_linearize_tests.cpp | 39 +++++++++ src/test/feefrac_tests.cpp | 2 +- src/test/fuzz/cluster_linearize.cpp | 51 +++++++---- src/test/util/cluster_linearize.h | 124 +++++++++++++++++++-------- src/util/check.h | 2 +- src/util/feefrac.h | 8 +- 7 files changed, 267 insertions(+), 79 deletions(-) diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h index bcc455874d..a2450f5490 100644 --- a/src/cluster_linearize.h +++ b/src/cluster_linearize.h @@ -57,9 +57,20 @@ class DepGraph /** Data for each transaction, in the same order as the Cluster it was constructed from. */ std::vector entries; + /** Which positions are used. */ + SetType m_used; + public: /** Equality operator (primarily for testing purposes). */ - friend bool operator==(const DepGraph&, const DepGraph&) noexcept = default; + friend bool operator==(const DepGraph& a, const DepGraph& b) noexcept + { + if (a.m_used != b.m_used) return false; + // Only compare the used positions within the entries vector. + for (auto idx : a.m_used) { + if (a.entries[idx] != b.entries[idx]) return false; + } + return true; + } // Default constructors. DepGraph() noexcept = default; @@ -80,6 +91,7 @@ class DepGraph entries[i].ancestors = SetType::Singleton(i); entries[i].descendants = SetType::Singleton(i); } + m_used = SetType::Fill(ntx); } /** Construct a DepGraph object given a cluster. @@ -97,24 +109,50 @@ class DepGraph } /** Construct a DepGraph object given another DepGraph and a mapping from old to new. + * + * @param depgraph The original DepGraph that is being remapped. + * + * @param mapping A Span such that mapping[i] gives the position in the new DepGraph + * for position i in the old depgraph. Its size must be equal to + * depgraph.PositionRange(). The value of mapping[i] is ignored if + * position i is a hole in depgraph (i.e., if !depgraph.Positions()[i]). + * + * @param pos_range The PositionRange() for the new DepGraph. It must equal the largest + * value in mapping for any used position in depgraph plus 1, or 0 if + * depgraph.TxCount() == 0. * * Complexity: O(N^2) where N=depgraph.TxCount(). */ - DepGraph(const DepGraph& depgraph, Span mapping) noexcept : DepGraph(depgraph.TxCount()) + DepGraph(const DepGraph& depgraph, Span mapping, ClusterIndex pos_range) noexcept : entries(pos_range) { - Assert(mapping.size() == depgraph.TxCount()); - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + Assume(mapping.size() == depgraph.PositionRange()); + Assume((pos_range == 0) == (depgraph.TxCount() == 0)); + for (ClusterIndex i : depgraph.Positions()) { + auto new_idx = mapping[i]; + Assume(new_idx < pos_range); + // Add transaction. + entries[new_idx].ancestors = SetType::Singleton(new_idx); + entries[new_idx].descendants = SetType::Singleton(new_idx); + m_used.Set(new_idx); // Fill in fee and size. - entries[mapping[i]].feerate = depgraph.entries[i].feerate; + entries[new_idx].feerate = depgraph.entries[i].feerate; + } + for (ClusterIndex i : depgraph.Positions()) { // Fill in dependencies by mapping direct parents. SetType parents; for (auto j : depgraph.GetReducedParents(i)) parents.Set(mapping[j]); AddDependencies(parents, mapping[i]); } + // Verify that the provided pos_range was correct (no unused positions at the end). + Assume(m_used.None() ? (pos_range == 0) : (pos_range == m_used.Last() + 1)); } + /** Get the set of transactions positions in use. Complexity: O(1). */ + const SetType& Positions() const noexcept { return m_used; } + /** Get the range of positions in this DepGraph. All entries in Positions() are in [0, PositionRange() - 1]. */ + ClusterIndex PositionRange() const noexcept { return entries.size(); } /** Get the number of transactions in the graph. Complexity: O(1). */ - auto TxCount() const noexcept { return entries.size(); } + auto TxCount() const noexcept { return m_used.Count(); } /** Get the feerate of a given transaction i. Complexity: O(1). */ const FeeFrac& FeeRate(ClusterIndex i) const noexcept { return entries[i].feerate; } /** Get the mutable feerate of a given transaction i. Complexity: O(1). */ @@ -124,25 +162,59 @@ class DepGraph /** Get the descendants of a given transaction i. Complexity: O(1). */ const SetType& Descendants(ClusterIndex i) const noexcept { return entries[i].descendants; } - /** Add a new unconnected transaction to this transaction graph (at the end), and return its - * ClusterIndex. + /** Add a new unconnected transaction to this transaction graph (in the first available + * position), and return its ClusterIndex. * * Complexity: O(1) (amortized, due to resizing of backing vector). */ ClusterIndex AddTransaction(const FeeFrac& feefrac) noexcept { - Assume(TxCount() < SetType::Size()); - ClusterIndex new_idx = TxCount(); - entries.emplace_back(feefrac, SetType::Singleton(new_idx), SetType::Singleton(new_idx)); + static constexpr auto ALL_POSITIONS = SetType::Fill(SetType::Size()); + auto available = ALL_POSITIONS - m_used; + Assume(available.Any()); + ClusterIndex new_idx = available.First(); + if (new_idx == entries.size()) { + entries.emplace_back(feefrac, SetType::Singleton(new_idx), SetType::Singleton(new_idx)); + } else { + entries[new_idx] = Entry(feefrac, SetType::Singleton(new_idx), SetType::Singleton(new_idx)); + } + m_used.Set(new_idx); return new_idx; } + /** Remove the specified positions from this DepGraph. + * + * The specified positions will no longer be part of Positions(), and dependencies with them are + * removed. Note that due to DepGraph only tracking ancestors/descendants (and not direct + * dependencies), if a parent is removed while a grandparent remains, the grandparent will + * remain an ancestor. + * + * Complexity: O(N) where N=TxCount(). + */ + void RemoveTransactions(const SetType& del) noexcept + { + m_used -= del; + // Remove now-unused trailing entries. + while (!entries.empty() && !m_used[entries.size() - 1]) { + entries.pop_back(); + } + // Remove the deleted transactions from ancestors/descendants of other transactions. Note + // that the deleted positions will retain old feerate and dependency information. This does + // not matter as they will be overwritten by AddTransaction if they get used again. + for (auto& entry : entries) { + entry.ancestors &= m_used; + entry.descendants &= m_used; + } + } + /** Modify this transaction graph, adding multiple parents to a specified child. * * Complexity: O(N) where N=TxCount(). */ void AddDependencies(const SetType& parents, ClusterIndex child) noexcept { + Assume(m_used[child]); + Assume(parents.IsSubsetOf(m_used)); // Compute the ancestors of parents that are not already ancestors of child. SetType par_anc; for (auto par : parents - Ancestors(child)) { @@ -257,7 +329,7 @@ class DepGraph * * Complexity: O(TxCount()). */ - bool IsConnected() const noexcept { return IsConnected(SetType::Fill(TxCount())); } + bool IsConnected() const noexcept { return IsConnected(m_used); } /** Append the entries of select to list in a topologically valid order. * @@ -507,11 +579,11 @@ class AncestorCandidateFinder */ AncestorCandidateFinder(const DepGraph& depgraph LIFETIMEBOUND) noexcept : m_depgraph(depgraph), - m_todo{SetType::Fill(depgraph.TxCount())}, - m_ancestor_set_feerates(depgraph.TxCount()) + m_todo{depgraph.Positions()}, + m_ancestor_set_feerates(depgraph.PositionRange()) { // Precompute ancestor-set feerates. - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + for (ClusterIndex i : m_depgraph.Positions()) { /** The remaining ancestors for transaction i. */ SetType anc_to_add = m_depgraph.Ancestors(i); FeeFrac anc_feerate; @@ -634,22 +706,26 @@ class SearchCandidateFinder SearchCandidateFinder(const DepGraph& depgraph, uint64_t rng_seed) noexcept : m_rng(rng_seed), m_sorted_to_original(depgraph.TxCount()), - m_original_to_sorted(depgraph.TxCount()), - m_todo(SetType::Fill(depgraph.TxCount())) + m_original_to_sorted(depgraph.PositionRange()) { - // Determine reordering mapping, by sorting by decreasing feerate. - std::iota(m_sorted_to_original.begin(), m_sorted_to_original.end(), ClusterIndex{0}); + // Determine reordering mapping, by sorting by decreasing feerate. Unusued positions are + // not included, as they will never be looked up anyway. + ClusterIndex sorted_pos{0}; + for (auto i : depgraph.Positions()) { + m_sorted_to_original[sorted_pos++] = i; + } std::sort(m_sorted_to_original.begin(), m_sorted_to_original.end(), [&](auto a, auto b) { auto feerate_cmp = depgraph.FeeRate(a) <=> depgraph.FeeRate(b); if (feerate_cmp == 0) return a < b; return feerate_cmp > 0; }); // Compute reverse mapping. - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + for (ClusterIndex i = 0; i < m_sorted_to_original.size(); ++i) { m_original_to_sorted[m_sorted_to_original[i]] = i; } // Compute reordered dependency graph. - m_sorted_depgraph = DepGraph(depgraph, m_original_to_sorted); + m_sorted_depgraph = DepGraph(depgraph, m_original_to_sorted, m_sorted_to_original.size()); + m_todo = m_sorted_depgraph.Positions(); } /** Check whether any unlinearized transactions remain. */ @@ -1161,7 +1237,7 @@ void PostLinearize(const DepGraph& depgraph, Span lineari // During an even pass, the diagram above would correspond to linearization [2,3,0,1], with // groups [2] and [3,0,1]. - std::vector entries(linearization.size() + 1); + std::vector entries(depgraph.PositionRange() + 1); // Perform two passes over the linearization. for (int pass = 0; pass < 2; ++pass) { diff --git a/src/test/cluster_linearize_tests.cpp b/src/test/cluster_linearize_tests.cpp index d15e783ea1..cb87dcbffb 100644 --- a/src/test/cluster_linearize_tests.cpp +++ b/src/test/cluster_linearize_tests.cpp @@ -18,6 +18,10 @@ using namespace cluster_linearize; namespace { +/** Special magic value that indicates to TestDepGraphSerialization that a cluster entry represents + * a hole. */ +constexpr std::pair HOLE{FeeFrac{0, 0x3FFFFF}, {}}; + template void TestDepGraphSerialization(const Cluster& cluster, const std::string& hexenc) { @@ -26,6 +30,13 @@ void TestDepGraphSerialization(const Cluster& cluster, const std::strin // Run normal sanity and correspondence checks, which includes a round-trip test. VerifyDepGraphFromCluster(cluster, depgraph); + // Remove holes (which are expected to be present as HOLE entries in cluster). + SetType holes; + for (ClusterIndex i = 0; i < cluster.size(); ++i) { + if (cluster[i] == HOLE) holes.Set(i); + } + depgraph.RemoveTransactions(holes); + // There may be multiple serializations of the same graph, but DepGraphFormatter's serializer // only produces one of those. Verify that hexenc matches that canonical serialization. std::vector encoding; @@ -133,6 +144,34 @@ BOOST_AUTO_TEST_CASE(depgraph_ser_tests) skip insertion C): D,A,B,E,C */ "00" /* end of graph */ ); + + // Transactions: A(1,2), B(3,1), C(2,1), D(1,3), E(1,1). Deps: C->A, D->A, D->B, E->D. + // In order: [_, D, _, _, A, _, B, _, _, _, E, _, _, C] (_ being holes). Internally serialized + // in order A,B,C,D,E. + TestDepGraphSerialization( + {HOLE, {{1, 3}, {4, 6}}, HOLE, HOLE, {{1, 2}, {}}, HOLE, {{3, 1}, {}}, HOLE, HOLE, HOLE, {{1, 1}, {1}}, HOLE, HOLE, {{2, 1}, {4}}}, + "02" /* A size */ + "02" /* A fee */ + "03" /* A insertion position (3 holes): _, _, _, A */ + "01" /* B size */ + "06" /* B fee */ + "06" /* B insertion position (skip B->A dependency, skip 4 inserts, add 1 hole): _, _, _, A, _, B */ + "01" /* C size */ + "04" /* C fee */ + "01" /* C->A dependency (skip C->B dependency) */ + "0b" /* C insertion position (skip 6 inserts, add 5 holes): _, _, _, A, _, B, _, _, _, _, _, C */ + "03" /* D size */ + "02" /* D fee */ + "01" /* D->B dependency (skip D->C dependency) */ + "00" /* D->A dependency (no skips) */ + "0b" /* D insertion position (skip 11 inserts): _, D, _, _, A, _, B, _, _, _, _, _, C */ + "01" /* E size */ + "02" /* E fee */ + "00" /* E->D dependency (no skips) */ + "04" /* E insertion position (skip E->C dependency, E->B and E->A are implied, skip 3 + inserts): _, D, _, _, A, _, B, _, _, _, E, _, _, C */ + "00" /* end of graph */ + ); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/feefrac_tests.cpp b/src/test/feefrac_tests.cpp index 5af3c3d7ed..41c9c0a633 100644 --- a/src/test/feefrac_tests.cpp +++ b/src/test/feefrac_tests.cpp @@ -15,7 +15,7 @@ BOOST_AUTO_TEST_CASE(feefrac_operators) FeeFrac sum{1500, 400}; FeeFrac diff{500, -200}; FeeFrac empty{0, 0}; - FeeFrac zero_fee{0, 1}; // zero-fee allowed + [[maybe_unused]] FeeFrac zero_fee{0, 1}; // zero-fee allowed BOOST_CHECK(empty == FeeFrac{}); // same as no-args diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp index bf7db4482c..e75883878e 100644 --- a/src/test/fuzz/cluster_linearize.cpp +++ b/src/test/fuzz/cluster_linearize.cpp @@ -37,7 +37,7 @@ class SimpleCandidateFinder public: /** Construct an SimpleCandidateFinder for a given graph. */ SimpleCandidateFinder(const DepGraph& depgraph LIFETIMEBOUND) noexcept : - m_depgraph(depgraph), m_todo{SetType::Fill(depgraph.TxCount())} {} + m_depgraph(depgraph), m_todo{depgraph.Positions()} {} /** Remove a set of transactions from the set of to-be-linearized ones. */ void MarkDone(SetType select) noexcept { m_todo -= select; } @@ -107,7 +107,7 @@ class ExhaustiveCandidateFinder public: /** Construct an ExhaustiveCandidateFinder for a given graph. */ ExhaustiveCandidateFinder(const DepGraph& depgraph LIFETIMEBOUND) noexcept : - m_depgraph(depgraph), m_todo{SetType::Fill(depgraph.TxCount())} {} + m_depgraph(depgraph), m_todo{depgraph.Positions()} {} /** Remove a set of transactions from the set of to-be-linearized ones. */ void MarkDone(SetType select) noexcept { m_todo -= select; } @@ -153,7 +153,7 @@ std::pair, bool> SimpleLinearize(const DepGraph linearization; SimpleCandidateFinder finder(depgraph); - SetType todo = SetType::Fill(depgraph.TxCount()); + SetType todo = depgraph.Positions(); bool optimal = true; while (todo.Any()) { auto [candidate, iterations_done] = finder.FindCandidateSet(max_iterations); @@ -170,7 +170,7 @@ std::pair, bool> SimpleLinearize(const DepGraph void MakeConnected(DepGraph& depgraph) { - auto todo = BS::Fill(depgraph.TxCount()); + auto todo = depgraph.Positions(); auto comp = depgraph.FindConnectedComponent(todo); Assume(depgraph.IsConnected(comp)); todo -= comp; @@ -206,7 +206,7 @@ template std::vector ReadLinearization(const DepGraph& depgraph, SpanReader& reader) { std::vector linearization; - TestBitSet todo = TestBitSet::Fill(depgraph.TxCount()); + TestBitSet todo = depgraph.Positions(); // In every iteration one topologically-valid transaction is appended to linearization. while (todo.Any()) { // Compute the set of transactions with no not-yet-included ancestors. @@ -327,6 +327,17 @@ FUZZ_TARGET(clusterlin_cluster_serialization) // check for the serialization). DepGraph depgraph(cluster); VerifyDepGraphFromCluster(cluster, depgraph); + + // Remove an arbitrary subset (in order to construct a graph with holes) and verify that it + // still sanity checks (incl. round-tripping serialization). + uint64_t del = provider.ConsumeIntegralInRange(1, (uint64_t{1} << TestBitSet::Size()) - 1); + TestBitSet setdel; + for (ClusterIndex i = 0; i < TestBitSet::Size(); ++i) { + if (del & 1) setdel.Set(i); + del >>= 1; + } + depgraph.RemoveTransactions(setdel); + SanityCheck(depgraph); } FUZZ_TARGET(clusterlin_depgraph_serialization) @@ -356,7 +367,7 @@ FUZZ_TARGET(clusterlin_components) reader >> Using(depgraph); } catch (const std::ios_base::failure&) {} - TestBitSet todo = TestBitSet::Fill(depgraph.TxCount()); + TestBitSet todo = depgraph.Positions(); while (todo.Any()) { // Find a connected component inside todo. auto component = depgraph.FindConnectedComponent(todo); @@ -367,7 +378,7 @@ FUZZ_TARGET(clusterlin_components) // If todo is the entire graph, and the entire graph is connected, then the component must // be the entire graph. - if (todo == TestBitSet::Fill(depgraph.TxCount())) { + if (todo == depgraph.Positions()) { assert((component == todo) == depgraph.IsConnected()); } @@ -404,7 +415,7 @@ FUZZ_TARGET(clusterlin_components) reader >> VARINT(subset_bits); } catch (const std::ios_base::failure&) {} TestBitSet subset; - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + for (ClusterIndex i : depgraph.Positions()) { if (todo[i]) { if (subset_bits & 1) subset.Set(i); subset_bits >>= 1; @@ -457,7 +468,7 @@ FUZZ_TARGET(clusterlin_chunking) } // Naively recompute the chunks (each is the highest-feerate prefix of what remains). - auto todo = TestBitSet::Fill(depgraph.TxCount()); + auto todo = depgraph.Positions(); for (const auto& chunk_feerate : chunking) { assert(todo.Any()); SetInfo accumulator, best; @@ -488,7 +499,7 @@ FUZZ_TARGET(clusterlin_ancestor_finder) } catch (const std::ios_base::failure&) {} AncestorCandidateFinder anc_finder(depgraph); - auto todo = TestBitSet::Fill(depgraph.TxCount()); + auto todo = depgraph.Positions(); while (todo.Any()) { // Call the ancestor finder's FindCandidateSet for what remains of the graph. assert(!anc_finder.AllDone()); @@ -553,7 +564,7 @@ FUZZ_TARGET(clusterlin_search_finder) ExhaustiveCandidateFinder exh_finder(depgraph); AncestorCandidateFinder anc_finder(depgraph); - auto todo = TestBitSet::Fill(depgraph.TxCount()); + auto todo = depgraph.Positions(); while (todo.Any()) { assert(!src_finder.AllDone()); assert(!smp_finder.AllDone()); @@ -657,7 +668,7 @@ FUZZ_TARGET(clusterlin_linearization_chunking) } catch (const std::ios_base::failure&) {} // Retrieve a topologically-valid subset of depgraph. - auto todo = TestBitSet::Fill(depgraph.TxCount()); + auto todo = depgraph.Positions(); auto subset = SetInfo(depgraph, ReadTopologicalSet(depgraph, todo, reader)); // Retrieve a valid linearization for depgraph. @@ -840,8 +851,8 @@ FUZZ_TARGET(clusterlin_linearize) // Only for very small clusters, test every topologically-valid permutation. if (depgraph.TxCount() <= 7) { - std::vector perm_linearization(depgraph.TxCount()); - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) perm_linearization[i] = i; + std::vector perm_linearization; + for (ClusterIndex i : depgraph.Positions()) perm_linearization.push_back(i); // Iterate over all valid permutations. do { // Determine whether perm_linearization is topological. @@ -925,9 +936,17 @@ FUZZ_TARGET(clusterlin_postlinearize_tree) // Now construct a new graph, copying the nodes, but leaving only the first parent (even // direction) or the first child (odd direction). DepGraph depgraph_tree; - for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { - depgraph_tree.AddTransaction(depgraph_gen.FeeRate(i)); + for (ClusterIndex i = 0; i < depgraph_gen.PositionRange(); ++i) { + if (depgraph_gen.Positions()[i]) { + depgraph_tree.AddTransaction(depgraph_gen.FeeRate(i)); + } else { + // For holes, add a dummy transaction which is deleted below, so that non-hole + // transactions retain their position. + depgraph_tree.AddTransaction(FeeFrac{}); + } } + depgraph_tree.RemoveTransactions(TestBitSet::Fill(depgraph_gen.PositionRange()) - depgraph_gen.Positions()); + if (direction & 1) { for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { auto children = depgraph_gen.GetReducedChildren(i); diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index bec4d41d38..44cd6590d3 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -27,7 +27,7 @@ using TestBitSet = BitSet<32>; template bool IsAcyclic(const DepGraph& depgraph) noexcept { - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + for (ClusterIndex i : depgraph.Positions()) { if ((depgraph.Ancestors(i) & depgraph.Descendants(i)) != SetType::Singleton(i)) { return false; } @@ -57,11 +57,14 @@ bool IsAcyclic(const DepGraph& depgraph) noexcept * by parent relations that were serialized before it). * - The various insertion positions in the cluster, from the very end of the cluster, to the * front. + * - The appending of 1, 2, 3, ... holes at the end of the cluster, followed by appending the new + * transaction. * - * Let's say you have a 7-transaction cluster, consisting of transactions F,A,C,B,G,E,D, but - * serialized in order A,B,C,D,E,F,G, because that happens to be a topological ordering. By the - * time G gets serialized, what has been serialized already represents the cluster F,A,C,B,E,D (in - * that order). G has B and E as direct parents, and E depends on C. + * Let's say you have a 7-transaction cluster, consisting of transactions F,A,C,B,_,G,E,_,D + * (where _ represent holes; unused positions within the DepGraph) but serialized in order + * A,B,C,D,E,F,G, because that happens to be a topological ordering. By the time G gets serialized, + * what has been serialized already represents the cluster F,A,C,B,_,E,_,D (in that order). G has B + * and E as direct parents, and E depends on C. * * In this case, the possibilities are, in order: * - [ ] the dependency G->F @@ -71,17 +74,23 @@ bool IsAcyclic(const DepGraph& depgraph) noexcept * - [ ] the dependency G->A * - [ ] put G at the end of the cluster * - [ ] put G before D + * - [ ] put G before the hole before D * - [X] put G before E + * - [ ] put G before the hole before E * - [ ] put G before B * - [ ] put G before C * - [ ] put G before A * - [ ] put G before F + * - [ ] add 1 hole at the end of the cluster, followed by G + * - [ ] add 2 holes at the end of the cluster, followed by G + * - [ ] add ... * - * The skip values in this case are 1 (G->F), 1 (G->D), 3 (G->A, G at end, G before D). No skip - * after 3 is needed (or permitted), because there can only be one position for G. Also note that - * G->C is not included in the list of possibilities, as it is implied by the included G->E and - * E->C that came before it. On deserialization, if the last skip value was 8 or larger (putting - * G before the beginning of the cluster), it is interpreted as wrapping around back to the end. + * The skip values in this case are 1 (G->F), 1 (G->D), 4 (G->A, G at end, G before D, G before + * hole). No skip after 4 is needed (or permitted), because there can only be one position for G. + * Also note that G->C is not included in the list of possibilities, as it is implied by the + * included G->E and E->C that came before it. On deserialization, if the last skip value was 8 or + * larger (putting G before the beginning of the cluster), it is interpreted as wrapping around + * back to the end. * * * Rationale: @@ -125,16 +134,17 @@ struct DepGraphFormatter static void Ser(Stream& s, const DepGraph& depgraph) { /** Construct a topological order to serialize the transactions in. */ - std::vector topo_order(depgraph.TxCount()); - std::iota(topo_order.begin(), topo_order.end(), ClusterIndex{0}); + std::vector topo_order; + topo_order.reserve(depgraph.TxCount()); + for (auto i : depgraph.Positions()) topo_order.push_back(i); std::sort(topo_order.begin(), topo_order.end(), [&](ClusterIndex a, ClusterIndex b) { auto anc_a = depgraph.Ancestors(a).Count(), anc_b = depgraph.Ancestors(b).Count(); if (anc_a != anc_b) return anc_a < anc_b; return a < b; }); - /** Which transactions the deserializer already knows when it has deserialized what has - * been serialized here so far. */ + /** Which positions (incl. holes) the deserializer already knows when it has deserialized + * what has been serialized here so far. */ SetType done; // Loop over the transactions in topological order. @@ -165,10 +175,19 @@ struct DepGraphFormatter } } // Write position information. - // The new transaction is to be inserted N positions back from the end of the cluster. - // Emit N to indicate that that many insertion choices are skipped. - auto skips = (done - SetType::Fill(idx)).Count(); - s << VARINT(diff + skips); + auto add_holes = SetType::Fill(idx) - done - depgraph.Positions(); + if (add_holes.None()) { + // The new transaction is to be inserted N positions back from the end of the + // cluster. Emit N to indicate that that many insertion choices are skipped. + auto skips = (done - SetType::Fill(idx)).Count(); + s << VARINT(diff + skips); + } else { + // The new transaction is to be appended at the end of the cluster, after N holes. + // Emit current_cluster_size + N, to indicate all insertion choices are skipped, + // plus N possibilities for the number of holes. + s << VARINT(diff + done.Count() + add_holes.Count()); + done |= add_holes; + } done.Set(idx); } @@ -185,8 +204,7 @@ struct DepGraphFormatter /** Mapping from serialization order to cluster order, used later to reconstruct the * cluster order. */ std::vector reordering; - /** How big the entries vector in the reconstructed depgraph will be (before the - * introduction of holes in a further commit, this always equals reordering.size()). */ + /** How big the entries vector in the reconstructed depgraph will be (including holes). */ ClusterIndex total_size{0}; // Read transactions in topological order. @@ -235,18 +253,43 @@ struct DepGraphFormatter assert(reordering.size() < SetType::Size()); auto topo_idx = topo_depgraph.AddTransaction(new_feerate); topo_depgraph.AddDependencies(new_ancestors, topo_idx); - diff %= total_size + 1; - // Insert the new transaction at distance diff back from the end. - for (auto& pos : reordering) { - pos += (pos >= total_size - diff); + if (total_size < SetType::Size()) { + // Normal case. + diff %= SetType::Size(); + if (diff <= total_size) { + // Insert the new transaction at distance diff back from the end. + for (auto& pos : reordering) { + pos += (pos >= total_size - diff); + } + reordering.push_back(total_size++ - diff); + } else { + // Append diff - total_size holes at the end, plus the new transaction. + total_size = diff; + reordering.push_back(total_size++); + } + } else { + // In case total_size == SetType::Size, it is not possible to insert the new + // transaction without exceeding SetType's size. Instead, interpret diff as an + // index into the holes, and overwrite a position there. This branch is never used + // when deserializing the output of the serializer, but gives meaning to otherwise + // invalid input. + diff %= (SetType::Size() - reordering.size()); + SetType holes = SetType::Fill(SetType::Size()); + for (auto pos : reordering) holes.Reset(pos); + for (auto pos : holes) { + if (diff == 0) { + reordering.push_back(pos); + break; + } + --diff; + } } - reordering.push_back(total_size++ - diff); // Stop if a read error was encountered during deserialization. if (read_error) break; } // Construct the original cluster order depgraph. - depgraph = DepGraph(topo_depgraph, reordering); + depgraph = DepGraph(topo_depgraph, reordering, total_size); } }; @@ -254,8 +297,19 @@ struct DepGraphFormatter template void SanityCheck(const DepGraph& depgraph) { + // Verify Positions and PositionRange consistency. + ClusterIndex num_positions{0}; + ClusterIndex position_range{0}; + for (ClusterIndex i : depgraph.Positions()) { + ++num_positions; + position_range = i + 1; + } + assert(num_positions == depgraph.TxCount()); + assert(position_range == depgraph.PositionRange()); + assert(position_range >= num_positions); + assert(position_range <= SetType::Size()); // Consistency check between ancestors internally. - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + for (ClusterIndex i : depgraph.Positions()) { // Transactions include themselves as ancestors. assert(depgraph.Ancestors(i)[i]); // If a is an ancestor of b, then b's ancestors must include all of a's ancestors. @@ -264,8 +318,8 @@ void SanityCheck(const DepGraph& depgraph) } } // Consistency check between ancestors and descendants. - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { - for (ClusterIndex j = 0; j < depgraph.TxCount(); ++j) { + for (ClusterIndex i : depgraph.Positions()) { + for (ClusterIndex j : depgraph.Positions()) { assert(depgraph.Ancestors(i)[j] == depgraph.Descendants(j)[i]); } // No transaction is a parent or child of itself. @@ -305,12 +359,12 @@ void SanityCheck(const DepGraph& depgraph) // In acyclic graphs, the union of parents with parents of parents etc. yields the // full ancestor set (and similar for children and descendants). - std::vector parents, children; - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { - parents.push_back(depgraph.GetReducedParents(i)); - children.push_back(depgraph.GetReducedChildren(i)); + std::vector parents(depgraph.PositionRange()), children(depgraph.PositionRange()); + for (ClusterIndex i : depgraph.Positions()) { + parents[i] = depgraph.GetReducedParents(i); + children[i] = depgraph.GetReducedChildren(i); } - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { + for (auto i : depgraph.Positions()) { // Initialize the set of ancestors with just the current transaction itself. SetType ancestors = SetType::Singleton(i); // Iteratively add parents of all transactions in the ancestor set to itself. @@ -382,7 +436,7 @@ void SanityCheck(const DepGraph& depgraph, Span lin TestBitSet done; for (auto i : linearization) { // Check transaction position is in range. - assert(i < depgraph.TxCount()); + assert(depgraph.Positions()[i]); // Check topology and lack of duplicates. assert((depgraph.Ancestors(i) - done) == TestBitSet::Singleton(i)); done.Set(i); diff --git a/src/util/check.h b/src/util/check.h index a02a1de8dc..8f28f5dc94 100644 --- a/src/util/check.h +++ b/src/util/check.h @@ -40,7 +40,7 @@ void assertion_fail(std::string_view file, int line, std::string_view func, std: /** Helper for Assert()/Assume() */ template -T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion) +constexpr T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion) { if constexpr (IS_ASSERT #ifdef ABORT_ON_FAILED_ASSUME diff --git a/src/util/feefrac.h b/src/util/feefrac.h index 9772162010..161322b50a 100644 --- a/src/util/feefrac.h +++ b/src/util/feefrac.h @@ -64,13 +64,13 @@ struct FeeFrac int32_t size; /** Construct an IsEmpty() FeeFrac. */ - inline FeeFrac() noexcept : fee{0}, size{0} {} + constexpr inline FeeFrac() noexcept : fee{0}, size{0} {} /** Construct a FeeFrac with specified fee and size. */ - inline FeeFrac(int64_t f, int32_t s) noexcept : fee{f}, size{s} {} + constexpr inline FeeFrac(int64_t f, int32_t s) noexcept : fee{f}, size{s} {} - inline FeeFrac(const FeeFrac&) noexcept = default; - inline FeeFrac& operator=(const FeeFrac&) noexcept = default; + constexpr inline FeeFrac(const FeeFrac&) noexcept = default; + constexpr inline FeeFrac& operator=(const FeeFrac&) noexcept = default; /** Check if this is empty (size and fee are 0). */ bool inline IsEmpty() const noexcept { From 1c24c625105cd62518d2eee7e95c3b1c74ea9923 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 23 Sep 2024 10:02:59 -0400 Subject: [PATCH 30/36] clusterlin: merge two DepGraph fuzz tests into simulation test This combines the clusterlin_add_dependency and clusterlin_cluster_serialization fuzz tests into a single clusterlin_depgraph_sim fuzz test. This tests starts from an empty DepGraph and performs a arbitrary number of AddTransaction, AddDependencies, and RemoveTransactions operations on it, and compares the resulting state with a naive reimplementation. --- src/test/fuzz/cluster_linearize.cpp | 224 +++++++++++++++++----------- 1 file changed, 139 insertions(+), 85 deletions(-) diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp index e75883878e..5b3770636a 100644 --- a/src/test/fuzz/cluster_linearize.cpp +++ b/src/test/fuzz/cluster_linearize.cpp @@ -241,103 +241,157 @@ std::vector ReadLinearization(const DepGraph& depgraph, SpanRe } // namespace -FUZZ_TARGET(clusterlin_add_dependencies) +FUZZ_TARGET(clusterlin_depgraph_sim) { - // Verify that computing a DepGraph from a cluster, or building it step by step using - // AddDependencies has the same effect. + // Simulation test to verify the full behavior of DepGraph. FuzzedDataProvider provider(buffer.data(), buffer.size()); - auto rng_seed = provider.ConsumeIntegral(); - InsecureRandomContext rng(rng_seed); - - // Construct a cluster of a certain length, with no dependencies. - auto num_tx = provider.ConsumeIntegralInRange(2, TestBitSet::Size()); - Cluster cluster(num_tx, std::pair{FeeFrac{0, 1}, TestBitSet{}}); - // Construct the corresponding DepGraph object (also no dependencies). - DepGraph depgraph_batch(cluster); - SanityCheck(depgraph_batch); - // Read (parents, child) pairs, and add the dependencies to the cluster and depgraph. - std::vector> deps_list; - LIMITED_WHILE(provider.remaining_bytes() > 0, TestBitSet::Size()) { - const auto parents_mask = provider.ConsumeIntegralInRange(0, (uint64_t{1} << num_tx) - 1); - auto child = provider.ConsumeIntegralInRange(0, num_tx - 1); - - auto parents_mask_shifted = parents_mask; - TestBitSet deps; - for (ClusterIndex i = 0; i < num_tx; ++i) { - if (parents_mask_shifted & 1) { - deps.Set(i); - cluster[child].second.Set(i); + + /** Real DepGraph being tested. */ + DepGraph real; + /** Simulated DepGraph (sim[i] is std::nullopt if position i does not exist; otherwise, + * sim[i]->first is its individual feerate, and sim[i]->second is its set of ancestors. */ + std::array>, TestBitSet::Size()> sim; + /** The number of non-nullopt position in sim. */ + ClusterIndex num_tx_sim{0}; + + /** Read a valid index of a transaction from the provider. */ + auto idx_fn = [&]() { + auto offset = provider.ConsumeIntegralInRange(0, num_tx_sim - 1); + for (ClusterIndex i = 0; i < sim.size(); ++i) { + if (!sim[i].has_value()) continue; + if (offset == 0) return i; + --offset; + } + assert(false); + return ClusterIndex(-1); + }; + + /** Read a valid subset of the transactions from the provider. */ + auto subset_fn = [&]() { + auto range = (uint64_t{1} << num_tx_sim) - 1; + const auto mask = provider.ConsumeIntegralInRange(0, range); + auto mask_shifted = mask; + TestBitSet subset; + for (ClusterIndex i = 0; i < sim.size(); ++i) { + if (!sim[i].has_value()) continue; + if (mask_shifted & 1) { + subset.Set(i); } - parents_mask_shifted >>= 1; + mask_shifted >>= 1; } - assert(parents_mask_shifted == 0); - depgraph_batch.AddDependencies(deps, child); - for (auto i : deps) { - deps_list.emplace_back(i, child); - assert(depgraph_batch.Ancestors(child)[i]); - assert(depgraph_batch.Descendants(i)[child]); + assert(mask_shifted == 0); + return subset; + }; + + /** Read any set of transactions from the provider (including unused positions). */ + auto set_fn = [&]() { + auto range = (uint64_t{1} << sim.size()) - 1; + const auto mask = provider.ConsumeIntegralInRange(0, range); + TestBitSet set; + for (ClusterIndex i = 0; i < sim.size(); ++i) { + if ((mask >> i) & 1) { + set.Set(i); + } } - } - // Sanity check the result. - SanityCheck(depgraph_batch); - // Verify that the resulting DepGraph matches one recomputed from the cluster. - assert(DepGraph(cluster) == depgraph_batch); - - DepGraph depgraph_individual; - // Add all transactions to depgraph_individual. - for (const auto& [feerate, parents] : cluster) { - depgraph_individual.AddTransaction(feerate); - } - SanityCheck(depgraph_individual); - // Add all individual dependencies to depgraph_individual in randomized order. - std::shuffle(deps_list.begin(), deps_list.end(), rng); - for (auto [parent, child] : deps_list) { - depgraph_individual.AddDependencies(TestBitSet::Singleton(parent), child); - assert(depgraph_individual.Ancestors(child)[parent]); - assert(depgraph_individual.Descendants(parent)[child]); - } - // Sanity check and compare again the batched version. - SanityCheck(depgraph_individual); - assert(depgraph_individual == depgraph_batch); -} + return set; + }; -FUZZ_TARGET(clusterlin_cluster_serialization) -{ - // Verify that any graph of transactions has its ancestry correctly computed by DepGraph, and - // if it is a DAG, that it can be serialized as a DepGraph in a way that roundtrips. This - // guarantees that any acyclic cluster has a corresponding DepGraph serialization. + /** Propagate ancestor information in sim. */ + auto anc_update_fn = [&]() { + while (true) { + bool updates{false}; + for (ClusterIndex chl = 0; chl < sim.size(); ++chl) { + if (!sim[chl].has_value()) continue; + for (auto par : sim[chl]->second) { + if (!sim[chl]->second.IsSupersetOf(sim[par]->second)) { + sim[chl]->second |= sim[par]->second; + updates = true; + } + } + } + if (!updates) break; + } + }; - FuzzedDataProvider provider(buffer.data(), buffer.size()); + /** Compare the state of transaction i in the simulation with the real one. */ + auto check_fn = [&](ClusterIndex i) { + // Compare used positions. + assert(real.Positions()[i] == sim[i].has_value()); + if (sim[i].has_value()) { + // Compare feerate. + assert(real.FeeRate(i) == sim[i]->first); + // Compare ancestors (note that SanityCheck verifies correspondence between ancestors + // and descendants, so we can restrict ourselves to ancestors here). + assert(real.Ancestors(i) == sim[i]->second); + } + }; - // Construct a cluster in a naive way (using a FuzzedDataProvider-based serialization). - Cluster cluster; - auto num_tx = provider.ConsumeIntegralInRange(1, 32); - cluster.resize(num_tx); - for (ClusterIndex i = 0; i < num_tx; ++i) { - cluster[i].first.size = provider.ConsumeIntegralInRange(1, 0x3fffff); - cluster[i].first.fee = provider.ConsumeIntegralInRange(-0x8000000000000, 0x7ffffffffffff); - for (ClusterIndex j = 0; j < num_tx; ++j) { - if (i == j) continue; - if (provider.ConsumeBool()) cluster[i].second.Set(j); + LIMITED_WHILE(provider.remaining_bytes() > 0, 1000) { + uint8_t command = provider.ConsumeIntegral(); + if (num_tx_sim == 0 || ((command % 3) <= 0 && num_tx_sim < TestBitSet::Size())) { + // AddTransaction. + auto fee = provider.ConsumeIntegralInRange(-0x8000000000000, 0x7ffffffffffff); + auto size = provider.ConsumeIntegralInRange(1, 0x3fffff); + FeeFrac feerate{fee, size}; + // Apply to DepGraph. + auto idx = real.AddTransaction(feerate); + // Verify that the returned index is correct. + assert(!sim[idx].has_value()); + for (ClusterIndex i = 0; i < TestBitSet::Size(); ++i) { + if (!sim[i].has_value()) { + assert(idx == i); + break; + } + } + // Update sim. + sim[idx] = {feerate, TestBitSet::Singleton(idx)}; + ++num_tx_sim; + continue; + } + if ((command % 3) <= 1 && num_tx_sim > 0) { + // AddDependencies. + ClusterIndex child = idx_fn(); + auto parents = subset_fn(); + // Apply to DepGraph. + real.AddDependencies(parents, child); + // Apply to sim. + sim[child]->second |= parents; + continue; + } + if (num_tx_sim > 0) { + // Remove transactions. + auto del = set_fn(); + // Propagate all ancestry information before deleting anything in the simulation (as + // intermediary transactions may be deleted which impact connectivity). + anc_update_fn(); + // Compare the state of the transactions being deleted. + for (auto i : del) check_fn(i); + // Apply to DepGraph. + real.RemoveTransactions(del); + // Apply to sim. + for (ClusterIndex i = 0; i < sim.size(); ++i) { + if (sim[i].has_value()) { + if (del[i]) { + --num_tx_sim; + sim[i] = std::nullopt; + } else { + sim[i]->second -= del; + } + } + } + continue; } + // This should be unreachable (one of the 3 above actions should always be possible). + assert(false); } - // Construct dependency graph, and verify it matches the cluster (which includes a round-trip - // check for the serialization). - DepGraph depgraph(cluster); - VerifyDepGraphFromCluster(cluster, depgraph); - - // Remove an arbitrary subset (in order to construct a graph with holes) and verify that it - // still sanity checks (incl. round-tripping serialization). - uint64_t del = provider.ConsumeIntegralInRange(1, (uint64_t{1} << TestBitSet::Size()) - 1); - TestBitSet setdel; - for (ClusterIndex i = 0; i < TestBitSet::Size(); ++i) { - if (del & 1) setdel.Set(i); - del >>= 1; - } - depgraph.RemoveTransactions(setdel); - SanityCheck(depgraph); + // Compare the real obtained depgraph against the simulation. + anc_update_fn(); + for (ClusterIndex i = 0; i < sim.size(); ++i) check_fn(i); + assert(real.TxCount() == num_tx_sim); + // Sanity check the result (which includes round-tripping serialization, if applicable). + SanityCheck(real); } FUZZ_TARGET(clusterlin_depgraph_serialization) From 0b3ec8c59b2b6d598531cd4e688eb276e597c825 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 23 Sep 2024 11:32:58 -0400 Subject: [PATCH 31/36] clusterlin: remove Cluster type --- src/cluster_linearize.h | 39 +--------------------------- src/test/cluster_linearize_tests.cpp | 14 +++++----- src/test/util/cluster_linearize.h | 37 -------------------------- 3 files changed, 8 insertions(+), 82 deletions(-) diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h index a2450f5490..757c81f108 100644 --- a/src/cluster_linearize.h +++ b/src/cluster_linearize.h @@ -19,14 +19,6 @@ namespace cluster_linearize { -/** Data type to represent cluster input. - * - * cluster[i].first is tx_i's fee and size. - * cluster[i].second[j] is true iff tx_i spends one or more of tx_j's outputs. - */ -template -using Cluster = std::vector>; - /** Data type to represent transaction indices in clusters. */ using ClusterIndex = uint32_t; @@ -54,7 +46,7 @@ class DepGraph Entry(const FeeFrac& f, const SetType& a, const SetType& d) noexcept : feerate(f), ancestors(a), descendants(d) {} }; - /** Data for each transaction, in the same order as the Cluster it was constructed from. */ + /** Data for each transaction. */ std::vector entries; /** Which positions are used. */ @@ -79,35 +71,6 @@ class DepGraph DepGraph& operator=(const DepGraph&) noexcept = default; DepGraph& operator=(DepGraph&&) noexcept = default; - /** Construct a DepGraph object for ntx transactions, with no dependencies. - * - * Complexity: O(N) where N=ntx. - **/ - explicit DepGraph(ClusterIndex ntx) noexcept - { - Assume(ntx <= SetType::Size()); - entries.resize(ntx); - for (ClusterIndex i = 0; i < ntx; ++i) { - entries[i].ancestors = SetType::Singleton(i); - entries[i].descendants = SetType::Singleton(i); - } - m_used = SetType::Fill(ntx); - } - - /** Construct a DepGraph object given a cluster. - * - * Complexity: O(N^2) where N=cluster.size(). - */ - explicit DepGraph(const Cluster& cluster) noexcept : DepGraph(cluster.size()) - { - for (ClusterIndex i = 0; i < cluster.size(); ++i) { - // Fill in fee and size. - entries[i].feerate = cluster[i].first; - // Fill in dependencies. - AddDependencies(cluster[i].second, i); - } - } - /** Construct a DepGraph object given another DepGraph and a mapping from old to new. * * @param depgraph The original DepGraph that is being remapped. diff --git a/src/test/cluster_linearize_tests.cpp b/src/test/cluster_linearize_tests.cpp index cb87dcbffb..265ccdc805 100644 --- a/src/test/cluster_linearize_tests.cpp +++ b/src/test/cluster_linearize_tests.cpp @@ -23,18 +23,18 @@ namespace { constexpr std::pair HOLE{FeeFrac{0, 0x3FFFFF}, {}}; template -void TestDepGraphSerialization(const Cluster& cluster, const std::string& hexenc) +void TestDepGraphSerialization(const std::vector>& cluster, const std::string& hexenc) { - DepGraph depgraph(cluster); - - // Run normal sanity and correspondence checks, which includes a round-trip test. - VerifyDepGraphFromCluster(cluster, depgraph); - - // Remove holes (which are expected to be present as HOLE entries in cluster). + // Construct DepGraph from cluster argument. + DepGraph depgraph; SetType holes; for (ClusterIndex i = 0; i < cluster.size(); ++i) { + depgraph.AddTransaction(cluster[i].first); if (cluster[i] == HOLE) holes.Set(i); } + for (ClusterIndex i = 0; i < cluster.size(); ++i) { + depgraph.AddDependencies(cluster[i].second, i); + } depgraph.RemoveTransactions(holes); // There may be multiple serializations of the same graph, but DepGraphFormatter's serializer diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index 44cd6590d3..871aa9d74e 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -390,43 +390,6 @@ void SanityCheck(const DepGraph& depgraph) } } -/** Verify that a DepGraph corresponds to the information in a cluster. */ -template -void VerifyDepGraphFromCluster(const Cluster& cluster, const DepGraph& depgraph) -{ - // Sanity check the depgraph, which includes a check for correspondence between ancestors and - // descendants, so it suffices to check just ancestors below. - SanityCheck(depgraph); - // Verify transaction count. - assert(cluster.size() == depgraph.TxCount()); - // Verify feerates. - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { - assert(depgraph.FeeRate(i) == cluster[i].first); - } - // Verify ancestors. - for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { - // Start with the transaction having itself as ancestor. - auto ancestors = SetType::Singleton(i); - // Add parents of ancestors to the set of ancestors until it stops changing. - while (true) { - const auto old_ancestors = ancestors; - for (auto ancestor : ancestors) { - ancestors |= cluster[ancestor].second; - } - if (old_ancestors == ancestors) break; - } - // Compare against depgraph. - assert(depgraph.Ancestors(i) == ancestors); - // Some additional sanity tests: - // - Every transaction has itself as ancestor. - assert(ancestors[i]); - // - Every transaction has its direct parents as ancestors. - for (auto parent : cluster[i].second) { - assert(ancestors[parent]); - } - } -} - /** Perform a sanity check on a linearization. */ template void SanityCheck(const DepGraph& depgraph, Span linearization) From fa5ebc99207fb3dc9d052fbd489aa7abbaaf737f Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 8 Oct 2024 10:46:41 +0200 Subject: [PATCH 32/36] ci: Double ctest timeout --- ci/test/03_test_script.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh index 9cc0e8e864..12fb028e0a 100755 --- a/ci/test/03_test_script.sh +++ b/ci/test/03_test_script.sh @@ -146,7 +146,7 @@ if [ "$RUN_CHECK_DEPS" = "true" ]; then fi if [ "$RUN_UNIT_TESTS" = "true" ]; then - DIR_UNIT_TEST_DATA="${DIR_UNIT_TEST_DATA}" LD_LIBRARY_PATH="${DEPENDS_DIR}/${HOST}/lib" CTEST_OUTPUT_ON_FAILURE=ON ctest "${MAKEJOBS}" --timeout $((TEST_RUNNER_TIMEOUT_FACTOR * 30 )) + DIR_UNIT_TEST_DATA="${DIR_UNIT_TEST_DATA}" LD_LIBRARY_PATH="${DEPENDS_DIR}/${HOST}/lib" CTEST_OUTPUT_ON_FAILURE=ON ctest "${MAKEJOBS}" --timeout $(( TEST_RUNNER_TIMEOUT_FACTOR * 60 )) fi if [ "$RUN_UNIT_TESTS_SEQUENTIAL" = "true" ]; then From ca2e4ba352cb0a3b5983c002b09ce15ebbf95177 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Tue, 8 Oct 2024 15:24:35 +0000 Subject: [PATCH 33/36] refactor: include the proper header rather than forward-declaring RemovalReasonToString This was not in its own header when it was added, but now that it is the forward-declare makes no sense. --- src/validationinterface.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index e8ff1d78e3..da2685d771 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -19,8 +20,6 @@ #include #include -std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept; - /** * ValidationSignalsImpl manages a list of shared_ptr callbacks. * From 1786be7b4a56db8f4a0dd13cf3672bf53d1d2a51 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 10 Oct 2024 12:20:34 +0200 Subject: [PATCH 34/36] scripted-diff: drop config/ subdir for bitcoin-config.h, rename to bitcoin-build-config.h Follow-up for PR #30856, commit 0dd66251. -BEGIN VERIFY SCRIPT- sed -i "s|config/bitcoin-config\.h|bitcoin-build-config.h|g" $(git grep -l config/bitcoin-config\.h) sed -i "s|bitcoin-config\.h|bitcoin-build-config.h|g" $(git grep -l "bitcoin-config\.h" ./src ./test ./cmake) git mv ./cmake/bitcoin-config.h.in ./cmake/bitcoin-build-config.h.in -END VERIFY SCRIPT- --- ...n-config.h.in => bitcoin-build-config.h.in} | 0 cmake/introspection.cmake | 2 +- src/CMakeLists.txt | 2 +- src/addrdb.cpp | 2 +- src/addrman.cpp | 2 +- src/bench/wallet_create.cpp | 2 +- src/bench/wallet_ismine.cpp | 2 +- src/bench/wallet_loading.cpp | 2 +- src/bitcoin-cli.cpp | 2 +- src/bitcoin-tx.cpp | 2 +- src/bitcoin-util.cpp | 2 +- src/bitcoin-wallet.cpp | 2 +- src/bitcoind.cpp | 2 +- src/clientversion.cpp | 2 +- src/clientversion.h | 4 ++-- src/common/netif.cpp | 2 +- src/common/run_command.cpp | 2 +- src/common/settings.cpp | 2 +- src/common/system.cpp | 2 +- src/common/system.h | 2 +- src/crypto/sha256.cpp | 2 +- src/httpserver.cpp | 2 +- src/init.cpp | 2 +- src/init/common.cpp | 2 +- src/mapport.cpp | 2 +- src/net.cpp | 2 +- src/netbase.cpp | 2 +- src/node/interfaces.cpp | 2 +- src/node/kernel_notifications.cpp | 2 +- src/node/warnings.cpp | 2 +- src/qt/bitcoin.cpp | 2 +- src/qt/bitcoin.h | 2 +- src/qt/bitcoingui.cpp | 2 +- src/qt/bitcoingui.h | 2 +- src/qt/clientmodel.cpp | 2 +- src/qt/createwalletdialog.cpp | 2 +- src/qt/intro.cpp | 2 +- src/qt/modaloverlay.cpp | 2 +- src/qt/notificator.cpp | 2 +- src/qt/notificator.h | 2 +- src/qt/optionsdialog.cpp | 2 +- src/qt/optionsmodel.cpp | 2 +- src/qt/qrimagewidget.cpp | 2 +- src/qt/receiverequestdialog.cpp | 2 +- src/qt/rpcconsole.cpp | 2 +- src/qt/rpcconsole.h | 2 +- src/qt/sendcoinsdialog.cpp | 2 +- src/qt/signverifymessagedialog.cpp | 2 +- src/qt/splashscreen.cpp | 2 +- src/qt/test/optiontests.cpp | 2 +- src/qt/test/test_main.cpp | 2 +- src/qt/utilitydialog.cpp | 2 +- src/random.cpp | 2 +- src/randomenv.cpp | 2 +- src/rest.cpp | 2 +- src/rpc/external_signer.cpp | 2 +- src/rpc/mining.cpp | 2 +- src/rpc/node.cpp | 2 +- src/rpc/register.h | 2 +- src/rpc/server.cpp | 2 +- src/rpc/util.cpp | 2 +- src/test/system_tests.cpp | 2 +- src/util/check.cpp | 2 +- src/util/fs_helpers.cpp | 2 +- src/util/syserror.cpp | 2 +- src/util/threadnames.cpp | 2 +- src/util/tokenpipe.cpp | 2 +- src/util/trace.h | 2 +- src/validation.cpp | 2 +- src/wallet/init.cpp | 2 +- src/wallet/rpc/addresses.cpp | 2 +- src/wallet/rpc/backup.cpp | 2 +- src/wallet/rpc/wallet.cpp | 2 +- src/wallet/sqlite.cpp | 2 +- src/wallet/test/db_tests.cpp | 2 +- src/wallet/test/fuzz/wallet_bdb_parser.cpp | 2 +- src/wallet/test/util.h | 2 +- src/wallet/wallet.cpp | 2 +- src/wallet/walletdb.cpp | 2 +- src/wallet/wallettool.cpp | 2 +- test/lint/test_runner/src/main.rs | 18 +++++++++--------- 81 files changed, 89 insertions(+), 89 deletions(-) rename cmake/{bitcoin-config.h.in => bitcoin-build-config.h.in} (100%) diff --git a/cmake/bitcoin-config.h.in b/cmake/bitcoin-build-config.h.in similarity index 100% rename from cmake/bitcoin-config.h.in rename to cmake/bitcoin-build-config.h.in diff --git a/cmake/introspection.cmake b/cmake/introspection.cmake index 5435a109d4..29c93869a7 100644 --- a/cmake/introspection.cmake +++ b/cmake/introspection.cmake @@ -6,7 +6,7 @@ include(CheckCXXSourceCompiles) include(CheckCXXSymbolExists) include(CheckIncludeFileCXX) -# The following HAVE_{HEADER}_H variables go to the bitcoin-config.h header. +# The following HAVE_{HEADER}_H variables go to the bitcoin-build-config.h header. check_include_file_cxx(sys/prctl.h HAVE_SYS_PRCTL_H) check_include_file_cxx(sys/resources.h HAVE_SYS_RESOURCES_H) check_include_file_cxx(sys/vmmeter.h HAVE_SYS_VMMETER_H) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index d10638e887..4a86465bba 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -5,7 +5,7 @@ include(GNUInstallDirs) include(AddWindowsResources) -configure_file(${PROJECT_SOURCE_DIR}/cmake/bitcoin-config.h.in config/bitcoin-config.h USE_SOURCE_PERMISSIONS @ONLY) +configure_file(${PROJECT_SOURCE_DIR}/cmake/bitcoin-build-config.h.in bitcoin-build-config.h USE_SOURCE_PERMISSIONS @ONLY) include_directories(${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR}) add_custom_target(generate_build_info diff --git a/src/addrdb.cpp b/src/addrdb.cpp index b89141c88e..4637906441 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -3,7 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include // IWYU pragma: keep +#include // IWYU pragma: keep #include diff --git a/src/addrman.cpp b/src/addrman.cpp index 84d228dc82..358d4fc0a8 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -3,7 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include // IWYU pragma: keep +#include // IWYU pragma: keep #include #include diff --git a/src/bench/wallet_create.cpp b/src/bench/wallet_create.cpp index 43b5b5c91e..3b916d7c39 100644 --- a/src/bench/wallet_create.cpp +++ b/src/bench/wallet_create.cpp @@ -3,7 +3,7 @@ // file COPYING or https://www.opensource.org/licenses/mit-license.php. #include -#include // IWYU pragma: keep +#include // IWYU pragma: keep #include #include #include diff --git a/src/bench/wallet_ismine.cpp b/src/bench/wallet_ismine.cpp index 29e370ce29..5343814ab2 100644 --- a/src/bench/wallet_ismine.cpp +++ b/src/bench/wallet_ismine.cpp @@ -4,7 +4,7 @@ #include #include -#include // IWYU pragma: keep +#include // IWYU pragma: keep #include #include #include