feat(services/graphql): 4/n - simulate classic single-operation transactions in simulateStateChanges - #732
Conversation
The network deducts the transaction fee before the operation applies, so the spendable balance during apply is reduced by the fee when the operation's source is also the fee payer. The payment and createAccount would-fail checks now subtract it, closing the borderline case where an account covers the amount and reserve but not the fee: previously it got a preview for a transaction that would fail on-chain. Operations with their own source account are unaffected since a different account pays the fee.
JiahuiWho
changed the title
There was a problem hiding this comment.
Pull request overview
Adds classic single-operation transaction simulation to simulateStateChanges.
Changes:
- Derives ledger changes for five classic operations.
- Adds corresponding GraphQL state-change variants and converters.
- Adds service and resolver coverage.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
internal/services/transaction_simulation.go |
Integrates classic simulation. |
internal/services/transaction_simulation_test.go |
Updates unsupported-operation coverage. |
internal/services/transaction_simulation_classic.go |
Implements classic derivation. |
internal/services/transaction_simulation_classic_test.go |
Tests supported classic operations. |
internal/serve/graphql/schema/simulation.graphqls |
Defines simulated classic variants. |
internal/serve/graphql/schema/queries.graphqls |
Documents expanded query support. |
internal/serve/graphql/resolvers/simulation.go |
Converts classic state changes. |
internal/serve/graphql/resolvers/simulation_test.go |
Tests new conversions. |
internal/serve/graphql/generated/models_gen.go |
Adds generated GraphQL models. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 12 changed files in this pull request and generated 4 comments.
Suppressed comments (2)
internal/services/transaction_simulation_classic.go:467
- Set-options masks are applied without protocol validation. For example, overlapping
setFlags/clearFlags, unknown bits, or clearing flags on an AUTH_IMMUTABLE account are rejected by Core (BAD_FLAGS,UNKNOWN_FLAG, orCANT_CHANGE), but this code synthesizes a successful account update. Validate these constraints before applying either mask.
if so.ClearFlags != nil {
after.Flags &^= *so.ClearFlags
}
if so.SetFlags != nil {
after.Flags |= *so.SetFlags
internal/services/transaction_simulation_classic.go:123
- XDR enforces the 64-byte upper bound but still permits an empty data name. Core returns
MANAGE_DATA_INVALID_NAMEfor that input (internal/indexer/processors/codes.go:390-391), whereas this validation falls through and later creates an empty-name entry. Reject the empty name as a simulation failure.
// createAccount's starting balance is validated against the minimum
// reserve in its handler; manageData's field bounds are enforced by
// XDR decoding (String64 / opaque<64>).
| feeSource, ok := lookupAccount(before, txSource) | ||
| if !ok { | ||
| return ingest.LedgerTransaction{}, 0, wouldFail("transaction source account %s does not exist", txSource.Address()) | ||
| } | ||
| if accountSpendableBalance(feeSource) < fee { |
There was a problem hiding this comment.
Sequence is skipped deliberately, matching RPC simulateTransaction, which ignores sequence numbers. Wallets simulate before assigning the final sequence, so enforcing it would fail legitimate previews.
| if p.Asset.Type != xdr.AssetTypeAssetTypeNative { | ||
| issuer := p.Asset.GetIssuer() | ||
| if opSource.Address() != issuer { | ||
| keys = append(keys, trustlineLedgerKey(opSource, p.Asset)) | ||
| } | ||
| if dst := p.Destination.ToAccountId(); dst.Address() != issuer { | ||
| keys = append(keys, trustlineLedgerKey(dst, p.Asset)) | ||
| } |
There was a problem hiding this comment.
This was removed from the protocol: CAP-0030 (protocol 13, June 2020) deleted PAYMENT_NO_ISSUER and the issuer-existence checks, so a payment of an asset whose issuer was merged succeeds on-chain. Adding the check would make the preview fail transactions the network accept.
On testnet I created an asset, deleted the issuer, and paid the asset between two holders, and it succeeded.
| if opSource.Address() == asset.GetIssuer() { | ||
| return nil, wouldFail("an issuer cannot trust its own asset %s", assetString(asset)) | ||
| } |
There was a problem hiding this comment.
In early protocol versions self-trust did succeed as a no-op, and old ledgers contain such transactions, which is why the ingestion code has to handle them.
On the current protocol the network refuses it: I verified on testnet that an issuer submitting changeTrust for its own asset is rejected with CHANGE_TRUST_MALFORMED.
What
simulateStateChangesnow accepts single-operation classic transactions for five operation types: payment, createAccount, changeTrust, setOptions, and manageData.Three commits, each reviewable on its own:
transaction_simulation_classic.go): Derive the classic op simulation outcome steps:getLedgerEntries,Simulated*types for the variants classic operations produce (signer, threshold, account flags, home domain, data entry, and trustline families). Each one mirrors its history twin minus the fields that only exist after a transaction is included in a ledgerstatechange.resolvers.go.Why
Design doc: Simulated Transaction State Changes
Known limitations
Issue that this PR addresses
#618
Checklist
PR Structure
allif the changes are broad or impact many packages.Thoroughness
Release