diff --git a/src/addresstype.cpp b/src/addresstype.cpp index eac79a84f3..3c47f58fb7 100644 --- a/src/addresstype.cpp +++ b/src/addresstype.cpp @@ -67,11 +67,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])); @@ -100,11 +101,7 @@ 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: @@ -112,6 +109,7 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet) case TxoutType::NONSTANDARD: case TxoutType::OP_TRUE: // ELEMENTS (no address type) case TxoutType::FEE: // ELEMENTS (no address type) + addressRet = CNoDestination(scriptPubKey); return false; } // no default case, so the compiler can warn about missing cases assert(false); @@ -123,7 +121,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 @@ -153,7 +156,7 @@ 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(); } // ELEMENTS @@ -166,6 +169,20 @@ public: return script; } }; + +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; } + bool operator()(const NullData& dest) const { return false; } // ELEMENTS +}; } // namespace CScript GetScriptForDestination(const CTxDestination& dest) @@ -174,5 +191,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 b65bf034b4..ce00f5411f 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 @@ -85,23 +106,27 @@ 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: CPubKey blinding_pubkey; // ELEMENTS + 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(); } }; @@ -121,26 +146,32 @@ public: }; /** - * 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) * * NullData: TxoutType::NULL_DATA destination (OP_RETURN) // ELEMENTS * 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/blind.h b/src/blind.h index 98c2c683b4..25e521a5e7 100644 --- a/src/blind.h +++ b/src/blind.h @@ -93,6 +93,7 @@ public: void operator()(WitnessV1Taproot &dest) const { dest.blinding_pubkey = blinding_pubkey; } void operator()(WitnessUnknown &dest) const { dest.blinding_pubkey = blinding_pubkey; } void operator()(NullData &dest) const { } + void operator()(PubKeyDestination &dest) const { } }; #endif // BITCOIN_BLIND_H diff --git a/src/key_io.cpp b/src/key_io.cpp index a0f17d603e..6c31cffca6 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -123,26 +123,28 @@ 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); + std::vector data = {(unsigned char)id.GetWitnessVersion()}; + data.reserve(1 + (program.size() * 8 + 4) / 5); if (id.blinding_pubkey.IsFullyValid()) { std::vector bytes(id.blinding_pubkey.begin(), id.blinding_pubkey.end()); - bytes.insert(bytes.end(), id.program, id.program + id.length); + bytes.insert(bytes.end(), program.begin(), program.end()); ConvertBits<8, 5, true>([&](unsigned char c) { data.push_back(c); }, bytes.begin(), bytes.end()); const std::string& hrp = for_parent ? m_params.ParentBlech32HRP() : m_params.Blech32HRP(); return blech32::Encode(blech32::Encoding::BLECH32M, hrp, data); } - ConvertBits<8, 5, true>([&](unsigned char c) { data.push_back(c); }, id.program, id.program + id.length); + ConvertBits<8, 5, true>([&](unsigned char c) { data.push_back(c); }, program.begin(), program.end()); const std::string& hrp = for_parent ? m_params.ParentBech32HRP() : m_params.Bech32HRP(); return bech32::Encode(bech32::Encoding::BECH32M, hrp, data); } std::string operator()(const CNoDestination& no) const { return {}; } - std::string operator()(const NullData& null) const { return "null"; } + std::string operator()(const PubKeyDestination& pk) const { return {}; } + std::string operator()(const NullData& null) const { return "null"; } // ELEMENTS }; CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, const bool for_parent, std::string& error_str, std::vector* error_locations) @@ -287,11 +289,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(); @@ -371,11 +369,8 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par return CNoDestination(); } - WitnessUnknown unk; - unk.version = version; - std::copy(data.begin(), data.end(), unk.program); + WitnessUnknown unk{version, {data.begin(), data.end()}}; unk.blinding_pubkey = blinding_pubkey; - unk.length = data.size(); return unk; } } diff --git a/src/rpc/node.cpp b/src/rpc/node.cpp index 0d571a3e65..9624da9753 100644 --- a/src/rpc/node.cpp +++ b/src/rpc/node.cpp @@ -601,6 +601,7 @@ public: } void operator()(const NullData& id) const {} + void operator()(const PubKeyDestination& id) const {} }; diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp index 4ae154d932..6930dcf1e4 100644 --- a/src/rpc/output_script.cpp +++ b/src/rpc/output_script.cpp @@ -312,6 +312,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 9b10205f74..7340a834bc 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -258,6 +258,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); @@ -308,8 +313,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; } @@ -1340,6 +1345,11 @@ public: return CPubKey(); } + CPubKey operator()(const PubKeyDestination& dest) const + { + return CPubKey(); + } + CPubKey operator()(const PKHash& keyID) const { return keyID.blinding_pubkey; @@ -1391,6 +1401,7 @@ public: explicit DescribeBlindAddressVisitor() {} 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/test/fuzz/key.cpp b/src/test/fuzz/key.cpp index 60f4081432..be45443172 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 acc82f55f6..fe41a8c6ae 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 91839f87d1..c71a0ba6ca 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -175,6 +175,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)}; }, @@ -191,15 +200,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}; }, [&] { tx_destination = NullData{}; diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index 1673b9fb12..eb2278b15a 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 ec845aeffb..1afb28cc10 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 4771065904..f0e8af4d9e 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -258,12 +258,12 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo return Result::WALLET_ERROR; } + CTxDestination dest; + ExtractDestination(output.scriptPubKey, dest); if (reduce_output.has_value() ? reduce_output.value() == i : is_change) { - CTxDestination change_dest; - ExtractDestination(output.scriptPubKey, change_dest); - destinations[output.nAsset.GetAsset()] = change_dest; + destinations[output.nAsset.GetAsset()] = dest; } else if (!is_change && !is_fee) { - wallet::CRecipient recipient = {output.scriptPubKey, output.nValue.GetAmount(), output.nAsset.GetAsset(), CPubKey(output.nNonce.vchCommitment), false}; + wallet::CRecipient recipient = {dest, output.nValue.GetAmount(), output.nAsset.GetAsset(), CPubKey(output.nNonce.vchCommitment), false}; recipients.push_back(recipient); } new_outputs_value += output.nValue.GetAmount(); @@ -293,7 +293,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo // If the output differs from the original tx output (because the user customized it) a new change output will be created. const auto script = GetScriptForDestination(destination); const CPubKey blinding_pubkey = wallet.GetBlindingKey(&script).GetPubKey(); - recipients.emplace_back(CRecipient{script, new_outputs_value, asset, blinding_pubkey, /*fSubtractFeeFromAmount=*/true}); + recipients.emplace_back(CRecipient{destination, new_outputs_value, asset, blinding_pubkey, /*fSubtractFeeFromAmount=*/true}); new_coin_control.destChange.clear(); } diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index ad8887a8a8..47f7c78880 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -445,6 +445,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 { @@ -517,6 +518,7 @@ public: explicit DescribeWalletBlindAddressVisitor(const CWallet& _wallet, isminetype mine_in) : wallet(_wallet), mine(mine_in) {} 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 511d07eedc..af97066913 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -53,7 +53,6 @@ static void ParseRecipients(const UniValue& address_amounts, const UniValue& add } destinations.insert(dest); - CScript script_pub_key = GetScriptForDestination(dest); CAmount amount = AmountFromValue(address_amounts[i++], asset == Params().GetConsensus().pegged_asset); bool subtract_fee = false; @@ -64,7 +63,7 @@ static void ParseRecipients(const UniValue& address_amounts, const UniValue& add } } - CRecipient recipient = {script_pub_key, amount, asset, GetDestinationBlindingKey(dest), subtract_fee}; + CRecipient recipient = {dest, amount, asset, GetDestinationBlindingKey(dest), subtract_fee}; recipients.push_back(recipient); } } diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 93b65b9b18..6203c6a6ab 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -575,8 +575,15 @@ std::map> ListCoins(const CWallet& wallet) continue; } - 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); } } @@ -1402,7 +1409,7 @@ static util::Result CreateTransactionInternal( bool may_need_blinded_dummy = !!blind_details; for (const auto& recipient : vecSend) { - CTxOut txout(recipient.asset, recipient.nAmount, recipient.scriptPubKey); + CTxOut txout(recipient.asset, recipient.nAmount, GetScriptForDestination(recipient.dest)); txout.nNonce.vchCommitment = std::vector(recipient.confidentiality_key.begin(), recipient.confidentiality_key.end()); // Include the fee cost for outputs. @@ -2098,7 +2105,9 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet, continue; } - CRecipient recipient = {txOut.scriptPubKey, txOut.nValue.GetAmount(), txOut.nAsset.GetAsset(), CPubKey(txOut.nNonce.vchCommitment), setSubtractFeeFromOutputs.count(idx) == 1}; + CTxDestination dest; + ExtractDestination(txOut.scriptPubKey, dest); + CRecipient recipient = {dest, txOut.nValue.GetAmount(), txOut.nAsset.GetAsset(), CPubKey(txOut.nNonce.vchCommitment), 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 2da0f44dd5..1ebad6a220 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, CAsset(), CPubKey(), /*subtract_fee=*/true}; + CRecipient recipient{PubKeyDestination({}), 50 * COIN - leftover_input_amount, CAsset(), CPubKey(), /*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 4cd344dbe7..bd139e8a52 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -652,7 +652,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, CAsset(), CPubKey(), /*fSubtractFeeFromAmount=*/true}); + CWalletTx& wtx = context.AddTx(CRecipient{*dest, amount, CAsset(), CPubKey(), /*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 a6c8437ef7..42921e18d8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2588,15 +2588,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 dba3800a6d..5b52ec3682 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -328,7 +328,7 @@ inline std::optional PurposeFromString(std::string_view s) struct CRecipient { - CScript scriptPubKey; + CTxDestination dest; CAmount nAmount; CAsset asset; CPubKey confidentiality_key; diff --git a/test/functional/rpc_deriveaddresses.py b/test/functional/rpc_deriveaddresses.py index 0d472bee44..8cca8e49bd 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), ["2dnaGtwYgBhXYQGTArxKKapi52Mkf3KTQhb", "2dnaGtwYgBhXYQGTArxKKapi52Mkf3KTQhb", address, "XY2Fo8bxL1EViXjWrZ5iZrb5thmfPvWJxw"]) + 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