Repository navigation
VERIFY (temporary): #1038 + #1042 combined — do not merge - #1043
Closed
arkhindi12 wants to merge 5 commits into
Closed
arkhindi12 wants to merge 5 commits into
arkhindi12 wants to merge 5 commits into
Conversation
added 5 commits
November 4, 2026 20:55
Fetching N streams cost N `simulateTransaction` calls. The existing
`getStreamInfos` resolves each id and simulates it, and `getStreamsBatch` —
despite its name — only chunked that same per-stream loop, so "batching" was
still N+1. `BatchGetStreamsOptions` declared `chunkSize`/`chunkDelayMs` but the
concurrency knob did nothing for call count.
Soroban exposes `getLedgerEntries`, which takes many keys per request, and the
protocol stores everything needed in directly addressable entries:
* the factory maps `DataKey::StreamAddr(u64)` → stream contract address, in
persistent storage (`contracts/factory/src/storage.rs`); and
* each stream keeps its entire `StreamInfo` under `DataKey::Config` in
**instance** storage — `env.storage().instance().set(&DataKey::Config, info)`
in `contracts/stream/src/state.rs` — and instance storage is one ledger entry
per contract.
So `getStreamInfosBatched` issues **two** `getLedgerEntries` calls for any
number of streams: one for the address index, one for the instance state. No
contract call runs and no signer is needed. Verified against testnet: two keys,
one call, 1.4s.
Parsing is shared with the per-stream path — both decode the same on-chain
`StreamInfo` map through `parseStreamInfo`, so the returned shape is identical.
`getStreamsBatch` now drives the batched path, which is what its name and its
docstring already claimed.
## Key encoding
`DataKey` is a `#[contracttype]` enum, and a variant **with a payload** encodes
as `ScVal::Vec` whose first element is the variant *name* as a `Symbol`, not a
`u32` index:
```rust
// soroban-sdk-macros, derive_enum.rs
let tup = (Symbol::try_from_val(env, &case_name_str_lit)?, #(#field_convs,)*);
```
So `DataKey::StreamAddr(42)` is `scvVec([scvSymbol("StreamAddr"), scvU64(42)])`.
Encoding it as an index produces a key the RPC silently never matches, so this
is pinned by a round-trip test.
## `@stellar/stellar-base` typing gaps
Two things had to be worked around, both documented at the call site:
* `LedgerKeyContractData` is declared only inside the private `xdrHidden`
namespace and never re-exported, so the public constructor is untyped and
constructing it directly yields a bare `ChildStruct` without the union
accessors. Keys therefore go through the typed static factory
`xdr.LedgerKey.contractData`.
* The typings declare `key`, `contract`, `durability` and `val` as accessor
*methods* (`key(): ScVal`), but at runtime they are plain *properties* —
`entry.key()` throws `is not a function`. A single `field()` accessor reads
the property and keeps the cast in one place.
Also fixed while here: `LedgerEntryResult.val` is a `LedgerEntryData`, so the
stored address lives under its `contractData` arm, not directly on `val`.
## Tests
21 new/updated tests. The central one asserts the point of the issue directly:
five streams produce **two** `getLedgerEntries` calls and **zero**
`simulateTransaction` calls.
```
getStreamInfosBatched 7 tests (call count, parse parity, missing ids,
dedupe, empty input, abort)
ledger-keys 11 tests (key encoding incl. name-not-index, round-trip
readers, foreign-key rejection, Config
extraction from instance storage)
streams-batch 3 tests (chunking via the batched path, no fallback to
per-stream, failures preserved)
```
Verified: 21/21 pass. Full suite unchanged from `main` — 28 pre-existing
failures in both, passing files 143 → 145 (the two new test files). Typecheck
27 → 25 pre-existing errors (none introduced; `main` is red until conduit-protocol#1038 lands).
`LedgerEntryDataContractData.val` is typed as an accessor method (`val(): ScVal`) but is a plain property at runtime, the same typing gap as `key` / `contract` / `durability` documented in ledger-keys.ts. The earlier cast still called it as a method, which typechecked but would throw `val is not a function` on a real RPC response. Reads it through an explicit property cast, matching the `field()` accessor used by the key helpers. Typecheck: 26 pre-existing errors on main -> 24 with this branch.
`main` has not typechecked since 2026-10-31; CI has been red for 36 consecutive runs. `Lint & typecheck` and `Test` both fail, which skips `Build`, `@streamfi/react`, and the bundle-size check. Most of these are not type-only problems: several reference identifiers that do not exist in scope, so the affected paths could not have executed since they were merged. Two features that were merged were then silently dropped by a later merge and are restored here from their original commits. Typecheck: 23 errors -> 0 Tests: 28 failed / 1615 passed -> 0 failed / 1643 passed Lint: 14 errors -> 13 (none introduced) Runtime bugs fixed ------------------ * custom-fetch: `sendOnce` referenced `controller`, `timer`, `onCallerAbort` and `registration`, none of which existed in the file. The function could not compile, so every custom-fetch RPC request was dead code. Rebuilt the abort plumbing around a single AbortController so the caller's signal and the request timeout both cancel the in-flight fetch, and wired up `registration.timeoutMs`, which was stored but never read. * soroban: `getLatestLedgerInfo` read `ledger.closedAt`, which does not exist. `getLatestLedger` returns `closeTime` (Unix seconds, as a string) — verified against a live testnet node and the JSON-RPC schema. `new Date(undefined)` yielded NaN, so the returned ledger timestamp was always NaN. Read the correct field and fall back to now when it is absent. * client: `streams`, `governor` and `tokens` were declared `readonly` and then reassigned in `tryFailoverRpc()`. That did not compile, which means RPC failover could never run. Dropped `readonly` (the fields are genuinely mutable across failover) and guarded the index lookup against `noUncheckedIndexedAccess` widening it to `undefined`. * with-retry: a server-supplied `Retry-After` was parsed and then destroyed by jitter — `Math.random() * delay` meant a 2s directive could retry after 0ms, immediately re-triggering the 429. Jitter now applies only to client-guessed backoff, since `Retry-After` is a directive rather than an estimate. Features restored (merged, then dropped by a later merge) --------------------------------------------------------- * `simulateBatch` / `BatchPayload` / `BatchSimulationSummary` (conduit-protocol#959): added by conduit-protocol#1034, removed when conduit-protocol#1026 rewrote `batch-tx.ts` from an older base. * `StreamStatus` / `deriveStreamStatus`: added by 1662982, absent from `types/index.ts` on main. Restored verbatim. Both were re-added from their original commits rather than reimplemented. Overload fixes -------------- `onStreamEvent` in both `events.ts` and `streams.ts` declared overloads whose narrower single-parameter callbacks were incompatible with the implementation signature under `strictFunctionTypes`. The implementation now takes the union of the two callback shapes. Test fixes ---------- * streams.test.ts: three call sites used a 55-character contract address, so token validation failed before reaching the duration/rate assertions. Swapped for the valid 56-character address already used elsewhere in the file. * soroban-retry-after.test.ts: the no-header fallback test asserted an exact 500ms boundary while the default `full` jitter picks a random delay in [0, 500]. Stubbed Math.random instead of changing the production default. * relayer-stress.test.ts: the heartbeat tests awaited `connect()` while fake timers were installed, but the mock fires `onopen` via `setTimeout(0)` — which never ran, so `connect()` hung until the 5s test timeout. Now advance timers around the await. File runtime drops from 26s to 1s. * freighter-wallet-adapter.test.ts / batch-tx-simulate-batch.test.ts: mock and fixture shapes adjusted for `exactOptionalPropertyTypes`.
`Lint & typecheck` runs lint before typecheck, so the 13 pre-existing lint
errors on main kept the job red even after the typecheck step passed.
Lint: 13 errors -> 0 (warnings unchanged at 54)
* unused imports dropped: `StrKey` (constants), `SorobanRpc` (errors — it only
appeared in prose and the `SorobanRpcError` class name, never as a value),
`TopUpEvent` / `ClawbackEvent` (events), `BatchWithdrawItem` (streams).
* client: `require("./soroban.js")` inside `tryFailoverRpc` replaced with a
static import of `getConsecutiveTimeouts`. `soroban.ts` does not import
`client.ts`, so there is no cycle to protect against with a lazy require.
* walletconnect: `(...args: any[])` -> `unknown[]` on the emitter shims. The
listener bodies never touch the arguments.
* builder: `(builder as any)._startTime` -> a narrow
`{ _startTime: number }` cast, keeping the accessed shape explicit.
* signer: the lazily imported `@ledgerhq` modules now carry a local
`LedgerStrApp` interface matching `LedgerDevice`, plus an explicit guard on
`TransportWebUSB.create`. The previous `any` was hiding the fact that a
missing `create` would throw a TypeError rather than the intended
`LedgerHardwareError`.
* indexer: `while (true)` -> `for (;;)` in `fetchAll`. Same semantics, but the
loop no longer trips `no-constant-condition`.
* streams: the operation bag stays `Record<string, any>` with a justification
comment and a scoped disable. A concrete type here would have to be a union
of every operation variant, and each switch branch already narrows the keys
it reads — tightening it to `unknown` produced 16 errors.
Verified: tsc --noEmit clean, eslint 0 errors, vitest 1643/1643, build ok.
The mock's inferred as `never[]`, which does not match `GetStreamInfosResult` under `exactOptionalPropertyTypes`. Cast the implementation the same way the neighbouring mocks do.
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.
Temporary draft PR, used only to run the full CI matrix on #1038 + #1042 together. #1042's jobs cannot pass while
mainfails typecheck, so this proves the combined state is green. Will be closed once verification completes.