fix(indexer): fold fee-phase changes into native balances#641
Conversation
…#637) AccountsProcessor now folds the transaction fee debit (GetFeeChanges) and Soroban fee refund (GetPostApplyFeeChanges) into native balances, not just per-operation meta — ordered fee < op < refund via ledger floor/ceiling TOID keys, matching Horizon. Fixes over-reported XLM for accounts whose balance moves only in the fee phase (e.g. fee-bump fee sources), with no change to the buffer dedup. Sliced as-is from PR #622 for independent review.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16dd8e1835
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR fixes native XLM balance indexing for accounts whose balances change only during transaction fee phases (fee debit and Soroban post-apply fee refund), which previously could be missed by the per-operation metadata loop.
Changes:
- Add an
AccountsProcessortransaction-level pass to capture fee debits (GetFeeChanges) and Soroban fee refunds (GetPostApplyFeeChanges). - Extend the indexer pipeline to push these fee-phase account changes into the buffer alongside per-operation changes.
- Add unit and end-to-end tests covering fee-only balance movement and buffer dedup behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/indexer/processors/processors_test_utils.go | Adds a helper to build test transactions containing fee/refund change sets. |
| internal/indexer/processors/accounts.go | Implements ProcessTransactionFees and shared account-change construction for fee phases + operations. |
| internal/indexer/processors/accounts_test.go | Adds unit tests validating fee/refund extraction and signer-only / non-account filtering. |
| internal/indexer/mocks.go | Extends the accounts mock with ProcessTransactionFees. |
| internal/indexer/indexer.go | Introduces an accounts-specific interface and invokes fee-phase extraction during transaction processing. |
| internal/indexer/indexer_test.go | Updates mocks and adds an end-to-end test for fee-only balance movement (#637). |
| internal/indexer/indexer_buffer_test.go | Adds tests around account-change dedup ordering with fee/refund synthetic OperationIDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aditya1702
changed the title
Replace the synthetic floor/ceiling TOIDs that ordered native-balance fee and refund changes with an explicit (phase, tx, op) sort key, so the buffer's max-key dedup is a tie-free total order that is correct independent of push/merge order. This removes the unstated reliance on tx-ordered merge for breaking fee/fee and refund/refund ties, and on the TOID op-field width for keeping fees below operations. Rename AccountChange.OperationID -> SortKey to reflect that it is an in-memory dedup rank (never persisted), not a TOID. Behavior-preserving for all valid ledgers; adds buffer regression tests covering multi-tx fee/fee and refund/refund ties and asserting the winner is independent of push/merge order.
Add a fee-bump fixture whose fee source is touched by no operation, so its native balance moves only in the fee phase. Assert the wallet-backend indexes the fee debit (funded 10000 XLM, charged exactly MinBaseFee x 2 = 200 stroops -> 9999.9999800), exercising the #637 path end-to-end through live ingestion. Previously native-balance assertions used accounts that never pay fees, so the fee-phase fold-in was not covered end-to-end.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a59dbfb45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A CREATE→REMOVE (accounts) or ADD→REMOVE (trustlines, SAC balances) within one ledger nets to nothing, so the buffer deleted the key. The bare delete erased the highest-order-wins guard, letting a later lower-order change (e.g. a fee-phase change, pushed after the operation loop) re-insert a stale phantom. Only a domain invariant kept this unreachable. Add shared generic pushWithTombstone/mergeWithTombstone that record the order at which a key was cancelled: a later change is dropped when its order is <= the tombstone and re-creates the key only when strictly higher. Route account, trustline, and SAC push + merge through them, collapsing six near-identical dedup blocks. Merge propagates tombstones across buffers, required because transactions are processed into separate buffers then merged. Also decouple the buffer test's accountRank helper from the production accountSortKey bit layout: the buffer only compares int64, and the encoding is verified by TestAccountSortKey in the processors package.
ProcessOperation duplicated feeAccountChanges' skip/build/append loop, differing only in the per-call-constant sort key. Rename the helper to buildAccountChanges and have ProcessOperation delegate to it. Clarify the sort-key docs: refunds come from GetPostApplyFeeChanges, the protocol-23+ post-tx-set-apply refund Core applies after every operation in the ledger, so phaseRefund dominating operations is correct rather than an inversion. Correct the claim that only the live buffer dedups balances — the backfill buffer also dedups account changes in memory, but never persists them.
Assert the fee-bump fee source's native balance as DefaultFundingAmount minus FeeBumpFixtureFeeStroops instead of the hardcoded "9999.9999800", so the check tracks the fixture constants.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
…ve balance Add a fixture where SorobanRefundSourceKP sources a Soroban tx that over-declares its resource fee (Core refunds the surplus post-apply) and is the `from` of the native-XLM SAC transfer it invokes — so its native balance moves in the fee, operation, and post-apply refund phases of one ledger. TestLiveIngestion_SorobanRefundSource_NativeBalanceMatchesChain asserts the indexed native balance equals the on-chain balance (read from RPC), proving the refund is folded in and reflects the operation applied before it — the end-to-end counterpart to the buffer unit tests that rank the refund phase above operations.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
1 similar comment
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Closes #637
Accounts whose XLM balance moves only in the fee phase (fee-bump fee sources, or txs whose operations don't touch the source's own entry) had stale, too-high balances — the per-operation meta loop never saw them.
AccountsProcessor.ProcessTransactionFeesnow folds the fee debit (GetFeeChanges) and Soroban fee refund (GetPostApplyFeeChanges) into native balances.Changes are ranked by Stellar's canonical close order fee < operation < refund using an explicit
(phase, tx, op)sort key (accountSortKey) instead of a TOID. This makes the buffer's per-account "keep highest key" dedup a tie-free total order — correct regardless of push/merge order, with no dependence on tx-ordered merge or TOID field widths. The key is in-memory only and never persisted, soAccountChange.OperationIDis renamedSortKey.Tests: sort-key ordering plus multi-tx fee/fee and refund/refund tie cases (unit); a fee-bump fixture whose fee source is touched by no operation, asserting the indexed native balance reflects the fee debit (integration).
Review follow-ups
deleted the key, which erased the highest-key guard and let a later lower-key change re-insert a stale phantom. Now tracked with a tombstone (records the cancellation's order value): a later change is dropped when its order is ≤ the tombstone and re-creates the key only when strictly higher. Applied to account, trustline, and SAC-balance dedup (push + merge) via shared genericpushWithTombstone/mergeWithTombstonehelpers; merge propagates tombstones across buffers.GetPostApplyFeeChanges, the protocol-23+ post-tx-set-apply refund that Core applies after every operation in the ledger, sophaseRefunddominating operations is correct (not an inversion). Stated explicitly in the sort-key docs.buildAccountChangesshared with the fee path); decoupled a buffer test helper from the production sort-key encoding; derived the fee-bump integration balance fromDefaultFundingAmount/FeeBumpFixtureFeeStroopsinstead of a hardcoded string.