Replace result XDR with fee_charged, result_code and is_fee_bump
#472
Conversation
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
- Extend Trustline struct with Code and Issuer fields - Add GetTrustlines() with JOIN to trustline_assets table - Remove GetTrustlineAssetIDs() and GetTrustlinesWithBalances() - Remove TrustlineBalanceInfo struct (no longer needed) - Update AccountTokensModelInterface 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
No longer needed since GetTrustlines uses JOIN to get code/issuer directly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Update GetAccountTrustlines to return []Trustline with full data - Remove GetAccountTrustlinesWithBalances (consolidated into GetAccountTrustlines) - Remove TrustlineWithBalanceInfo struct - Simplify NewTokenCacheReader (remove trustlineAssetModel dependency) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Create internal/indexer/processors/trustlines.go with TrustLinesProcessor - Add TrustLinesProcessorInterface to indexer.go - Add trustlinesProcessor field to Indexer struct - Instantiate TrustLinesProcessor in NewIndexer Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ersion - Add trustlinesProcessor.ProcessOperation call in processTransaction - Remove StateChangeCategoryTrustline case from state changes loop - Trustlines are now processed directly from ledger changes Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove WithTrustlineXDRFields calls from all cases - Remove getPostLedgerEntryState/getPrevLedgerEntryState calls for XDR extraction - Keep basic add/remove/update tracking for state change history - Add comment explaining TrustLinesProcessor handles balance tracking Co-Authored-By: Claude Opus 4.5 <[email protected]>
No longer needed - TrustLinesProcessor returns TrustlineChange directly. Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove TrustlineBalance, TrustlineLimitValue, TrustlineBuyingLiabilities, TrustlineSellingLiabilities, TrustlineFlags - These fields are now only in TrustlineChange struct - TrustLinesProcessor handles balance tracking directly Co-Authored-By: Claude Opus 4.5 <[email protected]>
Stores native XLM balance data (balance, buying/selling liabilities, last_modified_ledger) to eliminate RPC calls for native balance queries. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Adds AccountChange struct and AccountOpType constants (CREATE/UPDATE/REMOVE) to track native XLM balance changes during ingestion. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Processes xdr.LedgerEntryTypeAccount ledger changes to extract native XLM balance modifications. Skips signer-only changes using AccountChangedExceptSigners(). Co-Authored-By: Claude Opus 4.5 <[email protected]>
Adds accountChanges slice to IndexerBuffer with Push/Get methods. Updates Merge() and Clear() to handle account changes. Extends IndexerBufferInterface with account change methods. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Replaces TrustLinesProcessorInterface with generic LedgerChangeProcessor[T] that works for both trustline and account processors. Processes both in same loop for efficiency. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Adds NativeBalance struct and implements: - GetNativeBalance: query native balance for an account - BatchUpsertNativeBalances: upsert/delete for live ingestion - BulkInsertNativeBalances: COPY protocol for checkpoint population Co-Authored-By: Claude Opus 4.5 <[email protected]>
Updates ingest_live.go to get account changes directly from buffer (not filtered data) since native balances should be tracked for all accounts regardless of participant filtering. Updates ingest_backfill.go to include AccountChanges in BatchTokenChanges struct and aggregate them during catchup mode for proper ordering. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Replaces RPC calls for native XLM balance with PostgreSQL queries: - BalancesByAccountAddress now fetches native balance from DB - BalancesByAccountAddresses fetches native balance in parallel Phase 1 - Adds buildNativeBalanceFromDB helper function - Updates accountKeyInfo struct to include nativeBalance field - Removes native XLM ledger key building (SAC contracts only now) This eliminates RPC round-trips for native balance queries. Co-Authored-By: Claude Opus 4.5 <[email protected]>
…tlines and contract tokens (#451)
…able - Update migration to use fee_charged BIGINT and result_code TEXT instead of result_xdr - Update Transaction struct in types.go with new fields - Update ConvertTransaction to extract fee_charged and result_code from XDR - Update BatchInsert and BatchCopy in transactions.go data layer - Update GraphQL schema with feeCharged and resultCode fields
- Update all test files that reference result_xdr with fee_charged and result_code - Update SQL INSERT statements in tests to use new column names - Update Transaction struct assertions in unit tests - Update GraphQLTransaction in wbclient types
- Fix TransactionResultPair field access path for FeeCharged and ResultCode - Update expected fee value in utils_test.go to match actual XDR data - Remove unused result variable in transactions_test.go - Update integration test to check FeeCharged and ResultCode instead of ResultXdr - Regenerate GraphQL code
- Fix tx3 rows in operations_test.go with proper fee_charged and result_code values - Fix tx3 rows in statechanges_test.go with proper fee_charged and result_code values - Update wbclient queries to request resultCode and feeCharged instead of resultXdr
aditya1702
changed the title
fee_charged, result_code and is_fee_bump
There was a problem hiding this comment.
Pull request overview
Removes storage of full transaction result XDR and replaces it with extracted transaction fields (fee_charged, result_code, is_fee_bump) across the DB schema, indexer pipeline, and GraphQL API.
Changes:
- Replaced
resultXdr/result_xdrwithfeeCharged/fee_charged,resultCode/result_code, andisFeeBump/is_fee_bumpin transaction types and GraphQL schema/queries. - Updated indexer transaction conversion and data-layer inserts/copy operations to persist the new fields.
- Updated unit/integration tests and fixtures to match the new schema and API fields.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/wbclient/types/types.go | Updates client GraphQL transaction type to use fee/result/isFeeBump fields. |
| pkg/wbclient/queries.go | Updates default GraphQL selection set to query new transaction fields. |
| internal/services/ingest_test.go | Updates test inserts/fixtures to use new transaction columns. |
| internal/serve/graphql/schema/transaction.graphqls | Replaces GraphQL resultXdr field with feeCharged, resultCode, isFeeBump. |
| internal/serve/graphql/resolvers/test_utils.go | Updates resolver test DB seeding to use new columns. |
| internal/serve/graphql/resolvers/queries_resolvers_test.go | Updates GraphQL resolver tests to assert new fields. |
| internal/serve/graphql/generated/generated.go | Regenerates GraphQL server bindings for new transaction fields. |
| internal/integrationtests/data_validation_test.go | Updates integration validation to check new fields instead of result XDR. |
| internal/indexer/types/types.go | Updates indexer Transaction model to include fee/result/isFeeBump fields. |
| internal/indexer/processors/utils_test.go | Updates conversion tests to expect new Transaction fields. |
| internal/indexer/processors/utils.go | Updates transaction conversion to populate fee/result/isFeeBump instead of result XDR. |
| internal/db/migrations/2025-06-10.2-create_indexer_table_transactions.sql | Updates transactions table definition to new columns. |
| internal/data/transactions_test.go | Updates data-layer transaction tests/fixtures to use new fields. |
| internal/data/transactions.go | Updates BatchInsert/BatchCopy to write new transaction columns. |
| internal/data/statechanges_test.go | Updates state change tests that insert transactions to use new columns. |
| internal/data/operations_test.go | Updates operation tests that insert transactions to use new columns. |
| internal/data/ingest_store_test.go | Updates ingest store tests that insert transactions to use new columns. |
| internal/data/accounts_test.go | Updates account tests that insert transactions to use new columns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+8
to
+13
| fee_charged BIGINT NOT NULL, | ||
| result_code TEXT NOT NULL, | ||
| meta_xdr TEXT, | ||
| ledger_number INTEGER NOT NULL, | ||
| ledger_created_at TIMESTAMPTZ NOT NULL, | ||
| is_fee_bump BOOLEAN NOT NULL DEFAULT false, |
There was a problem hiding this comment.
This PR updates an existing migration file to change the transactions table schema, but sql-migrate will not re-run already-applied migrations. Existing environments will keep the old schema (including result_xdr and missing fee_charged/result_code/is_fee_bump), causing runtime SQL errors when the updated code reads/writes the new columns. Add a new forward migration that ALTERs transactions (add fee_charged, result_code, is_fee_bump, backfill from result_xdr if needed, then drop result_xdr) and avoid modifying the historical migration in-place.
Contributor
Author
There was a problem hiding this comment.
This can be ignored since wallet backend is not in production yet
| if err != nil { | ||
| return nil, fmt.Errorf("marshalling transaction result: %w", err) | ||
| } | ||
| feeCharged, _ := transaction.FeeCharged() |
There was a problem hiding this comment.
transaction.FeeCharged() returns an error but it is currently ignored; if it fails, feeCharged will silently become 0 and still be persisted/returned. Handle the error (return it or provide an explicit fallback with logging) so fee data isn’t silently corrupted.
Suggested change
| feeCharged, _ := transaction.FeeCharged() | |
| feeCharged, err := transaction.FeeCharged() | |
| if err != nil { | |
| return nil, fmt.Errorf("getting transaction fee: %w", err) | |
| } |
Contributor
Author
There was a problem hiding this comment.
The FeeCharged() function always returns true in the SDK: https://github.com/stellar/go-stellar-sdk/blob/main/ingest/ledger_transaction.go#L332
aristidesstaffieri
approved these changes
Jan 30, 2026
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
Remove the
resultXDRcolumn from transactions table and replace with the following fields:Why
We are storing the entire result XDR but only actually need the above mentioned columns. This is inefficient as it wastes space and also needs decoding of base64 encoding XDR to extract this info at query time
Known limitations
N/A
Issue that this PR addresses
Closes #473