Merge bitcoin/bitcoin#25220: rpc: fix incorrect warning for address type p2sh-segwit in createmultisig

3a9b9bb38e test: ensure createmultisig and addmultisigaddress are not returning any warning for expected cases (brunoerg)
eaf6f630c0 rpc: fix inappropriate warning for address type p2sh-segwit in createmultisig and addmultisigaddress (brunoerg)

Pull request description:

  Fixes #25127

  If there are any uncompressed keys when calling `AddAndGetMultisigDestination`, it will just default to a legacy address regardless of the chosen `address_type`. So, #23113 added a warnings field which will warn the user why their address format is different.

  However, when creating a multisig (p2sh-segwit), it is returning an inappropriate warning, because when getting the output type from destination (`OutputTypeFromDestination`), it returns `ScriptHash` for both legacy and `P2SH_SEGWIT`. So, since `P2SH_SEGWIT` is different from `ScriptHash`, it returns the warning:
  192d639a6b/src/rpc/output_script.cpp (L166-L169)

  So, to avoid this mistake I changed `OutputTypeFromDestination` to `descriptor->GetOutputType()` to get the appropriate output type.

ACKs for top commit:
  jonatack:
    ACK 3a9b9bb38e
  laanwj:
    Code review ACK 3a9b9bb38e

Tree-SHA512: 49f717479c2b8906277e7591ddd4747f7961c2d5c77494b5124045de9036a4277d46b9ad99279d51f0c4484284c445f1e1d3c55c49bbf0716741bad426a89369
This commit is contained in:
laanwj
2022-06-06 17:10:17 +02:00
3 changed files with 11 additions and 8 deletions

View File

@@ -163,11 +163,11 @@ static RPCHelpMan createmultisig()
result.pushKV("descriptor", descriptor->ToString());
UniValue warnings(UniValue::VARR);
if (!request.params[2].isNull() && OutputTypeFromDestination(dest) != output_type) {
if (descriptor->GetOutputType() != 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);
if (!warnings.empty()) result.pushKV("warnings", warnings);
return result;
},

View File

@@ -302,11 +302,11 @@ RPCHelpMan addmultisigaddress()
result.pushKV("descriptor", descriptor->ToString());
UniValue warnings(UniValue::VARR);
if (!request.params[3].isNull() && OutputTypeFromDestination(dest) != output_type) {
if (descriptor->GetOutputType() != 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);
if (!warnings.empty()) result.pushKV("warnings", warnings);
return result;
},

View File

@@ -91,15 +91,17 @@ class RpcCreateMultiSigTest(BitcoinTestFramework):
assert 'warnings' not in result
# Generate addresses with the segwit types. These should all make legacy addresses
err_msg = ["Unable to make chosen address type, please ensure no uncompressed public keys are present."]
for addr_type in ['bech32', 'p2sh-segwit']:
result = self.nodes[0].createmultisig(2, keys, addr_type)
result = self.nodes[0].createmultisig(nrequired=2, keys=keys, address_type=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."])
assert_equal(result['warnings'], err_msg)
if self.is_bdb_compiled():
result = wmulti0.addmultisigaddress(2, keys, '', addr_type)
result = wmulti0.addmultisigaddress(nrequired=2, keys=keys, address_type=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."])
assert_equal(result['warnings'], err_msg)
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:
@@ -173,6 +175,7 @@ class RpcCreateMultiSigTest(BitcoinTestFramework):
desc = descsum_create(desc)
msig = node2.createmultisig(self.nsigs, self.pub, self.output_type)
assert 'warnings' not in msig
madd = msig["address"]
mredeem = msig["redeemScript"]
assert_equal(desc, msig['descriptor'])