Page MenuHomePhabricator

[avalanche] Only sign the subset of data we want to commit the stakes to
ClosedPublic

Authored by Fabien on Sep 17 2021, 16:37.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Maniphest Tasks
Restricted Maniphest Task
Commits
rABC1f00a7020089: [avalanche] Only sign the subset of data we want to commit the stakes to
Summary

This makes it possible to update all the other parameters from a proof without having the stakers to sign again.

Ref T1676.

Depends on D10140.

Test Plan
ninja all check-all

Diff Detail

Event Timeline

Fabien requested review of this revision.Sep 17 2021, 16:37
PiRK added inline comments.
src/avalanche/test/proof_tests.cpp
570 ↗(On Diff #30036)

--but

deadalnix requested changes to this revision.Sep 20 2021, 14:45
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/avalanche/proof.cpp
118 ↗(On Diff #30036)

Why does this need to be a lambda?

122 ↗(On Diff #30036)

This calls for a ternary operator.

src/avalanche/proof.h
72 ↗(On Diff #30036)

Clearly, this can't be "getHash". What hash? What are these parameters? Can I really pass anything in there?

96 ↗(On Diff #30036)

This is clearly not a useful API. It happens because you can't decide who's in charge of what.

src/avalanche/proofbuilder.cpp
43 ↗(On Diff #30036)

If the proof knows to use legacy or not, then it is clear that the switch between the two behavior doesn't belong here.

src/avalanche/proofbuilder.h
30 ↗(On Diff #30036)

What do I need to pass in there? I can really pass any random crap and I get an appropriately signed stake?

src/avalanche/test/proof_tests.cpp
319 ↗(On Diff #30036)

You just removed all the test cases for the legacy format.

This revision now requires changes to proceed.Sep 20 2021, 14:45

Cleanup the API by rebasing on top of D10175 and D10179.
By using a strongly typed StackCommitment it is no longer possible to sign arbitrary data.

deadalnix requested changes to this revision.Sep 27 2021, 22:06
deadalnix added inline comments.
src/avalanche/proofbuilder.cpp
45

Send the masterPubKey through the pipe directly.

src/avalanche/test/proof_tests.cpp
314

You need to keep the test cases for the legacy format and add new ones for the new format.

This revision now requires changes to proceed.Sep 27 2021, 22:06

Rebase, don't use a temporary to hold the pubkey

This revision is now accepted and ready to land.Oct 14 2021, 14:49