Repository navigation
fix: correct native XLM SAC defaults and add derivation-based test - #251
Conversation
|
@Daveeee-tech Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Miracle656
left a comment
There was a problem hiding this comment.
This is a live production bug and the fix is correct. Merging.
I checked the constants against the SDK rather than taking the diff at face value, and main has them wrong in both slots:
$ node -e "const {Asset,Networks}=require('@stellar/stellar-sdk');
console.log('mainnet:', Asset.native().contractId(Networks.PUBLIC));
console.log('testnet:', Asset.native().contractId(Networks.TESTNET))"
mainnet native SAC: CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA
testnet native SAC: CDLZFC3SYJYDZT7K67VZ75HPJVIEUVNIXF47ZG2FB2RMQQVU2HHGCYSC
On main:
export const DEFAULT_XLM_SAC_MAINNET = "CDLZFC3SYJYDZT7K67VZ75HPJVIEUVNIXF47ZG2FB2RMQQVU2HHGCYSC"; // this is TESTNET's
export const DEFAULT_XLM_SAC_TESTNET = "CDMLFMKMMD7MWZP3FKUBZPVHTUEDLSX4BYGYKH4GCESXYHS3IHQ4EIG4"; // this is neitherSo the mainnet indexer has been watching testnet's native XLM contract, and the testnet indexer a contract that is not native XLM on either network. Native XLM transfers on mainnet were not being indexed at all, and nothing failed loudly — the indexer ran, found no events for a contract nobody uses, and reported itself healthy. That is the worst failure shape available: silent, and indistinguishable from "no activity".
Your values are the derived ones, in the right slots.
And the test is what stops it coming back:
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 a second copy of the literal is the whole point. A test that asserts the constant equals itself would have passed happily against the wrong value for as long as this bug has existed. Removing the duplicated literal from sacDetect.test.ts and importing the real export closes the other copy.
On CI: Typecheck & build is red because this branch predates #250, which fixed main's OpenAPI coverage failure. Merged current main locally and re-ran: openapiCoverage, sacDetect, multiNetworkIndexer — 42 passed, tsc --noEmit clean. Integration tests fails identically on main (Postgres read path, since at least 10-16) and is not yours.
Someone should check whether anything downstream cached results from the wrong mainnet SAC.
closes #222
Summary
Fixes the hard-coded native XLM Stellar Asset Contract (SAC) addresses, which were each shifted one network along. Adds a test that checks them against the SDK's own derivation so this can't regress silently.
Problem
DEFAULT_XLM_SAC_MAINNETandDEFAULT_XLM_SAC_TESTNETinsrc/indexer.ts, andKNOWN_SAC_IDSinsrc/indexer/sac-detect.ts, did not matchAsset.native().contractId(...):CDLZFC3S…)CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMACDMLFMKM…)CDLZFC3SYJYDZT7K67VZ75HPJVIEUVNIXF47ZG2FB2RMQQVU2HHGCYSCAll three are valid StrKey contract IDs, so nothing flagged the error. Only the derivation was wrong.
Impact
SAC_CONTRACT_IDSunset,resolveSacContractIds()falls back to a contract that doesn't exist on mainnet.getEventsreturns nothing forever, and the service looks healthy while the table stays empty.SAC_CONTRACT_IDSexplicitly.sac-detect.ts: real mainnet XLM transfers missedKNOWN_SAC_IDSand took the slow RPC path. Testnet XLM hit the set through the entry labelled "mainnet", so it was right for the wrong reason.Changes
src/indexer.ts: correctedDEFAULT_XLM_SAC_MAINNETandDEFAULT_XLM_SAC_TESTNET.src/indexer/sac-detect.ts: corrected the two values inKNOWN_SAC_IDS..env.example:SAC_CONTRACT_IDS_TESTNET/SAC_CONTRACT_IDS_MAINNETexamplesCCW67TSZV3SSS2HXMBQ5JFGCKJNXKZM7UQUWUZPUTHXSTZLEO7SJMI75, which is whatnew Asset('USDC', 'GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN').contractId(Networks.PUBLIC)produces (the issuer key was already correct)Asset.native().contractId(Networks.PUBLIC | Networks.TESTNET)instead of against a literal.Background
The wrong value originated in #166, which quoted
CDLZFC3SY…as the mainnet SAC and carried intodocs/DEPLOY_MAINNET.mdvia #211. The doc was corrected on merge, but the code was not.Acceptance criteria
DEFAULT_XLM_SAC_MAINNET/DEFAULT_XLM_SAC_TESTNETequal the SDK derivation forNetworks.PUBLIC/Networks.TESTNETKNOWN_SAC_IDSholds the same two corrected values.env.examplecorrected (both native fallbacks, known-mainnet-SAC block, USDC example)Asset.native().contractId(...)Notes for reviewers
SAC_CONTRACT_IDSexplicitly are unaffected.