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

Branch

avalanche_stake_no_proofid

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 16701
Build 33257: Build Diff	build-without-wallet · lint-circular-dependencies · build-clang-tidy · build-clang · build-diff · build-debug
Build 33256: arc lint + arc unit

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	--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	Why does this need to be a lambda?
122	This calls for a ternary operator.
src/avalanche/proof.h
72	Clearly, this can't be "getHash". What hash? What are these parameters? Can I really pass anything in there?
96	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	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	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	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.

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

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.