Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[0.12] Backport BIP9, BIP68 and BIP112 with softfork #7543

Merged
merged 23 commits into from Apr 4, 2016

Conversation

btcdrak
Copy link
Contributor

@btcdrak btcdrak commented Feb 16, 2016

Backport of #7184 (BIP68), #7524 (BIP112), #7187 (lockpoints), #7575 (BIP9) and #7648 (softfork) for 0.12.

EDIT: added #7531 (one line add test to extendedTests array and trivial comment edit #7773

@morcos
Copy link
Member

morcos commented Feb 19, 2016

ACK on this being the correct backport, but I would prefer to see 1 pull with the soft fork logic and all accompanying implementations backported at the same time. I think it would be much safer to test and review it together like that. Lets wait until we have the soft fork logic finished and merged.

@btcdrak btcdrak changed the title Backport BIP68 implementation for 0.12 Backport BIP68 and BIP112 implementation for 0.12 Mar 15, 2016
@btcdrak btcdrak changed the title Backport BIP68 and BIP112 implementation for 0.12 Backport BIP68 and BIP112 mempool logic for 0.12 Mar 15, 2016
@btcdrak btcdrak changed the title Backport BIP68 and BIP112 mempool logic for 0.12 [0.12] Backport BIP68 and BIP112 mempool only Mar 15, 2016
@btcdrak
Copy link
Contributor Author

btcdrak commented Mar 16, 2016

I added #7187 to this backport.

morcos and others added 14 commits March 18, 2016 09:14
SequenceLocks functions are used to evaluate sequence lock times or heights per BIP 68.

The majority of this code is copied from maaku in bitcoin#6312
Further credit: btcdrak, sipa, NicolasDorier
it boggles the mind why these nits can't be delivered on a more timely basis
- 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.
For the sake of a little repetition, make code more readable.
This if statement is a little obtuse and using braces here
improves readability.
Obtain LockPoints to store in CTxMemPoolEntry and during a reorg, evaluate whether they are still valid and if not, recalculate them.
Inspired by former implementations by Eric Lombrozo and Rusty Russell, and
based on code by Jorge Timon.
sipa and others added 5 commits March 18, 2016 09:28
This commit introduces a way to gracefully bump the default
transaction version in a two step process.
This RPC test will test both the activation mechanism of the first versionbits soft fork as well as testing many code branches of the consensus logic for BIP's 68, 112, and 113.
@btcdrak btcdrak changed the title [0.12] Backport BIP68 and BIP112 mempool only [0.12] Backport BIP9, BIP68 and BIP112 with softfork Mar 18, 2016
@morcos
Copy link
Member

morcos commented Mar 18, 2016

ACK 3a99feb (backported myself and got the same code)

@sipa
Copy link
Member

sipa commented Mar 28, 2016

Mechanical backport check ACK 26e9a05

Ran the following script:

while read F L; do git show $F | egrep -v '^index ' | egrep -v '^commit ' | egrep -v '^@@ ' >/tmp/f; git show $L | egrep -v '^index ' | egrep -v '^commit ' | egrep -v '^@@ ' >/tmp/l; diff /tmp/f /tmp/l; done

with input:
12c89c9 159ee3d
02c2435 9713ed3
478fba6 648be9b
65751a3 ee40924
d23f6c6 0bdaacd
6851107 6f83cf2
982670c ade85e1
a381076 c8d309e
c3c3752 6170506
53e53a3 c0c5e09
b043c4b 197c376
15ba08c c6c2f0f

(the corresponding non-test commits)

The output is:

29c29
<              return state.DoS(10, false, REJECT_INVALID, "bad-txns-nonfinal", false, "non-final transaction");
---
>              return state.DoS(10, error("%s: contains a non-final transaction", __func__), REJECT_INVALID, "bad-txns-nonfinal");
68,70c68,70
< diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
< --- a/src/rpc/blockchain.cpp
< +++ b/src/rpc/blockchain.cpp
---
> diff --git a/src/rpcblockchain.cpp b/src/rpcblockchain.cpp
> --- a/src/rpcblockchain.cpp
> +++ b/src/rpcblockchain.cpp
40,42c40,42
< diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
< --- a/src/rpc/blockchain.cpp
< +++ b/src/rpc/blockchain.cpp
---
> diff --git a/src/rpcblockchain.cpp b/src/rpcblockchain.cpp
> --- a/src/rpcblockchain.cpp
> +++ b/src/rpcblockchain.cpp
12d11
<    utilmoneystr.h \
14a14
>    version.h \
18c18
<    wallet/rpcwallet.h \
---
>    wallet/wallet.h \

