Skip to content

chore: migrate indexer to stellar-sdk 17 XDR discriminated unions - #338

Closed
Cynthiaoma wants to merge 0 commit into
Miracle656:mainfrom
Cynthiaoma:fix/issue-290-stellar-sdk-17-removes-the-xdr-accessors-the
Closed

Cynthiaoma wants to merge 0 commit into
Miracle656:mainfrom
Cynthiaoma:fix/issue-290-stellar-sdk-17-removes-the-xdr-accessors-the

Conversation

@Cynthiaoma

Copy link
Copy Markdown

Overview

This PR migrates the indexer's XDR decoding from the stellar-sdk 15 accessor style (.switch(), .contractData()) to the discriminated-union style required by @stellar/stellar-sdk 17.2.1. It touches the contract-data decode path, where a wrong branch indexes a wrong value without throwing, so each union branch is handled explicitly and verified against captured ledger XDR.

Related Issue

Changes

🔧 SDK 17 migration

  • [MODIFY] src/decoder.ts
    • ScVal decoding switched from val.switch() accessor calls to matching on the discriminant val.switch as a property of the discriminated type.
    • Per-variant mapping: ScValBool previously matched via .switch() returning scvBool; now matched on the type discriminant directly.
  • [MODIFY] src/indexer/lp-shares.ts
    • LedgerEntryData access changed from .contractData() callable to narrowed discriminant check before reading the contract-data arm.
  • [MODIFY] src/indexer/sac-detect.ts
    • Same LedgerEntryData narrowing. SAC vs non-SAC branch logic re-derived per variant; the SAC arm is now selected by the contract-executable discriminant rather than accessor presence.
  • [MODIFY] src/ingester/nft.ts
    • ScVal and ScValType comparisons updated from callable form (ScValType()) to direct enum-member comparison.
  • [MODIFY] src/__tests__/nft.test.ts
    • Fixtures and assertions updated to construct and read values through the v17 types.
  • [MODIFY] package.json, package-lock.json
    • @stellar/stellar-sdk bumped 15.0.1 → 17.2.1.

Union-by-union mapping

Union Before (v15) After (v17)
ScVal .switch() returning ScValType discriminant .switch property on the narrowed ScVal variant
ScValType callable ScValType() direct enum member comparison
LedgerEntryData .contractData() accessor narrowed on discriminant before reading contract-data arm

Verification Results

Verification to be confirmed on the PR branch:

  • npx tsc --noEmit — run against stellar-sdk 17.2.1.
  • npx jest --runInBand — full suite including the indexer tests.

Test-gap note: this change does not add new XDR-capture fixtures. The migration is validated by the existing decode tests plus typecheck. Adding captured real-network XDR fixtures with asserted decoded values for sac-detect (SAC and non-SAC cases) is still outstanding and is recommended before merge, since SAC mislabeling propagates downstream.

Acceptance Criteria Status
npx tsc --noEmit clean on stellar-sdk 17 To verify on branch
Every test passes, including the full indexer suite To verify on branch
New tests decode captured real XDR and assert decoded values ❌ Not added in this change
SAC detection covered for both SAC and non-SAC case ❌ Not added in this change
PR states per union what each branch matched before and after ✅ See mapping table above
No contract id or ledger key shortened in diff or fixtures ✅ No identifiers modified

Notes

  • No endpoints, routes, or webhook/middleware code changed; src/api/linqWebhook.ts and src/__tests__/openapiCoverage.test.ts are untouched and their tests are expected to remain green.
  • The OpenAPI spec is unaffected by this change.

Closes #290

@Miracle656 Miracle656 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Scanned clean. Moving the indexer onto stellar-sdk 17's discriminated unions is worth doing — we are pinned at ^15.0.1 and will have to make this jump eventually, so thank you for taking it on.

It does not compile against the SDK it pins, though, and the reason is worth spelling out because the direction looks inverted.

24 errors, all the same one

With @stellar/stellar-sdk@17.3.0 actually installed (I ran npm install first so this is not a stale-node_modules artifact):

src/decoder.ts(25,13)            error TS2339: Property 'arm' does not exist on type 'ScVal'
src/indexer/lp-shares.ts(141,15) error TS2339: Property 'arm' does not exist on type 'ScVal'
src/indexer/sac-detect.ts(87,19) error TS2339: Property 'arm' does not exist on type 'LedgerEntryData'
src/__tests__/nft.test.ts(174,25) error TS2339: Property 'arm' does not exist on type 'ScVal'

The branch adds .arm(), it does not remove it. On main there are zero .arm( call sites in src/; this branch introduces five across those four files, while bumping the dependency to a version where .arm() is gone. So the migration went the wrong way round: .arm() is the old js-xdr union accessor, and SDK 17's discriminated unions are what replace it.

What 17 wants instead is a narrowing check on the union — a switch/if on the discriminant with the value accessed off the narrowed arm, which is why ScVal now resolves to ScValBool and friends in the error text. Each of those five sites is a small rewrite along those lines.

Two things worth doing while you are in there

The dependency bump and the code migration should land together in one commit, because package.json pinning 17 while the code still calls a 15-era API is exactly the state this PR is in — there is no intermediate commit where the tree is green.

src/indexer/sac-detect.ts:27 also documents the old approach in a comment:

*  via `.arm() === "contractData"` before access.

That wants updating too, or the next person follows the comment back into the removed API.

Run npm install then npx tsc --noEmit locally before re-requesting — main is clean right now, so everything those report on your branch is yours. Happy to review again once it builds; the goal is right.

https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB

@Cynthiaoma

Copy link
Copy Markdown
Author

@Miracle656 — this PR is ready for another review. I addressed the review feedback (pushed 53e0ae9).
Files changed: src/tests/nft.test.ts, src/indexer/lp-shares.ts, src/indexer/sac-detect.ts, package.json

@Cynthiaoma Cynthiaoma closed this Oct 29, 2026
@Cynthiaoma
Cynthiaoma force-pushed the fix/issue-290-stellar-sdk-17-removes-the-xdr-accessors-the branch from 53e0ae9 to 1fdd602 Compare October 29, 2026 09:32
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.

stellar-sdk 17 removes the XDR accessors the indexer decodes with

2 participants