BIP-112: Mempool-only CHECKSEQUENCEVERIFY #7524

pull btcdrak wants to merge 3 commits into bitcoin:master from btcdrak:checksequenceverify changing 8 files +248 −3

btcdrak commented at 5:29 PM on February 12, 2016: contributor
Replace NOP3 with CHECKSEQUENCEVERIFY (BIP-112)

The BIP text is at https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki

This PR follows on from #6564 and is rebased (with #7184 which has been merged top master) so can now be tested directly.

Functional test scripts by @ajtowns can be found at https://github.com/ajtowns/op_csv-test

For reviewers please note that #6564 remained unchanged for several months. The list of reviews in #6564 were as follows:
- Kanzure utACK
- rustyrussell utACK
- instagibbs utACK
- dcousens utACK
- rubensayshi utACK
- afk11 utACK
- jgarzik utACK
- greenaddress utACK
- rustyrussel Tested ACK
- ajtowns Tested ACK
- josephpoon Tested ACK
- CodeShark Tested ACK
- NicolasDorier Tested ACK
btcdrak cross-referenced this on Feb 12, 2016 from issue BIP-112: Mempool-only CHECKSEQUENCEVERIFY by maaku
morcos commented at 8:45 PM on February 12, 2016: member

My comment remains that I would prefer VerifyLockTime not be encapsulated for reuse for nLockTime and nSequence checks.
btcdrak commented at 6:39 AM on February 13, 2016: contributor

@morcos @petertodd I agree the encapsulation hampers readability. I propose something like 2fcd56f.
NicolasDorier commented at 7:22 AM on February 13, 2016: contributor

ACK 2fcd56f
instagibbs commented at 2:56 PM on February 13, 2016: member

utACK https://github.com/btcdrak/bitcoin/commit/2fcd56f70845e9c78ab3ffde38fbb11ccf33e6da

BIP112: Implement CHECKSEQUENCEVERIFY

- Replace NOP3 with CHECKSEQUENCEVERIFY (BIP112)
  <nSequence> CHECKSEQUENCEVERIFY -> <nSequence>
- Fails if txin.nSequence < nSequence, allowing funds of a txout to be locked for a number of blocks or a duration of time after its inclusion in a block.
- Pull most of CheckLockTime() out into VerifyLockTime(), a local function that will be reused for CheckSequence()
- Add bitwise AND operator to CScriptNum
- Enable CHECKSEQUENCEVERIFY as a standard script verify flag
- Transactions that fail CSV verification will be rejected from the mempool, making it easy to test the feature. However blocks containing "invalid" CSV-using transactions will still be accepted; this is *not* the soft-fork required to actually enable CSV for production use.

53e53a33c9

Separate CheckLockTime() and CheckSequence() logic

For the sake of a little repetition, make code more readable.

c3c375226e

btcdrak force-pushed on Feb 14, 2016
btcdrak commented at 7:43 PM on February 14, 2016: contributor

I've squashed to two commits. 53e53a3 corresponds to the state of #6564 as per OP and c3c3752 (was 2fcd56f) addresses request from @morcos and @petertodd.

Previous branch state for verification checksequenceverify_2fcd56f.
laanwj added the label Mempool on Feb 15, 2016
laanwj added the label TX fees and policy on Feb 15, 2016

in src/script/interpreter.cpp:None in c3c375226e outdated

1226 | +    // the nSequenceMasked in the transaction.
1227 | +    if (!(
1228 | +        (txToSequenceMasked <  CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG && nSequenceMasked <  CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG) ||
1229 | +        (txToSequenceMasked >= CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG && nSequenceMasked >= CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG)
1230 | +    ))
1231 | +        return false;

laanwj commented at 9:09 AM on February 16, 2016:

This return false here seems to come out of the blue, which could make people misread the code and introduce bugs. I'd suggest adding the )) to the line before it, or surrounding with {}.

laanwj commented at 9:24 AM on February 16, 2016: member

utACK 2fcd56f

Code style fix.

This if statement is a little obtuse and using braces here
improves readability.

a38107643f

laanwj merged this on Feb 16, 2016
laanwj closed this on Feb 16, 2016
laanwj referenced this in commit 93c85d458a on Feb 16, 2016
btcdrak cross-referenced this on Feb 16, 2016 from issue Backport BIP112 implementation for 0.12 by btcdrak
btcdrak cross-referenced this on Mar 15, 2016 from issue [0.12] Backport BIP9, BIP68 and BIP112 with softfork by btcdrak
afk11 cross-referenced this on Mar 18, 2016 from issue BIP9 versionbits softfork for BIP68, BIP112 and BIP113 by btcdrak
NicolasDorier cross-referenced this on Mar 20, 2016 from issue Test of BIP9 fork activation of mtp, csv, sequence_lock by NicolasDorier
btcdrak deleted the branch on Dec 3, 2016
meyer9 cross-referenced this on Apr 20, 2018 from issue Merge CHECKSEQUENCEVERIFY by meyer9
wqking referenced this in commit 77e685e4e7 on Apr 26, 2018
wqking referenced this in commit b63acd8e7f on Apr 26, 2018
meyer9 referenced this in commit e7321f8dff on Jun 16, 2018
meyer9 referenced this in commit 90454d2bc4 on Jun 16, 2018
meyer9 referenced this in commit e1c1ffdf0d on Jun 16, 2018
meyer9 referenced this in commit a444a5b0a8 on Jun 16, 2018
meyer9 referenced this in commit de7acaba10 on Jul 4, 2018
meyer9 referenced this in commit 4b42293ee9 on Jul 4, 2018
wqking referenced this in commit 1fde81f71d on Apr 4, 2021
wqking referenced this in commit e506572175 on Apr 4, 2021
bitcoin locked this on Sep 8, 2021

Contributors

Labels

TX fees and policy Mempool

Linked (view graph)

#6564 BIP-112: Mempool-only CHECKSEQUENCEVERIFY #7184 Implement SequenceLocks functions for BIP 68 #7543 [0.12] Backport BIP9, BIP68 and BIP112 with softfork #7544 Backport BIP112 implementation for 0.12 #7648 BIP9 versionbits softfork for BIP68, BIP112 and BIP113 #27096 script: remove unused bitwise `CScriptNum` operators