From 0c9fedfc45fa7cbd6801ca5fd756863ec9a6911c Mon Sep 17 00:00:00 2001 From: furszy Date: Mon, 21 Aug 2023 17:45:34 -0300 Subject: [PATCH] fix incorrect multisig redeem script size limit for segwit The multisig script generation process currently fails when the user exceeds 15 keys, even when it shouldn't. The maximum number of keys allowed for segwit redeem scripts (p2sh-segwit and bech32) is 20 keys. This is because the redeem script placed in the witness is not restricted by the item size limit. The reason behind this issue is the utilization of the legacy p2sh redeem script restrictions on segwit ones. Redeem scripts longer than 520 bytes are blocked from being inserted into the keystore, which causes the signing process and the descriptor inference process to fail. This occurs because the multisig generation flow uses the same keystore as the legacy spkm (FillableSigningProvider), which contains the 520-byte limit. --- src/outputtype.cpp | 8 ++++---- src/outputtype.h | 2 +- src/rpc/output_script.cpp | 3 +-- src/rpc/util.cpp | 2 +- src/rpc/util.h | 2 +- src/wallet/rpc/addresses.cpp | 12 ++++++++++-- 6 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/outputtype.cpp b/src/outputtype.cpp index c72d9deacb..8c2b76494b 100644 --- a/src/outputtype.cpp +++ b/src/outputtype.cpp @@ -81,11 +81,11 @@ std::vector GetAllDestinationsForKey(const CPubKey& key) } } -CTxDestination AddAndGetDestinationForScript(FillableSigningProvider& keystore, const CScript& script, OutputType type) +CTxDestination AddAndGetDestinationForScript(FlatSigningProvider& keystore, const CScript& script, OutputType type) { // Add script to keystore - keystore.AddCScript(script); - // Note that scripts over MAX_SCRIPT_ELEMENT_SIZE bytes are not yet supported. + keystore.scripts.emplace(CScriptID(script), script); + switch (type) { case OutputType::LEGACY: return ScriptHash(script); @@ -94,7 +94,7 @@ CTxDestination AddAndGetDestinationForScript(FillableSigningProvider& keystore, CTxDestination witdest = WitnessV0ScriptHash(script); CScript witprog = GetScriptForDestination(witdest); // Add the redeemscript, so that P2WSH and P2SH-P2WSH outputs are recognized as ours. - keystore.AddCScript(witprog); + keystore.scripts.emplace(CScriptID(witprog), witprog); if (type == OutputType::BECH32) { return witdest; } else { diff --git a/src/outputtype.h b/src/outputtype.h index a2d5942320..feef7991a6 100644 --- a/src/outputtype.h +++ b/src/outputtype.h @@ -46,7 +46,7 @@ std::vector GetAllDestinationsForKey(const CPubKey& key); * This function will automatically add the script (and any other * necessary scripts) to the keystore. */ -CTxDestination AddAndGetDestinationForScript(FillableSigningProvider& keystore, const CScript& script, OutputType); +CTxDestination AddAndGetDestinationForScript(FlatSigningProvider& keystore, const CScript& script, OutputType); /** Get the OutputType for a CTxDestination */ std::optional OutputTypeFromDestination(const CTxDestination& dest); diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp index f9343f48a8..91d4f283f0 100644 --- a/src/rpc/output_script.cpp +++ b/src/rpc/output_script.cpp @@ -143,8 +143,7 @@ static RPCHelpMan createmultisig() output_type = parsed.value(); } - // Construct using pay-to-script-hash: - FillableSigningProvider keystore; + FlatSigningProvider keystore; CScript inner; const CTxDestination dest = AddAndGetMultisigDestination(required, pubkeys, output_type, keystore, inner); diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index f683878054..435801b45b 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -224,7 +224,7 @@ CPubKey AddrToPubKey(const FillableSigningProvider& keystore, const std::string& } // Creates a multisig address from a given list of public keys, number of signatures required, and the address type -CTxDestination AddAndGetMultisigDestination(const int required, const std::vector& pubkeys, OutputType type, FillableSigningProvider& keystore, CScript& script_out) +CTxDestination AddAndGetMultisigDestination(const int required, const std::vector& pubkeys, OutputType type, FlatSigningProvider& keystore, CScript& script_out) { // Gather public keys if (required < 1) { diff --git a/src/rpc/util.h b/src/rpc/util.h index 177af90c05..6bfe414688 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -117,7 +117,7 @@ std::string HelpExampleRpcNamed(const std::string& methodname, const RPCArgList& CPubKey HexToPubKey(const std::string& hex_in); CPubKey AddrToPubKey(const FillableSigningProvider& keystore, const std::string& addr_in); -CTxDestination AddAndGetMultisigDestination(const int required, const std::vector& pubkeys, OutputType type, FillableSigningProvider& keystore, CScript& script_out); +CTxDestination AddAndGetMultisigDestination(const int required, const std::vector& pubkeys, OutputType type, FlatSigningProvider& keystore, CScript& script_out); UniValue DescribeAddress(const CTxDestination& dest); diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index bed9ec029a..bcc39b05b8 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -289,9 +289,17 @@ RPCHelpMan addmultisigaddress() output_type = parsed.value(); } - // Construct using pay-to-script-hash: + // Construct multisig scripts + FlatSigningProvider provider; CScript inner; - CTxDestination dest = AddAndGetMultisigDestination(required, pubkeys, output_type, spk_man, inner); + CTxDestination dest = AddAndGetMultisigDestination(required, pubkeys, output_type, provider, inner); + + // Import scripts into the wallet + for (const auto& [id, script] : provider.scripts) { + spk_man.AddCScript(script); + } + + // Store destination in the addressbook pwallet->SetAddressBook(dest, label, AddressPurpose::SEND); // Make the descriptor