fix(ingest): verify SAC before recording contract balances - #695
Open
JiahuiWho wants to merge 2 commits into
Open
fix(ingest): verify SAC before recording contract balances#695JiahuiWho wants to merge 2 commits into
JiahuiWho wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Gates live and checkpoint SAC balance ingestion on verified SAC contract instances.
Changes:
- Adds SAC contract verification for live ingestion.
- Defers and filters checkpoint SAC balances.
- Adds model support, dependency wiring, mocks, and regression tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
internal/services/token_ingestion.go |
Gates live SAC balance writes. |
internal/services/token_ingestion_test.go |
Tests verified and unverified balances. |
internal/services/checkpoint.go |
Defers and filters checkpoint balances. |
internal/services/checkpoint_test.go |
Tests checkpoint verification behavior. |
internal/ingest/ingest.go |
Wires the contract model. |
internal/data/mocks.go |
Extends the contract model mock. |
internal/data/contract_tokens.go |
Adds the SAC lookup query. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
Record a SAC balance only when its contract is confirmed as a SAC via its instance entry(i.e. it exists in
contract_tokenswithtype='SAC').token_ingestion.go): gate the balance upsert on a newContractModel.GetExistingSACByIDlookup.checkpoint.go): stop creating a contract_tokens SAC row from balance shape. Stream balances through the batch as before, then in finalize delete any whose contract is not a confirmed SAC viaSACBalanceModel.DeleteUnverified. This runs before COMMIT, so the deferred fk_contract_token holds and no unconfirmed contract is recorded.Why
SAC balances were recorded from the shape of a contract-data entry alone, without confirming that the writing contract is actually a Stellar Asset Contract. Any contract can write that shape into its own storage, so balances got attached to arbitrary contracts.
Two consequences:
contract_idwith nocontract_tokensparent, so the deferredfk_contract_tokenconstraint fires atCOMMIT(SQLSTATE 23503). That error is classified as permanent (non-retryable) and shares the transaction with the ingestion cursor update, so the ledger rolls back without advancing the cursor and re-fails identically on restart.contract_tokensrow withtype='SAC'from the shape, classifying an arbitrary contract as a SAC.Known limitations
A balance cannot be authenticated from its own entry, so a balance whose SAC instance entry isn't present(i.e. a dormant SAC whose instance was state-archived at bootstrap time) is now dropped rather than trusted. This is bounded and self-healing: such a SAC is inactive, and the next time it's used its instance must be restored, which live ingestion re-ingests and gates the balances back in.
Issue that this PR addresses
https://hackerone.com/reports/3923230
Checklist
PR Structure
allif the changes are broad or impact many packages.Thoroughness
Release