Repository navigation
Model tests: the layout tiles the word, an update writes no handle bit, storage parity checks get and export - #169
Closed
thedavidmeister wants to merge 8 commits into
Closed
thedavidmeister wants to merge 8 commits into
thedavidmeister wants to merge 8 commits into
Conversation
The types test compares the library's layout constants, which #162 adds to src, against the test tree's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The layout test checks the library's LIST_COUNT, SLOT_BITS, POINTER_MASK, COUNT_BIT_OFFSET and NODE_BYTES against the test tree's, and that the count and fifteen head slots tile the 256-bit word (#123). The two zero-word tests could not fail: any shift or mask of zero is zero. They are replaced by a test that fills all fifteen lists and checks the count and that each head slot points at its own list's node (#137). The count and heads are read through lengthOf/headOf (#124, #149), and the exports are checked exactly once with countPair (#149). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The asymmetry test snapshots every handle word before the update and asserts each is identical after it, including the handle set returns (#107). The header states what the file pins, with no mutation framing (#121, #138). buildSaturatedExternal checks that the decoded keys end at or below SATURATED_BASE before it rewinds the free pointer, and a test pins the boundary on both sides (#134). The layout constants come from the test helpers and the pointer is positioned with setFreePointer (#123, #126). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One assertion helper serves both multi-set tests: every key set reads back through get with storage's value, is exported exactly once with it, and the export is two words per distinct key (#115, #136). Each store has its own storage oracle, both stores are checked only once both are built, and the PAIRS_PER_STORE cap is documented (#115). The fuzz struct NatSpec calls it a key/value pair; testSingleGetSet names its local got and drops the no-op wrap/unwrap (#146, #158). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
origin/main carries #162, so this branch no longer carries it as its own merge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The test tree names no layout value, so the types, crossFunction and storageParity tests read LibMemoryKV's constants directly. The constants test keeps only what the constants must satisfy together: the count sits above the last list and is the top slot, and POINTER_MASK is one slot wide as both the widest pointer and the widest count. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 57 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
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. Comment |
The helpers file is split into files named for what they hold; the owned tests import from LibMemoryKVExport, LibMemoryKVHandle, LibMemoryKVKeys, LibMemoryKVAssert and LibFreeMemory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings in 02043b3, the export gas uniformity tolerance that fixes testExportGasIsUniformAcrossSlots (564 != 563). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #163 (
test-lib-support). Merges last in wave 2.Closes #107
Closes #121
Closes #134
Closes #137
Closes #115
Closes #136
Closes #146
Closes #158
Closes #123
Closes #124
Closes #126
Closes #138
Closes #149
#123, #124, #126, #138 and #149 are split across W2b–W2e. This PR is their closer and merges after those.
Base
This branch merges
origin/main(8220918), so it no longer carries #162 itself: #162 is on main. It also mergestest-lib-supportat 02043b3, which contains #162, so the diff shows only this PR's three files.Imports follow the split helpers:
LibMemoryKVExport,LibMemoryKVHandle,LibMemoryKVKeys,LibMemoryKVAssert,LibFreeMemory.Owned files:
test/src/lib/LibMemoryKV.types.t.sol,LibMemoryKV.crossFunction.t.sol,LibMemoryKV.storageParity.t.sol. No other file is edited.Changes
types (
LibMemoryKV.types.t.sol)testLayoutConstantsTileTheWordchecks,on
LibMemoryKV's own constants, that the count sits directly above the last list(
COUNT_BIT_OFFSET == LIST_COUNT * SLOT_BITS), is the top slot (COUNT_BIT_OFFSET + SLOT_BITS == 256), thatPOINTER_MASKis one slot wide, and that the count slot holds exactlyPOINTER_MASK. The localSLOTSis gone.testCountAndHeadPointersTileTheWord. It fills all fifteenlists, checks the count is two words per pair, and checks each head pointer is non-zero and points at the node
holding that list's key, with its value after it.
lengthOf/headOf;countPair(...) != 0becomesassertEq(countPair(...), 1), so each pair is exported exactly once.crossFunction (
LibMemoryKV.crossFunction.t.sol)it, the handle
setreturns included. The NatSpec states it: an insert writes the count and the head, an updatewrites no bit.
buildSaturatedExternalrequires the free memory pointer after decodingkeysto be at or belowSATURATED_BASE("keys overlap SATURATED_BASE") before it moves the pointer there withsetFreePointer([A23] [LOW] Tests hand-roll free-pointer, cheatcode and approx-assert primitives the dependencies provide #126,A23).
testSaturatedBuilderRefusesKeysReachingPastItsBasepins both sides: 19 keys build (count 38), 20 revert withthat exact string.
LIST_COUNT/SLOT_BITS/NODE_SIZEand the literal0xf0are replaced byLibMemoryKV'sconstants.
storageParity (
LibMemoryKV.storageParity.t.sol)assertMatchesStorage, checks export length == 2 × distinct keys, and for everykey set:
getanswers with the value storage holds, and the pair is exported exactly once. Each store has its ownstorage oracle (
StorageStore{values, seen, distinct}), written bysetBoth.PAIRS_PER_STORE = 10is documented: uncapped arrays push nodes pastPOINTER_MASKandsetrevertsMemoryKVOverflow(seen asMemoryKVOverflow(65568)without the cap).get#158 (A55): struct NatSpec, localgotinstead ofget, the no-op wrap/unwrap dropped.Deviations from the plan
half remains, stated on src's constants, renamed
testLayoutConstantsTileTheWord.testSaturatedBuilderRefusesKeysReachingPastItsBaseis added to pin the A31 guard in both directions.testMultiGetSetSingle→testSetSequenceMatchesStorage,testMultiGetSetDouble→testTwoLiveStoresEachMatchTheirOwnStorage.countPair(...) != 0→ exactly once (A46), beyond the plan's types bullet.testSetSequenceMatchesStoragegas rises from μ 2.77M to μ ~10M: it now callsgetandcountPairper key.Test counts
Owned files: 11 before (base 70a7ab3) → 12 after (types 6, crossFunction 3, storageParity 3). The one new test is
testSaturatedBuilderRefusesKeysReachingPastItsBase; the two zero-word tests become two layout tests.Full suite, from the CI
testjob logs: 159 passed of 159 on basetest-lib-supportat 02043b3(https://github.com/rainlanguage/rain.lib.memkv/actions/runs/35339867369) → 160 passed of 160 at head 90c3daa
(https://github.com/rainlanguage/rain.lib.memkv/actions/runs/35340530745).
QA
All probes:
mutation-probefrom rainlanguage/adversarial-mutation-test, base 70a7ab3 (the owned files before thisPR) vs head 83b25c2. The final head 90c3daa differs from 83b25c2 only in
LibMemoryKV.gasClaims.t.sol, fromthe base merge, which no probe below runs. Raw reports:
/home/gildlab/artifacts/memkv/audit-2026-09-18/w2f/.testAsymmetryHoldsAcrossManyLiveHandles, M01 (set's update also re-points the list head at the matchednode): base SURVIVED, head KILLED.
getfinds a key only at the head of its list): base SURVIVED, head KILLED bytestSetSequenceMatchesStorageandtestTwoLiveStoresEachMatchTheirOwnStorage. M13 (export reads list 1 in placeof list 0) and M14 (an update also counts two words) are KILLED on both, so the exactly-once and export-length
assertions are not claimed as new coverage.
testEmptyStoreHasNoWordCount,testEmptyStoreHasNoHeadPointerInAnySlot): M02(
LIST_COUNT14), M03 (SLOT_BITS0x11), M05 (COUNT_BIT_OFFSET0xe0), M07 (insert never writes the new head)all SURVIVED. Head
testCountAndHeadPointersTileTheWord: M03, M05, M07 KILLED; M02 SURVIVED. The test fillsLibMemoryKV.LIST_COUNTlists andkeyForSlothashes modulo the same constant, so a smaller count moves the testwith it.
testLayoutConstantsTileTheWordkills M02 (COUNT_BIT_OFFSET0xf0 ≠ 14 × 0x10).testLayoutConstantsTileTheWord: M02, M03, M04 (POINTER_MASK0x7FFF), M05 KILLED; M06 (NODE_BYTES0x80)SURVIVED, since node size is not a tiling fact.
testSaturatedBuilderRefusesKeysReachingPastItsBaseis new: M10 (guard<= SATURATED_BASE→<= type(uint256).max) and M11 (<=→<) both KILLED.--match-path test/src/lib/LibMemoryKV.{types,crossFunction,storageParity}.t.sol):carry the before/after evidence.
only because it spaced nodes with its own
NODE_SIZE = 0x60. That copy is gone, so the test readsLibMemoryKV.NODE_BYTESand moves with it. At head,LibMemoryKV.setCapacity.t.solKILLS M06 (for exampletestSet682PairsFitAnOtherwiseEmptyFrame).MemoryKVNatSpec (fifteen 16-bit head slots, the count in the top slot,POINTER_MASKthe widest pointer and count). The tiling test's expected node contents are the keys and values itinserted. crossFunction's expected handle word is built from the addresses the building frame was forced to allocate
at. The asymmetry test's expected words are the same handles snapshotted before the update. The guard's
fitcomesfrom the ABI layout of a decoded
bytes32[](entry pointer0x80, a length word, one word per key). storageParity'soracle is one Solidity storage mapping plus a distinct-key counter per store.
tests replaced; [A12] [LOW] storageParity testMultiGetSetDouble never checks two live stores against each other #115
get, exactly-once export and export length; [A33] [INFO] testMultiGetSetSingle never calls get and only checks exported pairs against storage #136 one oracle per store; [A43] [INFO] storageParity test KV struct NatSpec calls a key/value pair a "get/set pair" #146 struct NatSpec;[A55] [INFO] storageParity test compares against a no-op wrap/unwrap and names a local after
get#158 localgot, no-op wrap dropped; [A20] [LOW] MemoryKV handle layout is spelled as literals and per-file constants, not one definition #123 local layout constants and0xf0replaced by src's, and the tiling test(Amendment 4 drops the equality half); [A21] [LOW] Word count read under three names: wordCount alias, lengthOf, and inline >> 0xf0 #124
lengthOf/headOf; [A23] [LOW] Tests hand-roll free-pointer, cheatcode and approx-assert primitives the dependencies provide #126setFreePointer; [A35] [INFO] NatSpec narrates design rationale, replaced figures and killed mutants, not current behaviour #138 present-tense header;[A46] [INFO] Tests re-inline lengthOf/headOf/countPair/key/node-crafting instead of using test helpers #149 exactly-once
countPair. Each is this PR's owned-file part; the other files are W2b–W2e's.🤖 Generated with Claude Code