getLogs: keep cached logs when trimming instead of persisting an empty range

[0xshubhs]

Fixes #247.

The 300MB trim path in addLogsToCache built its clone with an empty log list and only ever filled it inside if (c.logs.length > 200_000):

const clone: any = { logs: [], metadata: JSON.parse(JSON.stringify(c.metadata)) }
if (c.logs.length > 200_000) { ... clone.logs = c.logs.slice(0, 200_000) ... }
return clone

There is no else, so any cache entry at or under the cap was persisted as logs: [] while metadata kept the original fromBlock/toBlock. On the next run the read path matches that range, filters an empty array and returns nothing, and the gap filler skips it because the metadata says it is covered — so the range reads as zero logs until the cache file is deleted. That the metadata is only narrowed in the >200k branch is the tell that the empty case was never intended.

The shape that makes it likely rather than theoretical: when several ranges are cached the code first collapses them to [cacheWithLatestFromBlock], and a single range is very often under the cap, so the common outcome of tripping the threshold is one entry with every log dropped.

the change

Default the clone to the logs it already holds, and only replace them when actually trimming. Pulled the block out into trimCachesForStorage(caches, maxLogs) so it can be tested without standing up a 300MB cache — the behaviour is otherwise unchanged, including keeping only the newest range and narrowing fromBlock to the oldest surviving log.

tests

Four cases in src/util/logs.test.ts, no network:

• logs survive when the range is under the cap
• no cache is ever returned claiming a range it has no logs for
• over the cap, exactly maxLogs newest logs are kept and fromBlock moves to the oldest survivor
• with several ranges cached, only the newest is kept

Against the old logic 3 of the 4 fail; the trimming case passes on both, since that path was already correct. With the change all 4 pass, run twice.

[BhariGowda]

Reviewed this against master. The diagnosis holds, the fix is the right direction, and using [...c.logs].sort() also stops the in-place sort mutating the live in-memory cache, which master did. Ran the file twice: the four new trimCachesForStorage cases pass every time. Three network tests failed on one of my runs and passed on the next, and full logs.test.ts is green on master, so those are flaky rather than yours.

One thing worth deciding before this merges, because the fix changes what the branch does rather than only what it stores.

The guard exists to cap the file at ~300MB. It fires on estimated size, but trims on a log count:

if (fileSizeInMB > 300) { ... }
...
if (c.logs.length > maxLogs) { ...trim... }

On master those two disagreeing is what produced the empty cache. With this change they disagree in the other direction: when the size threshold trips at fewer than 200k logs, trimCachesForStorage returns the entry untouched and the file is not capped at all, while the line above still prints retaining only the latest 200k logs.

That regime is not hypothetical. Sampling Seaport 1.6 OrderFulfilled on mainnet just now, 93 logs across a 40 block window:

min 1553   median 2192   max 2512   bytes of JSON per log
300MB / 2192 = ~143,500 logs

So a cache of that contract crosses 300MB at roughly 143.5k logs and never reaches the 200k cap. The break-even is 1572 bytes per log, and any event with a few dynamic fields is above it. Those are exactly the contracts busy enough to trip the threshold in the first place.

Since maxLogs is already a parameter, the call site can derive it from the same budget the estimate uses:

const maxLogs = Math.min(200_000, Math.floor((300 * 1024 * 1024) / randomLogLength))
storeCaches = trimCachesForStorage(storeCaches, maxLogs)

which keeps this PR's fix and makes the cap hold for wide logs too. Happy either way, and it is a fair argument that unbounded growth is better than the silent zeroing this removes. Just better to say which one is intended than to leave the log line claiming a trim that did not happen.

[0xshubhs]

Good catch, and you are right that it needed deciding rather than leaving the log line lying. Taken your derivation.

I checked the break-even you gave before changing anything and it lands exactly where you said: 300 * 1024 * 1024 / 200_000 is 1572.86, so 1572 bytes per log still keeps the flat cap and 1573 drops below it. At the 2192 bytes you sampled off Seaport the cap becomes 143,509, which is the number the size check was already implying.

Rather than inline it at the call site I pulled it out so it can be tested:

export function maxLogsForStorage(bytesPerLog: number, cap = 200_000, budgetMB = 300): number {
  if (!bytesPerLog || bytesPerLog < 1) return cap
  return Math.max(1, Math.min(cap, Math.floor((budgetMB * 1024 * 1024) / bytesPerLog)))
}

randomLogLength is 0 when every cached range is empty, so the guard returns the flat cap instead of dividing by zero and trimming to a single log.

The log line now prints the cap it actually applied instead of a hardcoded 200k, which was the part that made this hard to spot from production output.

Four new cases cover it: the wide log capping below 200k and staying inside the byte budget, both sides of the 1572 boundary, the zero and absurd length fallbacks, and trimCachesForStorage honouring a byte derived cap. Eight pure tests in the file pass, tsc --noEmit is clean. Same flaky network tests you saw.

On which behaviour was intended, I agree capping is the right call over unbounded growth — the 300MB guard exists for a reason and this makes it hold for wide logs too, which are exactly the contracts that trip it.