Page MenuHomePhabricator

[avalanche] Add an API to store and retrieve a proof
ClosedPublic

Authored by Fabien on May 27 2021, 11:50.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABC605bfbfce5a9: [avalanche] Add an API to store and retrieve a proof
Summary

We will need to store proofs that we downloaded and retrieve them for
e.g. relaying. The proof storage is done by the peer manager, which acts
as the valid proof pool and creates a new peer for each new proof. The
interface is provided by the processor so the peer manager remains
private.

Depends on D9617.

Test Plan
ninja check-avalanche

Event Timeline

Fabien requested review of this revision.May 27 2021, 11:50
deadalnix added inline comments.
src/avalanche/processor.cpp
504 ↗(On Diff #28638)

You'd want to know from this API if it was added or not, but this isn't what's returned.

deadalnix requested changes to this revision.May 27 2021, 14:05
deadalnix added inline comments.
src/avalanche/processor.h
306 ↗(On Diff #28638)

What is the lifetime of that pointer?

There is no way this is going to work properly while the proof is owned by the PeerManager is it?

This revision now requires changes to proceed.May 27 2021, 14:05

Return an optional proof to get rid of the lifetime issue while keeping the nullable property.
Don't return success if the proof isn't added because already existing, and add test for this case.

deadalnix requested changes to this revision.May 28 2021, 13:07

I think it's time to start using smart pointers instead of chasing proof copies all around.

This revision now requires changes to proceed.May 28 2021, 13:07

Return a shared pointer to the proof

deadalnix requested changes to this revision.May 31 2021, 15:16
deadalnix added inline comments.
src/avalanche/processor.h
305

I sense some inconsistencies in this API.

This revision now requires changes to proceed.May 31 2021, 15:16

Rebase on top of D9617, use shared pointers consistently

deadalnix requested changes to this revision.Jun 1 2021, 15:40
deadalnix added inline comments.
src/avalanche/processor.cpp
506 ↗(On Diff #28698)

const ref

src/avalanche/processor.h
305 ↗(On Diff #28698)

const ref

This revision now requires changes to proceed.Jun 1 2021, 15:40

Use const ref to shared ptr

This revision is now accepted and ready to land.Jun 2 2021, 12:07
This revision was landed with ongoing or failed builds.Jun 2 2021, 16:06
This revision was automatically updated to reflect the committed changes.