fix(runtime): never discard the real signature when confirmTransaction times out - #341
fix(runtime): never discard the real signature when confirmTransaction times out#341Morenikeoa wants to merge 1 commit into
Conversation
…n times out
simulateOrSend wrapped sendTransaction + confirmTransaction +
getTransaction in a single try/catch. If sendTransaction succeeded
(producing a real signature) but confirmTransaction subsequently
threw — which happens on an ordinary RPC timeout / blockhash expiry
(TransactionExpiredBlockheightExceededError), not an edge case — the
catch block discarded the real signature and returned signature: "".
This left the caller with no way to check whether the transaction
actually landed before deciding what to do next. simulateOrSend is
the SDK's only send path. For any non-idempotent operation (deposit,
withdraw, trade), a caller who treats signature:"" as "definitely
didn't happen" and naively retries can double-submit a transaction
that had, in fact, already landed.
Fix:
- Split sendTransaction into its own try/catch — only there is
signature: "" actually correct, since no signature was ever
produced.
- On confirmTransaction/getTransaction failure, check the real
on-chain status via getSignatureStatus(signature, {
searchTransactionHistory: true }) before reporting failure. If the
status shows the transaction landed (with or without an on-chain
error), return that real outcome with the real signature instead of
a generic timeout error.
- If status is genuinely unknown (lookup also fails, or returns null),
still return the real signature rather than discarding it, with an
error message that explicitly tells the caller to check that
signature before retrying, instead of looking identical to "this
definitely never happened."
This is independent of and complementary to dcccrypto#311/dcccrypto#314 (defaulting
send commitment to "finalized") — that change affects which finality
level is awaited, not what happens when the wait itself fails.
Added 3 regression tests in test/tx.test.ts covering: confirm-timeout
where the tx actually succeeded, confirm-timeout where the tx landed
with an on-chain error, and confirm-timeout where status lookup also
can't confirm landing. Confirmed all 3 fail against the prior code
(signature "" instead of the real signature) and pass against this fix.
Full suite: 854 passed | 31 skipped (882 -> 885, +3 new tests).
|
Warning Review limit reached
More reviews will be available in 45 minutes and 8 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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 |
1 participant
Summary
simulateOrSendwrappedsendTransaction+confirmTransaction+getTransactionin a single try/catch. IfsendTransactionsucceeded (producing a real signature) butconfirmTransactionsubsequently threw — which happens on an ordinary RPC timeout / blockhash expiry (TransactionExpiredBlockheightExceededError), not an edge case — the catch block discarded the real signature and returnedsignature: "".This left the caller with no way to check whether the transaction actually landed before deciding what to do next.
simulateOrSendis the SDK's only send path. For any non-idempotent operation (deposit, withdraw, trade), a caller who treatssignature:""as "definitely didn't happen" and naively retries can double-submit a transaction that had, in fact, already landed.This is independent of and complementary to #311/#314 (defaulting send commitment to
"finalized") — that change affects which finality level is awaited, not what happens when the wait itself fails.Fix
sendTransactioninto its own try/catch — only there issignature: ""actually correct, since no signature was ever produced.confirmTransaction/getTransactionfailure, check the real on-chain status viagetSignatureStatus(signature, { searchTransactionHistory: true })before reporting failure. If the status shows the transaction landed (with or without an on-chain error), return that real outcome with the real signature instead of a generic timeout error.Test plan
test/tx.test.tscovering: confirm-timeout where the tx actually succeeded, confirm-timeout where the tx landed with an on-chain error, and confirm-timeout where status lookup also can't confirm landing.signature: ""instead of the real signature) and pass against this fix.