Skip to content
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

net: Make it easier to reason about node eviction by removing unused NodeEvictionCandidate::addr (CAddress) #20302

Merged
merged 1 commit into from Nov 5, 2020

Conversation

practicalswift
Copy link
Contributor

Make it easier to reason about node eviction by removing unused NodeEvictionCandidate::addr (CAddress).

@DrahtBot DrahtBot added the P2P label Nov 4, 2020
Copy link
Contributor

@amitiuttarwar amitiuttarwar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack f1f433e

very simple change. the addr field indeed seems unused. eviction logic is plenty complicated without red herrings.

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 4, 2020

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

Conflicts

Reviewers, 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.

@jnewbery
Copy link
Contributor

jnewbery commented Nov 5, 2020

utACK f1f433e

Thanks for removing unused variables from this confusing piece of code!

@maflcko maflcko merged commit d94777b into bitcoin:master Nov 5, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 5, 2020
…n by removing unused NodeEvictionCandidate::addr (CAddress)

f1f433e Make it easier to reason about node eviction by removing unused NodeEvictionCandidate::addr (CAddress) (practicalswift)

Pull request description:

  Make it easier to reason about node eviction by removing unused `NodeEvictionCandidate::addr` (`CAddress`).

ACKs for top commit:
  jnewbery:
    utACK f1f433e

Tree-SHA512: fef91d7b412b8a4f172370cff6c37eb8c3db0ba618f5daf2dcc8737c8fcef7b9b820d7ee99cd0a9eae7dd653a096cf83d5113776b0d1d9a324147581674e9ede
@practicalswift practicalswift deleted the NodeEvictionCandidate-addr branch April 10, 2021 19:42
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jun 15, 2021
…victionCandidate::addr (CAddress)

Summary:
Backport of [[bitcoin/bitcoin#20302 | core#20302]].

Depends on D9682.

Ref T1611.

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, PiRK

Reviewed By: #bitcoin_abc, PiRK

Maniphest Tasks: T1611

Differential Revision: https://reviews.bitcoinabc.org/D9683
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
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.

None yet

5 participants