From 936cc59fcf9abecf18fc6e79446e8632367810e6 Mon Sep 17 00:00:00 2001 From: Michal Rzeszutko Date: Wed, 25 Mar 2026 15:54:22 +0000 Subject: [PATCH] fix: unbounded memory in calldataRetriever --- .../archiver/src/l1/calldata_retriever.ts | 3 +- .../foundation/src/collection/index.ts | 1 + .../foundation/src/collection/lru_set.test.ts | 131 ++++++++++++++++++ .../foundation/src/collection/lru_set.ts | 115 +++++++++++++++ 4 files changed, 249 insertions(+), 1 deletion(-) create mode 100644 yarn-project/foundation/src/collection/lru_set.test.ts create mode 100644 yarn-project/foundation/src/collection/lru_set.ts diff --git a/yarn-project/archiver/src/l1/calldata_retriever.ts b/yarn-project/archiver/src/l1/calldata_retriever.ts index e21f499b9e9..98bcbe52866 100644 --- a/yarn-project/archiver/src/l1/calldata_retriever.ts +++ b/yarn-project/archiver/src/l1/calldata_retriever.ts @@ -1,6 +1,7 @@ import { MULTI_CALL_3_ADDRESS, type ViemCommitteeAttestations, type ViemHeader } from '@aztec/ethereum/contracts'; import type { ViemPublicClient, ViemPublicDebugClient } from '@aztec/ethereum/types'; import { CheckpointNumber } from '@aztec/foundation/branded-types'; +import { LruSet } from '@aztec/foundation/collection'; import { Fr } from '@aztec/foundation/curves/bn254'; import { EthAddress } from '@aztec/foundation/eth-address'; import type { Logger } from '@aztec/foundation/log'; @@ -44,7 +45,7 @@ type CheckpointData = { */ export class CalldataRetriever { /** Tx hashes we've already logged for trace+debug failure (log once per tx per process). */ - private static readonly traceFailureWarnedTxHashes = new Set(); + private static readonly traceFailureWarnedTxHashes = new LruSet(1000); /** Clears the trace-failure warned set. For testing only. */ static resetTraceFailureWarnedForTesting(): void { diff --git a/yarn-project/foundation/src/collection/index.ts b/yarn-project/foundation/src/collection/index.ts index 00f8115dd60..64ebf402e3b 100644 --- a/yarn-project/foundation/src/collection/index.ts +++ b/yarn-project/foundation/src/collection/index.ts @@ -1,2 +1,3 @@ export * from './array.js'; +export * from './lru_set.js'; export * from './object.js'; diff --git a/yarn-project/foundation/src/collection/lru_set.test.ts b/yarn-project/foundation/src/collection/lru_set.test.ts new file mode 100644 index 00000000000..0e19b5196c7 --- /dev/null +++ b/yarn-project/foundation/src/collection/lru_set.test.ts @@ -0,0 +1,131 @@ +import { LruSet } from './lru_set.js'; + +describe('LruSet', () => { + it('stores and retrieves items', () => { + const set = new LruSet(3); + set.add('a'); + set.add('b'); + expect(set.has('a')).toBe(true); + expect(set.has('b')).toBe(true); + expect(set.has('c')).toBe(false); + }); + + it('reports correct size', () => { + const set = new LruSet(5); + expect(set.size).toBe(0); + set.add(1); + expect(set.size).toBe(1); + set.add(2); + set.add(3); + expect(set.size).toBe(3); + }); + + it('does not grow beyond maxSize', () => { + const set = new LruSet(3); + set.add(1); + set.add(2); + set.add(3); + set.add(4); + expect(set.size).toBe(3); + expect(set.has(1)).toBe(false); // evicted (least recent) + expect(set.has(2)).toBe(true); + expect(set.has(3)).toBe(true); + expect(set.has(4)).toBe(true); + }); + + it('evicts least recently used, not least recently added', () => { + const set = new LruSet(3); + set.add('a'); + set.add('b'); + set.add('c'); + + // Access 'a' via has(), making it the most recently used + expect(set.has('a')).toBe(true); + + // Now 'b' is the least recently used. Adding 'd' should evict 'b'. + set.add('d'); + expect(set.has('b')).toBe(false); // evicted + expect(set.has('a')).toBe(true); // kept (was refreshed) + expect(set.has('c')).toBe(true); + expect(set.has('d')).toBe(true); + }); + + it('refreshes recency on add() of existing item', () => { + const set = new LruSet(3); + set.add('a'); + set.add('b'); + set.add('c'); + + // Re-add 'a', refreshing its recency + set.add('a'); + + // 'b' is now least recent. Adding 'd' should evict 'b'. + set.add('d'); + expect(set.has('b')).toBe(false); // evicted + expect(set.has('a')).toBe(true); + expect(set.size).toBe(3); + }); + + it('does not duplicate on add() of existing item', () => { + const set = new LruSet(5); + set.add(1); + set.add(2); + set.add(1); + set.add(1); + expect(set.size).toBe(2); + }); + + it('clears all entries', () => { + const set = new LruSet(5); + set.add(1); + set.add(2); + set.add(3); + set.clear(); + expect(set.size).toBe(0); + expect(set.has(1)).toBe(false); + expect(set.has(2)).toBe(false); + expect(set.has(3)).toBe(false); + }); + + it('works correctly after clear and re-add', () => { + const set = new LruSet(2); + set.add('a'); + set.add('b'); + set.clear(); + set.add('c'); + set.add('d'); + expect(set.size).toBe(2); + expect(set.has('a')).toBe(false); + expect(set.has('c')).toBe(true); + expect(set.has('d')).toBe(true); + }); + + it('works with maxSize of 1', () => { + const set = new LruSet(1); + set.add(1); + expect(set.has(1)).toBe(true); + set.add(2); + expect(set.has(1)).toBe(false); + expect(set.has(2)).toBe(true); + expect(set.size).toBe(1); + }); + + it('throws on invalid maxSize', () => { + expect(() => new LruSet(0)).toThrow('LruSet maxSize must be at least 1'); + expect(() => new LruSet(-1)).toThrow('LruSet maxSize must be at least 1'); + }); + + it('handles sequential evictions correctly', () => { + const set = new LruSet(3); + // Fill to capacity + for (let i = 0; i < 3; i++) { + set.add(i); + } + // Evict each one in FIFO order (no access refreshes) + for (let i = 3; i < 10; i++) { + set.add(i); + expect(set.size).toBe(3); + expect(set.has(i - 3)).toBe(false); // oldest was evicted + } + }); +}); diff --git a/yarn-project/foundation/src/collection/lru_set.ts b/yarn-project/foundation/src/collection/lru_set.ts new file mode 100644 index 00000000000..09fb0442276 --- /dev/null +++ b/yarn-project/foundation/src/collection/lru_set.ts @@ -0,0 +1,115 @@ +/** Node in a doubly-linked list used by {@link LruSet}. */ +type LruNode = { + value: T; + prev: LruNode | undefined; + next: LruNode | undefined; +}; + +/** + * A bounded set with Least Recently Used (LRU) eviction. + * Both {@link has} and {@link add} count as an access and refresh the entry's + * recency, so items that are actively checked stay in the set longest. + * + * Uses a doubly-linked list for O(1) ordering and a Map for O(1) lookup. + * Head = least recent, tail = most recent. + */ +export class LruSet { + /** Map from value to its linked-list node for O(1) lookup. */ + private readonly map = new Map>(); + private head: LruNode | undefined; + private tail: LruNode | undefined; + + constructor(private readonly maxSize: number) { + if (maxSize < 1) { + throw new Error('LruSet maxSize must be at least 1'); + } + } + + /** Number of entries in the set. */ + get size(): number { + return this.map.size; + } + + /** + * Returns true if the item is in the set. + * Refreshes the item's recency so it becomes the most recently used. + */ + has(item: T): boolean { + const node = this.map.get(item); + if (!node) { + return false; + } + this.moveToTail(node); + return true; + } + + /** + * Adds an item to the set. If the item already exists, refreshes its recency. + * If the set is at capacity, evicts the least recently used item. + */ + add(item: T): void { + const existing = this.map.get(item); + if (existing) { + this.moveToTail(existing); + return; + } + + if (this.map.size >= this.maxSize) { + this.evictHead(); + } + + const node: LruNode = { value: item, prev: this.tail, next: undefined }; + if (this.tail) { + this.tail.next = node; + } else { + this.head = node; + } + this.tail = node; + this.map.set(item, node); + } + + /** Removes all entries from the set. */ + clear(): void { + this.map.clear(); + this.head = undefined; + this.tail = undefined; + } + + /** Unlinks a node from its current position and relinks it at the tail. */ + private moveToTail(node: LruNode): void { + if (node === this.tail) { + return; + } + + // Unlink + if (node.prev) { + node.prev.next = node.next; + } else { + this.head = node.next; + } + if (node.next) { + node.next.prev = node.prev; + } + + // Relink at tail + node.prev = this.tail; + node.next = undefined; + if (this.tail) { + this.tail.next = node; + } + this.tail = node; + } + + /** Evicts the head (least recently used) node. */ + private evictHead(): void { + const oldHead = this.head!; + this.map.delete(oldHead.value); + + this.head = oldHead.next; + if (this.head) { + this.head.prev = undefined; + } else { + this.tail = undefined; + } + } +}