Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Jun 8, 2022

Split out from #24697 because it makes sense on its own.

nTime is always initialized on deserialization or default-initialized
with TIME_INIT, so special casing 0 does not make sense.
Copy link
Member

@dergoegge dergoegge left a 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.
@maflcko maflcko force-pushed the 2206-less-addrman-time-checks- branch from fa8bdb7 to 8888bd4 Compare June 8, 2022 11:35
@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 8, 2022

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #24697 (refactor address relay time by MarcoFalke)
  • #23829 (refactor: use braced init for integer literals instead of c style casts by PastaPastaPasta)

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.

@dergoegge
Copy link
Member

re-ACK 8888bd4

@naumenkogs
Copy link
Member

utACK 8888bd4

@fanquake fanquake merged commit 9edc513 into bitcoin:master Jun 9, 2022
@maflcko maflcko deleted the 2206-less-addrman-time-checks-🚶 branch June 9, 2022 13:05
@mzumsande
Copy link
Contributor

Post-merge utACK 8888bd4

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 13, 2022
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
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jan 5, 2024
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 17, 2024
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 19, 2024
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 19, 2024
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
@bitcoin bitcoin locked and limited conversation to collaborators Apr 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants