Don't return the address of a P2SH of a P2SH #8845

pull jnewbery wants to merge 1 commits into bitcoin:master from jnewbery:trivial-P2SH-P2SH changing 1 files +10 −2

jnewbery commented at 9:48 PM on September 29, 2016: member
https://github.com/bitcoin/bitcoin/commit/be066fad7d2a8f95d50667262f255ce67f49b51a added an RPC to decode a hex input script. The returned object is:
```
{
  "asm":"asm",   (string) Script public key
  "hex":"hex",   (string) hex encoded public key
  "type":"type", (string) The output type
  "reqSigs": n,    (numeric) The required signatures
  "addresses": [   (json array of string)
     "address"     (string) bitcoin address
     ,...
  ],
  "p2sh","address" (string) script address.
}
```
That "p2sh" return value is the address of a P2SH with the hex input script as the redeem script.

If the input script is already a P2SH script, this doesn't make sense (we can't wrap a P2SH in a P2SH). Only return a P2SH address if the input script is not a P2SH script.
MarcoFalke added the label RPC/REST/ZMQ on Sep 29, 2016

in src/rpc/rawtransaction.cpp:None in 925a8be829 outdated

 553 | @@ -554,7 +554,14 @@ UniValue decodescript(const UniValue& params, bool fHelp)
 554 |      }
 555 |      ScriptPubKeyToJSON(script, r, false);
 556 |  
 557 | -    r.push_back(Pair("p2sh", CBitcoinAddress(CScriptID(script)).ToString()));
 558 | +    UniValue type;
 559 | +    type = find_value(r, "type");
 560 | +
 561 | +    if ((!type.isNull()) && (type.isStr()) && (type.get_str() != "scripthash"))

ryanofsky commented at 7:52 PM on October 3, 2016:

Should probably drop the isNull check because a value can't be both a string and a null at the same time.

Also maybe drop the parens around && conditions (searching for "&&" in this file shows no parens used in the other cases).

jnewbery commented at 11:25 PM on October 3, 2016:

Thanks. Both done.

jnewbery force-pushed on Oct 3, 2016
jnewbery commented at 11:26 PM on October 3, 2016: member

@ryanofsky's suggestions added and rebased.
sipa commented at 12:42 PM on October 4, 2016: member

utACK
luke-jr commented at 11:17 AM on October 5, 2016: member

"Incorporated suggested changes from @ryanofsky." doesn't belong in the commit description (it will ping him every time the commit goes somewhere..)
MarcoFalke commented at 11:22 AM on October 5, 2016: member

Agree. Also please don't put irrelevant information in the commit subject line.
jnewbery force-pushed on Oct 5, 2016
Don't return the address of a P2SH of a P2SH. d51f182461
jnewbery force-pushed on Oct 5, 2016
jnewbery commented at 12:58 PM on October 5, 2016: member

Fixed commit description.
laanwj commented at 2:46 PM on October 11, 2016: member

utACK

in src/rpc/rawtransaction.cpp:None in d51f182461

 553 | @@ -554,7 +554,15 @@ UniValue decodescript(const UniValue& params, bool fHelp)
 554 |      }
 555 |      ScriptPubKeyToJSON(script, r, false);
 556 |  
 557 | -    r.push_back(Pair("p2sh", CBitcoinAddress(CScriptID(script)).ToString()));
 558 | +    UniValue type;
 559 | +    type = find_value(r, "type");
 560 | +
 561 | +    if (type.isStr() && type.get_str() != "scripthash") {

laanwj commented at 2:51 PM on October 11, 2016:

I think looking at the generated JSON here instead of the script object itself is a bit confusing, but I tend to agree it's the best way to go here to not have to repeat ExtractDestinations.

laanwj merged this on Oct 18, 2016
laanwj closed this on Oct 18, 2016
laanwj referenced this in commit 6e094e54f7 on Oct 18, 2016
jnewbery deleted the branch on Oct 18, 2016
luke-jr referenced this in commit 103c724d82 on Oct 20, 2016
laanwj referenced this in commit 71ed4e3e66 on Oct 25, 2016
laanwj referenced this in commit 1d048b917b on Oct 27, 2016
schinzelh referenced this in commit 2354e50f84 on Oct 23, 2017
DrahtBot locked this on Sep 8, 2021

Contributors

Labels