Skip to content

test: pin that padMemory rewinds the pointer and makes measurement order immaterial - #194

Merged
thedavidmeister merged 1 commit into
mainfrom
issue-193-padmemory-property
Sep 18, 2026
Merged

thedavidmeister merged 1 commit into
mainfrom
issue-193-padmemory-property

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Closes #193

Problem

padMemory in test/src/lib/LibMemoryKV.gasClaims.t.sol expands memory past what the measurements allocate, then rewinds the free memory pointer. No test failed if it stopped doing either one. P1 (it never rewinds) and P3 (it pads nothing) both survived the gasClaims file.

Fix

Adds testPadMemoryRewindsThePointerAndMeasurementOrderChangesNeither, which checks what the pad's NatSpec promises:

  • Rewind: the free memory pointer after padMemory equals the pointer before it.
  • Order: the test builds a store with every list occupied. After one pad it measures the linear export and then the walk export. After a second pad it measures the walk export and then the linear export. Each export must cost the same whether it is measured first or second.

These two exports are the largest allocation any test in the file makes after one pad: 62 words (0x7c0 bytes). testAnEmptyListCostsTheExportNothing and testExportGasSavingFallsAsListsFill allocate the same amount at 15 lists. testExportGasIsTheSameForEveryList allocates 61 words, and the insert tests allocate less.

Why this is correct

Before the second pad, memory is already expanded through everything the first pair touched. If the first pair paid for any memory expansion, the second pair pays less for the same allocation, so the gas differs. The cause does not matter: no pad, a pad too small, or a pad left below the allocations because the pointer was never rewound all show up the same way. The check therefore does not rely on the pad being the right size. It fails whenever the pad does not cover the largest allocation, including a pad of size 0. A check against the pad's own size would pass with that size set to 0. Both measurements come from the same build, and the test contains no absolute gas figures.

msize would read memory expansion directly. solc rejects it while the Yul optimizer is on (error 6553), so the test measures gas instead.

QA

  • Discriminating tests: testPadMemoryRewindsThePointerAndMeasurementOrderChangesNeither. It passes on the real padMemory and fails on each broken padMemory below (mutation-probe at 49b6a9e, with the suite set to this test alone and a baseline of green (1 passed)). On main, P1 and P3 survive the whole gasClaims file (the [A23] [LOW] Tests hand-roll free-pointer, cheatcode and approx-assert primitives the dependencies provide #126 probe cited in the issue).
  • Mutations applied:
    • P1 setFreePointer(pointer); → (pointer); (never rewinds) → KILLED by testPadMemoryRewindsThePointerAndMeasurementOrderChangesNeither
    • P3 new bytes(0x1000) → new bytes(0) (pads nothing) → KILLED by testPadMemoryRewindsThePointerAndMeasurementOrderChangesNeither
    • P4 new bytes(0x1000) → new bytes(0x780) (one word short of the 0x7c0-byte allocation) → KILLED by testPadMemoryRewindsThePointerAndMeasurementOrderChangesNeither
    • Control C1 new bytes(0x1000) → new bytes(0x7a0) (the pad reaches exactly 0x7c0) → SURVIVED, as expected. Together with P4, this puts the test's boundary at the largest allocation.
    • Result: 3/3 killed, 0 no-run, 0 harness errors, exit 0.
  • Oracle: the pointer before padMemory is the reference for the rewind. For each export, the reference is the same export measured second after a pad, when the first pair has already expanded memory over the same region, so it pays no expansion. Neither reference depends on the pad size.
  • Category check: the issue asks for (a) the free pointer back where it started, covered by the rewind assert, and (b) memory expanded by at least the pad, so that a later allocation inside the pad pays no expansion and matches a second allocation of the same size. (b) is covered by the first-vs-second gas equality over the largest allocation, which is the [A41] [INFO] gasClaims NatSpec says measuring the linear walk first is 'pessimistic'; padMemory makes order irrelevant #144 both-orders template the issue cites. The full suite passes at 49b6a9e: 27 suites, 161 tests, 0 failed.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Added coverage verifying that memory padding restores the free-memory pointer.
    • Confirmed gas measurements remain consistent regardless of whether linear or mask-walk exports are measured first in a fully occupied store.

…der immaterial

No test failed if padMemory stopped padding or stopped rewinding the free
memory pointer. The new test checks the pointer is back where it started
after padMemory, and that the linear and walk exports of a store with every
list occupied, the most any test in the file allocates after one pad, each
cost the same measured first after a pad as measured second.

Closes #193

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Sep 18, 2026
@coderabbitai

coderabbitai Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: be7ad4f3-db82-459e-bdd6-f6436bbfae3d

📥 Commits

Reviewing files that changed from the base of the PR and between 05dd781 and 49b6a9e.

📒 Files selected for processing (1)
  • test/src/lib/LibMemoryKV.gasClaims.t.sol

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The gas-claim suite adds a test that verifies padMemory rewinds the free-memory pointer and that linear and mask-walk export measurements remain unchanged when their measurement order changes.

Changes

Memory padding gas-claim validation

Layer / File(s) Summary
Add memory padding gas-claim test
test/src/lib/LibMemoryKV.gasClaims.t.sol
The new test builds a store that occupies every list, checks the free-memory pointer after padMemory, and compares linear and mask-walk export gas measurements in both orders.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 49b6a

No concrete merge-blocking risk remains in this test-only change.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the added test and its two purposes: verifying that padMemory rewinds the pointer and that measurement order does not affect results.
Linked Issues check ✅ Passed Issue #193 requires an automated gas-claims test for pointer rewind, sufficient memory padding, and measurement-order independence. The added `testPadMemoryRewindsThePointerAndMeasurementOrderChangesN…
Out of Scope Changes check ✅ Passed The reviewed change adds only the test described for issue #193 in test/src/lib/LibMemoryKV.gasClaims.t.sol. The test setup and assertions directly validate the linked issue requirements. No unrelat…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@thedavidmeister
thedavidmeister merged commit 28e6eed into main Sep 18, 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.

No gasClaims test fails if padMemory stops padding or stops rewinding the free pointer

1 participant