From 81f6e8f124561d7a0a0fb501978974981a786766 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Sun, 20 Sep 2026 10:20:51 -0500 Subject: [PATCH 1/2] Merge bitcoin/bitcoin#28139: test: create wallet specific for test_locked_wallet case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit c648bdbda21c7ae90c6b40e506ca4ed62b1dbb6c test: create wallet specific for test_locked_wallet case (furszy) Pull request description: Coming from https://github.com/bitcoin/bitcoin/pull/28089#discussion_r1265478128. Several test cases are relying on the node1 default wallet, which thanks to 'test_locked_wallet' is encrypted. And can be only accessed within a specific timeframe (100ms), a duration internally set by the same test. This situation introduces a potential race condition, where other tests must complete their operations within the specified 100ms window to pass (otherwise the wallet gets re-locked and they fail). This can be seen running the test in valgrind (https://github.com/bitcoin/bitcoin/pull/28089), where other test cases fail due the wallet re-locking itself after the 100ms. ACKs for top commit: MarcoFalke: lgtm ACK c648bdbda21c7ae90c6b40e506ca4ed62b1dbb6c ishaanam: utACK c648bdbda21c7ae90c6b40e506ca4ed62b1dbb6c Tree-SHA512: 01cde5a4a0cb3405adb9ea3c1f73841f3fa237d1162268ed06f0d49ca38541006b423a029e0b5e5955e1aa7e018c4600d894e555a68cf17ff60a4b8be58f4aa9 Dash adaptations: - test/functional/wallet_fundrawtransaction.py: kept Dash's `pkh(...)` descriptors instead of upstream's `wpkh(...)` (Dash has no segwit) - test/functional/wallet_fundrawtransaction.py: kept Dash's `walletpassphrase(..., 999000)` timeouts instead of upstream's 10/100/600 — Dash already carries the later bitcoin#28403 which bumped these, so restoring the small values would regress it - test/functional/wallet_fundrawtransaction.py: kept Dash's amounts (sendtoaddress 12, output 11, balance delta 511.0000000) rather than upstream's 1.2/1.1/51.10000000 — Dash's block subsidy and this test's amounts are 10x - test/functional/wallet_fundrawtransaction.py: did not add upstream's `wallet.getrawchangeaddress()` (keypool drain) or `wallet.keypoolrefill(8)` lines — those lines do not exist in Dash's copy of this test (they were never introduced in Dash history); only the self.nodes[1]->wallet rename part of those upstream hunks applies - test/functional/wallet_fundrawtransaction.py: changeless-tx fee deduction is `0.00000500` instead of upstream's `0.00002200`. Upstream's constant is exactly the fee of a 110-vB segwit tx at Bitcoin's test fallbackfee of 0.0002/kB; Dash's test fallbackfee/min relay fee is 0.00001/kB (1 duff/byte) and the tx is a legacy 1-in/1-out ~192 bytes, so the required fee is ~192 duffs. With 2200 deducted the 2008-duff remainder exceeds min_viable_change (discard fee 10000/kB * 148 B + 1 = 1481 duffs), so the wallet would demand a change output and `fundrawtransaction` would fail with -4 on the locked wallet instead of returning changepos -1. 500 duffs covers the fee and leaves 308 < 1481, giving the changeless tx upstream intends. Dash's old 2200 (with its stale 'bnb doesn't work same way as in bitcoin' comment) was sized for the previous multi-input selection and no longer applies now that the wallet holds exactly one UTXO; replaced by upstream's comments plus a one-line note about the value. --- test/functional/wallet_fundrawtransaction.py | 62 +++++++++++++------- 1 file changed, 40 insertions(+), 22 deletions(-) diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index 83fb7d67e730..2a907c847004 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -543,11 +543,20 @@ def test_spend_2of2(self): def test_locked_wallet(self): self.log.info("Test fundrawtxn with locked wallet and hardened derivation") - self.nodes[1].encryptwallet("test") + 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") + + # Add some balance to the wallet (this will be reverted at the end of the test) + df_wallet.sendall(recipients=[wallet.getnewaddress()]) + self.generate(self.nodes[1], 1) + + # Encrypt wallet and import descriptors + wallet.encryptwallet("test") if self.options.descriptors: - self.nodes[1].walletpassphrase('test', 999000) - self.nodes[1].importdescriptors([{ + wallet.walletpassphrase('test', 999000) + wallet.importdescriptors([{ 'desc': descsum_create('pkh(tprv8ZgxMBicQKsPdYeeZbPSKd2KYLmeVKtcFA7kqCxDvDR13MQ6us8HopUR2wLcS2ZKPhLyKsqpDL2FtL73LMHcgoCL7DXsciA8eX8nbjCR2eG/0h/*h)'), 'timestamp': 'now', 'active': True @@ -558,48 +567,57 @@ def test_locked_wallet(self): 'active': True, 'internal': True }]) - self.nodes[1].walletlock() + wallet.walletlock() # Drain the keypool. - self.nodes[1].getnewaddress() - - # Choose 2 inputs - # bnb doesn't work same way as in bitcoin, so, `value` is also calculated by different way - inputs = self.nodes[1].listunspent() - value = sum(inp["amount"] for inp in inputs) - Decimal("0.00002200") + wallet.getnewaddress() + + # 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") outputs = {self.nodes[0].getnewaddress():value} - rawtx = self.nodes[1].createrawtransaction(inputs, outputs) + rawtx = wallet.createrawtransaction(inputs, outputs) # fund a transaction that does not require a new key for the change output - self.nodes[1].fundrawtransaction(rawtx) + funded_tx = wallet.fundrawtransaction(rawtx) + assert_equal(funded_tx["changepos"], -1) # fund a transaction that requires a new key for the change output # creating the key must be impossible because the wallet is locked outputs = {self.nodes[0].getnewaddress():value - Decimal("0.1")} - rawtx = self.nodes[1].createrawtransaction(inputs, outputs) - assert_raises_rpc_error(-4, "Transaction needs a change address, but we can't generate it.", self.nodes[1].fundrawtransaction, rawtx) + rawtx = wallet.createrawtransaction(inputs, outputs) + assert_raises_rpc_error(-4, "Transaction needs a change address, but we can't generate it.", wallet.fundrawtransaction, rawtx) # Refill the keypool. - self.nodes[1].walletpassphrase("test", 999000) - self.nodes[1].walletlock() + wallet.walletpassphrase("test", 999000) + wallet.walletlock() - assert_raises_rpc_error(-13, "walletpassphrase", self.nodes[1].sendtoaddress, self.nodes[0].getnewaddress(), 12) + assert_raises_rpc_error(-13, "walletpassphrase", wallet.sendtoaddress, self.nodes[0].getnewaddress(), 12) oldBalance = self.nodes[0].getbalance() inputs = [] outputs = {self.nodes[0].getnewaddress():11} - rawtx = self.nodes[1].createrawtransaction(inputs, outputs) - fundedTx = self.nodes[1].fundrawtransaction(rawtx) + rawtx = wallet.createrawtransaction(inputs, outputs) + fundedTx = wallet.fundrawtransaction(rawtx) + assert fundedTx["changepos"] != -1 # Now we need to unlock. - self.nodes[1].walletpassphrase("test", 999000) - signedTx = self.nodes[1].signrawtransactionwithwallet(fundedTx['hex']) - self.nodes[1].sendrawtransaction(signedTx['hex']) + wallet.walletpassphrase("test", 999000) + signedTx = wallet.signrawtransactionwithwallet(fundedTx['hex']) + wallet.sendrawtransaction(signedTx['hex']) self.generate(self.nodes[1], 1) # Make sure funds are received at node1. assert_equal(oldBalance+Decimal('511.0000000'), self.nodes[0].getbalance()) + # Restore pre-test wallet state + wallet.sendall(recipients=[df_wallet.getnewaddress(), df_wallet.getnewaddress(), df_wallet.getnewaddress()]) + wallet.unloadwallet() + self.generate(self.nodes[1], 1) + def test_many_inputs_fee(self): """Multiple (~19) inputs tx test | Compare fee.""" self.log.info("Test fundrawtxn fee with many inputs") From 7a37ac85d26a40f257bf87aeecc2b06f85ebb46a Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Sun, 20 Sep 2026 10:24:54 -0500 Subject: [PATCH 2/2] Merge bitcoin/bitcoin#28232: test: locked_wallet, skip default fee estimation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: import conflict resolved by keeping Dash's existing `satoshi_round` (still used at the outputs-value assertion in test_change_position) alongside upstream's new `get_fee`, in alphabetical order - test/functional/wallet_fundrawtransaction.py: tx_size changed from upstream's 110 (p2wpkh->p2wpkh) to 192 (p2pkh->p2pkh) since Dash has no segwit — 4 version + 1 vin count + 148 input (36 prevout + 1 + 107 scriptSig + 4 sequence) + 1 vout count + 34 output + 4 locktime; 107-byte scriptSig follows from DUMMY_MAXIMUM_SIGNATURE_CREATOR(33,32) -> 72-byte DER sig plus a 33-byte compressed pubkey - test/functional/wallet_fundrawtransaction.py: dropped the Dash-only comment explaining the hardcoded 0.00000500 constant, since upstream replaces that constant with an exact get_fee() computation and the comment no longer describes the code --- 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 2a907c847004..6e6d786727cd 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 @@ -546,6 +547,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()]) @@ -574,10 +577,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 = 192 # Total tx size: 192 bytes, p2pkh -> p2pkh. Input 148 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