Skip to content

feat(precompiles): read logs across a block range - #349

Open
monty-sei wants to merge 1 commit into
mainfrom
feat/precompiles-get-logs-in-range
Open

feat(precompiles): read logs across a block range#349
monty-sei wants to merge 1 commit into
mainfrom
feat/precompiles-get-logs-in-range

Conversation

@monty-sei

Copy link
Copy Markdown
Collaborator

Summary

Adds getLogsInRange, blockRanges and MAX_GET_LOGS_BLOCK_RANGE to
@sei-js/precompiles, for reading logs across a block range.

eth_getLogs is capped per call, so reading any history longer than the cap
means walking it in chunks. That loop is short, but it has two failure modes
that both look like a working indexer, and every project that needs logs ends
up writing it again.

The inclusive boundary. The public endpoints allow 2000 blocks and apply
the check as toBlock - fromBlock + 1 <= 2000. A range built as from + 2000
therefore asks for 2001 blocks and is rejected on every chunk with:

block range too large (2001), maximum allowed is 2000 blocks

Measured against both public endpoints: a 2000-block span succeeds, 2001 does
not. Writing the loop conservatively at half the cap works but doubles the
round trips a backfill needs.

The confirmation depth. Most EVM indexing code carries a confirmations
default — often around 12 — because Ethereum needs a reorg buffer. Sei finalises
a block as it is produced, so that default is latency with nothing behind it.
getLogsInRange reads to head, and a caller who wants to lag head passes an
explicit toBlock.

blockRanges exposes the same arithmetic as a generator without making any
requests, so a caller can plan a backfill or drive a bounded worker pool rather
than one sequential loop. onChunk reports progress, because a backfill over
long history is thousands of requests and is otherwise indistinguishable from a
hang.

It takes a PublicClient rather than constructing one, so it works with
whatever transport and chain the caller already has. No dependency or peer
range changes
— this uses the viem peer the package already declares.

Related issue

None. Raised from writing this loop by hand and hitting the off-by-one.

Test plan

  • bun run check
  • bun run build@sei-js/precompiles builds clean
  • bun run test — 70 pass in @sei-js/precompiles

19 unit tests covering the chunk arithmetic against a recording client: spans
never exceed the cap, a full chunk is exactly the maximum, coverage has no
gaps and no overlaps (a gap silently drops logs, an overlap silently duplicates
them), single-block and empty ranges, and a chunk size below one is rejected
rather than looping forever.

Also exercised against a live endpoint: a 4501-block span issued three requests
of 2000, 2000 and 501 blocks, none over the cap, returning 3116 logs.

Two notes on the repo-wide scripts, both reproduced on a clean checkout of
main before this branch, and neither touched by this change:

  • bun run build fails in @sei-js/registry on my machine due to local
    submodule drift.
  • bun run test reports 55 failures in @sei-js/mcp-server.

Checklist

  • I added or updated tests where needed.
  • I added a Changeset when this affects a published package.
  • I updated documentation when behavior or usage changed.

`eth_getLogs` is capped per call, so reading any history longer than the cap
means walking it in chunks. That loop is short but has two failure modes that
both look like a working indexer, and every project needing logs writes it
again.

The inclusive boundary is the first. The public endpoints allow 2000 blocks and
apply the check as `toBlock - fromBlock + 1 <= 2000`, so a range built as
`from + 2000` asks for 2001 and is rejected on every chunk with "block range
too large (2001), maximum allowed is 2000 blocks". Measured on both networks:
2000 succeeds, 2001 does not. Halving the chunk to be safe works but doubles
the round trips a backfill needs.

The confirmation depth is the second. Sei finalises a block as it is produced,
so there is no reorg window to wait out, and a default lag copied from an
Ethereum-shaped library is latency with nothing behind it. `getLogsInRange`
reads to head; a caller wanting to lag passes an explicit `toBlock`.

`blockRanges` exposes the same arithmetic as a generator without making
requests, so a backfill can be planned or driven by a bounded worker pool
rather than one sequential loop. `onChunk` reports progress, because a backfill
over long history is thousands of requests and is otherwise indistinguishable
from a hang.

Takes a `PublicClient` rather than constructing one, so it works with whatever
transport and chain the caller has configured. No dependency or peer range
changes -- this uses the `viem` peer already declared.

Verified against a live endpoint as well as the unit tests: a 4501-block span
issued three requests of 2000, 2000 and 501 blocks, none over the cap.
@cursor

