Page MenuHomePhabricator

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

Authored by deadalnix on Thu, Nov 22, 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.Thu, Nov 22, 13:42
Herald added a reviewer: Restricted Project. · View Herald TranscriptThu, Nov 22, 13:42
deadalnix changed the visibility from "Public (No Login Required)" to "Restricted Project (Project)".Thu, Nov 22, 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.Thu, Nov 22, 21:42
deadalnix edited the summary of this revision. (Show Details)
deadalnix updated this revision to Diff 6042.Thu, Nov 22, 21:43

Only poll blocks worth polling.

deadalnix changed the visibility from "Restricted Project (Project)" to "Public (No Login Required)".Thu, Nov 22, 23:26
deadalnix added a child revision: Restricted Differential Revision.
Fabien requested changes to this revision.Thu, Nov 22, 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.Thu, Nov 22, 23:32
deadalnix updated this revision to Diff 6047.Fri, Nov 23, 00:21

Fix comments

Fabien requested changes to this revision.Fri, Nov 23, 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.Fri, Nov 23, 18:37
deadalnix updated this revision to Diff 6073.Sat, Nov 24, 14:52

Fix comments

jasonbcox requested changes to this revision.Sun, Nov 25, 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.Sun, Nov 25, 07:27

The rest of these changes look good.

deadalnix marked an inline comment as done.Sun, Nov 25, 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.Sun, Nov 25, 17:02

Fix vairable name

Fabien requested changes to this revision.Sun, Nov 25, 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.Sun, Nov 25, 19:57
deadalnix updated this revision to Diff 6128.Tue, Nov 27, 13:05

Add a check for invalid blocks

Fabien accepted this revision.Tue, Nov 27, 13:28
jasonbcox accepted this revision.Tue, Nov 27, 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.Tue, Nov 27, 18:07
This revision was automatically updated to reflect the committed changes.