Repository navigation
feat(soroban): stable simulation-failure categories; cover the fee-bump paths - #1572
Merged
SheyeJDev merged 1 commit intoOct 19, 2026
Conversation
…mp paths Split-Naira#1495 — a rejected submission surfaced submit.errorResult.toString() verbatim, so the user saw a host protocol string like 'HostError: Error(Contract, #1)'. That text is not stable across SDK or host versions, so nothing could branch on it, and it names no remedy, so nothing could be acted on either. Adds classifySimulationFailure, returning { category, message, diagnostic }. The category is the stable half — callers that need to retry, prompt or escalate branch on it. Six categories, with anything unrecognised landing in 'unknown' rather than being guessed into an actionable bucket: an unrecognised failure must not be told to retry when retrying is what failed. The rules are ordered most-specific-first because the order is load-bearing: tx_insufficient_fee matches the generic insufficient/overrun rule, and it needs different advice from a budget overrun. A contract revert still prefers its specific SplitError message over the generic per-category text, so the existing parseContractError output is preserved where it already existed. The diagnostic drops everything from the first comma onward. errorResult is commonly 'Error(Contract, #2, <base64 XDR>)' and XDR carries contract arguments — amounts and addresses. Sanitizing only at the reporting layer would still leave the payload in the Error message the UI renders, so it is stripped at classification. Verified against a payload-bearing sample: no XDR survives into the diagnostic, and truncation holds at 120 chars. Unmapped failures keep returning the original text, matching prior behaviour rather than replacing a specific message with a generic one. Split-Naira#1496 — both entry points accept a FeeBumpTransaction but the suite only ever built a plain Transaction, so 'the fee-bump path shares the inner path' was an untested assumption. Adds coverage for the accept/defer/reject cases, the poll path, and malformed envelopes. The load-bearing test is that the hash tracked is the envelope's, not the inner transaction's: they differ, and tracking the inner hash would let the UI unblock on a transaction the RPC never accepted — the exact failure Split-Naira#1494 exists to prevent. Not run: dependencies not installed and no tests executed, per the task constraint. Verified the classification and sanitisation logic by re-deriving it into a standalone harness run against real error strings; verified isUnconfirmedSubmission keys off error.name === 'TimeoutError', which is what the NOT_FOUND branch sets; verified no existing assertion breaks, since all three call sites assert on the substring 'tx_bad_seq' and the new message still contains it.
|
@ridercoder4 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
|
LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Assigned issues
Per the assignment, this PR implements 2 of the 4 assigned issues. The two not
implemented are referenced with
Closesso tracking stays accurate, but they arenot fixed here and a maintainer should close them manually rather than relying
on the merge.
On the two not implemented
real product decisions attached — what the report is scoped to, how long it
retains history, who can read it, and whether it ever mutates. Producing one
speculatively risked locking in answers a maintainer should make.
captured on-chain event fixtures, a replay harness around
EventListenerService.pollEvents(), and database-level assertions across itsupsertcall chain. That is a coherent PR on its own, not one squeezed inbeside two unrelated changes.
#1495 — simulation error mapping
The problem
A rejected submission surfaced
submit.errorResult.toString()verbatim, so theuser saw a host protocol string:
That text is not stable — it changes between SDK and host versions — so
nothing could branch on it. It is also not actionable, because it names no
remedy. It is the worst of both: unbranchable by callers and useless to users.
The fix
classifySimulationFailurereturns{ category, message, diagnostic }.The category is the stable half — callers that need to retry, prompt, or
escalate branch on it. Six categories:
contract_errorinsufficient_fundssequence_mismatchsignature_mismatchnetworkunknownAnything unrecognised lands in
unknownrather than being guessed into anactionable bucket. That restraint is the point: a failure that is wrongly
classified as retryable gets retried, which is exactly how a stuck submission
turns into a duplicate payment.
Rule order is load-bearing.
tx_insufficient_feealso matches the genericinsufficient|overrunpattern, so specificity is checked first — the two needdifferent advice (one is about the account's sequence, the other its balance).
A contract revert still prefers its specific
SplitErrormessage over the genericper-category text, preserving existing
parseContractErroroutput where italready existed.
Sanitising the diagnostic
errorResultis commonlyError(Contract, #2, <base64 XDR>), and XDR carriescontract arguments — for this contract, amounts and addresses.
Stripping at the reporting layer alone would still leave the payload inside the
Errormessage that the UI renders. So sanitisation happens at classification:everything from the first comma onward is dropped, and the remainder is truncated
to 120 characters.
Verified against a payload-bearing sample — no XDR survives into the diagnostic.
Preserving prior behaviour where it is better
An unmapped failure returns the original text, not the generic
"could not be prepared" message. Replacing a specific, accurate message with a
vaguer one would be a regression, and there is no reason to do it — the generic
text exists for the failures that genuinely have no better text.
#1496 — fee-bump transaction behavior tests
Both
sendSorobanTransactionandsubmitSorobanTransactionAndPollaccept aFeeBumpTransaction, but the existing suite only ever built a plainTransaction. A fee bump is a different envelope — its hash covers the innertransaction and the fee-bump operation, and it can be rejected for reasons the
inner transaction never sees. "The fee-bump path shares the inner path" was an
assumption nobody had tested.
The new suite covers the accept / defer / reject cases, the poll path, and
malformed envelopes.
The load-bearing test is that the hash tracked is the envelope's, not the inner
transaction's. They differ, and tracking the inner hash would let the UI unblock
on a transaction the RPC never accepted — precisely the failure mode Issue #1494
exists to prevent.
Files changed
frontend/src/lib/contract-errors.tsfrontend/src/lib/soroban-transaction.tsfrontend/src/lib/soroban-transaction-fee-bump.test.tsVerification
Dependencies are not installed and no tests were executed, per the task
constraint. Verified by:
harness, run against 14 real error strings. All 14 classified correctly; the
XDR-bearing sample leaked nothing; truncation held at 120 chars.
isUnconfirmedSubmissionkeys offerror.name === "TimeoutError", which iswhat the
NOT_FOUNDbranch sets — confirming the unconfirmed-fee-bump test isasserting the right property.
"tx_bad_seq"(
soroban-transaction.test.ts:55,observability.test.ts:86). All usetoThrow(string), which is a substring match, and the new message stillcontains
tx_bad_seq— so thesubmissionErrorMessagechange breaks none ofthem.
setTimeout(30)makes atime-bounds envelope unique per call, so a second
feeBumpTransaction()wouldhash to a different value and make the hash assertions fail spuriously.
Closes #1473
Closes #1491
Closes #1495
Closes #1496