Page MenuHomePhabricator
Differential D2120

Move CLTV from scriptSig to scriptPubKey in bip65-cltv-p2p.py
ClosedPublic
Actions

Authored by Fabien on Nov 22 2018, 18:26.

Details

Reviewers
deadalnix
jasonbcox
Group Reviewers
Restricted Project
Commits
rSTAGINGa1f16b44e6ba: Move CLTV from scriptSig to scriptPubKey in bip65-cltv-p2p.py
rABCa1f16b44e6ba: Move CLTV from scriptSig to scriptPubKey in bip65-cltv-p2p.py
Summary

Prepare for magnetic anomaly with push-only rule

Depends on D2119

Test Plan
./test/functional/test_runner.py bip65-cltv-p2p

Diff Detail

Repository
rABC Bitcoin ABC
Branch
bip65_moveCLTV
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 4086
Build 6243: Bitcoin ABC Buildbot (legacy)
Build 6242: arc lint + arc unit

Event Timeline

Fabien created this revision.Nov 22 2018, 18:26
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 22 2018, 18:26
Fabien updated this revision to Diff 6028.Nov 22 2018, 18:31

Rebase

Fabien mentioned this in D2116: [qa] Fix bip65-cltv-p2p to be compatible with magnetic anomaly.Nov 22 2018, 18:35
deadalnix requested changes to this revision.Nov 22 2018, 18:49
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
test/functional/bip65-cltv-p2p.py
145 ↗(On Diff #6028)

This comment is obviously wrong. because the code following it accepts the block. I think the comment refers to the large chunk of code coming after.

This comment should mention that a block with the funding transaction is mined.

173 ↗(On Diff #6028)

Shouldn't the creation of the coinbase left to create_block ?

174 ↗(On Diff #6028)

Why do you need this version ? Doesn't create_block provide you with a proper version ?

This revision now requires changes to proceed.Nov 22 2018, 18:49
Fabien marked 2 inline comments as done.Nov 22 2018, 19:15
Fabien added inline comments.
test/functional/bip65-cltv-p2p.py
173 ↗(On Diff #6028)

create_block from blocktools does not create the coinbase

174 ↗(On Diff #6028)

create_block defaults the version to 1

Fabien updated this revision to Diff 6035.Nov 22 2018, 19:19

Update comments

teamcity added a subscriber: teamcity.Nov 22 2018, 20:24

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Nov 22 2018, 20:25

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Nov 22 2018, 20:33

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B4060: Diff 6027.Nov 22 2018, 20:33
teamcity added a comment.Nov 22 2018, 20:33

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B4061: Diff 6028.Nov 22 2018, 20:35
teamcity added a comment.Nov 22 2018, 20:37

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Nov 22 2018, 20:45

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B4066: Diff 6035.Nov 22 2018, 20:46
deadalnix accepted this revision.Nov 23 2018, 00:25
This revision is now accepted and ready to land.Nov 23 2018, 00:25
jasonbcox requested changes to this revision.Nov 23 2018, 06:24
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
test/functional/bip65-cltv-p2p.py
152 ↗(On Diff #6035)

Can't this be simplified to assert_equal(self.nodes[0].getbestblockhash(), block.hash)?

Looks like other asserts in this test can be simplified as well

210 ↗(On Diff #6035)

Ditto

This revision now requires changes to proceed.Nov 23 2018, 06:24
Fabien updated this revision to Diff 6058.Nov 23 2018, 14:29

Simplify assertions

teamcity added a comment.Nov 23 2018, 14:30

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Fabien added a child revision: D2116: [qa] Fix bip65-cltv-p2p to be compatible with magnetic anomaly.Nov 23 2018, 14:33
teamcity added a comment.Nov 23 2018, 14:39

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B4086: Diff 6058.Nov 23 2018, 14:39
jasonbcox accepted this revision.Nov 24 2018, 02:57
This revision is now accepted and ready to land.Nov 24 2018, 02:57
Closed by commit rABCa1f16b44e6ba: Move CLTV from scriptSig to scriptPubKey in bip65-cltv-p2p.py (authored by Fabien). · Explain WhyNov 24 2018, 12:55
This revision was automatically updated to reflect the committed changes.
markblundeberg mentioned this in D3080: Merge #13527: policy: Remove promiscuousmempoolflags.May 21 2019, 06:14
markblundeberg mentioned this in D3095: re-fix feature_cltv.py.May 22 2019, 23:59
Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com> mentioned this in rABC7af708e9f744: re-fix feature_cltv.py.May 23 2019, 14:15
Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com> mentioned this in rABCc08418054e83: Merge #13527: policy: Remove promiscuousmempoolflags.May 23 2019, 17:12
Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com> mentioned this in rSTAGING7af708e9f744: re-fix feature_cltv.py.Sep 18 2019, 23:41
Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com> mentioned this in rSTAGINGc08418054e83: Merge #13527: policy: Remove promiscuousmempoolflags.

Revision Contents
Changeset List

PathSize
test/
functional/
119 lines

Diff 6058

View Options

test/functional/bip65-cltv-p2p.py

Loading...