perf(batch-tx): parallelize batch simulation + test/doc coverage for indexer, adapters, client - #420
Merged
Jaydbrown merged 10 commits intoJul 30, 2026
Conversation
|
@bbjiggy 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! 🚀 |
buildBatchTransactions() (used whenever context.rpcUrl is set) had no dedicated tests. It builds one transaction per operation and simulates each via RPC to attach footprint/auth before submission, but nothing exercised that path directly — only the offline buildBatchTransactionsSync path was covered. This adds tests for the happy path, the getAccount-derived sequence path, simulation error propagation, and the offline fallback. The first test asserts that all of a batch's simulateTransaction calls are dispatched before any of them resolve — this fails against the current sequential for-loop implementation, which only calls simulateTransaction for the first operation until it resolves. Related to conduit-protocol#361.
Note in docs/api.md that operations in an RPC-prepared batch are simulated concurrently, and add a CHANGELOG entry under [Unreleased]. Related to conduit-protocol#361.
GraphQLIndexer.subscribe() has two transports (a graphql-transport-ws WebSocket path and an SSE/fetch fallback for environments without a WebSocket constructor), and both were essentially untested — existing coverage only checked subscription bookkeeping, not message handling. Adds coverage for: the connection_init/subscribe handshake, next/data message delivery, error message routing, malformed-JSON resilience, socket error/close handling and auto-unsubscribe, unsubscribe idempotency, and, on the SSE side, data-line parsing, non-ok HTTP responses, and abort-vs-real-error distinction on fetch rejection. Related to conduit-protocol#362.
GraphQLIndexer is exported from src/index.ts but had no entry in docs/api.md. Documents the constructor, query(), subscribe() (including its WebSocket-vs-SSE-fallback behaviour), getSubscriptionCount(), and cleanup(). Adds a CHANGELOG entry under [Unreleased]. Closes conduit-protocol#362
KeypairWalletAdapter is exported (via adapters/index.ts) and used internally whenever config.keypair is supplied without config.wallet, but had no dedicated test file — the raw XDR string signing branch of signTransaction() (including its missing-networkPassphrase error) was entirely untested. Covers getPublicKey(), isConnected(), signing a Transaction instance, signing a raw XDR string with/without networkPassphrase, and that accountToSign is ignored since a keypair only ever signs as itself. Related to conduit-protocol#363.
Adds a "Wallet Adapters" section to docs/api.md — neither adapter had any entry despite both being part of the public API surface. Adds a CHANGELOG entry under [Unreleased]. Closes conduit-protocol#363
ConduitClient.pauseStream()/unpauseStream() delegate to client.streams.pause()/resume(), which are themselves well covered, but the ConduitClient-level wrappers had no direct test — nothing verified the delegation itself (argument passthrough, return value, or error propagation). Related to conduit-protocol#364.
setWallet()'s JSDoc comment — including its @throws tag and the wallet propagation contract describing which modules setWallet() does and doesn't update — sat directly above pauseStream(), not above setWallet() itself. pauseStream()/unpauseStream() each already had their own correct JSDoc immediately below the orphaned block, so setWallet() ended up completely undocumented in editor tooltips and generated docs. Moves the block down to sit directly above setWallet(). No behavior change. Related to conduit-protocol#364.
…llet config field ConduitConfig's table was also missing the wallet field entirely. Documents pauseStream(), unpauseStream(), and setWallet() under ConduitClient in docs/api.md, and adds the CHANGELOG entry for this issue's fix and test additions. Closes conduit-protocol#364
Spreading a destructured optional field straight into a new object literal
(e.g. { rpcUrl, contractId, sourceAccount, network }) produces a type where
that key is present-but-possibly-undefined, which exactOptionalPropertyTypes
rejects against BatchTransactionContext's optional fields — the key must be
entirely absent when the value is undefined, not present with value undefined.
Jaydbrown
force-pushed
the
perf/361-364-batch-parallel-and-coverage
branch
from
July 30, 2026 14:40
c663919 to
fa5bc00
Compare
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
3 participants
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.
Summary
Four fixes bundled from four assigned Stellar Wave issues, each addressing a distinct, verified gap found while investigating the SDK:
buildBatchTransactions()(the RPC-prepared batch path insrc/batch-tx.ts) simulated every operation in a batch sequentially in aforloop, even though each operation's simulation is an independent RPC round trip against its own transaction. This branch originally parallelized it viaPromise.all; while resolving merge conflicts againstmain, I found that perf: cache stream addresses, memoize RPC proxy, parallelize batch si… #403 and Fix nonce-lock deadlock, cache token decimals, room cleanup, and parallelize batch simulation #416 had independently merged the identical fix first, sosrc/batch-tx.tshere now just takesmain's version as-is — no functional change from this PR on that file. What this PR still adds for Enhancement: Detailed SDK Feature #27 #361: test coveragemain's own fix didn't have (src/tests/batch-tx-rpc-simulation.test.ts— thegetAccount-derived-sequence path, simulation-error naming, and the offline fallback, none of which the upstreambatch-tx-concurrent-simulation.test.tscovers). Flagging this transparently rather than claiming credit for the parallelization itself — happy to dropCloses #361if a maintainer would rather close it via perf: cache stream addresses, memoize RPC proxy, parallelize batch si… #403/Fix nonce-lock deadlock, cache token decimals, room cleanup, and parallelize batch simulation #416 directly.GraphQLIndexer(src/indexer.ts) sat at ~40% coverage; its WebSocket (graphql-transport-ws) and SSE/fetch-fallback transports had no message-lifecycle tests at all (only construction/cleanup bookkeeping was covered). Added tests for the connection handshake, message routing, error/close handling, and SSE data-line parsing. Also documentedGraphQLIndexerindocs/api.md— it was exported but undocumented.KeypairWalletAdapter(src/adapters/keypair.ts) had no dedicated test file; its raw-XDR-string signing branch (including the missing-networkPassphraseerror) was entirely untested. Added full coverage and a new "Wallet Adapters" docs section (also coveringWalletConnectAdapter, previously undocumented).src/client.ts:setWallet()'s JSDoc block (its@throwstag and wallet-propagation contract) was orphaned abovepauseStream()/unpauseStream(), leavingsetWallet()completely undocumented. Fixed the placement, added missing test coverage forConduitClient.pauseStream()/unpauseStream()(the delegation itself was untested), and documented all three methods plus the missingwalletfield inConduitConfig's docs table.Coverage impact
indexer.ts40.68% → 87.58% stmts,adapters/keypair.ts54.54% → 100% stmts,client.tsfuncs 60% → 100%.Checklist
npm run typecheck— no errorsnpm run lint— no warningsnpm test— 702 passed, 2 skipped (post-merge withmain)npm run build— bundle compiles cleanlyanytypes introduced in production codedocs/api.mdCHANGELOG.mdupdated under[Unreleased]Closes #361, Closes #362, Closes #363, Closes #364