From a76dc401d67a567bb6b7e2a5dfe19647e94bc2e8 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 25 Mar 2022 17:16:04 +0100 Subject: [PATCH] Merge bitcoin/bitcoin#24502: wallet: don't create long chains by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit da2bc865d644f6be748c305556bdd02f02d1b161 [wallet] don't create long chains by default (glozow) Additional functional test for too-long-chains rejected. Pull request description: Default mempool policy doesn't let you have chains longer than 25 transactions. This is locally configurable of course, but it's not really safe to assume that a chain longer than 25 transactions will propagate. Thus, the wallet should probably avoid creating such transactions by default; set `DEFAULT_WALLET_REJECT_LONG_CHAINS` to true. Closes #9752 Closes #10004 ACKs for top commit: MarcoFalke: re-ACK da2bc865d644f6be748c305556bdd02f02d1b161 only change is fixing typos in tests 🎏 Tree-SHA512: 65d8e4ec437fe928adf554aa7e819a52e0599b403d5310895f4e371e99bbc838219b3097c4d2f775bc870ac617ef6b4227b94291f2b376f824f14e8f2b152f31 --- src/wallet/wallet.h | 2 +- test/functional/wallet_balance.py | 4 +++- test/functional/wallet_basic.py | 3 ++- test/functional/wallet_create_tx.py | 26 ++++++++++++++++++++++++++ 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 330fd17d7ef..2150d7a605d 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -98,7 +98,7 @@ static const CAmount WALLET_INCREMENTAL_RELAY_FEE = 5000; //! Default for -spendzeroconfchange static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true; //! Default for -walletrejectlongchains -static const bool DEFAULT_WALLET_REJECT_LONG_CHAINS = false; +static const bool DEFAULT_WALLET_REJECT_LONG_CHAINS{true}; //! -txconfirmtarget default static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 6; //! -walletrbf default diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 1db7c49c2dd..6ba84020cb0 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -53,7 +53,9 @@ def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True self.extra_args = [ - ['-limitdescendantcount=3'], # Limit mempool descendants as a hack to have wallet txs rejected from the mempool + # Limit mempool descendants as a hack to have wallet txs rejected from the mempool. + # Set walletrejectlongchains=0 so the wallet still creates the transactions. + ['-limitdescendantcount=3', '-walletrejectlongchains=0'], [], ] diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index c8cf15b389b..a32872ca125 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -27,6 +27,7 @@ def set_test_params(self): self.extra_args = [[ "-acceptnonstdtxn=1", "-bech32_hrp=bcrt", + "-walletrejectlongchains=0" ]] * self.num_nodes self.setup_clean_chain = True self.supports_cli = False @@ -573,7 +574,7 @@ def run_test(self): self.log.info("Test -reindex") self.stop_nodes() # set lower ancestor limit for later - self.start_node(0, ['-reindex', "-limitancestorcount=" + str(chainlimit)]) + self.start_node(0, ['-reindex', "-walletrejectlongchains=0", "-limitancestorcount=" + str(chainlimit)]) self.start_node(1, ['-reindex', "-limitancestorcount=" + str(chainlimit)]) self.start_node(2, ['-reindex', "-limitancestorcount=" + str(chainlimit)]) # reindex will leave rpc warm up "early"; Wait for it to finish diff --git a/test/functional/wallet_create_tx.py b/test/functional/wallet_create_tx.py index 958d3a4fc37..35bf0498479 100755 --- a/test/functional/wallet_create_tx.py +++ b/test/functional/wallet_create_tx.py @@ -29,6 +29,7 @@ def run_test(self): self.test_anti_fee_sniping() self.test_tx_size_too_large() + self.test_create_too_long_mempool_chain() def test_anti_fee_sniping(self): self.log.info('Check that we have some (old) blocks and that anti-fee-sniping is disabled') @@ -80,6 +81,31 @@ def test_tx_size_too_large(self): ) self.nodes[0].settxfee(0) + def test_create_too_long_mempool_chain(self): + self.log.info('Check too-long mempool chain error') + df_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + + self.nodes[0].createwallet("too_long") + test_wallet = self.nodes[0].get_wallet_rpc("too_long") + + tx_data = df_wallet.send(outputs=[{test_wallet.getnewaddress(): 25}], options={"change_position": 0}) + txid = tx_data['txid'] + vout = 1 + + options = {"change_position": 0, "add_inputs": False} + for i in range(1, 25): + options['inputs'] = [{'txid': txid, 'vout': vout}] + tx_data = test_wallet.send(outputs=[{test_wallet.getnewaddress(): 25 - i}], options=options) + txid = tx_data['txid'] + + options = {"include_unsafe": True, 'add_inputs': True} + # Sending one more chained transaction will fail + assert_raises_rpc_error(-4, "Insufficient funds", + test_wallet.send, outputs=[{test_wallet.getnewaddress(): 0.3}], options=options) + + test_wallet.unloadwallet() + + if __name__ == '__main__': CreateTxWalletTest().main()