feat: add typed transaction result reconciliation (Closes #62) - #153
Open
panditdhamdhere wants to merge 1 commit into
Open
feat: add typed transaction result reconciliation (Closes #62)#153panditdhamdhere wants to merge 1 commit into
panditdhamdhere wants to merge 1 commit into
Conversation
Reconcile submitted Soroban transactions into confirmed, failed, pending, rejected, and unknown states so dashboards stop inferring outcomes from raw RPC status strings. Mapping is conservative: NOT_FOUND stays pending because absence means either "not included yet" or "outside the retention window", and unrecognised statuses resolve to unknown rather than confirmed. Polling is bounded and only ever calls getTransaction, so no reconciliation path can cause duplicate ledger effects.
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.
Description
Submitting a Soroban transaction and knowing what happened to it are two different
problems.
sendTransactiononly reports whether the network accepted the transactionfor inclusion, and
getTransactionreturnsNOT_FOUNDboth for transactions that havenot landed yet and for ones outside the RPC retention window. Consumers currently have
to interpret those raw strings themselves, which is how pending transactions get
mistaken for failures and get resubmitted.
This PR reconciles both signals into one stable status model, exposed as
client.transaction.Why this approach: the mapping is deliberately conservative.
NOT_FOUNDstayspendingbecause absence is not proof of rejection, and any status this SDK versiondoes not recognise resolves to
unknownrather thanconfirmed.rejected(refusedbefore inclusion, no fee or sequence consumed) is kept distinct from
failed(includedin a ledger, then failed), because merging them would hide whether ledger state changed.
Polling reads state only — no reconciliation path calls
sendTransaction, so a longpoll cannot cause duplicate ledger effects.
Files added
src/types/transaction-result.ts— discriminatedTransactionResultunion over the five statessrc/transactions/reconciliation.ts— pure reconcilers for status, send responses, and get responsessrc/transactions/module.ts—TransactionModulewithgetResultand boundedwaitForResultsrc/errors/transaction.ts—TransactionReconciliationErrorsrc/soroban/transaction-result.ts—decodeTransactionResultCode, reads only the result switch namedocs/transaction-reconciliation.mdtests/transaction-reconciliation.test.ts,tests/transaction-module.test.ts,tests/fixtures/transaction-results.tsFiles modified
src/client.ts— wiresclient.transaction(singular, to avoid colliding with the mock client's existingtransactionsreceipt array)src/index.ts— public exports for the module, helpers, types, and errordocs/api-reference.md— fullTransactionModulesection with signatures, params, returns, and every throw pathREADME.md— quickstart sectionCONTRIBUTING.md— doc-update rule and review checklist itemCloses: #62
Evidence Checklist
1. Issue Reference
Closes #62).2. Implementation Summary
3. Tests
45 new tests across two dedicated suites. Coverage includes every send and get
status, unrecognised future statuses,
NOT_FOUND→SUCCESS,NOT_FOUND→FAILED, exhausted observation windows, bounded exponential backoff, RPCtimeouts surfacing as
NetworkFailure, validation rejecting bad hashes andpoll options before any RPC call, and an assertion that
sendTransactionisnever called during reconciliation or polling.
tests/transaction-module.test.tsmocksrpc.Serverand injects asleepfunction, so polling tests are deterministic with no real timers.
4. Commands Run
Command output
5. CI Status
(Check this after CI finishes on the PR — leave unchecked until then.)
6. Acceptance Criteria Coverage
reconcileTransactionStatusplusreconcileSendTransactionResponseandreconcileGetTransactionResponsewrappers, all exported from the package root.
TransactionResultStatusunion with one discriminated interface per state,so narrowing on
statusgives type-safe field access.waitForResultis bounded bymaxAttempts, backs off up tomaxIntervalMs, and routes every readthrough
client.runNetworkOperationso RPC problems arrive as typedNetworkFailure.unrecognised statuses, backoff bounds, and validation paths.
docs/transaction-reconciliation.mdhas adedicated section with five numbered rules, including that
NetworkFailure.retryablerefers to retrying the read, not the submission.calls
sendTransaction(asserted by test).safeToResubmitistrueonlyfor
rejected, the one case where the network never accepted thetransaction — and the docs still require a corrected, re-signed transaction
rather than resending the same envelope.
Reviewer Notes
Branch is rebased onto current
upstream/main(includes role-aware clientfactory #141 and the recent contributor docs PRs). Local
npm run checkpassesafter the rebase.
npm run lintandnpm run formatcannot run on this branch or onmain.Both scripts invoke
eslintandprettier, but neither package is indevDependencies, sonpm run verifyexits ateslint: command not found. Thisis pre-existing and unrelated to this change; CI runs
npm run check, whichpasses.
Known gap, documented rather than fixed:
AssetModule.mintandtransferstill return a bare hash and never inspect
sendTransaction's status, so anERRORorTRY_AGAIN_LATERsubmission is currently indistinguishable from anaccepted one. Changing their return type is API-breaking, so I left an explicit
note in
docs/api-reference.mdpointing users atclient.transaction.waitForResult.Happy to take that on as a follow-up issue if maintainers want it.
Naming: the module is
client.transaction(singular) because the mock clientin
@aegis/sdk/testingalready exposes atransactionsarray of receipts.