Page MenuHomePhabricator
Differential D10151

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

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

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien created this revision.Sep 17 2021, 16:37
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 17 2021, 16:37
Fabien requested review of this revision.Sep 17 2021, 16:37
Harbormaster completed remote builds in B16701: Diff 30036.Sep 17 2021, 16:50
PiRK added a subscriber: PiRK.Sep 20 2021, 12:43
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
Fabien updated this revision to Diff 30117.Sep 22 2021, 10:01

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.

Fabien edited the summary of this revision. (Show Details)Sep 22 2021, 10:02
Fabien added a parent revision: D10179: [avalanche] Introduce the StakeCommitment class.
Fabien removed a parent revision: D10109: [avalanche] Add a payout script to the proof.
Harbormaster completed remote builds in B16752: Diff 30117.Sep 22 2021, 10:20
Fabien updated this revision to Diff 30142.Sep 23 2021, 10:08

Rebase

Fabien edited the summary of this revision. (Show Details)Sep 23 2021, 10:09
Fabien added a parent revision: D10140: [avalanche] Sign the avalanche proof.
Fabien removed a parent revision: D10179: [avalanche] Introduce the StakeCommitment class.
Harbormaster completed remote builds in B16766: Diff 30142.Sep 23 2021, 10:27
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
Fabien mentioned this in D10222: [avalanche] Let the peermanager select whether a conflicting proof should be preferred.Sep 30 2021, 11:50
Fabien updated this revision to Diff 30527.Oct 14 2021, 09:58

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

Harbormaster completed remote builds in B17021: Diff 30527.Oct 14 2021, 10:17
deadalnix accepted this revision.Oct 14 2021, 14:49
This revision is now accepted and ready to land.Oct 14 2021, 14:49
Closed by commit rABC1f00a7020089: [avalanche] Only sign the subset of data we want to commit the stakes to (authored by Fabien). · Explain WhyOct 14 2021, 15:13
This revision was automatically updated to reflect the committed changes.
Fabien added a commit: rABC1f00a7020089: [avalanche] Only sign the subset of data we want to commit the stakes to.

Revision Contents
Changeset List

PathSize
src/
avalanche/
6 lines
13 lines
4 lines
test/
225 lines

Diff 30543

View Options

src/avalanche/proof.h

Loading...
View Options

src/avalanche/proof.cpp

Loading...
View Options

src/avalanche/proofbuilder.cpp

Loading...
View Options

src/avalanche/test/proof_tests.cpp

Loading...