Skip to content

fix(hooks): add tx confirmation + signature validation to useInsuranceLP#154

Open
6figpsolseeker wants to merge 1 commit intodcccrypto:mainfrom
6figpsolseeker:fix/insurance-lp-tx-confirmation
Open

fix(hooks): add tx confirmation + signature validation to useInsuranceLP#154
6figpsolseeker wants to merge 1 commit intodcccrypto:mainfrom
6figpsolseeker:fix/insurance-lp-tx-confirmation

Conversation

@6figpsolseeker
Copy link
Copy Markdown

@6figpsolseeker 6figpsolseeker commented Apr 6, 2026

Summary

  • Both depositInsuranceLP and withdrawInsuranceLP returned immediately after signAndSend() without confirming the transaction landed on-chain. Users saw "success" even when transactions were dropped due to network congestion or expired blockhashes.
  • Adds confirmTransactionSafe() calls after signAndSend() in both deposit and withdraw paths — matching the existing pattern in useTrade.ts and useCollateral.ts
  • Destructures lastValidBlockHeight alongside blockhash from getLatestBlockhash() (required for slot-based TTL validation)
  • Adds null-check on signatures[0] before passing to confirmation (guards against MWA returning empty signatures array)

What changed

File Change
src/hooks/useInsuranceLP.ts Import confirmTransactionSafe, destructure lastValidBlockHeight, add signature null-check + confirmation call in both deposit & withdraw
__tests__/hooks/useInsuranceLP.test.ts Mock confirmTransactionSafe, add 4 new tests: confirmation args validation + on-chain error propagation for both paths

Why this matters

Without transaction confirmation, the app reports success to the user after MWA signing — but the transaction may never land on-chain. This is especially dangerous for insurance LP operations where users expect their collateral to be deposited/withdrawn. On a congested network, dropped transactions would silently fail while the UI shows a success toast.

Test plan

  • All 21 useInsuranceLP tests pass (17 existing + 4 new)
  • Full hook test suite passes (useTrade, useCollateral, useStake, useMWA, usePriceStream)
  • Manual test: deposit insurance LP on devnet, verify confirmation is awaited
  • Manual test: withdraw insurance LP on devnet, verify on-chain error surfaces

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Strengthened transaction signature validation to ensure signatures are properly received before confirmation.
    • Enhanced error handling for on-chain transaction failures with improved error messaging.
    • Added explicit confirmation validation for deposit and withdrawal operations.
  • Tests

    • Expanded test coverage for transaction confirmation scenarios and failure cases.

Both depositInsuranceLP and withdrawInsuranceLP returned immediately
after signAndSend without confirming the transaction landed on-chain.
Users saw "success" even when transactions were dropped due to network
congestion or expired blockhashes.

Changes:
- Destructure lastValidBlockHeight alongside blockhash from
  getLatestBlockhash (needed for slot-based TTL validation)
- Call confirmTransactionSafe after signAndSend to detect on-chain
  failures and dropped transactions
- Add null-check on signatures[0] before passing to confirmation
  (guards against MWA returning empty signatures array)
- Add 4 new tests covering confirmation call args and on-chain error
  propagation for both deposit and withdraw paths

Matches the established pattern already used in useTrade.ts and
useCollateral.ts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 6, 2026

📝 Walkthrough

Walkthrough

Added transaction confirmation validation to insurance LP deposit and withdraw operations. The hooks now capture lastValidBlockHeight alongside blockhash, validate that signatures are returned, call confirmTransactionSafe to verify transactions have landed on-chain, and handle confirmation failures by returning null and setting error states. Corresponding test cases verify this behavior.

Changes

Cohort / File(s) Summary
Test Coverage
__tests__/hooks/useInsuranceLP.test.ts
Added Jest mock for confirmTransactionSafe with reset in beforeEach. Extended deposit and withdraw test suites to assert confirmTransactionSafe is invoked once with correct arguments (connection, signature, blockhash, lastValidBlockHeight), and to verify that confirmation rejection throws errors appropriately.
Hook Implementation
src/hooks/useInsuranceLP.ts
Imported and integrated confirmTransactionSafe into both deposit and withdraw flows. Added explicit validation that results.signatures[0] exists; throws if missing. Captures lastValidBlockHeight from getLatestBlockhash() and passes it along with blockhash to confirmTransactionSafe after transaction send, adding a confirmation step before returning success.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰✨ A signature sealed, a blockhash held tight,
We confirm every hop lands just right!
No phantom transactions shall pass our wise gate—
With confirmation safe, all deposits are fate.
Hopping forward with trust, precision, and might! 🚀

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and concisely summarizes the main changes: adding transaction confirmation and signature validation to the useInsuranceLP hook.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
__tests__/hooks/useInsuranceLP.test.ts (1)

