From fc62271015e9585bd3a3889adac894c9ef2e2ab2 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Thu, 9 Nov 2023 17:51:20 +0100 Subject: [PATCH 1/7] [refactor] Add helper for iterating through mempool entries Instead of reaching into the mapTx data structure, use a helper method that provides the required vector of CTxMemPoolEntry pointers. Github-Pull: #28391 Rebased-From: 453b4813ebc74859864803e9972b58e4be76a4d6 --- src/kernel/mempool_entry.h | 2 ++ src/node/interfaces.cpp | 2 +- src/rpc/mempool.cpp | 5 ++--- src/txmempool.cpp | 12 ++++++++++++ src/txmempool.h | 1 + 5 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index 1f175a5ccf..edd95e8d53 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -176,4 +176,6 @@ class CTxMemPoolEntry mutable Epoch::Marker m_epoch_marker; //!< epoch when last touched, useful for graph algorithms }; +using CTxMemPoolEntryRef = CTxMemPoolEntry::CTxMemPoolEntryRef; + #endif // BITCOIN_KERNEL_MEMPOOL_ENTRY_H diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index f6dbe4f008..3930280797 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -806,7 +806,7 @@ class ChainImpl : public Chain { if (!m_node.mempool) return; LOCK2(::cs_main, m_node.mempool->cs); - for (const CTxMemPoolEntry& entry : m_node.mempool->mapTx) { + for (const CTxMemPoolEntry& entry : m_node.mempool->entryAll()) { notifications.transactionAddedToMempool(entry.GetSharedTx()); } } diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 136969eb87..e113441993 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -344,14 +344,13 @@ UniValue MempoolToJSON(const CTxMemPool& pool, bool verbose, bool include_mempoo } LOCK(pool.cs); UniValue o(UniValue::VOBJ); - for (const CTxMemPoolEntry& e : pool.mapTx) { - const uint256& hash = e.GetTx().GetHash(); + for (const CTxMemPoolEntry& e : pool.entryAll()) { UniValue info(UniValue::VOBJ); entryToJSON(pool, info, e); // Mempool has unique entries so there is no advantage in using // UniValue::pushKV, which checks if the key already exists in O(N). // UniValue::pushKVEnd is used instead which currently is O(1). - o.pushKVEnd(hash.ToString(), info); + o.pushKVEnd(e.GetTx().GetHash().ToString(), info); } return o; } else { diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 461662ad93..8b744698ba 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -836,6 +836,18 @@ static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), it->GetFee(), it->GetTxSize(), it->GetModifiedFee() - it->GetFee()}; } +std::vector CTxMemPool::entryAll() const +{ + AssertLockHeld(cs); + + std::vector ret; + ret.reserve(mapTx.size()); + for (const auto& it : GetSortedDepthAndScore()) { + ret.emplace_back(*it); + } + return ret; +} + std::vector CTxMemPool::infoAll() const { LOCK(cs); diff --git a/src/txmempool.h b/src/txmempool.h index cbeabb31fa..fd7006ab44 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -695,6 +695,7 @@ class CTxMemPool /** Returns info for a transaction if its entry_sequence < last_sequence */ TxMempoolInfo info_for_relay(const GenTxid& gtxid, uint64_t last_sequence) const; + std::vector entryAll() const EXCLUSIVE_LOCKS_REQUIRED(cs); std::vector infoAll() const; size_t DynamicMemoryUsage() const; From fe0f8fe8aa4a8c4dddf45f4e3519a5ded8c79ad5 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sun, 7 Jan 2024 15:53:08 -0600 Subject: [PATCH 2/7] net: create I2P sessions with both ECIES-X25519 and ElGamal encryption A Bitcoin Core node may only connect to a peer destination via I2P if both sides have sessions with the same encryption type. The encryption type is a property of the session, not the destination. Sessions may support multiple encryption types. As Bitcoin Core is not currently setting the I2P encryption type when creating sessions, it is using the older default, ElGamal (type 0). This pull updates Bitcoin Core to use both ECIES-X25519 and ElGamal (types 4 and 0, respectively). This allows to connect to I2P peers with either type, and the newer, faster ECIES-X25519 will be preferred. See also the recently updated section "Signature and Encryption Types" in https://geti2p.net/en/docs/api/samv3 Thanks and credit to zzzi2p (https://github.com/zzzi2p) for reporting. Closes https://github.com/bitcoin/bitcoin/issues/29197. Github-Pull: #29200 Rebased-From: 9d728916b27e18efc6f8839770ed5ec14789fc08 --- src/i2p.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/i2p.cpp b/src/i2p.cpp index 685b43ba18..c891562d00 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -427,7 +427,7 @@ void Session::CreateIfNotCreatedAlready() const Reply& reply = SendRequestAndGetReply( *sock, strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=TRANSIENT SIGNATURE_TYPE=7 " - "inbound.quantity=1 outbound.quantity=1", + "i2cp.leaseSetEncType=4,0 inbound.quantity=1 outbound.quantity=1", session_id)); m_private_key = DecodeI2PBase64(reply.Get("DESTINATION")); @@ -445,7 +445,7 @@ void Session::CreateIfNotCreatedAlready() SendRequestAndGetReply(*sock, strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=%s " - "inbound.quantity=3 outbound.quantity=3", + "i2cp.leaseSetEncType=4,0 inbound.quantity=3 outbound.quantity=3", session_id, private_key_b64)); } From 7ec34554afca9159096720de36f44707a4c628ce Mon Sep 17 00:00:00 2001 From: glozow Date: Thu, 11 Jan 2024 11:12:40 +0000 Subject: [PATCH 3/7] [log] mempool loading Log at the top before incrementing so that this log isn't printed when there's only 1 tx. Github-Pull: #29227 Rebased-From: eb78ea4eebfe150bc1746282bfdad6eb0f764e3c --- src/kernel/mempool_persist.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp index ff655c5ffa..f49cbe4439 100644 --- a/src/kernel/mempool_persist.cpp +++ b/src/kernel/mempool_persist.cpp @@ -60,10 +60,20 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active if (version != MEMPOOL_DUMP_VERSION) { return false; } - uint64_t num; - file >> num; - while (num) { - --num; + uint64_t total_txns_to_load; + file >> total_txns_to_load; + uint64_t txns_tried = 0; + LogPrintf("Loading %u mempool transactions from disk...\n", total_txns_to_load); + int next_tenth_to_report = 0; + while (txns_tried < total_txns_to_load) { + const int percentage_done(100.0 * txns_tried / total_txns_to_load); + if (next_tenth_to_report < percentage_done / 10) { + LogPrintf("Progress loading mempool transactions from disk: %d%% (tried %u, %u remaining)\n", + percentage_done, txns_tried, total_txns_to_load - txns_tried); + next_tenth_to_report = percentage_done / 10; + } + ++txns_tried; + CTransactionRef tx; int64_t nTime; int64_t nFeeDelta; From 438ac2947dd76f9abd11d73b442656d5c77754cf Mon Sep 17 00:00:00 2001 From: Mark Friedenbach Date: Sat, 4 Nov 2023 12:32:17 -0700 Subject: [PATCH 4/7] snapshots: don't core dump when running -checkblockindex after `loadtxoutset` Github-Pull: #28791 Rebased-From: cdc6ac4126b31426261605a757c52ea2dbfb2a81 --- src/validation.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index a6cab6b095..5d435fef8b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4850,7 +4850,9 @@ void ChainstateManager::CheckBlockIndex() // For testing, allow transaction counts to be completely unset. || (pindex->nChainTx == 0 && pindex->nTx == 0) // For testing, allow this nChainTx to be unset if previous is also unset. - || (pindex->nChainTx == 0 && prev_chain_tx == 0 && pindex->pprev)); + || (pindex->nChainTx == 0 && prev_chain_tx == 0 && pindex->pprev) + // Transaction counts prior to snapshot are unknown. + || pindex->IsAssumedValid()); if (pindexFirstAssumeValid == nullptr && pindex->nStatus & BLOCK_ASSUMED_VALID) pindexFirstAssumeValid = pindex; if (pindexFirstInvalid == nullptr && pindex->nStatus & BLOCK_FAILED_VALID) pindexFirstInvalid = pindex; From ecb8ebc6608c71676f377398b8dd38fc484dc48e Mon Sep 17 00:00:00 2001 From: Gloria Zhao Date: Wed, 3 Jan 2024 16:44:12 +0000 Subject: [PATCH 5/7] [test] rescan legacy wallet with reorged parent + IsFromMe child in mempool Test that wallet rescans process transactions topologically, even if a parent's entry into the mempool is later than that of its child. This behavior is important because IsFromMe requires the ability to look up a transaction's inputs. Github-Pull: #29179 Rebased-From: c3d02be536ac3f35c10efa03653186a17ebbfc12 --- test/functional/wallet_import_rescan.py | 56 +++++++++++++++++++++---- 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 0ac67607e1..7f01d23941 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -20,7 +20,10 @@ """ from test_framework.test_framework import BitcoinTestFramework -from test_framework.address import AddressType +from test_framework.address import ( + AddressType, + ADDRESS_BCRT1_UNSPENDABLE, +) from test_framework.util import ( assert_equal, set_node_times, @@ -109,7 +112,7 @@ def check(self, txid=None, amount=None, confirmation_height=None): address, = [ad for ad in addresses if txid in ad["txids"]] assert_equal(address["address"], self.address["address"]) - assert_equal(address["amount"], self.expected_balance) + assert_equal(address["amount"], self.amount_received) assert_equal(address["confirmations"], confirmations) # Verify the transaction is correctly marked watchonly depending on # whether the transaction pays to an imported public key or @@ -223,11 +226,11 @@ def run_test(self): variant.node = self.nodes[2 + IMPORT_NODES.index(ImportNode(variant.prune, expect_rescan))] variant.do_import(variant.timestamp) if expect_rescan: - variant.expected_balance = variant.initial_amount + variant.amount_received = variant.initial_amount variant.expected_txs = 1 variant.check(variant.initial_txid, variant.initial_amount, variant.confirmation_height) else: - variant.expected_balance = 0 + variant.amount_received = 0 variant.expected_txs = 0 variant.check() @@ -247,7 +250,7 @@ def run_test(self): # Check the latest results from getbalance and listtransactions. for variant in IMPORT_VARIANTS: self.log.info('Run check for variant {}'.format(variant)) - variant.expected_balance += variant.sent_amount + variant.amount_received += variant.sent_amount variant.expected_txs += 1 variant.check(variant.sent_txid, variant.sent_amount, variant.confirmation_height) @@ -267,14 +270,45 @@ def run_test(self): address_type=variant.address_type.value, )) variant.key = self.nodes[1].dumpprivkey(variant.address["address"]) - variant.initial_amount = get_rand_amount() + variant.initial_amount = get_rand_amount() * 2 variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount) variant.confirmation_height = 0 variant.timestamp = timestamp + # Mine a block so these parents are confirmed + assert_equal(len(self.nodes[0].getrawmempool()), len(mempool_variants)) + self.sync_mempools() + block_to_disconnect = self.generate(self.nodes[0], 1)[0] + assert_equal(len(self.nodes[0].getrawmempool()), 0) + + # For each variant, create an unconfirmed child transaction from initial_txid, sending all + # the funds to an unspendable address. Importantly, no change output is created so the + # transaction can't be recognized using its outputs. The wallet rescan needs to know the + # inputs of the transaction to detect it, so the parent must be processed before the child. + # An equivalent test for descriptors exists in wallet_rescan_unconfirmed.py. + unspent_txid_map = {txin["txid"] : txin for txin in self.nodes[1].listunspent()} + for variant in mempool_variants: + # Send full amount, subtracting fee from outputs, to ensure no change is created. + child = self.nodes[1].send( + add_to_wallet=False, + inputs=[unspent_txid_map[variant.initial_txid]], + outputs=[{ADDRESS_BCRT1_UNSPENDABLE : variant.initial_amount}], + subtract_fee_from_outputs=[0] + ) + variant.child_txid = child["txid"] + variant.amount_received = 0 + self.nodes[0].sendrawtransaction(child["hex"]) + + # Mempools should contain the child transactions for each variant. assert_equal(len(self.nodes[0].getrawmempool()), len(mempool_variants)) self.sync_mempools() + # Mock a reorg so the parent transactions are added back to the mempool + for node in self.nodes: + node.invalidateblock(block_to_disconnect) + # Mempools should now contain the parent and child for each variant. + assert_equal(len(node.getrawmempool()), 2 * len(mempool_variants)) + # For each variation of wallet key import, invoke the import RPC and # check the results from getbalance and listtransactions. for variant in mempool_variants: @@ -283,11 +317,15 @@ def run_test(self): variant.node = self.nodes[2 + IMPORT_NODES.index(ImportNode(variant.prune, expect_rescan))] variant.do_import(variant.timestamp) if expect_rescan: - variant.expected_balance = variant.initial_amount + # Ensure both transactions were rescanned. This would raise a JSONRPCError if the + # transactions were not identified as belonging to the wallet. + assert_equal(variant.node.gettransaction(variant.initial_txid)['confirmations'], 0) + assert_equal(variant.node.gettransaction(variant.child_txid)['confirmations'], 0) + variant.amount_received = variant.initial_amount variant.expected_txs = 1 - variant.check(variant.initial_txid, variant.initial_amount) + variant.check(variant.initial_txid, variant.initial_amount, 0) else: - variant.expected_balance = 0 + variant.amount_received = 0 variant.expected_txs = 0 variant.check() From ac1b9a51dbb0ac682ac04e0a2a711091d5e962d8 Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 9 Jan 2024 10:03:20 +0000 Subject: [PATCH 6/7] [test] import descriptor wallet with reorged parent + IsFromMe child in mempool Test that wallet rescans process transactions topologically, even if a parent's entry into the mempool is later than that of its child. This behavior is important because IsFromMe requires the ability to look up a transaction's inputs. Co-authored-by: furszy Github-Pull: #29179 Rebased-From: df30247705940c50c5eaafd74e2abbeb8b0cec07 --- test/functional/test_runner.py | 1 + test/functional/wallet_rescan_unconfirmed.py | 83 ++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100755 test/functional/wallet_rescan_unconfirmed.py diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 148902032f..a642a1ee7d 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -330,6 +330,7 @@ 'wallet_create_tx.py --descriptors', 'wallet_inactive_hdchains.py --legacy-wallet', 'wallet_spend_unconfirmed.py', + 'wallet_rescan_unconfirmed.py --descriptors', 'p2p_fingerprint.py', 'feature_uacomment.py', 'feature_init.py', diff --git a/test/functional/wallet_rescan_unconfirmed.py b/test/functional/wallet_rescan_unconfirmed.py new file mode 100755 index 0000000000..ad9fa081c2 --- /dev/null +++ b/test/functional/wallet_rescan_unconfirmed.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python3 +# Copyright (c) 2024 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test that descriptor wallets rescan mempool transactions properly when importing.""" + +from test_framework.address import ( + address_to_scriptpubkey, + ADDRESS_BCRT1_UNSPENDABLE, +) +from test_framework.messages import COIN +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal +from test_framework.wallet import MiniWallet +from test_framework.wallet_util import test_address + + +class WalletRescanUnconfirmed(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + + def set_test_params(self): + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + self.skip_if_no_sqlite() + + def run_test(self): + self.log.info("Create wallets and mine initial chain") + node = self.nodes[0] + tester_wallet = MiniWallet(node) + + node.createwallet(wallet_name='w0', disable_private_keys=False) + w0 = node.get_wallet_rpc('w0') + + self.log.info("Create a parent tx and mine it in a block that will later be disconnected") + parent_address = w0.getnewaddress() + tx_parent_to_reorg = tester_wallet.send_to( + from_node=node, + scriptPubKey=address_to_scriptpubkey(parent_address), + amount=COIN, + ) + assert tx_parent_to_reorg["txid"] in node.getrawmempool() + block_to_reorg = self.generate(tester_wallet, 1)[0] + assert_equal(len(node.getrawmempool()), 0) + node.syncwithvalidationinterfacequeue() + assert_equal(w0.gettransaction(tx_parent_to_reorg["txid"])["confirmations"], 1) + + # Create an unconfirmed child transaction from the parent tx, sending all + # the funds to an unspendable address. Importantly, no change output is created so the + # transaction can't be recognized using its outputs. The wallet rescan needs to know the + # inputs of the transaction to detect it, so the parent must be processed before the child. + w0_utxos = w0.listunspent() + + self.log.info("Create a child tx and wait for it to propagate to all mempools") + # The only UTXO available to spend is tx_parent_to_reorg. + assert_equal(len(w0_utxos), 1) + assert_equal(w0_utxos[0]["txid"], tx_parent_to_reorg["txid"]) + tx_child_unconfirmed_sweep = w0.sendall([ADDRESS_BCRT1_UNSPENDABLE]) + assert tx_child_unconfirmed_sweep["txid"] in node.getrawmempool() + node.syncwithvalidationinterfacequeue() + + self.log.info("Mock a reorg, causing parent to re-enter mempools after its child") + node.invalidateblock(block_to_reorg) + assert tx_parent_to_reorg["txid"] in node.getrawmempool() + + self.log.info("Import descriptor wallet on another node") + descriptors_to_import = [{"desc": w0.getaddressinfo(parent_address)['parent_desc'], "timestamp": 0, "label": "w0 import"}] + + node.createwallet(wallet_name="w1", disable_private_keys=True) + w1 = node.get_wallet_rpc("w1") + w1.importdescriptors(descriptors_to_import) + + self.log.info("Check that the importing node has properly rescanned mempool transactions") + # Check that parent address is correctly determined as ismine + test_address(w1, parent_address, solvable=True, ismine=True) + # This would raise a JSONRPCError if the transactions were not identified as belonging to the wallet. + assert_equal(w1.gettransaction(tx_parent_to_reorg["txid"])["confirmations"], 0) + assert_equal(w1.gettransaction(tx_child_unconfirmed_sweep["txid"])["confirmations"], 0) + +if __name__ == '__main__': + WalletRescanUnconfirmed().main() From 11f3a7e6baf145360190635f47b1fb371fb38912 Mon Sep 17 00:00:00 2001 From: Mark Friedenbach Date: Wed, 20 Dec 2023 16:24:37 -0800 Subject: [PATCH 7/7] Use hardened runtime on macOS release builds. The Apple notary service requires submitted app bundles to be configured to use the hardened runtime libraries. This is configured at signing time, and supported by the signapple tool Bitcoin Core uses for reproduceable signed binaries. We simply need to pass "--hardened-runtime" when the signature is created. Once attached to the bundle, the resulting codesigned binary can be successfully submitted to the Apple binary notarization service by any Apple Developer. Github-Pull: #29127 Rebased-From: 4fdd836db92e789c98b9e68398ca931a968cc9c3 --- contrib/macdeploy/detached-sig-create.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/macdeploy/detached-sig-create.sh b/contrib/macdeploy/detached-sig-create.sh index 626381cf43..097a7c35ee 100755 --- a/contrib/macdeploy/detached-sig-create.sh +++ b/contrib/macdeploy/detached-sig-create.sh @@ -24,7 +24,7 @@ fi rm -rf ${TEMPDIR} mkdir -p ${TEMPDIR} -${SIGNAPPLE} sign -f --detach "${TEMPDIR}/${OUTROOT}" "$@" "${BUNDLE}" +${SIGNAPPLE} sign -f --detach "${TEMPDIR}/${OUTROOT}" "$@" "${BUNDLE}" --hardened-runtime tar -C "${TEMPDIR}" -czf "${OUT}" . rm -rf "${TEMPDIR}"