Skip to content

Bump rain-lib-memkv to 0.2.0: fingerprint the stateKV contents, not the handle word - #586

Merged
thedavidmeister merged 5 commits into
mainfrom
issue-585-memkv-0.2.0
Sep 19, 2026
Merged

thedavidmeister merged 5 commits into
mainfrom
issue-585-memkv-0.2.0

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Closes #585

What

  • Pins rain-lib-memkv 0.2.0 (tag sol-v0.2.0 on rainlanguage/rain.lib.memkv) in foundry.toml, soldeer.lock and remappings.txt, and moves the versioned import prefix rain-lib-memkv-0.1.5/ to rain-lib-memkv-0.2.0/ in the 17 files that import it.
  • test/lib/state/LibInterpreterStateFingerprint.sol: the fingerprint hashes the pairs stateKV holds instead of its handle word.

Why

On 0.2.0 a non-empty MemoryKV is the address of a fixed header, so the handle word stops changing after the first insert. The old fingerprint was keccak256(abi.encode(state)), which includes that word, so an insert into a non-empty stateKV left it unchanged. Updates were already invisible to it on 0.1.5. OpTest.opReferenceCheckActual and LibOpStack.t.sol use this fingerprint to assert an op leaves the state alone.

How

fingerprint sets state.stateKV to MEMORY_KV_EMPTY, hashes abi.encode(state, stateKVPairHashes(stateKV)), and restores the handle before returning. Every other field still goes through abi.encode(state), so a field added to InterpreterState is still covered. stateKVPairHashes is the keccak256 of each key/value pair, sorted with OpenZeppelin Arrays.sort, because toBytes32Array documents its pair order as unspecified. Inserts and updates move the fingerprint. Stores holding the same pairs fingerprint the same wherever they sit in memory.

