Skip to content

Commit 53302a0

Browse files
committed
bugfix: addmultisigaddress, add unsupported operation for redeem scripts over 520 bytes
Due to a bug in the legacy wallet, the p2sh maximum script size limit is also imposed on 'p2sh-segwit' and 'bech32' redeem scripts. Although redeem scripts over MAX_SCRIPT_ELEMENT_SIZE bytes are technically valid for segwit output types, we don't want to enable this feature in legacy wallets for the following reasons: 1) It introduces a compatibility-breaking change requiring downgrade protection; older wallets would be unable to interact with these "new" legacy wallets. 2) Considering the ongoing deprecation of the legacy spkm, this issue adds another good reason to transition towards descriptors.
1 parent 9be6065 commit 53302a0

File tree

1 file changed

+14
-1
lines changed

1 file changed

+14
-1
lines changed

src/wallet/rpc/addresses.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,20 @@ RPCHelpMan addmultisigaddress()
296296

297297
// Import scripts into the wallet
298298
for (const auto& [id, script] : provider.scripts) {
299-
spk_man.AddCScript(script);
299+
// Due to a bug in the legacy wallet, the p2sh maximum script size limit is also imposed on 'p2sh-segwit' and 'bech32' redeem scripts.
300+
// Even when redeem scripts over MAX_SCRIPT_ELEMENT_SIZE bytes are valid for segwit output types, we don't want to
301+
// enable it because:
302+
// 1) It introduces a compatibility-breaking change requiring downgrade protection; older wallets would be unable to interact with these "new" legacy wallets.
303+
// 2) Considering the ongoing deprecation of the legacy spkm, this issue adds another good reason to transition towards descriptors.
304+
if (script.size() > MAX_SCRIPT_ELEMENT_SIZE) throw JSONRPCError(RPC_WALLET_ERROR, "Unsupported multisig script size for legacy wallet. Upgrade to descriptors to overcome this limitation for p2sh-segwit or bech32 scripts");
305+
306+
if (!spk_man.AddCScript(script)) {
307+
if (CScript inner_script; spk_man.GetCScript(CScriptID(script), inner_script)) {
308+
CHECK_NONFATAL(inner_script == script); // Nothing to add, script already contained by the wallet
309+
continue;
310+
}
311+
throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Error importing script into the wallet"));
312+
}
300313
}
301314

302315
// Store destination in the addressbook

0 commit comments

Comments
 (0)