ithacaxyz / op-rs

Apache License 2.0
58 stars 6 forks source link

feat(net): Make ListenConfig Parameterized #40

Closed refcell closed 2 months ago

refcell commented 2 months ago

Description

https://github.com/paradigmxyz/op-rs/pull/34 introduces peer discovery, configuring discv5 to listen to a specified socket address.

This ticket is to refactor out the ListenConfig so that it can be parameterized.

GrapeBaBa commented 2 months ago

Does it mean refactor like this? It seems a change in the discv5 repo?

#[derive(Clone, Debug)]
pub enum ListenConfig<T: Into<IpAddr> = IpAddr> {
    Ip {
        ip: T,
        port: u16,
    },
    DualStack {
        ipv4: T,
        ipv4_port: u16,
        ipv6: T,
        ipv6_port: u16,
    },
}
refcell commented 2 months ago

Ah yes sorry for causing confusion, I see how that wording can be misinterpreted.

What I meant was the net crate currently builds the discovery service and manually constructs the ListenConfig using the address and port here.

We'd like to "parameterize" this by adding a with_listen_config function on the builder that allows the consumer of the net crate to specify the entire ListenConfig object, and if not, it defaults to this singular address+port construction.

Since the entire network stack is constructed using the top-level builder, which builds the discovery service, we should probably add the with_listen_config method to the top-level NetworkDriverBuilder as well and set it on the discovery builder here.

Effectively this "parameterizes" the listen config since a downstream user of the net crate can pass an instantiation of the ListenConfig into the NetworkDriverBuilder as an argument 😄

GrapeBaBa commented 2 months ago

Ok, can I pick this?

refcell commented 2 months ago

Ok, can I pick this?

Certainly!