[0.11] Backport BIP9 and softfork for BIP's 68,112,113 #7716

morcos commented at 3:38 PM on March 18, 2016: member

(I updated this with the new deployment dates)

BIP9 Implementation

Inspired by former implementations by Eric Lombrozo and Rusty Russell, and
based on code by Jorge Timon.

f80fe0d958

Versionbits tests a4057fa47b

Softfork status report in RPC 38ccdc4d51

Add testing of ComputeBlockVersion b3ab8e208a

Test versionbits deployments 741a06aa4d

Make GetAncestor more robust

This check for pskip != NULL was introduced in #5927 for 0.12.
It is in general safer and allows GetAncestor to be used in more places, specifically in the mining tests for the backport of BIP 68 to 0.11.

148e3ba757

[0.11] Backport BIP 68 57b7a5231e

fixup RPC test to work with 0.11 988d84691e

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.

e8067c4401

Separate CheckLockTime() and CheckSequence() logic

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

aa6973e8f4

Code style fix.

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

6f1f6ee01d

morcos renamed this:
~~[0.12] Backport BIP9 and softfork for BIP's 68,112,113~~
[0.11] Backport BIP9 and softfork for BIP's 68,112,113
on Mar 18, 2016

Add CHECKSEQUENCEVERIFY softfork through BIP9 3d46c721d3

Soft fork logic for BIP113 d18480e7e2

Soft fork logic for BIP68 cbb76c5ce7

Policy: allow transaction version 2 relay policy.

This commit introduces a way to gracefully bump the default
transaction version in a two step process.

afb99c68c5

morcos force-pushed on Mar 18, 2016

laanwj added the label Validation on Mar 18, 2016

morcos commented at 7:00 PM on March 18, 2016: member

This was tested by running bip68-1112-113-p2p.py from #7648 against the binary built in this PR.

laanwj added the label Consensus on Mar 18, 2016

btcdrak commented at 9:55 PM on March 28, 2016: contributor

@morcos shouldnt this backport include the bip68-1112-113-p2p.py tests?

morcos commented at 1:01 AM on March 29, 2016: member

@btcdrak I thought I commented about that, but now I'm not sure where I wrote that. Some of those python tests are quite annoying to backport as I discovered when backporting the bip68-sequence.py. I didn't want to mess with whatever was going to have to be done to backport the changes to the comparison test framework itself. But because of the way the comparison test works, its easy to start up the current version of the comparison test and run it against the backported binary, which is really what you want anyway.

btcdrak commented at 8:42 AM on March 29, 2016: contributor

OK. I tested the backport by compiling then switching to master and running the bip68-1112-113-p2p.py and bip9-softforks.py tests using the 0.11 binaries. All checks out. (note switching back to master requires a tree clean and rerun of autogen/configure).

ACK afb99c6

NicolasDorier commented at 11:45 AM on March 31, 2016: contributor

Not sure if big deal but no Lockpoint ? (982670c333aff6d5660c18ed00931df764733529 removeForReorg does not seem to exist in 0.11 though)

EDIT: nevermind, I forgot that 0.11 do have not the perf improvement that morcos introduced in 0.12 which assume the mempool is not corrupted, so 982670c333aff6d5660c18ed00931df764733529 is not needed, which is why test_nonzero_locks was adapted.

NicolasDorier commented at 11:56 AM on March 31, 2016: contributor

ACK afb99c68c538cb045071f6ebe0dd2b69ad82656f

btcdrak commented at 11:58 AM on March 31, 2016: contributor

Not sure if big deal but no Lockpoint ? @NicolasDorier not for 0.11. iirc, lockpoints were introduced to preserve the speed improvements to CNB() that were introduced in 0.12.

sdaftuar commented at 4:35 PM on March 31, 2016: member

ACK. Reviewed each backported commit.

Also, if anyone else is trying to verify that bip68-112-113-p2p.py from master passes when run against this 0.11 binary, note that after #7751 the RPC tests will encode decimal values as strings, which can fail when run against an 0.11 node (it wasn't until 0.12 that AmountFromValue() will accept strings). However, after reverting d7b80b54fbb73acc92ddee84697ac4cc10d4d336, the test from master passes.

afk11 commented at 1:27 PM on April 4, 2016: contributor

ACK afb99c6 - verified using sdaftuars method

morcos commented at 5:00 PM on May 18, 2016: member

Closing due to lack of interest in another 0.11 release. Seems ok not to backport CSV to 0.11. I can reopen if that changes for some reason.

morcos closed this on May 18, 2016

luke-jr commented at 6:19 PM on May 18, 2016: member

It would be a serious problem if CSV were to activate without a 0.11 release. This should just wait for the fixes in #7935

jtimon commented at 12:05 PM on May 20, 2016: contributor

If it gets the review, I don't see any reason not to release another 0.11. I mean, if people want to code it back to 0.8 (I think that's highly unlikely), I don't see the problem. Maybe just clearly warn users that they shouldn't expect further maintenance for 0.11 (I believe we already say "only expect the last 2 versions to be maintained" somewhere anyway).

DrahtBot locked this on Sep 8, 2021