bitcoin/bitcoin — pull request #3843 by petertodd (2014-03-11 05:55:53+00:00)
Title: Fix checkmultisig mutability and remove P2SH CHECKMULTISIG # of signatures limit

tl;dr: CHECKMULTISIG has a bug that causes it to consume an extra dummy argument on the stack. While normally OP_0 is used, the actual value of this argument is not checked, and thus creates a source of mutability for any transaction with CHECKMULTISIG inputs.

So to fix this we do a few things:

1) Introduce the notion of mandatory and standard script verification flags. The former is what all *new* blocks must adhere to - currently just P2SH - while the latter are soft restrictions that may later become mandatory in a soft-fork such as signature encoding checks. (the canonical PUSHDATA checks should become a verification flag as well in a future pull-req)

2) Modify tx_(in)valid unit tests to allow verification flags to be specified by name rather than only P2SH. (again, tests of the STRICTENC flags should be added in a future pull-req)

3) Add the SCRIPT_VERIFY_NULLDUMMY flag and corresponding code to EvalScript() to verify that the dummy argument is in fact null. Rational: checking this as a IsStandard() test would be awkward, and a future soft-fork should make this mandatory anyway. I also added more unittests, including an explicit check to tx_invalid.json that a lack of a dummy argument *does* fail the transaction. Tested that blocks breaking this rule are accepted, including a testnet reindex. (mainnet reindex pending)

4) Generalize the SCRIPTENC DoS-ban exemption to all non-mandatory SCRIPT_VERIFY flags so that we don't split the network. Note the minor logic error we fixed: previously an invalid P2SH transaction with the inner script failing was classified as REJECT_NONSTANDARD with the message "non-canonical"; a future pull-req can make the new messages more useful if EvalScript() is modified to return rejection strings or similar. (or god forbid, something like the scary-complex exception based scheme in my python-bitcoinlib that'll surely add a consensus bug) Tested by injecting non-compliant transactions elsewhere on the network and observing debug.log

With the dummy argument checked, we can remove the weird arbitrary limits on the # of signatures by upping the allowed scriptSig size, letting advanced escrow/oracle/etc. applications do as they wish within the 15 pubkey/520 byte redeemScript limit with P2SH. (see https://github.com/bitcoin/bips/pull/24) With the dummy value forced to always be null, the "stuff data in the chain" situation remains essentially unchanged as all the extra scriptSig space can only be used for signatures; the non-P2SH limit is kept as-is and remains <= 3 pubkeys.

Finally, now is a good time to teach addmultisigaddress/createmultisig/the wallet to raise an error on >520 byte redeemScripts.