feat: implement echo connection check with ping/pong mechanism by pbeza · Pull Request #1687 · near/mpc

pbeza · 2025-12-17T17:29:05Z

Fixes #1647

…onnection-check

pbeza · 2025-12-19T18:02:32Z

I’m not particularly proud of this PR, mainly because it builds on top of an already messy P2P implementation and adds more complexity. To do ping/pong properly, this layer would really need a complete redesign (and if it had been designed well in the first place, the ping-pong mechanism would probably be redundant anyway).

As for the comments I added: they helped me a lot in understanding the networking layer, but I can imagine they might feel excessive to someone already familiar with the code. If you think they should be removed, I’m totally fine with that.

That said, feel free to review.

Copilot

Pull request overview

This PR implements a ping/pong heartbeat mechanism to actively monitor P2P connection health, addressing issue #1647. The implementation adds bidirectional health checks where each node periodically sends ping packets to its peers and expects pong responses, automatically closing connections that fail to respond within a timeout period.

Key changes:

Added sequence-numbered Ping/Pong packets for connection health monitoring
Implemented a watchdog task that monitors pong timeouts and closes dead connections
Added PongState shared between incoming/outgoing streams to track connection health with unidirectional I/O

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…onnection-check

pbeza · 2025-12-22T17:39:40Z

@gilcu3 any idea whether this test is flaky, or if there’s something wrong with my implementation?

ERROR tests/robust_ecdsa/test_parallel_sign_calls.py::test_parallel_sign_calls[3] - AssertionError: Nodes did not reach expected MPC presignature counts (available) before timeout.

BTW, I’ve refined the implementation a bit since I wrote this, so feel free to review it now (cc @gilcu3 @DSharifi).

gilcu3 · 2025-12-22T18:24:08Z

@gilcu3 any idea whether this test is flaky, or if there’s something wrong with my implementation?
ERROR tests/robust_ecdsa/test_parallel_sign_calls.py::test_parallel_sign_calls[3] - AssertionError: Nodes did not reach expected MPC presignature counts (available) before timeout.

That test has something weird, as explained in #1691. But indeed here I am seeing:

...
Available presignatures: [4, 0, 0, 5, 5, 3]
Available presignatures: [4, 0, 0, 5, 5, 3]
Available presignatures: [4, 0, 0, 5, 5, 3]
Available presignatures: [4, 0, 0, 5, 5, 3]
Available presignatures: [4, 0, 0, 5, 5, 3]
Available presignatures: [4, 0, 0, 5, 5, 3]
Available presignatures: [4, 0, 0, 5, 5, 3]
Available presignatures: [4, 0, 0, 5, 5, 3]
Available presignatures: [4, 0, 0, 5, 5, 3]
Available presignatures: [4, 0, 0, 5, 5, 3]
Available presignatures: [4, 0, 0, 5, 5, 3]
E

which means that after some time 2 nodes still did not have any presignature, so there must be something not working as expected.

pbeza · 2025-12-22T19:06:11Z

@gilcu3 any idea whether this test is flaky, or if there’s something wrong with my implementation?
ERROR tests/robust_ecdsa/test_parallel_sign_calls.py::test_parallel_sign_calls[3] - AssertionError: Nodes did not reach expected MPC presignature counts (available) before timeout.
That test has something weird, as explained in #1691. But indeed here I am seeing:

(...)

@gilcu3 I ran this test locally and it passes for me, so it seems CI-specific unless I’m missing something:

((venv) ) ➜  pytest git:(1647-implement-echo-connection-check) ✗ pytest --non-reproducible  tests/robust_ecdsa/test_parallel_sign_calls.py::test_parallel_sign_calls     
================================================================================ test session starts ================================================================================
platform darwin -- Python 3.12.12, pytest-8.3.4, pluggy-1.6.0
rootdir: /Users/patryk/mpc/pytest
configfile: pytest.ini
plugins: libtmux-0.53.0, locust-2.42.6, rerunfailures-16.1
collected 1 item                                                                                                                                                                    

tests/robust_ecdsa/test_parallel_sign_calls.py 

.                                                                                                                              [100%]

=========================================================================== 1 passed in 355.51s (0:05:55) ===========================================================================

