Details

Reviewers

Fabien
jasonbcox

Group Reviewers

Restricted Project

Commits

rSTAGING1f50cf0e7ef3: [avalanche] Ensure block added to avalanche have the proper acceptance.
rABC1f50cf0e7ef3: [avalanche] Ensure block added to avalanche have the proper acceptance.

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

Branch

avaaddaccept

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 4075
Build 6221: Bitcoin ABC Buildbot (legacy)
Build 6220: arc lint + arc unit

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)

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.

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

Fix comments

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B4075: Diff 6047.Nov 23 2018, 00:32

deadalnix added a child revision: D2122: [avalanche] Enforce cooldown request present in AvalancheResponse.Nov 23 2018, 02:13

Fabien requested changes to this revision.Nov 23 2018, 18:37

Fabien added inline comments.

src/test/avalanche_tests.cpp
158	nit, remove "are"
315	I think it's accepted instead of rejected ?

This revision now requires changes to proceed.Nov 23 2018, 18:37

Fix comments

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B4097: Diff 6073.Nov 24 2018, 15:01

rebase

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B4100: Diff 6077.Nov 24 2018, 18:10

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.