feat: add parsigex

[varex83]

No description provided.

[emlautarom1]

Could you merge main to facilitate reviewing?

[varex83]

Found a few issues while testing with pluto, so converted to draft

[varex83agent]

Code Review

Summary

This PR adds the parsigex crate — a libp2p NetworkBehaviour/ConnectionHandler pair that implements the /charon/parsigex/2.0.0 partial-signature exchange protocol — together with proto conversion helpers in pluto-core and a multi-node example. The approach is sound and the structure closely follows the peerinfo pattern already in the repo. A few correctness and parity issues are noted below.

---

Findings

[Critical] Cargo.toml workspace path typo — build error

Impact: The workspace will fail to build because the path points to a non-existent directory.
Evidence: Cargo.toml:103 — pluto-parsigex = { path = "crates/parasigex" } (note "parasigex" vs actual dir crates/parsig**ex**). The workspace member on line 3 correctly spells "crates/parsigex".
Recommendation: path = "crates/parsigex"

---

[High] Max message size mismatch: parsigex uses 16 MB, Go uses 128 MB

Impact: Any cluster where a ParSigExMsg exceeds 16 MB will be rejected by the Rust node but accepted by Go peers. Large clusters or future duty types with verbose signed data may trigger this.
Evidence: crates/parsigex/src/protocol.rs:17 — const MAX_MESSAGE_SIZE: usize = 16 * 1024 * 1024;
Go reference: charon/p2p/sender.go:27 — maxMsgSize = 128 << 20 // 128MB
Recommendation: Use pluto_p2p::proto::MAX_MESSAGE_SIZE (128 MB, now exported) instead of the local constant, matching Go and the recent peerinfo unification.

---

[High] Handler only processes one inbound stream at a time — silent drop under concurrency

Impact: If a second inbound stream is fully negotiated while the first is still being read/verified, on_connection_event overwrites self.inbound, silently dropping the first receive future. Go's handler processes each inbound stream in its own goroutine; there is no such cap.
Evidence: crates/parsigex/src/handler.rs — inbound: Option<RecvFuture> is a single slot. ConnectionEvent::FullyNegotiatedInbound unconditionally assigns self.inbound = Some(recv_message(...)) with no check for an existing future.
Go reference: charon/p2p/receive.go:40-65 — SetStreamHandlerMatch forks a goroutine per stream.
Recommendation: Use a VecDeque<RecvFuture> (mirroring outbound_queue) and drive all pending inbound futures in poll, emitting events as each one completes.

---

[Medium] SignedDataError::Custom is not Send + Sync

Impact: Box<dyn std::error::Error> is not Send + Sync. Any async code that holds a SignedDataError::Custom across an await point will fail to compile.
Evidence: crates/core/src/signeddata.rs:51 — Custom(Box<dyn std::error::Error>)
Recommendation: Custom(Box<dyn std::error::Error + Send + Sync>)

---

[Medium] Signature error boxed as a fake serde_json::Error — API abuse

Impact: Error information is discarded and the type contract is violated. serde_json::Error::io is intended for I/O failures during JSON serialization, not arbitrary domain errors. Downstream callers catching ParSigExCodecError::Serialize for JSON-decode failures will also receive what are actually signature errors.
Evidence: crates/core/src/types.rs:597-601

let signature = data.signed_data.signature().map_err(|err| {
    ParSigExCodecError::Serialize(serde_json::Error::io(std::io::Error::other(
        err.to_string(),
    )))
})?;

Recommendation: Add a dedicated ParSigExCodecError::Signature(String) (or SignedDataError) variant, or re-use ParSigExCodecError::InvalidParSignedProto and attach the message via a separate field.

---

[Medium] Behaviour::poll processes at most one command per wake cycle

Impact: Under high broadcast load the command channel can starve: each poll call dequeues at most one Command from rx, even if many are queued. This can add per-message latency proportional to the queue depth.
Evidence: crates/parsigex/src/behaviour.rs — poll calls self.rx.poll_recv(cx) exactly once; handle_command pushes to pending_actions but only one action is returned per poll invocation (the check on pending_actions pops only one before returning Ready).
Recommendation: Drain rx in a while let Poll::Ready(Some(cmd)) loop (bounded by a budget, e.g. 32) before returning, similar to how libp2p's own behaviours handle this.

---

[Medium] .with_quic_enabled(true) added to relay-node builder without explanation

Impact: One Node::new_* variant now enables QUIC when it previously did not. QUIC-over-relay is not a standard libp2p transport combination and may cause unexpected connection failures or interop issues with Go peers.
Evidence: crates/p2p/src/p2p.rs:341 — .with_quic_enabled(true) added to the builder that uses .with_relay_client(...).
Recommendation: Confirm this is intentional and add a comment explaining why this variant needs QUIC enabled.

---

[Low] Redundant peer_id parameter in Behaviour::new

Impact: None at runtime, but the API is confusing — callers must pass the same value twice.
Evidence: crates/parsigex/src/behaviour.rs:167 — pub fn new(config: Config, peer_id: PeerId) while config.peer_id already holds the same value; enforced only via debug_assert_eq!.
Recommendation: Remove the second peer_id parameter and derive it from config.peer_id internally.

---

[Low] DutySentinel serialization is asymmetric

Impact: From<&DutyType> maps DutySentinel(_) => 14, but TryFrom<i32> maps 14 (and anything ≥ 14) to InvalidDuty. Sentinels are never transmitted so this has no operational impact, but it's a latent footgun.
Evidence: crates/core/src/types.rs:84 and types.rs:105.
Recommendation: Either exclude DutySentinel from the From impl (unreachable!/panic) or round-trip it correctly.

---

Parity Matrix

Component	Go	Rust	Match	Notes
Protocol ID	/charon/parsigex/2.0.0	/charon/parsigex/2.0.0	yes
Wire format	protobuf length-delimited	protobuf length-delimited	yes
Max message size	128 MB	16 MB	no	See High finding above
DutyType integer mapping	0–13	0–13	yes
Empty data set rejection	yes	yes	yes	InvalidParSignedDataSetFields
Duty validity check on recv	yes	yes	yes	duty_gater
Per-entry signature verification	yes	yes	yes	verifier callback
Concurrent inbound streams	yes (goroutine per stream)	no (single slot)	no	See High finding above
BuilderProposer rejected	yes (deprecated)	yes (DeprecatedBuilderProposer)	yes

---

Tests

Tests were not run locally. The example (crates/parsigex/examples/parsigex.rs) provides an integration-level smoke test for the broadcast path but requires a live multi-node setup. Unit tests for encode_message/decode_message round-trips and codec edge cases (empty set, unknown duty type, oversized message) appear absent — these would be worth adding.

---

Open Questions

1. Is the 16 MB cap intentional (memory-safety limit) or an oversight vs. Go's 128 MB?
2. Was .with_quic_enabled(true) on the relay node builder intentional? The other three Node::new_* builders in the same file were not changed.
3. SignedDataError::Custom is only added in signeddata.rs but never constructed in this PR — is it intended for follow-up work, or can it be deferred?