-
Notifications
You must be signed in to change notification settings - Fork 120
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
Parse received addrv2 messages #3022
Conversation
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.
Looks great! I added two suggestions, but they're optional.
141ac63
to
0f14a64
Compare
c5f94de
to
96e2078
Compare
6200581
to
626e7e4
Compare
OK, I've added a few tests to a |
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.
Overall looks good. There's one documentation typo, and I also added some optional suggestions.
2e9c722
to
c371aa1
Compare
626e7e4
to
b453056
Compare
I'm going to put this in draft until I add test vectors from |
756f64d
to
dc225fb
Compare
dc225fb
to
f246cbc
Compare
@zancas thanks, we're now testing these cases in Zebra as well. I added some extra addrv1 and addrv2 cases to cover various parsing edge-cases. zebra/zebra-test/src/network_addr.rs Line 46 in f246cbc
zebra/zebra-test/src/network_addr.rs Line 122 in f246cbc
Our test implementation is here, but it's probably not that interesting for you:
|
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.
Looks good! I added a few suggestions, but they are all optional 👍
This reverts commit 9e69777.
```sh fastmod AddrV2UnimplementedError UnsupportedAddrV2NetworkIdError zebra-network fastmod Unimplemented Unsupported zebra-network ```
f246cbc
to
a7f11c2
Compare
Motivation
ZIP-155 activates at the same time as NU5 on mainnet.
To implement this ZIP, Zebra needs to parse received
addrv2
messages.Sending
addrv2
messages is out of scope.Specifications
https://zips.z.cash/zip-0155#specification
Solution
AddrV2
type for parsingaddrv2
messagesAddrV2
for parsingaddrv2
messagesaddr
andaddrv2
messages toMAX_ADDRS_IN_MESSAGE
addr
messagesAddrV2
Closes #2681.
Review
I'd like both @jvff and @oxarbitrage to review this change, because it's the riskiest part of the ticket.
This is the last in the series of
addrv2
PRs. This PR is based on PR #3032.Reviewer Checklist
Follow Up Work
We should try to get some test vectors from the
zcashd
team.