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 ↗(On Diff #30142)

Send the masterPubKey through the pipe directly.

src/avalanche/test/proof_tests.cpp
314 ↗(On Diff #30142)

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