Differential D10789

[avalanche] Add a peer replacement cooldown
ClosedPublic
Actions

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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Jan 7 2022, 16:23

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 7 2022, 16:23

Fabien requested review of this revision.Jan 7 2022, 16:23

Harbormaster completed remote builds in B17840: Diff 31701.Jan 7 2022, 16:44

Rebase

Harbormaster completed remote builds in B17881: Diff 31752.Jan 11 2022, 09:49

tyler-smith added a subscriber: tyler-smith.Jan 21 2022, 07:01

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.