((venv) ) ➜  pytest git:(1647-implement-echo-connection-check) ✗ pytest --non-reproducible "tests/robust_ecdsa/test_parallel_sign_calls.py::test_parallel_sign_calls[3]"
================================================================= test session starts =================================================================
platform darwin -- Python 3.12.12, pytest-8.3.4, pluggy-1.6.0
rootdir: /Users/patryk/mpc/pytest
configfile: pytest.ini
plugins: libtmux-0.53.0, locust-2.42.6, rerunfailures-16.1
collected 1 item                                                                                                                                      

tests/robust_ecdsa/test_parallel_sign_calls.py .                                                                                                [100%]

============================================================ 1 passed in 79.64s (0:01:19) ============================================================

((venv) ) ➜  pytest git:(1647-implement-echo-connection-check) ✗ git rev-parse HEAD
e85ca872feeed5eef72623ba02adcce57272fa4f

gilcu3 · 2025-12-22T19:13:53Z

@gilcu3 any idea whether this test is flaky, or if there’s something wrong with my implementation?
ERROR tests/robust_ecdsa/test_parallel_sign_calls.py::test_parallel_sign_calls[3] - AssertionError: Nodes did not reach expected MPC presignature counts (available) before timeout.
That test has something weird, as explained in #1691. But indeed here I am seeing:
(...)

@gilcu3 I ran this test locally and it passes for me, so it seems CI-specific unless I’m missing something:

((venv) ) ➜  pytest git:(1647-implement-echo-connection-check) ✗ pytest --non-reproducible  tests/robust_ecdsa/test_parallel_sign_calls.py::test_parallel_sign_calls     
================================================================================ test session starts ================================================================================
platform darwin -- Python 3.12.12, pytest-8.3.4, pluggy-1.6.0
rootdir: /Users/patryk/mpc/pytest
configfile: pytest.ini
plugins: libtmux-0.53.0, locust-2.42.6, rerunfailures-16.1
collected 1 item                                                                                                                                                                    

tests/robust_ecdsa/test_parallel_sign_calls.py 

.                                                                                                                              [100%]

=========================================================================== 1 passed in 355.51s (0:05:55) ===========================================================================

((venv) ) ➜  pytest git:(1647-implement-echo-connection-check) ✗ git rev-parse HEAD
e85ca872feeed5eef72623ba02adcce57272fa4f

Interesting, so all tests are passing locally for you now? Did you rebuild the mpc-node (or rebase on main, as it now occurs automatically)? If yes to all, can you try disabling that one only and see if CI passes?

kevindeforth

Need to understand the reason for some design choices, before I can approve or request changes.

kevindeforth · 2026-01-09T11:23:36Z

