Skip to content

Implement echo connection check #1647

Closed as not planned
Closed as not planned

Description

@netrome

Background

While we haven't concluded the investigation in #1206 because it is difficult to replicate this state, it is evident that implementing an echo/ping-pong check would remove the possibility of the network ending up in these states.

What is an echo check? It's a check where one node sends a message (just an incremental ID is sufficient) to another node, and the other node echos this message back to the first node. When the node receives the echo message, it knows the connection is up in both directions and can also measure the network latency/round-trip time as a bonus.

User Story

As a node operator I'd like my node to be able to notice if loses it's connection with a peer.

Acceptance Criteria

MPC nodes uses a ping-pong protocol to determine if their peers are up or not.

Resources & Additional Notes

No response

Activity

  1. self-assigned this
    on Dec 17, 2025
  2. pbeza commented on Dec 17, 2025

    @pbeza
    Contributor

    Note to self: #97 (comment)

    (thanks @gilcu3 for spotting this and calling it out during today’s call — I would’ve totally missed it otherwise)

  3. added a commit that references this issue on Dec 17, 2025
    976d545
  4. pbeza commented on Dec 18, 2025

    @pbeza
    Contributor

    Quick update:

    I spent some time digging into p2p.rs while trying to integrate a ping/pong keep-alive mechanism, and I have to say the code is quite convoluted—probably one of the most complex parts I’ve run into so far in our codebase. To the point where even the original authors weren’t entirely sure whether a ping mechanism was necessary (see this discussion: #97 (comment)). IIUC, in the end, it was added (without pong replies) mostly because it “didn’t hurt.”

    I actually ended up asking Copilot for help just to understand the overall context and how the p2p primitive structs are used. That helped me add more comprehensive docstrings, which I’ve double-checked for correctness. If you feel there are too many comments, I’m happy to revert or trim them, but personally they helped me get my head around the code much faster. I still need to verify a few things before this is fully ready for review, but feel free to take an early look if you have some time and patience for the complexity involved here.

    I should also mention that there are some design decisions I don’t have a good explanation for—for example, why we maintain two pairs of TLS connections for each pair of nodes. This feels redundant and adds extra complexity, especially when implementing ping/pong, since it’s not clear whether a ping should be sent on one connection and received on the other, or handled in the classic way on a single stream.

  5. netrome commented on Dec 18, 2025

    @netrome
    CollaboratorAuthor

    Thanks, I'll review the PR tomorrow. Very excited to see how it looks. I also share the feeling that our networking layer is very convoluted. Would love to look into what we can do about it. Some of this is already in scope for #1422.

  6. netrome commented on Mar 9, 2026

    @netrome
    CollaboratorAuthor

    Closing this as it has been de-prioritized due to our recent other network fixes making the network much more stable. Long term we do want to simplify the network layer to just use a single connection per peer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions