feat: support for robust ecdsa signature in the mpc-node #1645

gilcu3 · 2025-12-10T13:28:26Z

gilcu3 · 2025-12-10T13:38:13Z

crates/node/src/primitives.rs

    CKDTaskId(CKDTaskId),
-}
-
-pub fn participants_from_triples(


this function was simply quite misplaced

gilcu3 · 2025-12-10T13:39:00Z

crates/node/src/keyshare.rs

    Secp256k1(threshold_signatures::ecdsa::KeygenOutput),
    Ed25519(threshold_signatures::eddsa::KeygenOutput),
    Bls12381(threshold_signatures::confidential_key_derivation::KeygenOutput),
+    V2Secp256k1(threshold_signatures::ecdsa::KeygenOutput),


this is not exactly optimal, but we had the same situation with Ckd, and this was the best we could come up with then

crates/node/src/providers/robust_ecdsa.rs

gilcu3 · 2025-12-10T13:42:00Z

crates/node/src/providers/robust_ecdsa/presign.rs

+                let participants = match client.select_random_active_participants_including_me(
+                    threshold,
+                    &running_participants,
+                ) {


this was probably the only not trivial part of the PR. Which participants to select to compute a presignature, which will be exactly the same used later for the corresponding signature? I opted to do exactly the same as we currently do for triples in Cait-Sith

Yep, we do have the robustness bug so right now we use the same set for both signatures and presignatures. Down the line we need to figure out how to leverage the robustness properties.

crates/node/src/providers/robust_ecdsa/presign.rs

gilcu3 · 2025-12-10T14:14:56Z

crates/node/src/providers/robust_ecdsa/presign.rs

+                            e
+                        );
+                        // that should not happen often, so sleeping here is okay
+                        tokio::time::sleep(Duration::from_millis(100)).await;


we should certainly have an issue for removing these sleep ops, not doing so just in case something actually breaks

netrome

Shallow skimming. Some nits, but so far so good. Happy to go deeper once the PR is ready.

crates/node/src/coordinator.rs

crates/node/src/providers/robust_ecdsa/sign.rs

netrome · 2025-12-10T14:18:42Z

crates/node/src/providers/robust_ecdsa/presign.rs

+                let participants = match client.select_random_active_participants_including_me(
+                    threshold,
+                    &running_participants,
+                ) {


Yep, we do have the robustness bug so right now we use the same set for both signatures and presignatures. Down the line we need to figure out how to leverage the robustness properties.

crates/node/src/providers/robust_ecdsa/presign.rs

crates/node/src/providers/robust_ecdsa/sign.rs

gilcu3 · 2025-12-10T17:08:28Z

crates/node/src/tests/resharing.rs


 // Test a simple resharing of one node joining a cluster of 4 nodes.
 #[tokio::test]
 async fn test_key_resharing_simple() {


it seems I made this test too fat. It passes locally but fails in CI, will need to improve it somehow

could not improve, seems asset generation is too heavy for github CI, as the test when adding the new domain passes locally but not there

gilcu3 · 2025-12-12T13:46:19Z

crates/node/src/p2p.rs

    impl PortSeed {
+        // The base port number used, hoping the OS is not using ports in this range
+        pub const BASE_PORT: u16 = 10000;
+        // This constant must be equal to the total number of ports defined below
+        pub const TOTAL_DEFINED_PORTS: u16 = 19;
+        // Maximum number of nodes that can be handled without port collisions
+        pub const MAX_NODES: u16 = 10;
+        // Maximum number of cases that can be handled without port collisions
+        pub const MAX_CASES: u16 = 4;
+        // Each function below corresponds to a port per node. Each defines an offset,
+        // and all offsets must be different
+        pub const TOTAL_PORTS_PER_NODE: u16 = 3;


changes in this file were needed to be able to use rstest with different domains and still avoid port collisions

gilcu3 · 2025-12-12T13:46:50Z

crates/node/src/tests.rs

-const DEFAULT_MAX_PROTOCOL_WAIT_TIME: std::time::Duration = std::time::Duration::from_secs(30);
-const DEFAULT_MAX_SIGNATURE_WAIT_TIME: std::time::Duration = std::time::Duration::from_secs(30);
+const DEFAULT_MAX_PROTOCOL_WAIT_TIME: std::time::Duration = std::time::Duration::from_secs(60);
+const DEFAULT_MAX_SIGNATURE_WAIT_TIME: std::time::Duration = std::time::Duration::from_secs(60);


unfortunately the old timeout is now too small

gilcu3 · 2025-12-12T13:49:22Z

pytest/tests/robust_ecdsa/conftest.py

given the needs of robust ecdsa (more nodes) I made a module with a shared cluster for those. Trying to make them work in the existing shared cluster seems impossible at the moment, because many tests would simply fail with timeouts. Also, it would make them considerably slower.

…natures

gilcu3 · 2025-12-12T19:44:29Z

pytest/common_lib/shared/mpc_cluster.py

    def kill_all(self):
-        for node in self.mpc_nodes:
-            node.kill(False)
+        with ThreadPoolExecutor(max_workers=len(self.mpc_nodes)) as executor:
+            executor.map(lambda node: node.kill(False), self.mpc_nodes)


we were losing many seconds in each test because of this, as the underlying implementation waits for 5 seconds for the process to die

gilcu3 · 2025-12-12T19:45:07Z

pytest/tests/test_request_during_resharing.py

+TRIPLES_TO_BUFFER = 20
+PRESIGNATURES_TO_BUFFER = 10
+


these are sane values for the tests in this file

…rence

gilcu3 linked an issue Dec 10, 2025 that may be closed by this pull request

MPC nodes processes Robust ECDSA signature requests. #1640

Open

gilcu3 force-pushed the 1640-mpc-nodes-processes-robust-ecdsa-signature-requests branch 2 times, most recently from 3c7347e to 95f32e1 Compare December 10, 2025 13:34

gilcu3 commented Dec 10, 2025

View reviewed changes

crates/node/src/providers/robust_ecdsa.rs Outdated Show resolved Hide resolved

gilcu3 commented Dec 10, 2025

View reviewed changes

crates/node/src/providers/robust_ecdsa/presign.rs Show resolved Hide resolved

gilcu3 commented Dec 10, 2025

View reviewed changes

netrome reviewed Dec 10, 2025

View reviewed changes

gilcu3 commented Dec 10, 2025

View reviewed changes

gilcu3 force-pushed the 1640-mpc-nodes-processes-robust-ecdsa-signature-requests branch from 6d7a1a8 to 391706f Compare December 11, 2025 16:39

netrome and others added 18 commits December 12, 2025 13:05

chore: Introduce SignatureScheme enum variant

1784e01

fix: remaining compilation issues

71cb303

feat: add support for robust ecdsa

5b07511

chore: remove repeated files

c3c3006

fix: after rebase on main

858c7cf

fix: nits, which includes forgotten abi

c7b3ef1

fix: there were some hidden unused functions

7474353

fix: remove unneeded clones

2be4263

fix: remove unneeded Arcs

3f19fd4

test: added tests in the node

00b143a

fix: contract tests that were failing because of the theshold tweak

2d5f5c2

feat: proper threshold handling for robust ecdsa, adapted tests

d9dd359

fix: shot in the dark

8488c0e

fix: path with sleep?

c5b97f6

fix: last try?

d3b1515

fix: downgrade tests

4ed36e7

fix: the last show

e3454b9

chore: add type for msg_hash

000d61a

fix: increase timeout?

6d11553

gilcu3 force-pushed the 1640-mpc-nodes-processes-robust-ecdsa-signature-requests branch from 73dc604 to 3ebf0fe Compare December 12, 2025 13:41

test: added pytest support

08fa259

gilcu3 force-pushed the 1640-mpc-nodes-processes-robust-ecdsa-signature-requests branch from 3ebf0fe to 08fa259 Compare December 12, 2025 13:41

gilcu3 commented Dec 12, 2025

View reviewed changes

gilcu3 added 18 commits December 12, 2025 14:52

fix: nits

ae388cb

fix: longer timeout for failing function

f6540e3

fix: try with more powerful CI nodes

815b9ef

chore: revert beefy CI, did not work

367f8df

fix: reuse existing functions to wait for the nodes generating presig…

b704117

…natures

fix: increase timeouts

bf42efb

fix: manage triple generation

b4af3b3

fix: put up timeout again, show more info

8fbc903

fix: recent bug

36c0c57

fix: maybe metric takes some time to be available

dada598

fix: beefy computers

122c7f0

fix: the bad patch

a77ecd5

fix: the proper file

6d39f59

fix: a bit too late

022bedc

fix: still too late

c120298

fix: same old bug again

1043bf8

fix: another failing test

d0d78be

fix: no need to kill nodes so slowly

760af21

gilcu3 commented Dec 12, 2025

View reviewed changes

fix: observers should be killed efficiently as well

4f07140

gilcu3 marked this pull request as ready for review December 12, 2025 20:01

chore: beef up actions building the node, there is a noticeable diffe…

9c6c7e7

…rence

		TRIPLES_TO_BUFFER = 20
		PRESIGNATURES_TO_BUFFER = 10

feat: support for robust ecdsa signature in the mpc-node #1645

Are you sure you want to change the base?

feat: support for robust ecdsa signature in the mpc-node #1645

Uh oh!

Conversation

gilcu3 commented Dec 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

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!

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!

gilcu3 Dec 12, 2025 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gilcu3 Dec 12, 2025 •

edited

Loading