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

Add constructor for NodeContact #280

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

KolbyML
Copy link
Contributor

@KolbyML KolbyML commented Feb 12, 2025

Description

I want to be able to construct the NodeContact object myself. As I want to impl From for NodeContact {} in our codebase, since for us Enr will always be present so having it behind a option is inconvenient

Notes & open questions

Change checklist

  • Self-review
  • Documentation updates if relevant
  • Tests if relevant

@jxs @AgeManning @ackintosh ping for review

@AgeManning
Copy link
Member

This PR seems fine to me, but it doesn't implement From.

Maybe I misunderstood the description?

@KolbyML
Copy link
Contributor Author

KolbyML commented Feb 12, 2025

This PR seems fine to me, but it doesn't implement From.

Maybe I misunderstood the description?

Oh I meant to say I am implementing

/// The contact info for a remote node.
#[derive(Clone, Debug)]
pub struct NodeContact {
    /// Key to use for communications with this node.
    pub public_key: CombinedPublicKey,
    /// The node's ENR.
    pub enr: Enr,
    /// The node's observed socket address.
    pub socket_addr: SocketAddr,
}

impl From<NodeContact> for Discv5NodeContact {
    fn from(value: NodeContact) -> Self {
        Discv5NodeContact::new(value.public_key, value.socket_addr, Some(value.enr))
    }
}

This in our codebase

@AgeManning
Copy link
Member

Oh yeah, cool.

@AgeManning AgeManning merged commit 60e24b9 into sigp:master Feb 12, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants