Page MenuHomePhabricator

[avalanche] Check the delegation generated at startup is valid
ClosedPublic

Authored by Fabien on Apr 26 2021, 10:14.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Maniphest Tasks
Restricted Maniphest Task
Commits
rABCa668273790a0: [avalanche] Check the delegation generated at startup is valid
Summary

This makes sure we don't generate an invalid delegation. The proof and
delegation verification are moved to their own function to unbloat the
processor factory.

The appropriated tests will be added when the -avadelegation is added as it will allow for triggering the failures.

Ref T1635.

Test Plan
ninja check-functional

Diff Detail

Event Timeline

Fabien requested review of this revision.Apr 26 2021, 10:14
deadalnix requested changes to this revision.Apr 26 2021, 21:24
deadalnix added a subscriber: deadalnix.

Why does this depends on D9365 . It seems to me that verifying that the delegation is correct isn't really something that depends on being able to supply it. If anything, it should be done no matter what.

src/avalanche/processor.cpp
304 ↗(On Diff #28268)

Once again, is this checking for the delegation's validity? Why? If not, then this code is even more puzzling.

321 ↗(On Diff #28268)

Is this used anywhere?

test/functional/abc_rpc_avalancheproof.py
267 ↗(On Diff #28268)

The fact that this comment bars no resemblance to the error message reported is a sure sign something's not quite right. You can't express what is going on in a consistent manner because there is nothing consistent about this behavior to express.

I think I kinda understand where the edge case come from, but ultimately, that means the code needs to be reworked, and reading that test should have been a clear sign this is the case.

This revision now requires changes to proceed.Apr 26 2021, 21:24
Fabien planned changes to this revision.Jun 4 2021, 20:01
Fabien added a task: Restricted Maniphest Task.

Rebase. The updated version for this diff and the compagnon D9365 should address all the feedback

This revision is now accepted and ready to land.Jun 14 2021, 19:10