Skip to content

Net processing: Tidy up CNodeState ctor #21424

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.

Sign up for GitHub

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

Jump to bottom
Merged
maflcko merged 2 commits into from
Mar 15, 2021
Merged

Net processing: Tidy up CNodeState ctor #21424

maflcko merged 2 commits into from
Mar 15, 2021

Conversation

@jnewbery
Copy link
Contributor

@jnewbery jnewbery commented Mar 12, 2021

This addresses the two outstanding review comments from #21370.

jnewbery added 2 commits March 12, 2021 11:37
@jnewbery
[net processing] Remove CNodeState ctor body
55966e0 
It's a no-op. The ctor for RollingBloomFilter already calls reset().
@jnewbery
[net processing] Add ChainSyncTimeoutState default initializers
6927933 
Moves the default values closer to the member definitions.
@jnewbery
Copy link
Contributor Author

jnewbery commented Mar 12, 2021

@MarcoFalke @practicalswift

@jnewbery jnewbery mentioned this pull request Mar 12, 2021
Merged
@fanquake fanquake added the P2P label Mar 12, 2021
hebasto
hebasto approved these changes Mar 12, 2021
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 6927933, I have reviewed the code and it looks OK, I agree it can be merged.

@practicalswift
Copy link
Contributor

practicalswift commented Mar 15, 2021 •
edited
Loading

cr ACK 6927933: patch looks correct

Note to reviewers regarding the removal of m_recently_announced_invs.reset(): CRollingBloomFilter::reset() is already run as part of the CRollingBloomFilter ctor.

@maflcko maflcko merged commit c771fc0 into bitcoin:master Mar 15, 2021
@jnewbery jnewbery deleted the 2021-03-cnodestate-ctor-tidy branch March 15, 2021 17:17
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 15, 2021
Merge bitcoin#21424: Net processing: Tidy up CNodeState ctor
345d143
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@hebasto hebasto hebasto approved these changes

Assignees

No one assigned

Labels

P2P Refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jnewbery @practicalswift @hebasto @fanquake @maflcko