Skip to content

Respect ModuleCache max size#5441

Merged
ndr-ds merged 1 commit intomainfrom
ndr-ds/respect_modulecache_max_size
Feb 16, 2026
Merged

Respect ModuleCache max size#5441
ndr-ds merged 1 commit intomainfrom
ndr-ds/respect_modulecache_max_size

Conversation

@ndr-ds
Copy link
Copy Markdown
Contributor

@ndr-ds ndr-ds commented Feb 13, 2026

Motivation

ModuleCache::insert() never incremented total_size after inserting a new entry. It only decremented during
eviction in reduce_size_to(). This meant:

  • total_size was permanently 0
  • The eviction check if self.total_size + bytecode_size > self.max_size never triggered
  • The cache grew unboundedly despite the 512 MiB limit
  • reduce_size_to() would underflow on the subtraction self.total_size -= bytecode_size

Proposal

  • Add self.total_size += bytecode_size after insertion, only when the key wasn't already present (to avoid
    double-counting on re-insertion of the same bytecode)
  • Add an early return when a single bytecode exceeds max_size, which prevents an underflow in the self.max_size - bytecode_size computation passed to reduce_size_to()
  • Add 4 unit tests covering: size tracking, eviction triggering, oversized rejection, and duplicate key handling

Test Plan

  • CI + new tests pass

Release Plan

  • These changes should be backported to the latest testnet branch, then
    • be released in a validator hotfix.

Copy link
Copy Markdown
Contributor Author

ndr-ds commented Feb 13, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ma2bd ma2bd requested review from Twey February 14, 2026 07:53
@ndr-ds ndr-ds requested review from afck, deuszx and ma2bd February 16, 2026 15:05
@afck afck self-requested a review February 16, 2026 15:28
@ndr-ds ndr-ds force-pushed the ndr-ds/respect_modulecache_max_size branch from c1a015f to ef75161 Compare February 16, 2026 15:49
Comment on lines +76 to +79
if self.modules.contains(&bytecode) {
self.modules.put(bytecode, module);
return;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this equivalent? No need to put if the module is already in the cache?

Suggested change
if self.modules.contains(&bytecode) {
self.modules.put(bytecode, module);
return;
}
if self.modules.promote(&bytecode) {
return;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I need up upgrade the lru crate version for that though. Will do it

@ndr-ds ndr-ds force-pushed the ndr-ds/respect_modulecache_max_size branch from ef75161 to e621bc1 Compare February 16, 2026 17:55
@ndr-ds ndr-ds force-pushed the ndr-ds/respect_modulecache_max_size branch from e621bc1 to a1a216e Compare February 16, 2026 18:11
@ndr-ds ndr-ds added this pull request to the merge queue Feb 16, 2026
github-merge-queue bot pushed a commit that referenced this pull request Feb 16, 2026
## Motivation

`ModuleCache::insert()` never incremented `total_size` after inserting a
new entry. It only decremented during
eviction in `reduce_size_to()`. This meant:

- `total_size` was permanently 0
- The eviction check `if self.total_size + bytecode_size >
self.max_size` never triggered
- The cache grew unboundedly despite the 512 MiB limit
- `reduce_size_to()` would underflow on the subtraction `self.total_size
-= bytecode_size`

## Proposal

- Add `self.total_size += bytecode_size` after insertion, only when the
key wasn't already present (to avoid
double-counting on re-insertion of the same bytecode)
- Add an early return when a single bytecode exceeds `max_size`, which
prevents an underflow in the `self.max_size -
bytecode_size` computation passed to `reduce_size_to()`
- Add 4 unit tests covering: size tracking, eviction triggering,
oversized rejection, and duplicate key handling

## Test Plan

- CI + new tests pass

## Release Plan

- These changes should be backported to the latest `testnet` branch,
then
    - be released in a validator hotfix.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 16, 2026
@ndr-ds ndr-ds added this pull request to the merge queue Feb 16, 2026
Merged via the queue into main with commit c12bf49 Feb 16, 2026
35 checks passed
@ndr-ds ndr-ds deleted the ndr-ds/respect_modulecache_max_size branch February 16, 2026 21:23
ndr-ds added a commit that referenced this pull request Feb 17, 2026
## Motivation

`ModuleCache::insert()` never incremented `total_size` after inserting a
new entry. It only decremented during
eviction in `reduce_size_to()`. This meant:

- `total_size` was permanently 0
- The eviction check `if self.total_size + bytecode_size >
self.max_size` never triggered
- The cache grew unboundedly despite the 512 MiB limit
- `reduce_size_to()` would underflow on the subtraction `self.total_size
-= bytecode_size`

## Proposal

- Add `self.total_size += bytecode_size` after insertion, only when the
key wasn't already present (to avoid
double-counting on re-insertion of the same bytecode)
- Add an early return when a single bytecode exceeds `max_size`, which
prevents an underflow in the `self.max_size -
bytecode_size` computation passed to `reduce_size_to()`
- Add 4 unit tests covering: size tracking, eviction triggering,
oversized rejection, and duplicate key handling

## Test Plan

- CI + new tests pass

## Release Plan

- These changes should be backported to the latest `testnet` branch,
then
    - be released in a validator hotfix.
ndr-ds added a commit that referenced this pull request Feb 17, 2026
## Motivation

`ModuleCache::insert()` never incremented `total_size` after inserting a
new entry. It only decremented during
eviction in `reduce_size_to()`. This meant:

- `total_size` was permanently 0
- The eviction check `if self.total_size + bytecode_size >
self.max_size` never triggered
- The cache grew unboundedly despite the 512 MiB limit
- `reduce_size_to()` would underflow on the subtraction `self.total_size
-= bytecode_size`

## Proposal

- Add `self.total_size += bytecode_size` after insertion, only when the
key wasn't already present (to avoid
double-counting on re-insertion of the same bytecode)
- Add an early return when a single bytecode exceeds `max_size`, which
prevents an underflow in the `self.max_size -
bytecode_size` computation passed to `reduce_size_to()`
- Add 4 unit tests covering: size tracking, eviction triggering,
oversized rejection, and duplicate key handling

## Test Plan

- CI + new tests pass

## Release Plan

- These changes should be backported to the latest `testnet` branch,
then
    - be released in a validator hotfix.
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.

3 participants