273-329: Please add tests for the new “no signature returned” branches.

The hook now throws explicit errors when results.signatures[0] is missing, but these branches are not covered yet. Add one deposit and one withdraw test with signAndSend returning an empty signatures array.

Also applies to: 420-475

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@__tests__/hooks/useInsuranceLP.test.ts` around lines 273 - 329, Add tests
that exercise the branches where signAndSend returns no signatures by mocking
mockSignAndSend to resolve with { signatures: [] } and verifying the hook
surfaces the explicit error: for deposit, call depositInsuranceLP (after
setupConnectedWallet and existing account mocks like in the surrounding tests),
have mockSignAndSend.mockResolvedValueOnce({ signatures: [] }), await the call
and assert it returns null, result.current.error matches the "no signature"
error (e.g. /No signature/ or the exact message your hook throws), and
result.current.submitting is false; repeat the same pattern for withdraw by
calling withdrawInsuranceLP with mockSignAndSend returning { signatures: [] }
and asserting the same outcomes; use the same test scaffolding and mocks
(mockGetAccountInfo, mockConfirmTransactionSafe) and reference mockSignAndSend,
depositInsuranceLP, withdrawInsuranceLP, and results.signatures to locate the
code paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/hooks/useInsuranceLP.ts`:
- Around line 355-356: The hook useInsuranceLP currently calls
confirmTransactionSafe (at the locations returning { signature }) which swallows
non-definitive confirmation failures; change those calls to a strict
confirmation flow: either replace confirmTransactionSafe with a strict variant
(e.g., confirmTransactionStrict) or update confirmTransactionSafe usage to throw
when the confirmation result is not explicitly finalized/confirmed
(network/polling/timeout paths). Ensure the code paths in the functions that
call confirmTransactionSafe (the hook's send/confirm branches that currently
"return { signature }") only return on an explicit success state and rethrow or
propagate an error for unknown/failed confirmation states so the caller does not
treat uncertain confirmations as success.

---

Nitpick comments:
In `@__tests__/hooks/useInsuranceLP.test.ts`:
- Around line 273-329: Add tests that exercise the branches where signAndSend
returns no signatures by mocking mockSignAndSend to resolve with { signatures:
[] } and verifying the hook surfaces the explicit error: for deposit, call
depositInsuranceLP (after setupConnectedWallet and existing account mocks like
in the surrounding tests), have mockSignAndSend.mockResolvedValueOnce({
signatures: [] }), await the call and assert it returns null,
result.current.error matches the "no signature" error (e.g. /No signature/ or
the exact message your hook throws), and result.current.submitting is false;
repeat the same pattern for withdraw by calling withdrawInsuranceLP with
mockSignAndSend returning { signatures: [] } and asserting the same outcomes;
use the same test scaffolding and mocks (mockGetAccountInfo,
mockConfirmTransactionSafe) and reference mockSignAndSend, depositInsuranceLP,
withdrawInsuranceLP, and results.signatures to locate the code paths.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d50bbc94-a9d4-42ac-82fb-35e3a95e313d

📥 Commits

Reviewing files that changed from the base of the PR and between df5b0fd and d5d975f.

📒 Files selected for processing (2)
  • __tests__/hooks/useInsuranceLP.test.ts
  • src/hooks/useInsuranceLP.ts

Comment on lines +355 to +356
await confirmTransactionSafe(connection, signature, blockhash, lastValidBlockHeight);
return { signature };
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Confirmation can still report success when landing is uncertain.

Line 355 and Line 449 rely on confirmTransactionSafe, but that helper currently swallows non-definitive confirmation failures (polling/timeout/network paths) and resolves. In those cases this hook still returns { signature }, so the original “false success” UX can still occur under congestion/expiry scenarios.

Please make this path strict (rethrow on unknown/failed confirmation states) or use a strict confirmation helper variant for insurance LP.

Also applies to: 449-450

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useInsuranceLP.ts` around lines 355 - 356, The hook useInsuranceLP
currently calls confirmTransactionSafe (at the locations returning { signature
}) which swallows non-definitive confirmation failures; change those calls to a
strict confirmation flow: either replace confirmTransactionSafe with a strict
variant (e.g., confirmTransactionStrict) or update confirmTransactionSafe usage
to throw when the confirmation result is not explicitly finalized/confirmed
(network/polling/timeout paths). Ensure the code paths in the functions that
call confirmTransactionSafe (the hook's send/confirm branches that currently
"return { signature }") only return on an explicit success state and rethrow or
propagate an error for unknown/failed confirmation states so the caller does not
treat uncertain confirmations as success.

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