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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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

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

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

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.

Revision Contents
Changeset List

PathPackages
Mtest/functional/bip65-cltv-p2p.py (119 lines)

Diff 6069

View Options

test/functional/bip65-cltv-p2p.py

Loading...