Skip to content

[DRAFT] Class XDR Implementation#1422

Draft
Ryang-21 wants to merge 49 commits into
masterfrom
class-xdr
Draft

[DRAFT] Class XDR Implementation#1422
Ryang-21 wants to merge 49 commits into
masterfrom
class-xdr

Conversation

@Ryang-21

@Ryang-21 Ryang-21 commented May 26, 2026

Copy link
Copy Markdown
Contributor

Why

The legacy @stellar/js-xdr runtime had several rough edges that
constrained the SDK from inside:

  • Union access required method-call accessors (value.switch(),
    value.value(), value.arm()) with no TypeScript narrowing — every
    union access needed an as cast.
  • Enum access required a function call (AssetType.assetTypeNative())
    even though the members are cached singletons under the hood. The
    function-call ergonomics broke pattern-matching on enum identity and
    made switch statements over enums verbose.
  • Wide ints had a multi-arg parts constructor (new Int128(lo, hi))
    and exposed slice() for accessing 32- or 64-bit chunks. The bigint
    value was only reachable via .toBigInt().
  • Int64 / Uint64 were object-boxed (Hyper / UnsignedHyper
    extending LargeInt) rather than native bigint.
  • No SEP-0051 JSON outputstellar-xdr-json was a separate process.
  • Method names used all-caps acronyms (toXDR, fromXDR) where the
    SDK's broader convention is single-initial-cap (toString, etc.).

