Standard template for CHECKMULTISIG with 17~20 keys #13348

pull jl2012 wants to merge 2 commits into bitcoin:master from jl2012:20multisig changing 2 files +62 −12

jl2012 commented at 4:49 PM on May 30, 2018: contributor

Making CHECKMULTISIG with 17~20 keys standard. Trying to redo #11545 after #13194 is merged
sipa commented at 4:54 PM on May 30, 2018: member

I think this should be conditional on the environment (and only enabled inside SegWit scripts).
fivepiece commented at 5:02 PM on May 30, 2018: contributor

Out of interest, does the soft fork nature (or rather, change of policy only) of it means that {17,18,19} checkmultisig(verify) will still count as 20 sigops (MAX_PUBKEYS_PER_MULTISIG) ? As in here : https://github.com/bitcoin/bitcoin/blob/fd96d54f39cf4f66890e0bb40812d47e69728cec/src/script/script.cpp#L161-L167
jl2012 commented at 5:33 PM on May 30, 2018: contributor

@sipa, I think legacy CHECKMULTISIG is always restricted to up to 3 keys, and P2SH is restricted by MAX_P2SH_SIGOPS
sipa commented at 5:34 PM on May 30, 2018: member

@jl2012 I'm aware, but I'm worried about situations were Solver is called on a script that isn't found in a transaction that already passed validation.
jl2012 commented at 5:35 PM on May 30, 2018: contributor

oh.....actually, arbitrary scripts are already standard in witness v0....so this is only for wallet?
Standard template for CHECKMULTISIG with 17~20 keys 34d51f844b
Test: Standard template for CHECKMULTISIG with 17~20 keys 159e5aac91
jl2012 force-pushed on May 31, 2018
instagibbs commented at 9:45 PM on June 4, 2018: member

Yes they're already standard. The only thing preventing their use was wallet code.
in src/script/standard.cpp:60 in 34d51f844b outdated
```
  56 | @@ -57,10 +57,18 @@ static bool MatchPayToPubkeyHash(const CScript& script, valtype& pubkeyhash)
  57 |      return false;
  58 |  }
  59 |  
  60 | -/** Test for "small positive integer" script opcodes - OP_1 through OP_16. */
  61 | -static constexpr bool IsSmallInteger(opcodetype opcode)
  62 | +/** Test for "small positive integer" script opcodes - OP_1 through OP_16, or a single byte push for 17 to 20. */
```
instagibbs commented at 9:49 PM on June 4, 2018:

The old definition is used elsewhere still. Perhaps rename the function(or make a new one) to "IsCheckMultisigInteger" or something more direct?

jl2012 commented at 7:10 PM on June 5, 2018:

I think it's only used here, and is newly introduced in #13194

instagibbs commented at 7:13 PM on June 5, 2018:

I meant the term "small integer" i.e. "encode small integer" etc
jl2012 commented at 7:10 PM on June 5, 2018: contributor

@fivepiece, no, this has nothing to do with sigop counting. They are still counted as 20
DrahtBot closed this on Jul 29, 2018
DrahtBot commented at 3:16 PM on July 29, 2018: member

The last travis run for this pull request was 59 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.
DrahtBot reopened this on Jul 29, 2018
jl2012 commented at 12:30 PM on July 30, 2018: contributor

Closed in favour of #13449
jl2012 closed this on Jul 30, 2018
DrahtBot locked this on Sep 8, 2021

Contributors

Linked (view graph)

#11545 [wallet] Support CMV signing of 17~20 key template #13194 Remove template matching and pseudo opcodes #13449 [WIP] support new multisig template in wallet for Solver, signing, and signature combining