-
Notifications
You must be signed in to change notification settings - Fork 38.7k
fuzz: Rework FillNode #23575
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
fuzz: Rework FillNode #23575
Conversation
|
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. |
|
Pushed a commit to avoid newly uncovered error: |
theStack
left a comment
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.
Concept ACK
Would it make sense to use CNetMsgMaker(...)::Make(..., ...) helper here for crafting the version message? Also, two yocto-nits: could replace magic strings "version" and "verack" by NetMsgType::VERSION / NetMsgType::VERACK.
faf4d45 to
faaf33a
Compare
|
Thanks, done. |
Also, pass ConnmanTestMsg& and PeerManager& (needed for later commits).
faaf33a to
fa19bab
Compare
|
cc @jnewbery |
|
Strong concept ACK and light code review ACK fa19bab |
Currently
FillNodeis a bit clumsy because it directly modifies memory ofCNode. This gets in the way of moving that memory toPeer. Also, it isn't particularly consistent. See for example #21160 (comment) .Fix all issues by sending a
version/verackinFillNodeand let net_processing figure out the internal details.