createmultisig with uncompressed pubkeys ignores value of addr_type enum #21368

1. 
   instagibbs commented at 7:56 am on March 5, 2021: member

   Tested in v0.21.0

   0createmultisig 1 '["04ceba29da1af96a0f2ef7cda6950b8be2baeb1adf12c0d5efebb70dbcaa086ba029cd82a0cfb8dedf65b8760cf271f2b8a50466bbf0b9339c5ffefbe2a4165326"]' p2sh-segwit
   1{
   2  "address": "3LfGXVPHtrC9A99JNiAPnygnQuPdpASvN3",
   3  "redeemScript": "514104ceba29da1af96a0f2ef7cda6950b8be2baeb1adf12c0d5efebb70dbcaa086ba029cd82a0cfb8dedf65b8760cf271f2b8a50466bbf0b9339c5ffefbe2a416532651ae",
   4  "descriptor": "sh(multi(1,04ceba29da1af96a0f2ef7cda6950b8be2baeb1adf12c0d5efebb70dbcaa086ba029cd82a0cfb8dedf65b8760cf271f2b8a50466bbf0b9339c5ffefbe2a4165326))#dyera70q"
   5}
   

   same result for:

   0createmultisig 1 '["04ceba29da1af96a0f2ef7cda6950b8be2baeb1adf12c0d5efebb70dbcaa086ba029cd82a0cfb8dedf65b8760cf271f2b8a50466bbf0b9339c5ffefbe2a4165326"]' bech32
   

   Best if this throws an error telling the user it won’t make a nonstandard address.

2. instagibbs added the label Bug on Mar 5, 2021
3. 
   theStack commented at 10:26 pm on March 7, 2021: member

   At least according to the corresponding functional test this seems to be desired behaviour: https://github.com/bitcoin/bitcoin/blob/1a4a9305c2d25eafa5ad5365b4daa2bd15fc6e05/test/functional/rpc_createmultisig.py#L81-L85 (one of the pubkeys in keys is in uncompressed form)

   I agree though that we should rather throw an error here if we can’t fulfill what the user desires, rather than silently changing the address type. Ping @achow101 who wrote this part of the test.

4. 
   achow101 commented at 10:56 pm on March 7, 2021: member

   This is the expected behavior, but I suppose it is surprising to the user. Maybe we could add a warning to the createmultisig output that informs the user what happened?
5. 
   theStack commented at 1:38 am on March 8, 2021: member

   Thanks for clarifying – then at least warning the user is a good idea. But how? The only way I see is adding a “warnings” field to the RPC result, not sure this is in-line with our RPC design guidelines.
6. 
   achow101 commented at 5:15 pm on March 8, 2021: member

   What we’ve done for other RPCs is to add a warnings field in the returned object.
7. MarcoFalke referenced this in commit ac92ab6da5 on Dec 11, 2021
8. MarcoFalke closed this on Dec 11, 2021

   ---

9. sidhujag referenced this in commit 7b5b81499d on Dec 11, 2021
10. DrahtBot locked this on Dec 11, 2022

Contributors
instagibbs theStack achow101

Labels
Bug