Additionally, this PR introduces several capabilities the legacy runtime
didn't have at all (not fixes, additions):

  • toJson / fromJson on every XDR value, SEP-0051-compliant.
  • toXdrObject / fromXdrObject bridging methods (legacy types held
    wire shape directly, so this distinction wasn't meaningful).
  • The XdrString wrapper for byte-faithful string<N> handling.

What

A full in-tree replacement of the XDR layer (this will be moved to js-xdr but for testing purposes it lives here for now):

  • src/xdr/core/Reader, Writer, BaseType<T>. Buffer I/O
    primitives.
  • src/xdr/types/ — schema primitives (struct, union, enum,
    opaque, varOpaque, string, int32/int64/etc., array,
    fixedArray, option, lazy). Charset-agnostic; no dependency on
    consumer value classes. Designed to be liftable into a standalone XDR
    runtime.
  • src/xdr/values/ — consumer-facing bases. XdrValue (universal
    base with toXdr/fromXdr/toJson/fromJson), EnumValue,
    BytesValue, BigIntValue, XdrString. The SEP-0051 JSON walker
    lives here (to-json.ts).
  • src/xdr/generated/ — 447 generated classes produced by
    tools/xdrgen/generate.mjs reading xdr/xdr.json. TSDoc on each class
    carries the original .x source so hovers show the upstream XDR
    declaration.
  • src/xdr/dx/ — hand-written DX overlays (Int128/Uint128/
    Int256/Uint256 exposing a single bigint) and primitive shims
    (Int64/Uint64/Int32/Uint32 returning native primitives via a
    Proxy construct trap).

Plus:

  • Method-name normalization: toXDRtoXdr, fromXDRfromXdr
    on every type. toXDRObject / fromXDRObject / fromJSON listed
    elsewhere are new methods with no legacy analogue.
  • Drop of the LargeInt classes in src/base/numbers/Int128,
    Uint128, Int256, Uint256 now hold a single bigint.
  • XdrString wrapper for string<N> fields — bytes-faithful, with
    explicit .toString() / .toStringStrict() / .asStringOrBytes() /
    .bytes / .toJson() access patterns.
  • SEP-0051 JSON walker — value.toJson() and Type.fromJson(json) on
    every XDR class.
  • Layered test suite validating wire-compatibility with the legacy SDK:
    hand-written smoke tests (~15), real-traffic mainnet corpus (~15),
    schema-driven exhaustive coverage (~2000), JSON walker round-trips
    (~120).
  • docs/XDR_MIGRATION.md — caller-facing migration guide.
  • docs/ARCHITECTURE.md — contributor-facing internals.

Example changes

Union access.type literal narrowing replaces method-call accessors:

// Before 
if (op.body().switch() === xdr.OperationType.invokeHostFunction()) {
  const args = op.body().value().invokeHostFunctionOp();
  args.hostFunction().value();  // any
}

// After
if (op.body.type === "invokeHostFunction") {
  const args = op.body.invokeHostFunctionOp;  // typed
  args.hostFunction.invokeContract;           // typed
}

Enum access — drop the parens; the singleton was already cached:

// Before — function call returning a cached singleton
xdr.AssetType.assetTypeNative();
xdr.ScValType.scvU32();

// After — property access, same identity guarantee
xdr.AssetType.assetTypeNative;
xdr.ScValType.scvU32;

Wide ints — single bigint instead of parts arithmetic:

// Before
const v = new xdr.Int128(lo, hi);       // multi-arg parts constructor
v.toBigInt();                            // need a method to get the bigint
v.slice(64);                             // get 64-bit chunks

// After
const v = new Int128(42n);              // bigint-direct
v.value;                                 // bigint property
v.toParts();                             // { hi, lo } for wire-shape access

XDR strings — XdrString wrapper with explicit decoding:

// Before — string<N> read returned a raw Buffer; opt-in .toString("utf8")
const memo = parsedMemo;
memo.value();                            // Buffer
memo.value().toString("utf8");           // string (lossy if non-UTF-8)

// After — explicit access patterns
const memo = Memo.memoText("Hi");        // accepts string | Uint8Array
memo.text.toString();                    // "Hi" — lenient UTF-8
memo.text.toStringStrict();              // "Hi" — throws on invalid
memo.text.bytes;                         // Uint8Array — raw wire bytes
memo.text.toJson();                      // "Hi" — SEP-0051 escape form

SEP-0051 JSON output (new — no legacy equivalent):

ScVal.scvI128(new Int128Parts({ hi: 0n, lo: 7n })).toJson();
// → { i128: "7" }

ScAddress.scAddressTypeAccount(PublicKey.publicKeyTypeEd25519(bytes)).toJson();
// → "GAAQEAYEAUDA…"  (StrKey per SEP-0051)

Asset.assetTypeNative().toJson();
// → "native"  (snake_case case-name, prefix stripped)

new AlphaNum4({ assetCode: bytes, issuer: pk }).toJson();
// → { asset_code: "USD", issuer: "GAAQ…" }   (snake_case keys)

Method renames:

// Before                         // After
obj.toXDR("base64")               obj.toXdr("base64")
Foo.fromXDR(bytes)                Foo.fromXdr(bytes)
asset.toChangeTrustXDRObject()    asset.toChangeTrustXdrObject()
asset.toTrustLineXDRObject()      asset.toTrustLineXdrObject()

Bytes — explicit wrappers required:

// Before
xdr.ContractExecutable.contractExecutableWasm(Buffer.alloc(32));
xdr.ScVal.scvBytes(Buffer.from([1, 2, 3]));

// After
xdr.ContractExecutable.contractExecutableWasm(new xdr.Hash(Buffer.alloc(32)));
xdr.ScVal.scvBytes(new xdr.ScBytes(Buffer.from([1, 2, 3])));

Typedef-opaque aliases became distinct classes:

// Before — PoolId === Hash at runtime
xdr.ScAddress.scAddressTypeContract(new xdr.Hash(bytes));        // worked
xdr.ScAddress.scAddressTypeLiquidityPool(new xdr.Hash(bytes));   // worked

// After — distinct types with strkey-aware JSON output
xdr.ScAddress.scAddressTypeContract(new xdr.ContractId(bytes));
xdr.ScAddress.scAddressTypeLiquidityPool(new xdr.PoolId(bytes));

Ryang-21 and others added 30 commits April 22, 2026 13:41
* Replace __USE_AXIOS__ dynamic require with static fetch default

* Add babel and webpack aliasing to emit axios variant from shared source

* Flip package.json exports: fetch default, /axios opt-in, drop /no-axios

* Update vitest configs and test harness for TRANSPORT=axios variant

* Update CI, README, and stale references for new fetch-default build matrix
* update eventsource to v4.1.0 and remove conditional import of eventsource

* remove no-eventsource lib bundle as the eventsource dependency runs in the workerd runtime

* remove the dom-monkeypatch as its now included in the tsconfig lib field

* add additional tests for eventsource behavior
* add type: module in pkg.json and update js config files to esm style

* build and export cjs and esm build varients, remove export of umd bundles

* remove unused dependency (causing issue with hook files being cjs format)
* replace webpack + babel dependencies for rollup

* migrate build tools to rollup
* move stellar-base src under src/base

* add unit tests from stellar-base

* port xdr and makefile for generating xdr

* port manual type declarations for js only dependencies

* inline js-xdr for node esm compatibility

* disable bundle_size workflow
* migrate from classic yarn to pnpm

* add pnpm setup action to git workflows
* remove randomBytes for universal crypto.getRandomValues

* replace sha.js with noble/hashes and update to version 2.2.0

* update BigNumber to v11.0.0

* replace noble/curves with noble/ed25519 for reduced bundle size

* move tsconfig project root

* replace toml with smol-toml
* refactor: replace URI usage for native URL + URLSearchParam objects

* remove usage of non-null assertion

* allow expandUriTemplate to handle relative templated links
* test src code directly instead of the built lib
…p nyc (#1408)

* Update husky config + remove nyc

* Root .nvmrc + bump Node to v22

* pnpm minimumReleaseAge config

* Cleanup
@github-project-automation github-project-automation Bot moved this to Backlog (Not Ready) in DevX May 26, 2026
@Ryang-21 Ryang-21 mentioned this pull request May 27, 2026
@Ryang-21 Ryang-21 changed the title Class xdr [DRAFT] Class XDR Implementation May 27, 2026
@Ryang-21 Ryang-21 requested a review from Copilot May 27, 2026 16:38

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 wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@Ryang-21 Ryang-21 requested review from Shaptic and quietbits May 27, 2026 16:41
Ryang-21 added 2 commits May 28, 2026 08:15
  Expose enum schema nameByValue metadata through the public EnumSchema type and
  reuse it for generated enum fromValue/fromName helpers. This removes the
  per-enum byValue maps and generated fromName switch statements, leaving the
  schema as the single source of truth for enum member metadata.

@Shaptic Shaptic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Left some comments on the migration guide but none of them are necessarily blocking for v0.

However, this should come alongside a SKILL.md or something that will do the porting for someone automatically. Doing this section-by-section yourself (or even w/ an LLM) would be annoying and we should try to streamline the adoption experience as much as possible.

Comment thread docs/XDR_MIGRATION.md
Comment on lines +61 to +65
// Before
if (op.body().switch() === xdr.OperationType.payment()) { … }

// After
if (op.body.type === "payment") { … }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there still support for number-based comparisons? This covers the .name case on the type, but not the .value case. Those are faster and can be nicer to use in tight loops for a little extra performance. Not a show-stopper, but it's at least worth noting in a feature request.

Comment thread docs/XDR_MIGRATION.md
Comment thread docs/XDR_MIGRATION.md Outdated
Comment on lines +290 to +292
or `toXdr("hex")` returns a string. **No more `.toXDR().toString("base64")`
pattern** — that was a Buffer idiom and will now produce comma-separated
bytes instead of base64.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not a fan of this behavior changing silently; it's gonna break a lot of things more subtly since it's a runtime change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This should fail at compile time for users who are doing this. Uint8Array does not take an argument in its toString() function

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeaahhhh but it's optional in the Buffer case :/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I agree but the fix is generally pretty easy as you move the "base64" parameter inside the toXdr call. It's a tad more annoying to get back utf-8 but its the cost of moving away from Buffer

Comment thread docs/XDR_MIGRATION.md Outdated
| Method | Description |
| -------------------------------------- | ----------- |
| `value.toXdrObject()` / `Class.fromXdrObject(wire)` | Bridges instance ↔ wire-shape object. Legacy types directly held their wire shape, so this distinction wasn't meaningful. |
| `value.toJson()` / `Class.fromJson(json)` | SEP-0051-compliant JSON serialization. See § 13. |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: add the cross-reference anywhere SEP-51 is mentioned:

Suggested change
| `value.toJson()` / `Class.fromJson(json)` | SEP-0051-compliant JSON serialization. See § 13. |
| `value.toJson()` / `Class.fromJson(json)` | [SEP-51](https://stellar.org/protocol/sep-51)-compliant JSON serialization. See § 13. |

Comment thread docs/XDR_MIGRATION.md
Comment on lines +570 to +571
If you previously did `someMemo.value === "expected-string"`, switch to
`someMemo.value?.toString("utf8") === "expected-string"`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is lowkey obnoxious but I guess strictly more correct?

@quietbits quietbits left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Love the new approach, it reads much better. Just a few minor comments. Thanks for the update! 🙌

Comment thread docs/ARCHITECTURE.md Outdated
Comment thread docs/ARCHITECTURE.md
- **`schema.kind` matches one of the kinds enumerated in
`values/to-json.ts`.** Adding a new primitive without updating the
walker will produce runtime errors on any consumer call to `toJson()`.
- **Schemas reachable from each other must form a DAG** *or* go through

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is DAG?

@Ryang-21 Ryang-21 May 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It stands for directed acyclic graph

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

basically no cycles

Comment thread docs/XDR_MIGRATION.md Outdated
## 2. Union types: discriminated classes, not switch/value pairs

The biggest behavioral change. Every XDR `union` (and any type defined like
one — `Asset`, `SCVal`, `OperationBody`, `LedgerEntryData`, `TransactionEnvelope`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should SCVal be ScVal (lowercase "c")?

Comment thread docs/XDR_MIGRATION.md Outdated
Comment on lines +367 to +368
SCValAddress,
SCAddressMuxedAccount,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should these have lowercase "c"?

Suggested change
SCValAddress,
SCAddressMuxedAccount,
ScValAddress,
ScAddressMuxedAccount,

Comment thread docs/XDR_MIGRATION.md Outdated
Comment on lines +479 to +480
import type { SCValAddress } from "@stellar/stellar-sdk";
const addr = (scv as SCValAddress).address;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar here?

Suggested change
import type { SCValAddress } from "@stellar/stellar-sdk";
const addr = (scv as SCValAddress).address;
import type { ScValAddress } from "@stellar/stellar-sdk";
const addr = (scv as ScValAddress).address;

Comment thread src/contract/spec.ts
}
switch (value) {
case xdr.ScSpecType.scSpecTypeVoid().value:
case xdr.ScSpecType.scSpecTypeOption().value:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't need scSpecTypeOption anymore?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These functions are kinda a mess but tyType get checked at the beginning of the function for scSpecTypeOption and gets handled there

Comment thread src/horizon/server.ts Outdated
Comment on lines +366 to +370
if (isUnionVarient(responseXDR.result, "txSuccess")) {
results = responseXDR.result.results;
} else if (isUnionVarient(responseXDR.result, "txFailed")) {
results = responseXDR.result.results;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

results in both cases is the same. Is there some kind of magic going on with type setting?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No magic the XDR spec defines the field result: OperationResult[] for both success and failure

 * ```xdr
 * union switch (TransactionResultCode code)
 *     {
 *     case txFEE_BUMP_INNER_SUCCESS:
 *     case txFEE_BUMP_INNER_FAILED:
 *         InnerTransactionResultPair innerResultPair;
 *     case txSUCCESS:
 *     case txFAILED:
 *         OperationResult results<>;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I refactored this to be less verbose

Comment thread src/rpc/server.ts Outdated
const entry = await this.getLedgerEntry(trustlineLedgerKey);
return entry.val.trustLine();
if (entry.val.type !== "trustline") {
throw new Error("unexpected ledger entry type");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we add entry.val.type to the error message?

Comment thread src/webauth/utils.ts Outdated
Comment on lines 81 to 82

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @param {string} accountId The signer's public key.
* @returns {boolean} Whether or not `accountId` was found to have signed the

@@ -0,0 +1,96 @@
/* eslint-disable @typescript-eslint/no-use-before-define */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it be possible to add a comment that this is generated file? It would be helpful when looking at the file without checking its path.

Comment thread src/xdr/core/writer.ts
readonly #chunks: number[] = [];

writeBytes(bytes: Uint8Array): void {
this.#chunks.push(...bytes);

@sagpatil sagpatil May 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

writeBytes overflows the stack on large byte arrays

writeBytes(bytes: Uint8Array): void {
  this.#chunks.push(...bytes);
}

Writer.writeBytes() currently stores output in a number[] and appends each byte slice with this.#chunks.push(...bytes). This makes encoding riskier for larger XDR values: every write expands the whole Uint8Array into JS call arguments, and the final Uint8Array.from(this.#chunks) copies everything again. Across many fields/chunks, that can drift toward O(n²)-style allocation/copy overhead, and very large opaque/string/vector fields could also hit engine argument limits.

push(...bytes) spreads every byte as a separate function argument, so it's bounded by the engine's argument-count limit (~64k–125k depending on the JS engine). Every primitive writer funnels through writeBytes, so this caps the size of any opaque / varOpaque / string value we can encode.

In practice this breaks large ScBytes and contract Wasm uploads: an uploadContractWasm / InvokeHostFunction payload over ~125KB throws RangeError: Maximum call stack size exceeded at encode time. Wasm blobs routinely exceed that.

Suggested fix — accumulate into a list of chunks (or at minimum loop) instead of spreading:

readonly #chunks: Uint8Array[] = [];
#length = 0;

writeBytes(bytes: Uint8Array): void {
  if (bytes.length === 0) return;
  this.#chunks.push(bytes);
  this.#length += bytes.length;
}

toUint8Array(): Uint8Array {
  const out = new Uint8Array(this.#length);
  let offset = 0;

  for (const chunk of this.#chunks) {
    out.set(chunk, offset);
    offset += chunk.length;
  }

  return out;
}

and concatenate in toUint8Array(). That also avoids the per-byte number[] overhead. If you'd rather keep the number[] accumulator, replace the spread with a for loop so it scales past the argument limit.

A regression test encoding a >256KB opaque value would lock this down.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The fix already exists upstream — js-xdr#140 rewrote Writer to a growable Uint8Array (#ensureCapacity + #buffer.set) with a 200KB encode test. Since this copy is moving to js-xdr anyway, porting that verbatim keeps them in sync.

+1 on the >256KB regression test!

marcelosalloum added a commit to stellar/stellar-mpp-sdk that referenced this pull request May 28, 2026
…source

Replace the published @stellar/stellar-sdk@15.0.1 with the in-development
class-XDR rewrite from stellar/js-stellar-sdk#1422 (commit c7eb18e), vendored
as a prebuilt tarball and pinned via a pnpm override so the branch installs
reproducibly (the SDK's own prepare/setup step can't run inside pnpm's git
fetch sandbox).

Migrate all SDK source to the new XDR surface:
- union access via .type string discriminant + narrowing instead of
  .switch()/.value()/.arm()
- enum singletons as property access (drop the call parens)
- toXDR/fromXDR -> toXdr/fromXdr
- ScVal arms via .value (wide ints expose .hi/.lo bigint; ScBytes.value is
  a Uint8Array)

Add "node" to tsconfig types: the class-XDR build uses Uint8Array instead of
Buffer and no longer transitively pulls @types/node, so process/Buffer
globals must be declared explicitly.
Comment thread src/xdr/core/helpers.ts
export function isPlainObject(
value: unknown,
): value is Record<string, unknown> {
return typeof value === "object" && value !== null && !Array.isArray(value);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the same issue Copilot flagged on js-xdr#140. isPlainObject treats any non-array object as valid, including things like Date, Map, and Uint8Array. When one of those is passed into a struct or union, it slips past this check and only fails later with a confusing "missing field" error instead of a clear one here. Checking the prototype directly would catch it right away.

@socket-security

socket-security Bot commented May 29, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedesbuild@​0.27.7911007390100
Added@​stellar/​stellar-xdr-json@​26.0.0761008094100
Addedplaywright@​1.60.01001001009980
Addedtsx@​4.22.31001008293100

View full report

Base automatically changed from modernization to master June 5, 2026 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog (Not Ready)

Development

Successfully merging this pull request may close these issues.

6 participants