Implement excessive sighashing protection policy with tight sighash estimation #8755

pull jl2012 wants to merge 4 commits into bitcoin:master from jl2012:sighashpolicy changing 18 files +574 −7

jl2012 commented at 7:17 PM on September 18, 2016: contributor

This implements a static estimation of sighash size for a transaction. A transaction with more than 90bytes of sighash per weight is non-standard. This is equivalent to 36MB for an 100kB non-segwit transaction, or 360MB for a block in the worst case. All transactions below 100kB with legitimate use of CHECK(MULTI)SIG should remain standard with this limit.

The estimation of sighash is based on the following 3 assumptions: a. OP_CODESEPARATOR and FindAndDelete are disabled by SCRIPT_VERIFY_CONST_SCRIPTCODE. This ensures that the scriptCode serialized in SignatureHash is always the same as the original script passing to the EvalScript. (part of this PR) b. SignatureHash is performed once only for each SIGHASH type. (#8654) c. Only 6 sighash types are allowed: ALL, NONE, SINGLE, and combinations with ANYONECANPAY (already enforced as policy with STRICTENC)

Todo: unit tests
jl2012 commented at 7:21 PM on September 18, 2016: contributor

@TheBlueMatt As you suggested, we could be more aggressive when disabling FindAndDelete. So eventually we may retire this function after a softfork.
jl2012 force-pushed on Sep 18, 2016
jl2012 cross-referenced this on Sep 19, 2016 from issue Implement excessive sighashing protection policy with loose sighash estimation by jl2012
jl2012 cross-referenced this on Sep 19, 2016 from issue [WIP] Reuse sighash computations across evaluation by jl2012

in src/primitives/transaction.cpp:None in 04d9e6f5fa outdated

 162 | +        size -= scriptSigSize;
 163 | +        // If the scriptSig size is larger than 252, 2 bytes compactSize encoding is deducted.
 164 | +        if (scriptSigSize > 252)
 165 | +            size -= 2;
 166 | +        /*
 167 | +         * // scriptSig larger than 10000 btyes is invalid

instagibbs commented at 1:50 PM on September 19, 2016:

dead code

jl2012 commented at 7:35 PM on September 29, 2016:

removed

laanwj added the label P2P on Sep 21, 2016
jl2012 commented at 6:03 PM on September 21, 2016: contributor

A draft BIP is made for the detailed rationale of this PR: https://github.com/jl2012/bips/blob/sighash/bip-sighash.mediawiki
jl2012 force-pushed on Sep 29, 2016
jl2012 force-pushed on Sep 30, 2016
jl2012 force-pushed on Sep 30, 2016
jl2012 renamed this:
~~Implement excessive sighashing protection policy~~
Implement excessive sighashing protection policy with tight sighash estimation
on Sep 30, 2016
jl2012 commented at 10:36 AM on September 30, 2016: contributor

Unit tests are completed and related BIP updated
jl2012 force-pushed on Oct 27, 2016
jl2012 force-pushed on Oct 28, 2016

Add constant scriptCode policy in non-segwit scripts

This disables OP_CODESEPARATOR in non-segwit scripts (even in an unexecuted branch), and makes a positive FindAndDelete result invalid. This ensures that the scriptCode serialized in SignatureHash is always the same as the script passing to the EvalScript.

98416d6462

Add sighash limitation policy

This implements a static estimation of sighash size for a transaction. A transaction with more than 90bytes of sighash per weight is non-standard. This is equivalent to 36MB for an 100kB non-segwit transaction, or 360MB for a block in the worst case. All transactions below 100kB with legitimate use of CHECK(MULTI)SIG should remain standard with this limit.

e81ecd2ebe

Add transaction tests for constant scriptCode 67f4eba3ec
jl2012 force-pushed on Dec 22, 2016
jl2012 force-pushed on Dec 22, 2016
Test sighash limit policy aa8c27515c
jl2012 force-pushed on Dec 22, 2016
jonasschnelli added this to the milestone 0.14.0 on Dec 22, 2016
laanwj removed this from the milestone 0.14.0 on Jan 12, 2017
laanwj commented at 7:27 PM on January 12, 2017: member

Removing 0.14 tag as discussed in today's meeting
TheBlueMatt commented at 6:19 PM on September 29, 2017: contributor

Strong Concept ACK on at least making CODESEPARATOR and FindAndDelete non-standard. Can we push forward on that independantly to get it done sooner rather than later, then at least the sighash limits can be reviewed separately and are more straight-forward? Any plans to rebase this @jl2012?
jl2012 commented at 9:18 PM on September 29, 2017: contributor

@TheBlueMatt : sure, I'll make another PR just for the SCRIPT_VERIFY_CONST_SCRIPTCODE policy
jl2012 cross-referenced this on Sep 29, 2017 from issue [Policy] Several transaction standardness rules by jl2012
ajtowns cross-referenced this on Dec 10, 2017 from issue Add Travis check for unused Python imports by practicalswift
sipa commented at 5:09 PM on March 6, 2018: member

@jl2012 What's the status here?
jl2012 commented at 6:56 PM on March 6, 2018: contributor

@sipa: this requires #8654 and #11423. But #8654 needs to maintain a cache of 256 32-bytes hashes per input which might impact validation. It could be reduced to 6 hashes/input if we softfork away those 250 non-std nHashType.

The alternative is #8756, which does not require #8654 and #11423. But the counting is more conservative (overestimating)

in src/script/interpreter.h:112 in aa8c27515c

 105 | @@ -106,6 +106,10 @@ enum
 106 |      // Public keys in segregated witness scripts must be compressed
 107 |      //
 108 |      SCRIPT_VERIFY_WITNESS_PUBKEYTYPE = (1U << 15),
 109 | +
 110 | +    // Making OP_CODESEPARATOR and FindAndDelete non-standard in non-segwit scripts
 111 | +    //
 112 | +    SCRIPT_VERIFY_CONST_SCRIPTCODE = (1U << 16),

MarcoFalke commented at 7:37 PM on June 5, 2018:

Does adding a new constant require the tests to be updated as well? See ValidateCheckInputsForAllFlags

MarcoFalke added the label Needs rebase on Jun 6, 2018
jl2012 closed this on Sep 4, 2018
fanquake removed this from the "In progress" column in a project

laanwj removed the label Needs rebase on Oct 24, 2019
bitcoin locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#8654 [WIP] Reuse sighash computations across evaluation #8756 Implement excessive sighashing protection policy with loose sighash estimation #11423 [Policy] Several transaction standardness rules #11835 Add Travis check for unused Python imports