Page MenuHomePhabricator

[avalanche] Ensure block added to avalanche have the proper acceptance.
ClosedPublic

Authored by deadalnix on Nov 22 2018, 13:42.

Details

Summary

This ensure the state within avalanche is consistent with the outside world.

Depends on D2142

Test Plan

Adapted test cases.

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.Nov 22 2018, 13:42
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 22 2018, 13:42
deadalnix changed the visibility from "Public (No Login Required)" to "Restricted Project (Project)".Nov 22 2018, 13:42
deadalnix edited the summary of this revision. (Show Details)
deadalnix added a parent revision: Restricted Differential Revision.
deadalnix edited parent revisions, added: D2046: [avalanche] Implement the challenge/response protocol; removed: Restricted Differential Revision.Nov 22 2018, 21:42
deadalnix edited the summary of this revision. (Show Details)
deadalnix updated this revision to Diff 6042.Nov 22 2018, 21:43

Only poll blocks worth polling.

deadalnix changed the visibility from "Restricted Project (Project)" to "Public (No Login Required)".Nov 22 2018, 23:26
deadalnix added a child revision: Restricted Differential Revision.
Fabien requested changes to this revision.Nov 22 2018, 23:32
Fabien added a subscriber: Fabien.

Maybe you could also add a test to check that you won't poll an invalid block

src/test/avalanche_tests.cpp
158 ↗(On Diff #6042)

New blocks are now considered accepted

170 ↗(On Diff #6042)

Please remove the "now" from the comment

315 ↗(On Diff #6042)

It is accepted

This revision now requires changes to proceed.Nov 22 2018, 23:32
deadalnix updated this revision to Diff 6047.Nov 23 2018, 00:21

Fix comments

Fabien requested changes to this revision.Nov 23 2018, 18:37
Fabien added inline comments.
src/test/avalanche_tests.cpp
158 ↗(On Diff #6047)

nit, remove "are"

315 ↗(On Diff #6047)

I think it's accepted instead of rejected ?

This revision now requires changes to proceed.Nov 23 2018, 18:37
deadalnix updated this revision to Diff 6073.Nov 24 2018, 14:52

Fix comments

jasonbcox requested changes to this revision.Nov 25 2018, 07:27
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/test/avalanche_tests.cpp
34 ↗(On Diff #6077)

Having a really hard time reading this variable name. Is it supposed to be vraccepter?

223 ↗(On Diff #6077)

I think we should stick to zero-indexing to prevent future errors. I think AVALANCHE_FINALIZATION_SCORE - 1 reads better.

This revision now requires changes to proceed.Nov 25 2018, 07:27

The rest of these changes look good.

deadalnix marked an inline comment as done.Nov 25 2018, 17:00
deadalnix added inline comments.
src/test/avalanche_tests.cpp
223 ↗(On Diff #6077)

The index right now indicate how many confirmations were made. Zero based index represent nothing, so I fail to see haw it makes the code more meaningful in any way.

deadalnix updated this revision to Diff 6090.Nov 25 2018, 17:02

Fix vairable name

Fabien requested changes to this revision.Nov 25 2018, 19:57

Don't you want to add a test to ensure that invalid blocks are not polled ?

src/test/avalanche_tests.cpp
34 ↗(On Diff #6090)

vraccpeted => vraccepted

This revision now requires changes to proceed.Nov 25 2018, 19:57
deadalnix updated this revision to Diff 6128.Nov 27 2018, 13:05

Add a check for invalid blocks

Fabien accepted this revision.Nov 27 2018, 13:28
jasonbcox accepted this revision.Nov 27 2018, 18:07
jasonbcox added inline comments.
src/test/avalanche_tests.cpp
223 ↗(On Diff #6077)

I guess that makes sense.

This revision is now accepted and ready to land.Nov 27 2018, 18:07
This revision was automatically updated to reflect the committed changes.