Page MenuHomePhabricator

[avalanche] Keep track of the proof registration time
AbandonedPublic

Authored by Fabien on Jun 17 2022, 15:35.

Details

Reviewers
sdulfari
Group Reviewers
Restricted Project
Summary

Actually only the peer registration time is stored, this diff does it for all the proofs. This will be used to expire the proofs that have been inactive for long enough.
The peer registration time will eventually be replaced by this time as well to avoid duplication, but this will be another diff.

Test Plan
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Branch
avalanche_proofpool_registrationtime
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 19403
Build 38539: Build Diffbuild-without-wallet · build-diff · lint-circular-dependencies · build-clang-tidy · build-clang · build-debug
Build 38538: arc lint + arc unit

Event Timeline

Fabien requested review of this revision.Jun 17 2022, 15:35
sdulfari requested changes to this revision.Jun 17 2022, 17:12
sdulfari added a subscriber: sdulfari.

Only blocking on the public API change.

src/avalanche/proofpool.h
42

Nit: "registration" is a term already used by the peer manager and means slightly different things than it does here. I think this should be ok as long as a divide doesn't grow between the meaning of "adding" a proof to a pool and "registering" it. Something to think about.

145

The public API should return std::chrono::duration. Implementation can remain as int64_t

src/avalanche/test/proofpool_tests.cpp
372

poool

This revision now requires changes to proceed.Jun 17 2022, 17:12

Typo in the Diff title:
if -> of

Fabien retitled this revision from [avalanche] Keep track if the proof registration time to [avalanche] Keep track of the proof registration time.Jun 17 2022, 19:35

Update the API to return an optional std::chrono::seconds, adapt thet tests accordingly, fix typo

This revision is now accepted and ready to land.Jun 20 2022, 16:14
Fabien planned changes to this revision.Jun 22 2022, 20:12

on hold as it's probably not needed