Page MenuHomePhabricator

[avalanche] change how the proof id is computed
ClosedPublic

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

Event Timeline

PiRK requested review of this revision.May 18 2021, 10:18
deadalnix requested changes to this revision.May 18 2021, 13:21
deadalnix added a subscriber: deadalnix.

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

src/avalanche/test/delegation_tests.cpp
86

Why is everything different in there?

src/avalanche/test/proof_tests.cpp
163

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

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")
This revision is now accepted and ready to land.May 20 2021, 16:26

use an uncompressed master pubkey like in the original test, to make the diff more readable