Chore/e2e coverage

[d4rp4t]

test(e2e): Wave 1 & 2 E2E coverage — rejection tests, multi-input, chain swap security

Completes the Wave 1 & 2 test items from the E2E coverage audit #88 .

New tests

Rejection / negative paths

• CollabExit_WithBoardingInput_IsRejected — confirms arkd rejects a collaborative exit that uses a boarding UTXO (on-chain, not yet settled into the VTXO tree) as input
• SubdustVtxo_SpendOffchain_IsRejected — subdust VTXO spend rejected by arkd
• SubdustVtxo_IntentScheduler_SkipsSettle — scheduler skips subdust VTXOs
• NoteRedemption_IsRejected_WhenNoteAlreadyRedeemed — second redemption of an already-spent Arkade note is cancelled with VTXO-spent reason

Batch / settlement

• TwoWallets_ConcurrentSettle_HaveIdenticalCommitmentTxId — two independent wallets settling in the same batch share the same commitment txid
• MultiInputOffchainTx_ProducesOneCheckpointPerInput — offchain tx with N input VTXOs produces exactly N checkpoint transactions (1-per-input protocol invariant)

Unilateral exit

• ProgressExits_AdvancesFromBroadcastingToAwaitingCsvDelay
• ProgressExits_WorksForOffchainFundedVtxo — full Broadcasting → CSV → Completed path
• ProgressExits_WorksForPreconfirmedVtxo
• AwaitingCsvDelay_DoesNotAdvanceUntilDelayMatures — pre-CSV CompleteUnroll blocked

Chain swaps

• Extended CanDoBtcToArkChainSwap with cross-sign BTC claim assertion — verifies Boltz actually swept the BTC lockup address after we revealed the preimage
• BtcToArkChainSwap_RejectsWhenBoltzReturnsWrongLockupAddress — SDK rejects swap creation when Boltz returns a lockup address that doesn't match our locally-computed VHTLC address (anti-scam check)

Known issue (documented, test ignored)

SameNote_ConcurrentRedemption_ExactlyOneSucceeds — written and ignored. AlreadyLockedVtxoException recovery uses a VTXO-key-based DeleteProof, so the second host evicts the first host's registration. The evicted intent hangs in WaitingForBatch indefinitely.

Bug fix

• ChainSwapTests: BTC amount formatting now uses InvariantCulture — Polish locale was producing 0,00051232 instead of 0.00051232, breaking bitcoin-cli sendtoaddress.

[arkanaai]

Review: Chore/e2e coverage (Wave 2)

Test-only PR — no production code changes. All modifications are in NArk.Tests.End2End/ and NArk.Tests/. Safe to merge.

What's good

• Rejection/negative-path coverage is excellent. CollabExit_WithBoardingInput_IsRejected, SubdustVtxo_SpendOffchain_IsRejected, NoteRedemption_IsRejected_WhenNoteAlreadyRedeemed — these are the tests that catch real-money bugs before they hit mainnet.
• BtcToArkChainSwap_RejectsWhenBoltzReturnsWrongLockupAddress — security-critical anti-scam check. The MockBoltzServer deliberately returns a wrong address and the SDK must reject. Well structured.
• MockBoltzServer is comprehensive and well-documented. The VHTLC address computation mirrors production code correctly (sender/receiver ordering per swap type). The WebSocket session management with SemaphoreSlim + copy-then-iterate pattern is correct.
• TestWaiter eliminates ad-hoc deadline loops scattered across tests — good DRY.
• InvariantCulture fix (ChainSwapTests.cs:108,326) — real bug. Polish locale 0,00051232 would break bitcoin-cli sendtoaddress. Correct fix.
• BoltzSwapProvider404SafetyNetTests — tests the 404 safety-net at threshold=10 with proper counter-reset verification. Good use of NSubstitute.
• Unilateral exit tests re-enabled (removed [Ignore] from ProgressExits_AdvancesFromBroadcastingToAwaitingCsvDelay and AwaitingCsvDelay_DoesNotAdvanceUntilDelayMatures). The ProgressExits_WorksForPreconfirmedVtxo is correctly left [Ignore] with a clear explanation of the missing ErrWaitingForConfirmation retry.
• SameNote_ConcurrentRedemption_ExactlyOneSucceeds — correctly [Ignore]d with a documented bug (evicted intent hangs in WaitingForBatch). Good practice to write the test and document the known issue.

Minor nits (non-blocking)

1. PR description mentions RecoversPendingTx_WhenFinalizeNeverFollowedSubmit but this test is not present in the diff. Either it was dropped during development or it's in a commit that didn't make it. Please update the description or add the test.

2. SubdustRejectionTests.cs:95-98 — Assert.CatchAsync catches any exception type. If Spend() throws for a reason unrelated to dust (e.g., network error, null ref), the test still passes. Consider Assert.ThrowsAsync<SpecificException> or at minimum assert on the exception message:

   Assert.That(ex!.Message, Does.Contain("dust").IgnoreCase);

3. CheckpointSpyTransport (BatchSessionTests.cs:200-255) — manual decorator over all 22 IClientTransport methods. If the interface gains a method, this breaks silently at compile time rather than at test time. Not urgent but consider a comment // If this fails to compile, add the new method here or use a source-generated proxy.

