Page MenuHomePhabricator

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

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

Details

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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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

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
This revision is now accepted and ready to land.Nov 24 2018, 02:57
This revision was automatically updated to reflect the committed changes.