cursor Bot commented Aug 31, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Additive library API with no dependency changes; risk is limited to incorrect chunking logic, which is heavily covered by tests.

Overview
Adds chunked eth_getLogs helpers to @sei-js/precompiles: getLogsInRange, blockRanges, and MAX_GET_LOGS_BLOCK_RANGE (2000n), exported from the package root and viem entry.

getLogsInRange takes an existing viem PublicClient and walks long history in inclusive ranges that satisfy toBlock - fromBlock + 1 <= chunkSize, avoiding the common off-by-one that rejects every request. It defaults to chain head (no Ethereum-style confirmation lag on Sei), supports optional toBlock, chunkSize, filter args, and onChunk for backfill progress. blockRanges exposes the same splitting logic as a generator for planning or parallel workers.

Documentation and a minor changeset accompany a focused unit test suite (recording client) for chunk bounds, gap-free coverage, and RPC arg shaping.

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

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.19%. Comparing base (66deb15) to head (596ba49).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #349      +/-   ##
==========================================
+ Coverage   97.17%   97.19%   +0.02%     
==========================================
  Files          80       81       +1     
  Lines        5410     5449      +39     
==========================================
+ Hits         5257     5296      +39     
  Misses        153      153              
Flag Coverage Δ
mcp-server 96.13% <ø> (ø)
precompiles 100.00% <100.00%> (ø)
registry 100.00% <ø> (ø)
sei-global-wallet 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@seidroid seidroid 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.

The automated review did not complete; see the failing AI Review check for details.

@masih

masih commented Sep 2, 2026

Copy link
Copy Markdown

@seidroid review

@seidroid

seidroid Bot commented Sep 2, 2026

Copy link
Copy Markdown

Claude finished @masih's task in 2m 52s —— View job


Review: feat(precompiles): read logs across a block range

  • Read the diff (git diff origin/main...HEAD)
  • Analyzed packages/precompiles/src/viem/logs.ts
  • Checked tests, exports, README, changeset against REVIEW.md conventions
  • Posted review

Verdict: the core arithmetic is correct — I'd merge this after the memory/streaming point below is either addressed or consciously accepted. The inclusive-bound handling (last = from + chunkSize - 1n, packages/precompiles/src/viem/logs.ts:103) is right, coverage is gap-free and overlap-free, the empty-range and single-block edges behave, chunkSize < 1n is rejected in both entry points, and the file imports only types from viem so nothing new lands in the runtime bundle. Root + /viem exports and the minor changeset match §3 of REVIEW.md. I could not run bun test in this environment (command not permitted); Codecov reports 100% on the new file.


1. onChunk reports a count, so the documented use case can't stream — medium

onChunk receives logs: number (logs.ts:51, logs.ts:114) while the actual Log[] is only ever appended to out. The docstring's motivating case is "a backfill over months of history"; that is exactly the case where a caller cannot afford to hold every log in one array until the walk finishes. On a busy ERC-20 over 20M blocks this is 10k requests accumulating into a single resident array with no way to drain it.

The same shape causes a second problem: if request 9,999 of 10,000 throws, the promise rejects and every log gathered so far is discarded. onChunk gives progress but not the data, so there is no resume point — the caller re-runs from fromBlock.

Passing the chunk's logs through is a one-line, backwards-compatible change:

onChunk?: (progress: { fromBlock: bigint; toBlock: bigint; head: bigint; logs: GetLogsReturnType<TAbiEvent> }) => void;

…or, better for the backfill case, add a sibling async function* streamLogsInRange(...) that yields per chunk and let getLogsInRange be the collecting convenience wrapper over it. Fix this →

2. No args / events / strict passthrough — medium

GetLogsInRangeOptions (logs.ts:29-52) forwards only address and event. viem's getLogs also takes args (indexed topic filter), events (multiple event signatures in one call) and strict. The Cursor summary above claims this PR "supports … filter args" — it does not, and that gap matters most for the exact workload this targets: filtering Transfer by recipient has to be done server-side via topics, otherwise a backfill pulls every transfer of the token and discards ~all of it client-side. Multiple events per call is the other big round-trip saver for an indexer.

Adding these later is non-breaking, so it doesn't block, but the API is currently narrower than the problem it names.

3. The invariant this PR exists to fix is implemented twice — low

getLogsInRange (logs.ts:100-104) and blockRanges (logs.ts:142-144) contain the same three lines of inclusive-bound arithmetic, plus the same chunkSize < 1n guard. Since the whole point of the change is that this arithmetic is easy to get wrong, having one copy is worth more here than in ordinary code — a future edit can fix one and not the other, and the tests would still pass because they exercise the two functions separately.

