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

fix: allow empty IP when decoding Ping's from field #9953

Merged
merged 10 commits into from
Jul 31, 2024

Conversation

Rjected
Copy link
Member

@Rjected Rjected commented Jul 31, 2024

This sets the IP to UNSPECIFIED when 0x80 is encountered as a Ping packet from field.

@Rjected Rjected added the A-discv4 Related to discv4 discovery label Jul 31, 2024
@@ -31,7 +31,7 @@ secp256k1 = { workspace = true, features = [
enr.workspace = true

# async/futures
tokio = { workspace = true, features = ["io-util", "net", "time"] }
tokio = { workspace = true, features = ["io-util", "net", "time", "rt-multi-thread"] }
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

probably not, left over from something being messed up with my setup, for some reason I can't test discv4 without it

crates/net/discv4/src/proto.rs Outdated Show resolved Hide resolved
Comment on lines 244 to 261
impl From<NodeRecord> for PingNodeEndpoint {
fn from(NodeRecord { address, tcp_port, udp_port, .. }: NodeRecord) -> Self {
Self(NodeEndpoint { address, tcp_port, udp_port })
}
}

impl From<NodeEndpoint> for PingNodeEndpoint {
fn from(value: NodeEndpoint) -> Self {
Self(value)
}
}

impl PingNodeEndpoint {
/// Creates a new [`PingNodeEndpoint`] from a given UDP address and TCP port.
pub const fn from_udp_address(udp_address: &std::net::SocketAddr, tcp_port: u16) -> Self {
Self(NodeEndpoint { address: udp_address.ip(), udp_port: udp_address.port(), tcp_port })
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need these?

we only need the .0?

Copy link
Member Author

Choose a reason for hiding this comment

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

don't need these

@Rjected Rjected requested a review from mattsse July 31, 2024 17:33
@Rjected Rjected added this pull request to the merge queue Jul 31, 2024
Merged via the queue into main with commit 0fece98 Jul 31, 2024
32 checks passed
@Rjected Rjected deleted the dan/fix-bootnode-packet branch July 31, 2024 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-discv4 Related to discv4 discovery
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants