feat(services): 2/n - Phase 1 Soroban state change simulation - #719
Open
JiahuiWho wants to merge 4 commits into
Open
feat(services): 2/n - Phase 1 Soroban state change simulation#719JiahuiWho wants to merge 4 commits into
JiahuiWho wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Implements Soroban/SAC state-change simulation through the existing ingestion pipeline.
Changes:
- Synthesizes ledger transactions from RPC simulation results.
- Adds simulation failure handling.
- Adds Soroban simulation and ledger-change tests.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
internal/services/transaction_simulation.go |
Builds and processes simulated ledger transactions. |
internal/services/transaction_simulation_test.go |
Tests Soroban simulation and ledger-entry conversion. |
internal/serve/graphql/utils.go |
Allows the new client-safe error code. |
internal/serve/graphql/resolvers/queries.resolvers.go |
Maps simulation failures to GraphQL errors. |
Files not reviewed (1)
- internal/serve/graphql/resolvers/queries.resolvers.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Merged
8 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- internal/serve/graphql/resolvers/queries.resolvers.go: Generated file
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
internal/services/transaction_simulation.go:214
- Fee-bump previews calculate the debit from the wrong fee field.
TransactionEnvelope.Fee()returns the inner transaction fee for fee-bump envelopes, whileFeeBump.Tx.Feeis the outer fee payer's full bid. This therefore reportsinner inclusion + minResourceFeefor the outer fee source and can materially understate the history row. Select the outer fee before subtracting the inner declared resource fee.
internal/services/transaction_simulation.go:400 - This path still returns success for custom SEP-41 invocations even though the core indexer's token-transfer processor intentionally drops their events and
sep41.Processoris never run. BecauseisSorobanTransactionaccepts everyInvokeHostFunction, callers receive an incomplete, often fee-only preview rather than an unsupported error, contradicting the method's history-equivalence contract. Wire the SEP-41 processor into simulation or reject unsupported non-SAC token invocations before returning a result.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- internal/serve/graphql/resolvers/queries.resolvers.go: Generated file
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
internal/services/transaction_simulation_test.go:102
- This map retains only one BALANCE row per reason. A SAC transfer also produces a transaction-fee DEBIT, so the later transfer DEBIT overwrites it and this test still passes if fee synthesis or
minResourceFeehandling is broken. Collect all debit rows and assert both the fee and transfer amounts; include a case with an existing resource-fee bid to exercise its subtraction.
internal/serve/graphql/resolvers/simulation_test.go:62 - This claims simulated SEP-41 approvals currently reach the converter, but
stateChangesForTransactionexplicitly does not runsep41.Processor, so such allowance changes are not produced by this simulation path. Describe the two supported representations without implying current SEP-41 simulation coverage.
JiahuiWho
changed the title
Base automatically changed from
feat/618-simulate-state-changes-phase0
to
main-simulate-state-changes
September 11, 2026 18:07
Add TransactionSimulationService, which previews the state changes an unsubmitted transaction would produce without persisting anything. For Soroban transactions it calls RPC simulateTransaction, synthesizes an in-memory ingest.LedgerTransaction from the result (ledger-entry changes and contract events), and runs it through the same indexer processors that build the history API's state changes, so a preview looks exactly like history. Details: - Synthesize TransactionMetaV4 (per-operation changes and events) to match the protocol 23+ network meta, so SAC authz events are not dropped. - Inject the auth entries RPC records for the unsigned invocation onto the InvokeHostFunction operation, so nested-deploy and participant detection see the same auth a real submission would. - Treat a non-empty RestorePreamble as a simulation failure rather than returning a misleading preview of a transaction that needs archived entries restored first. - Classic transactions are not simulatable via RPC and return ErrUnsupportedTransaction for now (derivation is a later phase). - Map the sentinel errors onto GraphQL extensions.code and allowlist them in clientSafeErrorCodes so they reach the client unmasked. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…avior - SIMULATION_FAILED was in the presenter allowlist but absent from both the resolver error-mapping table test and the presenter pass-through table, so the client-visible code could regress or be masked without a failing test. - convertToSimulatedStateChange had no tests, even though its one real bug so far (allowance live_until_ledger arriving as in-memory uint32 vs JSONB float64) lived exactly there. Covers all four variants, both KeyValue shapes, missing-required-field errors, and the unexposed-variant error.
…imulated converter keyValueUint32 collapsed a present-but-wrong-typed value into the same (0, false) as an absent key, so the error always said the value was missing. Return distinct errors for absent, wrong-typed, and out-of-range values, and pin the wrong-typed case in the converter test.
JiahuiWho
force-pushed
the
feat/618-simulate-state-changes-phase1
branch
from
September 11, 2026 18:12
0116c92 to
17439f3
Compare
Comment on lines
+404
to
+408
| // Known limitation: this only runs the core Indexer processors, not the separate | ||
| // protocol processors. SEP-41 custom tokens are handled by internal/services/sep41, | ||
| // which the Indexer's token_transfer processor skips, and that processor is not | ||
| // wired in here yet. So previews for SEP-41 tokens miss their balance changes. | ||
| // Native and SAC token changes are covered. |
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.
What
Implement
SimulateStateChangesfor SAC transaction. It now returns the real state changes the transaction wouldproduce if submitted, by reusing the ingestion pipeline:
simulateTransaction.ingest.LedgerTransactionfrom the result (before/after ledger entries + contract events, asTransactionMetaV4).Why
Part of #618.
Known limitations
Issue that this PR addresses
Part of #618.
Checklist
PR Structure
allif the changes are broad or impact many packages.Thoroughness
Release