Skip to content

test: mutation-harden LibMemoryKV overflow boundary coverage - #17

Merged
thedavidmeister merged 1 commit into
mainfrom
2026-06-15-memkv-coverage
Jun 15, 2026
Merged

test: mutation-harden LibMemoryKV overflow boundary coverage#17
thedavidmeister merged 1 commit into
mainfrom
2026-06-15-memkv-coverage

Conversation

@thedavidmeister

Copy link
Copy Markdown
Contributor

Group hardened: MemoryKVOverflow boundary (max valid 0xFFFF pointer acceptance)

Scoped adversarial-mutation pass over the sole module src/lib/LibMemoryKV.sol
(get / set / toBytes32Array). Tests-only — git diff src/ is empty, no
source or bytecode change.

The mutation sweep found the suite to be very strong: every mutation across slot
spread, pointer stride, key/value placement, length tracking, allocation, and the
entire toBytes32Array bisect tree was already killed. The one weak spot was
the overflow boundary in set:

if (pointer > 0xFFFF) {
    revert MemoryKVOverflow(pointer);
}

testSetOverflow only drove the free-memory pointer to 0x10000 and asserted a
revert — the over-boundary case. Nothing asserted that an insert landing on the
maximum valid 16-bit pointer 0xFFFF is accepted, so off-by-one mutations of
the boundary survived undetected. This is exactly the line issue
#13 (16→32-bit pointer
rewrite) will touch; pinning the boundary now protects that future change. This PR
does not implement #13.

Mutation matrix (behavior → mutation → killer)

Behavior Mutation Pre-PR Killer
overflow boundary lower edge pointer > 0xFFFF>= 0xFFFF SURVIVED testSetPointerBoundaryMaxAccepted (new)
overflow boundary lower edge pointer > 0xFFFF> 0xFFFE SURVIVED testSetPointerBoundaryMaxAccepted / ...BelowMaxAccepted (new)
overflow boundary upper edge revert payload 0x10000 killed testSetOverflow + new ...OverflowReverts (exact payload)
get: slot spread mod(...,15)*0x10 *0x10*0x20, 1516 killed existing
get: pointer mask 0xFFFF walk stride 0x400x60 killed existing
get: key-compare slot mload(ptr) mload(ptr+0x20) killed existing
get: value read ptr+0x20 ptr+0x40 killed existing
get: exists := 1 0 killed existing
set: slot spread mod(...,15) 1516 (get/set sync) killed existing
set: walk stride 0x40 0x60 killed existing
set: update value slot ptr+0x20 ptr+0x40 killed existing
set: insert key/value placement swap key↔value killed existing
set: insert next-ptr startPointer 0 (chain break) killed existing
set: length +2 +1 killed existing
set: alloc bump 0x60 0x40 killed existing
set: length/pointer write masks not(shl(...,0xFFFF))not(0) killed existing
set: bitOffset write shl(bitOffset,ptr) shl(0,ptr) killed existing
toBytes32Array: length shr 0xf0 shr 0xe0 killed existing
toBytes32Array: copyFromPtr stride/cursor/key/value offset mutations killed existing
toBytes32Array: bisect leaves (all 15) force-skip a leaf killed testSaturate
toBytes32Array: bisect masks (mask128, shr 0x90) narrow/shift killed existing

New tests

  • testSetPointerBoundaryMaxAccepted — insert with free-mem-ptr at exactly
    0xFFFF must NOT revert; asserts the pointer 0xFFFF is encoded into a kv
    slot and length is exactly 2.
  • testSetPointerBoundaryBelowMaxAccepted — same for 0xFFFE, guarding the
    boundary from below.
  • testSetPointerBoundaryOverflowReverts0x10000 reverts with the exact
    MemoryKVOverflow(0x10000) payload.

(The accepted-case tests deliberately assert the kv pointer encoding and
non-revert rather than reading the value back: a node placed at 0xFFFF lives in
low memory and may be clobbered, so the encoding/non-revert are the safe oracles.)

Verification

  • forge build: clean (Compiler run successful!; only pre-existing boolean-cst
    lint warnings from the source bisect tree / slow ref impl, unrelated to this PR).
  • forge fmt --check: clean.
  • Full suite: 28 passed, 0 failed (was 25; +3).
  • Each new accepted-case test confirmed to FAIL under both surviving mutations and
    PASS on the restored baseline; source restored (git diff src/ empty).

Remaining gaps checklist

  • No standalone test pins MemoryKVOverflow for the update path (updates
    never allocate, so they cannot overflow — currently a non-issue, but worth a
    note if support larger pointers #13 changes allocation).
  • Boundary accepted-case asserts pointer encoding, not a full get
    round-trip at 0xFFFF (intentional — 0xFFFF low memory is clobberable).
  • get/set hash-sync is covered only indirectly (both mod(...,15)
    mutations are killed); no dedicated test asserting identical slot derivation.
  • The 15-slot bisect leaves are killed only by the statistical testSaturate
    fuzz (which deterministically fills all slots via re-hashing); no
    per-leaf deterministic unit test.

🤖 Generated with Claude Code

Mutation testing of src/lib/LibMemoryKV.sol surfaced two surviving
mutants on the MemoryKVOverflow boundary check (`pointer > 0xFFFF`):

  - `>` -> `>=`            : SURVIVED
  - `0xFFFF` -> `0xFFFE`   : SURVIVED

The existing `testSetOverflow` only exercises the over-boundary case
(an insert forced to `0x10000` reverts). Nothing asserted that an
insert landing on the MAXIMUM valid 16-bit pointer (`0xFFFF`) is
ACCEPTED, so both off-by-one mutations of the boundary went unkilled.

Adds three tests-only boundary cases (no source/bytecode change):
  - max pointer 0xFFFF accepted (no revert; exact pointer encoded in kv)
  - 0xFFFE accepted (guards the boundary from below)
  - 0x10000 reverts with the exact overflowing pointer payload

All other get/set/toBytes32Array behaviors (slot spread, pointer stride,
key/value placement, length tracking, bisect leaves, masks) were already
killed by the existing suite. This hardening also protects the future
16->32-bit pointer rewrite (issue #13).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Jun 15, 2026
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@thedavidmeister, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 2 minutes and 50 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 54ceb144-8b3b-4dcf-828a-4c49d851c961

📥 Commits

Reviewing files that changed from the base of the PR and between 0c2239a and 3b22a94.

📒 Files selected for processing (1)
  • test/src/lib/LibMemoryKV.getset.t.sol
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 2026-06-15-memkv-coverage

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@thedavidmeister

Copy link
Copy Markdown
Contributor Author

Reviewed 3b22a94: mutation-validated tests-only coverage for the LibMemoryKV max 0xFFFF pointer-overflow boundary (was untested; >/>= and >0xFFFE mutants survived); source byte-identical. LGTM.

@thedavidmeister
thedavidmeister merged commit 0cfd907 into main Jun 15, 2026
4 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