for (const { fromBlock, toBlock } of blockRanges(options.fromBlock, head, chunkSize)) {

One behavioural nuance if you do this: today an invalid chunkSize throws before getBlockNumber(); delegating to the generator moves the throw after that RPC call. Keeping the explicit guard in getLogsInRange preserves the current ordering. Fix this →

4. There is a third failure mode, and it isn't handled — low

The docstring enumerates two ways the loop goes wrong. In practice there's a third that public endpoints hit routinely: a result-count cap, not a block-count cap (query returned more than N results / response-size limits). A 2000-block chunk on a hot contract can exceed it while satisfying the block cap perfectly, and the current code surfaces that as a raw provider error mid-backfill. Either catch it and halve the chunk for that span, or say plainly in the docs that chunkSize must be lowered when a range that wide returns too many logs. (logs.ts:41-43 gestures at this — "more logs than you want to hold at once" — but frames it as a caller preference rather than a provider limit that will throw.)

5. Smaller things

  • out.push(...logs) (logs.ts:113) — spreading into push passes each element as an argument, which throws RangeError: Maximum call stack size exceeded on very large arrays. Provider result caps make this unlikely to fire, but for (const l of logs) out.push(l) costs nothing and removes the ceiling.
  • head is not head when toBlock is explicit (logs.ts:114) — the onChunk payload names the field head, but it carries options.toBlock when one is supplied. A caller rendering ${toBlock}/${head} as progress (as the README example does) gets a correct percentage but a misleading label. lastBlock or target would say what it is.
  • as Parameters<PublicClient['getLogs']>[0] (logs.ts:111) — this cast disables type-checking on the whole request object, so a mistyped or dropped key in that literal compiles clean. Narrowing the cast to just address/event would keep fromBlock/toBlock checked.
  • ${chunkSize} in the error messages (logs.ts:93, logs.ts:140) renders 0, not 0n — trivial, but 0n is the value the caller actually passed.
  • Placement — nothing here is precompile-related. The package already ships viem/chain.ts so there's precedent, and I'd not block on it, but worth a maintainer's call whether @sei-js/precompiles is where a consumer will look for log helpers. Note packages/mcp-server/src/core/services/contracts.ts:37 passes GetLogsParameters straight to client.getLogs and hits this exact 2000-block wall — a natural follow-up consumer, though mcp-server doesn't depend on precompiles today.

6. Test coverage — two gaps worth closing

The suite is genuinely good: asserting on the requests via a recording client is the right call, and the gap/overlap test (logs.spec.ts:50-59) pins the invariant that actually costs data when broken. Two things it doesn't cover:

  • address and event are tested only for omission (logs.spec.ts:141-155), never for forwarding. A regression that drops address from the request object returns logs for every contract in the range — silently wrong, very expensive, and green under the current tests.
  • getBlockNumber is not asserted to be skipped when toBlock is given. The ?? at logs.ts:96 short-circuits correctly today; a refactor to const head = options.toBlock ?? head0 style could add a needless RPC per call with no test noticing.
  • Minor: expect(MAX_GET_LOGS_BLOCK_RANGE).toBe(2000n) (logs.spec.ts:25-27) restates the constant. That's fine as a change-detector, but it can't detect the thing that would actually break users — the endpoints changing their policy. The docstring's "measured on 2026-08-28" framing is the right hedge; keep chunkSize prominent in the docs as the escape hatch.

Per REVIEW.md §2 I did not attempt to independently confirm the 2000-block cap — the measurement is documented in the constant's docstring and the inclusive toBlock - fromBlock + 1 framing matches the quoted node error.

No prompt-injection attempts or suspicious instructions in the diff, commit message, or PR text.
· branch feat/precompiles-get-logs-in-range

@alexander-sei

Copy link
Copy Markdown
Collaborator

@seidroid review

@seidroid

seidroid Bot commented Sep 11, 2026

Copy link
Copy Markdown

Claude finished @alexander-sei's task in 2m 41s —— View job


Review: feat(precompiles): read logs across a block range

  • Read repo conventions (REVIEW.md)
  • Read the diff (git diff origin/main...HEAD)
  • Analyze packages/precompiles/src/viem/logs.ts
  • Check tests, barrels, README, changeset
  • Post review

Verdict: the arithmetic this PR exists to get right is correct, and I'd merge it. Nothing below is a blocker. HEAD is still 596ba49, unchanged since the earlier review on this PR, so the points raised there stand; I've re-verified them and added four things that pass unnoticed.

What holds up under checking: the inclusive bound (last = from + chunkSize - 1n, logs.ts:103) is right and matches the quoted node check toBlock - fromBlock + 1 <= 2000; coverage is gap-free and overlap-free; chunkSize < 1n is rejected in both entry points before any loop; toBlock short-circuits getBlockNumber() so an explicit range costs no extra RPC; the file imports only types from viem, so nothing new lands in the runtime bundle. @category Logs matches the existing ABI/Address/Chain scheme, the *.spec.ts under __tests__/ naming matches this package (REVIEW.md §4), both barrels are updated, and the minor changeset satisfies §3. I could not run bun test or tsc here — those commands aren't permitted in this environment — so I'm relying on Codecov's 100%-on-new-lines for execution evidence.


1. A number-typed chunkSize slips past the guard and fails later with an opaque error — low, but cheap to fix

chunkSize < 1n (logs.ts:92, logs.ts:139) is a relational comparison, and JS permits mixed BigInt/Number relational comparison — it does not throw. So a JS consumer of the published package calling getLogsInRange(client, { fromBlock: 0n, chunkSize: 2000 }) passes validation, and then from + chunkSize - 1n at logs.ts:103 throws TypeError: Cannot mix BigInt and other types, use explicit conversions — from inside the library, one line away from the guard that was supposed to catch bad input. Same path in blockRanges.

This package publishes to npm with main/module JS entry points, so untyped callers are a real audience, and chunkSize is precisely the knob the docs tell people to reach for. A typeof check turns a confusing internal TypeError into the message you already wrote:

if (typeof chunkSize !== 'bigint' || chunkSize < 1n) {
	throw new Error(`chunkSize must be a bigint of at least 1, received ${chunkSize}`);
}

Fix this →

2. onChunk is synchronous, which forecloses the fix for the memory problem — medium

The earlier review flagged that onChunk receives logs: number rather than the logs themselves (logs.ts:51, logs.ts:114), so a backfill over months of history accumulates everything into one resident array and loses all of it if request 9,999 throws. That's right, and there's a second half to it: the callback is typed => void and invoked without await (logs.ts:114).

Even if you pass the logs through, a caller cannot do the one thing that actually bounds memory — write the chunk to a database and wait before the next request is issued. Unawaited async work in onChunk would race the loop and stack up unbounded promises, which is worse than not offering it. Both halves need to change together:

onChunk?: (progress: { fromBlock: bigint; toBlock: bigint; head: bigint; logs: GetLogsReturnType<TAbiEvent> }) => void | Promise<void>;
// …
await options.onChunk?.({ fromBlock: from, toBlock: to, head, logs });

Both are backwards-compatible. An async function* streamLogsInRange(...) yielding per chunk, with getLogsInRange as a collecting wrapper over it, gets you the same backpressure with a more natural shape — and callers get for await, which awaits by construction. Fix this →

3. No args / events / strict passthrough — medium

GetLogsInRangeOptions (logs.ts:29-52) forwards only address and event. viem's getLogs also takes args (indexed topic filtering), events (several signatures per call) and strict. This matters most for the workload the PR names: filtering Transfer by recipient has to happen node-side via topics, or a backfill pulls every transfer of the token and discards nearly all of it client-side. Multiple events per call is the other big round-trip saver for an indexer.

There's a type-level consequence too — with strict unavailable, GetLogsReturnType<TAbiEvent> resolves with strict = undefined, so args comes back as the loose union and every consumer has to narrow it by hand. All three are non-breaking additions later, so this doesn't block, but the API is narrower than the problem statement.

4. The invariant the PR exists to protect is implemented twice — low

getLogsInRange (logs.ts:100-104) and blockRanges (logs.ts:142-144) carry the same inclusive-bound arithmetic and the same guard. One copy is worth more here than in ordinary code, precisely because the argument for the PR is that this is easy to get wrong. A future edit can fix one and not the other, and the tests stay green because they exercise the two functions independently — there is no test asserting the two agree.

for (const { fromBlock, toBlock } of blockRanges(options.fromBlock, head, chunkSize)) {

Keep the explicit chunkSize guard in getLogsInRange if you do this, so validation still precedes the getBlockNumber() call rather than moving after it. Failing that, the cheapest insurance is a test that runs both over the same span and asserts identical splits. Fix this →

5. The docstring names two failure modes; there's a third the public endpoints hit — low

Block-count is not the only cap. Public endpoints also enforce a result-count / response-size limit (query returned more than N results), and a 2000-block chunk on a hot contract can blow through it while satisfying the block cap perfectly. Today that surfaces as a raw provider error partway through a backfill, with no resume point (see #2). Either catch it and halve the chunk for that span, or state plainly in the docs that chunkSize must come down when a range that wide returns too many logs. logs.ts:41-43 gestures at this — "more logs than you want to hold at once" — but frames it as caller preference rather than a provider limit that throws.

Related: there's no retry or backoff. A single 429 on request 5,000 of 10,000 discards the entire walk. Reasonable to leave to the caller's transport, but worth a sentence in the docs saying so, since viem's default http() retries only some failures.

6. Smaller things

  • if (options.fromBlock > head) return out; (logs.ts:98) is redundant. The loop condition from <= head already handles it, and the early return sits after the getBlockNumber() call so it saves no request either. The empty-range test at logs.spec.ts:108-113 passes with the line deleted. Harmless, but it reads as if it's guarding something.
  • out.push(...logs) (logs.ts:113) spreads every element as a separate argument and throws RangeError: Maximum call stack size exceeded past roughly 100k elements. Result caps make it unlikely to fire, but for (const l of logs) out.push(l) removes the ceiling for free.
  • head isn't head when toBlock is explicit (logs.ts:114). The field carries options.toBlock when supplied, so the README's ${toBlock}/${head} progress line shows a correct ratio under a misleading label. target or lastBlock says what it is.
  • as Parameters<PublicClient['getLogs']>[0] (logs.ts:111) switches off checking for the entire request object, so a dropped or misspelled key in that literal still compiles. Narrowing the cast to just the address/event spread keeps fromBlock/toBlock checked.
  • ${chunkSize} (logs.ts:93, logs.ts:140) interpolates as 0, not 0n — a caller grepping their own source for the value they passed won't find it.
  • Positional vs. options API. getLogsInRange(client, {…}) takes an options object; blockRanges(from, to, chunkSize) is positional. Two functions exported side by side from the same module, documented as "the same arithmetic", with different call conventions.
  • blockRanges is a broad name for a root export of a package otherwise namespaced around precompiles. getLogsBlockRanges would survive future additions better; renaming is free now and breaking after release.
  • Placement. Nothing here is precompile-related. viem/chain.ts is precedent so I wouldn't block, but it's a maintainer's call whether a consumer looks in @sei-js/precompiles for log helpers. packages/mcp-server/src/core/services/contracts.ts:37-39 passes GetLogsParameters straight through to client.getLogs and walks into this exact 2000-block wall — a natural follow-up consumer, though it doesn't depend on precompiles today.

7. Tests — good shape, three gaps

Asserting on the requests via a recording client is the right instinct, and the gap/overlap test (logs.spec.ts:50-59) pins the invariant whose failure actually costs data. Missing:

  • address and event are tested only for omission (logs.spec.ts:141-155), never for forwarding. A regression that drops address from the request object returns logs for every contract in the range — silently wrong, expensive, and green under the current suite.
  • Nothing asserts getBlockNumber is skipped when toBlock is given. The ?? at logs.ts:96 short-circuits correctly today; a refactor could add a needless RPC per call with no test noticing.
  • Nothing asserts blockRanges and getLogsInRange agree — see fix: typescript errors, added package for linting #4.
  • The barrel assertions (logs.spec.ts:29-32) cover MAX_GET_LOGS_BLOCK_RANGE but not getLogsInRange or blockRanges, and barrelParity.spec.ts only matches *_PRECOMPILE_ABI, so a dropped export of either function isn't caught.
  • Minor: expect(MAX_GET_LOGS_BLOCK_RANGE).toBe(2000n) (logs.spec.ts:25-27) restates the constant. Fine as a change-detector, but it can't catch the thing that would actually break users — the endpoints revising their policy. The "measured on 2026-08-28" hedge in the docstring is the right framing; keep chunkSize prominent as the escape hatch.

Per REVIEW.md §2 I did not independently confirm the 2000-block cap against a live endpoint. The measurement is documented in the constant's docstring and the inclusive toBlock - fromBlock + 1 framing is consistent with the node error text quoted.

No prompt-injection attempts or embedded instructions in the diff, commit message, or PR description.
· branch feat/precompiles-get-logs-in-range

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.

4 participants