From 2d7b1557844663913550b307658181bb0371602e Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Sat, 17 Oct 2026 22:36:29 -0500 Subject: [PATCH 1/4] Merge bitcoin/bitcoin#27800: streams: Drop confusing DataStream::Serialize method and << operator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 5cd0717a54ce7a2065b29d90717aa2eb1c5e302d streams: Drop confusing DataStream::Serialize method and << operator (Ryan Ofsky) Pull request description: DataStream Serialize method has surprising behavior because it just serializes raw bytes without a length prefix. When you serialize a string or vector, a length prefix is serialized before the raw object contents so the object can be unambiguously deserialized later. But DataStreams don't support deserializing at all and just dump the raw bytes. Having this inconsistency is not necessary and could be confusing (see https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1212315030) so this PR just drops the DataStream::Serialize method. ACKs for top commit: furszy: lgtm ACK 5cd0717a MarcoFalke: lgtm ACK 5cd0717a54ce7a2065b29d90717aa2eb1c5e302d 🌙 Tree-SHA512: 49dd117de266f091a5336b13a91c5d8658abe1b3a0a9c51c8b5f6a2e0e814781b73afc39256353e79dade603a8a2761e8536716d1a48499720c266f4500477e2 Dash adaptations: - src/net_processing.cpp: final-alert hunk not applied; Dash's ProcessMessage has no legacy final-alert block, so there is nothing to convert - src/flat-database.h: Dash-only `fileout << ssObj` becomes `fileout << Span{ssObj}` since DataStream::Serialize is removed - src/llmq/net_quorum.cpp: QDATA error reply streams `Span{body}` and passes `Span{ssResponse}` to Make(), as upstream did for the alert message - src/governance/net_governance.cpp: MNGOVERNANCEOBJECT/MNGOVERNANCEOBJECTVOTE replies pass `Span{ss}` to Make() instead of the CDataStream itself --- src/flat-database.h | 2 +- src/governance/net_governance.cpp | 4 ++-- src/llmq/dkgsessionmgr.cpp | 2 +- src/llmq/net_quorum.cpp | 4 ++-- src/llmq/quorums.cpp | 2 +- src/llmq/quorumsman.cpp | 2 +- src/streams.h | 8 -------- src/wallet/dump.cpp | 6 +----- src/wallet/test/util.cpp | 2 +- src/wallet/wallet.cpp | 4 +--- 10 files changed, 11 insertions(+), 25 deletions(-) diff --git a/src/flat-database.h b/src/flat-database.h index a55231900545..2c484e3c37e7 100644 --- a/src/flat-database.h +++ b/src/flat-database.h @@ -59,7 +59,7 @@ class CFlatDB // Write and commit header, data try { - fileout << ssObj; + fileout << Span{ssObj}; } catch (std::exception &e) { LogError("%s: Serialize or I/O error - %s\n", __func__, e.what()); diff --git a/src/governance/net_governance.cpp b/src/governance/net_governance.cpp index 2e8b5da79140..56d193d9ad62 100644 --- a/src/governance/net_governance.cpp +++ b/src/governance/net_governance.cpp @@ -288,7 +288,7 @@ bool NetGovernance::ProcessGetData(CNode& pfrom, const CInv& inv, const CNetMsgM CDataStream ss(SER_NETWORK, pfrom.GetCommonVersion()); ss.reserve(1000); if (!m_gov_manager.SerializeObjectForHash(inv.hash, ss)) return false; - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCEOBJECT, ss)); + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCEOBJECT, Span{ss})); return true; } if (inv.type == MSG_GOVERNANCE_OBJECT_VOTE) { @@ -296,7 +296,7 @@ bool NetGovernance::ProcessGetData(CNode& pfrom, const CInv& inv, const CNetMsgM CDataStream ss(SER_NETWORK, pfrom.GetCommonVersion()); ss.reserve(1000); if (!m_gov_manager.SerializeVoteForHash(inv.hash, ss)) return false; - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCEOBJECTVOTE, ss)); + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCEOBJECTVOTE, Span{ss})); return true; } return false; diff --git a/src/llmq/dkgsessionmgr.cpp b/src/llmq/dkgsessionmgr.cpp index 0749ff23395c..9a8e5705a527 100644 --- a/src/llmq/dkgsessionmgr.cpp +++ b/src/llmq/dkgsessionmgr.cpp @@ -129,7 +129,7 @@ void CDKGSessionManager::WriteVerifiedVvecContribution(Consensus::LLMQType llmqT for (auto& pubkey : *vvec) { s << CBLSPublicKeyVersionWrapper(pubkey, false); } - db->Write(std::make_tuple(DB_VVEC, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash), s); + db->Write(std::make_tuple(DB_VVEC, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash), Span{s}); } void CDKGSessionManager::WriteVerifiedSkContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSSecretKey& skContribution) diff --git a/src/llmq/net_quorum.cpp b/src/llmq/net_quorum.cpp index 0f26ae924fc9..193e959f91f1 100644 --- a/src/llmq/net_quorum.cpp +++ b/src/llmq/net_quorum.cpp @@ -113,8 +113,8 @@ void NetQuorum::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataS } request.SetError(nError); CDataStream ssResponse{SER_NETWORK, pfrom.GetCommonVersion()}; - ssResponse << request << body; - m_connman.PushMessage(&pfrom, CNetMsgMaker(pfrom.GetCommonVersion()).Make(NetMsgType::QDATA, ssResponse)); + ssResponse << request << Span{body}; + m_connman.PushMessage(&pfrom, CNetMsgMaker(pfrom.GetCommonVersion()).Make(NetMsgType::QDATA, Span{ssResponse})); return misbehave; }; diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index 69b2100bd778..4d804c115d8f 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -200,7 +200,7 @@ void CQuorum::WriteContributions(CDBWrapper& db) const for (auto& pubkey : *quorumVvec) { s << CBLSPublicKeyVersionWrapper(pubkey, false); } - db.Write(std::make_pair(DB_QUORUM_QUORUM_VVEC, dbKey), s); + db.Write(std::make_pair(DB_QUORUM_QUORUM_VVEC, dbKey), Span{s}); } if (skShare.IsValid()) { db.Write(std::make_pair(DB_QUORUM_SK_SHARE, dbKey), skShare); diff --git a/src/llmq/quorumsman.cpp b/src/llmq/quorumsman.cpp index f494c9673249..6a0f13d67b52 100644 --- a/src/llmq/quorumsman.cpp +++ b/src/llmq/quorumsman.cpp @@ -676,7 +676,7 @@ void CQuorumManager::MigrateOldQuorumDB(CEvoDB& evoDb) const if (!evoDb.GetRawDB().ReadDataStream(k, s)) { break; } - batch.Write(k, s); + batch.Write(k, Span{s}); } if (prefix == DB_QUORUM_SK_SHARE) { if (!pcursor->GetValue(sk)) { diff --git a/src/streams.h b/src/streams.h index b1d261b29935..87b16ff996ca 100644 --- a/src/streams.h +++ b/src/streams.h @@ -316,14 +316,6 @@ class DataStream vch.insert(vch.end(), src.begin(), src.end()); } - template - void Serialize(Stream& s) const - { - // Special case: stream << stream concatenates like stream += stream - if (!vch.empty()) - s.write(MakeByteSpan(vch)); - } - template DataStream& operator<<(const T& obj) { diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index 3344cfd0be7e..b55b64859c09 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -252,11 +252,7 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs:: std::vector k = ParseHex(key); std::vector v = ParseHex(value); - - DataStream ss_key{k}; - DataStream ss_value{v}; - - if (!batch->Write(ss_key, ss_value)) { + if (!batch->Write(Span{k}, Span{v})) { error = strprintf(_("Error: Unable to write record to new wallet")); ret = false; break; diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index 84382cb59efa..4f4dc8c11fe5 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -75,7 +75,7 @@ std::unique_ptr DuplicateMockDatabase(WalletDatabase& database, DatabaseCursor::Status status = cursor->Next(key, value); assert(status != DatabaseCursor::Status::FAIL); if (status == DatabaseCursor::Status::DONE) break; - new_batch->Write(key, value); + new_batch->Write(Span{key}, Span{value}); } return new_database; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 467bb4ba432c..27c6fd97a586 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4735,9 +4735,7 @@ bool CWallet::MigrateToSQLite(bilingual_str& error) bool began = batch->TxnBegin(); assert(began); // This is a critical error, the new db could not be written to. The original db exists as a backup, but we should not continue execution. for (const auto& [key, value] : records) { - DataStream ss_key{key}; - DataStream ss_value{value}; - if (!batch->Write(ss_key, ss_value)) { + if (!batch->Write(MakeUCharSpan(key), MakeUCharSpan(value))) { batch->TxnAbort(); m_database->Close(); fs::remove(m_database->Filename()); From a579befd06f70089dc62921acfdc74a1824a93d2 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Sat, 17 Oct 2026 22:38:32 -0500 Subject: [PATCH 2/4] Merge bitcoin/bitcoin#28125: wallet: bugfix, disallow migration of invalid scripts 8e7e3e614955e60d3bf9e9a481ef8916bf9e22d9 test: wallet, verify migration doesn't crash for an invalid script (furszy) 1de8a2372ab39386e689b27d15c4d029be239319 wallet: disallow migration of invalid or not-watched scripts (furszy) Pull request description: Fixing #28057. The legacy wallet allows to import any raw script (#28126), without checking if it was valid or not. Appending it to the watch-only set. This causes a crash in the migration process because we are only expecting to find valid scripts inside the legacy spkm. These stored scripts internally map to `ISMINE_NO` (same as if they weren't stored at all..). So we need to check for these special case, and take into account that the legacy spkm could be storing invalid not watched scripts. Which, in code words, means `IsMineInner()` returning `IsMineResult::INVALID` for them. Note: To verify this, can run the test commit on top of master. `wallet_migration.py` will crash without the bugfix commit. ACKs for top commit: achow101: ACK 8e7e3e614955e60d3bf9e9a481ef8916bf9e22d9 Tree-SHA512: c2070e8ba78037a8f573b05bf6caa672803188f05429adf5b93f9fc1493faedadecdf018dee9ead27c656710558c849c5da8ca5f6f3bc9c23b3c4275d2fb50c7 Dash adaptations: - test/functional/wallet_migration.py: new sh(sh(pkh())) import and asserts go into Dash's existing test_migrate_raw_p2sh, which has no wsh step and expects balance 2, not 5 - test/functional/wallet_migration.py: script_to_p2sh import added next to Dash's existing key_to_p2pkh import on the same line --- src/wallet/scriptpubkeyman.cpp | 17 +++++++++++++++- src/wallet/scriptpubkeyman.h | 6 ++++++ src/wallet/wallet.cpp | 14 +++++++++++++ test/functional/wallet_migration.py | 31 ++++++++++++++++++++++++++++- 4 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 9817d69eda55..a5b6dd8ccbd0 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1873,8 +1873,23 @@ std::unordered_set LegacyScriptPubKeyMan::GetScriptPub } // All watchonly scripts are raw - spks.insert(setWatchOnly.begin(), setWatchOnly.end()); + for (const CScript& script : setWatchOnly) { + // As the legacy wallet allowed to import any script, we need to verify the validity here. + // LegacyScriptPubKeyMan::IsMine() return 'ISMINE_NO' for invalid or not watched scripts (IsMineResult::INVALID or IsMineResult::NO). + // e.g. a "sh(sh(pkh()))" which legacy wallets allowed to import!. + if (IsMine(script) != ISMINE_NO) spks.insert(script); + } + + return spks; +} +std::unordered_set LegacyScriptPubKeyMan::GetNotMineScriptPubKeys() const +{ + LOCK(cs_KeyStore); + std::unordered_set spks; + for (const CScript& script : setWatchOnly) { + if (IsMine(script) == ISMINE_NO) spks.insert(script); + } return spks; } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 11ad0a558b10..04e78b5e7292 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -535,6 +535,12 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv std::set GetKeys() const override; std::unordered_set GetScriptPubKeys() const override; + /** + * Retrieves scripts that were imported by bugs into the legacy spkm and are + * simply invalid, such as a sh(sh(pkh())) script, or not watched. + */ + std::unordered_set GetNotMineScriptPubKeys() const; + /** Get the DescriptorScriptPubKeyMans (with private keys) that have the same scriptPubKeys as this LegacyScriptPubKeyMan. * Does not modify this ScriptPubKeyMan. */ std::optional MigrateToDescriptor(); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 27c6fd97a586..d6265a49b881 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4772,6 +4772,13 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) return false; } + // Get all invalid or non-watched scripts that will not be migrated + std::set not_migrated_dests; + for (const auto& script : legacy_spkm->GetNotMineScriptPubKeys()) { + CTxDestination dest; + if (ExtractDestination(script, dest)) not_migrated_dests.emplace(dest); + } + for (auto& desc_spkm : data.desc_spkms) { if (m_spk_managers.count(desc_spkm->GetID()) > 0) { error = _("Error: Duplicate descriptors created during migration. Your wallet may be corrupted."); @@ -4896,6 +4903,13 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) continue; } } + + // Skip invalid/non-watched scripts that will not be migrated + if (not_migrated_dests.count(addr_pair.first) > 0) { + dests_to_delete.push_back(addr_pair.first); + continue; + } + // Not ours, not in watchonly wallet, and not in solvable error = _("Error: Address book data in wallet cannot be identified to belong to migrated wallets"); return false; diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 8def79c7ac0c..33c8d27cf2e4 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -7,7 +7,7 @@ import os import random import shutil -from test_framework.address import key_to_p2pkh +from test_framework.address import key_to_p2pkh, script_to_p2sh from test_framework.descriptors import descsum_create from test_framework.key import ECPubKey from test_framework.test_framework import BitcoinTestFramework @@ -620,6 +620,21 @@ def send_to_script(script, amount): wallet.rpc.importaddress(address=script_pkh.hex(), label="raw_spk", rescan=True, p2sh=True) assert_equal(wallet.getbalances()['watchonly']['trusted'], 2) + # Import sh(pkh()) script, by using importaddress(), with the p2sh flag enabled. + # This will wrap the script under another sh level, which is invalid!, and store it inside the wallet. + # The migration process must skip the invalid scripts and the addressbook records linked to them. + # They are not being watched by the current wallet, nor should be watched by the migrated one. + label_sh_pkh = "raw_sh_pkh" + script_pkh = key_to_p2pkh_script(df_wallet.getaddressinfo(df_wallet.getnewaddress())["pubkey"]) + script_sh_pkh = script_to_p2sh_script(script_pkh) + addy_script_sh_pkh = script_to_p2sh(script_pkh) # valid script address + addy_script_double_sh_pkh = script_to_p2sh(script_sh_pkh) # invalid script address + + # Note: 'importaddress()' will add two scripts, a valid one sh(pkh()) and an invalid one 'sh(sh(pkh()))'. + # Both of them will be stored with the same addressbook label. And only the latter one should + # be discarded during migration. The first one must be migrated. + wallet.rpc.importaddress(address=script_sh_pkh.hex(), label=label_sh_pkh, rescan=False, p2sh=True) + # Migrate wallet and re-check balance info_migration = self.migrate_wallet(wallet) wallet_wo = self.nodes[0].get_wallet_rpc(info_migration["watchonly_name"]) @@ -629,6 +644,20 @@ def send_to_script(script, amount): # The watch-only scripts are no longer part of the main wallet assert_equal(wallet.getbalances()['mine']['trusted'], 0) + # The invalid sh(sh(pk())) script label must not be part of the main wallet anymore + assert label_sh_pkh not in wallet.listlabels() + # But, the standard sh(pkh()) script should be part of the watch-only wallet. + addrs_by_label = wallet_wo.getaddressesbylabel(label_sh_pkh) + assert addy_script_sh_pkh in addrs_by_label + assert addy_script_double_sh_pkh not in addrs_by_label + + # Also, the watch-only wallet should have the descriptor for the standard sh(pkh()) + desc = descsum_create(f"addr({addy_script_sh_pkh})") + assert next(it['desc'] for it in wallet_wo.listdescriptors()['descriptors'] if it['desc'] == desc) + # And doesn't have a descriptor for the invalid one + desc_invalid = descsum_create(f"addr({addy_script_double_sh_pkh})") + assert_equal(next((it['desc'] for it in wallet_wo.listdescriptors()['descriptors'] if it['desc'] == desc_invalid), None), None) + # Just in case, also verify wallet restart self.nodes[0].unloadwallet(info_migration["watchonly_name"]) self.nodes[0].loadwallet(info_migration["watchonly_name"]) From 3e2077c3808617917cc3700f48de9d0935afb386 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Sat, 17 Oct 2026 22:39:36 -0500 Subject: [PATCH 3/4] Merge bitcoin/bitcoin#28232: test: locked_wallet, skip default fee estimation 5364dd8666ae1afa35536f9b4fc0170eeaf37888 test: locked_wallet, skip default fee estimation (furszy) Pull request description: Coming from https://github.com/bitcoin/bitcoin/pull/28139#discussion_r1284563239. No test case in this file is meant to exercise fee estimation. All default wallets have a custom tx fee set [here](https://github.com/bitcoin/bitcoin/blob/b7138252ace6d21476964774e094ed1143cd7a1c/test/functional/wallet_fundrawtransaction.py#L100). The only one missing is the one created for `locked_wallet`. ACKs for top commit: theStack: ACK 5364dd8666ae1afa35536f9b4fc0170eeaf37888 Tree-SHA512: 514c02708081d18330d759d10e306cee16c6350de243c68f0973777d2582f5d81968a237393c1f59aba245297e03f3f98d3ae5249a042469d0d016255f568719 Dash adaptations: - test/functional/wallet_fundrawtransaction.py: keeps the Dash-only `satoshi_round` import next to the new `get_fee` import (still used at line 208). - test/functional/wallet_fundrawtransaction.py: the exact-fee size is 191 bytes for Dash's p2pkh->p2pkh tx (147-byte input + 44 bytes), not upstream's 110 vbytes for p2wpkh; Dash has no segwit. - test/functional/wallet_fundrawtransaction.py: replaces Dash's earlier fixed 500-duff fee deduction (and its comment) with upstream's exact get_fee() calculation. --- test/functional/wallet_fundrawtransaction.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index 5686c015b25b..5afdf33a5153 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -20,6 +20,7 @@ assert_raises_rpc_error, count_bytes, find_vout_for_address, + get_fee, satoshi_round, ) from test_framework.wallet_util import bytes_to_wif @@ -550,6 +551,8 @@ def test_locked_wallet(self): df_wallet = self.nodes[1].get_wallet_rpc(self.default_wallet_name) self.nodes[1].createwallet(wallet_name="locked_wallet", descriptors=self.options.descriptors) wallet = self.nodes[1].get_wallet_rpc("locked_wallet") + # This test is not meant to exercise fee estimation. Making sure all txs are sent at a consistent fee rate. + wallet.settxfee(self.min_relay_tx_fee) # Add some balance to the wallet (this will be reverted at the end of the test) df_wallet.sendall(recipients=[wallet.getnewaddress()]) @@ -578,10 +581,11 @@ def test_locked_wallet(self): # Choose input inputs = wallet.listunspent() - # Deduce fee to produce a changeless transaction - # (500 duffs is above the fee of this 1-in/1-out tx at the test feerate, - # while the remainder stays below the cost of creating a change output) - value = inputs[0]["amount"] - Decimal("0.00000500") + + # Deduce exact fee to produce a changeless transaction + tx_size = 191 # Total tx size: 191 bytes, p2pkh -> p2pkh. Input 147 bytes + rest of tx is 44 bytes. + value = inputs[0]["amount"] - get_fee(tx_size, self.min_relay_tx_fee) + outputs = {self.nodes[0].getnewaddress():value} rawtx = wallet.createrawtransaction(inputs, outputs) # fund a transaction that does not require a new key for the change output From 8cfc3c4522cac08214eacc6255d9025f6663d2fd Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Sat, 17 Oct 2026 22:42:10 -0500 Subject: [PATCH 4/4] Merge bitcoin/bitcoin#27695: test: Add test to check tx in the last block can be downloaded fa4c16b186a34f4172deda617166813c8cb92c59 test: Add test to check tx in the last block can be downloaded (MarcoFalke) fadc8490ab4fdd91aaf446e08108657b304a3ded test: Split up test_notfound_on_unannounced_tx test case (MarcoFalke) Pull request description: If a peer received an `inv` about a transaction, which was included in a block before receiving the corresponding `getdata`, it can be beneficial to send this transaction to the peer to aid compact block relay. Add a test for this to avoid breaking it in the future. ACKs for top commit: sdaftuar: ACK fa4c16b186a34f4172deda617166813c8cb92c59 instagibbs: ACK fa4c16b186a34f4172deda617166813c8cb92c59 Tree-SHA512: 1ec16dcc216dd29c849928e753158d45c409612e9ac528db16e5af465bb5b69cd42241ac6f67e5a4583b8e558eeba49fa0a0889a4247b3fb0053b2758eec9490 Dash adaptations: - test/functional/p2p_leak_tx.py: uses send_self_transfer()["txid"] instead of ["wtxid"]; Dash has no segwit so MiniWallet returns no wtxid - test/functional/p2p_leak_tx.py: test_tx_in_block checks tx.rehash() == txid instead of tx.getwtxid() == wtxid, since Dash's CTransaction has no getwtxid() - test/functional/p2p_leak_tx.py: test_tx_in_block calls self.bump_mocktime(60) before waiting for the inv; Dash's framework freezes mocktime, so the inbound inv timer would never fire --- test/functional/p2p_leak_tx.py | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/test/functional/p2p_leak_tx.py b/test/functional/p2p_leak_tx.py index 2ebfec9c8b86..69b6246c30c2 100755 --- a/test/functional/p2p_leak_tx.py +++ b/test/functional/p2p_leak_tx.py @@ -2,7 +2,7 @@ # Copyright (c) 2017-2020 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 we don't leak txs to inbound peers that we haven't yet announced to""" +"""Test transaction upload""" from test_framework.messages import msg_getdata, CInv, MSG_TX from test_framework.p2p import p2p_lock, P2PDataStore @@ -23,16 +23,38 @@ def set_test_params(self): self.num_nodes = 1 def run_test(self): - gen_node = self.nodes[0] # The block and tx generating node - miniwallet = MiniWallet(gen_node) + self.gen_node = self.nodes[0] # The block and tx generating node + self.miniwallet = MiniWallet(self.gen_node) - inbound_peer = self.nodes[0].add_p2p_connection(P2PNode()) # An "attacking" inbound peer + self.test_tx_in_block() + self.test_notfound_on_unannounced_tx() + + def test_tx_in_block(self): + self.log.info("Check that a transaction in the last block is uploaded (beneficial for compact block relay)") + inbound_peer = self.gen_node.add_p2p_connection(P2PNode()) + + self.log.debug("Generate transaction and block") + inbound_peer.last_message.pop("inv", None) + txid = self.miniwallet.send_self_transfer(from_node=self.gen_node)["txid"] + self.bump_mocktime(60) + inbound_peer.wait_until(lambda: "inv" in inbound_peer.last_message and inbound_peer.last_message.get("inv").inv[0].hash == int(txid, 16)) + want_tx = msg_getdata(inv=inbound_peer.last_message.get("inv").inv) + self.generate(self.gen_node, 1) + + self.log.debug("Request transaction") + inbound_peer.last_message.pop("tx", None) + inbound_peer.send_and_ping(want_tx) + assert_equal(inbound_peer.last_message.get("tx").tx.rehash(), txid) + + def test_notfound_on_unannounced_tx(self): + self.log.info("Check that we don't leak txs to inbound peers that we haven't yet announced to") + inbound_peer = self.gen_node.add_p2p_connection(P2PNode()) # An "attacking" inbound peer MAX_REPEATS = 100 self.log.info("Running test up to {} times.".format(MAX_REPEATS)) for i in range(MAX_REPEATS): self.log.info('Run repeat {}'.format(i + 1)) - txid = miniwallet.send_self_transfer(from_node=gen_node)['txid'] + txid = self.miniwallet.send_self_transfer(from_node=self.gen_node)["txid"] want_tx = msg_getdata() want_tx.inv.append(CInv(t=MSG_TX, h=int(txid, 16)))