No src logic changes. Every src use of stateKV assigns the result of set back to state.stateKV (LibOpSet, the LibOpGet miss cache, the BaseRainlangInterpreter.eval4 overlay, LibEval's export, the init in LibInterpreterStateDataContract). Nothing copies, branches or restores a handle, so 0.2.0's shared-store copies change nothing here.

Gas figures (issue step 3): nothing to update in this repo. rainlang has no committed gas figures. .gas-snapshot left with the deploy half in 93f2825, and none of the tests assert a gas number.

Out of scope (follow-ups, not in this PR)

  • The Rainterpreter runtime bytecode changes (BaseRainlangInterpreter, LibEval, LibOpGet, LibOpSet compile against the new LibMemoryKV), so the release needs a new deploy through rainlang.deploy.
  • raindex, rain.erc4626.words, rain.flare, rain.merkle and rain.pyth pin rain-lib-memkv next to rainlang and need both bumped together.

QA

  • Discriminating tests (test/src/lib/state/LibInterpreterState.fingerprint.t.sol, all fuzzed):
    • testFingerprintChangesOnInsertIntoNonEmptyStateKV is the test the issue asks for. With 0.2.0 installed and main's helper (keccak256(abi.encode(state))) restored, it FAILS on the first fuzz input (runs: 0). On this branch it passes 2048 runs.
    • testFingerprintChangesOnStateKVUpdate FAILS on main's helper and passes here.
    • testFingerprintSameForSamePairsInSeparateStores builds two stores from the same 32 pairs inserted in opposite orders. It first asserts that their toBytes32Array exports differ, so the sort is exercised, and then that their fingerprints are equal. It FAILS on main's helper (the handles differ) and passes here.
    • testFingerprintDiffersForDifferentKeys and testFingerprintLeavesStateKVHandle test the new mechanism: the key is part of each pair hash, and the handle is restored. The mutation pass below shows each one kills a mutant that nothing else kills.
    • Run on main's helper: 3 passed, 3 failed (the insert, update and separate-stores tests). Run on this branch: 7 passed, 0 failed.
  • Mutations applied: 8 mutants of LibInterpreterStateFingerprint.sol, each applied with sed. After each one I ran the fingerprint suite, checked it ran 7 tests, restored the file and checked the tree was clean. 8 killed, 0 survived:
    • no clear of the handle before encoding: killed by SameForSamePairsInSeparateStores
    • no restore of the handle: killed by LeavesStateKVHandle
    • no sort: killed by SameForSamePairsInSeparateStores
    • pair hash of the key only: killed by ChangesOnStateKVUpdate
    • pair hash of the value only: killed by DiffersForDifferentKeys
    • pair hashes dropped from the encoding: killed by 3 tests, including ChangesOnInsertIntoNonEmptyStateKV
    • pair count set to the word count: killed by 5 tests
    • last pair skipped: killed by 3 tests
  • Oracle: the NatSpec of LibMemoryKV in rain.lib.memkv 0.2.0. It says a non-empty handle is a header address that every copy shares, and that toBytes32Array's pair order "is unspecified and MUST NOT be relied upon; a caller that needs a canonical form MUST sort". The issue's own probe, the reference clone's test/scratch/FingerprintProbe.t.sol, fails at "fingerprint must move on an insert into a non-empty stateKV" for the same reason the first test above does.
  • Category check against the issue's four steps:
    • Step 1: pin, lock, remappings and all 17 versioned imports moved. I deleted the stale rain-lib-memkv-0.1.5/ remapping that soldeer update leaves behind, so no 0.1.5 path can resolve.
    • Step 2: done, covering both inserts and updates, and made independent of pair order and handle address.
    • Step 3: this repo has no gas figures to move. See "Gas figures" above.
    • Step 4: out of scope, listed above.
    • The other behaviour change in 0.2.0 is that copies of a handle share one store. I checked every src use of stateKV (git grep): each assigns set's result back to state.stateKV, and nothing copies or restores a handle. The test-side handle comparisons (LibOpGet.t.sol, LibOpSet.t.sol, LibInterpreterStateDataContract.t.sol) compare against the empty handle 0, which 0.2.0 keeps.
  • Static gates, run locally in github:rainlanguage/rainix/8657b83b68f41957ab85da91132c3f652c1f32c0#sol-shell, the shell CI uses: pre-commit run --all-files and rainix-sol-single-contract both exit 0. forge lint -D warnings exits 1 on the same 21 boolean-cst findings as unmodified main. None of them is in a file this PR touches, and Scope boolean-cst lint disables on the tuple-return bool literals #588 fixes them.
  • Suite: CI is the full suite. Locally I ran only the fingerprint suite. Logs and the mutation script are in /home/gildlab/artifacts/rainlang/issue-585/.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Improvements

    • State fingerprints now consistently reflect stored key-value contents, regardless of storage location or insertion order.
    • Fingerprints change when values are added, updated, or replaced.
  • Maintenance

    • Updated the underlying memory key-value library to version 0.2.0.
    • Added clarifying code comments and lint guidance without changing runtime behavior.
    • Expanded validation around state fingerprinting and key-value storage scenarios.

thedavidmeister and others added 2 commits September 18, 2026 21:12
rain-lib-memkv 0.2.0 makes a non-empty MemoryKV handle the address of a
fixed header, so the handle word no longer changes on an insert. The
test fingerprint hashed abi.encode(state), which includes that word, so an
insert into a non-empty stateKV left the fingerprint unchanged (updates
were already invisible to it).

The fingerprint now hashes every other field of the state as before and
replaces the stateKV handle with the sorted keccak256 of each pair the
store holds. Inserts and updates move it, and stores holding the same
pairs fingerprint the same regardless of address or export order.

Closes #585

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: rainlanguage/rainlang/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 257e0c1c-21fd-456b-a402-482ddc72b789

📥 Commits

Reviewing files that changed from the base of the PR and between 5874169 and 864cf10.

⛔ Files ignored due to path filters (1)
  • soldeer.lock is excluded by !**/*.lock
📒 Files selected for processing (31)
  • foundry.toml
  • remappings.txt
  • src/abstract/BaseRainlangInterpreter.sol
  • src/abstract/BaseRainlangSubParser.sol
  • src/concrete/extern/RainlangReferenceExtern.sol
  • src/lib/eval/LibEval.sol
  • src/lib/op/store/LibOpGet.sol
  • src/lib/op/store/LibOpSet.sol
  • src/lib/parse/LibSubParse.sol
  • src/lib/parse/literal/LibParseLiteral.sol
  • src/lib/state/LibInterpreterState.sol
  • src/lib/state/LibInterpreterStateDataContract.sol
  • test/abstract/OpTest.sol
  • test/lib/state/LibInterpreterStateFingerprint.sol
  • test/src/abstract/HappyPathLiteralSubParser.sol
  • test/src/abstract/MismatchedLiteralSubParser.sol
  • test/src/concrete/MockExternBadLiteralIndex.sol
  • test/src/concrete/RainlangStore.t.sol
  • test/src/lib/eval/LibEval.fBounds.t.sol
  • test/src/lib/eval/LibEval.inputsLengthMismatch.t.sol
  • test/src/lib/eval/LibEval.maxOutputs.t.sol
  • test/src/lib/eval/LibEval.remainderOnly.t.sol
  • test/src/lib/op/logic/LibOpAny.t.sol
  • test/src/lib/op/store/LibOpGet.t.sol
  • test/src/lib/op/store/LibOpSet.t.sol
  • test/src/lib/parse/BadLengthSubParser.sol
  • test/src/lib/parse/ConstantReturningSubParser.sol
  • test/src/lib/parse/ContextReturningSubParser.sol
  • test/src/lib/parse/MultiConstantSubParser.sol
  • test/src/lib/state/LibInterpreterState.fingerprint.t.sol
  • test/src/lib/state/LibInterpreterStateDataContract.t.sol

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


📝 Walkthrough

Walkthrough

The project upgrades rain-lib-memkv from 0.1.5 to 0.2.0. State fingerprints now include sorted MemoryKV key/value pair hashes. Parser returns gain comments and targeted lint suppressions.

Changes

MemoryKV upgrade and fingerprint behavior

Layer / File(s) Summary
MemoryKV dependency and imports
foundry.toml, remappings.txt, src/..., test/...
Configuration, source imports, and test imports now reference rain-lib-memkv 0.2.0.
State fingerprint calculation
test/lib/state/LibInterpreterStateFingerprint.sol
Fingerprinting temporarily uses MEMORY_KV_EMPTY, hashes sorted key/value pair hashes, and restores the original stateKV handle.
Fingerprint tests and parser lint annotations
test/src/lib/state/LibInterpreterState.fingerprint.t.sol, src/abstract/*, src/lib/parse/*, test/src/lib/parse/*
Tests cover inserts, updates, pair ordering, handle preservation, and different keys. Intentional constant boolean returns receive explanatory comments and boolean-cst suppressions. One test sub-parser now returns at the end of each function.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant InterpreterState
  participant LibInterpreterStateFingerprint
  participant MemoryKV
  participant Arrays
  InterpreterState->>LibInterpreterStateFingerprint: fingerprint(state)
  LibInterpreterStateFingerprint->>MemoryKV: toBytes32Array()
  LibInterpreterStateFingerprint->>LibInterpreterStateFingerprint: hash key/value pairs
  LibInterpreterStateFingerprint->>Arrays: sort pair hashes
  LibInterpreterStateFingerprint->>LibInterpreterStateFingerprint: hash state and pair hashes
  LibInterpreterStateFingerprint-->>InterpreterState: restore stateKV and return fingerprint
Loading

Merge Risk: ⚪ Minimal · up to b3be7

The dependency upgrade is internally consistent and introduces no concrete merge-blocking risk.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning Issue #585 core implementation is present: the PR updates the visible dependency references, hashes sorted stateKV pair values, restores the handle, and adds fingerprint tests. The required `soldeer… Update soldeer.lock to pin rain-lib-memkv 0.2.0. Apply all gas figure changes required by the dependency update. Coordinate the new Rainterpreter deployment bytecode and the required downstream dependency bumps for #585.
Out of Scope Changes check ⚠️ Warning The boolean-cst comment and lint-suppression changes in parser files and parser test doubles do not change rain-lib-memkv, stateKV fingerprinting, gas figures, deployment bytecode, or downstream d… Remove the unrelated boolean-cst lint cleanup and associated return-statement edits, or provide a direct coding dependency that makes each change necessary for #585.
✅ Passed checks (3 passed)
Check name Status Explanation
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…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes both primary changes: upgrading rain-lib-memkv to 0.2.0 and fingerprinting stateKV contents instead of the handle word. It is specific and concise.
Full details: Linked Issues check

Explanation

Issue #585 core implementation is present: the PR updates the visible dependency references, hashes sorted stateKV pair values, restores the handle, and adds fingerprint tests. The required soldeer.lock change cannot be verified because that file is excluded from review. The PR summary reports no gas updates, although #585 identifies changed gas figures and a 107375 deployment-gas reduction. The summary also states that Rainterpreter deployment bytecode and downstream dependency bumps remain out of scope, although #585 requires them.

Full details: Out of Scope Changes check

Explanation

The boolean-cst comment and lint-suppression changes in parser files and parser test doubles do not change rain-lib-memkv, stateKV fingerprinting, gas figures, deployment bytecode, or downstream dependencies. The BadLengthSubParser return-statement edits have the same lint-only purpose. The summary identifies these as existing lint findings, so no connection to #585 is established.

  • Fix all pre-merge checks with AI
✨ 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.

The rainix static job now runs `forge lint -D warnings`, which flags 21
`return (true|false, ...)` sites on unmodified main as boolean-cst. Each
literal is the bool the function returns, not a condition operand, so
each gets a scoped disable with that reason. Comments only: no code
changes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rals"

This reverts commit 864cf10.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@thedavidmeister
thedavidmeister merged commit b33708c into main Sep 19, 2026
5 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment:

S/M/L PR Classification Guidelines:

This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed.

Small (S)

Characteristics:

  • Simple bug fixes, typos, or minor refactoring
  • Single-purpose changes affecting 1-2 files
  • Documentation updates
  • Configuration tweaks
  • Changes that require minimal context to review

Review Effort: Would have taken 5-10 minutes

Examples:

  • Fix typo in variable name
  • Update README with new instructions
  • Adjust configuration values
  • Simple one-line bug fixes
  • Import statement cleanup

Medium (M)

Characteristics:

  • Feature additions or enhancements
  • Refactoring that touches multiple files but maintains existing behavior
  • Breaking changes with backward compatibility
  • Changes requiring some domain knowledge to review

Review Effort: Would have taken 15-30 minutes

Examples:

  • Add new feature or component
  • Refactor common utility functions
  • Update dependencies with minor breaking changes
  • Add new component with tests
  • Performance optimizations
  • More complex bug fixes

Large (L)

Characteristics:

  • Major feature implementations
  • Breaking changes or API redesigns
  • Complex refactoring across multiple modules
  • New architectural patterns or significant design changes
  • Changes requiring deep context and multiple review rounds

Review Effort: Would have taken 45+ minutes

Examples:

  • Complete new feature with frontend/backend changes
  • Protocol upgrades or breaking changes
  • Major architectural refactoring
  • Framework or technology upgrades

Additional Factors to Consider

When deciding between sizes, also consider:

  • Test coverage impact: More comprehensive test changes lean toward larger classification
  • Risk level: Changes to critical systems bump up a size category
  • Team familiarity: Novel patterns or technologies increase complexity

Notes:

  • the assessment must be for the totality of the PR, that means comparing the base branch to the last commit of the PR
  • the assessment output must be exactly one of: S, M or L (single-line comment) in format of: SIZE={S/M/L}
  • do not include any additional text, only the size classification
  • your assessment comment must not include tips or additional sections
  • do NOT tag me or anyone else on your comment

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.

Bump rain-lib-memkv to 0.2.0: fingerprint the stateKV contents, not the handle word

1 participant