+        let closed_for_keepalive = closed.clone();
        let keepalive_task = tracking::spawn(
-            &format!("TCP keepalive for {}", target_participant_id),
+            &format!("Ping sender for {}", target_participant_id),
            async move {
+                let peer_id_str = target_participant_id.to_string();
+                let mut seq: u64 = 0;
+                let mut interval = tokio::time::interval(Self::PING_INTERVAL);
+                interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay);
+
                loop {
-                    tokio::time::sleep(std::time::Duration::from_secs(1)).await;
-                    if sender_clone.send(Packet::Ping).is_err() {
+                    interval.tick().await;
+                    seq += 1;
+                    let ping_sent_at = Instant::now();
+                    if sender_clone.send(Packet::Ping(seq)).is_err() {
                        // The receiver side will be dropped when the sender task is
                        // dropped (i.e. connection is closed).
-                        break;
+                        return;
+                    }
+                    crate::metrics::MPC_P2P_PING_SEQUENCE_SENT
+                        .with_label_values(&[&peer_id_str])
+                        .set(seq as i64);
+
+                    // Wait for pong matching this ping (non-matching pongs are silently ignored)
+                    match tokio::time::timeout(
+                        Self::PONG_TIMEOUT,
+                        pong_rx.wait_for(|info| info.seq == seq),
+                    )
+                    .await
+                    {
+                        Ok(Ok(_)) => {
+                            let rtt = ping_sent_at.elapsed();
+                            crate::metrics::MPC_P2P_PONG_SEQUENCE_RECEIVED
+                                .with_label_values(&[&peer_id_str])
+                                .set(seq as i64);
+                            crate::metrics::MPC_P2P_RTT_SECONDS
+                                .with_label_values(&[&peer_id_str])
+                                .set(rtt.as_secs_f64());
+                            tracking::set_progress(&format!(
+                                "Received pong {} from {target_participant_id}, RTT: {rtt:?}",
+                                seq
+                            ));
+                        }
+                        Ok(Err(_)) => {
+                            // Channel closed, exit to trigger reconnection
+                            return;
+                        }
+                        Err(_) => {
+                            tracing::warn!(
+                                peer = %target_participant_id,
+                                timeout = ?Self::PONG_TIMEOUT,
+                                "no pong received, closing connection"
+                            );
+                            closed_for_keepalive.cancel();
+                            return;
+                        }


What is the reason for spawning this logic in the constructor, as opposed to in the run_network_client function, and the run_receive_messages_loop function, like we currently do for Indexer Heights and MPC tasks?

mpc/crates/node/src/network.rs

Line 383 in 31060c3

async fn run_receive_messages_loop(

The keepalive_task was already responsible for sending pings, so extending it to handle pongs as well felt like a natural choice, IMO.

Also, I’m not sure we need to be that careful when refactoring this particular piece of code — it’s already quite complex and does a lot of work in the constructor. AFAIK, a full reimplementation of the networking layer is likely to be one of this year’s priorities (though that’s still to be decided by @netrome).

Yeah, I need to think about this.

The keepalive_task was already responsible for sending pings, so extending it to handle pongs as well felt like a natural choice, IMO.

The keepalive task is very simple and does not require any state. While I probably wouldn't have spawned it in the constructor, I do think it can be justified.
But the ping-pong mechanism is considerably more complex and requires additional state, which now, we add to TlsConnection, which just seems like a very odd choice, hence my asking if there is any good reason for it.
IMO, this makes the code harder to understand and reason about.

Also, I’m not sure we need to be that careful when refactoring this particular piece of code — it’s already quite complex and does a lot of work in the constructor. AFAIK, a full reimplementation of the networking layer is likely to be one of this year’s priorities

The fact that we are unhappy with the current state of the code is not a good reason for not trying to improve it. Nobody is working on that refactor right now and it is unclear when we will get to it. We have to expect this code to be used in production and that the networking layer might need some troubleshooting before we have a refactor in place.

Got it, I see your point — I’m just not convinced a small refactor of already messy code will help much, especially if we’re going to reimplement it soon anyway. Let’s sync with @netrome and @DSharifi on Monday.

As a general rule of thumb we should honor the boy scout rule (i.e. leave the code in a better place than you found it) even for legacy code. We will prioritize bigger efforts to brush up the networking layer this year, it's obvious that we need it considering recent incidents and upgrade problems. But that should not block us from maintaining what we have in the meantime. Anything that makes the current networking stack easier to understand (the added documentation comments seem helpful for this - thanks) and maintain is going to help us when we revamp it too.

Done a deeper review now. This doesn't look like it should be too hard to test (lmk if I'm overlooking something), so I'd like to see a test case covering this logic. We could either break it out somewhere else and test it in isolation (my preferred alternative), or just test the whole function (still OK). In either case we should also make sure the TLS connection is covered by tests so it's working as expected. The same goes for the PersistentConnection type.

I know this is a lot, but as per our contribution guidelines we should ensure all our changes are covered by tests (https://github.com/near/mpc/blob/main/CONTRIBUTING.md#add-tests)

With good tests I don't think we should fear changing the code a bit. I know this is a substantial ask, but I feel it's important. The only hard blocker imo is the lack of tests. If there are tests that ensure this behaves as expected, I'm happy - though I believe it will be easier to write these tests if the code is a bit more modularized.

kevindeforth · 2026-01-09T11:25:18Z

+    /// The incoming handler sends PongInfo when it receives a Pong packet. The keepalive task
+    /// monitors this channel to detect connection health and calculate RTT. This enables clean
+    /// async communication without mutexes.
+    pong_tx: tokio::sync::watch::Sender<PongInfo>,


I am not sure this should be part of the TlsConnection struct. With this, we tightly couple the ping-pong mechanism with the connection itself.

Same comment as in here.

kevindeforth · 2026-01-09T11:30:51Z

+                            new_conn.wait_for_close().await;
+                            tracing::info!(
+                                peer = %target_participant_id,
+                                "connection closed, reconnecting"


Same question here. What was the rationale for placing this logic on such a low level?
IMO, this should be handled by the caller of the constructor, but I haven't spent nearly as much time with the code as you, so I might be missing an important detail or good reason to do so.

No strong reason — it was kind of an arbitrary decision to put it where it seemed like it could work.

Honestly, just wrapping my head around the current implementation and getting something workable was hard enough, so I didn’t really try to redesign it properly — especially since the existing design is already pretty messy.

If you feel strongly that this should live somewhere else, I’m happy to move it — just please elaborate a bit on how you’re imagining it should be implemented.

pbeza · 2026-01-09T14:48:21Z

Need to understand the reason for some design choices, before I can approve or request changes.

@kevindeforth, I just replied.

…onnection-check

netrome

Partial review so far. Just a nit about import ordering and chiming in on the keepalive task refactor conversation.

Will continue reviewing and understanding this better ~~after lunch~~ tonight.

netrome · 2026-01-12T10:52:18Z

+        let closed_for_keepalive = closed.clone();
        let keepalive_task = tracking::spawn(
-            &format!("TCP keepalive for {}", target_participant_id),
+            &format!("Ping sender for {}", target_participant_id),
            async move {
+                let peer_id_str = target_participant_id.to_string();
+                let mut seq: u64 = 0;
+                let mut interval = tokio::time::interval(Self::PING_INTERVAL);
+                interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay);
+
                loop {
-                    tokio::time::sleep(std::time::Duration::from_secs(1)).await;
-                    if sender_clone.send(Packet::Ping).is_err() {
+                    interval.tick().await;
+                    seq += 1;
+                    let ping_sent_at = Instant::now();
+                    if sender_clone.send(Packet::Ping(seq)).is_err() {
                        // The receiver side will be dropped when the sender task is
                        // dropped (i.e. connection is closed).
-                        break;
+                        return;
+                    }
+                    crate::metrics::MPC_P2P_PING_SEQUENCE_SENT
+                        .with_label_values(&[&peer_id_str])
+                        .set(seq as i64);
+
+                    // Wait for pong matching this ping (non-matching pongs are silently ignored)
+                    match tokio::time::timeout(
+                        Self::PONG_TIMEOUT,
+                        pong_rx.wait_for(|info| info.seq == seq),
+                    )
+                    .await
+                    {
+                        Ok(Ok(_)) => {
+                            let rtt = ping_sent_at.elapsed();
+                            crate::metrics::MPC_P2P_PONG_SEQUENCE_RECEIVED
+                                .with_label_values(&[&peer_id_str])
+                                .set(seq as i64);
+                            crate::metrics::MPC_P2P_RTT_SECONDS
+                                .with_label_values(&[&peer_id_str])
+                                .set(rtt.as_secs_f64());
+                            tracking::set_progress(&format!(
+                                "Received pong {} from {target_participant_id}, RTT: {rtt:?}",
+                                seq
+                            ));
+                        }
+                        Ok(Err(_)) => {
+                            // Channel closed, exit to trigger reconnection
+                            return;
+                        }
+                        Err(_) => {
+                            tracing::warn!(
+                                peer = %target_participant_id,
+                                timeout = ?Self::PONG_TIMEOUT,
+                                "no pong received, closing connection"
+                            );
+                            closed_for_keepalive.cancel();
+                            return;
+                        }


As a general rule of thumb we should honor the boy scout rule (i.e. leave the code in a better place than you found it) even for legacy code. We will prioritize bigger efforts to brush up the networking layer this year, it's obvious that we need it considering recent incidents and upgrade problems. But that should not block us from maintaining what we have in the meantime. Anything that makes the current networking stack easier to understand (the added documentation comments seem helpful for this - thanks) and maintain is going to help us when we revamp it too.

netrome · 2026-01-12T19:07:34Z

Will continue reviewing and understanding this better ~~after lunch~~ tonight.

Had to prioritize other work for the evening. Will circle back to this tomorrow instead.

netrome

Thanks for doing this, I know this part of the node is quite complex to dive into and I don't fully understand it myself.

However, I think this should be possible to test so I'd like to see at least some basic tests covering the changed behavior. This is the only big blocker I see.

Beyond this there's a few tiny blockers as well:

Unconventional import order (from previous review)
A silent lossy as conversion.
Implicit overflowing arithmitic.

netrome · 2026-01-13T20:23:18Z

+        let closed_for_keepalive = closed.clone();
        let keepalive_task = tracking::spawn(
-            &format!("TCP keepalive for {}", target_participant_id),
+            &format!("Ping sender for {}", target_participant_id),
            async move {
+                let peer_id_str = target_participant_id.to_string();
+                let mut seq: u64 = 0;
+                let mut interval = tokio::time::interval(Self::PING_INTERVAL);
+                interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay);
+
                loop {
-                    tokio::time::sleep(std::time::Duration::from_secs(1)).await;
-                    if sender_clone.send(Packet::Ping).is_err() {
+                    interval.tick().await;
+                    seq += 1;
+                    let ping_sent_at = Instant::now();
+                    if sender_clone.send(Packet::Ping(seq)).is_err() {
                        // The receiver side will be dropped when the sender task is
                        // dropped (i.e. connection is closed).
-                        break;
+                        return;
+                    }
+                    crate::metrics::MPC_P2P_PING_SEQUENCE_SENT
+                        .with_label_values(&[&peer_id_str])
+                        .set(seq as i64);
+
+                    // Wait for pong matching this ping (non-matching pongs are silently ignored)
+                    match tokio::time::timeout(
+                        Self::PONG_TIMEOUT,
+                        pong_rx.wait_for(|info| info.seq == seq),
+                    )
+                    .await
+                    {
+                        Ok(Ok(_)) => {
+                            let rtt = ping_sent_at.elapsed();
+                            crate::metrics::MPC_P2P_PONG_SEQUENCE_RECEIVED
+                                .with_label_values(&[&peer_id_str])
+                                .set(seq as i64);
+                            crate::metrics::MPC_P2P_RTT_SECONDS
+                                .with_label_values(&[&peer_id_str])
+                                .set(rtt.as_secs_f64());
+                            tracking::set_progress(&format!(
+                                "Received pong {} from {target_participant_id}, RTT: {rtt:?}",
+                                seq
+                            ));
+                        }
+                        Ok(Err(_)) => {
+                            // Channel closed, exit to trigger reconnection
+                            return;
+                        }
+                        Err(_) => {
+                            tracing::warn!(
+                                peer = %target_participant_id,
+                                timeout = ?Self::PONG_TIMEOUT,
+                                "no pong received, closing connection"
+                            );
+                            closed_for_keepalive.cancel();
+                            return;
+                        }


Done a deeper review now. This doesn't look like it should be too hard to test (lmk if I'm overlooking something), so I'd like to see a test case covering this logic. We could either break it out somewhere else and test it in isolation (my preferred alternative), or just test the whole function (still OK). In either case we should also make sure the TLS connection is covered by tests so it's working as expected. The same goes for the PersistentConnection type.

I know this is a lot, but as per our contribution guidelines we should ensure all our changes are covered by tests (https://github.com/near/mpc/blob/main/CONTRIBUTING.md#add-tests)

With good tests I don't think we should fear changing the code a bit. I know this is a substantial ask, but I feel it's important. The only hard blocker imo is the lack of tests. If there are tests that ensure this behaves as expected, I'm happy - though I believe it will be easier to write these tests if the code is a bit more modularized.

…onnection-check

…ion settings

…onnection-check

pbeza · 2026-01-15T17:41:45Z

Feel free to do an early re-review, @kevindeforth @netrome @DSharifi. There’s still a bit of a mystery around one test I’d like to dig into tomorrow—specifically this one (it passes even if you replace + with - here):

mpc/crates/node/src/p2p.rs

Lines 1403 to 1422 in 0fe1bcb

    
           #[tokio::test(start_paused = true)] 
        
           async fn test_pong_timeout_triggers_reconnection() { 
        
               // given 
        
               let mut env = KeepaliveTestEnv::new(); 
        
               let handle = env.spawn_keepalive(); 
        
               env.advance(Duration::from_secs(1)).await; 
        
               env.expect_ping(KeepaliveTestEnv::FIRST_PING_SEQ).await; 
        
               // when: don't respond with pong - advance past timeout 
        
               env.advance(env.config.pong_timeout + Duration::from_secs(1)) 
        
                   .await; 
        
               let _ = handle.await; 
        
               // then 
        
               assert!( 
        
                   env.timed_out(), 
        
                   "connection should be cancelled on pong timeout" 
        
               ); 
        
           }

Overall, the code shouldn’t change much unless you ask for more changes.

I did refactor it a bit to make it easier to test and tried to follow Kevin’s suggestions. This part of the code is pretty hard to reason about, so I might’ve missed something — would appreciate an extra careful review 🙏

…onnection-check

netrome · 2026-01-16T11:08:02Z

I did refactor it a bit to make it easier to test and tried to follow Kevin’s suggestions. This part of the code is pretty hard to reason about, so I might’ve missed something — would appreciate an extra careful review 🙏

Sounds good. I'll do a quick pass of the PR now and look a bit deeper again after lunch.

netrome

Mostly looks good from a quick pass, but I think we have a compatibility issue. I'll can verify this with a localnet setup after lunch.

netrome · 2026-01-16T11:17:58Z

-                            let packet = Packet::try_from_slice(&buf)
-                                .context("Failed to deserialize packet")?;


Hmm, the fact that we return here on errors sounds like we'll crash if old nodes encounter Pong messages. If that's the case, we'll need to deploy this in two steps. First, make a small PR to make the packet deserialization robust towards unfamiliar packets which we have to release first, and then rebase this PR on top of that and make this in a subsequent release.

Verified in localnet. Trying to spin up a node with this code against a node on main results in these logs on the main node:

2026-01-16T12:32:46.518985Z ERROR mpc_node::tracking: task failed; description: new connection handler tls mesh network; error msg: Failed to deserialize packet; trace:
from <1s new connection handler tls mesh network: Authenticated as 1 (for 1ms)
from 54s incoming connection handler tls mesh network: (for 54s)
from 54s Initializing: ecdsa key generation: parties: [0, 1], sent [0, 0] (inflight [[0, 4]]), received [[0, 0]] (receiving) (for 7s)
from 1m root: Initializing (for 54s)
2026-01-16T12:32:46.519431Z INFO mpc_node::p2p: Incoming 0 <-- 1 connected
2026-01-16T12:32:46.520549Z ERROR mpc_node::tracking: task failed; description: new connection handler tls mesh network; error msg: Failed to deserialize packet; trace:
from <1s new connection handler tls mesh network: Authenticated as 1 (for 1ms)
from 54s incoming connection handler tls mesh network: (for 54s)
from 54s Initializing: ecdsa key generation: parties: [0, 1], sent [0, 0] (inflight [[0, 4]]), received [[0, 0]] (receiving) (for 7s)
from 1m root: Initializing (for 54s)
2026-01-16T12:32:46.521053Z INFO mpc_node::p2p: Incoming 0 <-- 1 connected
2026-01-16T12:32:46.522156Z ERROR mpc_node::tracking: task failed; description: new connection handler tls mesh network; error msg: Failed to deserialize packet; trace:
from <1s new connection handler tls mesh network: Authenticated as 1 (for 1ms)
from 54s incoming connection handler tls mesh network: (for 54s)
from 54s Initializing: ecdsa key generation: parties: [0, 1], sent [0, 0] (inflight [[0, 4]]), received [[0, 0]] (receiving) (for 7s)
from 1m root: Initializing (for 54s)
2026-01-16T12:32:46.522735Z INFO mpc_node::p2p: Incoming 0 <-- 1 connected
2026-01-16T12:32:46.523868Z ERROR mpc_node::tracking: task failed; description: new connection handler tls mesh network; error msg: Failed to deserialize packet; trace:
from <1s new connection handler tls mesh network: Authenticated as 1 (for 1ms)
from 54s incoming connection handler tls mesh network: (for 54s)
from 54s Initializing: ecdsa key generation: parties: [0, 1], sent [0, 0] (inflight [[0, 4]]), received [[0, 0]] (receiving) (for 7s)
from 1m root: Initializing (for 54s)
2026-01-16T12:32:46.524345Z INFO mpc_node::p2p: Incoming 0 <-- 1 connected
2026-01-16T12:32:46.525451Z ERROR mpc_node::tracking: task failed; description: new connection handler tls mesh network; error msg: Failed to deserialize packet; trace:
from <1s new connection handler tls mesh network: Authenticated as 1 (for 1ms)
from 54s incoming connection handler tls mesh network: (for 54s)
from 54s Initializing: ecdsa key generation: parties: [0, 1], sent [0, 0] (inflight [[0, 4]]), received [[0, 0]] (receiving) (for 7s)
from 1m root: Initializing (for 54s)
2026-01-16T12:32:46.525969Z INFO mpc_node::p2p: Incoming 0 <-- 1 connected
2026-01-16T12:32:46.527118Z ERROR mpc_node::tracking: task failed; description: new connection handler tls mesh network; error msg: Failed to deserialize packet; trace:
from <1s new connection handler tls mesh network: Authenticated as 1 (for 1ms)
from 54s incoming connection handler tls mesh network: (for 54s)
from 54s Initializing: ecdsa key generation: parties: [0, 1], sent [0, 0] (inflight [[0, 4]]), received [[0, 0]] (receiving) (for 7s)

And on the ping pong node:

2026-01-16T12:31:59.727592Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.727737Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.728896Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.729378Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.729450Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.730594Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.731708Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.731825Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.732972Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.734166Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.734289Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.735458Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.736040Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.736109Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.737242Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.738265Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.738374Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.739503Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.740454Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.740559Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.741695Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.742640Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.742750Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.743995Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.744568Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.744636Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.745807Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.746440Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.746513Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.747671Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.748685Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.748833Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.750000Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.750471Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.750507Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.751682Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.752169Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.752207Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.753358Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.753772Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.753844Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.754980Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.755346Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.755383Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.756523Z INFO mpc_node::p2p: connection closed, reconnecting peer=0
2026-01-16T12:31:59.756914Z INFO mpc_node::p2p: performing P2P handshake target_address="localhost:3000"
2026-01-16T12:31:59.756950Z INFO mpc_node::p2p: outgoing connection established from=1 to=0
2026-01-16T12:31:59.758077Z INFO mpc_node::p2p: connection closed, reconnecting peer=0

Hmm... reflecting more on this, simply adding tolerance for unrecognized packets won't solve this as the ping-pong mechanism would still kill the connection with any node who doesn't recognize it. Instead, we have to break up this change into two steps. Step 1 is to create a version of the node that recognizes these packets, returns Pong messages if received but still sends the old Ping (without any value) messages.

Once that's released we can follow up with a PR that sends the new ping messages and drops the connection if the pong messages aren't received.

@pbeza - what do you think of this?

Hmm... reflecting more on this, simply adding tolerance for unrecognized packets won't solve this as the ping-pong mechanism would still kill the connection with any node who doesn't recognize it. Instead, we have to break up this change into two steps. Step 1 is to create a version of the node that recognizes these packets, returns Pong messages if received but still sends the old Ping (without any value) messages.

Once that's released we can follow up with a PR that sends the new ping messages and drops the connection if the pong messages aren't received.

@pbeza - what do you think of this?

Agree, except we don’t need to respond with Pong yet in step 1, IIUC.

Addressed here: #1782.

gilcu3 · 2026-02-20T16:48:11Z

This solution was eventually superseded by #1831. If anyone feels like this is not the case please re-open, closing for now

feat: implement echo connection check with ping/pong mechanism

976d545

Fixes #1647

pbeza linked an issue Dec 17, 2025 that may be closed by this pull request

Implement echo connection check #1647

Closed

pbeza added 9 commits December 17, 2025 18:29

Merge remote-tracking branch 'origin/main' into 1647-implement-echo-c…

3f74993

…onnection-check

Merge remote-tracking branch 'origin/main' into 1647-implement-echo-c…

512bc89

…onnection-check

Add more documentation to p2p.rs

e51c665

Use separate channels for sending and receiving ping/pong messages

9f18a65

Merge remote-tracking branch 'origin/main' into 1647-implement-echo-c…

ec728f7

…onnection-check

Merge remote-tracking branch 'origin/main' into 1647-implement-echo-c…

67cf900

…onnection-check

Fix clippy

081fce5

Fixes

ab7b91d

Merge remote-tracking branch 'origin/main' into 1647-implement-echo-c…

22765ae

…onnection-check

pbeza requested review from DSharifi and gilcu3 December 19, 2025 18:02

pbeza marked this pull request as ready for review December 19, 2025 18:02

Copilot AI review requested due to automatic review settings December 19, 2025 18:02

Copilot started reviewing on behalf of pbeza December 19, 2025 18:03

View session

Copilot AI reviewed Dec 19, 2025

View reviewed changes

Comment thread crates/node/src/p2p.rs Outdated

Comment thread crates/node/src/p2p.rs Outdated

Comment thread crates/node/src/p2p.rs Outdated

pbeza added 7 commits December 19, 2025 19:16

Address Copilot's code review

bbdcc29

Merge remote-tracking branch 'origin/main' into 1647-implement-echo-c…

f6b5471

…onnection-check

simplify

47f630b

Merge remote-tracking branch 'origin/main' into 1647-implement-echo-c…

e68d25d

…onnection-check

fixup! simplify

615a040

fixup! fixup! simplify

cd9ffb5

fixup! fixup! fixup! simplify

e85ca87

skip test_parallel_sign_calls for now

d047c9a

kevindeforth reviewed Jan 9, 2026

View reviewed changes

pbeza added 2 commits January 9, 2026 15:49

Merge remote-tracking branch 'origin/main' into 1647-implement-echo-c…

d63c305

…onnection-check

Merge remote-tracking branch 'origin/main' into 1647-implement-echo-c…

d6422d1

…onnection-check

netrome reviewed Jan 12, 2026

View reviewed changes

netrome requested changes Jan 13, 2026

View reviewed changes

gilcu3 mentioned this pull request Jan 14, 2026

enable TCP_KEEPALIVE to tcp connections #1751

Closed

pbeza added 10 commits January 15, 2026 11:31

Merge remote-tracking branch 'origin/main' into 1647-implement-echo-c…

ece890d

…onnection-check

Revert deny.toml

74177b9

fix(p2p): handle overflow in TLS sequence number

eed1da8

fix(p2p): handle overflow in TLS sequence number by converting to i64

0164312

refactor + tests

c71a050

feat(p2p): add OutgoingConnectionConfig for managing outgoing connect…

453de4a

…ion settings

Merge remote-tracking branch 'origin/main' into 1647-implement-echo-c…

7c69b17

…onnection-check

Cleanup

948f614

Tests cleanup

75a0b8c

Merge remote-tracking branch 'origin/main' into 1647-implement-echo-c…

0fe1bcb

…onnection-check

pbeza added 2 commits January 16, 2026 11:34

Merge remote-tracking branch 'origin/main' into 1647-implement-echo-c…

67ef832

…onnection-check

refine test_pong_timeout_triggers_reconnection

1202ebc

netrome reviewed Jan 16, 2026

View reviewed changes

More comprehensive tests

7003987

netrome mentioned this pull request Jan 16, 2026

Introduce new Ping/Pong message types #1781

Closed

Copilot AI mentioned this pull request Jan 16, 2026

feat(p2p): introduce new EchoPing and EchoPong message types #1782

Closed

kevindeforth mentioned this pull request Jan 16, 2026

test: add a test for checking backwards compatibility of network protocol #1784

Closed

gilcu3 closed this Feb 20, 2026

pbeza mentioned this pull request Oct 13, 2026

feat(node): measure round-trip time to each peer #4763

Open

		let packet = Packet::try_from_slice(&buf)
		.context("Failed to deserialize packet")?;

Conversation

pbeza commented Dec 17, 2025

Uh oh!

pbeza commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pbeza commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gilcu3 commented Dec 22, 2025

Uh oh!

pbeza commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gilcu3 commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevindeforth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pbeza Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevindeforth Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pbeza Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevindeforth Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pbeza commented Jan 9, 2026

Uh oh!

netrome left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

netrome commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netrome left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pbeza commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netrome commented Jan 16, 2026

Uh oh!

pbeza commented Dec 19, 2025 •

edited

Loading

pbeza commented Dec 22, 2025 •

edited

Loading

pbeza commented Dec 22, 2025 •

edited

Loading

gilcu3 commented Dec 22, 2025 •

edited

Loading

pbeza Jan 9, 2026 •

edited

Loading

kevindeforth Jan 9, 2026 •

edited

Loading

pbeza Jan 9, 2026 •

edited

Loading

kevindeforth Jan 9, 2026 •

edited

Loading

netrome left a comment •

edited

Loading

netrome commented Jan 12, 2026 •

edited

Loading

pbeza commented Jan 15, 2026 •

edited

Loading