maflcko
commented at 2:15 pm on December 12, 2025:
member
It is a bit confusing to have some code use the deprecated GetTime, which returns a duration and not a time point, and other code to use NodeClock time points.
Fix one place m_addr_token_timestamp to use NodeClock::time_point.
Also:
Extract a ProcessAddrs helper, similar to the other Process*() helpers, to cut down the ProcessMessage with a massive scope.
Rename the confusing current_a_time to now_seconds. (The a in this context refers to the removed “adjusted” time, see commit fadd8b2676f6d68ec87189871461c9a6a6aa3cac, which removed adjusted time here)
Clean-up a test, to make ProcessMessage private again. Also, change pointers to references to avoid the mental and literal overhead of (redundantly) checking for nullptr every time.
DrahtBot renamed this:
refactor: Use NodeClock::time_point for m_addr_token_timestamp
refactor: Use NodeClock::time_point for m_addr_token_timestamp
on Dec 12, 2025
DrahtBot added the label
Refactoring
on Dec 12, 2025
DrahtBot
commented at 2:15 pm on December 12, 2025:
contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.
Conflicts
Reviewers, this pull request conflicts with the following ones:
#34181 (refactor: [p2p] Make ProcessMessage private again, Use references when non-null by maflcko)
#33854 (fix assumevalid is ignored during reindex by Eunovo)
#32278 (doc: better document NetEventsInterface and the deletion of “CNode"s by vasild)
#31650 (refactor: Avoid copies by using const references or by move-construction by maflcko)
#29415 (Broadcast own transactions only via short-lived Tor or I2P connections by vasild)
#27052 (test: rpc: add last block announcement time to getpeerinfo result by LarryRuane)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
ajtowns
commented at 11:45 pm on December 12, 2025:
contributor
Nice branch. I re-created it to avoid refactoring real code to accommodate tests, which are better served by just using the pre-existing ConnmanTestMsg helpers.
DrahtBot removed the label
CI failed
on Dec 16, 2025
DrahtBot added the label
Needs rebase
on Dec 16, 2025
test: [refactor] Avoid calling private ProcessMessage() function
Calling this low-level function from tests is confusing, and also makes
it harder to change the peer manager implementation.
So juse use the pre-existing test helpers to achieve the same.
faa4e5498a
refactor: Make ProcessMessage private again
It is not used in tests anymore.
fa6fbf3fa8
refactor: Pass CNode& to ProcessMessages and SendMessages
The node is never nullptr.
This can be reviewed with the git option:
--word-diff-regex=.
fa0bf0d81d
refactor: Pass Peer& to ProcessMessage
The peer is never nullptr.
fa7cf4cd70
refactor: Separate peer/maybe_peer in ProcessMessages and SendMessages
Introducing two names to refer to the peer makes it possible to have one
refer to a non-null reference in a later commit.
fa13fc46a2
scripted-diff: Use references when nullptr is not possible
This allows to skip nullptr checks later in the code, both mentally and literally.
This can be reviewed via the git option:
--word-diff-regex=.
-BEGIN VERIFY SCRIPT-
sed --regexp-extended --in-place '
/^bool PeerManagerImpl::SendMessages\(/,/^}$/ {
s/auto& peer\{maybe_peer\}; .. alias cleaned up .*/Peer\& peer{*maybe_peer};/;
s/peer->/peer./g;
s/\*peer\>/peer/g;
/CNode\* pto\{&node\}; .. alias removed .*/d;
s/pto->/node./g;
s/\*pto\>/node/g;
}
' src/net_processing.cpp
sed --regexp-extended --in-place '
/^void PeerManagerImpl::ProcessMessage\(/,/^}$/ {
/Peer\* peer\{&peer_alias_removed_in_later_commit};/d;
s/peer_alias_removed_in_later_commit/peer/;
s/peer->/peer./g;
s/\*peer\>/peer/g;
}
' src/net_processing.cpp
sed --regexp-extended --in-place '
/^bool PeerManagerImpl::ProcessMessages\(/,/^}$/ {
s/auto& peer\{maybe_peer\}; .. alias cleaned up .*/Peer\& peer{*maybe_peer};/;
s/peer->/peer./g;
s/\*peer\>/peer/g;
/CNode\* pfrom\{&node\}; .. alias removed .*/d;
s/pfrom->/node./g;
s/\*pfrom\>/node/g;
}
' src/net_processing.cpp
-END VERIFY SCRIPT-
fa0468dab8
move-only: Extract ProcessAddrs() helper
This can be reviewed via the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
fa53143139
refactor: Use NodeClock::time_point for m_addr_token_timestamp
This refactor makes the field a bit more type-safe.
faad5ffdfd
refactor: Rename current_a_time to now_seconds
This better reflects the meaning and use.
fa4263a050
doc: Fix LLM linter complaints
Fix a typo and use a named arg for the Addrman::Add call to describe the
purpose of the arg at the call site.
fa651b93ae
maflcko force-pushed
on Dec 16, 2025
DrahtBot removed the label
Needs rebase
on Dec 16, 2025
DrahtBot added the label
CI failed
on Dec 16, 2025
DrahtBot removed the label
CI failed
on Dec 16, 2025
maflcko marked this as a draft
on Dec 30, 2025
maflcko
commented at 4:34 pm on December 30, 2025:
member
Let’s review #34181 first (the first few commits), as they are likely useful more generally.
DrahtBot added the label
Needs rebase
on Jan 12, 2026
DrahtBot
commented at 9:47 pm on January 12, 2026:
contributor
🐙 This pull request conflicts with the target branch and needs rebase.
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2026-01-22 03:13 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
