diff --git a/src/addresstype.cpp b/src/addresstype.cpp index 2454cfb5d95..f199d1b4794 100644 --- a/src/addresstype.cpp +++ b/src/addresstype.cpp @@ -54,11 +54,12 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet) switch (whichType) { case TxoutType::PUBKEY: { CPubKey pubKey(vSolutions[0]); - if (!pubKey.IsValid()) - return false; - - addressRet = PKHash(pubKey); - return true; + if (!pubKey.IsValid()) { + addressRet = CNoDestination(scriptPubKey); + } else { + addressRet = PubKeyDestination(pubKey); + } + return false; } case TxoutType::PUBKEYHASH: { addressRet = PKHash(uint160(vSolutions[0])); @@ -87,16 +88,13 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet) return true; } case TxoutType::WITNESS_UNKNOWN: { - WitnessUnknown unk; - unk.version = vSolutions[0][0]; - std::copy(vSolutions[1].begin(), vSolutions[1].end(), unk.program); - unk.length = vSolutions[1].size(); - addressRet = unk; + addressRet = WitnessUnknown{vSolutions[0][0], vSolutions[1]}; return true; } case TxoutType::MULTISIG: case TxoutType::NULL_DATA: case TxoutType::NONSTANDARD: + addressRet = CNoDestination(scriptPubKey); return false; } // no default case, so the compiler can warn about missing cases assert(false); @@ -108,7 +106,12 @@ class CScriptVisitor public: CScript operator()(const CNoDestination& dest) const { - return CScript(); + return dest.GetScript(); + } + + CScript operator()(const PubKeyDestination& dest) const + { + return CScript() << ToByteVector(dest.GetPubKey()) << OP_CHECKSIG; } CScript operator()(const PKHash& keyID) const @@ -138,9 +141,22 @@ public: CScript operator()(const WitnessUnknown& id) const { - return CScript() << CScript::EncodeOP_N(id.version) << std::vector(id.program, id.program + id.length); + return CScript() << CScript::EncodeOP_N(id.GetWitnessVersion()) << id.GetWitnessProgram(); } }; + +class ValidDestinationVisitor +{ +public: + bool operator()(const CNoDestination& dest) const { return false; } + bool operator()(const PubKeyDestination& dest) const { return false; } + bool operator()(const PKHash& dest) const { return true; } + bool operator()(const ScriptHash& dest) const { return true; } + bool operator()(const WitnessV0KeyHash& dest) const { return true; } + bool operator()(const WitnessV0ScriptHash& dest) const { return true; } + bool operator()(const WitnessV1Taproot& dest) const { return true; } + bool operator()(const WitnessUnknown& dest) const { return true; } +}; } // namespace CScript GetScriptForDestination(const CTxDestination& dest) @@ -149,5 +165,5 @@ CScript GetScriptForDestination(const CTxDestination& dest) } bool IsValidDestination(const CTxDestination& dest) { - return dest.index() != 0; + return std::visit(ValidDestinationVisitor(), dest); } diff --git a/src/addresstype.h b/src/addresstype.h index 6b651e90140..d3422c68130 100644 --- a/src/addresstype.h +++ b/src/addresstype.h @@ -14,9 +14,30 @@ #include class CNoDestination { +private: + CScript m_script; + public: - friend bool operator==(const CNoDestination &a, const CNoDestination &b) { return true; } - friend bool operator<(const CNoDestination &a, const CNoDestination &b) { return true; } + CNoDestination() = default; + CNoDestination(const CScript& script) : m_script(script) {} + + const CScript& GetScript() const { return m_script; } + + friend bool operator==(const CNoDestination& a, const CNoDestination& b) { return a.GetScript() == b.GetScript(); } + friend bool operator<(const CNoDestination& a, const CNoDestination& b) { return a.GetScript() < b.GetScript(); } +}; + +struct PubKeyDestination { +private: + CPubKey m_pubkey; + +public: + PubKeyDestination(const CPubKey& pubkey) : m_pubkey(pubkey) {} + + const CPubKey& GetPubKey() const LIFETIMEBOUND { return m_pubkey; } + + friend bool operator==(const PubKeyDestination& a, const PubKeyDestination& b) { return a.GetPubKey() == b.GetPubKey(); } + friend bool operator<(const PubKeyDestination& a, const PubKeyDestination& b) { return a.GetPubKey() < b.GetPubKey(); } }; struct PKHash : public BaseHash @@ -69,45 +90,55 @@ struct WitnessV1Taproot : public XOnlyPubKey //! CTxDestination subtype to encode any future Witness version struct WitnessUnknown { - unsigned int version; - unsigned int length; - unsigned char program[40]; +private: + unsigned int m_version; + std::vector m_program; + +public: + WitnessUnknown(unsigned int version, const std::vector& program) : m_version(version), m_program(program) {} + WitnessUnknown(int version, const std::vector& program) : m_version(static_cast(version)), m_program(program) {} + + unsigned int GetWitnessVersion() const { return m_version; } + const std::vector& GetWitnessProgram() const LIFETIMEBOUND { return m_program; } friend bool operator==(const WitnessUnknown& w1, const WitnessUnknown& w2) { - if (w1.version != w2.version) return false; - if (w1.length != w2.length) return false; - return std::equal(w1.program, w1.program + w1.length, w2.program); + if (w1.GetWitnessVersion() != w2.GetWitnessVersion()) return false; + return w1.GetWitnessProgram() == w2.GetWitnessProgram(); } friend bool operator<(const WitnessUnknown& w1, const WitnessUnknown& w2) { - if (w1.version < w2.version) return true; - if (w1.version > w2.version) return false; - if (w1.length < w2.length) return true; - if (w1.length > w2.length) return false; - return std::lexicographical_compare(w1.program, w1.program + w1.length, w2.program, w2.program + w2.length); + if (w1.GetWitnessVersion() < w2.GetWitnessVersion()) return true; + if (w1.GetWitnessVersion() > w2.GetWitnessVersion()) return false; + return w1.GetWitnessProgram() < w2.GetWitnessProgram(); } }; /** - * A txout script template with a specific destination. It is either: - * * CNoDestination: no destination set - * * PKHash: TxoutType::PUBKEYHASH destination (P2PKH) - * * ScriptHash: TxoutType::SCRIPTHASH destination (P2SH) - * * WitnessV0ScriptHash: TxoutType::WITNESS_V0_SCRIPTHASH destination (P2WSH) - * * WitnessV0KeyHash: TxoutType::WITNESS_V0_KEYHASH destination (P2WPKH) - * * WitnessV1Taproot: TxoutType::WITNESS_V1_TAPROOT destination (P2TR) - * * WitnessUnknown: TxoutType::WITNESS_UNKNOWN destination (P2W???) + * A txout script categorized into standard templates. + * * CNoDestination: Optionally a script, no corresponding address. + * * PubKeyDestination: TxoutType::PUBKEY (P2PK), no corresponding address + * * PKHash: TxoutType::PUBKEYHASH destination (P2PKH address) + * * ScriptHash: TxoutType::SCRIPTHASH destination (P2SH address) + * * WitnessV0ScriptHash: TxoutType::WITNESS_V0_SCRIPTHASH destination (P2WSH address) + * * WitnessV0KeyHash: TxoutType::WITNESS_V0_KEYHASH destination (P2WPKH address) + * * WitnessV1Taproot: TxoutType::WITNESS_V1_TAPROOT destination (P2TR address) + * * WitnessUnknown: TxoutType::WITNESS_UNKNOWN destination (P2W??? address) * A CTxDestination is the internal data type encoded in a bitcoin address */ -using CTxDestination = std::variant; +using CTxDestination = std::variant; -/** Check whether a CTxDestination is a CNoDestination. */ +/** Check whether a CTxDestination corresponds to one with an address. */ bool IsValidDestination(const CTxDestination& dest); /** - * Parse a standard scriptPubKey for the destination address. Assigns result to - * the addressRet parameter and returns true if successful. Currently only works for P2PK, - * P2PKH, P2SH, P2WPKH, and P2WSH scripts. + * Parse a scriptPubKey for the destination. + * + * For standard scripts that have addresses (and P2PK as an exception), a corresponding CTxDestination + * is assigned to addressRet. + * For all other scripts. addressRet is assigned as a CNoDestination containing the scriptPubKey. + * + * Returns true for standard destinations with addresses - P2PKH, P2SH, P2WPKH, P2WSH, P2TR and P2W??? scripts. + * Returns false for non-standard destinations and those without addresses - P2PK, bare multisig, null data, and nonstandard scripts. */ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet); diff --git a/src/key_io.cpp b/src/key_io.cpp index 1a0b51a28bc..5bcbb8a0694 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -67,16 +67,18 @@ public: std::string operator()(const WitnessUnknown& id) const { - if (id.version < 1 || id.version > 16 || id.length < 2 || id.length > 40) { + const std::vector& program = id.GetWitnessProgram(); + if (id.GetWitnessVersion() < 1 || id.GetWitnessVersion() > 16 || program.size() < 2 || program.size() > 40) { return {}; } - std::vector data = {(unsigned char)id.version}; - data.reserve(1 + (id.length * 8 + 4) / 5); - ConvertBits<8, 5, true>([&](unsigned char c) { data.push_back(c); }, id.program, id.program + id.length); + std::vector data = {(unsigned char)id.GetWitnessVersion()}; + data.reserve(1 + (program.size() * 8 + 4) / 5); + ConvertBits<8, 5, true>([&](unsigned char c) { data.push_back(c); }, program.begin(), program.end()); return bech32::Encode(bech32::Encoding::BECH32M, m_params.Bech32HRP(), data); } std::string operator()(const CNoDestination& no) const { return {}; } + std::string operator()(const PubKeyDestination& pk) const { return {}; } }; CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, std::string& error_str, std::vector* error_locations) @@ -189,11 +191,7 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par return CNoDestination(); } - WitnessUnknown unk; - unk.version = version; - std::copy(data.begin(), data.end(), unk.program); - unk.length = data.size(); - return unk; + return WitnessUnknown{version, data}; } else { error_str = strprintf("Invalid padding in Bech32 data section"); return CNoDestination(); diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp index 4dd424fa147..f9343f48a89 100644 --- a/src/rpc/output_script.cpp +++ b/src/rpc/output_script.cpp @@ -280,6 +280,11 @@ static RPCHelpMan deriveaddresses() for (const CScript& script : scripts) { CTxDestination dest; if (!ExtractDestination(script, dest)) { + // ExtractDestination no longer returns true for P2PK since it doesn't have a corresponding address + // However combo will output P2PK and should just ignore that script + if (scripts.size() > 1 && std::get_if(&dest)) { + continue; + } throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Descriptor does not have a corresponding address"); } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 74ef04033e3..9a941be1816 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -253,6 +253,11 @@ public: return UniValue(UniValue::VOBJ); } + UniValue operator()(const PubKeyDestination& dest) const + { + return UniValue(UniValue::VOBJ); + } + UniValue operator()(const PKHash& keyID) const { UniValue obj(UniValue::VOBJ); @@ -303,8 +308,8 @@ public: { UniValue obj(UniValue::VOBJ); obj.pushKV("iswitness", true); - obj.pushKV("witness_version", (int)id.version); - obj.pushKV("witness_program", HexStr({id.program, id.length})); + obj.pushKV("witness_version", id.GetWitnessVersion()); + obj.pushKV("witness_program", HexStr(id.GetWitnessProgram())); return obj; } }; diff --git a/src/test/fuzz/key.cpp b/src/test/fuzz/key.cpp index 60f4081432a..be45443172f 100644 --- a/src/test/fuzz/key.cpp +++ b/src/test/fuzz/key.cpp @@ -186,7 +186,7 @@ FUZZ_TARGET(key, .init = initialize_key) const CTxDestination tx_destination = GetDestinationForKey(pubkey, output_type); assert(output_type == OutputType::LEGACY); assert(IsValidDestination(tx_destination)); - assert(CTxDestination{PKHash{pubkey}} == tx_destination); + assert(PKHash{pubkey} == *std::get_if(&tx_destination)); const CScript script_for_destination = GetScriptForDestination(tx_destination); assert(script_for_destination.size() == 25); diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script.cpp index acc82f55f6c..fe41a8c6ae4 100644 --- a/src/test/fuzz/script.cpp +++ b/src/test/fuzz/script.cpp @@ -149,13 +149,16 @@ FUZZ_TARGET(script, .init = initialize_script) const CTxDestination tx_destination_2{ConsumeTxDestination(fuzzed_data_provider)}; const std::string encoded_dest{EncodeDestination(tx_destination_1)}; const UniValue json_dest{DescribeAddress(tx_destination_1)}; - Assert(tx_destination_1 == DecodeDestination(encoded_dest)); (void)GetKeyForDestination(/*store=*/{}, tx_destination_1); const CScript dest{GetScriptForDestination(tx_destination_1)}; const bool valid{IsValidDestination(tx_destination_1)}; - Assert(dest.empty() != valid); - Assert(valid == IsValidDestinationString(encoded_dest)); + if (!std::get_if(&tx_destination_1)) { + // Only try to round trip non-pubkey destinations since PubKeyDestination has no encoding + Assert(dest.empty() != valid); + Assert(tx_destination_1 == DecodeDestination(encoded_dest)); + Assert(valid == IsValidDestinationString(encoded_dest)); + } (void)(tx_destination_1 < tx_destination_2); if (tx_destination_1 == tx_destination_2) { diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index ca2218e94cd..87ca2f6aede 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -172,6 +172,15 @@ CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) no [&] { tx_destination = CNoDestination{}; }, + [&] { + bool compressed = fuzzed_data_provider.ConsumeBool(); + CPubKey pk{ConstructPubKeyBytes( + fuzzed_data_provider, + ConsumeFixedLengthByteVector(fuzzed_data_provider, (compressed ? CPubKey::COMPRESSED_SIZE : CPubKey::SIZE)), + compressed + )}; + tx_destination = PubKeyDestination{pk}; + }, [&] { tx_destination = PKHash{ConsumeUInt160(fuzzed_data_provider)}; }, @@ -188,15 +197,11 @@ CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) no tx_destination = WitnessV1Taproot{XOnlyPubKey{ConsumeUInt256(fuzzed_data_provider)}}; }, [&] { - WitnessUnknown witness_unknown{}; - witness_unknown.version = fuzzed_data_provider.ConsumeIntegralInRange(2, 16); - std::vector witness_unknown_program_1{fuzzed_data_provider.ConsumeBytes(40)}; - if (witness_unknown_program_1.size() < 2) { - witness_unknown_program_1 = {0, 0}; + std::vector program{ConsumeRandomLengthByteVector(fuzzed_data_provider, /*max_length=*/40)}; + if (program.size() < 2) { + program = {0, 0}; } - witness_unknown.length = witness_unknown_program_1.size(); - std::copy(witness_unknown_program_1.begin(), witness_unknown_program_1.end(), witness_unknown.program); - tx_destination = witness_unknown; + tx_destination = WitnessUnknown{fuzzed_data_provider.ConsumeIntegralInRange(2, 16), program}; })}; Assert(call_size == std::variant_size_v); return tx_destination; diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index 1a205728d6e..58bdb37b7c8 100644 --- a/src/test/script_standard_tests.cpp +++ b/src/test/script_standard_tests.cpp @@ -203,8 +203,8 @@ BOOST_AUTO_TEST_CASE(script_standard_ExtractDestination) // TxoutType::PUBKEY s.clear(); s << ToByteVector(pubkey) << OP_CHECKSIG; - BOOST_CHECK(ExtractDestination(s, address)); - BOOST_CHECK(std::get(address) == PKHash(pubkey)); + BOOST_CHECK(!ExtractDestination(s, address)); + BOOST_CHECK(std::get(address) == PubKeyDestination(pubkey)); // TxoutType::PUBKEYHASH s.clear(); @@ -249,10 +249,7 @@ BOOST_AUTO_TEST_CASE(script_standard_ExtractDestination) s.clear(); s << OP_1 << ToByteVector(pubkey); BOOST_CHECK(ExtractDestination(s, address)); - WitnessUnknown unk; - unk.length = 33; - unk.version = 1; - std::copy(pubkey.begin(), pubkey.end(), unk.program); + WitnessUnknown unk{1, ToByteVector(pubkey)}; BOOST_CHECK(std::get(address) == unk); } diff --git a/src/util/message.cpp b/src/util/message.cpp index ec845aeffbd..1afb28cc10d 100644 --- a/src/util/message.cpp +++ b/src/util/message.cpp @@ -47,7 +47,7 @@ MessageVerificationResult MessageVerify( return MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED; } - if (!(CTxDestination(PKHash(pubkey)) == destination)) { + if (!(PKHash(pubkey) == *std::get_if(&destination))) { return MessageVerificationResult::ERR_NOT_SIGNED; } diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index f99da926bc5..512a011dfc0 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -257,12 +257,12 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo const auto& txouts = outputs.empty() ? wtx.tx->vout : outputs; for (size_t i = 0; i < txouts.size(); ++i) { const CTxOut& output = txouts.at(i); + CTxDestination dest; + ExtractDestination(output.scriptPubKey, dest); if (reduce_output.has_value() ? reduce_output.value() == i : OutputIsChange(wallet, output)) { - CTxDestination change_dest; - ExtractDestination(output.scriptPubKey, change_dest); - new_coin_control.destChange = change_dest; + new_coin_control.destChange = dest; } else { - CRecipient recipient = {output.scriptPubKey, output.nValue, false}; + CRecipient recipient = {dest, output.nValue, false}; recipients.push_back(recipient); } new_outputs_value += output.nValue; @@ -278,7 +278,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo // Add change as recipient with SFFO flag enabled, so fees are deduced from it. // If the output differs from the original tx output (because the user customized it) a new change output will be created. - recipients.emplace_back(CRecipient{GetScriptForDestination(new_coin_control.destChange), new_outputs_value, /*fSubtractFeeFromAmount=*/true}); + recipients.emplace_back(CRecipient{new_coin_control.destChange, new_outputs_value, /*fSubtractFeeFromAmount=*/true}); new_coin_control.destChange = CNoDestination(); } diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index c1b99a4f97e..e9b93afc30b 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -427,6 +427,7 @@ public: explicit DescribeWalletAddressVisitor(const SigningProvider* _provider) : provider(_provider) {} UniValue operator()(const CNoDestination& dest) const { return UniValue(UniValue::VOBJ); } + UniValue operator()(const PubKeyDestination& dest) const { return UniValue(UniValue::VOBJ); } UniValue operator()(const PKHash& pkhash) const { diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index c4206e9897c..c2f4321a608 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -39,7 +39,6 @@ static void ParseRecipients(const UniValue& address_amounts, const UniValue& sub } destinations.insert(dest); - CScript script_pub_key = GetScriptForDestination(dest); CAmount amount = AmountFromValue(address_amounts[i++]); bool subtract_fee = false; @@ -50,7 +49,7 @@ static void ParseRecipients(const UniValue& address_amounts, const UniValue& sub } } - CRecipient recipient = {script_pub_key, amount, subtract_fee}; + CRecipient recipient = {dest, amount, subtract_fee}; recipients.push_back(recipient); } } diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 96c9a3dc167..7e6fba33aad 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -505,8 +505,15 @@ std::map> ListCoins(const CWallet& wallet) coins_params.skip_locked = false; for (const COutput& coin : AvailableCoins(wallet, &coin_control, /*feerate=*/std::nullopt, coins_params).All()) { CTxDestination address; - if ((coin.spendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.solvable)) && - ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) { + if ((coin.spendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.solvable))) { + if (!ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) { + // For backwards compatibility, we convert P2PK output scripts into PKHash destinations + if (auto pk_dest = std::get_if(&address)) { + address = PKHash(pk_dest->GetPubKey()); + } else { + continue; + } + } result[address].emplace_back(coin); } } @@ -1071,7 +1078,7 @@ static util::Result CreateTransactionInternal( // vouts to the payees for (const auto& recipient : vecSend) { - CTxOut txout(recipient.nAmount, recipient.scriptPubKey); + CTxOut txout(recipient.nAmount, GetScriptForDestination(recipient.dest)); // Include the fee cost for outputs. coin_selection_params.tx_noinputs_size += ::GetSerializeSize(txout, PROTOCOL_VERSION); @@ -1319,7 +1326,9 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet, // Turn the txout set into a CRecipient vector. for (size_t idx = 0; idx < tx.vout.size(); idx++) { const CTxOut& txOut = tx.vout[idx]; - CRecipient recipient = {txOut.scriptPubKey, txOut.nValue, setSubtractFeeFromOutputs.count(idx) == 1}; + CTxDestination dest; + ExtractDestination(txOut.scriptPubKey, dest); + CRecipient recipient = {dest, txOut.nValue, setSubtractFeeFromOutputs.count(idx) == 1}; vecSend.push_back(recipient); } diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index eca1d74cf63..68c98ae6b9e 100644 --- a/src/wallet/test/spend_tests.cpp +++ b/src/wallet/test/spend_tests.cpp @@ -27,7 +27,7 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup) // leftover input amount which would have been change to the recipient // instead of the miner. auto check_tx = [&wallet](CAmount leftover_input_amount) { - CRecipient recipient{GetScriptForRawPubKey({}), 50 * COIN - leftover_input_amount, /*subtract_fee=*/true}; + CRecipient recipient{PubKeyDestination({}), 50 * COIN - leftover_input_amount, /*subtract_fee=*/true}; constexpr int RANDOM_CHANGE_POSITION = -1; CCoinControl coin_control; coin_control.m_feerate.emplace(10000); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 5c297d76e48..dac6e87983e 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -645,7 +645,7 @@ void TestCoinsResult(ListCoinsTest& context, OutputType out_type, CAmount amount { LOCK(context.wallet->cs_wallet); util::Result dest = Assert(context.wallet->GetNewDestination(out_type, "")); - CWalletTx& wtx = context.AddTx(CRecipient{{GetScriptForDestination(*dest)}, amount, /*fSubtractFeeFromAmount=*/true}); + CWalletTx& wtx = context.AddTx(CRecipient{*dest, amount, /*fSubtractFeeFromAmount=*/true}); CoinFilterParams filter; filter.skip_locked = false; CoinsResult available_coins = AvailableCoins(*context.wallet, nullptr, std::nullopt, filter); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6f5248efafb..9f65db6c7b9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2213,15 +2213,13 @@ OutputType CWallet::TransactionChangeType(const std::optional& chang bool any_pkh{false}; for (const auto& recipient : vecSend) { - std::vector> dummy; - const TxoutType type{Solver(recipient.scriptPubKey, dummy)}; - if (type == TxoutType::WITNESS_V1_TAPROOT) { + if (std::get_if(&recipient.dest)) { any_tr = true; - } else if (type == TxoutType::WITNESS_V0_KEYHASH) { + } else if (std::get_if(&recipient.dest)) { any_wpkh = true; - } else if (type == TxoutType::SCRIPTHASH) { + } else if (std::get_if(&recipient.dest)) { any_sh = true; - } else if (type == TxoutType::PUBKEYHASH) { + } else if (std::get_if(&recipient.dest)) { any_pkh = true; } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 091a573151e..97d06641a73 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -288,7 +288,7 @@ inline std::optional PurposeFromString(std::string_view s) struct CRecipient { - CScript scriptPubKey; + CTxDestination dest; CAmount nAmount; bool fSubtractFeeFromAmount; }; diff --git a/test/functional/rpc_deriveaddresses.py b/test/functional/rpc_deriveaddresses.py index e96b6bda906..64994d6bb30 100755 --- a/test/functional/rpc_deriveaddresses.py +++ b/test/functional/rpc_deriveaddresses.py @@ -42,7 +42,10 @@ class DeriveaddressesTest(BitcoinTestFramework): assert_raises_rpc_error(-8, "Range should be greater or equal than 0", self.nodes[0].deriveaddresses, descsum_create("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), [-1, 0]) combo_descriptor = descsum_create("combo(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)") - assert_equal(self.nodes[0].deriveaddresses(combo_descriptor), ["mtfUoUax9L4tzXARpw1oTGxWyoogp52KhJ", "mtfUoUax9L4tzXARpw1oTGxWyoogp52KhJ", address, "2NDvEwGfpEqJWfybzpKPHF2XH3jwoQV3D7x"]) + assert_equal(self.nodes[0].deriveaddresses(combo_descriptor), ["mtfUoUax9L4tzXARpw1oTGxWyoogp52KhJ", address, "2NDvEwGfpEqJWfybzpKPHF2XH3jwoQV3D7x"]) + + # P2PK does not have a valid address + assert_raises_rpc_error(-5, "Descriptor does not have a corresponding address", self.nodes[0].deriveaddresses, descsum_create("pk(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK)")) # Before #26275, bitcoind would crash when deriveaddresses was # called with derivation index 2147483647, which is the maximum