which is good, as there are no lines starting with '+' or '-' that changed between the diffs.

@instagibbs
Copy link
Member

26e9a05 ACK, backport matches

@btcdrak
Copy link
Contributor Author

btcdrak commented Mar 29, 2016

cherry-picked #7531 6ba8b2a as caf1381 - one line including bit68-sequence.py in extended tests array.

@sdaftuar
Copy link
Member

ACK caf1381

@jl2012
Copy link
Contributor

jl2012 commented Mar 30, 2016

backport matches, tACK caf1381

@btcdrak
Copy link
Contributor Author

btcdrak commented Mar 31, 2016

Added trivial comment edit from #7773 as c270b62

@petertodd
Copy link
Contributor

utACK

self.height = 3 # height of the next block to build
self.tip = int ("0x" + self.nodes[0].getbestblockhash() + "L", 0)
self.nodeaddress = self.nodes[0].getnewaddress()
self.last_block_time = time.time()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should explicitly be int. Sadly, py2 will silently cast the float to int where needed.

@maflcko
Copy link
Member

maflcko commented Apr 2, 2016

utACK c270b62. My nits can be fixed later

@NicolasDorier
Copy link
Contributor

utACK, checked with sipa's script

@@ -181,7 +181,7 @@ def test_sequence_lock_confirmed_inputs(self):
value += utxos[j]["amount"]*COIN
# Overestimate the size of the tx - signatures should be less than 120 bytes, and leave 50 for the output
tx_size = len(ToHex(tx))/2 + 120*num_inputs + 50
tx.vout.append(CTxOut(value-self.relayfee*tx_size*COIN/1000, CScript([b'a'])))
tx.vout.append(int(CTxOut(value-self.relayfee*tx_size*COIN/1000, CScript([b'a']))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be surprised if this worked. (You are passing the CTxOut into int())

Edit: You could just try a git cherry-pick ...; git reset HEAD~; git commit qa/rpc-tests/bip*

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@btcdrak
Copy link
Contributor Author

btcdrak commented Apr 3, 2016

@MarcoFalke I cherry-picked your int/float fixes for RPC tests, from #7778 commit fa2cea1 in 640666b

@maflcko
Copy link
Member

maflcko commented Apr 3, 2016

utACK 640666b

@laanwj laanwj merged commit 640666b into bitcoin:0.12 Apr 4, 2016
laanwj added a commit that referenced this pull request Apr 4, 2016
640666b [qa] rpc-tests: Properly use integers, floats (BtcDrak)
c270b62 Fix comments in tests (BtcDrak)
caf1381 Add bip68-sequence.py to extended rpc tests (BtcDrak)
26e9a05 Test of BIP9 fork activation of mtp, csv, sequence_lock (NicolasDorier)
3a99feb Add RPC test for BIP 68/112/113 soft fork. (Alex Morcos)
159ee3d Policy: allow transaction version 2 relay policy. (BtcDrak)
9713ed3 Soft fork logic for BIP68 (BtcDrak)
648be9b Soft fork logic for BIP113 (BtcDrak)
ee40924 Add CHECKSEQUENCEVERIFY softfork through BIP9 (Pieter Wuille)
6ff0b9f RPC test for BIP9 warning logic (Suhas Daftuar)
0710b30 Test versionbits deployments (Suhas Daftuar)
8ebc6f2 Add testing of ComputeBlockVersion (Suhas Daftuar)
0bdaacd Softfork status report in RPC (Pieter Wuille)
5f90d4e Versionbits tests (Pieter Wuille)
6f83cf2 BIP9 Implementation (Pieter Wuille)
ade85e1 Add LockPoints (Alex Morcos)
c8d309e Code style fix. (BtcDrak)
6170506 Separate CheckLockTime() and CheckSequence() logic (BtcDrak)
c0c5e09 BIP112: Implement CHECKSEQUENCEVERIFY (Mark Friedenbach)
197c376 fix sdaftuar's nits again (Alex Morcos)
0a79c04 Bug fix to RPC test (Alex Morcos)
0d09af7 Add RPC test exercising BIP68 (mempool only) (Suhas Daftuar)
15ba08c Implement SequenceLocks functions (Alex Morcos)
@afk11
Copy link
Contributor

afk11 commented Apr 4, 2016

ACK 3a99feb

@btcdrak btcdrak deleted the dot12_backport_bip68 branch December 3, 2016 10:57
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet