Repository navigation
feat: batched streamAddresses, governor config TTL cache, setWallet/doc fixes - #887
Merged
Jaydbrown merged 8 commits intoSep 29, 2026
Conversation
…protocol#783) `FactoryModule.streamAddress(id)` costs one simulated RPC call per id, so rendering a page of 50 streams from `streamsBySender()` costs 50 round trips on a cold cache — the per-id cache only helps once the same id is asked for twice. These tests describe the contract for a batched `streamAddresses(ids[])`: one call resolves a whole page, returning a `Map` keyed by decimal id string, with `null` for ids the contract reports as not-found. They pin: - ordering, duplicate/string-id collation and the empty-list case - abort semantics, both pre-aborted and aborting mid-flight - that it shares `streamAddress()`'s cache and negative-cache TTL rather than growing a second cache - that in-flight simulations are bounded by `maxConcurrency` but still run in parallel at the default - that a resolution which *throws* propagates and is never recorded as a not-found result, so a retry re-fetches only the ids that failed All 16 fail on main with "streamAddresses is not a function". Refs conduit-protocol#783
…er (conduit-protocol#783) Adds `FactoryModule.streamAddresses(ids[], signal?, options?)`, returning a `Map` of decimal stream-id string to contract address (`null` when the contract reports `None`). Rendering a page of `streamsBySender()` results no longer costs one simulated RPC round trip per row on a cold cache. Design notes: - Every id is resolved through the existing `streamAddress()`, so there is one cache, one negative-cache TTL and one set of hit/miss counters rather than a second set that could drift. Cached ids (positive hits, and negative hits still inside `negativeCacheTtlMs`) therefore cost no RPC at all, and the batch method is free of any cache-expiry logic of its own. - Ids are de-duplicated before dispatch, so a page that repeats an id — or mixes the string and bigint forms of one — schedules a single simulation per distinct id. - Concurrency is bounded (default 8) rather than fanned out with `Promise.all`. A default `stellar-rpc` serves `simulateTransaction` from 8 preflight workers, so a 50-id `Promise.all` would only convert into queue time and then timeouts. `mapWithConcurrency` and its default bound are extracted from `streams.ts` into `src/map-with-concurrency.ts` so the paged `list()`/`getStreamInfos()` path and this one cannot disagree; the helper now also clamps a sub-1 or non-finite bound to 1, which previously made `getStreamInfos({ maxConcurrency: 0 })` resolve to no results at all. - A resolution that *throws* propagates and is never recorded as a not-found result — caching a transport failure as `null` would serve a bogus "stream does not exist" for the whole negative-cache TTL. Ids that did resolve stay cached, so a retry re-fetches only the failures. `StreamAddressesOptions` is exported from the package entry point. Refs conduit-protocol#783
…ocol#785) `GovernorModule.getConfig()` re-simulates the `config` contract call on every invocation, unlike `FactoryModule`'s address cache or `soroban.ts`'s token-decimals cache. Protocol parameters change only via a passed governance proposal, so a dashboard polling `getConfig()` on an interval pays a full simulation round trip for data that is almost always unchanged. These tests describe the contract for a short-TTL cache, configurable via `ConduitConfig.governorConfigCacheTtlMs` following the existing `negativeCacheTtlMs` precedent. They pin: - a repeated call inside the TTL is served without a simulation, and a call past the TTL picks up the new on-chain value - `governorConfigCacheTtlMs: 0` disables caching entirely - concurrent misses share one simulation (no thundering herd at expiry) - a failed simulation is never cached, and a shared failure rejects every concurrent caller rather than leaving a poisoned entry behind - `clearConfigCache()` forces the next call to re-simulate - every caller gets its own object, so mutating one result cannot rewrite the cached config seen by every other consumer in the process - abort and missing-`governorAddress` paths are unaffected, and caches do not leak between module instances 7 of the 12 fail on main, where every call re-simulates. Refs conduit-protocol#785
…protocol#785) `getConfig()` re-simulated the `config` contract call on every invocation, unlike `FactoryModule`'s address cache or `soroban.ts`'s token-decimals cache. Protocol parameters change only when a governance proposal passes, so a dashboard polling `getConfig()` on an interval paid a full simulation round trip per tick for data that is almost always unchanged. The result is now reused for `ConduitConfig.governorConfigCacheTtlMs` (default 30s, ~6 ledgers at the 5s close cadence, and the same default as the existing `negativeCacheTtlMs`). Details worth noting: - Concurrent misses share one simulation. A TTL cache without coalescing produces a thundering herd the instant its entry expires — exactly the polling case this is meant to help. Reused `coalesceAsync`, already used for token decimals, and released the entry once it settles: `coalesceAsync` only evicts on *rejection*, so a retained fulfilled promise would pin the first result forever and silently defeat the TTL. A failed simulation is therefore never cached and the next call retries. - A caller's `signal` gates only that call. It is checked before the cache is consulted and never reaches the shared fetch, so one caller aborting cannot cancel a simulation other concurrent callers are awaiting. - Each caller gets its own object. Returning the cached reference would let one component's local edit (`config.feeBps = ...`) silently rewrite the protocol config for every other consumer in the process; `GovernorConfig` is flat, so the copy costs nothing measurable. - The TTL is stamped on *completion*, not on call start, so it bounds staleness from when the data was actually read. A TTL of `0` yields an entry that is never fresh again, which disables caching without a second code path. - `clearConfigCache()` forces the next call to re-simulate, for tests and for callers that just observed a governance proposal passing. Refs conduit-protocol#785
…it-protocol#784) `ConduitClient.setWallet()`'s "Wallet propagation contract" doc block claimed `FactoryModule` "does not hold a wallet reference and is unaffected by setWallet()". It does hold one, and does implement `setWallet()` with async caller-address resolution, so a dApp calling `client.setWallet(newWallet)` left `client.factory`'s simulated caller address pinned to whatever it resolved first — the doc described an intentional design the module's own code contradicts. These tests specify the propagation contract instead: - an already-constructed `client.factory` re-resolves its read-simulation source to the new wallet's public key - a wallet set before the factory is ever constructed still applies, via the lazily-built module reading `config.wallet` - `StreamsModule` keeps receiving the wallet - a cross-network wallet is still rejected before reaching any module, and the factory's source is left untouched The first fails on main, where the factory stays pinned to the first wallet; the other three are regression guards that must keep passing. Refs conduit-protocol#784
…odule (conduit-protocol#784) `ConduitClient.setWallet()`'s "Wallet propagation contract" doc block claimed `FactoryModule` "does not hold a wallet reference and is unaffected by setWallet()". It does hold one, and does implement `setWallet()` with async caller-address resolution — so a dApp calling `client.setWallet(newWallet)` expecting it to propagate everywhere (as it does to the streams module) silently left `client.factory`'s simulated caller address pinned to whatever it resolved first. Resolved by making the code match the module's own intent rather than by deleting `FactoryModule.setWallet()`/`activeWallet`: those are public API, removing them would be a breaking change, and the module was plainly built to support the update path the client-level doc claimed did not exist. `setWallet()` now forwards to `this._factory` when it has already been constructed; when it has not, the lazily built module reads the already-updated `config.wallet` and has nothing to catch up on. `GovernorModule` is deliberately left out of the update and keeps its "not updated" doc entry — that claim is accurate, it holds no wallet adapter at all and sources its simulations from `config.keypair`. The doc block is corrected to say which is which instead of asserting the same thing about two modules that behaved differently. Refs conduit-protocol#784
…and the new/changed APIs left a reader unable to tell whether the pair were two features, a deprecated/replacement set, or a string-typed wrapper — they are the last of those. It now says what it is (a thin string-argument wrapper over `topUp`, with no behaviour of its own), why it exists, and which one new code should prefer: `topUp` is the primary method, takes the `bigint` amount the SDK uses everywhere else, and accepts an `AbortSignal` that the wrapper cannot forward. It also now states plainly that it is *not* deprecated, so nobody reads "alias" as "legacy". `topUp()` gained a one-line back-reference. Also documents the conduit-protocol#783 / conduit-protocol#784 / conduit-protocol#785 public surface: - `docs/api.md` — `factory.streamAddresses()` (params, the `Map<string, string | null>` shape, ordering, dedupe, the shared cache and negative-cache TTL, the failure-vs-not-found distinction), `governor.getConfig()` (the TTL cache, coalescing, per-caller copies, `clearConfigCache()`), `negativeCacheTtlMs` / `governorConfigCacheTtlMs` in the `ConduitConfig` table, and the corrected `setWallet()` propagation contract. - `README.md` — `streamAddresses()` in the `client.factory` example, the `getConfig()` TTL, and the `setWallet()` propagation sentence. - `CHANGELOG.md` — entries under `[Unreleased] / Added` and `/ Changed`. Two pre-existing doc errors in the files being edited are corrected because they name the exact methods changed here: `docs/api.md` and `README.md` both documented `governor.config()`, a method that does not exist (it is `getConfig()`), and both omitted `maxDurationSeconds` from the `GovernorConfig` listing. Closes conduit-protocol#786
…ol#795 tests that fail on main Not part of conduit-protocol#783-conduit-protocol#786, and independently droppable. These three tests came in with b40e6f6 and have been red on `main` since, so they sit in the two suites this branch touches and a reviewer cannot otherwise tell a pre-existing failure from a regression. - `factory.test.ts` "shares the streamAddress cache": mocked one simulation for three calls, so the second `hasStream(999n)` got `undefined` back and threw `Cannot read properties of undefined`. It also asserted three network hits, but a live negative-cache TTL serves the repeat from memory, so the correct count is two. Now mocks both responses and the comment states why. - `factory.test.ts` / `governor.test.ts` abort tests: asserted `rejects.toThrow('AbortError')`, which matches the message. The SDK throws `new DOMException('Aborted', 'AbortError')`, whose message is 'Aborted' and whose `name` is 'AbortError' — so both assertions matched nothing and failed. Now match on `name`, the same way `profile-page-rpc-timeout.test.ts` already does. - `governor.test.ts`: `beforeEach` re-resolved `mockBuildTx` without resetting it, so call counts leaked between tests and the `not.toHaveBeenCalled()` assertion could never pass once an earlier test in the file had called it. The 5 remaining `relayer-stress.test.ts` heartbeat failures are also pre-existing on `main` and unrelated to this branch; they are left alone deliberately. Refs conduit-protocol#783, conduit-protocol#784, conduit-protocol#785
|
@jayteemoney 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! 🚀 |
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
2 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.
Closes #783
Closes #784
Closes #785
Closes #786
Summary
Four independent issues, four separate commits (test-then-implementation for
each feature, per
CONTRIBUTING.md).#783 —
FactoryModule.streamAddresses(ids[])Resolves a page of stream IDs to addresses in one call, returning
Map<string, string | null>keyed by decimal id string (null= not found).The motivation is the stream-list UI: rendering a page of
streamsBySender()results previously cost one simulated RPC round trip per row, because
list()and the UI each resolved addresses throughstreamAddress()and acold cache could not help. Only the cache-miss subset is now fetched, in
parallel, bounded by
options.maxConcurrency(default 8).To be precise about what this does and does not buy: Soroban RPC has no
JSON-RPC request batching, and
simulateTransactiontakes a singletransaction, so this is not a single network call. It is bounded concurrent
fan-out over the cache-miss subset — latency drops from N sequential round
trips to roughly
ceil(N / maxConcurrency)sequential steps. The default of8 matches the 8 preflight workers a stock
stellar-rpcruns, so raising itfurther would just queue on the server. This matches how the existing
buildBatchTransactions()concurrency path and viem/ethers provider batchingwork today.
Every id is resolved through
streamAddress(), so the address cache, itshit/miss counters, and the negative-cache TTL are shared with single-id
lookups in both directions. Duplicate ids — including the same id in both
string and bigint form — are fetched once, and input order is preserved.
A resolution that throws rejects the whole call and is never cached as a
not-found result, so a transient RPC failure cannot be mistaken for "this
stream does not exist". Ids that did resolve stay cached, so a retry re-fetches
only the failures.
#784 —
ConduitClient.setWallet()documentationThe JSDoc claimed
FactoryModulewas "NOT updated … it does not hold a walletreference and is unaffected by
setWallet()", which contradicted the module'sown public
setWallet()/activeWalletsupport. A dApp swapping wallets withan existing
client.factorykept simulating as the previous wallet, with docsaffirming that was intended.
ConduitClient.setWallet()now propagates to an already-constructedFactoryModulevia the existing_factory?.setWallet(wallet). A factorylazily built afterwards picks the new wallet up from
config, so only thealready-constructed case needs catching up.
FactoryModule.setWallet()itselfis unchanged and still public API.
GovernorModuleis deliberately excluded —it holds no wallet at all.
#785 —
GovernorModule.getConfig()TTL cacheConduitConfig.governorConfigCacheTtlMs(default 30s, ~6 ledgers at the 5sclose cadence) bounds how long a config read is reused. Protocol parameters
only change when a governance proposal passes, so a dashboard polling
getConfig()on an interval was paying a simulation per tick for data that isalmost always identical.
TTL-caching protocol config is standard practice —
withCachein viem andAbstractProvider's internal caches in ethers.js both default to short-livedcaches for exactly this reason. Beyond the TTL: concurrent misses share a
single simulation, a failed simulation is never cached, and each caller gets
its own object so mutating a result cannot corrupt what the next caller sees.
clearConfigCache()forces a refresh; TTL0disables caching.#786 —
topUpStreamvstopUptopUpStream()was documented only as "Alias for topUp.", leaving a readerunable to tell whether these were two features, a deprecated/replacement set,
or a string-typed wrapper. They are the last of those. It now states what it
is, why it exists, and which one new code should prefer:
topUpis the primarymethod, takes the
bigintamount the SDK uses everywhere else, and accepts anAbortSignalthe wrapper cannot forward. It also says plainly that it is notdeprecated, so nobody reads "alias" as "legacy". No behaviour change.
Test-only commit
The last commit fixes three tests in
factory.test.ts/governor.test.tsthathave been red on
mainsince b40e6f6, unrelated to the four issues andindependently droppable:
factory.test.ts"shares the streamAddress cache" mocked one simulation forthree calls and asserted three network hits, but a live negative-cache TTL
serves the repeat from memory — the correct count is two.
rejects.toThrow('AbortError'), which matches themessage. The SDK throws
DOMException('Aborted', 'AbortError'), so'AbortError'is in
.nameand the assertion matched nothing. Now matches onname, asprofile-page-rpc-timeout.test.tsalready does.governor.test.tsre-resolvedmockBuildTxinbeforeEachwithout resettingit, so call counts leaked and
not.toHaveBeenCalled()could never pass.Verification
Measured against
upstream/main(63604d4) in a clean worktree, samenode_modules:upstream/maintsc --noEmiteslint srcnpm run buildnpx vitest runThe 6 lint errors and the 5 test failures are pre-existing on
mainandunrelated to this branch. The 5 failures are all
relayer-stress.test.ts > Heartbeat/Ping Support, which mixesvi.useFakeTimers()with realsetTimeoutwaits; fixing them is a separateconcern and out of scope here.
New tests: 35 across
factory-stream-addresses.test.ts(16),governor-config-cache.test.ts(12),client-set-wallet-propagation.test.ts(4),plus the 3 repaired.
streams.test.ts(53 tests) still passes after themapWithConcurrencyextraction.Notes
upstream/mainat 63604d4. ThesetWallet()conflict wasadditive — upstream added
TokenModulepropagation in the same method — soboth propagations are kept.
CHANGELOG.md/docs/api.mdconflicts werelikewise additive.
mapWithConcurrencylives insrc/map-with-concurrency.tsrather thansrc/utils.tsso the bounded-concurrency policy has one home shared bylist(),getStreamInfos()andstreamAddresses(), and they cannot drift.