4. BoltzSwapProvider404SafetyNetTests.cs:504 — the inner class DelegatingHandler shadows System.Net.Http.DelegatingHandler. No functional issue since it's private sealed, but FakeHandler or LambdaHandler would avoid confusion during debugging.

5. Missing newline at EOF in BatchSessionTests.cs, NoteTests.cs, OnchainTests.cs — \ No newline at end of file. Trivial but some tools complain.

Protocol-critical assessment

No production code touching VTXOs, signing, forfeit paths, round lifecycle, or exit paths is modified. All changes are test infrastructure. The UnilateralExitTests exercise protocol-critical paths but only as consumers — the exit logic itself is unchanged.

Verdict: Approve. Ship it. 🚀

---

Reviewed by Arkana (automated, model: opus) — @d4rp4t please address nit #1 (missing test in description) before merge.

[arkanaai]

Review: Chore/e2e coverage (Wave 2)

Test-only PR — no production code changes. All modifications are in NArk.Tests.End2End/ and NArk.Tests/. Safe to merge.

What's good

• Rejection/negative-path coverage is excellent. CollabExit_WithBoardingInput_IsRejected, SubdustVtxo_SpendOffchain_IsRejected, NoteRedemption_IsRejected_WhenNoteAlreadyRedeemed — these are the tests that catch real-money bugs before they hit mainnet.
• BtcToArkChainSwap_RejectsWhenBoltzReturnsWrongLockupAddress — security-critical anti-scam check. The MockBoltzServer deliberately returns a wrong address and the SDK must reject. Well structured.
• MockBoltzServer is comprehensive and well-documented. The VHTLC address computation mirrors production code correctly (sender/receiver ordering per swap type). The WebSocket session management with SemaphoreSlim + copy-then-iterate pattern is correct.
• TestWaiter eliminates ad-hoc deadline loops scattered across tests — good DRY.
• InvariantCulture fix (ChainSwapTests.cs:108,326) — real bug. Polish locale 0,00051232 would break bitcoin-cli sendtoaddress. Correct fix.
• BoltzSwapProvider404SafetyNetTests — tests the 404 safety-net at threshold=10 with proper counter-reset verification. Good use of NSubstitute.
• Unilateral exit tests re-enabled (removed [Ignore] from ProgressExits_AdvancesFromBroadcastingToAwaitingCsvDelay and AwaitingCsvDelay_DoesNotAdvanceUntilDelayMatures). The ProgressExits_WorksForPreconfirmedVtxo is correctly left [Ignore] with a clear explanation of the missing ErrWaitingForConfirmation retry.
• SameNote_ConcurrentRedemption_ExactlyOneSucceeds — correctly [Ignore]d with a documented bug (evicted intent hangs in WaitingForBatch). Good practice to write the test and document the known issue.

Minor nits (non-blocking)

1. PR description mentions RecoversPendingTx_WhenFinalizeNeverFollowedSubmit but this test is not present in the diff. Either it was dropped during development or it's in a commit that didn't make it. Please update the description or add the test.

2. SubdustRejectionTests.cs:95-98 — Assert.CatchAsync catches any exception type. If Spend() throws for a reason unrelated to dust (e.g., network error, null ref), the test still passes. Consider Assert.ThrowsAsync<SpecificException> or at minimum assert on the exception message:

   Assert.That(ex!.Message, Does.Contain("dust").IgnoreCase);

3. CheckpointSpyTransport (BatchSessionTests.cs:200-255) — manual decorator over all 22 IClientTransport methods. If the interface gains a method, this breaks at compile time which is fine, but consider a comment noting the maintenance cost.

4. BoltzSwapProvider404SafetyNetTests.cs:504 — the inner class DelegatingHandler shadows System.Net.Http.DelegatingHandler. No functional issue since it's private sealed, but FakeHandler or LambdaHandler would avoid confusion during debugging.

5. Missing newline at EOF in BatchSessionTests.cs, NoteTests.cs, OnchainTests.cs — \ No newline at end of file. Trivial but some tools complain.

Protocol-critical assessment

No production code touching VTXOs, signing, forfeit paths, round lifecycle, or exit paths is modified. All changes are test infrastructure. The UnilateralExitTests exercise protocol-critical paths but only as consumers — the exit logic itself is unchanged.

Verdict: Approve. Ship it. 🚀

---

Reviewed by Arkana (automated, model: opus) — @d4rp4t please address nit #1 (missing test in description) before merge.

[arkanaai]

Follow-up review (commit 4db1854)

All 5 nits from my previous review addressed:

#	Nit	Status
1	PR description referenced missing RecoversPendingTx test	✅ Removed from description
2	SubdustRejectionTests — Assert.CatchAsync too broad	✅ Added Does.Contain("dust").IgnoreCase assertion
3	CheckpointSpyTransport missing maintenance comment	✅ Comment added explaining intentional compile-time breakage
4	DelegatingHandler name shadowed System.Net.Http.DelegatingHandler	✅ Renamed to FakeHandler
5	Missing newline at EOF in 3 files	✅ Fixed

No new issues. Still test-only — no production code changes.

Verdict: Approve. ✅

---

Reviewed by Arkana (automated, model: opus)