feat: add escape hatch for fallback decoding

[klkvr]

Description

In Reth we'd like to make it possible to bind both discv4 and discv5 to the same port. This requires having a way to share the binded socket and handle decode failures of one of the protocols to then attempt the fallback.

This PR implements 2 changes to support this usecase:

1. ListenConfig::FromSockets variant is added that allows caller to prepare sockets beforehand
2. on_decode_failure callback is added that allows handling payloads that were not decoded succesfully as a discv5 packet

Notes & open questions

Change checklist

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

[jxs]

Hi, and thanks for the interest.
I don't think discovery should be aware of other sockets nor perform callbacks when fails to parse a frame, but we can adapt the api for a sans-io usage that I think also fulfills your goal.
I.e:

#[tokio::main]
async fn main() -> anyhow::Result<()> {
    // 1) Build discv5 normally
    let enr_key = discv5::enr::CombinedKey::generate_secp256k1();
    let enr = discv5::enr::Enr::empty(&enr_key)?;
    // Set ListenConfig to None to specify ingress-only mode
    let cfg = ConfigBuilder::new(None)
    .build();
    let mut discv5: Discv5 = Discv5::new(enr, enr_key, cfg).unwrap();
    // 2) Start in ingress-only mode
    discv5.start().await?;
    // 3) Shared socket owned by the router (not by discv5/discv4)
    let udp = UdpSocket::bind("0.0.0.0:30303").await?;
    // Your discv4 service handle (placeholder)
    let mut discv4 = MyDiscv4::new(/* ... */);
    let mut buf = [0u8; 2048];
    loop {
        tokio::select! {
            // Inbound UDP datagram (single reader)
            Ok((n, src)) = udp.recv_from(&mut buf) => {
                let packet = &buf[..n];
                // Try discv5 framing first (or vice versa, your policy)
                if let Ok(frame) = discv5.try_ingest_frame(src, packet) {
                    // Hand valid discv5 frame into protocol pipeline
                    let _ = discv5.ingest_frame(src, frame).await;
                } else {
                    // Not discv5 -> let discv4 try                    
                }
        }
    }
}

Would an api like this work for you? If so, would you like to pursue it? If not I can submit a PR addressing this.
Thanks

[klkvr]

hi @jxs thanks for the review! and yes, this makes sense and would likely be a proper alternative. I've considered approach like this but it seemed to require substantial invasive changes to the codebase given that right now message decoding/handling happens all the way down in RecvHandler

I would really appreciate if you could open a PR working on this as it seems like an opinionated refactor, but can also work on it if you don't have capacity rn

[jxs]

ok sorry I overpromised, I spent some time looking into a simple way to extract the socket io outside of discovery and it's still a considerable refactor, meanwhile what I came up with, was dd469c2 which albeit not as elegant as totally decoupling io from discovery I feel it's more elegant than the callback approach as discovery doesn't have to store nor the callback nor the socket to be passed to the callback. wdyt @klkvr ? If so you can add it to your PR and we'll merge it from here.
in the future I will look into a sans-io approach as it would also help merge code from discovery with libp2p-kad.
Thanks

[klkvr]

@jxs yep this seems reasonable! the only downside vs callback approach I can see is that it'd allocate a Vec for every unrecognized packet

to solve this for the sending side, do you think it's fine to keep the ListenConfig::FromSockets variant? so that protocol that is handling the unrecognized packets is still able to send responses through the socket

[jxs]

@jxs yep this seems reasonable! the only downside vs callback approach I can see is that it'd allocate a Vec for every unrecognized packet

to solve this for the sending side, do you think it's fine to keep the ListenConfig::FromSockets variant? so that protocol that is handling the unrecognized packets is still able to send responses through the socket

yeah makes sense 👍

[jxs]

LGTM, thanks!