Page MenuHomePhabricator

[avalanche] Add a peer replacement cooldown
ClosedPublic

Authored by Fabien on Jan 7 2022, 16:23.

Details

Reviewers
tyler-smith
Group Reviewers
Restricted Project
Maniphest Tasks
Restricted Maniphest Task
Commits
rABC2e1c1bc1d747: [avalanche] Add a peer replacement cooldown
Summary

This adds up the last missing cooldown feature. This prevents a malicious actor from updating a proof with a better one in a short time frame (dfefault 1h).

Ref T1854.

Depends on D10788.

Test Plan
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Branch
avalanche_peer_replacement_cooldown
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 18219
Build 36246: Build Diffbuild-diff · build-debug · build-without-wallet · lint-circular-dependencies · build-clang-tidy · build-clang
Build 36245: arc lint + arc unit

Event Timeline

Fabien requested review of this revision.Jan 7 2022, 16:23
tyler-smith added inline comments.
src/net_processing.cpp
4482 ↗(On Diff #31752)

It seems safer to me to have this logic wrapped into a function instead of pairing the right string ("-avalanchepeerreplacementcooldown") to the right default var (AVALANCHE_DEFAULT_PEER_REPLACEMENT_COOLDOWN) every time you might want to use this arg.

Fabien marked an inline comment as done.Jan 21 2022, 08:07
Fabien added inline comments.
src/net_processing.cpp
4482 ↗(On Diff #31752)

Agreed but at this point there is a single call site. If I need to add more it will be a good idea to encapsulate this.

The logic and implementation are good. The negative case added to the tests appears sufficient to check the new behavior.

src/net_processing.cpp
4482 ↗(On Diff #31752)

Sure. Really this is a wider observation; this pattern is used a lot and it seems error-prone. Perhaps in another diff we can hash out a new standard pattern.

This revision is now accepted and ready to land.Jan 30 2022, 23:25
Fabien marked an inline comment as not done.

Rebase, minor update to the test due to changes in dependencies

This revision was automatically updated to reflect the committed changes.