#35820 refactor: keep duration calculations typed

full analysis

https://github.com/bitcoin/bitcoin/pull/35820 · l0rinc · +107/-97 in 26 files, 13 commits · labels: Refactoring, Needs rebase

Goal

  • Prevent accidental time unit confusion across subsystems by using explicit typed durations
  • Clarify duration calculations where units were previously implicit in comments or parameter names

Refactors duration calculations across several subsystems to use std::chrono duration types and literals rather than raw integer counts. It updates time expressions in GUI utilities, peer relay age checks, RPC handlers, and consensus chain parameters while maintaining existing integer interfaces where appropriate. Accompanying unit and functional characterization tests pin the exact PoW timing and assumevalid horizon values.

Problem: Several fixed-duration calculations use raw integer counts of seconds, leaving their units implicit in comments or parameter names and risking accidental unit confusion.

Category: P2P (#59 of 65)

P4 · cleanup

  • P4 because it is an internal cleanup of duration arithmetic in peer processing
  • Does not change relay behavior, network protocols, or DoS resistance

P4 because this is an internal cleanup converting duration arithmetic in net_processing to std::chrono without changing relay behavior, network protocol, or DoS resistance.

Membership: Modifies duration constants and time comparisons in net_processing.cpp and banman.h.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0

Category: RPC / REST / ZMQ (#50 of 52)

P4 · cleanup

  • P4 because it is an internal cleanup of duration expressions in RPC handlers
  • Caller-visible interfaces, RPC behavior, and performance remain unchanged

P4 because it is an internal refactoring of duration expressions in two RPC handlers without affecting interfaces, caller-visible behavior, or performance.

Membership: Modifies duration parsing in mockscheduler and block count estimation in getchaintxstats.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0

Reviewability: Stale: Needs rebase

  • Needs rebase due to merge conflicts, with author inactive for a month

The PR has merge conflicts with several open and recently merged PRs and is labeled Needs rebase; the author has been inactive for 30 days.

Author status: silent since 2026-08-18 rebase

Resolved concerns:

  • stickies-v and maflcko questioned whether converting raw integers to chrono literals and casts added genuine type safety or merely added noise.
  • maflcko pointed out that initial commits violated util/time.h guidelines by using TicksSeconds, which the author subsequently reworked and removed.
  • maflcko objected to noisy conversions in Qt code, which the author reverted.

Agreement: Positive

  • Concept approval for improved readability (janb84)
  • Skeptical that the change adds genuine type safety or clarity (stickies-v)
  • Objected to guideline violations and noisy conversions, which the author addressed (maflcko)

Positive: Concept ACK from janb84; stickies-v (~0) and maflcko raised skepticism and style objections that the author addressed.

janb84 gave a Concept ACK favoring the improved readability. stickies-v offered a lukewarm ~0 seeing minimal benefit, and maflcko pushed back against noisy chrono casts and guideline violations, but the author adjusted the code to satisfy those concerns with no further objections raised.

  • janb84: 'Concept ACK... I like the clarity / readability this changes gives.'
  • stickies-v: '~0, some minor clarity improvements here and there but for the most part I don't see the point as this doesn't add any actual type safety or clarity.'
  • maflcko: 'Tend to agree with stickies. The changes as-is also violate the util/time.h docs.'
  • l0rinc: 'Sure, I’ve reverted the ones that were arguably noisier than the original code. Thanks for the pushback!'

Review verdicts (DrahtBot): 0

Files

47 lines under test/bench/ci.

  • src/qt/guiutil.cpp +15/-30
  • test/functional/feature_assumevalid.py +19/-9
  • src/kernel/chainparams.cpp +11/-10
  • src/net_processing.cpp +9/-9
  • src/test/pow_tests.cpp +10/-6
  • src/qt/rpcconsole.cpp +6/-6
  • src/rpc/node.cpp +4/-4
  • src/qt/bitcoingui.cpp +3/-2
  • src/qt/intro.cpp +3/-2
  • src/validation.cpp +2/-3
  • src/chain.h +2/-2
  • src/net.cpp +2/-2
  • src/qt/rpcconsole.h +3/-1
  • src/banman.h +2/-1
  • src/rpc/blockchain.cpp +2/-1
  • src/txmempool.h +2/-1
  • src/wallet/spend.cpp +2/-1
  • src/headerssync.cpp +1/-1
  • src/node/chainstate.cpp +1/-1
  • src/qt/guiutil.h +1/-1
  • src/qt/modaloverlay.cpp +1/-1
  • src/qt/sendcoinsdialog.cpp +1/-1
  • src/test/chain_tests.cpp +2/-0
  • src/txmempool.cpp +1/-1
  • src/wallet/wallet.cpp +1/-1
  • src/test/mempool_tests.cpp +1/-0

Card

This PR refactors raw integer duration calculations across multiple subsystems into typed std::chrono durations and literals while preserving existing integer interfaces. Its goal is to make units self-documenting in expressions rather than relying on comments, reducing the risk of unit conversion mistakes. The changes are developer-facing refactors that do not alter node behavior, supported by new characterization tests for consensus parameters. It received a Concept ACK from janb84 and lukewarm feedback from stickies-v, while review comments from maflcko were addressed; it currently requires a rebase.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T15:56, confidence high, input hash 23f5e2f84c55c4e8