test: Add ParSigEx end-to-end signature exchange test#468
Conversation
Adds the first automated end-to-end test for partial signature exchange: four nodes connect over real loopback-TCP libp2p, each signs a shared message with its own threshold-BLS share, and broadcasts the partial through the `ParSigEx` protocol. One node then aggregates the threshold of partials it received over the network and verifies the result against the group public key, proving the exchange-and-aggregation path works end to end. The test is self-contained (no relay, no external network) and reuses the existing `ParSigEx` behaviour as-is. Also adds `pluto-crypto`, `pluto-testutil`, and `k256` as `dev-dependencies`.
emlautarom1
left a comment
There was a problem hiding this comment.
Few things could be improved. Main issue right now is that the test swallows certain errors, and the timeout is extremely generous.
| async fn with_timeout<F, T>(future: F, msg: &'static str) -> Result<T> | ||
| where | ||
| F: Future<Output = Result<T>>, | ||
| { | ||
| timeout(TEST_TIMEOUT, future).await.context(msg)? | ||
| } |
There was a problem hiding this comment.
nit: I don't think this helper provides much value.
There was a problem hiding this comment.
Also, many operations use this timeout which is quite large (30s), meaning that the full test could run for multiple minutes. Prefer a per-case timeout
| async fn broadcast_partial( | ||
| handle: Handle, | ||
| duty: Duty, | ||
| data_set: ParSignedDataSet, | ||
| ) -> parsigex::Result<u64> { | ||
| handle.broadcast_and_wait(duty, data_set).await | ||
| } |
There was a problem hiding this comment.
Single usage, prefer to inline it
| /// Each node signs [`MSG`] with its share and broadcasts it via ParSigEx. | ||
| fn broadcast_all(&self) -> Result<Vec<JoinHandle<parsigex::Result<u64>>>> { | ||
| let duty = Duty::new(SlotNumber::new(SLOT), DutyType::Randao); | ||
| let mut tasks = Vec::with_capacity(self.running.len()); |
There was a problem hiding this comment.
This should be a JoinSet
| use pluto_parsigex::{self as parsigex, DutyGater, Event, Handle, Verifier}; | ||
| use pluto_testutil::random::{generate_insecure_k1_key, generate_test_bls_key}; | ||
| use tokio::{ | ||
| spawn, |
There was a problem hiding this comment.
Prefer to not import standalone functions, instead use them with their module (ex. tokio::spawn instead of importing spawn)
|
|
||
| /// Spawns each node's swarm loop, forwarding listen/connection/receive events. | ||
| fn spawn_nodes(bundles: Vec<NodeBundle>, sinks: &EventSinks) -> Vec<RunningNode> { | ||
| let mut running = Vec::with_capacity(bundles.len()); |
| /// Collects partials addressed to `receiver` until the threshold is met. | ||
| async fn collect_partials(&mut self, receiver: usize) -> Result<HashMap<u64, Signature>> { | ||
| with_timeout( | ||
| self.recv_threshold_partials(receiver), | ||
| "timed out collecting partial signatures", | ||
| ) | ||
| .await | ||
| } |
There was a problem hiding this comment.
Inline this with the comment, it's a single call used only once
| let received = harness.collect_partials(0).await?; | ||
| ensure!( | ||
| received.len() == THRESHOLD, | ||
| "node 0 should receive exactly {THRESHOLD} partials, got {}", |
There was a problem hiding this comment.
As it is, this ensure! is not useful:
- We receive one partial signature at the time, until it receives
THRESHOLD - Early return happens on panic, internal error or timeout (.await?)
Instead, the timeout should be handled inside recv_threshold_partials so that on timeout we can actually print the number of received shares (on error/panic we propagate which is fine already)
| ); | ||
|
|
||
| harness.aggregate_and_verify(&received)?; | ||
| await_broadcasts(broadcasts).await?; |
There was a problem hiding this comment.
This is handled better by JoinSet
| index: usize, | ||
| sinks: EventSinks, | ||
| mut dial_rx: mpsc::UnboundedReceiver<Vec<Multiaddr>>, | ||
| mut stop_rx: oneshot::Receiver<()>, |
There was a problem hiding this comment.
You can use a CancellationToken here for consistency, and then await the join handle at the end.
Note that this is not needed, everything will be dropped at the end of the scope
| if let Ok(signature) = data.signed_data.signature() { | ||
| let _ = sinks.recv_tx.send((index, data.share_idx, signature)); | ||
| } |
There was a problem hiding this comment.
We're ignoring potential errors on .signature(). We should probably propagate them at the caller, or panic.
Adds the first automated end-to-end test for partial signature exchange: four nodes connect over real loopback-TCP libp2p, each signs a shared message with its own threshold-BLS share, and broadcasts the partial through the
ParSigExprotocol.One node then aggregates the threshold of partials it received over the network and verifies the result against the group public key, proving the exchange-and-aggregation path works end to end.
The test is self-contained (no relay, no external network) and reuses the existing
ParSigExbehaviour as-is. Also addspluto-crypto,pluto-testutil, andk256asdev-dependencies.