phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-24 18:23:26 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

descriptors: inference process, do not return unparsable descriptors

Browse source 
The internal script-to-descriptor inference procedure shouldn't return invalid
descriptors or ones that fail the string-to-descriptor parsing process. These
two procedures should always support seamless roundtrips.

This inlines `InferScript`/`InferDescriptor` to the actual descriptors
`ParseScript` logic for the multisig path, ensuring only valid descriptors are
produced.

Examples where this presents an issue:
1) Because the legacy wallet allowed the import of invalid scripts, the process
   could end up inferring and storing an unparsable descriptor, which crashes
   the software during any subsequent wallet load

2) The `decodescript()` RPC command currently return unusable multisig descriptors.
   For example, providing a P2SH multisig redeem script with more than 520 bytes
   results in a descriptor when it shouldn't due to its large size.
This commit is contained in:
furszy 2024-12-01 12:59:24 -05:00
parent 088bbb81e3
commit e1d6179b61
No known key found for this signature in database
GPG key ID: 5DD23CCC686AA623
7 changed files with 47 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/core_write.cpp
View file

@ -153,7 +153,9 @@ void ScriptToUniv(const CScript& script, UniValue& out, bool include_hex, bool i
out.pushKV("asm", ScriptToAsmStr(script));
if (include_address) {
out.pushKV("desc", InferDescriptor(script, provider ? *provider : DUMMY_SIGNING_PROVIDER)->ToString());
auto desc = InferDescriptor(script, provider ? *provider : DUMMY_SIGNING_PROVIDER);
// future: make 'InferDescriptor' return the error cause
if (desc) out.pushKV("desc", desc->ToString());
}
if (include_hex) {
out.pushKV("hex", HexStr(script));

7
src/rpc/blockchain.cpp
View file

@ -1105,7 +1105,7 @@ static RPCHelpMan gettxout()
{RPCResult::Type::STR_AMOUNT, "value", "The transaction value in " + CURRENCY_UNIT},
{RPCResult::Type::OBJ, "scriptPubKey", "", {
{RPCResult::Type::STR, "asm", "Disassembly of the output script"},
{RPCResult::Type::STR, "desc", "Inferred descriptor for the output"},
{RPCResult::Type::STR, "desc", /*optional=*/true, "Inferred top level descriptor for the script (if any)"},
{RPCResult::Type::STR_HEX, "hex", "The raw output script bytes, hex-encoded"},
{RPCResult::Type::STR, "type", "The type, eg pubkeyhash"},
{RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"},
@ -2287,9 +2287,10 @@ static RPCHelpMan scantxoutset()
FlatSigningProvider provider;
auto scripts = EvalDescriptorStringOrObject(scanobject, provider);
for (CScript& script : scripts) {
std::string inferred = InferDescriptor(script, provider)->ToString();
auto desc = InferDescriptor(script, provider);
if (!desc) throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid descriptor found: '%s'", scanobject.get_str()));
needles.emplace(script);
descriptors.emplace(std::move(script), std::move(inferred));
descriptors.emplace(std::move(script), desc->ToString());
}
}

1
src/rpc/output_script.cpp
View file

@ -146,6 +146,7 @@ static RPCHelpMan createmultisig()
// Make the descriptor
std::unique_ptr<Descriptor> descriptor = InferDescriptor(GetScriptForDestination(dest), keystore);
if (!descriptor) throw JSONRPCError(RPC_INTERNAL_ERROR, "Invalid descriptor generated");
UniValue result(UniValue::VOBJ);
result.pushKV("address", EncodeDestination(dest));

15
src/rpc/rawtransaction.cpp
View file

@ -493,7 +493,7 @@ static RPCHelpMan decodescript()
RPCResult::Type::OBJ, "", "",
{
{RPCResult::Type::STR, "asm", "Disassembly of the script"},
{RPCResult::Type::STR, "desc", "Inferred descriptor for the script"},
{RPCResult::Type::STR, "desc", /*optional=*/true, "Inferred top level descriptor for the script (if any)"},
{RPCResult::Type::STR, "type", "The output type (e.g. " + GetAllOutputTypes() + ")"},
{RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"},
{RPCResult::Type::STR, "p2sh", /*optional=*/true,
@ -505,7 +505,7 @@ static RPCHelpMan decodescript()
{RPCResult::Type::STR_HEX, "hex", "The raw output script bytes, hex-encoded"},
{RPCResult::Type::STR, "type", "The type of the output script (e.g. witness_v0_keyhash or witness_v0_scripthash)"},
{RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"},
{RPCResult::Type::STR, "desc", "Inferred descriptor for the script"},
{RPCResult::Type::STR, "desc", /*optional=*/true, "Inferred descriptor for the script (if any)"},
{RPCResult::Type::STR, "p2sh-segwit", "address of the P2SH script wrapping this witness redeem script"},
}},
},
@ -529,9 +529,14 @@ static RPCHelpMan decodescript()
std::vector<std::vector<unsigned char>> solutions_data;
const TxoutType which_type{Solver(script, solutions_data)};
bool can_wrap_p2sh = true;
const bool can_wrap{[&] {
switch (which_type) {
case TxoutType::MULTISIG:
case TxoutType::MULTISIG: {
// Verify it doesn't exceed p2sh consensus limits
can_wrap_p2sh = script.size() <= MAX_SCRIPT_ELEMENT_SIZE;
break;
}
case TxoutType::NONSTANDARD:
case TxoutType::PUBKEY:
case TxoutType::PUBKEYHASH:
@ -561,7 +566,7 @@ static RPCHelpMan decodescript()
}()};
if (can_wrap) {
r.pushKV("p2sh", EncodeDestination(ScriptHash(script)));
if (can_wrap_p2sh) r.pushKV("p2sh", EncodeDestination(ScriptHash(script)));
// P2SH and witness programs cannot be wrapped in P2WSH, if this script
// is a witness program, don't return addresses for a segwit programs.
const bool can_wrap_P2WSH{[&] {
@ -822,7 +827,7 @@ const RPCResult decodepsbt_inputs{
{RPCResult::Type::OBJ, "scriptPubKey", "",
{
{RPCResult::Type::STR, "asm", "Disassembly of the output script"},
{RPCResult::Type::STR, "desc", "Inferred descriptor for the output"},
{RPCResult::Type::STR, "desc", /*optional=*/true, "Inferred top level descriptor for the script (if any)"},
{RPCResult::Type::STR_HEX, "hex", "The raw output script bytes, hex-encoded"},
{RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"},
{RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"},

13
src/script/descriptor.cpp
View file

@ -1853,7 +1853,7 @@ std::vector<std::unique_ptr<DescriptorImpl>> ParseScript(uint32_t& key_exp_index
}
if (ctx == ParseScriptContext::TOP) {
if (providers.size() > MAX_BARE_MULTISIG_PUBKEYS_NUM) {
error = strprintf("Cannot have %u pubkeys in bare multisig; only at most %d pubkeys", providers.size(), MAX_BARE_MULTISIG_PUBKEYS_NUM);
error = strprintf("Cannot have %u pubkeys in bare multisig; only at most %u pubkeys", providers.size(), MAX_BARE_MULTISIG_PUBKEYS_NUM);
return {};
}
}
@ -2239,6 +2239,17 @@ std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptCo
break;
}
}
// Check bare multisig pubkeys number limit
if (ctx == ParseScriptContext::TOP && providers.size() > MAX_BARE_MULTISIG_PUBKEYS_NUM) {
return nullptr;
}
// Verify we don't exceed consensus limits
if (ctx == ParseScriptContext::P2SH && script.size() > MAX_SCRIPT_ELEMENT_SIZE) {
return nullptr;
}
if (ok) return std::make_unique<MultisigDescriptor>((int)data[0][0], std::move(providers));
}
if (txntype == TxoutType::SCRIPTHASH && ctx == ParseScriptContext::TOP) {

10
src/wallet/rpc/addresses.cpp
View file

@ -292,6 +292,13 @@ RPCHelpMan addmultisigaddress()
CScript inner;
CTxDestination dest = AddAndGetMultisigDestination(required, pubkeys, output_type, provider, inner);
// Before importing the scripts into the wallet, check we can make a descriptor out of it.
std::unique_ptr<Descriptor> descriptor = InferDescriptor(GetScriptForDestination(dest), spk_man);
if (!descriptor) {
// Shouldn't happen; we crafted the destination internally.
throw JSONRPCError(RPC_INTERNAL_ERROR, "Invalid descriptor generated");
}
// Import scripts into the wallet
for (const auto& [id, script] : provider.scripts) {
// Due to a bug in the legacy wallet, the p2sh maximum script size limit is also imposed on 'p2sh-segwit' and 'bech32' redeem scripts.
@ -313,9 +320,6 @@ RPCHelpMan addmultisigaddress()
// Store destination in the addressbook
pwallet->SetAddressBook(dest, label, AddressPurpose::SEND);
// Make the descriptor
std::unique_ptr<Descriptor> descriptor = InferDescriptor(GetScriptForDestination(dest), spk_man);
UniValue result(UniValue::VOBJ);
result.pushKV("address", EncodeDestination(dest));
result.pushKV("redeemScript", HexStr(inner));

10
test/functional/rpc_decodescript.py
View file

@ -96,6 +96,16 @@ class DecodeScriptTest(BitcoinTestFramework):
assert_equal('witness_v0_scripthash', rpc_result['segwit']['type'])
assert_equal('0 ' + multisig_script_hash, rpc_result['segwit']['asm'])
# Check decoding a multisig redeem script doesn't return an invalid descriptor
self.log.info("- multisig invalid")
# Decode a +520 bytes consensus-invalid p2sh multisig - provided script translates to multi(20, keys)
large_multisig_script = '0114' + push_public_key * 20 + '0114' + 'ae'
rpc_result = self.nodes[0].decodescript(large_multisig_script)
# Verify it is valid only under segwit rules, not for bare multisig nor p2sh.
assert 'desc' not in rpc_result
assert 'p2sh' not in rpc_result
assert 'segwit' in rpc_result
self.log.info ("- P2SH")
# OP_HASH160 <Hash160(redeemScript)> OP_EQUAL.
# push_public_key_hash here should actually be the hash of a redeem script.