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

Merged
merged 1 commit into from Oct 18, 2016

Projects

None yet

6 participants

@jnewbery
Contributor

be066fa 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.

src/rpc/rawtransaction.cpp
+ UniValue type;
+ type = find_value(r, "type");
+
+ if ((!type.isNull()) && (type.isStr()) && (type.get_str() != "scripthash"))
@ryanofsky
ryanofsky Oct 3, 2016 Contributor

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
jnewbery Oct 3, 2016 Contributor

Thanks. Both done.

@jnewbery
Contributor
jnewbery commented Oct 3, 2016

@ryanofsky's suggestions added and rebased.

@sipa
Member
sipa commented Oct 4, 2016

utACK

@luke-jr
Member
luke-jr commented Oct 5, 2016

"Incorporated suggested changes from @ryanofsky." doesn't belong in the commit description (it will ping him every time the commit goes somewhere..)

@MarcoFalke
Member

Agree. Also please don't put irrelevant information in the commit subject line.

@jnewbery
Contributor
jnewbery commented Oct 5, 2016

Fixed commit description.

@laanwj
Member
laanwj commented Oct 11, 2016

utACK

+ UniValue type;
+ type = find_value(r, "type");
+
+ if (type.isStr() && type.get_str() != "scripthash") {
@laanwj
laanwj Oct 11, 2016 Member

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 laanwj merged commit d51f182 into bitcoin:master Oct 18, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@laanwj laanwj added a commit that referenced this pull request Oct 18, 2016
@laanwj laanwj Merge #8845: Don't return the address of a P2SH of a P2SH
d51f182 Don't return the address of a P2SH of a P2SH. (jnewbery)
6e094e5
@jnewbery jnewbery deleted the jnewbery:trivial-P2SH-P2SH branch Oct 18, 2016
@luke-jr luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Oct 20, 2016
@jnewbery @luke-jr jnewbery + luke-jr Bugfix: Don't return the address of a P2SH of a P2SH
Github-Pull: #8845
Rebased-From: cef66cb
103c724
@luke-jr luke-jr referenced this pull request Oct 25, 2016
Merged

0.13.2 backports #9011

@laanwj laanwj added a commit to laanwj/bitcoin that referenced this pull request Oct 25, 2016
@jnewbery @laanwj jnewbery + laanwj Don't return the address of a P2SH of a P2SH.
Github-Pull: #8845
Rebased-From: d51f182
71ed4e3
@laanwj laanwj added a commit to laanwj/bitcoin that referenced this pull request Oct 27, 2016
@jnewbery @laanwj jnewbery + laanwj Don't return the address of a P2SH of a P2SH.
Github-Pull: #8845
Rebased-From: d51f182
1d048b9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment