Update standardness rules congruent to Bitcoin Core #2178
Conversation
Pull Request Test Coverage Report for Build 9180393402Details
💛 - Coveralls |
Roasbeef
left a comment
There was a problem hiding this comment.
Nice PR, there're a few lingering policy rules bitcoind applies that we don't, so nice to be able to periodically bridge the gap. Hopefully one day this'll all be in a nice BIP to make it easier to track relevant policy that full node implementations should consider.
Left a few comments in line
| } | ||
| } | ||
|
|
||
| // Check that if the witness flag is set that we actually have |
There was a problem hiding this comment.
Nice, this is something that is actually spelled out in the old BIP (144):
If the witness is empty, the old serialization format must be used.
https://github.com/bitcoin/bips/blob/master/bip-0144.mediawiki
|
|
||
| // ScriptVerifyConstScriptCode fails non-segwit scripts if a signature | ||
| // match is found in the script code or if OP_CODESEPARATOR is used. | ||
| ScriptVerifyConstScriptCode |
There was a problem hiding this comment.
+1 for aligning here, this wasn't in the original set of BIPs, but was added afterwards: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2017-November/015292.html
Being closer to bitcoind here policy wise may make it easier to keep up with w/e the outcome of the Great Consensus Cleanup Revival is.
778dad0 to
d7a021b
Compare
d7a021b to
e7c694a
Compare
This is to mitigate CVE-2017-12842. Along the way, also error when deserializing transactions that have the witness marker flag set but have no witnesses. This matches Bitcoin Core's behaviour initially introduced here bitcoin/bitcoin#14039. Allowing such transactions is benign, but this makes sure that our parsing code matches Core's exactly.
e7c694a to
da2f3b1
Compare
This PR updates a couple of relay policy rules to bring them in line with what Bitcoin Core does. We do two things here,
References: