Skip to content

Fix snow error#194

Open
jCabala wants to merge 12 commits into
mainfrom
fix-snow-error
Open

Fix snow error#194
jCabala wants to merge 12 commits into
mainfrom
fix-snow-error

Conversation

@jCabala
Copy link
Copy Markdown
Member

@jCabala jCabala commented May 13, 2026

Session ID handling

  • Made --session (-S) required for the participant subcommand - participants must now always specify a session ID explicitly
  • Coordinator always prints the session ID on session creation, regardless of whether specific signers were provided
  • Recovery path: mina-frost-client sessions --group <group> lists active sessions with their IDs

Duplicate participant handling

  • Round 1: duplicate commitment from the same pubkey now logs a warning and is skipped; session continues
  • Round 2: decrypt failure (caused by a re-joined participant's mismatched Noise state) now logs a warning and is skipped; session continues
  • Neither case aborts the coordinator - other participants are unaffected

Tests

  • Added tests/duplicate_participant.rs - an integration test that spawns a duplicate participant mid-session and asserts the coordinator warns, does not crash, and produces a valid signature
  • Fixed existing sign_with_binary helper which was broken by the now-required --session flag; added get_session_id helper that polls the server for the active session UUID

@jCabala jCabala requested a review from scaraven May 13, 2026 09:50
Copy link
Copy Markdown
Member

@scaraven scaraven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All minor issues + cleanup. The main thing is that the silent warning/abort paths are inconsistent between commitment sending (round 1) and message sending (round 2) with no clear comment why.

Comment thread mina-frost-client/tests/assets-duplicate/0.toml Outdated
Comment thread mina-frost-client/src/coordinator/comms/http.rs Outdated
Comment thread mina-frost-client/src/coordinator/comms/http.rs Outdated
Comment thread mina-frost-client/src/coordinator/comms/http.rs
Comment thread mina-frost-client/src/coordinator/comms/http.rs Outdated
Comment thread mina-frost-client/tests/duplicate_participant.rs Outdated
Comment thread mina-frost-client/tests/duplicate_participant.rs
Comment on lines +129 to +134
thread::sleep(Duration::from_millis(1500));

// Spawn a duplicate of the first participant (fresh Noise context, same session)
let dupe_args = participant_args(&participants[0], server_url, group_pk_hex, &session_id);
let mut dupe = run_cli_spawn_piped(binary, cwd, &dupe_args);
thread::sleep(Duration::from_secs(2));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two sleep commands might be a bit flaky. The test will be more robust if we do something similiar as in get_session_id() where you poll until you see certain output that you expect + fail on a timeout. You can match on stderr messages such as Sending SigningPackage or Waiting for participants to send their SignatureShares.

However, that does mean that the test will rely on stderr output for robustness, that's not super futureproof so I'm 50/50 on whether it's worth implementing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants