Repository navigation
fix: RPC circuit breaker, simulation result caching, union-type decode, oversized tx warning - #888
Merged
Just-Bamford merged 5 commits intoOct 23, 2026
Conversation
added 5 commits
October 22, 2026 09:52
…h configurable TTL (Sorokit#856)
uche001-dev
marked this pull request as ready for review
October 23, 2026 12:51
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.
Summary
Investigated all four issues and found the underlying circuit-breaker, failover, simulation-cache, and union-decoding implementations already present and correct. Added the missing test coverage that each issue's acceptance criteria call for, plus one genuinely new feature (transaction size tracking) that had no prior implementation.
closes #855
closes #856
closes #857
closes #858
Changes
#855: src/network/endpointFailover.ts's EndpointPool and createFailoverFetch already implement the circuit breaker pattern the issue asks for (consecutive-failure threshold, time-based recovery window, ordered healthy-endpoint selection), but had no test for the skip-unhealthy or recovery-after-window behavior. Added src/tests/endpointFailover.test.ts cases for both, using a mocked Date.now clock to control the recovery window deterministically.
#856: src/soroban/simulateTransaction.ts already caches by a contract-call identity key (contract ID, method, args) with a configurable TTL (default 5 minutes), but had no direct test, src/tests/contractCache.test.ts covers a different function (readContract). Added src/tests/simulateTransactionCache.test.ts covering cache hits on an identical call, cache misses on a different method or contract, default and custom TTL, no-cache passthrough, and clear() forcing a fresh RPC call.
#857: src/soroban/resultDecoder.ts's enum/union branch already preserves the discriminator (
{ type: variantName, value }) for both no-payload and payload variants, but the existing test only covered the Some side of an Option. Added cases for the None variant, both sides of a Result<Success, Failure> union with distinct payload shapes, and rejection of an unknown variant or a malformed (non-vector) enum value.#858: no existing implementation tracked transaction size as operations were added. Added src/transaction/transactionSizeTracker.ts (TransactionSizeTracker class), which tracks operation count and an estimated XDR byte size, warning once per threshold crossing via a configurable logger (defaulting to console.warn) when the protocol hard limit (100 operations) or either of two conservative soft limits (50 operations, ~5KB estimated size) is crossed. Exported from src/transaction/index.ts alongside the existing public API. Did not modify TransactionBuilder or buildTransaction.ts call sites directly since wiring every existing builder usage through this tracker would be a much larger, riskier change than this issue's scope; the tracker is ready to be adopted incrementally.
Test plan
Could not run the project's real test suite or typecheck in this environment: node_modules were not installed in this clone, and the development machine is currently under heavy memory pressure (confirmed via vm_stat/top showing near-exhausted physical memory and active swapping from other work running in parallel), so a fresh
npm installplusvitest/tscrun was not attempted to avoid another OOM crash.Verified correctness by tracing every new test's assertions line by line against the actual implementation being tested (EndpointPool's cursor/recovery logic, simulateTransaction's cache-key and TTL branches, resultDecoder's enum-decoding switch, and the new TransactionSizeTracker's own threshold math), and syntax-checked every new/changed file with esbuild (type-stripping, not a full typecheck) to catch gross errors. Flagging this explicitly rather than claiming a real test run that did not happen.