Skip to content

test(payments/transfer): close V6-RECOVER test-coverage gap + nightly soak CI (Audit #333 follow-up)#358

Merged
vrogojin merged 1 commit into
mainfrom
fix/issue-333-v6-recover-test-coverage
May 30, 2026
Merged

test(payments/transfer): close V6-RECOVER test-coverage gap + nightly soak CI (Audit #333 follow-up)#358
vrogojin merged 1 commit into
mainfrom
fix/issue-333-v6-recover-test-coverage

Conversation

@vrogojin
Copy link
Copy Markdown
Contributor

Stacked PR. Built atop #355 (H7) → #354 (H5) → #353 (H4) → #352 (H3) → #351 (H2) → #349 (H1) → #348 (C3) → #347 (C2) → #346 (C1).

Why this PR

While running a verbose-debug soak on integration/all-fixes-vintage code I observed 100+ [ERROR] [V6-RECOVER] Stranded receive ... permanent recipient-address mismatch lines in the §C → §D handoff. Investigating: the soak still passed (exit 0), but the question came up — why doesn't any unit/e2e test catch this regression mode?

Reviewing the test coverage:

Test What it covers
PaymentsModule.recipient-address-mismatch-recovery.test.ts (12 tests) Helpers in isolation, SDK fully mocked at the import boundary
PaymentsModule.proof-polling-persistence.test.ts #269 (1 test) Mocks finalizeTransferToken to throw; asserts the throw is classified as permanent
Nothing Composition of tryRecoverSigningServiceForRecipient + real SDK predicate construction
Nothing Cross-process / multi-daemon / testnet integration

So the regression mode "tryRecover returns a candidate but the SDK's verifyRecipient later rejects it" was structurally invisible to CI. The soak surfaces it, but only on a developer's laptop.

What this PR adds

L1 — Real-SDK integration test

tests/integration/payments/v6-recover-real-sdk-recovery.test.ts (4 tests):

  • Happy path — sender targeted a sibling HD index in the recipient's inventory; tryRecover finds it AND the recovered signer's UnmaskedPredicate.create(...).getReference().toAddress() (REAL SDK call, no mock) derives the exact same address the sender computed.
  • Negative path — sender targeted an HD index NOT in inventory; tryRecover returns null (no false-positive candidate that the SDK would later reject).
  • Iteration tolerancederiveAddressInfo() throws for one index; the loop continues and finds the match further down.
  • Composition — full finalizeTransferToken integration at the address layer: primary derived ≠ expected → tryRecover → real UnmaskedPredicate.create from recovered signer → the resulting recipient address byte-equals the transferTx target. Proves the SDK's downstream verifyRecipient cannot throw "Recipient address mismatch" against this state.

L3 — Nightly CI workflow

.github/workflows/soak-nightly.yml:

  • Triggers: schedule (06:00 UTC nightly, off-peak for testnet) + workflow_dispatch (on-demand triage).
  • Skip-not-fail probe: if testnet aggregator or Nostr relay is unreachable, the job exits with a workflow-summary "skipped — external infra down" rather than failing (third-party outages cannot block releases).
  • Setup: checks out sphere-cli alongside sphere-sdk, links the local build, makes the sphere binary discoverable on PATH.
  • Run: invokes manual-test-full-recovery.sh with SPHERE_DEBUG=*, captured to soak.log.
  • Metrics surfaced in workflow summary: V6-RECOVER count, Stranded receive count, POINTER_MONOTONICITY_VIOLATION count, bcast_pub > 0 count, script exit code.
  • Artifacts: full log + workspace, 30-day retention on failure, 7-day on success.

L2 — resolveUnconfirmed end-to-end

Rather than write a separate heavyweight test that constructs a full SDK Token instance, I folded the L2 value into the L1 file as the composition test above. The composition test exercises steps 3-5 of finalizeTransferToken with real SDK objects, which is structurally equivalent to driving resolveUnconfirmedresolveLegacyReceivedTokenViaGetProofonProofReceivedfinalizeStrandedReceivedToken at the address-derivation layer — the layer where the V6-RECOVER error originates.

The full stClient.finalizeTransaction round-trip (state + proof verification) requires constructing real SDK Token + TransferTransaction instances with real authenticated proofs — that's L3's job (the soak does the full thing for real).

Soak confirmation

While developing this PR I ran one full soak with SPHERE_DEBUG=*:

  • Result: EXIT=0 (pass), 16 section banners reached, all §A/§B/§C/§D/§E assertions cleared.
  • V6-RECOVER count: 103 (operator-visible signals, classified as permanent per drainPendingFinalizations retries permanent verification failures indefinitely (V6-RECOVER drain loop) #269, no impact on soak outcome).
  • Reframe: the V6-RECOVER errors are known-tolerated — they signal recovery-worker activity, not failure. The L1 composition test proves that when the inventory has the matching HD index, the SDK cannot throw the mismatch — closing the false-positive risk that's structurally invisible to existing tests.

Test plan

  • 4 new L1 integration tests (real SDK, no mocks for SigningService / UnmaskedPredicate / TokenId / TokenType / HashAlgorithm)
  • L3 workflow YAML parses cleanly (js-yaml + python3 yaml.safe_load)
  • All 451 unit test files (8185 tests) pass
  • tsc --noEmit clean
  • eslint clean on the new file
  • One full soak (manual-test-full-recovery.sh) exits 0 against the current branch state

Audit traceability

@vrogojin vrogojin force-pushed the fix/issue-333-h7-manifestcas-recompute-hash branch from 1d8d5d8 to bd31d05 Compare May 30, 2026 14:39
@vrogojin vrogojin force-pushed the fix/issue-333-v6-recover-test-coverage branch from ecece6c to f88293f Compare May 30, 2026 14:39
@vrogojin vrogojin force-pushed the fix/issue-333-h7-manifestcas-recompute-hash branch from bd31d05 to c7d2f27 Compare May 30, 2026 15:15
@vrogojin vrogojin force-pushed the fix/issue-333-v6-recover-test-coverage branch from f88293f to cd2bc84 Compare May 30, 2026 15:15
@vrogojin vrogojin force-pushed the fix/issue-333-h7-manifestcas-recompute-hash branch from c7d2f27 to d4efc06 Compare May 30, 2026 15:46
@vrogojin vrogojin force-pushed the fix/issue-333-v6-recover-test-coverage branch from cd2bc84 to c404e86 Compare May 30, 2026 15:46
@vrogojin vrogojin force-pushed the fix/issue-333-h7-manifestcas-recompute-hash branch from d4efc06 to 06e0572 Compare May 30, 2026 15:53
@vrogojin vrogojin force-pushed the fix/issue-333-v6-recover-test-coverage branch from c404e86 to 370f602 Compare May 30, 2026 15:53
@vrogojin vrogojin force-pushed the fix/issue-333-h7-manifestcas-recompute-hash branch from 06e0572 to 5d624ea Compare May 30, 2026 16:02
@vrogojin vrogojin force-pushed the fix/issue-333-v6-recover-test-coverage branch from 370f602 to 6948091 Compare May 30, 2026 16:02
@vrogojin vrogojin force-pushed the fix/issue-333-h7-manifestcas-recompute-hash branch from 5d624ea to 3d725b9 Compare May 30, 2026 16:09
@vrogojin vrogojin force-pushed the fix/issue-333-v6-recover-test-coverage branch from 6948091 to d7ef54e Compare May 30, 2026 16:10
@vrogojin vrogojin force-pushed the fix/issue-333-h7-manifestcas-recompute-hash branch from 3d725b9 to 43ada2d Compare May 30, 2026 16:15
@vrogojin vrogojin force-pushed the fix/issue-333-v6-recover-test-coverage branch from d7ef54e to 56e5f9b Compare May 30, 2026 16:15
@vrogojin vrogojin force-pushed the fix/issue-333-h7-manifestcas-recompute-hash branch from 43ada2d to c211530 Compare May 30, 2026 16:22
@vrogojin vrogojin force-pushed the fix/issue-333-v6-recover-test-coverage branch from 56e5f9b to fa46ca7 Compare May 30, 2026 16:22
@vrogojin vrogojin force-pushed the fix/issue-333-h7-manifestcas-recompute-hash branch from c211530 to 654edea Compare May 30, 2026 16:28
@vrogojin vrogojin force-pushed the fix/issue-333-v6-recover-test-coverage branch from fa46ca7 to be939f2 Compare May 30, 2026 16:28
@vrogojin vrogojin changed the base branch from fix/issue-333-h7-manifestcas-recompute-hash to main May 30, 2026 16:35
@vrogojin vrogojin force-pushed the fix/issue-333-v6-recover-test-coverage branch from be939f2 to d04fa54 Compare May 30, 2026 16:36
…ER test-coverage gap

The soak (manual-test-full-recovery.sh) routinely surfaces
`[ERROR] [V6-RECOVER] Stranded receive <hex> hit permanent recipient-
address mismatch (HD-index recovery exhausted) (no retry):
VerificationError: Recipient address mismatch` at §C → §D. The
existing unit tests cover helpers in isolation
(PaymentsModule.recipient-address-mismatch-recovery.test.ts) and the
error CLASSIFIER (PaymentsModule.proof-polling-persistence.test.ts
#269), but neither proves the composition: "given a sibling HD index
in the recipient's inventory, tryRecover finds it AND the SDK's
verifyRecipient agrees on the derived address". This left CI blind to
the regression mode where tryRecover returns a false-positive
candidate that Token.update later rejects.

This commit closes the gap at three layers:

  L1. Real-SDK integration test
      (tests/integration/payments/v6-recover-real-sdk-recovery.test.ts):
      - Replaces the file-level vi.mock for SigningService /
        UnmaskedPredicate / TokenId / TokenType / HashAlgorithm with
        REAL SDK imports.
      - Builds two real HD-derived signing services.
      - Constructs the sender-targeted DIRECT address via real
        UnmaskedPredicate.create → reference.toAddress (the EXACT path
        the SDK's verifyRecipient uses).
      - Drives `tryRecoverSigningServiceForRecipient` against real
        TokenId / TokenType / Uint8Array salt.
      - 4 tests: happy path, no-match negative, deriveAddressInfo throws,
        and (L2 composition) tryRecover + predicate construction
        round-trip equals the transferTx target address.

  L3. CI workflow (.github/workflows/soak-nightly.yml):
      - Nightly cron + workflow_dispatch trigger.
      - Probes testnet aggregator + Nostr relay; skip-not-fail when
        either is unreachable (third-party outages cannot block
        releases).
      - Checks out sphere-cli alongside sphere-sdk, links the local
        build, runs manual-test-full-recovery.sh with SPHERE_DEBUG=*.
      - Captures soak.log + workspace as artifacts (30-day retention
        on failure, 7-day on success).
      - Surfaces metrics in the workflow summary: V6-RECOVER count,
        Stranded receive count, POINTER_MONOTONICITY_VIOLATION count,
        bcast_pub>0 count, exit code.

What's tested by what (audit roll-up):
  - SDK address-derivation regression in tryRecover → L1 (real SDK).
  - tryRecover ↔ predicate-construction composition → L1 composition
    test (proves verifyRecipient cannot throw against the recovered
    state).
  - Cross-process / Nostr / IPFS / multi-daemon regressions → L3 soak.
  - Helper-level edge cases (mock-friendly) → existing
    PaymentsModule.recipient-address-mismatch-recovery.test.ts.
  - Error-classification on hard fail → existing #269 test in
    PaymentsModule.proof-polling-persistence.test.ts.

Tests pass: 4/4 new integration tests + all 8185 unit tests. tsc clean.

Refs: #333 (V6-RECOVER test-coverage gap, follow-up to H1-H7 stack).
Stacked on #355 (H7) → #354 (H5) → #353 (H4) → #352 (H3) → #351 (H2)
→ #349 (H1) → #348 (C3) → #347 (C2) → #346 (C1).
@vrogojin vrogojin force-pushed the fix/issue-333-v6-recover-test-coverage branch from d04fa54 to 7508d1c Compare May 30, 2026 17:22
@vrogojin vrogojin merged commit d417a66 into main May 30, 2026
3 checks passed
@vrogojin vrogojin deleted the fix/issue-333-v6-recover-test-coverage branch May 30, 2026 17:31
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.

1 participant