-
Notifications
You must be signed in to change notification settings - Fork 37.8k
refactor: Remove redundant addrman time checks #25303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Remove redundant addrman time checks #25303
The head ref may contain hidden characters: "2206-less-addrman-time-checks-\u{1F6B6}"
Conversation
nTime is always initialized on deserialization or default-initialized with TIME_INIT, so special casing 0 does not make sense.
fa777c3
to
fa8bdb7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK fa8bdb7
CI failure seems unrelated.
All other places calculate "now - nLastTry", which is safe and correct to do when nLastTry is 0. So do the same here.
fa8bdb7
to
8888bd4
Compare
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
re-ACK 8888bd4 |
utACK 8888bd4 |
Post-merge utACK 8888bd4 |
8888bd4 Remove redundant nLastTry check (MarcoFalke) 00001e5 Remove redundant nTime checks (MarcoFalke) Pull request description: Split out from bitcoin#24697 because it makes sense on its own. ACKs for top commit: dergoegge: re-ACK 8888bd4 naumenkogs: utACK 8888bd4 Tree-SHA512: 32c6cde1c71e943c76b7991c2c24caf29ae467ab4ea2d758483a0cee64625190d1a833b468e8eab1f834beeb2c365af96552c14b05270f08cf63790e0707581d
Summary: > Remove redundant nTime checks > > nTime is always initialized on deserialization or default-initialized > with TIME_INIT, so special casing 0 does not make sense. > Remove redundant nLastTry check > > All other places calculate "now - nLastTry", which is safe and correct > to do when nLastTry is 0. So do the same here. This is a backport of [[bitcoin/bitcoin#25303 | core#25303]] Depends on D15088 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D15089
8888bd4 Remove redundant nLastTry check (MarcoFalke) 00001e5 Remove redundant nTime checks (MarcoFalke) Pull request description: Split out from bitcoin#24697 because it makes sense on its own. ACKs for top commit: dergoegge: re-ACK 8888bd4 naumenkogs: utACK 8888bd4 Tree-SHA512: 32c6cde1c71e943c76b7991c2c24caf29ae467ab4ea2d758483a0cee64625190d1a833b468e8eab1f834beeb2c365af96552c14b05270f08cf63790e0707581d
8888bd4 Remove redundant nLastTry check (MarcoFalke) 00001e5 Remove redundant nTime checks (MarcoFalke) Pull request description: Split out from bitcoin#24697 because it makes sense on its own. ACKs for top commit: dergoegge: re-ACK 8888bd4 naumenkogs: utACK 8888bd4 Tree-SHA512: 32c6cde1c71e943c76b7991c2c24caf29ae467ab4ea2d758483a0cee64625190d1a833b468e8eab1f834beeb2c365af96552c14b05270f08cf63790e0707581d
8888bd4 Remove redundant nLastTry check (MarcoFalke) 00001e5 Remove redundant nTime checks (MarcoFalke) Pull request description: Split out from bitcoin#24697 because it makes sense on its own. ACKs for top commit: dergoegge: re-ACK 8888bd4 naumenkogs: utACK 8888bd4 Tree-SHA512: 32c6cde1c71e943c76b7991c2c24caf29ae467ab4ea2d758483a0cee64625190d1a833b468e8eab1f834beeb2c365af96552c14b05270f08cf63790e0707581d
Split out from #24697 because it makes sense on its own.