Add 5-party networking and PRF key setup to ampc-actor-utils by marsenis · Pull Request #142 · worldcoin/ampc-common

marsenis · 2026-08-26T04:41:26Z

Generalizes the node-setup layer beyond the 3-party ring: NetworkSession gains Role-addressed send_to/receive_from, MpcNetworkHandle/PeerConnections now establish connections to any number of peers instead of exactly 2, and a new MeshControlChannel provides a Role-addressed control-plane channel (with an all-to-all sync barrier) alongside the untouched 3-party ControlChannel. Also fixes build_network_handle deriving only 3 local identities regardless of party count.

Adds two new shared-PRF key configurations for a 5-party protocol: ThresholdPrfKeys (a (3,5) config where k_{i,j} is known only to the 3 parties outside {i,j}) and PairwisePrfKeys (lambda_{i,j} known only to i and j), each with a symmetric XOR-combine network handshake in protocol::ops. All existing 3PC structs, signatures, and behavior are unchanged.

Generalizes the node-setup layer beyond the 3-party ring: NetworkSession gains Role-addressed send_to/receive_from, MpcNetworkHandle/PeerConnections now establish connections to any number of peers instead of exactly 2, and a new MeshControlChannel provides a Role-addressed control-plane channel (with an all-to-all sync barrier) alongside the untouched 3-party ControlChannel. Also fixes build_network_handle deriving only 3 local identities regardless of party count. Adds two new shared-PRF key configurations for a 5-party protocol: ThresholdPrfKeys (a (3,5) config where k_{i,j} is known only to the 3 parties outside {i,j}) and PairwisePrfKeys (lambda_{i,j} known only to i and j), each with a symmetric XOR-combine network handshake in protocol::ops. All existing 3PC structs, signatures, and behavior are unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

evagelia

Left a bunch of comments, in general looks good though!

evagelia · 2026-08-28T18:55:16Z

+    LOCAL_IDENTITY_NAMES[..n]
+        .iter()
+        .map(|name| Identity::from(*name))
+        .collect()


Is there a reason for making this general instead of just adding 3 more names in the previous function? Seems also that LOCAL_IDENTITY_NAMES is only used in this function.

I guess the generality was not necessary. I'm changing it to having two custom functions. One for 3PC identities and one for ORBIT5 identities to make sure no one accidentally uses this for 4 parties or some other value they're not supposed to.

evagelia · 2026-08-28T18:58:56Z

