Merged
Conversation
Security fixes: - Fix seed search loop to include seed 255 (was `<`, now `<=`) - Add AuthoringMetaTooLarge error for >256 word lists - Fix buildStructureSlow reference impl for zero signed contexts - Add natspec documenting namespace qualification requirement on EvalV4 - Fix stale IInterpreterStoreV2 reference to IInterpreterStoreV3 - Remove unused LibUint256Array import from LibContext Documentation: - Add natspec to OperandV2, StackItem, EvalV4, eval4, PragmaV1 - Add natspec to IParserV2/parse2 and IParserPragmaV1/parsePragma1 - Document bytecodeToSources prerequisites and return values - Document checkNoOOBPointers implicit monotonicity enforcement - Document lookupWord well-formedness precondition - Fix CLAUDE.md (NixOS -> Nix, add error/pragma/deprecated policies) - Fix README.md (Install nix develop -> Install Nix) Code quality: - Replace no-op (hashed); pattern with idiomatic (uint256 shifted,) Test coverage: - Add bytecodeToSources tests (5 concrete + 1 fuzz) - Add DuplicateFingerprint revert test - Add parseMetaConstantString tests (4 concrete + 1 fuzz) - Add findBestExpander tests (empty, single, large, invariant, reference) - Add LibGenParseMetaSlow reference implementation - Add InvalidSignature revert tests (4 cases) - Add lookupWord tests (4 concrete + 2 fuzz) - Add AuthoringMetaTooLarge boundary tests (concrete + fuzz) - Add zero-signed-contexts reference comparison tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lookupWord did not verify that the word's bit was set in the bloom filter expansion before comparing fingerprints. A 0 bit means no word was mapped to that position, so there is nothing to collide with — return (false, 0) immediately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fingerprint 0x000000 is the empty-slot sentinel in buildParseMetaV2. If a word hashed to fingerprint 0, its slot would appear empty and could be silently overwritten by a later word. Force fingerprint 0 to 1 in wordBitmapped. This introduces a small bias on fingerprint 1 (2 in 2^24 instead of 1 in 2^24) which is negligible. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
buildParseMetaV2 panicked with a generic index-out-of-bounds when the word set could not be compressed within maxDepth bloom layers. Added explicit MaxDepthExceeded custom error with the maxDepth value for debuggability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
META_ITEM_MASK was (1 << 4) - 1 = 0xF (4-bit) but should be a full 4-byte mask. Changed to type(uint32).max. No runtime corruption existed because memory was zero-initialized, but the mask was technically wrong. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Added a build-time validation function that checks parse meta byte length matches what the depth and expansion data imply. This avoids per-lookup bounds checks while still allowing callers to verify meta integrity once after construction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- wordBitmapped: document non-zero fingerprint guarantee - buildParseMetaV2: document bit-set check in lookup algorithm, structural validation, and error conditions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
WalkthroughThis PR introduces comprehensive audit documentation across five passes (process, security, test coverage, documentation, code quality), adds new type definitions and struct fields to interpreter interfaces, enhances library implementations with validation and error-handling mechanisms, and significantly expands test coverage with new test contracts and edge-case validations. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@audit/2026-03-01-01/pass0/process.md`:
- Line 8: The MD022 violations are due to missing blank lines immediately after
level-3 headings; open CLAUDE.md and after every line that starts with "###"
(specifically the headings at lines cited in the review) insert a single blank
line following the heading so there is an empty line between the "### ..."
header and the following paragraph or content, then save and re-run markdownlint
to confirm the MD022 heading-spacing errors are resolved.
In `@audit/2026-03-01-01/pass1/Interfaces.md`:
- Around line 8-28: Add required blank lines around the Markdown headings to
satisfy MD022: ensure there is an empty line above and below each heading such
as "Evidence of Thorough Reading", "1. `src/interface/IInterpreterV4.sol` (102
lines)", "2. `src/interface/IInterpreterStoreV3.sol` (66 lines)", and "3.
`src/interface/IInterpreterCallerV4.sol` (48 lines) so that there is a blank
line separating headings from surrounding content; update Interfaces.md to
insert those blank lines where markdownlint flagged Line 8, 14, 17, 20, 24 and
28.
In `@audit/2026-03-01-01/pass1/LibBytecode.md`:
- Around line 130-132: Add blank lines before and after each fenced code block
and include a language identifier "solidity" on the opening fences for the
assembly snippets shown (the line containing "offset :=
and(mload(add(add(bytecode, 3), mul(sourceIndex, 2))), 0xFFFF)" and the legacy
conversion loop lines "mstore8(add(cursor, 1), byte(0, mload(cursor)))" /
"mstore8(cursor, 0)"); update the markdown so both blocks have a preceding blank
line, a following blank line, and open with ```solidity to satisfy MD031/MD040
lint rules.
In `@audit/2026-03-01-01/pass1/LibContext.md`:
- Around line 29-37: Update the audit evidence section to match the current PR:
remove the stale LibUint256Array import from the Imports list and change the
referenced source for SignedContextV1 from IInterpreterCallerV2.sol to the V4
interface that is actually used (IInterpreterCallerV4), and ensure the remaining
listed symbols (LibHashNoAlloc, HASH_NIL, SignatureChecker, MessageHashUtils and
the SignedContextV1 offset constants) reflect the correct current lines/names in
the V4 interface so the import/type provenance in LibContext.md matches the
reviewed revision.
- Around line 12-19: The functions table in LibContext.md needs blank lines
before and after it to satisfy markdownlint MD058; insert an empty line
immediately above the table start and an empty line immediately after the table
end so the surrounding paragraphs/section headings (which reference functions
like base(), hash(SignedContextV1 memory), hash(SignedContextV1[] memory), and
build(bytes32[][] memory, SignedContextV1[] memory)) are separated from the
table.
In `@audit/2026-03-01-01/pass1/LibGenParseMeta.md`:
- Around line 39-50: The audit report still lists A04-1 (seed-255 omission) and
A04-2 (>255 opcode-index corruption) as active even though the code now uses the
corrected loop condition (seed <= type(uint8).max / or seed < 256) and includes
the AuthoringMetaTooLarge fix; update the report to mark these findings as
resolved (or explicitly state that the document is a pre-fix snapshot) and apply
the same resolution/status change to the other occurrences referenced (sections
around lines 53-76 and 149-151) so the report no longer reports fixed issues as
active.
- Line 5: The audit metadata contains a machine-local absolute path; replace the
absolute path string found in LibGenParseMeta.md (the entry referencing
/Users/thedavidmeister/Code/rain.interpreter.interface/src/lib/codegen/LibGenParseMeta.sol)
with a repository-relative path such as src/lib/codegen/LibGenParseMeta.sol,
updating any identical occurrences in the file so metadata uses only
repo-relative paths and does not leak workstation-specific details.
In `@audit/2026-03-01-01/pass1/LibParseMeta.md`:
- Around line 21-26: The Evidence section is out of date: update the inventory
in LibParseMeta.md to reflect the current source by adding the new symbol
entries for the function checkParseMetaStructure and the custom error
InvalidParseMeta (or explicitly mark this file as a pre-fix snapshot). Modify
the summary list that currently only mentions wordBitmapped and lookupWord to
include checkParseMetaStructure and the InvalidParseMeta error, and/or add a
short note that the document is a snapshot so readers know it does not reflect
the latest changes.
In `@audit/2026-03-01-01/pass2/LibParseMeta.md`:
- Around line 12-25: Add the missing blank lines around the markdown tables and
fenced blocks so markdownlint rules MD058 and MD031 stop firing: insert a blank
line before and after the table near the `wordBitmapped(uint256 seed, bytes32
word)` / `lookupWord(bytes memory meta, bytes32 word)` section and likewise add
surrounding blank lines for the tables/fenced blocks located around the
`findBestExpander(AuthoringMetaV2[] memory metas)`,
`buildParseMetaV2(AuthoringMetaV2[] memory authoringMeta, uint8 maxDepth)`, and
`parseMetaConstantString(Vm vm, bytes memory authoringMetaBytes, uint8
buildDepth)` sections (also apply the same fix for the table around the `60-66`
range). Ensure there is exactly one empty line above and below each table or
fenced block.
In `@audit/2026-03-01-01/pass4/CodeQuality.md`:
- Around line 121-165: Update the audit report to mark A02 and A04 as resolved
(or add a remediation timestamp/scope) because the PR already removes the unused
import/using in LibContext.sol (remove the import of LibUint256Array and the
using LibUint256Array for uint256[] line) and replaces the no-op discard pattern
in LibGenParseMeta.sol by omitting the unused variable in destructuring (change
"(uint256 shifted, uint256 hashed) = LibParseMeta.wordBitmapped(...);" to
"(uint256 shifted, ) = LibParseMeta.wordBitmapped(...);" in the occurrences that
were fixed); ensure the report summary reflects these changes and that remaining
findings (like A03) remain unchanged if still relevant.
In `@src/lib/caller/LibContext.sol`:
- Around line 163-166: The return-shape doc for LibContext.build is inaccurate
because it states a signers column always exists; update the comment (the block
describing the return matrix alongside references to LibContext.base() and
baseContext) to explicitly state that the signers column is included only when
signedContexts.length > 0 and that when there are no signed contexts the matrix
omits the signers column and only contains the base and any baseContext columns;
keep references to the base() function and the signed context data columns so
readers can locate the behavior in build.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (33)
.gitignoreCLAUDE.mdREADME.mdaudit/2026-03-01-01/pass0/process.mdaudit/2026-03-01-01/pass1/Interfaces.mdaudit/2026-03-01-01/pass1/LibBytecode.mdaudit/2026-03-01-01/pass1/LibContext.mdaudit/2026-03-01-01/pass1/LibGenParseMeta.mdaudit/2026-03-01-01/pass1/LibParseMeta.mdaudit/2026-03-01-01/pass1/SmallFiles.mdaudit/2026-03-01-01/pass2/LibBytecode.mdaudit/2026-03-01-01/pass2/LibContext.mdaudit/2026-03-01-01/pass2/LibParseMeta.mdaudit/2026-03-01-01/pass3/Interfaces.mdaudit/2026-03-01-01/pass3/Libraries.mdaudit/2026-03-01-01/pass4/CodeQuality.mdaudit/2026-03-01-01/triage.mdsrc/interface/IInterpreterCallerV4.solsrc/interface/IInterpreterV4.solsrc/interface/IParserPragmaV1.solsrc/interface/IParserV2.solsrc/lib/bytecode/LibBytecode.solsrc/lib/caller/LibContext.solsrc/lib/codegen/LibGenParseMeta.solsrc/lib/parse/LibParseMeta.soltest/src/lib/bytecode/LibBytecode.bytecodeToSources.t.soltest/src/lib/caller/LibContext.t.soltest/src/lib/caller/LibContextSlow.soltest/src/lib/codegen/LibGenParseMeta.buildMeta.t.soltest/src/lib/codegen/LibGenParseMeta.findExpander.t.soltest/src/lib/codegen/LibGenParseMetaSlow.soltest/src/lib/parse/LibParseMeta.lookupWord.t.soltest/src/lib/parse/LibParseMeta.wordBitmapped.t.sol
|
|
||
| ## Evidence of Thorough Reading | ||
|
|
||
| ### CLAUDE.md (73 lines) |
There was a problem hiding this comment.
Fix markdownlint MD022 heading-spacing violations.
At Line 8, Line 14, Line 17, Line 20, Line 24, and Line 28, add a blank line after each ### heading to satisfy the configured markdownlint rule.
Also applies to: 14-14, 17-17, 20-20, 24-24, 28-28
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 8-8: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@audit/2026-03-01-01/pass0/process.md` at line 8, The MD022 violations are due
to missing blank lines immediately after level-3 headings; open CLAUDE.md and
after every line that starts with "###" (specifically the headings at lines
cited in the review) insert a single blank line following the heading so there
is an empty line between the "### ..." header and the following paragraph or
content, then save and re-run markdownlint to confirm the MD022 heading-spacing
errors are resolved.
Comment on lines
+8
to
+28
|
|
||
| ## Evidence of Thorough Reading | ||
|
|
||
| ### 1. `src/interface/IInterpreterV4.sol` (102 lines) | ||
|
|
||
| - **Imports:** `FullyQualifiedNamespace`, `StateNamespace`, `SourceIndexV2`, `DEFAULT_STATE_NAMESPACE`, `OPCODE_CONSTANT`, `OPCODE_CONTEXT`, `OPCODE_EXTERN`, `OPCODE_UNKNOWN`, `OPCODE_STACK` from deprecated v2/IInterpreterV3.sol; `IInterpreterStoreV3` (lines 5-37) | ||
| - **Type `OperandV2`:** `bytes32` user-defined value type (line 39) | ||
| - **Type `StackItem`:** `bytes32` user-defined value type (line 41) | ||
| - **Struct `EvalV4`:** fields `store` (IInterpreterStoreV3), `namespace` (FullyQualifiedNamespace), `bytecode` (bytes), `sourceIndex` (SourceIndexV2), `context` (bytes32[][]), `inputs` (StackItem[]), `stateOverlay` (bytes32[]) (lines 43-51) | ||
| - **Interface `IInterpreterV4`:** single function `eval4(EvalV4 calldata) external view returns (StackItem[] calldata, bytes32[] calldata)` (lines 89-102) | ||
|
|
||
| ### 2. `src/interface/IInterpreterStoreV3.sol` (66 lines) | ||
|
|
||
| - **Imports:** `StateNamespace`, `FullyQualifiedNamespace`, `NO_STORE` from deprecated v2/IInterpreterStoreV2.sol (line 7) | ||
| - **Interface `IInterpreterStoreV3`:** (lines 31-66) | ||
| - **Event `Set`:** `(FullyQualifiedNamespace namespace, bytes32 key, bytes32 value)` (line 36) | ||
| - **Function `set`:** `(StateNamespace namespace, bytes32[] calldata kvs) external` (line 48) | ||
| - **Function `get`:** `(FullyQualifiedNamespace namespace, bytes32 key) external view returns (bytes32)` (line 65) | ||
|
|
||
| ### 3. `src/interface/IInterpreterCallerV4.sol` (48 lines) | ||
|
|
There was a problem hiding this comment.
Fix markdown heading spacing to satisfy MD022.
markdownlint warnings at Line 8, Line 14, Line 17, Line 20, Line 24, and Line 28 indicate missing blank lines around headings.
🛠️ Proposed markdownlint fix
---
## Evidence of Thorough Reading
### 1. `src/interface/IInterpreterV4.sol` (102 lines)
- **Imports:** `FullyQualifiedNamespace`, `StateNamespace`, `SourceIndexV2`, `DEFAULT_STATE_NAMESPACE`, `OPCODE_CONSTANT`, `OPCODE_CONTEXT`, `OPCODE_EXTERN`, `OPCODE_UNKNOWN`, `OPCODE_STACK` from deprecated v2/IInterpreterV3.sol; `IInterpreterStoreV3` (lines 5-37)
@@
- **Interface `IInterpreterV4`:** single function `eval4(EvalV4 calldata) external view returns (StackItem[] calldata, bytes32[] calldata)` (lines 89-102)
### 2. `src/interface/IInterpreterStoreV3.sol` (66 lines)
@@
- **Function `get`:** `(FullyQualifiedNamespace namespace, bytes32 key) external view returns (bytes32)` (line 65)
### 3. `src/interface/IInterpreterCallerV4.sol` (48 lines)
@@
- **Event `ContextV2`:** `(address sender, bytes32[][] context)` (line 47)
### 4. `src/interface/IInterpreterExternV4.sol` (47 lines)
@@
- **Function `extern`:** `(ExternDispatchV2 dispatch, StackItem[] calldata inputs) external view returns (StackItem[] calldata outputs)` (lines 43-46)
### 5. `src/interface/IParserV2.sol` (11 lines)🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 8-8: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 24-24: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 28-28: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@audit/2026-03-01-01/pass1/Interfaces.md` around lines 8 - 28, Add required
blank lines around the Markdown headings to satisfy MD022: ensure there is an
empty line above and below each heading such as "Evidence of Thorough Reading",
"1. `src/interface/IInterpreterV4.sol` (102 lines)", "2.
`src/interface/IInterpreterStoreV3.sol` (66 lines)", and "3.
`src/interface/IInterpreterCallerV4.sol` (48 lines) so that there is a blank
line separating headings from surrounding content; update Interfaces.md to
insert those blank lines where markdownlint flagged Line 8, 14, 17, 20, 24 and
28.
Comment on lines
+130
to
+132
| ``` | ||
| offset := and(mload(add(add(bytecode, 3), mul(sourceIndex, 2))), 0xFFFF) | ||
| ``` |
There was a problem hiding this comment.
Fix fenced code block lint violations (MD031/MD040).
The code fences need blank lines around them and language identifiers to satisfy configured markdownlint rules.
🛠️ Suggested markdown fix
-The assembly reads a 2-byte offset via:
-```
+The assembly reads a 2-byte offset via:
+
+```solidity
offset := and(mload(add(add(bytecode, 3), mul(sourceIndex, 2))), 0xFFFF)@@
-In the legacy conversion loop:
-solidity +In the legacy conversion loop: + +solidity
mstore8(add(cursor, 1), byte(0, mload(cursor)))
mstore8(cursor, 0)
Also applies to: 150-153
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 130-130: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 130-130: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 132-132: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@audit/2026-03-01-01/pass1/LibBytecode.md` around lines 130 - 132, Add blank
lines before and after each fenced code block and include a language identifier
"solidity" on the opening fences for the assembly snippets shown (the line
containing "offset := and(mload(add(add(bytecode, 3), mul(sourceIndex, 2))),
0xFFFF)" and the legacy conversion loop lines "mstore8(add(cursor, 1), byte(0,
mload(cursor)))" / "mstore8(cursor, 0)"); update the markdown so both blocks
have a preceding blank line, a following blank line, and open with ```solidity
to satisfy MD031/MD040 lint rules.
Comment on lines
+12
to
+19
| **Functions:** | ||
| | Function | Line | Visibility | | ||
| |----------|------|------------| | ||
| | `base()` | 63 | internal view | | ||
| | `hash(SignedContextV1 memory)` | 79 | internal pure | | ||
| | `hash(SignedContextV1[] memory)` | 108 | internal pure | | ||
| | `build(bytes32[][] memory, SignedContextV1[] memory)` | 166 | internal view | | ||
|
|
There was a problem hiding this comment.
Add blank lines around the functions table to satisfy markdownlint (MD058).
Line 13 starts the table without a separating blank line, and the section resumes immediately after the table. This will keep failing markdownlint in strict mode.
Suggested fix
**Functions:**
+
| Function | Line | Visibility |
|----------|------|------------|
| `base()` | 63 | internal view |
| `hash(SignedContextV1 memory)` | 79 | internal pure |
| `hash(SignedContextV1[] memory)` | 108 | internal pure |
| `build(bytes32[][] memory, SignedContextV1[] memory)` | 166 | internal view |
+
**Error:**📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| **Functions:** | |
| | Function | Line | Visibility | | |
| |----------|------|------------| | |
| | `base()` | 63 | internal view | | |
| | `hash(SignedContextV1 memory)` | 79 | internal pure | | |
| | `hash(SignedContextV1[] memory)` | 108 | internal pure | | |
| | `build(bytes32[][] memory, SignedContextV1[] memory)` | 166 | internal view | | |
| **Functions:** | |
| | Function | Line | Visibility | | |
| |----------|------|------------| | |
| | `base()` | 63 | internal view | | |
| | `hash(SignedContextV1 memory)` | 79 | internal pure | | |
| | `hash(SignedContextV1[] memory)` | 108 | internal pure | | |
| | `build(bytes32[][] memory, SignedContextV1[] memory)` | 166 | internal view | | |
| **Error:** |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 13-13: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@audit/2026-03-01-01/pass1/LibContext.md` around lines 12 - 19, The functions
table in LibContext.md needs blank lines before and after it to satisfy
markdownlint MD058; insert an empty line immediately above the table start and
an empty line immediately after the table end so the surrounding
paragraphs/section headings (which reference functions like base(),
hash(SignedContextV1 memory), hash(SignedContextV1[] memory), and
build(bytes32[][] memory, SignedContextV1[] memory)) are separated from the
table.
Comment on lines
+29
to
+37
| **Imports:** | ||
| - `LibUint256Array` from rain.solmem (line 5) | ||
| - `LibHashNoAlloc`, `HASH_NIL` from rain.lib.hash (line 6) | ||
| - `SignatureChecker` from OpenZeppelin (line 8) | ||
| - `MessageHashUtils` from OpenZeppelin (line 9) | ||
| - `SignedContextV1`, offset constants from IInterpreterCallerV4 (lines 11-16) | ||
|
|
||
| **Struct `SignedContextV1` (defined in IInterpreterCallerV2.sol line 62):** | ||
| - `address signer` (offset 0x00) |
There was a problem hiding this comment.
Evidence inventory is stale for imports/type source.
Line 30 still lists LibUint256Array and Line 36 references IInterpreterCallerV2.sol, but this PR context indicates the LibUint256Array import was removed and SignedContextV1 is tracked via V4 interfaces. The audit evidence section should match the reviewed revision.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@audit/2026-03-01-01/pass1/LibContext.md` around lines 29 - 37, Update the
audit evidence section to match the current PR: remove the stale LibUint256Array
import from the Imports list and change the referenced source for
SignedContextV1 from IInterpreterCallerV2.sol to the V4 interface that is
actually used (IInterpreterCallerV4), and ensure the remaining listed symbols
(LibHashNoAlloc, HASH_NIL, SignatureChecker, MessageHashUtils and the
SignedContextV1 offset constants) reflect the correct current lines/names in the
V4 interface so the import/type provenance in LibContext.md matches the reviewed
revision.
Comment on lines
+39
to
+50
| ### A04-1 | LOW | Seed search loop skips seed value 255 | ||
|
|
||
| **Location:** Line 57 | ||
|
|
||
| ```solidity | ||
| for (uint256 seed = 0; seed < type(uint8).max; seed++) { | ||
| ``` | ||
|
|
||
| `type(uint8).max` is 255, so the loop condition `seed < 255` means seed values 0 through 254 are tested, and seed 255 is never tried. The full search space of a `uint8` seed should include all 256 values (0-255). This means the function may miss an optimal seed that only exists at value 255. While this is code-generation tooling and the practical impact is marginal (finding a slightly less optimal bloom filter), it is a logic error. | ||
|
|
||
| **Recommendation:** Change to `seed <= type(uint8).max` or `seed < 256`. Note that `seed` is declared as `uint256` so there is no overflow concern with the `<=` form. | ||
|
|
There was a problem hiding this comment.
A04-1 and A04-2 are documented as active despite code-level remediation in this PR.
These sections still present the seed-255 omission and >255 opcode-index corruption as open findings, but the current source includes seed <= type(uint8).max and AuthoringMetaTooLarge. Please mark them as resolved (or explicitly label this report as pre-fix snapshot) to avoid security-status drift.
Also applies to: 53-76, 149-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@audit/2026-03-01-01/pass1/LibGenParseMeta.md` around lines 39 - 50, The audit
report still lists A04-1 (seed-255 omission) and A04-2 (>255 opcode-index
corruption) as active even though the code now uses the corrected loop condition
(seed <= type(uint8).max / or seed < 256) and includes the AuthoringMetaTooLarge
fix; update the report to mark these findings as resolved (or explicitly state
that the document is a pre-fix snapshot) and apply the same resolution/status
change to the other occurrences referenced (sections around lines 53-76 and
149-151) so the report no longer reports fixed issues as active.
Comment on lines
+21
to
+26
| **Functions defined:** | ||
| - `wordBitmapped(uint256 seed, bytes32 word)` (line 45) -- returns `(uint256 bitmap, uint256 hashed)`. Hashes word+seed and extracts a single-bit bitmap (from high byte) and fingerprint (from low 3 bytes). | ||
| - `lookupWord(bytes memory meta, bytes32 word)` (line 67) -- returns `(bool, uint256)`. Walks bloom filter expansions, computes position via ctpop, and compares fingerprints. | ||
|
|
||
| **Types/Errors defined:** None in this file. (No custom errors are defined, which is relevant to a finding below.) | ||
|
|
There was a problem hiding this comment.
Evidence section no longer matches the current LibParseMeta.sol surface.
Lines 21-26 state there are only two functions and no custom errors, but this PR’s source includes checkParseMetaStructure and InvalidParseMeta. Please refresh the inventory (or label as a pre-fix snapshot) to preserve audit traceability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@audit/2026-03-01-01/pass1/LibParseMeta.md` around lines 21 - 26, The Evidence
section is out of date: update the inventory in LibParseMeta.md to reflect the
current source by adding the new symbol entries for the function
checkParseMetaStructure and the custom error InvalidParseMeta (or explicitly
mark this file as a pre-fix snapshot). Modify the summary list that currently
only mentions wordBitmapped and lookupWord to include checkParseMetaStructure
and the InvalidParseMeta error, and/or add a short note that the document is a
snapshot so readers know it does not reflect the latest changes.
Comment on lines
+12
to
+25
| | Function | Line | Visibility | | ||
| |---|---|---| | ||
| | `wordBitmapped(uint256 seed, bytes32 word)` | 45 | internal pure | | ||
| | `lookupWord(bytes memory meta, bytes32 word)` | 67 | internal pure | | ||
|
|
||
| ### `src/lib/codegen/LibGenParseMeta.sol` | ||
|
|
||
| Library: `LibGenParseMeta` | ||
|
|
||
| | Function | Line | Visibility | | ||
| |---|---|---| | ||
| | `findBestExpander(AuthoringMetaV2[] memory metas)` | 49 | internal pure | | ||
| | `buildParseMetaV2(AuthoringMetaV2[] memory authoringMeta, uint8 maxDepth)` | 126 | internal pure | | ||
| | `parseMetaConstantString(Vm vm, bytes memory authoringMetaBytes, uint8 buildDepth)` | 241 | internal pure | |
There was a problem hiding this comment.
Fix markdownlint spacing issues (MD058, MD031).
Tables and fenced blocks in these ranges are missing required surrounding blank lines. This will trigger markdownlint warnings in CI.
Also applies to: 60-66
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 15-15: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 22-22: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@audit/2026-03-01-01/pass2/LibParseMeta.md` around lines 12 - 25, Add the
missing blank lines around the markdown tables and fenced blocks so markdownlint
rules MD058 and MD031 stop firing: insert a blank line before and after the
table near the `wordBitmapped(uint256 seed, bytes32 word)` / `lookupWord(bytes
memory meta, bytes32 word)` section and likewise add surrounding blank lines for
the tables/fenced blocks located around the `findBestExpander(AuthoringMetaV2[]
memory metas)`, `buildParseMetaV2(AuthoringMetaV2[] memory authoringMeta, uint8
maxDepth)`, and `parseMetaConstantString(Vm vm, bytes memory authoringMetaBytes,
uint8 buildDepth)` sections (also apply the same fix for the table around the
`60-66` range). Ensure there is exactly one empty line above and below each
table or fenced block.
Comment on lines
+121
to
+165
| ### A02 -- LOW: Unused Import and `using` Statement in `LibContext.sol` | ||
|
|
||
| **File:** `src/lib/caller/LibContext.sol`, lines 5 and 46 | ||
|
|
||
| ```solidity | ||
| import {LibUint256Array} from "rain.solmem/lib/LibUint256Array.sol"; | ||
| ... | ||
| using LibUint256Array for uint256[]; | ||
| ``` | ||
|
|
||
| `LibUint256Array` is imported and a `using` directive is declared for `uint256[]`, but no `uint256[]` variable in the library ever calls any method provided by `LibUint256Array`. All arrays in `LibContext` are typed as `bytes32[]` or `bytes32[][]`. The only reference to `uint256[]` beyond the `using` statement is in a comment on line 142. | ||
|
|
||
| This is dead code that should be removed for clarity. | ||
|
|
||
| --- | ||
|
|
||
| ### A03 -- LOW: Stale Interface Version Reference in `IInterpreterCallerV4` NatSpec | ||
|
|
||
| **File:** `src/interface/IInterpreterCallerV4.sol`, line 38 | ||
|
|
||
| ```solidity | ||
| /// - OPTIONALLY set state on the associated `IInterpreterStoreV2`. | ||
| ``` | ||
|
|
||
| The NatSpec comment references `IInterpreterStoreV2`, but `IInterpreterCallerV4` is designed to work with `IInterpreterStoreV3` (as shown in `EvaluableV4` defined in the same file on line 27, which uses `IInterpreterStoreV3`). This is a documentation-only issue but could mislead implementors. | ||
|
|
||
| --- | ||
|
|
||
| ### A04 -- LOW: Discarded Return Values via No-Op Expressions in `LibGenParseMeta.sol` | ||
|
|
||
| **File:** `src/lib/codegen/LibGenParseMeta.sol`, lines 61 and 89 | ||
|
|
||
| ```solidity | ||
| (uint256 shifted, uint256 hashed) = LibParseMeta.wordBitmapped(seed, metas[i].word); | ||
| (hashed); | ||
| ``` | ||
|
|
||
| The `hashed` return value is destructured but then immediately discarded via the no-op expression `(hashed);`. While this pattern suppresses unused variable warnings, it is unconventional and reduces readability. The more idiomatic Solidity pattern is to omit the unused variable name in the destructuring: | ||
|
|
||
| ```solidity | ||
| (uint256 shifted, ) = LibParseMeta.wordBitmapped(seed, metas[i].word); | ||
| ``` | ||
|
|
||
| This pattern appears twice (lines 60-61 and 88-89). | ||
|
|
There was a problem hiding this comment.
Resolved findings are still listed as open in the report summary.
A02 and A04 are presented as active issues, but the current PR state already removes the unused LibUint256Array usage and replaces the no-op discard pattern. Please mark these as resolved or timestamp/scope this report as pre-remediation.
Also applies to: 205-208
🧰 Tools
🪛 LanguageTool
[grammar] ~145-~145: Ensure spelling is correct
Context: ...umentation-only issue but could mislead implementors. --- ### A04 -- LOW: Discarded Return...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@audit/2026-03-01-01/pass4/CodeQuality.md` around lines 121 - 165, Update the
audit report to mark A02 and A04 as resolved (or add a remediation
timestamp/scope) because the PR already removes the unused import/using in
LibContext.sol (remove the import of LibUint256Array and the using
LibUint256Array for uint256[] line) and replaces the no-op discard pattern in
LibGenParseMeta.sol by omitting the unused variable in destructuring (change
"(uint256 shifted, uint256 hashed) = LibParseMeta.wordBitmapped(...);" to
"(uint256 shifted, ) = LibParseMeta.wordBitmapped(...);" in the occurrences that
were fixed); ensure the report summary reflects these changes and that remaining
findings (like A03) remain unchanged if still relevant.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Added slither-disable-next-line for intentionally unused return values in findBestExpander. Applied forge fmt formatting fixes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The signers column is only present when signedContexts.length > 0. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@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:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
|
Tip For best results, initiate chat on the files or code changes. SIZE=L |
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
1 participant
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.
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Tests