[avalanche] change how the proof id is computed
ClosedPublic
Actions

Authored by PiRK on May 18 2021, 10:18.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Commits

rABCe24d8ecdad4b: [avalanche] change how the proof id is computed

Summary

Do a first hash of the proof's data without the master key, then append the master key to this hash and hash again. Fix all tests that depend on the proofid.

This is a first step in creating a LimitedProofId without the master key, to be used in the avahello message to verify a delegation without knowing a proof.
For ease of review, I split the change in two diffs.

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.May 18 2021, 10:18

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 18 2021, 10:18

PiRK requested review of this revision.May 18 2021, 10:18

Harbormaster completed remote builds in B15747: Diff 28517.May 18 2021, 10:32

PiRK added a child revision: D9544: [avalanche] introduce LimitedProofId.May 18 2021, 10:40

Can you explain why so many things need to be fully recomputed?

src/avalanche/test/delegation_tests.cpp
86 ↗	(On Diff #28517)	Why is everything different in there?
src/avalanche/test/proof_tests.cpp
163 ↗	(On Diff #28517)	Why do these proof need to be recomputed? Maybe the signature do, but it doesn't look like the rest does.

This revision now requires changes to proceed.May 18 2021, 13:21

The proofs with expected invalid signatures did not need to be recomputed, only their ID changed. But for the valid ones I needed to know all the private keys for each UTXO to be able to recompute the signatures.
And then there are a bunch of tests that are derived from the valid proof, to show that the proof id changes if a parameter is changed.

src/avalanche/test/delegation_tests.cpp
86 ↗	(On Diff #28517)	I needed a proof with a master key that is known to me.

try to change as little data as possible in the previous tests.

I still had to change:

private keys when a signature needed to be generated
master public keys because buildavalancheproof did not accept previous ones ("Invalid pubkey")
a few sequence numbers (when buildavalancheproof raised "JSON integer out of range")
a few amounts (when buildavalancheproof raised "Amount out of range")