refactor: Use type-safe std::chrono for addrman time
ClosedPublic
Actions

Authored by PiRK on Jan 5 2024, 14:31.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABCc578c4851283: refactor: Use type-safe std::chrono for addrman time

Summary

util: Add HoursDouble

https://github.com/bitcoin/bitcoin/pull/24697/commits/fa253d385f9201ea10beacecf3e0e80ff69f3138

Add ChronoFormatter to serialize

https://github.com/bitcoin/bitcoin/pull/24697/commits/fa5103a9f5f8559ab005c0b012d3d3a8057d81fb

Add type-safe AdjustedTime() getter to timedata

Also, fix includes.

https://github.com/bitcoin/bitcoin/pull/24697/commits/fa2ae373f33fa76dc4e435e7cb4778055aa6afd5

refactor: Use type-safe std::chrono for addrman time

This concludes backport of core#24697
https://github.com/bitcoin/bitcoin/pull/24697/commits/fa64dd6673767992eb4e0e775fb0afdfd298610d

Depends on D15094

Test Plan

ninja all check-all bitcoin-fuzzers
src/seeder/bitcoin-seeder

Check that the seeder finds valid peers

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.Jan 5 2024, 14:31

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 5 2024, 14:31

PiRK requested review of this revision.Jan 5 2024, 14:31

teamcity edited the summary of this revision. (Show Details)Jan 5 2024, 14:31

PiRK added a child revision: D15096: Make adjusted time type safe.Jan 5 2024, 14:51

Harbormaster completed remote builds in B26179: Diff 43935.Jan 5 2024, 15:40

Fabien requested changes to this revision.Jan 5 2024, 16:51

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/addrman.cpp
38 ↗	(On Diff #43935)	just checked, days (operator "d") is added in C++20...
89 ↗	(On Diff #43935)	that's 60s ?
src/net.cpp
2025 ↗	(On Diff #43935)	This is not obvious but this is correct
src/protocol.h
443 ↗	(On Diff #43935)

This revision now requires changes to proceed.Jan 5 2024, 16:51

PiRK added inline comments.Jan 5 2024, 19:23

src/addrman.cpp
89 ↗	(On Diff #43935)	oops. Good catch.

fix IsTerrible, use auto for TIME_INIT

Harbormaster completed remote builds in B26203: Diff 43968.Jan 8 2024, 07:56

Fabien accepted this revision.Jan 8 2024, 08:15

Fabien added inline comments.

src/addrman.cpp
89 ↗	(On Diff #43968)	I guess there is not test for that ?

This revision is now accepted and ready to land.Jan 8 2024, 08:15

PiRK added inline comments.Jan 8 2024, 08:25

src/addrman.cpp
89 ↗	(On Diff #43968)	I don't see a test for `IsTerrible`. There is only one test that even mentions it (`addrman_getaddr`) and it just tweaks the `addr.nTime` to ensure it always returns `False` It should be trivial to add a few tests to would catch such obvious mistakes. I will have a look at it in a separate diff.