Skip to content

fix(indexer): correct native SAC network defaults - #225

Merged
Miracle656 merged 1 commit into
Miracle656:mainfrom
Wang1rrr:fix/native-sac-network-defaults
Oct 20, 2026
Merged

Miracle656 merged 1 commit into
Miracle656:mainfrom
Wang1rrr:fix/native-sac-network-defaults

Conversation

@Wang1rrr

@Wang1rrr Wang1rrr commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Deployments without an explicit SAC watch list currently select the testnet native SAC on mainnet and the standalone native SAC on testnet. Correct both defaults and the known-SAC detection set using the addresses derived by the installed Stellar SDK.

Update the native XLM and mainnet USDC examples in .env.example and the dual-network guide. Regression tests compare the defaults and detection behavior against SDK derivation, and ensure the standalone address does not bypass detection on mainnet or testnet.

Related issue

Closes #222

Type of change

  • Bug fix
  • New feature
  • Refactor
  • Docs
  • Tests
  • CI / tooling

Validation

  • The new regression tests produced four failures before the fix.
  • npm run db:generate
  • npx tsc --noEmit
  • npm run typecheck
  • npm run build
  • npm test: 53 suites, 648 tests passed, including coverage thresholds.

Validation ran on Windows with Node.js 24.14.0. npm run docs:openapi normalized the local OpenAPI snapshot to LF before the full suite; its Git content is unchanged and it is not part of this PR.

AI assistance: implemented with Codex and validated with the commands above.

Checklist

  • I have read CONTRIBUTING.md
  • npx tsc --noEmit passes
  • npm run build passes
  • I added / updated tests where relevant
  • I updated docs where relevant

Copilot AI balanced review requested due to automatic review settings October 9, 2026 15:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Miracle656

Copy link
Copy Markdown
Owner

This is already fixed on main — #251 landed today and corrects exactly this.

What it found, verified against the SDK rather than inferred:

mainnet native SAC: CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA
testnet native SAC: CDLZFC3SYJYDZT7K67VZ75HPJVIEUVNIXF47ZG2FB2RMQQVU2HHGCYSC

main had DEFAULT_XLM_SAC_MAINNET set to testnet's SAC, and DEFAULT_XLM_SAC_TESTNET set to a contract that is neither network's. So the mainnet indexer was watching testnet's native XLM contract, found no events, and reported itself healthy — mainnet native XLM transfers were never indexed.

#251 also added the assertion that stops it recurring:

expect(DEFAULT_XLM_SAC_MAINNET).toBe(Asset.native().contractId(Networks.PUBLIC));
expect(DEFAULT_XLM_SAC_TESTNET).toBe(Asset.native().contractId(Networks.TESTNET));

Deriving the expectation from the SDK rather than pasting the literal twice — a test that asserts the constant equals itself would have passed against the wrong value for as long as the bug existed.

This PR now conflicts with that. Please check whether anything here is still needed once you rebase on current main: if your version has coverage #251 lacks, rebase and I will review it; if it is fully superseded, close it and nothing is lost. You found a real bug — it is just that two of you found it.

Your call either way; I am not closing a contributor's PR.

@Miracle656
Miracle656 merged commit 9f70988 into Miracle656:main Oct 20, 2026
1 check passed
@Miracle656

Copy link
Copy Markdown
Owner

Merged. And I verified the addresses rather than taking the diff's word, because contract IDs are exactly the thing a review must not rubber-stamp — I derived them here:

native SAC mainnet (PUBLIC) : CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA
native SAC testnet          : CDLZFC3SYJYDZT7K67VZ75HPJVIEUVNIXF47ZG2FB2RMQQVU2HHGCYSC
USDC SAC mainnet            : CCW67TSZV3SSS2HXMBQ5JFGCKJNXKZM7UQUWUZPUTHXSTZLEO7SJMI75

All three match what you claimed.

The constants in src/indexer.ts had already been corrected on main since you opened this, but two things you fixed had not:

docs/DUAL_NETWORK.md still published the wrong pair — CDMLFMKM… as testnet and CDLZFC3SY… as mainnet. As the derivation above shows, CDLZFC3SY… is the testnet SAC, and CDMLFMKM… is neither network's. An operator following that table would have pointed a mainnet indexer at a testnet contract and watched it index nothing, with no error to explain why. Silent wrong-network configuration is about the worst failure mode a dual-network service has, because everything looks healthy.

The drift guard is the better half of this PR, and it's the part I'd have asked for if you hadn't written it:

it.each([
  ['mainnet', Networks.PUBLIC, DEFAULT_XLM_SAC_MAINNET],
  ['testnet', Networks.TESTNET, DEFAULT_XLM_SAC_TESTNET],
])('uses the SDK-derived native SAC for %s', (network, passphrase, defaultId) => {
  expect(defaultId).toBe(Asset.native().contractId(passphrase));
})

Deriving from the SDK instead of restating the constant is what makes this stick. A test that reads the same constant the code reads will confirm any value, right or wrong — which is presumably how the original pair survived as long as it did. This one fails on a wrong constant.

Merge note: src/__tests__/sacDetect.test.ts conflicted. I kept both tests — main's, which checks the exported constants, and yours, which derives from the SDK. They fail for different reasons and neither subsumes the other, so there was no reason to pick. Your detectSac(contractId, fetcher, network) third argument already exists on main, so both compile as-is.

Verified on the merge result rather than your branch: tsc --noEmit silent, 988 tests across 64 suites, nothing red.

Closes #184.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The built-in native XLM SAC defaults are wrong: mainnet holds the testnet address, testnet holds the standalone one

3 participants