+/// [`crate::network::mpc::NetworkHandle::mesh_control_channel`] again to
+/// reconnect.
+#[async_trait]
+pub trait MeshControlChannel: Send {


Could we potentially avoid duplication from adding this if we just add send_to(role) and recv_from(role) in the existing ControlChannel trait?

This is what Claude had to say:

No hidden failures. Merging would mean a 5-party channel where send_next/recv_prev compile fine but always fail at runtime — exactly the kind of latent failure your engineering rules say to avoid. With separate traits, calling ring methods on a mesh channel is a compile error, not a production incident.

Downstream compatibility. ControlChannel is implemented outside this crate — iris-mpc-cpu's checkpoint protocol has its own InMemoryRing implementation of the trait in iris-mpc/iris-mpc-cpu/src/checkpoint_protocol/transport.rs. Adding required methods to ControlChannel breaks that implementation the moment iris-mpc bumps its pinned rev; adding them as defaulted methods that return errors just reintroduces the hidden-failure problem in every existing implementor.

The semantics genuinely differ, not just the arity. sync() on the ring is a 3-party barrier over two streams; on the mesh it's an all-to-all barrier over N−1 streams with deterministic role ordering. And "next"/"prev" have no canonical meaning in a mesh — a merged trait would force every caller to know which subset of the API is live for the object they hold.

The mesh trait already subsumes the ring. 5PC code addresses parties by Role via send/recv; there is nothing a merged trait would let it do that MeshControlChannel doesn't. The two traits partition cleanly: ring code holds a ControlChannel, N-party code holds a MeshControlChannel, and NetworkHandle hands out whichever one the caller asks for.

The 2nd and 3rd point convince me it's technically easier to keep the separate trait for now.

And this applies only to the control channel. This is different from the channels they use for data during the MPC protocol, which are non-blocking channels already in a mesh configuration.

evagelia · 2026-08-28T19:23:21Z

+
+/// Number of parties in the 5-party protocol configuration used by
+/// [`ThresholdPrfKeys`] and [`PairwisePrfKeys`].
+pub const FIVE_PARTY_COUNT: u8 = 5;


There's already a constant "pub const N_PARTIES: usize = 3;" in ampc-actor-utils/src/constants.rs, maybe we can use that to switch between 3 and 5 party versions?

It's tricky. If we use that to switch between a 3-party and a 5-party version, then unit testing becomes complicated because they'd need to override the constant. I was thinking that the switch from 3-parties to 5-parties will happen on the "front-end", the code on the DI repo that will eventually call the 5-party functions instead of the 3-party ones.
But I agree that the name FIVE_PARTY_COUNT is weird. I'll rename it to ORBIT5_PARTY_COUNT.

evagelia · 2026-08-28T20:50:22Z

+    for pair in PartyPair::excluding(own_role) {
+        let (a, b) = pair.parties();
+        let co_owners: Vec<Role> = (0..FIVE_PARTY_COUNT)
+            .map(|i| Role::new(i as usize))


nit: This is what the five_party_roles() in prf.rs does, maybe use that?

Good point. Fixed all those instances.

evagelia · 2026-08-28T20:50:59Z

+
+    let mut seeds = BTreeMap::new();
+    for other in (0..FIVE_PARTY_COUNT)
+        .map(|i| Role::new(i as usize))


nit: same as comment above, maybe use five_party_roles() here?

evagelia · 2026-08-28T23:11:56Z

+    }
+
+    let mut seeds = BTreeMap::new();
+    for pair in PartyPair::excluding(own_role) {


I think equivalent to this whole loop we can write smth like: (0..5).filter(|r| *r != own).array_combinations(); (from itertools).

Yeah, but I think it's better abstraction to hide the implementation under the excluding method. We could apply this change inside excluding() but since it's working fine right now let's keep it this way.

ORBIT5 is the internal name of the 5-party system. Also makes the roles helper public and replaces the manual (0..N).map(Role::new) constructions in the PRF key setup functions and tests with it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Restores the original 3-party generate_local_identities() and adds generate_local_identities_orbit5() with the five party names hardcoded, instead of slicing a shared list by count. build_network_handle now dispatches on the address count and rejects anything other than 3 or 5 parties explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

marsenis

Thanks for the review, @evagelia

marsenis · 2026-09-01T21:49:54Z

+
+/// Number of parties in the 5-party protocol configuration used by
+/// [`ThresholdPrfKeys`] and [`PairwisePrfKeys`].
+pub const FIVE_PARTY_COUNT: u8 = 5;


It's tricky. If we use that to switch between a 3-party and a 5-party version, then unit testing becomes complicated because they'd need to override the constant. I was thinking that the switch from 3-parties to 5-parties will happen on the "front-end", the code on the DI repo that will eventually call the 5-party functions instead of the 3-party ones.
But I agree that the name FIVE_PARTY_COUNT is weird. I'll rename it to ORBIT5_PARTY_COUNT.

marsenis · 2026-09-01T22:17:42Z

+    }
+
+    let mut seeds = BTreeMap::new();
+    for pair in PartyPair::excluding(own_role) {


Yeah, but I think it's better abstraction to hide the implementation under the excluding method. We could apply this change inside excluding() but since it's working fine right now let's keep it this way.

marsenis · 2026-09-01T22:17:55Z

+    for pair in PartyPair::excluding(own_role) {
+        let (a, b) = pair.parties();
+        let co_owners: Vec<Role> = (0..FIVE_PARTY_COUNT)
+            .map(|i| Role::new(i as usize))


Good point. Fixed all those instances.

marsenis · 2026-09-01T22:35:07Z

+/// [`crate::network::mpc::NetworkHandle::mesh_control_channel`] again to
+/// reconnect.
+#[async_trait]
+pub trait MeshControlChannel: Send {


This is what Claude had to say:

No hidden failures. Merging would mean a 5-party channel where send_next/recv_prev compile fine but always fail at runtime — exactly the kind of latent failure your engineering rules say to avoid. With separate traits, calling ring methods on a mesh channel is a compile error, not a production incident.

Downstream compatibility. ControlChannel is implemented outside this crate — iris-mpc-cpu's checkpoint protocol has its own InMemoryRing implementation of the trait in iris-mpc/iris-mpc-cpu/src/checkpoint_protocol/transport.rs. Adding required methods to ControlChannel breaks that implementation the moment iris-mpc bumps its pinned rev; adding them as defaulted methods that return errors just reintroduces the hidden-failure problem in every existing implementor.

The semantics genuinely differ, not just the arity. sync() on the ring is a 3-party barrier over two streams; on the mesh it's an all-to-all barrier over N−1 streams with deterministic role ordering. And "next"/"prev" have no canonical meaning in a mesh — a merged trait would force every caller to know which subset of the API is live for the object they hold.

The mesh trait already subsumes the ring. 5PC code addresses parties by Role via send/recv; there is nothing a merged trait would let it do that MeshControlChannel doesn't. The two traits partition cleanly: ring code holds a ControlChannel, N-party code holds a MeshControlChannel, and NetworkHandle hands out whichever one the caller asks for.

The 2nd and 3rd point convince me it's technically easier to keep the separate trait for now.

And this applies only to the control channel. This is different from the channels they use for data during the MPC protocol, which are non-blocking channels already in a mesh configuration.

marsenis · 2026-09-01T22:46:59Z

+    LOCAL_IDENTITY_NAMES[..n]
+        .iter()
+        .map(|name| Identity::from(*name))
+        .collect()


I guess the generality was not necessary. I'm changing it to having two custom functions. One for 3PC identities and one for ORBIT5 identities to make sure no one accidentally uses this for 4 parties or some other value they're not supposed to.

gayathrigarimella

everything looks good to me!

marsenis requested review from evagelia and gayathrigarimella August 26, 2026 04:41

evagelia reviewed Aug 31, 2026

View reviewed changes

marsenis and others added 2 commits September 1, 2026 15:06

marsenis commented Sep 1, 2026

View reviewed changes

gayathrigarimella approved these changes Sep 2, 2026

View reviewed changes

marsenis merged commit 4a1fb29 into tfhard-5pc Sep 2, 2026
10 checks passed

marsenis deleted the tfhard/claude/networking-prf branch September 2, 2026 18:50

Uh oh!

Conversation

marsenis commented Aug 26, 2026

Uh oh!

evagelia left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marsenis left a comment

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!

Choose a reason for hiding this comment

Uh oh!

gayathrigarimella left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants