Skip to content

DKG failure paths are untested: 119 uncovered lines in dkg.rs #966

Description

@kwsantiago

Summary

keep-frost-net/src/dkg.rs sits at 88.36% line coverage after #963: 119 production lines never execute under the full suite, including the relay-backed e2e tests. The gaps are concentrated in failure paths.

This did not block #963 because those paths fail closed (they return Err rather than admitting a bad roster), while every security invariant is covered. But it is worth closing, for a specific reason given below.

Why this matters more than a coverage percentage

Both of the genuinely broken features found during #963 review lived in code that no test executed:

  1. ClientTransport::fetch_events used Client::fetch_events, which exits on EOSE. DKG kinds are ephemeral, so relays never store them and peers could never see each other. 24 passing DKG tests could not catch it, because the test transport was a Vec<Event> that replays everything ever pushed.
  2. fetch_group_roster published the group announcement on kind 21101, also in the ephemeral range, so a published roster could never be fetched back. That function had zero coverage.

Both were found by looking at what was untested, not by running the suite. The remaining uncovered lines are the same kind of blind spot.

Where the 119 lines are

Function Uncovered lines What is untested
run_software_dkg 48 flood-abort (> MAX_DKG_EVENTS_SEEN), per-round timeout, cancellation, peer-rejection arms
fetch_group_roster 18 candidate retry loop and the #674 decoy-skip path (happy path now covered by tests/roster_fetch_test.rs)
parse_roster_from_event 16 missing threshold/participants tags, unparseable p-tag, hash-mismatch branch
connect_hardened 9 proxy branch, connect-timeout teardown
DkgCertificate::verify 8 rejection branches (wrong count, bad signature, transcript mismatch)
tail ~20 error-formatting arms across parse_pubkey, authenticates, dkg_transcript

Suggested work

  • Drive a mock notification stream past MAX_DKG_EVENTS_SEEN distinct non-self events in one round and assert the flood-abort actually fires. This is the highest-value one: the abort was briefly unreachable because the transport cap was set equal to the abort bound, and only a code reading caught it. There is now a const _: () = assert!(...) guarding the relationship, but nothing proves the abort path itself runs.
  • Table-test parse_roster_from_event over malformed announcements: each tag missing in turn, a non-hex p-tag, a deliberately mismatched d tag.
  • Cover DkgCertificate::verify's rejection arms, particularly a certificate carrying n-1 signatures and one carrying a signature over a different transcript.
  • Exercise connect_hardened's timeout teardown, which exists to avoid leaking a reconnecting websocket.

Measuring

cargo llvm-cov --package keep-frost-net --features testing --lib --tests --summary-only

Current baseline for dkg.rs: 1375 lines, 1215 covered, 88.36%.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestnostr-frostNostr FROST coordination protocolp2Priority

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions