diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index decdbc70906..6e1552abd7f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1967,29 +1967,58 @@ OutputType CWallet::TransactionChangeType(const std::optional& chang return *change_type; } - // if m_default_address_type is legacy, use legacy address as change (even - // if some of the outputs are P2WPKH or P2WSH). + // if m_default_address_type is legacy, use legacy address as change. if (m_default_address_type == OutputType::LEGACY) { return OutputType::LEGACY; } - // if any destination is P2WPKH or P2WSH, use P2WPKH for the change - // output. + bool any_tr{false}; + bool any_wpkh{false}; + bool any_sh{false}; + bool any_pkh{false}; + for (const auto& recipient : vecSend) { - // Check if any destination contains a witness program: - int witnessversion = 0; - std::vector witnessprogram; - if (recipient.scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) { - if (GetScriptPubKeyMan(OutputType::BECH32M, true)) { - return OutputType::BECH32M; - } else if (GetScriptPubKeyMan(OutputType::BECH32, true)) { - return OutputType::BECH32; - } else { - return m_default_address_type; - } + std::vector> dummy; + const TxoutType type{Solver(recipient.scriptPubKey, dummy)}; + if (type == TxoutType::WITNESS_V1_TAPROOT) { + any_tr = true; + } else if (type == TxoutType::WITNESS_V0_KEYHASH) { + any_wpkh = true; + } else if (type == TxoutType::SCRIPTHASH) { + any_sh = true; + } else if (type == TxoutType::PUBKEYHASH) { + any_pkh = true; } } + const bool has_bech32m_spkman(GetScriptPubKeyMan(OutputType::BECH32M, /*internal=*/true)); + if (has_bech32m_spkman && any_tr) { + // Currently tr is the only type supported by the BECH32M spkman + return OutputType::BECH32M; + } + const bool has_bech32_spkman(GetScriptPubKeyMan(OutputType::BECH32, /*internal=*/true)); + if (has_bech32_spkman && any_wpkh) { + // Currently wpkh is the only type supported by the BECH32 spkman + return OutputType::BECH32; + } + const bool has_p2sh_segwit_spkman(GetScriptPubKeyMan(OutputType::P2SH_SEGWIT, /*internal=*/true)); + if (has_p2sh_segwit_spkman && any_sh) { + // Currently sh_wpkh is the only type supported by the P2SH_SEGWIT spkman + // As of 2021 about 80% of all SH are wrapping WPKH, so use that + return OutputType::P2SH_SEGWIT; + } + const bool has_legacy_spkman(GetScriptPubKeyMan(OutputType::LEGACY, /*internal=*/true)); + if (has_legacy_spkman && any_pkh) { + // Currently pkh is the only type supported by the LEGACY spkman + return OutputType::LEGACY; + } + + if (has_bech32m_spkman) { + return OutputType::BECH32M; + } + if (has_bech32_spkman) { + return OutputType::BECH32; + } // else use m_default_address_type for change return m_default_address_type; } diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 17d11a3ceb5..a8e6acea45e 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -571,12 +571,12 @@ class RawTransactionsTest(BitcoinTestFramework): if self.options.descriptors: self.nodes[1].walletpassphrase('test', 10) self.nodes[1].importdescriptors([{ - 'desc': descsum_create('tr(tprv8ZgxMBicQKsPdYeeZbPSKd2KYLmeVKtcFA7kqCxDvDR13MQ6us8HopUR2wLcS2ZKPhLyKsqpDL2FtL73LMHcgoCL7DXsciA8eX8nbjCR2eG/0h/*h)'), + 'desc': descsum_create('wpkh(tprv8ZgxMBicQKsPdYeeZbPSKd2KYLmeVKtcFA7kqCxDvDR13MQ6us8HopUR2wLcS2ZKPhLyKsqpDL2FtL73LMHcgoCL7DXsciA8eX8nbjCR2eG/0h/*h)'), 'timestamp': 'now', 'active': True }, { - 'desc': descsum_create('tr(tprv8ZgxMBicQKsPdYeeZbPSKd2KYLmeVKtcFA7kqCxDvDR13MQ6us8HopUR2wLcS2ZKPhLyKsqpDL2FtL73LMHcgoCL7DXsciA8eX8nbjCR2eG/1h/*h)'), + 'desc': descsum_create('wpkh(tprv8ZgxMBicQKsPdYeeZbPSKd2KYLmeVKtcFA7kqCxDvDR13MQ6us8HopUR2wLcS2ZKPhLyKsqpDL2FtL73LMHcgoCL7DXsciA8eX8nbjCR2eG/1h/*h)'), 'timestamp': 'now', 'active': True, 'internal': True @@ -778,18 +778,11 @@ class RawTransactionsTest(BitcoinTestFramework): for param, zero_value in product(["fee_rate", "feeRate"], [0, 0.000, 0.00000000, "0", "0.000", "0.00000000"]): assert_equal(self.nodes[3].fundrawtransaction(rawtx, {param: zero_value})["fee"], 0) - if self.options.descriptors: - # With no arguments passed, expect fee of 153 satoshis as descriptor wallets now have a taproot output. - assert_approx(node.fundrawtransaction(rawtx)["fee"], vexp=0.00000153, vspan=0.00000001) - # Expect fee to be 10,000x higher when an explicit fee rate 10,000x greater is specified. - result = node.fundrawtransaction(rawtx, {"fee_rate": 10000}) - assert_approx(result["fee"], vexp=0.0153, vspan=0.0001) - else: - # With no arguments passed, expect fee of 141 satoshis as legacy wallets only support up to segwit v0. - assert_approx(node.fundrawtransaction(rawtx)["fee"], vexp=0.00000141, vspan=0.00000001) - # Expect fee to be 10,000x higher when an explicit fee rate 10,000x greater is specified. - result = node.fundrawtransaction(rawtx, {"fee_rate": 10000}) - assert_approx(result["fee"], vexp=0.0141, vspan=0.0001) + # With no arguments passed, expect fee of 141 satoshis. + assert_approx(node.fundrawtransaction(rawtx)["fee"], vexp=0.00000141, vspan=0.00000001) + # Expect fee to be 10,000x higher when an explicit fee rate 10,000x greater is specified. + result = node.fundrawtransaction(rawtx, {"fee_rate": 10000}) + assert_approx(result["fee"], vexp=0.0141, vspan=0.0001) self.log.info("Test fundrawtxn with invalid estimate_mode settings") for k, v in {"number": 42, "object": {"foo": "bar"}}.items(): @@ -1080,7 +1073,7 @@ class RawTransactionsTest(BitcoinTestFramework): # Make sure the default wallet will not be loaded when restarted with a high minrelaytxfee self.nodes[0].unloadwallet(self.default_wallet_name, False) feerate = Decimal("0.1") - self.restart_node(0, [f"-minrelaytxfee={feerate}", "-discardfee=0", "-changetype=bech32", "-addresstype=bech32"]) # Set high minrelayfee, set discardfee to 0 for easier calculation + self.restart_node(0, [f"-minrelaytxfee={feerate}", "-discardfee=0"]) # Set high minrelayfee, set discardfee to 0 for easier calculation self.nodes[0].loadwallet(self.default_wallet_name, True) funds = self.nodes[0].get_wallet_rpc(self.default_wallet_name) diff --git a/test/functional/wallet_address_types.py b/test/functional/wallet_address_types.py index eb6e497951c..f7c80f805c2 100755 --- a/test/functional/wallet_address_types.py +++ b/test/functional/wallet_address_types.py @@ -346,13 +346,13 @@ class AddressTypeTest(BitcoinTestFramework): self.test_change_output_type(0, [to_address_bech32_1], 'legacy') if self.options.descriptors: - self.log.info("Nodes with addresstype=p2sh-segwit only use a bech32m change output if any destination address is bech32:") + self.log.info("Nodes with addresstype=p2sh-segwit match the change output") self.test_change_output_type(1, [to_address_p2sh], 'p2sh-segwit') - self.test_change_output_type(1, [to_address_bech32_1], 'bech32m') - self.test_change_output_type(1, [to_address_p2sh, to_address_bech32_1], 'bech32m') - self.test_change_output_type(1, [to_address_bech32_1, to_address_bech32_2], 'bech32m') + self.test_change_output_type(1, [to_address_bech32_1], 'bech32') + self.test_change_output_type(1, [to_address_p2sh, to_address_bech32_1], 'bech32') + self.test_change_output_type(1, [to_address_bech32_1, to_address_bech32_2], 'bech32') else: - self.log.info("Nodes with addresstype=p2sh-segwit only use a P2WPKH change output if any destination address is bech32:") + self.log.info("Nodes with addresstype=p2sh-segwit match the change output") self.test_change_output_type(1, [to_address_p2sh], 'p2sh-segwit') self.test_change_output_type(1, [to_address_bech32_1], 'bech32') self.test_change_output_type(1, [to_address_p2sh, to_address_bech32_1], 'bech32') @@ -363,13 +363,13 @@ class AddressTypeTest(BitcoinTestFramework): self.test_change_output_type(2, [to_address_p2sh], 'bech32') if self.options.descriptors: - self.log.info("Nodes with addresstype=bech32 always use either a bech32 or bech32m change output (unless changetype is set otherwise):") - self.test_change_output_type(3, [to_address_bech32_1], 'bech32m') - self.test_change_output_type(3, [to_address_p2sh], 'bech32') - else: - self.log.info("Nodes with addresstype=bech32 always use a P2WPKH change output (unless changetype is set otherwise):") + self.log.info("Nodes with addresstype=bech32 match the change output (unless changetype is set otherwise):") self.test_change_output_type(3, [to_address_bech32_1], 'bech32') - self.test_change_output_type(3, [to_address_p2sh], 'bech32') + self.test_change_output_type(3, [to_address_p2sh], 'p2sh-segwit') + else: + self.log.info("Nodes with addresstype=bech32 match the change output (unless changetype is set otherwise):") + self.test_change_output_type(3, [to_address_bech32_1], 'bech32') + self.test_change_output_type(3, [to_address_p2sh], 'p2sh-segwit') self.log.info('getrawchangeaddress defaults to addresstype if -changetype is not set and argument is absent') self.test_address(3, self.nodes[3].getrawchangeaddress(), multisig=False, typ='bech32') diff --git a/test/functional/wallet_groups.py b/test/functional/wallet_groups.py index 9052bc7f7f4..eb305c5fa27 100755 --- a/test/functional/wallet_groups.py +++ b/test/functional/wallet_groups.py @@ -108,17 +108,10 @@ class WalletGroupTest(BitcoinTestFramework): assert_equal(input_addrs[0], input_addrs[1]) # Node 2 enforces avoidpartialspends so needs no checking here - if self.options.descriptors: - # Descriptor wallets will use Taproot change by default which has different fees - tx4_ungrouped_fee = 3060 - tx4_grouped_fee = 4400 - tx5_6_ungrouped_fee = 5760 - tx5_6_grouped_fee = 8480 - else: - tx4_ungrouped_fee = 2820 - tx4_grouped_fee = 4160 - tx5_6_ungrouped_fee = 5520 - tx5_6_grouped_fee = 8240 + tx4_ungrouped_fee = 2820 + tx4_grouped_fee = 4160 + tx5_6_ungrouped_fee = 5520 + tx5_6_grouped_fee = 8240 self.log.info("Test wallet option maxapsfee") addr_aps = self.nodes[3].getnewaddress() diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index d78afb42127..ac878ea0aa8 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -136,7 +136,7 @@ class WalletHDTest(BitcoinTestFramework): keypath = self.nodes[1].getaddressinfo(out['scriptPubKey']['address'])['hdkeypath'] if self.options.descriptors: - assert_equal(keypath[0:14], "m/86'/1'/0'/1/") + assert_equal(keypath[0:14], "m/84'/1'/0'/1/") else: assert_equal(keypath[0:7], "m/0'/1'")