Page MenuHomePhabricator

Remove topological ordering constraint from block starting Nov, 15 2018
ClosedPublic

Authored by deadalnix on Jun 7 2018, 08:51.

Details

Summary

As per title.

Depends on D1484

Test Plan

Added an integration test checking for the proper activation of this feature.

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

deadalnix created this revision.Jun 7 2018, 08:51
Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 7 2018, 08:51
Mengerian requested changes to this revision.Jun 8 2018, 05:28
Mengerian added a subscriber: Mengerian.
Mengerian added inline comments.
test/functional/abc-transaction-ordering.py
12 ↗(On Diff #4082)

This comment is confusing.

Seems like it should be changed to mention transaction ordering?

This revision now requires changes to proceed.Jun 8 2018, 05:28
deadalnix updated this revision to Diff 4100.Jun 9 2018, 15:32

Fix test description comment

Mengerian accepted this revision as: Mengerian.Jun 11 2018, 00:15
jasonbcox requested changes to this revision.Jun 17 2018, 01:59
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
test/functional/abc-transaction-ordering.py
50 ↗(On Diff #4100)

I see this used in a few tests, but don't see it used anywhere. What does it do?

90 ↗(On Diff #4100)

engough -> enough

227 ↗(On Diff #4100)

acivate -> activate

240 ↗(On Diff #4100)

Actiavte -> Activate

249 ↗(On Diff #4100)

Isn't block 4444 already out of order? 4444 doesn't look like it was made to be in-order, so I think this line is redundant.

This revision now requires changes to proceed.Jun 17 2018, 01:59
deadalnix added inline comments.Jun 18 2018, 12:22
test/functional/abc-transaction-ordering.py
50 ↗(On Diff #4100)

It's not used, removing.

249 ↗(On Diff #4100)

Block 4444 is before the feature activates and is rejected.

deadalnix updated this revision to Diff 4130.Jun 18 2018, 12:27

Fix comments

jasonbcox accepted this revision.Jun 19 2018, 14:34
jasonbcox added inline comments.
test/functional/abc-transaction-ordering.py
249 ↗(On Diff #4100)

We cleared this up offline. For posterity: the block number *is not* height. The height is the order they're generated. I think the numbers can be replaced with variable names instead, but I'll take care of that improvement later.

This revision is now accepted and ready to land.Jun 19 2018, 14:34
This revision was automatically updated to reflect the committed changes.