Implement excessive sighashing protection policy with tight sighash estimation #8755
Conversation
|
@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
the
sighashpolicy
branch
from
613c9bf to
04d9e6f
Compare
src/primitives/transaction.cpp
Outdated
|
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
the
sighashpolicy
branch
3 times, most recently
from
7dd8315 to
3f87dc8
Compare
jl2012
changed the title
|
Unit tests are completed and related BIP updated |
jl2012
force-pushed
the
sighashpolicy
branch
2 times, most recently
from
99542d9 to
801c9b1
Compare
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.
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.
jl2012
force-pushed
the
sighashpolicy
branch
2 times, most recently
from
ce76710 to
62d471e
Compare
jl2012
force-pushed
the
sighashpolicy
branch
from
62d471e to
aa8c275
Compare
|
Removing 0.14 tag as discussed in today's meeting |
|
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? |
|
@TheBlueMatt : sure, I'll make another PR just for the |
|
@jl2012 What's the status here? |
|
@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) |
|
|
||
| // Making OP_CODESEPARATOR and FindAndDelete non-standard in non-segwit scripts | ||
| // | ||
| SCRIPT_VERIFY_CONST_SCRIPTCODE = (1U << 16), |
There was a problem hiding this comment.
Does adding a new constant require the tests to be updated as well? See ValidateCheckInputsForAllFlags
7 participants
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