fix(hooks): add tx confirmation + signature validation to useInsuranceLP#154
fix(hooks): add tx confirmation + signature validation to useInsuranceLP#1546figpsolseeker wants to merge 1 commit intodcccrypto:mainfrom
Conversation
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>
📝 WalkthroughWalkthroughAdded transaction confirmation validation to insurance LP deposit and withdraw operations. The hooks now capture Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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 withsignAndSendreturning an emptysignaturesarray.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
📒 Files selected for processing (2)
__tests__/hooks/useInsuranceLP.test.tssrc/hooks/useInsuranceLP.ts
| await confirmTransactionSafe(connection, signature, blockhash, lastValidBlockHeight); | ||
| return { signature }; |
There was a problem hiding this comment.
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.
Summary
depositInsuranceLPandwithdrawInsuranceLPreturned immediately aftersignAndSend()without confirming the transaction landed on-chain. Users saw "success" even when transactions were dropped due to network congestion or expired blockhashes.confirmTransactionSafe()calls aftersignAndSend()in both deposit and withdraw paths — matching the existing pattern inuseTrade.tsanduseCollateral.tslastValidBlockHeightalongsideblockhashfromgetLatestBlockhash()(required for slot-based TTL validation)signatures[0]before passing to confirmation (guards against MWA returning empty signatures array)What changed
src/hooks/useInsuranceLP.tsconfirmTransactionSafe, destructurelastValidBlockHeight, add signature null-check + confirmation call in both deposit & withdraw__tests__/hooks/useInsuranceLP.test.tsconfirmTransactionSafe, add 4 new tests: confirmation args validation + on-chain error propagation for both pathsWhy 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
useInsuranceLPtests pass (17 existing + 4 new)useTrade,useCollateral,useStake,useMWA,usePriceStream)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests