Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#23113: Add warnings to createmultisig and addmu…
Browse files Browse the repository at this point in the history
…ltisig if using uncompressed keys

74c34e2 Add createmultisig and addmultisigaddress warnings release note (Samuel Dobson)
c1f9fa4 Add warnings field to addmultisigaddress to warn about uncompressed keys (Samuel Dobson)
6d8bc3c Add warnings field to createmultisig to warn about uncompressed keys (Samuel Dobson)

Pull request description:

  Fixes #21368

  Currently, if there are any uncompressed keys when calling `AddAndGetMultisigDestination`, it will just default to a legacy address regardless of the chosen `address_type`. Rather than keeping this silent behaviour which may be confusing to users, we explicitly add a `warnings` field which will warn the user why their address format is different.

ACKs for top commit:
  achow101:
    ACK 74c34e2

Tree-SHA512: c2ac7f7689251bd4fcd8c26506f053921fbaf34c7a26a74e82ebc7f82cc0bd25407fd7954bf98365dcafa51fa45dcdbee6214320580ca69509690c3555e71cc0
  • Loading branch information
MarcoFalke committed Dec 11, 2021
2 parents 97f3a99 + 74c34e2 commit db15103
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 5 deletions.
9 changes: 9 additions & 0 deletions doc/release-notes-23113.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Notable changes
===============

Updated RPCs
------------

- Both `createmultisig` and `addmultisigaddress` now include a `warnings`
field, which will show a warning if a non-legacy address type is requested
when using uncompressed public keys.
11 changes: 11 additions & 0 deletions src/rpc/misc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ static RPCHelpMan createmultisig()
{RPCResult::Type::STR, "address", "The value of the new multisig address."},
{RPCResult::Type::STR_HEX, "redeemScript", "The string value of the hex-encoded redemption script."},
{RPCResult::Type::STR, "descriptor", "The descriptor for this multisig"},
{RPCResult::Type::ARR, "warnings", /* optional */ true, "Any warnings resulting from the creation of this multisig",
{
{RPCResult::Type::STR, "", ""},
}},
}
},
RPCExamples{
Expand Down Expand Up @@ -162,6 +166,13 @@ static RPCHelpMan createmultisig()
result.pushKV("redeemScript", HexStr(inner));
result.pushKV("descriptor", descriptor->ToString());

UniValue warnings(UniValue::VARR);
if (!request.params[2].isNull() && OutputTypeFromDestination(dest) != output_type) {
// Only warns if the user has explicitly chosen an address type we cannot generate
warnings.push_back("Unable to make chosen address type, please ensure no uncompressed public keys are present.");
}
if (warnings.size()) result.pushKV("warnings", warnings);

return result;
},
};
Expand Down
12 changes: 12 additions & 0 deletions src/wallet/rpc/addresses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,10 @@ RPCHelpMan addmultisigaddress()
{RPCResult::Type::STR, "address", "The value of the new multisig address"},
{RPCResult::Type::STR_HEX, "redeemScript", "The string value of the hex-encoded redemption script"},
{RPCResult::Type::STR, "descriptor", "The descriptor for this multisig"},
{RPCResult::Type::ARR, "warnings", /* optional */ true, "Any warnings resulting from the creation of this multisig",
{
{RPCResult::Type::STR, "", ""},
}},
}
},
RPCExamples{
Expand Down Expand Up @@ -295,6 +299,14 @@ RPCHelpMan addmultisigaddress()
result.pushKV("address", EncodeDestination(dest));
result.pushKV("redeemScript", HexStr(inner));
result.pushKV("descriptor", descriptor->ToString());

UniValue warnings(UniValue::VARR);
if (!request.params[3].isNull() && OutputTypeFromDestination(dest) != output_type) {
// Only warns if the user has explicitly chosen an address type we cannot generate
warnings.push_back("Unable to make chosen address type, please ensure no uncompressed public keys are present.");
}
if (warnings.size()) result.pushKV("warnings", warnings);

return result;
},
};
Expand Down
16 changes: 11 additions & 5 deletions test/functional/rpc_createmultisig.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,19 @@ def run_test(self):
for keys in itertools.permutations([pk0, pk1, pk2]):
# Results should be the same as this legacy one
legacy_addr = node0.createmultisig(2, keys, 'legacy')['address']
assert_equal(legacy_addr, wmulti0.addmultisigaddress(2, keys, '', 'legacy')['address'])
result = wmulti0.addmultisigaddress(2, keys, '', 'legacy')
assert_equal(legacy_addr, result['address'])
assert 'warnings' not in result

# Generate addresses with the segwit types. These should all make legacy addresses
assert_equal(legacy_addr, wmulti0.createmultisig(2, keys, 'bech32')['address'])
assert_equal(legacy_addr, wmulti0.createmultisig(2, keys, 'p2sh-segwit')['address'])
assert_equal(legacy_addr, wmulti0.addmultisigaddress(2, keys, '', 'bech32')['address'])
assert_equal(legacy_addr, wmulti0.addmultisigaddress(2, keys, '', 'p2sh-segwit')['address'])
for addr_type in ['bech32', 'p2sh-segwit']:
result = wmulti0.createmultisig(2, keys, addr_type)
assert_equal(legacy_addr, result['address'])
assert_equal(result['warnings'], ["Unable to make chosen address type, please ensure no uncompressed public keys are present."])

result = wmulti0.addmultisigaddress(2, keys, '', addr_type)
assert_equal(legacy_addr, result['address'])
assert_equal(result['warnings'], ["Unable to make chosen address type, please ensure no uncompressed public keys are present."])

self.log.info('Testing sortedmulti descriptors with BIP 67 test vectors')
with open(os.path.join(os.path.dirname(os.path.realpath(__file__)), 'data/rpc_bip67.json'), encoding='utf-8') as f:
Expand Down

0 comments on commit db15103

Please sign in to comment.