Skip to content

fix(bridge-core): space RPC retries across the rate period, not inside it - #91

Merged
Yoni-Starkware merged 1 commit into
mainfrom
fix/retry-delay-for-per-second-limit
Aug 23, 2026
Merged

Yoni-Starkware merged 1 commit into
mainfrom
fix/retry-delay-for-per-second-limit

Conversation

@Yoni-Starkware

@Yoni-Starkware Yoni-Starkware commented Aug 23, 2026 •

Copy link
Copy Markdown
Collaborator

Alchemy's throttle message names the limit explicitly: compute units per SECOND.

viem's backoff doubles from retryDelay, so at 250 ms with retryCount: 2 the three attempts land at 0, 250 and 750 ms — all inside the same exhausted second. Every retry re-spends a budget that hasn't refilled. That's why the provider's own advice — "If you have retries enabled, you can safely ignore this message" — wasn't holding for us: the retries were structurally incapable of succeeding.

At 1000 ms they land at 0, 1 s and 3 s, so each attempt gets a fresh window.

The property, not the number

A retry against a rate limit has to cross the rate's period. No smaller delay can, whatever the retry count. So the pinning test asserts retryDelay >= 1000 as a property of the ceiling being per-second, rather than hard-coding a chosen value.

Cost

Worst case before a rejection surfaces: 750 ms → 3 s. Paid only when the endpoint is actually shedding, and still a small fraction of a consumer's scan timeout (30 s in the app).

One existing test re-derived

keeps the whole backoff well inside a caller scan timeout bounded the total at 2000 ms. That bound was calibrated when the delay was 250 ms, before crossing the rate period was accounted for — a per-second ceiling makes 3000 ms the floor at two retries. Re-derived from the consumer's 30 s timeout (~1/6 of it) rather than bumped to fit, with the derivation written down so the next change doesn't have to guess.

Gate

  • 6/6 retry tests; full bridge-core suite 124/124 files after pnpm build.
  • tsc --noEmit clean; lint 0 errors / 4 warnings — byte-identical to origin/main.
  • Mutation-verified: restoring 250 ms fails the rate-period test and nothing else.

Note for the consumer

This makes each individual read's retry actually able to succeed. It does not by itself stop a scan from failing — that needs the caller to retry the failed indices rather than abort (offmarket #1245), which is drafted pending its own audit fixes.

🤖 Generated with Claude Code


This change is Reviewable

@cursor

cursor Bot commented Aug 23, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Small transport-config tweak (retry delay only) with tests; no auth, data, or request-shape changes. Worst-case stall on a shedding RPC grows from ~750ms to ~3s.

Overview
Makes Polygon RPC 429 retries actually able to succeed against a per-second provider cap.

RPC_RETRY_DELAY_MS on getPolygonPublicClient goes from 250ms to 1000ms. With viem’s doubling backoff and retryCount: 2, attempts used to land at 0 / 250 / 750ms (all in the same exhausted second). They now land at 0 / 1s / 3s so each retry gets a fresh window. Worst-case wait before rejection is ~3s, still a small fraction of a ~30s scan timeout.

Tests pin retryDelay >= 1000 as a rate-period property and raise the total-backoff ceiling from 2s to 5s to match the new floor (~3s).

Reviewed by Cursor Bugbot for commit 1b9b8f2. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit cca2b8f. Configure here.

Comment thread packages/bridge-core/package.json Resolved
…e it

Alchemy's throttle names the limit explicitly: compute units per SECOND. viem's
backoff doubles from retryDelay, so at 250ms with retryCount 2 the three attempts
land at 0, 250 and 750ms — all inside the same exhausted second. Every retry
re-spends a budget that has not refilled, which is why the provider's own advice
("if you have retries enabled, you can safely ignore this message") was not holding
for us: the retries were structurally incapable of succeeding.

At 1000ms they land at 0, 1s and 3s, so each attempt gets a fresh window. That is the
whole property — a retry against a RATE limit has to cross the rate's period, and no
smaller delay can, whatever the retry count. The pinning test asserts the property
(delay >= the rate period) rather than the number.

Worst case before a rejection surfaces goes 750ms -> 3s. That is paid only when the
endpoint is actually shedding, and it stays a small fraction of a consumer's scan
timeout (30s in the app).

An existing test bounded the total backoff at 2000ms. That bound was calibrated when
the delay was 250ms and crossing the rate period had not been accounted for; a
per-second ceiling now makes 3000ms the FLOOR at two retries. Re-derived from the
consumer's timeout — roughly a sixth of it — rather than bumped to fit.

No version change: this PR leaves packages/bridge-core/package.json at 0.1.21, which
is already published. Reaching a consumer needs a separate release bump, matching the
repo's chore(release) convention.

Gate: 6/6 retry tests, full bridge-core suite 124/124 files after a build. Typecheck
clean; lint 0 errors and 4 warnings, byte-identical to origin/main. Mutation-verified
— restoring 250ms fails the rate-period test and nothing else.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Yoni-Starkware
Yoni-Starkware force-pushed the fix/retry-delay-for-per-second-limit branch from cca2b8f to 1b9b8f2 Compare August 23, 2026 11:09
@Yoni-Starkware
Yoni-Starkware merged commit d6c6a2e into main Aug 23, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant