From 760e681d8151d1220e77a34ea2770d2bb6248606 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sun, 1 Mar 2026 20:41:12 +0400 Subject: [PATCH 01/13] audit: fix all findings from 2026-03-01-01 audit 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 --- .gitignore | 3 +- CLAUDE.md | 76 +++++ README.md | 2 +- audit/2026-03-01-01/pass0/process.md | 52 +++ audit/2026-03-01-01/pass1/Interfaces.md | 184 +++++++++++ audit/2026-03-01-01/pass1/LibBytecode.md | 170 ++++++++++ audit/2026-03-01-01/pass1/LibContext.md | 196 ++++++++++++ audit/2026-03-01-01/pass1/LibGenParseMeta.md | 156 +++++++++ audit/2026-03-01-01/pass1/LibParseMeta.md | 156 +++++++++ audit/2026-03-01-01/pass1/SmallFiles.md | 144 +++++++++ audit/2026-03-01-01/pass2/LibBytecode.md | 126 ++++++++ audit/2026-03-01-01/pass2/LibContext.md | 106 +++++++ audit/2026-03-01-01/pass2/LibParseMeta.md | 141 ++++++++ audit/2026-03-01-01/pass3/Interfaces.md | 300 ++++++++++++++++++ audit/2026-03-01-01/pass3/Libraries.md | 209 ++++++++++++ audit/2026-03-01-01/pass4/CodeQuality.md | 209 ++++++++++++ audit/2026-03-01-01/triage.md | 65 ++++ src/interface/IInterpreterCallerV4.sol | 2 +- src/interface/IInterpreterV4.sol | 24 ++ src/interface/IParserPragmaV1.sol | 9 + src/interface/IParserV2.sol | 6 + src/lib/bytecode/LibBytecode.sol | 6 + src/lib/caller/LibContext.sol | 7 +- src/lib/codegen/LibGenParseMeta.sol | 17 +- src/lib/parse/LibParseMeta.sol | 9 +- .../LibBytecode.bytecodeToSources.t.sol | 72 +++++ test/src/lib/caller/LibContext.t.sol | 126 +++++++- test/src/lib/caller/LibContextSlow.sol | 23 +- .../codegen/LibGenParseMeta.buildMeta.t.sol | 134 +++++++- .../LibGenParseMeta.findExpander.t.sol | 58 ++++ test/src/lib/codegen/LibGenParseMetaSlow.sol | 36 +++ .../lib/parse/LibParseMeta.lookupWord.t.sol | 122 +++++++ 32 files changed, 2919 insertions(+), 27 deletions(-) create mode 100644 CLAUDE.md create mode 100644 audit/2026-03-01-01/pass0/process.md create mode 100644 audit/2026-03-01-01/pass1/Interfaces.md create mode 100644 audit/2026-03-01-01/pass1/LibBytecode.md create mode 100644 audit/2026-03-01-01/pass1/LibContext.md create mode 100644 audit/2026-03-01-01/pass1/LibGenParseMeta.md create mode 100644 audit/2026-03-01-01/pass1/LibParseMeta.md create mode 100644 audit/2026-03-01-01/pass1/SmallFiles.md create mode 100644 audit/2026-03-01-01/pass2/LibBytecode.md create mode 100644 audit/2026-03-01-01/pass2/LibContext.md create mode 100644 audit/2026-03-01-01/pass2/LibParseMeta.md create mode 100644 audit/2026-03-01-01/pass3/Interfaces.md create mode 100644 audit/2026-03-01-01/pass3/Libraries.md create mode 100644 audit/2026-03-01-01/pass4/CodeQuality.md create mode 100644 audit/2026-03-01-01/triage.md create mode 100644 test/src/lib/bytecode/LibBytecode.bytecodeToSources.t.sol create mode 100644 test/src/lib/codegen/LibGenParseMetaSlow.sol create mode 100644 test/src/lib/parse/LibParseMeta.lookupWord.t.sol diff --git a/.gitignore b/.gitignore index 668aefc..7b33f59 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ cache -out \ No newline at end of file +out +.fixes \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..328a4bd --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,76 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Project Overview + +Solidity interfaces for the Rainlang interpreter and utility libraries for implementing them. Part of the Rain Protocol ecosystem for onchain interpreted compute. + +License: DecentraLicense 1.0 (DCL-1.0). REUSE 3.2 compliant — all files need SPDX headers and copyright notices. + +## Build & Development + +Requires the Nix package manager. **Only use the nix version of Foundry**, not a system-installed one. + +```bash +nix develop # Enter dev shell +rainix-sol-prelude # Setup (run before first build/test) +rainix-sol-test # Run all tests +rainix-sol-static # Static analysis (Slither) +rainix-sol-legal # License/REUSE compliance check +``` + +Compiler: Solidity 0.8.25, EVM target: cancun, optimizer enabled (1M runs). Fuzz tests run 2048 iterations. + +All reverts use custom errors — no `revert("string")` or `require()` with string messages. + +Interfaces use `pragma solidity ^0.8.18` for downstream compatibility; libraries and errors use `^0.8.25`. + +## Architecture + +### Core Interfaces (`src/interface/`) + +The current interface set (all in `src/interface/`): + +- **IInterpreterV4** — Evaluates Rainlang bytecode. Stateless: returns stack results and state writes without persisting anything itself. +- **IInterpreterStoreV3** — Key-value state storage with namespace isolation per caller. `set()` for bulk writes, `get()` for reads. +- **IInterpreterCallerV4** — Defines `EvaluableV4` struct (interpreter + store + bytecode). Contracts that call the interpreter implement this. +- **IParserV2** — Converts Rainlang source text to bytecode (`parse2()`). +- **ISubParserV4** — Extension point for custom literals and words in parsers. +- **IInterpreterExternV4** — External function dispatch with integrity checking. +- **IParserPragmaV1** — Pragma support (e.g. `usingWordsFrom`). + +Deprecated v1/v2 interfaces live in `src/interface/deprecated/`. Deprecated interfaces should not be modified unless undeprecating (moving back to `src/interface/`). + +### Libraries (`src/lib/`) + +- **LibBytecode** (`bytecode/`) — Parse and validate Rainlang bytecode structure (source counts, offsets, stack allocation, OOB checks). +- **LibContext** (`caller/`) — Build execution context arrays with signature verification (uses OZ SignatureChecker). Base context = `[msg.sender, calling_contract]`. +- **LibEvaluable** (`caller/`) — Hash utility for `EvaluableV4` structs. +- **LibNamespace** (`ns/`) — Qualifies state namespaces by hashing with sender address for caller isolation. +- **LibParseMeta** (`parse/`) — Bloom filter + fingerprint-based word lookup for parser metadata. +- **LibGenParseMeta** (`codegen/`) — Code generation for optimized parse metadata constants. + +### Key Types + +- `StackItem` — `bytes32`, the interpreter's stack value type +- `OperandV2` — `bytes32`, opcode operands +- `StateNamespace` / `FullyQualifiedNamespace` — `bytes32`, state isolation +- `SourceIndexV2` — `bytes32`, index into bytecode sources +- `EvaluableV4` — struct containing interpreter address, store address, and bytecode + +### Security Model + +Interpreters must be resilient to malicious expressions. Eval is read-only; state changes go through a separate `set()` call. Namespace qualification ensures caller isolation. If eval reverts, no state changes persist. + +## Tests + +Tests are in `test/src/lib/` mirroring the `src/lib/` structure. Test files use `.t.sol` suffix. Some test helpers use `.Slow.sol` suffix for reference implementations used in differential testing. + +## Dependencies + +Git submodules in `lib/`: forge-std, openzeppelin-contracts, and Rain Protocol libraries (rain.sol.codegen, rain.solmem, rain.lib.hash, rain.lib.typecast, rain.math.binary, rain.math.float, rain.intorastring). The `rain.sol.codegen` submodule has a remapping configured in `foundry.toml`. + +## Branch Naming + +Feature branches follow `YYYY-MM-DD-description` convention. diff --git a/README.md b/README.md index 89545a2..bd56234 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ interfaces. Uses nixos. -Install `nix develop` - https://nixos.org/download.html. +Install Nix - https://nixos.org/download.html. Run `nix develop` in this repo to drop into the shell. Please ONLY use the nix version of `foundry` for development, to ensure versions are all compatible. diff --git a/audit/2026-03-01-01/pass0/process.md b/audit/2026-03-01-01/pass0/process.md new file mode 100644 index 0000000..39cfd35 --- /dev/null +++ b/audit/2026-03-01-01/pass0/process.md @@ -0,0 +1,52 @@ +# Pass 0: Process Review + +Audit date: 2026-03-01 +Documents reviewed: CLAUDE.md, README.md, foundry.toml, flake.nix, slither.config.json, .github/workflows/rainix.yaml + +## Evidence of Thorough Reading + +### CLAUDE.md (73 lines) +- Sections: Project Overview, Build & Development, Architecture (Core Interfaces, Libraries, Key Types, Security Model), Tests, Dependencies, Branch Naming +- Commands listed: nix develop, rainix-sol-prelude, rainix-sol-test, rainix-sol-static, rainix-sol-legal +- Interfaces listed: IInterpreterV4, IInterpreterStoreV3, IInterpreterCallerV4, IParserV2, ISubParserV4, IInterpreterExternV4, IParserPragmaV1 +- Libraries listed: LibBytecode, LibContext, LibEvaluable, LibNamespace, LibParseMeta, LibGenParseMeta + +### README.md (50 lines) +- Sections: High level, Dev stuff (Local environment & CI), Legal stuff, Contributions + +### foundry.toml (28 lines) +- Settings: solc 0.8.25, optimizer true, optimizer_runs 1000000, evm_version cancun, bytecode_hash none, cbor_metadata false, fuzz runs 2048, one remapping + +### flake.nix (17 lines) +- Inputs: flake-utils, rainix +- Outputs: packages and devShells from rainix + +### slither.config.json (4 lines) +- Excluded detectors: assembly-usage, solc-version, unused-imports, pragma +- Filter paths: forge-std, openzeppelin + +### rainix.yaml (43 lines) +- Jobs: rainix-sol-test, rainix-sol-static, rainix-sol-legal +- Steps: checkout with submodules, nix install, cache, prelude, task + +## Findings + +### A00-1 [LOW] CLAUDE.md says "Requires NixOS" but NixOS is the Linux distribution — what's required is the Nix package manager + +CLAUDE.md line 13: "Requires NixOS." The README.md correctly says "Uses nixos" and directs to nix installation. The actual requirement is the Nix package manager (`nix develop`), which runs on macOS and any Linux distro, not just NixOS. A future session on macOS could be confused about whether the tooling works on their platform. + +### A00-2 [LOW] CLAUDE.md does not mention `revert("...")` prohibition or other Solidity-specific coding conventions + +The project's security model section in CLAUDE.md mentions stateless eval and namespace isolation but does not document Solidity coding conventions that a future session should follow — most importantly, whether string reverts or custom errors are expected. This information exists in the codebase (all errors use custom errors) but is not captured in process docs. + +### A00-3 [INFO] README.md says "Install `nix develop`" which is misleading + +README.md line 14: "Install `nix develop` - https://nixos.org/download.html" — `nix develop` is a command, not something you install. The instruction should say to install Nix and then run `nix develop`. This is a README issue, not CLAUDE.md, but it's the canonical human-facing setup doc. + +### A00-4 [INFO] CLAUDE.md mentions deprecated interfaces but doesn't state policy on modifying them + +Line 39: "Deprecated v1/v2 interfaces live in `src/interface/deprecated/`." A future session doesn't know whether these should be left untouched, whether they can be deleted, or whether they might still need updates. Recent git history shows `IParserPragmaV1` was undeprecated (commit 8bb60bc), suggesting the deprecation status is not always final. + +### A00-5 [INFO] No instruction on Solidity version policy for new vs. existing files + +foundry.toml sets solc 0.8.25, but some interface files use `pragma solidity ^0.8.18` while libraries use `>=0.8.25`. CLAUDE.md doesn't explain when to use which pragma. A future session adding a new file wouldn't know which pragma to use. diff --git a/audit/2026-03-01-01/pass1/Interfaces.md b/audit/2026-03-01-01/pass1/Interfaces.md new file mode 100644 index 0000000..10a9a4e --- /dev/null +++ b/audit/2026-03-01-01/pass1/Interfaces.md @@ -0,0 +1,184 @@ +# Audit Report: Interface Security Review (Agent A06) + +**Date:** 2026-03-01 +**Scope:** Non-deprecated interface files in `src/interface/` +**Pass:** 1 (Security) + +--- + +## 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) + +- **Imports:** `IParserV2`, `IInterpreterStoreV3`, `IInterpreterV4`, `SignedContextV1`, `SIGNED_CONTEXT_SIGNER_OFFSET`, `SIGNED_CONTEXT_CONTEXT_OFFSET`, `SIGNED_CONTEXT_SIGNATURE_OFFSET` (lines 7-18) +- **Struct `EvaluableV4`:** fields `interpreter` (IInterpreterV4), `store` (IInterpreterStoreV3), `bytecode` (bytes) (lines 25-29) +- **Interface `IInterpreterCallerV4`:** (lines 39-48) + - **Event `ContextV2`:** `(address sender, bytes32[][] context)` (line 47) + +### 4. `src/interface/IInterpreterExternV4.sol` (47 lines) + +- **Imports:** `StackItem` from IInterpreterV4.sol (line 5) +- **Type `EncodedExternDispatchV2`:** `bytes32` user-defined value type (line 7) +- **Type `ExternDispatchV2`:** `bytes32` user-defined value type (line 9) +- **Interface `IInterpreterExternV4`:** (lines 21-47) + - **Function `externIntegrity`:** `(ExternDispatchV2 dispatch, uint256 expectedInputs, uint256 expectedOutputs) external view returns (uint256 actualInputs, uint256 actualOutputs)` (lines 33-36) + - **Function `extern`:** `(ExternDispatchV2 dispatch, StackItem[] calldata inputs) external view returns (StackItem[] calldata outputs)` (lines 43-46) + +### 5. `src/interface/IParserV2.sol` (11 lines) + +- **Imports:** `AuthoringMetaV2` from deprecated v1/IParserV1.sol (line 7) +- **Interface `IParserV2`:** (lines 9-11) + - **Function `parse2`:** `(bytes calldata data) external view returns (bytes calldata bytecode)` (line 10) + +### 6. `src/interface/ISubParserV4.sol` (72 lines) + +- **Imports:** `AuthoringMetaV2` from deprecated v2/ISubParserV3.sol, `OperandV2` from IInterpreterV4.sol (lines 7-9) +- **Interface `ISubParserV4`:** (lines 18-72) + - **Function `subParseLiteral2`:** `(bytes calldata data) external view returns (bool success, bytes32 value)` (line 40) + - **Function `subParseWord2`:** `(bytes calldata data) external view returns (bool success, bytes memory bytecode, bytes32[] memory constants)` (line 68-71) + +### 7. `src/interface/IParserPragmaV1.sol` (11 lines) + +- **Struct `PragmaV1`:** field `usingWordsFrom` (address[]) (lines 5-7) +- **Interface `IParserPragmaV1`:** (lines 9-11) + - **Function `parsePragma1`:** `(bytes calldata data) external view returns (PragmaV1 calldata)` (line 10) + +--- + +## Findings + +### A06-1 | LOW | Stale documentation reference to `IInterpreterStoreV2` in `IInterpreterCallerV4` + +**File:** `src/interface/IInterpreterCallerV4.sol`, line 38 + +**Description:** The NatSpec comment for `IInterpreterCallerV4` states: + +``` +/// - OPTIONALLY set state on the associated `IInterpreterStoreV2`. +``` + +The current store interface is `IInterpreterStoreV3`, which the file itself imports at line 8. This stale reference to V2 could mislead implementors into integrating with the wrong store version, potentially causing silent compatibility issues or namespace qualification mismatches between the interpreter and store. While this is a documentation issue, in the context of a security-critical interface where correct version pairing between interpreter, store, and caller is essential for namespace isolation guarantees, an incorrect version reference carries risk. + +--- + +### A06-2 | INFO | `EncodedExternDispatchV2` type is defined but unused in any interface function + +**File:** `src/interface/IInterpreterExternV4.sol`, line 7 + +**Description:** The type `EncodedExternDispatchV2` is defined as `bytes32` but is never used in any function signature within the current interfaces. There is no library in this repository that provides encode/decode functionality between `EncodedExternDispatchV2` and `ExternDispatchV2` (unlike the deprecated versions where `EncodedExternDispatch` and `ExternDispatch` had a similar relationship with an associated library). The type is exported from the file but has zero references anywhere in the codebase. + +This is an informational finding: unused types add cognitive overhead and could lead implementors to believe they need to handle encoding/decoding between the two dispatch types without any provided specification for how to do so. If the type is intentionally left for future use, consider documenting its purpose. If it is vestigial from the V1/V2 pattern, it could be removed to reduce interface surface area. + +--- + +### A06-3 | LOW | `SourceIndexV2` uses `uint256` while V4 type system uses `bytes32` for similar-purpose types + +**File:** `src/interface/IInterpreterV4.sol`, line 47 (usage); defined in `src/interface/deprecated/v1/IInterpreterV2.sol`, line 35 + +**Description:** The `EvalV4` struct uses `SourceIndexV2` (defined as `type SourceIndexV2 is uint256`) for the `sourceIndex` field. However, the V4 type system has migrated similar types to `bytes32` (e.g., `StackItem is bytes32`, `OperandV2 is bytes32`, `ExternDispatchV2 is bytes32`). The `SourceIndexV2` type remains as `uint256` because it is imported from the deprecated v1 `IInterpreterV2.sol`. + +This type inconsistency means that `EvalV4` mixes `bytes32`-based types with a `uint256`-based type. A source index is a small value (typically fitting in `uint16` as the original `SourceIndex` type showed) but is stored in a full `uint256`. While this does not introduce a direct vulnerability at the interface level, it creates an inconsistency that could cause confusion during implementation and may result in implementations failing to validate that the upper bits are zero, which could lead to unexpected behavior depending on how the value is decoded. + +--- + +### A06-4 | INFO | `stateOverlay` format in `EvalV4` is unspecified + +**File:** `src/interface/IInterpreterV4.sol`, lines 50, 61-62 + +**Description:** The `stateOverlay` field in `EvalV4` is typed as `bytes32[]` with only a brief comment that it will "override corresponding gets from the store unless/until they are set to something else in the evaluated logic" (lines 96-98). Unlike `kvs` in `IInterpreterStoreV3.set()` which is documented as potentially pairwise key/value, the `stateOverlay` format is not specified at the interface level. + +The store's `set` function documentation explicitly warns that implementations must guard against corruption (odd number of items, etc.). The `stateOverlay` has no equivalent guidance. If implementations assume a pairwise key/value format (consistent with the store's `kvs` parameter), a malformed overlay with an odd number of elements could cause out-of-bounds reads or silent data corruption depending on the implementation. The interface should specify the expected format or explicitly state that the format is implementation-defined. + +--- + +### A06-5 | MEDIUM | `EvalV4.store` and `EvalV4.namespace` are caller-controlled with no interface-level guidance on validation + +**File:** `src/interface/IInterpreterV4.sol`, lines 43-51; `src/interface/IInterpreterStoreV3.sol`, lines 43-48 + +**Description:** The `EvalV4` struct includes both a `store` (an `IInterpreterStoreV3` address) and a `namespace` (a `FullyQualifiedNamespace`). Both values are provided by the caller. The `namespace` field is typed as `FullyQualifiedNamespace` rather than `StateNamespace`, which means the caller provides an already-qualified namespace directly to the interpreter. + +This design has a subtle security implication: the naming `FullyQualifiedNamespace` implies it has already been qualified (i.e., hashed with `msg.sender` via `LibNamespace.qualifyNamespace`), but the interface accepts it as a raw value from the caller. An interpreter implementation that trusts this value as-is would allow a caller to specify another caller's namespace, breaking the isolation guarantee. + +The security documentation in `IInterpreterV4` (lines 71-82) places the burden on the caller to guard against corrupt return values, and the store documentation (lines 25-30) places the burden on the store to enforce caller isolation. However, there is a gap: the interface does not specify whether the interpreter should re-qualify the namespace or trust the caller-provided one. If an interpreter trusts the `FullyQualifiedNamespace` directly (since it is typed as "fully qualified"), it would bypass namespace isolation. + +The `IInterpreterStoreV3.set()` function correctly accepts a `StateNamespace` (unqualified) and documents that the store MUST fully qualify it. But the reads path through `eval4` uses `FullyQualifiedNamespace`, creating an asymmetry where writes are protected by the store's qualification but reads during eval may not be if the interpreter passes the caller-provided namespace directly to `store.get()`. + +--- + +### A06-6 | INFO | `IInterpreterExternV4.externIntegrity` uses `uint256` for input/output counts while stack uses `StackItem` + +**File:** `src/interface/IInterpreterExternV4.sol`, lines 33-36 + +**Description:** The `externIntegrity` function accepts and returns `uint256` for `expectedInputs`, `expectedOutputs`, `actualInputs`, and `actualOutputs`. These are counts and `uint256` is appropriate for them. However, there is no interface-level constraint or documentation establishing a reasonable upper bound for these values. An implementation that allocates memory based on these counts without bounding them could be vulnerable to memory-based denial of service. Since all functions are `view`, this cannot cause state corruption, but it could cause excessive gas consumption or out-of-gas reverts in callers that do not guard against large return values. + +--- + +### A06-7 | INFO | `ISubParserV4.subParseWord2` returns `memory` while other V4 functions use `calldata` + +**File:** `src/interface/ISubParserV4.sol`, lines 68-71 + +**Description:** The `subParseWord2` function returns `(bool success, bytes memory bytecode, bytes32[] memory constants)` using `memory` return types, while `subParseLiteral2` in the same interface returns stack-based values, and most other V4 interface functions return `calldata` references. This is technically correct -- the sub parser may need to construct new bytecode and constants that do not exist in calldata -- but is worth noting as an intentional design choice. The `memory` return type means implementations must allocate and copy data, making this more expensive than `calldata` returns. Callers should be aware that the returned `bytecode` and `constants` from sub parsers are newly allocated memory that cannot be validated by calldata pointer bounds checking. + +--- + +### A06-8 | LOW | `PragmaV1.usingWordsFrom` has no length or duplication constraints + +**File:** `src/interface/IParserPragmaV1.sol`, lines 5-7 + +**Description:** The `PragmaV1` struct contains an `address[] usingWordsFrom` field with no interface-level constraints on: + +1. Maximum array length -- a malicious input could specify an extremely large number of sub-parser addresses, causing implementations to iterate over all of them during parsing. Since `parsePragma1` is a `view` function, this cannot cause state changes but could be used to force excessive gas consumption. +2. Duplicate addresses -- the interface does not specify whether duplicates should be rejected or deduplicated. Duplicate sub-parser addresses could cause the same sub-parser to be queried multiple times for every unknown word/literal, compounding gas waste. +3. Zero addresses -- `address(0)` in the array could cause implementations to make external calls to the zero address, which would succeed but return empty data, potentially causing subtle parsing failures. + +These are implementation-level concerns but the interface could provide guidance to reduce the risk of divergent implementations. + +--- + +### A06-9 | LOW | No interface guidance on `EvalV4.bytecode` validation or maximum size + +**File:** `src/interface/IInterpreterV4.sol`, lines 43-51 + +**Description:** The `EvalV4` struct accepts arbitrary `bytes bytecode` in calldata. The interface documentation does not specify: + +1. Any maximum bytecode size that implementations should enforce. +2. Whether an empty bytecode (`bytes(0)`) is valid or should revert. +3. Whether the interpreter must validate the bytecode structure before execution. + +The security model (lines 71-82) states the interpreter "MUST be resilient to malicious expressions" and "MAY return garbage or exhibit undefined behaviour or error during an eval, provided that no state changes are persisted." This provides some implicit guidance, but the absence of explicit bytecode validation requirements means implementations may diverge on how they handle malformed bytecode. Some may revert, some may return empty stacks, and some may execute partially -- all valid under the current spec but potentially surprising to callers who expect consistent behavior across interpreter implementations. + +The `LibBytecode` library in this repository does provide validation functions, but the interface does not reference or require their use. + +--- + +## Summary + +| ID | Severity | Title | +|----|----------|-------| +| A06-1 | LOW | Stale documentation reference to `IInterpreterStoreV2` in `IInterpreterCallerV4` | +| A06-2 | INFO | `EncodedExternDispatchV2` type is defined but unused in any interface function | +| A06-3 | LOW | `SourceIndexV2` uses `uint256` while V4 type system uses `bytes32` | +| A06-4 | INFO | `stateOverlay` format in `EvalV4` is unspecified | +| A06-5 | MEDIUM | `EvalV4.store` and `EvalV4.namespace` are caller-controlled with no interface-level guidance on validation | +| A06-6 | INFO | `IInterpreterExternV4.externIntegrity` has no upper bound guidance for input/output counts | +| A06-7 | INFO | `ISubParserV4.subParseWord2` returns `memory` while other V4 functions use `calldata` | +| A06-8 | LOW | `PragmaV1.usingWordsFrom` has no length or duplication constraints | +| A06-9 | LOW | No interface guidance on `EvalV4.bytecode` validation or maximum size | + +**Total findings: 9** (0 CRITICAL, 0 HIGH, 1 MEDIUM, 4 LOW, 4 INFO) diff --git a/audit/2026-03-01-01/pass1/LibBytecode.md b/audit/2026-03-01-01/pass1/LibBytecode.md new file mode 100644 index 0000000..106d64a --- /dev/null +++ b/audit/2026-03-01-01/pass1/LibBytecode.md @@ -0,0 +1,170 @@ +# Security Audit: LibBytecode.sol + +**Auditor:** Agent A01 +**Date:** 2026-03-01 +**File:** `/src/lib/bytecode/LibBytecode.sol` +**Error definitions:** `/src/error/ErrBytecode.sol` + +--- + +## Evidence of Thorough Reading + +### Library + +`LibBytecode` (line 29), a Solidity library for inspecting Rain interpreter bytecode headers. + +### Functions (with line numbers) + +| Function | Line | +|---|---| +| `sourceCount(bytes memory bytecode)` | 44 | +| `checkNoOOBPointers(bytes memory bytecode)` | 70 | +| `sourceRelativeOffset(bytes memory bytecode, uint256 sourceIndex)` | 188 | +| `sourcePointer(bytes memory bytecode, uint256 sourceIndex)` | 209 | +| `sourceOpsCount(bytes memory bytecode, uint256 sourceIndex)` | 227 | +| `sourceStackAllocation(bytes memory bytecode, uint256 sourceIndex)` | 246 | +| `sourceInputsOutputsLength(bytes memory bytecode, uint256 sourceIndex)` | 272 | +| `bytecodeToSources(bytes memory bytecode)` | 291 | + +### Imports + +- `LibPointer`, `Pointer` from `rain.solmem/lib/LibPointer.sol` (line 5) +- `LibBytes` from `rain.solmem/lib/LibBytes.sol` (line 6) +- `LibMemCpy` from `rain.solmem/lib/LibMemCpy.sol` (line 7) +- Seven custom errors from `../../error/ErrBytecode.sol` (lines 8-16) + +### Custom Errors (from ErrBytecode.sol) + +| Error | Description | +|---|---| +| `SourceIndexOutOfBounds(uint256, bytes)` | Source index out of bounds | +| `UnexpectedSources(bytes)` | 0 sources declared but trailing bytes exist | +| `UnexpectedTrailingOffsetBytes(bytes)` | Bytes between offsets and sources | +| `TruncatedSource(bytes)` | Source end does not match next source/end | +| `TruncatedHeader(bytes)` | Offset points past where header fits | +| `TruncatedHeaderOffsets(bytes)` | Bytecode truncated before offsets end | +| `StackSizingsNotMonotonic(bytes, uint256)` | inputs > outputs or outputs > allocation | + +### Using declarations (lines 30-32) + +- `LibPointer for Pointer` +- `LibBytes for bytes` +- `LibMemCpy for Pointer` + +--- + +## Security Findings + +### A01-1 | INFO | Reliance on caller discipline for bounds safety + +**Lines:** 200-285 (functions `sourcePointer`, `sourceOpsCount`, `sourceStackAllocation`, `sourceInputsOutputsLength`) + +**Description:** The functions `sourceOpsCount` (line 227), `sourceStackAllocation` (line 246), `sourceInputsOutputsLength` (line 272), and `sourcePointer` (line 209) all rely on the caller having previously called `checkNoOOBPointers`. Each function's NatSpec documents this requirement with "Callers MUST `checkNoOOBPointers` BEFORE attempting to traverse the bytecode." If a caller omits this prerequisite, `sourcePointer` could return a pointer to memory outside the bytecode allocation, and the subsequent `mload` in `sourceOpsCount` / `sourceStackAllocation` / `sourceInputsOutputsLength` would read arbitrary memory. This is an architectural pattern documented by design, but any consumer of the library that misses the prerequisite would silently read garbage data rather than revert. + +This is noted as INFO because the NatSpec is explicit about the contract, and there is no way to enforce this at the library level without duplicating the full validation check in every accessor. + +--- + +### A01-2 | LOW | `bytecodeToSources` does not call `checkNoOOBPointers` internally + +**Lines:** 291-316 + +**Description:** The function `bytecodeToSources` iterates over sources using `sourcePointer` and `sourceOpsCount` (lines 297-298) but does not call `checkNoOOBPointers` first. If called on malformed bytecode, the `sourcePointer` call at line 297 could return a pointer outside the bytecode memory, and the `unsafeCopyBytesTo` at line 300 would then copy from that out-of-bounds location into a newly allocated `bytes` array. This could read arbitrary memory contents from the EVM process memory (within the same transaction context). + +The function is documented as "not recommended for production code but useful for testing" (line 290), which mitigates the severity. However, a test helper that silently reads out-of-bounds memory could mask bugs rather than surface them. + +**Recommendation:** Consider adding a `checkNoOOBPointers(bytecode)` call at the beginning of `bytecodeToSources`, or at minimum documenting the prerequisite in the NatSpec the same way the other functions do. + +--- + +### A01-3 | INFO | `checkNoOOBPointers` allows sources with zero opcodes + +**Lines:** 136, 150-153 + +**Description:** A source header may declare `opsCount = 0` (line 136). In that case, `sourceEnd = headerEnd + 0 * 4 = headerEnd` (line 150). This is a valid 4-byte source (header only, no opcode body). The validation passes because `sourceEnd == endCursor` is checked at line 151. This is correct behavior and does not represent a vulnerability, but consumers of the library should be aware that a source may contain zero opcodes and handle that case accordingly if they iterate over opcodes. + +--- + +### A01-4 | INFO | `unchecked` arithmetic throughout `checkNoOOBPointers` is safe + +**Lines:** 71-176 + +**Description:** The entire `checkNoOOBPointers` body is wrapped in `unchecked` (line 71). The arithmetic operations within are: + +- `sourcesRelativeStart = 1 + count * 2` (line 75): `count` is at most 255 (a single byte), so this is at most `1 + 510 = 511`. No overflow possible. +- `headerEnd = absoluteOffset + 4` (line 115): `absoluteOffset` is a memory pointer. Adding 4 to a valid memory pointer cannot overflow a `uint256`. +- `sourceEnd = headerEnd + opsCount * 4` (line 150): `opsCount` is at most 255 (a single byte), so `opsCount * 4` is at most 1020. Adding this to a memory pointer cannot overflow. +- `uncheckedOffsetCursor -= 2` (line 158): The loop condition `uncheckedOffsetCursor >= end` at line 104 ensures this only executes while the cursor is at or above `end`. Since `end = bytecode + 0x21` and `uncheckedOffsetCursor` starts at `bytecode + 0x21 + (count-1)*2`, the subtraction always keeps the value at or above `bytecode + 0x1F` (one step past the valid range, which terminates the loop). No underflow to a dangerously large value occurs because the `while (>=)` check guards re-entry. + +All unchecked arithmetic is safe. No overflow or underflow vulnerabilities identified. + +--- + +### A01-5 | INFO | Loop termination in `checkNoOOBPointers` relies on unsigned underflow guard + +**Lines:** 100-104, 158 + +**Description:** The backward iteration uses `while (uncheckedOffsetCursor >= end)` at line 104, where `uncheckedOffsetCursor` is decremented by 2 at line 158 inside `unchecked`. When `count == 1`, `uncheckedOffsetCursor` starts equal to `end` (both are `bytecode + 0x21`). After the single iteration, `uncheckedOffsetCursor -= 2` would set it to `bytecode + 0x1F`, which is less than `end`, so the loop terminates correctly. + +If `count > 1`, the cursor walks backwards through each 2-byte offset slot. When the cursor reaches the first offset (at `end`), it processes it and then decrements once more, again going below `end`. + +Because this is `unchecked` and both values are raw memory pointers (large positive numbers), the subtraction `(bytecode + 0x21) - 2 = bytecode + 0x1F` does not underflow to a dangerous value. The loop terminates correctly. This is safe as implemented. + +--- + +### A01-6 | LOW | `checkNoOOBPointers` does not enforce monotonically increasing offsets + +**Lines:** 104-159 + +**Description:** The backward validation loop checks that each source's `absoluteOffset + 4 + opsCount * 4 == endCursor`, ensuring sources are contiguous and non-overlapping when read backwards. However, it does not explicitly verify that offsets are monotonically increasing (i.e., `offset[i] < offset[i+1]`). This is implicitly enforced: since the loop walks backwards and checks `sourceEnd == endCursor`, then sets `endCursor = absoluteOffset`, any offset that is not strictly less than the next would cause a `TruncatedHeader` or `TruncatedSource` revert (the header check at line 116 would fail because `absoluteOffset + 4 > endCursor`). + +This implicit enforcement is correct but relies on the contiguity check rather than an explicit ordering check. If a future refactor altered the contiguity logic, the ordering guarantee could be silently lost. + +--- + +### A01-7 | INFO | `sourceRelativeOffset` memory read alignment + +**Line:** 196 + +**Description:** In `sourceRelativeOffset`, the assembly reads a 2-byte offset via: +``` +offset := and(mload(add(add(bytecode, 3), mul(sourceIndex, 2))), 0xFFFF) +``` +This loads a 32-byte word starting at `bytecode + 3 + sourceIndex * 2`, then masks to the lowest 16 bits. Since `mload` always reads 32 bytes, the 2-byte offset value ends up in the least significant 16 bits of the loaded word. This is correct because: +- The offset bytes are at positions `bytecode + 0x20 + 1 + sourceIndex*2` and `bytecode + 0x20 + 1 + sourceIndex*2 + 1` in memory. +- `bytecode + 3 + sourceIndex*2` points to `bytecode_data_start - 0x1D + sourceIndex*2`, so the last 2 bytes of the 32-byte word loaded are exactly the offset bytes. + +Wait: `add(bytecode, 3) = bytecode + 3`. `mload(bytecode + 3)` reads 32 bytes from `bytecode + 3`, which gives bytes at memory positions `[bytecode+3, bytecode+3+32)` = `[bytecode+3, bytecode+35)`. The last 2 bytes of this are at `bytecode+33` and `bytecode+34`, i.e., `bytecode+0x21` and `bytecode+0x22`. These are the first offset (at index 0). For `sourceIndex = 0`, `add(add(bytecode, 3), 0) = bytecode + 3`, last two bytes = positions `bytecode + 0x22` and `bytecode + 0x23` which is bytes `[2]` and `[3]` of the bytecode data (0-indexed). But the first offset starts at byte index 1 (positions `bytecode + 0x21` and `bytecode + 0x22`). + +Correction: `mload(bytecode + 3)` loads 32 bytes. The 16-bit value extracted via `and(..., 0xFFFF)` takes the lowest 2 bytes, at positions `bytecode + 3 + 30` and `bytecode + 3 + 31` = `bytecode + 33` and `bytecode + 34` = `bytecode + 0x21` and `bytecode + 0x22`. These are data bytes at indices 1 and 2 of the bytecode (the first 2-byte offset pointer for `sourceIndex == 0`). This is correct. + +No issue found. The read is correctly aligned. + +--- + +### A01-8 | INFO | `bytecodeToSources` legacy opcode byte shifting + +**Lines:** 303-311 + +**Description:** In the legacy conversion loop: +```solidity +mstore8(add(cursor, 1), byte(0, mload(cursor))) +mstore8(cursor, 0) +``` +For each 4-byte opcode, this reads the first byte (the opcode index), writes it into the second byte position (the input position in legacy format), then zeros the first byte. This is a destructive in-place transformation of the freshly copied `source` bytes array. Since the source was copied into a new allocation at line 299-300, this does not corrupt the original bytecode. The logic correctly implements the legacy format transformation where opcode index moves from byte 0 to byte 1, and byte 0 becomes 0. + +No issue found. + +--- + +## Summary + +No CRITICAL or HIGH severity issues were identified. The library implements careful bounds checking in `checkNoOOBPointers` that, when called as a prerequisite, ensures all subsequent accessor functions operate within valid memory. The primary risk pattern is the reliance on callers to invoke `checkNoOOBPointers` before using accessor functions, which is documented but not enforced. + +| Severity | Count | +|---|---| +| CRITICAL | 0 | +| HIGH | 0 | +| MEDIUM | 0 | +| LOW | 2 | +| INFO | 5 | diff --git a/audit/2026-03-01-01/pass1/LibContext.md b/audit/2026-03-01-01/pass1/LibContext.md new file mode 100644 index 0000000..9c664b6 --- /dev/null +++ b/audit/2026-03-01-01/pass1/LibContext.md @@ -0,0 +1,196 @@ +# Audit: LibContext.sol + +**Agent:** A02 +**Pass:** 1 (Security) +**File:** `src/lib/caller/LibContext.sol` +**Date:** 2026-03-01 + +## Evidence of Thorough Reading + +**Library:** `LibContext` (line 45) + +**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:** +- `InvalidSignature(uint256 i)` -- line 20 + +**Constants:** +- `CONTEXT_BASE_COLUMN = 0` -- line 25 +- `CONTEXT_BASE_ROWS = 2` -- line 28 +- `CONTEXT_BASE_ROW_SENDER = 0` -- line 31 +- `CONTEXT_BASE_ROW_CALLING_CONTRACT = 1` -- line 34 + +**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) +- `bytes32[] context` (offset 0x20) +- `bytes signature` (offset 0x40) + +--- + +## Findings + +### A02-1 | INFO | No Replay Protection for Signed Contexts + +**Lines:** 191-216 + +**Description:** +The `build` function verifies that each `SignedContextV1` has a valid signature from the claimed signer over the context data, but provides no mechanism to prevent replay of the same signed context across multiple calls or across different contracts. The same signed context can be resubmitted to `build` an unlimited number of times. + +The NatDoc on `SignedContextV1` (IInterpreterCallerV2.sol lines 41-47) explicitly documents that replay protection (nonce tracking), expiry enforcement, and uniqueness enforcement are the **expression's responsibility**, not this library's. The library comment at lines 159-165 of `build` also acknowledges this, noting the expression is responsible for domain separators and ordering. + +This is documented as INFO because the design intent is clear and correct -- the library is intentionally a low-level building block that delegates policy decisions to the expression layer. However, any calling contract that uses `LibContext.build` without ensuring the expression enforces nonces/uniqueness is vulnerable to signature replay. This is a critical integration consideration rather than a bug in LibContext itself. + +--- + +### A02-2 | INFO | Scratch Space Usage in `hash(SignedContextV1)` Overwrites Solidity Zero Slot + +**Lines:** 84-96 + +**Description:** +The `hash(SignedContextV1 memory)` function uses the Solidity scratch space (memory locations 0x00 and 0x20) for intermediate hash computations. This is explicitly permitted by the Solidity memory model for functions marked `"memory-safe"` -- the scratch space at 0x00-0x3f is designated for short-term use. + +The calling `hash(SignedContextV1[] memory)` function (line 108) correctly accounts for this by saving and restoring `mload(0)` (the `mem0` variable at lines 119, 125, 129) around the sub-call to `hash(SignedContextV1 memory)`. + +This is correct behavior. The scratch space usage is safe within the Solidity memory model. No finding. + +--- + +### A02-3 | INFO | ERC-1271 Signer Can Return Time-Varying Results + +**Lines:** 202-208 + +**Description:** +The `build` function uses OpenZeppelin's `SignatureChecker.isValidSignatureNow`, which supports ERC-1271 smart contract signers. For smart contract signers, verification is done via `staticcall` to the signer contract's `isValidSignature` method. This means: + +1. A smart contract signer can return `true` during `build` but `false` later (or vice versa), making verification non-deterministic across time. +2. The `staticcall` to the signer contract gives it the ability to observe the current block state, which could be used for conditional signature validity. + +This is explicitly documented in the NatDoc for `SignedContextV1` (IInterpreterCallerV2.sol lines 49-53) and is an inherent property of ERC-1271. It is not a bug, but callers should be aware that signed context validity is only guaranteed at the moment `build` is called. + +--- + +### A02-4 | LOW | Reference Implementation Mismatch for Zero Signed Contexts + +**Lines:** 166-221 (LibContext.sol) vs lines 34-64 (test LibContextSlow.sol) + +**Description:** +The `build` function (line 176) conditionally adds the signers column and signed context columns only when `signedContexts.length > 0`: + +```solidity +uint256 contextLength = 1 + baseContext.length + + (signedContexts.length > 0 ? signedContexts.length + 1 : 0); +``` + +The reference implementation in `LibContextSlow.buildStructureSlow` (test file, line 39) always adds `1 + signedContexts.length` columns: + +```solidity +bytes32[][] memory context = new bytes32[][](1 + baseContext.length + 1 + signedContexts.length); +``` + +When `signedContexts.length == 0`, the optimized version produces a context of length `1 + baseContext.length`, while the reference produces `1 + baseContext.length + 1` (with an extra empty signers column at the end). + +The test `testBuildStructureReferenceImplementation` (LibContext.t.sol line 20) only fuzzes the `base` parameter while hardcoding `signedContexts` to exactly 1 entry, so it never tests the `signedContexts.length == 0` case against the reference implementation. A separate test `testBuild0` (line 51) tests the zero case against a manually constructed expectation but does not compare against the slow reference. + +The optimized version is arguably more correct (no point including an empty signers column), but the divergence between the implementation and its reference could mask bugs. The reference implementation should match the optimized version's behavior, or the test should explicitly acknowledge the divergence. + +--- + +### A02-5 | INFO | `unchecked` Block in `build` Is Safe + +**Lines:** 171-221 + +**Description:** +The entire body of `build` is wrapped in `unchecked`. The arithmetic operations within are: + +- `1 + baseContext.length + ...` (line 176): These are lengths of memory arrays which are bounded by available memory. Overflow is not practically possible. +- `offset++` (lines 183, 188, 214): `offset` is bounded by `contextLength` which is bounded by array lengths. +- `i++` in loops (lines 182, 191): Similarly bounded by array lengths. + +All index accesses into `context[]`, `signers[]`, `signedContexts[]`, and `baseContext[]` are done through Solidity's normal array access which includes bounds checking even inside `unchecked` (bounds checks are NOT arithmetic overflow checks). The `unchecked` block only disables overflow/underflow checks on arithmetic, not array bounds checks. + +This is correct and safe. + +--- + +### A02-6 | INFO | `base()` Assembly Is Memory-Safe + +**Lines:** 63-71 + +**Description:** +The `base()` function manually allocates a `bytes32[]` array in assembly: + +1. Reads the free memory pointer (line 65) +2. Stores the array length (2) at the base pointer (line 66) +3. Stores `caller()` and `address()` at the next two slots (lines 67-68) +4. Updates the free memory pointer to `baseArray + 0x60` (line 69) + +This is correct. The total memory used is 0x60 bytes: 0x20 for length + 0x20 for `caller()` + 0x20 for `address()`. The free memory pointer is properly advanced. The `caller()` value is left-padded to 32 bytes by the EVM (address is 20 bytes, upper 12 bytes are zero), which matches the `bytes32` type of the array elements. + +No issue found. + +--- + +### A02-7 | INFO | Signature Hashing Uses `encodePacked`-Equivalent Over Single Array + +**Lines:** 193-206 + +**Description:** +The comment at lines 196-200 acknowledges that the hashing approach is equivalent to `encodePacked` over a single array (without the length prefix). The code hashes the raw bytes of the `context` array: + +```solidity +LibHashNoAlloc.hashWords(signedContexts[i].context) +``` + +Which internally does `keccak256(add(words, 0x20), mul(mload(words), 0x20))` -- hashing the packed words without the length prefix. + +The comment correctly notes this would be a collision risk if multiple dynamic-length values were concatenated before hashing. Since only a single array is hashed (just its elements, contiguously), there is no ambiguity -- different arrays of different lengths produce different byte sequences for hashing (a 2-element array hashes 64 bytes; a 3-element array hashes 96 bytes). The length is implicitly encoded by the total byte count passed to keccak256. + +The hash is then wrapped with `toEthSignedMessageHash` (EIP-191 prefix), which provides domain separation from raw transaction hashes. This is standard and correct. + +No issue found. + +--- + +### A02-8 | INFO | `hash(SignedContextV1)` Hashes Signer as Full 32-Byte Word + +**Lines:** 84-85 + +**Description:** +The signer field (an `address`, 20 bytes) is hashed as a full 32-byte word: + +```solidity +mstore(0, keccak256(add(signedContext, signerOffset), 0x20)) +``` + +Because `signerOffset` is 0 (the first field of the struct), this reads the full 32-byte slot. In Solidity's memory layout, `address` values occupy the lower 20 bytes of a 32-byte slot, with the upper 12 bytes being zero. The reference implementation (`LibContextSlow.hashSlow`) creates a `uint256` from `uint160(signer)` and hashes that, which produces the same result (zero-extended in the upper bytes). + +These approaches are consistent. No issue. + +--- + +## Summary + +No CRITICAL, HIGH, or MEDIUM severity findings were identified in `LibContext.sol`. The library correctly: + +- Builds memory-safe assembly for the base context array +- Verifies signatures using OpenZeppelin's audited `SignatureChecker` +- Uses EIP-191 message hashing for signature verification +- Properly manages scratch space in hash functions +- Uses `unchecked` safely (only for arithmetic, array bounds checks remain active) + +The one LOW finding (A02-4) notes a divergence between the optimized implementation and its test reference implementation when zero signed contexts are provided, which is a test coverage gap rather than a vulnerability. + +The design explicitly and correctly delegates replay protection, nonce tracking, expiry enforcement, and signer authorization to the expression layer. This is well-documented but is a critical integration consideration for any contract using `LibContext.build`. diff --git a/audit/2026-03-01-01/pass1/LibGenParseMeta.md b/audit/2026-03-01-01/pass1/LibGenParseMeta.md new file mode 100644 index 0000000..30e3a5f --- /dev/null +++ b/audit/2026-03-01-01/pass1/LibGenParseMeta.md @@ -0,0 +1,156 @@ +# Audit: LibGenParseMeta.sol + +**Agent:** A04 +**Pass:** 1 (Security) +**File:** `/Users/thedavidmeister/Code/rain.interpreter.interface/src/lib/codegen/LibGenParseMeta.sol` +**Date:** 2026-03-01 + +--- + +## Evidence of Thorough Reading + +**Library name:** `LibGenParseMeta` (line 36) + +**Functions:** +| Function | Line | Visibility | +|---|---|---| +| `findBestExpander` | 49 | internal pure | +| `buildParseMetaV2` | 126 | internal pure | +| `parseMetaConstantString` | 241 | internal pure | + +**Types / Errors / Constants defined in this file:** +| Symbol | Kind | Line | +|---|---|---| +| `META_ITEM_MASK` | constant | 18 | +| `DuplicateFingerprint` | error | 21 | + +**Imported constants (from `LibParseMeta.sol`):** +- `META_ITEM_SIZE` = 4 (line 8 of LibParseMeta.sol) +- `FINGERPRINT_MASK` = 0xFFFFFF (line 23 of LibParseMeta.sol) +- `META_EXPANSION_SIZE` = 0x21 (line 26 of LibParseMeta.sol) +- `META_PREFIX_SIZE` = 1 (line 11 of LibParseMeta.sol) + +**Assembly blocks:** Lines 79-83, 155-157, 159-164, 169-171, 194-195, 202-203, 222-224. + +--- + +## Findings + +### 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. + +--- + +### A04-2 | LOW | Opcode index silently truncated for word lists exceeding 255 entries + +**Location:** Line 217 + +```solidity +toWrite = wordFingerprint | (k << 0x18); +``` + +The variable `k` is the loop index over `authoringMeta` and represents the opcode index for each word. It is shifted left by 24 bits (0x18) to occupy the top byte of a 4-byte item. When `k >= 256`, the value `k << 0x18` produces a result wider than 32 bits. + +At line 222-223, the write operation uses: +```solidity +uint256 mask = ~META_ITEM_MASK; // top 224 bits set, bottom 32 bits clear +assembly ("memory-safe") { + mstore(writeAt, or(and(mload(writeAt), mask), toWrite)) +} +``` + +The `and(mload(writeAt), mask)` clears the bottom 32 bits and preserves the top 224 bits. However, `toWrite` (with `k >= 256`) has bits set above bit 31. The `or` operation would corrupt adjacent data stored in the upper bytes of the 32-byte memory word. For example, with `k = 256`, `toWrite` has bit 32 set, which would flip a bit in the byte immediately preceding this item in the parse meta structure. + +In practice, parse meta is generated at build time and opcodes are unlikely to exceed 255, but this is an unchecked invariant that could produce silently corrupt output. + +**Recommendation:** Add explicit validation that `authoringMeta.length <= 256` or `k <= type(uint8).max` at the start of `buildParseMetaV2`, reverting with a descriptive custom error. + +--- + +### A04-3 | LOW | Insufficient memory allocated for `remaining` array of structs + +**Location:** Lines 79-83 + +```solidity +assembly ("memory-safe") { + remaining := mload(0x40) + mstore(remaining, remainingLength) + mstore(0x40, add(remaining, mul(0x20, add(1, remainingLength)))) +} +``` + +This manually allocates a memory array for `AuthoringMetaV2[] memory`. For a dynamic array of reference-type elements (structs), Solidity's memory layout stores: +- 1 word for the length +- 1 word per element (each word is a pointer to the struct data in memory) + +The allocation size is `0x20 * (1 + remainingLength)`, which is `0x20` for the length slot plus `0x20` per pointer slot. This is correct for storing pointers to existing struct data (which is what `remaining[j] = metas[i]` does -- it copies the pointer, not the struct). + +However, the allocated memory is not zero-initialized. If the `remaining` array is consumed by a caller that reads `remaining.length` elements, this is fine because all `remainingLength` slots get written in the loop at lines 87-96. But the memory is marked "memory-safe" in the assembly annotation, which asserts to the compiler that only memory between `[0x40, new_0x40)` is written and that it is properly allocated. Since the slots are written later in Solidity (not assembly), and the length is correctly set, this is technically safe. + +**Status:** After careful analysis, this allocation pattern is correct. The struct pointer slots are all populated before the function returns. No action required. Noting for completeness only. + +--- + +### A04-4 | INFO | Array out-of-bounds panic instead of descriptive custom error when maxDepth is insufficient + +**Location:** Lines 146-148 + +```solidity +seeds[depth] = seed; +expansions[depth] = expansion; +depth++; +``` + +The arrays `seeds` and `expansions` are allocated with length `maxDepth` (line 138-139). The while loop on line 142 continues as long as `remainingAuthoringMeta.length > 0`. If the bloom filter construction requires more depth levels than `maxDepth`, the array access `seeds[depth]` will revert with a Solidity Panic (0x32, array out-of-bounds), not a descriptive custom error. + +While `unchecked` is active (line 131), Solidity's array bounds checks are independent of `unchecked` and still apply. So this will revert safely -- but with an opaque panic rather than a meaningful error message. + +**Recommendation:** Add a guard such as `require(depth < maxDepth, ...)` or a custom error check before the array writes. + +--- + +### A04-5 | INFO | Unused return value `hashed` deliberately suppressed + +**Location:** Lines 61, 89 + +```solidity +(hashed); +``` + +The `hashed` return value from `LibParseMeta.wordBitmapped` is explicitly suppressed by referencing it in a bare expression statement. This is intentional -- in `findBestExpander`, only the `shifted` (bitmap) value is needed. This pattern avoids compiler warnings about unused variables. No action required, but noting for completeness as an intentional code pattern. + +--- + +### A04-6 | INFO | Function `parseMetaConstantString` accepts `Vm` parameter, indicating Foundry-only usage + +**Location:** Line 241 + +```solidity +function parseMetaConstantString(Vm vm, bytes memory authoringMetaBytes, uint8 buildDepth) +``` + +The `Vm` type from `forge-std/Vm.sol` is imported at line 14. This entire library is a code-generation utility designed for use in Foundry scripts/tests, not for deployment to production. The security surface is therefore limited to build-time correctness rather than on-chain exploitability. All findings in this report should be interpreted in that context. + +--- + +## Summary + +| ID | Severity | Title | +|---|---|---| +| A04-1 | LOW | Seed search loop skips seed value 255 | +| A04-2 | LOW | Opcode index silently truncated for word lists exceeding 255 entries | +| A04-3 | LOW | Insufficient memory allocated for `remaining` array of structs (dismissed after analysis) | +| A04-4 | INFO | Array out-of-bounds panic instead of descriptive custom error when maxDepth is insufficient | +| A04-5 | INFO | Unused return value `hashed` deliberately suppressed | +| A04-6 | INFO | Function accepts Foundry `Vm` parameter, indicating code-gen-only usage | + +No CRITICAL or HIGH severity findings were identified. The library is a build-time code generation utility (not deployed on-chain), which significantly limits the security impact of any issues found. The two LOW findings (A04-1 and A04-2) represent genuine logic defects that could produce suboptimal or incorrect output under specific conditions. diff --git a/audit/2026-03-01-01/pass1/LibParseMeta.md b/audit/2026-03-01-01/pass1/LibParseMeta.md new file mode 100644 index 0000000..3c3f99a --- /dev/null +++ b/audit/2026-03-01-01/pass1/LibParseMeta.md @@ -0,0 +1,156 @@ +# Audit: LibParseMeta.sol + +**Agent:** A03 +**Pass:** 1 (Security) +**File:** `src/lib/parse/LibParseMeta.sol` +**Solidity Version:** ^0.8.25 +**EVM Target:** cancun + +--- + +## Evidence of Thorough Reading + +**Library name:** `LibParseMeta` (line 32) + +**Constants defined:** +- `META_ITEM_SIZE = 4` (line 8) -- 1 byte opcode index + 3 byte fingerprint +- `META_PREFIX_SIZE = 1` (line 11) -- 1 byte for depth +- `FINGERPRINT_MASK = 0xFFFFFF` (line 23) -- 3-byte fingerprint mask +- `META_EXPANSION_SIZE = 0x21` (line 26) -- 32 bytes expansion + 1 byte seed + +**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.) + +**Imports:** +- `LibCtPop` from `rain.math.binary/lib/LibCtPop.sol` (line 5) + +--- + +## Findings + +### A03-1 | LOW | No input validation on `meta` bytes in `lookupWord` + +**Lines:** 67-82 + +The `lookupWord` function reads a `depth` byte from the first byte of `meta` (line 78) and uses it to compute `end` and `dataStart` without any validation that `meta.length` is consistent with the depth or contains enough data for the implied structure. + +```solidity +cursor := add(meta, 1) +let depth := and(mload(cursor), 0xFF) +end := add(cursor, mul(depth, metaExpansionSize)) +dataStart := add(end, metaItemSize) +``` + +If a caller passes a `meta` array that is shorter than `1 + depth * 33 + N * 4` bytes (where N is the total number of items), the function will read beyond the allocated bytes array into adjacent memory. This would read whatever happens to be in memory after the array, rather than reverting. + +**Impact:** Since this is an `internal pure` function, the caller controls the input. In production, meta is generated at build time by `LibGenParseMeta.buildParseMetaV2` and embedded as a constant, so malformed meta is unlikely. However, if `lookupWord` were ever called with user-supplied or truncated meta, it would silently read garbage from memory rather than reverting. The read is out-of-bounds relative to the logical array but still within the Solidity memory space, so it returns incorrect results rather than causing a revert. + +**Recommendation:** Consider adding a bounds check that `meta.length >= META_PREFIX_SIZE + depth * META_EXPANSION_SIZE` before entering the while loop, or document that the meta must be well-formed and produced by `buildParseMetaV2`. + +--- + +### A03-2 | LOW | No bounds check on computed `pos` before data read + +**Lines:** 101, 104-106 + +The `pos` value used to index into the data section is computed from `ctpop` of a portion of the expansion bitmap plus `cumulativeCt`. There is no check that `pos` falls within the actual number of items in the data section before the `mload`: + +```solidity +uint256 pos = LibCtPop.ctpop(expansion & (shifted - 1)) + cumulativeCt; +... +assembly ("memory-safe") { + posData := mload(add(dataStart, mul(pos, metaItemSize))) +} +``` + +For well-formed meta generated by `buildParseMetaV2`, this is safe because the expansions are constructed to be consistent with the item count. However, if the meta is malformed (e.g., an expansion has more bits set than there are corresponding items), `pos` could exceed the actual number of items, causing a read beyond the data section. + +**Impact:** Same as A03-1 -- only exploitable with malformed meta, which is unlikely in production. The out-of-bounds read would return garbage data from adjacent memory, which would almost certainly fail the fingerprint comparison and result in either a false negative (word not found) or, in extremely unlikely cases, a false positive (wrong index returned). + +**Recommendation:** Same as A03-1 -- either validate meta structure or document the invariant. + +--- + +### A03-3 | INFO | Operator precedence on fingerprint comparison is correct but potentially confusing + +**Line:** 110 + +```solidity +if (wordFingerprint == posData & FINGERPRINT_MASK) { +``` + +In Solidity, the bitwise AND operator `&` has higher precedence than the equality operator `==`. Therefore this expression is parsed as `wordFingerprint == (posData & FINGERPRINT_MASK)`, which is the intended behavior. + +This is notably different from C/C++/Java, where `==` has higher precedence than `&`, and the same expression would parse as `(wordFingerprint == posData) & FINGERPRINT_MASK` (a bug). Developers accustomed to C-family languages may misread this line. + +**Impact:** No functional impact. The code is correct. + +**Recommendation:** Consider adding explicit parentheses for clarity: `if (wordFingerprint == (posData & FINGERPRINT_MASK))`. This makes the intent unambiguous regardless of the reader's language background and avoids any doubt during future audits. + +--- + +### A03-4 | INFO | Scratch space usage in `wordBitmapped` is safe but overwrites the zero slot + +**Lines:** 46-57 + +```solidity +assembly ("memory-safe") { + mstore(0, word) + mstore8(0x20, seed) + hashed := keccak256(0, 0x21) + bitmap := shl(byte(0, hashed), 1) +} +``` + +The function uses Solidity's scratch space (addresses 0x00-0x3F) for hashing. `mstore(0, word)` writes 32 bytes to 0x00-0x1F, and `mstore8(0x20, seed)` writes 1 byte to 0x20. The `keccak256(0, 0x21)` hashes exactly 33 bytes from 0x00-0x20. + +This is a correct and gas-efficient use of scratch space. The "memory-safe" annotation is valid because Solidity's documentation permits temporary use of the scratch space within assembly blocks, and no memory beyond 0x3F is accessed. + +One consideration: `mstore(0, word)` overwrites the "zero slot" (0x00-0x1F) which is documented by Solidity as being used as initial value for dynamic memory arrays and as scratch space for hashing. Since this function is `pure` and the scratch space has no guaranteed persistence between assembly blocks, this is safe. + +**Impact:** No impact. + +--- + +### A03-5 | INFO | Bloom filter false positives handled by fingerprint comparison with 2^24 collision resistance + +**Lines:** 100-110 + +The lookup algorithm does not explicitly check whether the bloom filter bit is set in the expansion before computing a position and doing a fingerprint comparison. When the bloom filter bit for a word is NOT set in a given expansion (meaning the word was definitely not placed in this expansion), the function still computes a position via `ctpop` and performs a fingerprint comparison. + +This design is intentional: the fingerprint comparison acts as the secondary collision check. The combined effect of the bloom filter (1 byte = 256 possible positions) and the 3-byte fingerprint provides ~4 bytes (32 bits) of collision resistance, matching Solidity function selectors as documented in the comments (lines 14-22). + +**Impact:** The probability of a false positive for any given word (matching a bloom filter position AND fingerprint of a different word) is approximately 2^-24 per expansion depth, which is consistent with the documented design goals. No security concern beyond the documented collision resistance. + +--- + +### A03-6 | INFO | The `unchecked` block wrapping `lookupWord` is safe + +**Lines:** 68-121 + +The entire body of `lookupWord` is wrapped in `unchecked`. The arithmetic operations within are: +- `cursor < end` (comparison, not affected by unchecked) +- `cursor := add(cursor, 1)` and `cursor := add(cursor, 0x20)` (assembly, not affected by Solidity's checked/unchecked) +- `LibCtPop.ctpop(expansion & (shifted - 1)) + cumulativeCt` (line 101): ctpop returns at most 256, depth is at most 255, so cumulativeCt is at most 255 * 256 = 65,280. No overflow risk. +- `cumulativeCt += LibCtPop.ctpop(expansion)` (line 117): same reasoning, bounded by 255 * 256. +- `shifted - 1` (line 101): `shifted` is always a power of 2 (from `shl(byte(0, hashed), 1)` where `byte(0, hashed)` is 0-255), so `shifted >= 1` and `shifted - 1` never underflows. + +**Impact:** No overflow or underflow risk. The use of `unchecked` is justified for gas optimization. + +--- + +## Summary + +| ID | Severity | Title | +|----|----------|-------| +| A03-1 | LOW | No input validation on `meta` bytes in `lookupWord` | +| A03-2 | LOW | No bounds check on computed `pos` before data read | +| A03-3 | INFO | Operator precedence on fingerprint comparison is correct but potentially confusing | +| A03-4 | INFO | Scratch space usage in `wordBitmapped` is safe but overwrites the zero slot | +| A03-5 | INFO | Bloom filter false positives handled by fingerprint with 2^24 collision resistance | +| A03-6 | INFO | The `unchecked` block wrapping `lookupWord` is safe | + +No CRITICAL or HIGH severity issues were identified. The library is well-designed for its purpose. The two LOW findings relate to the absence of defensive bounds checks on the `meta` input, which is mitigated in practice by the fact that meta is generated at build time and embedded as a constant. The INFO findings document correct behavior for future reference. diff --git a/audit/2026-03-01-01/pass1/SmallFiles.md b/audit/2026-03-01-01/pass1/SmallFiles.md new file mode 100644 index 0000000..d663578 --- /dev/null +++ b/audit/2026-03-01-01/pass1/SmallFiles.md @@ -0,0 +1,144 @@ +# Audit Pass 1 (Security) -- Small Files Review + +**Agent:** A05 +**Date:** 2026-03-01 +**Scope:** +- `src/lib/caller/LibEvaluable.sol` +- `src/lib/ns/LibNamespace.sol` +- `src/error/ErrBytecode.sol` +- `src/error/ErrExtern.sol` +- `src/error/ErrIntegrity.sol` + +--- + +## Evidence of Thorough Reading + +### 1. `src/lib/caller/LibEvaluable.sol` + +- **Library name:** `LibEvaluable` (line 16) +- **Functions:** + - `hash(EvaluableV4 memory evaluable) internal pure returns (bytes32)` -- line 23 +- **Imports:** `IInterpreterStoreV3` (line 8), `EvaluableV4` (line 10) +- **Assembly block:** lines 26-34, marked `"memory-safe"`, uses scratch space (0x00-0x3f) +- **Hashing strategy:** Two-phase hash -- first hashes the two address fields (`interpreter`, `store`) as a 64-byte region, then hashes bytecode content, then combines the two hashes. + +### 2. `src/lib/ns/LibNamespace.sol` + +- **Library name:** `LibNamespace` (line 11) +- **Functions:** + - `qualifyNamespace(StateNamespace stateNamespace, address sender) internal pure returns (FullyQualifiedNamespace qualifiedNamespace)` -- line 24 +- **Imports:** `StateNamespace`, `FullyQualifiedNamespace` from `IInterpreterV4.sol` (line 5) +- **Types used:** `StateNamespace` is `uint256`, `FullyQualifiedNamespace` is `uint256` +- **Assembly block:** lines 29-33, marked `"memory-safe"`, uses scratch space (0x00-0x3f) + +### 3. `src/error/ErrBytecode.sol` + +- **Errors defined:** + - `SourceIndexOutOfBounds(uint256 sourceIndex, bytes bytecode)` -- line 8 + - `UnexpectedSources(bytes bytecode)` -- line 12 + - `UnexpectedTrailingOffsetBytes(bytes bytecode)` -- line 16 + - `TruncatedSource(bytes bytecode)` -- line 21 + - `TruncatedHeader(bytes bytecode)` -- line 26 + - `TruncatedHeaderOffsets(bytes bytecode)` -- line 30 + - `StackSizingsNotMonotonic(bytes bytecode, uint256 relativeOffset)` -- line 36 +- **All errors are custom errors** (no `require` strings). +- All seven errors are used in `src/lib/bytecode/LibBytecode.sol`. + +### 4. `src/error/ErrExtern.sol` + +- **Errors defined:** + - `NotAnExternContract(address extern)` -- line 6 + - `BadInputs(uint256 expected, uint256 actual)` -- line 12 +- **Usage:** Neither error is imported or used anywhere within this repository. They are interface-level definitions for implementations. + +### 5. `src/error/ErrIntegrity.sol` + +- **Errors defined:** + - `BadOpInputsLength(uint256 opIndex, uint256 calculatedInputs, uint256 bytecodeInputs)` -- line 9 + - `BadOpOutputsLength(uint256 opIndex, uint256 calculatedOutputs, uint256 bytecodeOutputs)` -- line 15 +- **Usage:** Neither error is imported or used anywhere within this repository. They are interface-level definitions for implementations. + +--- + +## Findings + +### A05-1 | INFO | LibEvaluable hash reads raw memory bytes for address fields rather than masking + +**File:** `src/lib/caller/LibEvaluable.sol`, line 28 + +**Description:** + +The `hash` function hashes the first 64 bytes of the `EvaluableV4` struct in memory via `keccak256(evaluable, 0x40)`. The struct fields at offset 0x00 and 0x20 are `IInterpreterV4` and `IInterpreterStoreV3` (both `address`-typed interface references), stored as 32-byte words with the address right-aligned and 12 bytes of zero-padding on the left. + +The reference implementation in `LibEvaluableSlow.hashSlow` (test file) explicitly cleans the addresses via `uint256(uint160(address(...)))` before hashing: + +```solidity +keccak256(abi.encodePacked( + uint256(uint160(address(evaluable.interpreter))), + uint256(uint160(address(evaluable.store))) +)) +``` + +The optimized assembly version hashes the raw memory including the high 12 bytes of each slot. In standard Solidity execution, these bytes are always zero because the compiler cleans address values when writing to memory. However, if a struct were constructed via assembly with dirty upper bits, the optimized and reference implementations would diverge. The existing fuzz test (`testEvaluableV4ReferenceImplementation`) uses Solidity-constructed structs, so it does not exercise this case. + +**Impact:** Negligible in practice. Solidity guarantees clean address representations in memory for normally constructed values. This only affects structs constructed via assembly with intentionally dirty padding, which would be misuse of the type system. The two implementations are equivalent for all well-formed inputs. + +--- + +### A05-2 | INFO | ErrExtern.sol and ErrIntegrity.sol errors are defined but unused in this repository + +**File:** `src/error/ErrExtern.sol` (lines 6, 12), `src/error/ErrIntegrity.sol` (lines 9, 15) + +**Description:** + +The errors `NotAnExternContract`, `BadInputs`, `BadOpInputsLength`, and `BadOpOutputsLength` are defined but never imported or referenced anywhere in this codebase. A search for `import.*ErrExtern` and `import.*ErrIntegrity` across the entire `src/` directory returns zero results. + +This is expected for an interface library -- these errors define a standard error ABI that implementing contracts (interpreters, extern contracts) are expected to use. However, the lack of usage within this repository means there is no compile-time verification that the error signatures remain consistent with any implementation expectations. + +**Impact:** No security impact. These are purely interface-level definitions. Downstream implementations that import and use these errors will get compile-time checking. + +--- + +### A05-3 | INFO | LibNamespace.qualifyNamespace does not enforce that `sender` is `msg.sender` + +**File:** `src/lib/ns/LibNamespace.sol`, line 24 + +**Description:** + +The `qualifyNamespace` function is `internal pure` and accepts any `address sender` parameter. It does not and cannot enforce that the caller passes `msg.sender`. The NatSpec on line 14 states it "essentially just hashes the `msg.sender` into the state namespace as-is," but the function itself has no such restriction. + +This is by design -- the function is a pure utility, and enforcing `msg.sender` would require it to be `view` and would limit composability. The responsibility for passing the correct sender rests entirely with the calling contract. The documentation on line 8-10 explicitly marks this as "OPTIONAL" functionality. + +**Impact:** No direct vulnerability. A calling contract that passes an incorrect `sender` could break namespace isolation, but this would be a bug in the calling contract, not in this library. The documentation is clear about the caller's responsibility. + +--- + +### A05-4 | INFO | No domain separation between LibEvaluable.hash and LibNamespace.qualifyNamespace + +**File:** `src/lib/caller/LibEvaluable.sol` line 33, `src/lib/ns/LibNamespace.sol` line 32 + +**Description:** + +Both `LibEvaluable.hash` (final step) and `LibNamespace.qualifyNamespace` produce a `bytes32` by hashing exactly 64 bytes from scratch space via `keccak256(0, 0x40)`. Neither incorporates a domain separator (e.g., a type tag or unique prefix). If the same pair of 32-byte values were placed at 0x00 and 0x20 in both contexts, the results would collide. + +In practice, this is not exploitable: +- `LibEvaluable.hash` stores two intermediate keccak256 hashes at 0x00 and 0x20 before the final hash. +- `LibNamespace.qualifyNamespace` stores a `StateNamespace` (uint256) and an `address` (cleaned to 20 bytes, 12 zero-padded). +- The output types are distinct (`bytes32` vs `FullyQualifiedNamespace`) and are never compared in the same context. +- The intermediate keccak256 hashes in `LibEvaluable.hash` are uniformly distributed, making an accidental match with a (stateNamespace, sender) pair astronomically unlikely. + +**Impact:** Theoretical only. The different input structures and disjoint usage contexts make cross-domain collision practically impossible. + +--- + +## Summary + +No CRITICAL, HIGH, MEDIUM, or LOW severity findings were identified. The five files under review are minimal, well-structured, and follow security best practices: + +- All error definitions use custom errors (no string-based reverts). +- Assembly blocks are correctly marked `"memory-safe"` and only use scratch space for writes. +- The hashing logic in `LibEvaluable.hash` correctly uses a two-phase Merkle-like approach that is sensitive to all three struct fields (interpreter, store, bytecode), as validated by comprehensive fuzz tests. +- Namespace qualification in `LibNamespace.qualifyNamespace` correctly hashes the sender with the state namespace to produce disjoint qualified namespaces per caller. +- The `keccak256`-based approach to namespace isolation is sound -- collisions require a keccak256 preimage attack which is computationally infeasible. + +Four INFO-level observations were noted for completeness, none of which represent actionable security concerns. diff --git a/audit/2026-03-01-01/pass2/LibBytecode.md b/audit/2026-03-01-01/pass2/LibBytecode.md new file mode 100644 index 0000000..d11b321 --- /dev/null +++ b/audit/2026-03-01-01/pass2/LibBytecode.md @@ -0,0 +1,126 @@ +# Audit Pass 2 -- Test Coverage for LibBytecode.sol + +**Agent:** A01 +**Date:** 2026-03-01 +**Source file:** `src/lib/bytecode/LibBytecode.sol` + +--- + +## Source Inventory + +**Library:** `LibBytecode` (line 29) + +| # | Function | Line | +|---|----------|------| +| 1 | `sourceCount(bytes memory)` | 44 | +| 2 | `checkNoOOBPointers(bytes memory)` | 70 | +| 3 | `sourceRelativeOffset(bytes memory, uint256)` | 188 | +| 4 | `sourcePointer(bytes memory, uint256)` | 209 | +| 5 | `sourceOpsCount(bytes memory, uint256)` | 227 | +| 6 | `sourceStackAllocation(bytes memory, uint256)` | 246 | +| 7 | `sourceInputsOutputsLength(bytes memory, uint256)` | 272 | +| 8 | `bytecodeToSources(bytes memory)` | 291 | + +--- + +## Test Inventory + +### `LibBytecodeSourceCountTest` (LibBytecode.sourceCount.t.sol) + +| # | Test Function | Line | +|---|---------------|------| +| 1 | `testSourceCount0()` | 11 | +| 2 | `testSourceCount1(bytes)` | 17 | +| 3 | `testSourceCountReference(bytes)` | 23 | + +### `LibBytecodeSourceOpsCountTest` (LibBytecode.sourceOpsCount.t.sol) + +| # | Test Function | Line | +|---|---------------|------| +| 1 | `testSourceOpsCount()` | 14 | +| 2 | `testSourceOpsCountIndexOutOfBounds(bytes,uint256,uint256,bytes32)` | 26 | +| 3 | `testSourceOpsCountAgainstSlow(bytes,uint256,uint256,bytes32)` | 40 | + +### `LibBytecodeSourceStackAllocationTest` (LibBytecode.sourceStackAllocation.t.sol) + +| # | Test Function | Line | +|---|---------------|------| +| 1 | `testSourceStackAllocationIndexOutOfBounds(bytes,uint256,uint256,bytes32)` | 20 | +| 2 | `testSourceStackAllocationAgainstSlow(bytes,uint256,uint256,bytes32)` | 34 | + +### `LibBytecodeSourceRelativeOffsetTest` (LibBytecode.sourceRelativeOffset.t.sol) + +| # | Test Function | Line | +|---|---------------|------| +| 1 | `testSourceRelativeOffsetHappy()` | 11 | +| 2 | `testSourceRelativeOffsetIndexError()` | 36 | +| 3 | `testSourceRelativeOffsetReference(bytes,uint256,uint256,bytes32)` | 65 | + +### `LibBytecodeSourceInputsOutputsTest` (LibBytecode.sourceInputsOutputs.t.sol) + +| # | Test Function | Line | +|---|---------------|------| +| 1 | `testSourceInputsOutputsIndexOutOfBounds(bytes,uint256,uint256,bytes32)` | 20 | +| 2 | `testSourceInputsOutputsAgainstSlow(bytes,uint256,uint256,bytes32)` | 34 | + +### `LibBytecodeCheckNoOOBPointersTest` (LibBytecode.checkNoOOBPointers.t.sol) + +| # | Test Function | Line | +|---|---------------|------| +| 1 | `testCheckNoOOBPointersConforming(bytes,uint256,bytes32)` | 18 | +| 2 | `testCheckNoOOBPointers0()` | 31 | +| 3 | `testCheckNoOOBPointers1()` | 36 | +| 4 | `testCheckNoOOBPointers1Fail(bytes)` | 41 | +| 5 | `testCheckNoOOBPointersOffsetsTruncated(bytes,uint8,uint256)` | 51 | +| 6 | `testCheckNoOOBPointersHeaderTruncated(bytes,uint8,bytes32,uint256)` | 73 | +| 7 | `testCheckNoOOBPointersSourceTruncated(bytes,uint8,bytes32,uint8)` | 111 | +| 8 | `testCheckNoOOBPointersTrailingOffsetBytes(bytes,bytes,uint8,bytes32)` | 138 | +| 9 | `testCheckNoOOBPointersCorruptSourcesCount(bytes,uint8,bytes32,uint8)` | 193 | +| 10 | `testCheckNoOOBPointersCorruptOffsetPointer(bytes,uint8,bytes32,uint8)` | 214 | +| 11 | `testCheckNoOOBPointersCorruptOpsCount(bytes,uint8,bytes32,uint8)` | 237 | +| 12 | `testCheckNoOOBPointersEndGarbage(bytes,bytes)` | 266 | +| 13 | `testCheckNoOOBPointersInputsNotMonotonic(bytes,uint8,bytes32,uint256,uint256)` | 278 | +| 14 | `testCheckNoOOBPointersOutputsNotMonotonic(bytes,uint8,bytes32,uint256,uint256)` | 333 | + +### `LibBytecodeSourcePointerTest` (LibBytecode.sourcePointer.t.sol) + +| # | Test Function | Line | +|---|---------------|------| +| 1 | `testSourcePointerEmpty0(uint256)` | 17 | +| 2 | `testSourcePointerEmpty1(uint256)` | 25 | +| 3 | `testSourcePointerIndexOutOfBounds(bytes,uint256,uint256,bytes32)` | 32 | +| 4 | `testSourcePointerAgainstSlow(bytes,uint256,uint256,bytes32)` | 46 | + +### Support files + +- `LibBytecodeSlow.sol`: Slow reference implementations for `sourceCount`, `sourceRelativeOffset`, `sourcePointer`, `sourceOpsCount`, `sourceStackAllocation`, `sourceInputsOutputsLength`. No reference implementation for `bytecodeToSources`. +- `BytecodeTest.sol`: Abstract test base providing `conformBytecode` (fuzz helper) and `randomSourceIndex`/`randomSourcePosition`. + +--- + +## Findings + +### A01-1 | HIGH | `bytecodeToSources` has zero test coverage + +The function `bytecodeToSources` (line 291-316) has no test file, no test function, and is never called anywhere in the test suite. A grep across the entire test directory for `bytecodeToSources` returns zero matches. There is also no slow reference implementation in `LibBytecodeSlow.sol`. + +This function contains non-trivial inline assembly (lines 303-311) that performs opcode index byte-shuffling in a loop (`mstore8` to move the opcode index into the input position and zero the original). Bugs in this assembly loop -- such as off-by-one in the cursor increment, incorrect byte offset in `mstore8`, or failure to handle zero-length sources -- would go entirely undetected. + +**Untested paths include:** +- Zero sources (empty array return) +- Single source with zero opcodes (zero-length inner `bytes`) +- Single source with one or more opcodes (byte-shuffling correctness) +- Multiple sources (correct iteration and offset calculation) +- Large opcode counts near the 255-op maximum per source + +### A01-2 | LOW | No explicit test for `sourceCount` with a single-byte input of value 0 + +The test `testSourceCount0` only tests the empty bytes case (`""`). While `testSourceCount1` is a fuzz test that covers `bytecode.length > 0`, there is no explicit unit test asserting `sourceCount(hex"00") == 0`. This is a documented equivalence in the function's NatSpec ("0x and 0x00 are equivalent, both having 0 sources"). The `checkNoOOBPointers` tests do exercise `hex"00"` indirectly (`testCheckNoOOBPointers1`), so the gap is minor, but the `sourceCount` test contract itself does not explicitly verify this documented invariant. + +### A01-3 | INFO | `conformBytecode` may mask edge cases at extreme source counts + +The `conformBytecode` helper in `BytecodeTest.sol` (line 8) caps `maxSourceCount` at 255 and requires each source to consume at least 6 bytes (2 offset + 4 header). This means fuzz testing never produces bytecode with a source count byte of 255 unless the input `bytes` is at least 1531 bytes long (1 + 255*2 + 255*4). Typical fuzz inputs may be much shorter, meaning high source counts (e.g., 128-255 sources) may be underexercised. This is an inherent limitation of the fuzz approach rather than a definitive gap, but it is worth noting that boundary behavior near `sourceCount == 255` is unlikely to be heavily tested in practice. + +### A01-4 | INFO | `sourceStackAllocation` has no explicit happy-path unit test + +Unlike `sourceOpsCount` and `sourceRelativeOffset`, which both have dedicated `testSourceOpsCount()` and `testSourceRelativeOffsetHappy()` unit tests with hand-crafted bytecode examples, `sourceStackAllocation` relies entirely on fuzz tests against the slow reference implementation. While the fuzz approach is arguably more thorough, the lack of any hand-crafted example test means a correlated bug in both the production and reference implementation would go undetected. The same observation applies to `sourceInputsOutputsLength`. diff --git a/audit/2026-03-01-01/pass2/LibContext.md b/audit/2026-03-01-01/pass2/LibContext.md new file mode 100644 index 0000000..9066e26 --- /dev/null +++ b/audit/2026-03-01-01/pass2/LibContext.md @@ -0,0 +1,106 @@ +# Pass 2 -- Test Coverage: LibContext.sol and LibEvaluable.sol + +Agent: A02 +Date: 2026-03-01 + +## Source Inventory + +### LibContext (src/lib/caller/LibContext.sol) + +| Line | Item | +|------|------| +| 20 | `error InvalidSignature(uint256 i)` | +| 25-34 | Constants: `CONTEXT_BASE_COLUMN`, `CONTEXT_BASE_ROWS`, `CONTEXT_BASE_ROW_SENDER`, `CONTEXT_BASE_ROW_CALLING_CONTRACT` | +| 45 | `library LibContext` | +| 63 | `function base() internal view returns (bytes32[] memory)` -- assembly: allocates array with `msg.sender` and `address(this)` | +| 79 | `function hash(SignedContextV1 memory) internal pure returns (bytes32)` -- assembly: hashes signer, context, signature fields | +| 108 | `function hash(SignedContextV1[] memory) internal pure returns (bytes32)` -- loops over array, chains subhashes starting from HASH_NIL | +| 166 | `function build(bytes32[][] memory, SignedContextV1[] memory) internal view returns (bytes32[][] memory)` -- merges base, caller context, signed contexts; verifies signatures; reverts `InvalidSignature(i)` on bad sig | + +### LibEvaluable (src/lib/caller/LibEvaluable.sol) + +| Line | Item | +|------|------| +| 16 | `library LibEvaluable` | +| 23 | `function hash(EvaluableV4 memory) internal pure returns (bytes32)` -- assembly: hashes interpreter+store together, then hashes with bytecode hash | + +## Test Inventory + +### LibContextTest (test/src/lib/caller/LibContext.t.sol) + +| Line | Test | Exercises | +|------|------|-----------| +| 10 | `testBase()` | `base()` -- checks length=2, sender, address(this) | +| 20 | `testBuildStructureReferenceImplementation(bytes32[][])` | `build()` with 1 valid signed context vs slow reference; fuzz 100 runs | +| 51 | `testBuild0()` | `build()` with empty base and empty signed contexts | +| 63 | `testBuildGas0()` | `build()` gas benchmark, empty inputs | + +### LibContextHashTest (test/src/lib/caller/LibContext.hash.t.sol) + +| Line | Test | Exercises | +|------|------|-----------| +| 10 | `testFuzzHash0()` | `hash(SignedContextV1[])` with 3 zero-filled entries | +| 19 | `testHash(uint256)` | raw keccak256 (not LibContext) | +| 26 | `testHashGas0()` | raw keccak256 gas bench | +| 34 | `testSignedContextHashReferenceImplementation(SignedContextV1)` | `hash(SignedContextV1)` vs slow reference; fuzz 100 runs | +| 38 | `testSignedContextArrayHashReferenceImplementation0()` | `hash(SignedContextV1[])` with length-1 array of zero values | +| 44 | `testSignedContextHashGas0()` | `hash(SignedContextV1)` gas bench | +| 52 | `testSignedContextHashEncodeGas0()` | abi.encode+keccak comparison gas bench | +| 60 | `testSignedContextArrayHashReferenceImplementation(SignedContextV1[])` | `hash(SignedContextV1[])` vs slow reference; fuzz 100 runs | + +### LibContextSlow (test/src/lib/caller/LibContextSlow.sol) + +Reference implementation used by tests. Not a test contract itself. + +- `hashSlow(SignedContextV1 memory)` -- line 15 +- `hashSlow(SignedContextV1[] memory)` -- line 24 +- `buildStructureSlow(bytes32[][], SignedContextV1[])` -- line 34 + +### LibEvaluableTest (test/src/lib/caller/LibEvaluable.t.sol) + +| Line | Test | Exercises | +|------|------|-----------| +| 17 | `testEvaluableV4KnownHash()` | `hash()` with known inputs, asserts known output | +| 23 | `testEvaluableV4HashDifferent(EvaluableV4, EvaluableV4)` | different inputs produce different hashes; fuzz | +| 30 | `testEvaluableV4HashSame(EvaluableV4)` | identical inputs produce identical hashes; fuzz | +| 35 | `testEvaluableV4HashSensitivity(EvaluableV4, EvaluableV4)` | changing each field individually changes hash; fuzz | +| 75 | `testEvaluableV4HashGas0()` | gas bench | +| 79 | `testEvaluableV4BytecodeLengthSensitivity()` | hex"01" vs hex"0100" produce different hashes (padding safety) | +| 88 | `testEvaluableV4HashGasSlow0()` | slow reference gas bench | +| 92 | `testEvaluableV4ReferenceImplementation(EvaluableV4)` | `hash()` vs slow reference; fuzz | + +### LibEvaluableSlow (test/src/lib/caller/LibEvaluableSlow.sol) + +Reference implementation. Not a test contract itself. + +- `hashSlow(EvaluableV2 memory)` -- line 9 +- `hashSlow(EvaluableV4 memory)` -- line 19 + +--- + +## Findings + +### A02-1 | LOW | No test triggers the `InvalidSignature` revert path in `build()` + +**Description:** `LibContext.build()` (line 202-210) reverts with `InvalidSignature(i)` when a signed context has an invalid signature. No test in the test suite ever calls `build()` with an invalid signature to verify this revert path is reached. `testBuildStructureReferenceImplementation` uses a hardcoded valid signature, `testBuild0` uses empty signed contexts, and `testBuildGas0` also uses empty signed contexts. The `InvalidSignature` error is not imported or referenced in any test file. + +This means: +- There is no confirmation that the revert triggers with the correct index `i`. +- There is no test for a partially-valid list (e.g., first signature valid, second invalid) to confirm the correct index is reported. +- There is no test that verifies the error selector or ABI encoding. + +### A02-2 | LOW | No test for `build()` with non-empty base context and zero signed contexts + +**Description:** `build()` (line 176) has a conditional branch: `signedContexts.length > 0 ? signedContexts.length + 1 : 0`. The zero-signed-contexts path is tested only with an empty base context (`testBuild0` at line 51). There is no test that calls `build()` with a non-empty `baseContext` and an empty `signedContexts` array. This means the context length calculation `1 + baseContext.length + 0` and the base-context-merging loop operating without any signed context appended are not tested together. Note that the reference `buildStructureSlow` always adds `+1` for the signers column regardless of whether `signedContexts` is empty, so it structurally disagrees with `build()` on empty signed contexts, which means `testBuildStructureReferenceImplementation` cannot actually be used to validate this case (the fuzz test always supplies exactly 1 signed context). + +### A02-3 | LOW | Reference implementation `buildStructureSlow` structurally disagrees with `build()` for zero signed contexts + +**Description:** `buildStructureSlow` (LibContextSlow.sol line 39) always allocates `1 + baseContext.length + 1 + signedContexts.length` columns, unconditionally adding a signers column even when `signedContexts.length == 0`. The production `build()` function (LibContext.sol line 176) conditionally omits the signers column when there are no signed contexts: `1 + baseContext.length + (signedContexts.length > 0 ? signedContexts.length + 1 : 0)`. This means the reference implementation cannot serve as a valid oracle for the zero-signed-contexts case. The fuzz test `testBuildStructureReferenceImplementation` works around this by always providing exactly 1 signed context, but this leaves the structural difference unvalidated and means the reference implementation does not actually cover the full behavior space of `build()`. + +### A02-4 | INFO | No test for `build()` with multiple signed contexts + +**Description:** `testBuildStructureReferenceImplementation` always provides exactly 1 signed context (line 34: `new SignedContextV1[](1)`). While the single-element case exercises the loop body, there is no explicit test for multiple signed contexts to verify that the signers array is populated correctly for all entries and that the context columns are appended in the right order. The fuzz varies the `base` array but not the number of signed contexts. + +### A02-5 | INFO | `hash(SignedContextV1[])` not tested with an empty array + +**Description:** `hash(SignedContextV1[] memory)` (line 108) starts with `HASH_NIL` and returns it immediately if the array is empty (since the while-loop body never executes). No test calls this function with an empty array to confirm that `HASH_NIL` is returned. The existing tests use arrays of length 1 (`testSignedContextArrayHashReferenceImplementation0`) and length 3 (`testFuzzHash0`), plus a fuzz that could randomly produce length 0 but does not assert the specific `HASH_NIL` return value. A dedicated test for the empty case would strengthen coverage of this boundary. diff --git a/audit/2026-03-01-01/pass2/LibParseMeta.md b/audit/2026-03-01-01/pass2/LibParseMeta.md new file mode 100644 index 0000000..3bdcfdd --- /dev/null +++ b/audit/2026-03-01-01/pass2/LibParseMeta.md @@ -0,0 +1,141 @@ +# Pass 2 — Test Coverage Report: LibParseMeta, LibGenParseMeta, LibNamespace + +Agent: A03 +Date: 2026-03-01 + +## Source File Inventory + +### `src/lib/parse/LibParseMeta.sol` + +Library: `LibParseMeta` + +| 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 | + +Error: `DuplicateFingerprint()` (line 21, reverted at line 208) + +### `src/lib/ns/LibNamespace.sol` + +Library: `LibNamespace` + +| Function | Line | Visibility | +|---|---|---| +| `qualifyNamespace(StateNamespace stateNamespace, address sender)` | 24 | internal pure | + +## Test File Inventory + +### `test/src/lib/parse/LibParseMeta.wordBitmapped.t.sol` + +Contract: `LitParseMetaTest` + +| Test Function | Line | +|---|---| +| `referenceWordBitmapped(uint256 seed, bytes32 word)` (helper) | 10 | +| `testWordBitmapped(uint256 seed, bytes32 word)` | 19 | + +### `test/src/lib/codegen/LibGenParseMeta.findExpander.t.sol` + +Contract: `LibGenParseMetaFindExpanderTest` + +| Test Function | Line | +|---|---| +| `testFindExpanderSmall(AuthoringMetaV2[] memory authoringMeta)` | 24 | + +### `test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol` + +Contract: `LibGenParseMetaBuildMetaTest` + +| Test Function | Line | +|---|---| +| `expanderDepth(uint256 n)` (helper) | 14 | +| `testBuildMeta(AuthoringMetaV2[] memory authoringMeta)` | 25 | +| `testRoundMetaExpanderShallow(AuthoringMetaV2[] memory authoringMeta, uint8 j, bytes32 notFound)` | 31 | +| `testRoundMetaExpanderDeeper(AuthoringMetaV2[] memory authoringMeta, uint8 j, bytes32 notFound)` | 52 | + +### `test/src/lib/ns/LibNamespace.t.sol` + +Contract: `LibNamespaceTest` + +| Test Function | Line | +|---|---| +| `testQualifyNamespaceReferenceImplementation(StateNamespace stateNamespace, address sender)` | 11 | +| `testQualifyNamespaceGas0(StateNamespace stateNamespace, address sender)` | 18 | +| `testQualifyNamespaceGasSlow0(StateNamespace stateNamespace, address sender)` | 22 | + +### Test Helpers + +- `test/src/lib/ns/LibNamespaceSlow.sol` — Slow reference implementation of `qualifyNamespace` using `abi.encode`. +- `test/lib/meta/LibAuthoringMeta.sol` — Helper to extract word arrays from `AuthoringMetaV2[]`. +- `test/lib/bloom/LibBloom.sol` — Bloom filter duplicate detection used by test preconditions. + +## Coverage Gap Findings + +### A03-1 + +**Severity:** MEDIUM + +**Title:** `DuplicateFingerprint` revert path in `buildParseMetaV2` is never tested + +**Description:** The `DuplicateFingerprint` error (defined at `src/lib/codegen/LibGenParseMeta.sol:21`, reverted at line 208) is never triggered by any test. A grep across the entire `test/` directory for `DuplicateFingerprint` returns zero results. This revert occurs when two different words produce the same 3-byte fingerprint at the same bloom filter slot across all expansion depths. Without a test exercising this path, there is no verification that (a) the error is correctly triggered when it should be, and (b) the detection logic around the fingerprint comparison at lines 206-208 is correct. Constructing a test that crafts two words with colliding fingerprints at a specific seed would verify this path. + +--- + +### A03-2 + +**Severity:** MEDIUM + +**Title:** `parseMetaConstantString` function has no test coverage + +**Description:** The function `LibGenParseMeta.parseMetaConstantString` at `src/lib/codegen/LibGenParseMeta.sol:241` has zero test coverage. A grep for `parseMetaConstantString` across the test directory returns no results. This function performs `abi.decode` of arbitrary bytes into `AuthoringMetaV2[]`, then calls `buildParseMetaV2` and `LibCodeGen` functions to produce a formatted constant string. The `abi.decode` step in particular could fail on malformed input in ways that differ from passing a pre-decoded array. While this function is primarily used during code generation (not at runtime), verifying it produces correct output for at least one known input would confirm the integration between `buildParseMetaV2` and the code generation utilities works as intended. + +--- + +### A03-3 + +**Severity:** LOW + +**Title:** `lookupWord` has no dedicated test file and lacks direct edge-case coverage + +**Description:** `LibParseMeta.lookupWord` at `src/lib/parse/LibParseMeta.sol:67` is only exercised indirectly through the round-trip tests in `LibGenParseMeta.buildMeta.t.sol` (lines 43, 47, 65, 69). There is no dedicated test file for `lookupWord` (such as `LibParseMeta.lookupWord.t.sol`). This means: + +1. The function is never tested with hand-crafted meta bytes that would exercise specific assembly paths (e.g., multi-depth traversal with known collision patterns). +2. There is no test with an empty or minimal meta (e.g., a 1-byte meta representing depth=0), which would exercise the early-exit path at line 120 where `cursor >= end` immediately. +3. There is no test with meta containing exactly one depth level, verifying the single-pass lookup path in isolation. +4. The function is never tested with corrupted or malformed meta bytes, which could expose unsafe memory reads in the assembly at lines 75-82 and 93-106. + +The existing fuzz tests provide good probabilistic coverage of the happy path, but deterministic edge-case tests would strengthen confidence in the assembly-heavy implementation. + +--- + +### A03-4 + +**Severity:** LOW + +**Title:** `findBestExpander` is not tested with empty input or large collision-heavy input + +**Description:** `LibGenParseMeta.findBestExpander` at `src/lib/codegen/LibGenParseMeta.sol:49` is tested by `testFindExpanderSmall` (line 24 in `LibGenParseMeta.findExpander.t.sol`), but only for arrays with up to 32 elements and no bloom-detected duplicates. The following cases lack coverage: + +1. **Empty input array:** Calling `findBestExpander` with a zero-length `AuthoringMetaV2[]` array. In this case, `bestCt` remains 0, and `remainingLength = 0 - 0 = 0`, which should work correctly, but this is never verified. +2. **Large input (>32 elements) where perfect expansion is impossible:** The test only covers inputs up to 32 words (where perfect expansion into 256 bloom slots is highly probable). There is no test for inputs between 33 and 255 elements, which would require the `remaining` array to be non-empty and would exercise the second loop (lines 87-96) more thoroughly, including the collision branch at line 92 where items are placed into `remaining`. + +--- + +### A03-5 + +**Severity:** INFO + +**Title:** `buildParseMetaV2` lacks a test for depth overflow beyond `maxDepth` + +**Description:** In `buildParseMetaV2` at `src/lib/codegen/LibGenParseMeta.sol:126`, the `depth` variable (line 137) increments inside a while loop until all remaining authoring meta is expanded. If the number of required expansion depths exceeds `maxDepth`, the `seeds[depth]` assignment at line 146 would cause an out-of-bounds array access. The existing fuzz tests use the `expanderDepth` helper to compute a generous `maxDepth`, so this boundary is unlikely to be hit in practice. However, there is no explicit test confirming that an insufficiently small `maxDepth` causes a clean revert (from the array bounds check), rather than corrupting memory. This is informational since the Solidity runtime would revert on the array bounds check, but a test documenting this behavior would be useful. diff --git a/audit/2026-03-01-01/pass3/Interfaces.md b/audit/2026-03-01-01/pass3/Interfaces.md new file mode 100644 index 0000000..e21be7a --- /dev/null +++ b/audit/2026-03-01-01/pass3/Interfaces.md @@ -0,0 +1,300 @@ +# Pass 3 — Documentation Audit (Agent A02) + +## Scope + +All current (non-deprecated) interface and error files in `src/interface/` and `src/error/`. + +--- + +## File-by-File Evidence of Review + +### 1. `src/interface/IInterpreterV4.sol` + +| Item | Kind | Line(s) | +|------|------|---------| +| `OperandV2` | user-defined value type | 39 | +| `StackItem` | user-defined value type | 41 | +| `EvalV4` | struct | 43-51 | +| `IInterpreterV4` | interface | 89-102 | +| `eval4` | function | 101 | + +### 2. `src/interface/IInterpreterStoreV3.sol` + +| Item | Kind | Line(s) | +|------|------|---------| +| `IInterpreterStoreV3` | interface | 31-66 | +| `Set` | event | 36 | +| `set` | function | 48 | +| `get` | function | 65 | + +### 3. `src/interface/IInterpreterCallerV4.sol` + +| Item | Kind | Line(s) | +|------|------|---------| +| `EvaluableV4` | struct | 25-29 | +| `IInterpreterCallerV4` | interface | 39-48 | +| `ContextV2` | event | 47 | + +### 4. `src/interface/IInterpreterExternV4.sol` + +| Item | Kind | Line(s) | +|------|------|---------| +| `EncodedExternDispatchV2` | user-defined value type | 7 | +| `ExternDispatchV2` | user-defined value type | 9 | +| `IInterpreterExternV4` | interface | 21-47 | +| `externIntegrity` | function | 33-36 | +| `extern` | function | 43-46 | + +### 5. `src/interface/IParserV2.sol` + +| Item | Kind | Line(s) | +|------|------|---------| +| `IParserV2` | interface | 9-11 | +| `parse2` | function | 10 | + +### 6. `src/interface/ISubParserV4.sol` + +| Item | Kind | Line(s) | +|------|------|---------| +| `ISubParserV4` | interface | 18-72 | +| `subParseLiteral2` | function | 40 | +| `subParseWord2` | function | 68-71 | + +### 7. `src/interface/IParserPragmaV1.sol` + +| Item | Kind | Line(s) | +|------|------|---------| +| `PragmaV1` | struct | 5-7 | +| `parsePragma1` | function | 10 | +| `IParserPragmaV1` | interface | 9-11 | + +### 8. `src/error/ErrBytecode.sol` + +| Item | Kind | Line(s) | +|------|------|---------| +| `SourceIndexOutOfBounds` | error | 8 | +| `UnexpectedSources` | error | 12 | +| `UnexpectedTrailingOffsetBytes` | error | 16 | +| `TruncatedSource` | error | 21 | +| `TruncatedHeader` | error | 26 | +| `TruncatedHeaderOffsets` | error | 30 | +| `StackSizingsNotMonotonic` | error | 36 | + +### 9. `src/error/ErrExtern.sol` + +| Item | Kind | Line(s) | +|------|------|---------| +| `NotAnExternContract` | error | 6 | +| `BadInputs` | error | 12 | + +### 10. `src/error/ErrIntegrity.sol` + +| Item | Kind | Line(s) | +|------|------|---------| +| `BadOpInputsLength` | error | 9 | +| `BadOpOutputsLength` | error | 15 | + +--- + +## Findings + +### A02-1 | LOW | `OperandV2` type has no natspec documentation + +**File:** `src/interface/IInterpreterV4.sol`, line 39 + +The user-defined value type `OperandV2` is declared as: + +```solidity +type OperandV2 is bytes32; +``` + +There is no natspec comment explaining what an operand represents in this context, how it differs from the prior `Operand` type (which was `uint256`-based and is still re-exported from the deprecated v1 path), or how the `bytes32` representation should be interpreted. + +--- + +### A02-2 | LOW | `StackItem` type has no natspec documentation + +**File:** `src/interface/IInterpreterV4.sol`, line 41 + +The user-defined value type `StackItem` is declared as: + +```solidity +type StackItem is bytes32; +``` + +There is no natspec comment. `StackItem` is a fundamental type used across the interpreter, extern, and eval interfaces. Its semantics (e.g. that it holds packed Rain decimal floats, or arbitrary 32-byte values) should be documented at the definition site. + +--- + +### A02-3 | LOW | `EvalV4` struct has no natspec documentation + +**File:** `src/interface/IInterpreterV4.sol`, lines 43-51 + +The `EvalV4` struct is declared without any natspec `@title`, `@notice`, or `@dev` comment, and none of its seven fields have `@param` documentation: + +```solidity +struct EvalV4 { + IInterpreterStoreV3 store; + FullyQualifiedNamespace namespace; + bytes bytecode; + SourceIndexV2 sourceIndex; + bytes32[][] context; + StackItem[] inputs; + bytes32[] stateOverlay; +} +``` + +The struct is the sole parameter to the central `eval4` function. Each field deserves a `@param` tag explaining its role, constraints, and expected format. In particular: +- `stateOverlay` is new in V4 and its encoding/semantics are undocumented at the struct level. +- `inputs` vs `context` distinction is not explained. +- The relationship between `bytecode` and `sourceIndex` is not described. + +--- + +### A02-4 | LOW | `eval4` function lacks `@param` and `@return` natspec tags + +**File:** `src/interface/IInterpreterV4.sol`, lines 90-101 + +The `eval4` function has a prose comment (lines 90-100) but no formal `@param` or `@return` tags: + +```solidity +/// Rainlang magic happens here. +/// +/// Pass Rainlang bytecode in calldata, and get back the stack and storage +/// writes. +/// +/// Key differences in `eval4`: +/// - Supports state overlays ... +/// - Numbers are treated as packed Rain decimal floats ... +function eval4(EvalV4 calldata eval) external view returns (StackItem[] calldata stack, bytes32[] calldata writes); +``` + +Missing: +- `@param eval` — description of the eval parameter. +- `@return stack` — what the returned stack represents (e.g. output values from the evaluated source). +- `@return writes` — what the writes represent and how the caller should handle them (e.g. pass to `IInterpreterStoreV3.set`). + +--- + +### A02-5 | LOW | `IParserV2` interface and `parse2` function have no natspec documentation + +**File:** `src/interface/IParserV2.sol`, lines 9-11 + +The entire interface is bare: + +```solidity +interface IParserV2 { + function parse2(bytes calldata data) external view returns (bytes calldata bytecode); +} +``` + +Missing: +- `@title` / `@notice` on `IParserV2` describing its purpose. +- `@param data` on `parse2` — what the input data represents (Rainlang source? encoded data?). +- `@return bytecode` — what format the bytecode is in, relationship to `EvalV4.bytecode`. +- Implementer responsibilities (e.g. revert on invalid input, what constitutes valid Rainlang). + +--- + +### A02-6 | LOW | `IParserPragmaV1` interface and `parsePragma1` function have no natspec documentation + +**File:** `src/interface/IParserPragmaV1.sol`, lines 9-11 + +```solidity +interface IParserPragmaV1 { + function parsePragma1(bytes calldata data) external view returns (PragmaV1 calldata); +} +``` + +Missing: +- `@title` / `@notice` on `IParserPragmaV1`. +- `@param data` on `parsePragma1` — what the input bytes represent. +- `@return` — when it might revert vs return an empty pragma. +- Implementer responsibilities. + +--- + +### A02-7 | LOW | `PragmaV1` struct has no natspec documentation + +**File:** `src/interface/IParserPragmaV1.sol`, lines 5-7 + +```solidity +struct PragmaV1 { + address[] usingWordsFrom; +} +``` + +Missing: +- Natspec explaining the purpose of this struct. +- `@param usingWordsFrom` — what addresses this refers to (sub-parser contracts? extern contracts?) and how they are used by the parser. + +--- + +### A02-8 | INFO | `EncodedExternDispatchV2` type has no natspec documentation + +**File:** `src/interface/IInterpreterExternV4.sol`, line 7 + +```solidity +type EncodedExternDispatchV2 is bytes32; +``` + +No natspec explains what "encoded" means in this context, how it differs from `ExternDispatchV2`, or what encoding scheme is used. The type is not used directly in the interface functions (which use `ExternDispatchV2`), but it is exported for downstream use. + +--- + +### A02-9 | INFO | `ExternDispatchV2` type has no natspec documentation + +**File:** `src/interface/IInterpreterExternV4.sol`, line 9 + +```solidity +type ExternDispatchV2 is bytes32; +``` + +No natspec explains the structure or semantics of this dispatch value. The function-level docs say it is "analogous to the opcode/operand in the interpreter" but the type definition itself is undocumented. + +--- + +### A02-10 | INFO | `NotAnExternContract` error has no `@param` tag for its parameter + +**File:** `src/error/ErrExtern.sol`, line 6 + +```solidity +/// Thrown when the extern interface is not supported. +error NotAnExternContract(address extern); +``` + +The error has a descriptive comment but no `@param extern` tag documenting its parameter. By contrast, `BadInputs` on line 12 in the same file does document its parameters. + +--- + +### A02-11 | INFO | `IInterpreterCallerV4` natspec references `IInterpreterStoreV2` instead of `IInterpreterStoreV3` + +**File:** `src/interface/IInterpreterCallerV4.sol`, line 38 + +The interface-level natspec says: + +```solidity +/// - OPTIONALLY set state on the associated `IInterpreterStoreV2`. +``` + +This should reference `IInterpreterStoreV3`, which is the store version actually imported and used in `EvaluableV4` on line 27 of the same file. The V2 reference is a stale leftover from the V3 caller interface. + +--- + +### A02-12 | INFO | `EvaluableV4` struct natspec uses bare `@param` tags without `@notice` or `@dev` context + +**File:** `src/interface/IInterpreterCallerV4.sol`, lines 22-29 + +The struct has `@param` tags (lines 22-24) but no surrounding `@notice` or `@dev` to describe the struct's overall purpose. The V3 predecessor (`EvaluableV3` in the deprecated file) had a prose description ("Struct over the return of `IParserV2.parse2` which MAY be more convenient to work with than raw addresses.") but this was dropped in V4. While the `@param` tags are present, a one-line purpose statement would improve clarity. + +--- + +## Summary + +| Severity | Count | +|----------|-------| +| LOW | 7 | +| INFO | 5 | +| **Total** | **12** | + +The most significant documentation gaps are concentrated in `IInterpreterV4.sol` (types `OperandV2`, `StackItem`, struct `EvalV4`, and function `eval4` lacking formal param/return tags) and `IParserV2.sol` / `IParserPragmaV1.sol` (entirely undocumented interfaces). The error files and `IInterpreterStoreV3`, `IInterpreterExternV4`, and `ISubParserV4` are generally well-documented, with only minor gaps noted at INFO level. diff --git a/audit/2026-03-01-01/pass3/Libraries.md b/audit/2026-03-01-01/pass3/Libraries.md new file mode 100644 index 0000000..82c893f --- /dev/null +++ b/audit/2026-03-01-01/pass3/Libraries.md @@ -0,0 +1,209 @@ +# Pass 3 -- Documentation Audit: Library Files + +**Agent:** A01 +**Date:** 2026-03-01 +**Scope:** Documentation completeness and accuracy for all library files + +--- + +## Files Reviewed + +### 1. LibBytecode.sol + +**Path:** `src/lib/bytecode/LibBytecode.sol` +**Library name:** `LibBytecode` (line 29) +**Library-level natspec:** Yes (lines 18-28). Title and notice present. + +| Function | Line | Natspec | @param | @return | Accuracy | +|---|---|---|---|---|---| +| `sourceCount` | 44 | Yes (lines 34-43) | `bytecode` documented | `count` documented | Accurate | +| `checkNoOOBPointers` | 70 | Yes (lines 55-68) | None documented (only `bytecode`) | No return | See A01-1 | +| `sourceRelativeOffset` | 188 | Yes (lines 178-187) | `bytecode`, `sourceIndex` documented | `offset` documented | Accurate | +| `sourcePointer` | 209 | Yes (lines 200-208) | `bytecode`, `sourceIndex` documented | `pointer` documented | Accurate | +| `sourceOpsCount` | 227 | Yes (lines 219-226) | `bytecode`, `sourceIndex` documented | `opsCount` documented | Accurate | +| `sourceStackAllocation` | 246 | Yes (lines 236-245) | `bytecode`, `sourceIndex` documented | `allocation` documented | Accurate | +| `sourceInputsOutputsLength` | 272 | Yes (lines 259-271) | `bytecode`, `sourceIndex` documented | `inputs`, `outputs` documented | Accurate | +| `bytecodeToSources` | 291 | Yes (lines 287-290) | None | None | See A01-2 | + +**Constants/Types:** None defined in this file (errors are imported from `ErrBytecode.sol`). + +--- + +### 2. LibContext.sol + +**Path:** `src/lib/caller/LibContext.sol` +**Library name:** `LibContext` (line 45) +**Library-level natspec:** Yes (lines 36-44). Title and notice present. + +| Function | Line | Natspec | @param | @return | Accuracy | +|---|---|---|---|---|---| +| `base` | 63 | Yes (lines 48-62) | None (takes no params) | `baseArray` documented | Accurate | +| `hash(SignedContextV1)` | 79 | Yes (lines 73-78) | `signedContext` documented | `hashed` documented -- see A01-3 | See A01-3 | +| `hash(SignedContextV1[])` | 108 | Yes (lines 99-107) | `signedContexts` documented | `hashed` documented | Accurate | +| `build` | 166 | Yes (lines 140-165) | `baseContext`, `signedContexts` documented | Missing | See A01-4 | + +**Constants:** `CONTEXT_BASE_COLUMN` (line 25), `CONTEXT_BASE_ROWS` (line 28), `CONTEXT_BASE_ROW_SENDER` (line 31), `CONTEXT_BASE_ROW_CALLING_CONTRACT` (line 34) -- all documented with `@dev`. + +**Errors:** `InvalidSignature` (line 20) -- documented. + +--- + +### 3. LibEvaluable.sol + +**Path:** `src/lib/caller/LibEvaluable.sol` +**Library name:** `LibEvaluable` (line 16) +**Library-level natspec:** Yes (lines 12-15). Title and notice present. + +| Function | Line | Natspec | @param | @return | Accuracy | +|---|---|---|---|---|---| +| `hash` | 23 | Yes (lines 17-22) | `evaluable` documented | `evaluableHash` documented (as unnamed return via description) | Accurate | + +No findings for this file. + +--- + +### 4. LibGenParseMeta.sol + +**Path:** `src/lib/codegen/LibGenParseMeta.sol` +**Library name:** `LibGenParseMeta` (line 36) +**Library-level natspec:** Yes (lines 23-35). Title and notice present. + +| Function | Line | Natspec | @param | @return | Accuracy | +|---|---|---|---|---|---| +| `findBestExpander` | 49 | Yes (lines 37-48) | `metas` documented | `bestSeed`, `bestExpansion`, `remaining` documented | Accurate | +| `buildParseMetaV2` | 126 | Yes (lines 100-125) | `authoringMeta`, `maxDepth` documented | `parseMeta` documented | Accurate | +| `parseMetaConstantString` | 241 | Yes (lines 232-240) | `vm`, `authoringMetaBytes`, `buildDepth` documented | Return documented (unnamed) | See A01-5 | + +**Constants:** `META_ITEM_MASK` (line 18) -- not documented with `@dev`. See A01-6. +**Errors:** `DuplicateFingerprint` (line 21) -- documented with `@dev`. + +--- + +### 5. LibNamespace.sol + +**Path:** `src/lib/ns/LibNamespace.sol` +**Library name:** `LibNamespace` (line 11) +**Library-level natspec:** Yes (lines 7-10). Title and notice present. + +| Function | Line | Natspec | @param | @return | Accuracy | +|---|---|---|---|---|---| +| `qualifyNamespace` | 24 | Yes (lines 12-23) | `stateNamespace`, `sender` documented | `qualifiedNamespace` documented | Accurate | + +No findings for this file. + +--- + +### 6. LibParseMeta.sol + +**Path:** `src/lib/parse/LibParseMeta.sol` +**Library name:** `LibParseMeta` (line 32) +**Library-level natspec:** Yes (lines 28-31). Title and notice present. + +| Function | Line | Natspec | @param | @return | Accuracy | +|---|---|---|---|---|---| +| `wordBitmapped` | 45 | Yes (lines 33-44) | `seed`, `word` documented | `bitmap`, `hashed` documented | See A01-7 | +| `lookupWord` | 67 | Yes (lines 60-66) | `meta`, `word` documented | Partially documented | See A01-8 | + +**Constants:** `META_ITEM_SIZE` (line 8), `META_PREFIX_SIZE` (line 11), `FINGERPRINT_MASK` (line 23), `META_EXPANSION_SIZE` (line 26) -- all documented with `@dev`. + +--- + +## Findings + +### A01-1 | INFO | `checkNoOOBPointers` missing `@param` for `bytecode` + +**File:** `src/lib/bytecode/LibBytecode.sol`, lines 55-70 + +The function `checkNoOOBPointers(bytes memory bytecode)` has thorough natspec describing what it does and what it checks, but it does not include an explicit `@param bytecode` tag. Every other function in `LibBytecode` that accepts `bytecode` explicitly documents it with `@param bytecode The bytecode to inspect.` + +--- + +### A01-2 | LOW | `bytecodeToSources` missing `@param` and `@return` documentation + +**File:** `src/lib/bytecode/LibBytecode.sol`, lines 287-316 + +The function `bytecodeToSources(bytes memory bytecode)` has a brief comment (lines 287-290) but is missing both `@param bytecode` and `@return` tags. Its comment mentions "backwards compatibility" and that it is "not recommended for production code" but does not document: +- What the `bytecode` parameter is or what format it must be in. +- What the return value `bytes[] memory` represents (the old-style individual source arrays). +- The structural transformation it performs (stripping 4-byte headers and shifting opcode indices). + +This is the only function in `LibBytecode` without `@param`/`@return` tags, creating an inconsistency with the rest of the library. + +--- + +### A01-3 | INFO | `hash(SignedContextV1)` natspec `@param` tag formatting for return value + +**File:** `src/lib/caller/LibContext.sol`, lines 77-78 + +The natspec for `hash(SignedContextV1 memory signedContext)` uses `@param hashed` instead of `@return hashed` for the return value on line 78: + +```solidity +/// @param signedContext The signed context to hash. +/// @param hashed The hashed signed context. +``` + +The second line should be `@return hashed The hashed signed context.` to match the Solidity natspec convention and to be consistent with `hash(SignedContextV1[] memory)` at line 107 which correctly uses `@return hashed`. + +--- + +### A01-4 | LOW | `build` missing `@return` documentation + +**File:** `src/lib/caller/LibContext.sol`, lines 140-169 + +The function `build(bytes32[][] memory baseContext, SignedContextV1[] memory signedContexts)` has thorough documentation for both parameters but does not include a `@return` tag for the returned `bytes32[][] memory`. The return value is the fully assembled context matrix, whose structure (base column first, then caller-provided columns, then signers column and signed context columns) is important for callers to understand. While the structure is partially described in the `@param` documentation, a dedicated `@return` tag would make it explicit. + +--- + +### A01-5 | INFO | `parseMetaConstantString` uses `Vm` parameter without noting it is Foundry-only + +**File:** `src/lib/codegen/LibGenParseMeta.sol`, lines 232-244 + +The `Vm` parameter is documented as `@param vm The Vm instance to use for generating the constant string` but the natspec does not make clear that `Vm` is Foundry's cheatcode interface and this function is only usable in a test/codegen context. The library-level natspec does not note this either. While the import of `forge-std/Vm.sol` on line 14 is a hint, an explicit note would improve clarity for consumers, especially given the library-level comment says the library is for "building parse meta from authoring meta" without mentioning the Foundry dependency. + +--- + +### A01-6 | INFO | `META_ITEM_MASK` constant missing `@dev` documentation + +**File:** `src/lib/codegen/LibGenParseMeta.sol`, line 18 + +The constant `META_ITEM_MASK` is defined as `(1 << META_ITEM_SIZE) - 1` but has no `@dev` natspec explaining its purpose. All other constants in the related `LibParseMeta.sol` (`META_ITEM_SIZE`, `META_PREFIX_SIZE`, `FINGERPRINT_MASK`, `META_EXPANSION_SIZE`) have `@dev` documentation. This constant is used as a mask in `buildParseMetaV2` at line 221 to mask out existing data when writing items, and would benefit from a brief explanation of its role. + +--- + +### A01-7 | INFO | `wordBitmapped` return documentation names mismatch + +**File:** `src/lib/parse/LibParseMeta.sol`, lines 33-44 + +The `@return` tag documents the second return value as `hashed` with the description "A uint256 with the low 3 bytes set" (line 43-44). However, the actual return value `hashed` is the full `keccak256` hash (line 49); it is the *caller* that must apply `FINGERPRINT_MASK` to extract only the low 3 bytes. The description "with the low 3 bytes set" could be misleading -- it implies the returned value already has only the low 3 bytes, when in fact it is a full 32-byte hash. The function's own inline comment (lines 50-56) correctly describes the situation, but the `@return` natspec is slightly inaccurate. + +--- + +### A01-8 | LOW | `lookupWord` return values not documented with `@return` names + +**File:** `src/lib/parse/LibParseMeta.sol`, lines 60-67 + +The function `lookupWord` returns `(bool, uint256)` and the natspec provides two `@return` tags (lines 65-66): + +```solidity +/// @return True if the word exists in the parse meta. +/// @return The index of the word in the parse meta. +``` + +The return values are unnamed in the function signature on line 67. While the descriptions are adequate, naming the return values (e.g., `exists` and `index`) would improve code readability and tooling support. Additionally, the natspec on line 60 mentions "return the index and io fn pointer" but the function does not return any "io fn pointer" -- it returns a boolean and an index. This text appears to be a stale remnant from a prior version of the function, making the natspec description inaccurate relative to the actual implementation. + +--- + +## Summary + +| ID | Severity | File | Title | +|---|---|---|---| +| A01-1 | INFO | LibBytecode.sol | `checkNoOOBPointers` missing `@param` for `bytecode` | +| A01-2 | LOW | LibBytecode.sol | `bytecodeToSources` missing `@param` and `@return` documentation | +| A01-3 | INFO | LibContext.sol | `hash(SignedContextV1)` uses `@param` instead of `@return` for return value | +| A01-4 | LOW | LibContext.sol | `build` missing `@return` documentation | +| A01-5 | INFO | LibGenParseMeta.sol | `parseMetaConstantString` does not note Foundry-only dependency | +| A01-6 | INFO | LibGenParseMeta.sol | `META_ITEM_MASK` constant missing `@dev` documentation | +| A01-7 | INFO | LibParseMeta.sol | `wordBitmapped` return documentation inaccurately describes `hashed` | +| A01-8 | LOW | LibParseMeta.sol | `lookupWord` natspec mentions stale "io fn pointer" and returns are unnamed | + +**Total findings:** 8 (3 LOW, 5 INFO) diff --git a/audit/2026-03-01-01/pass4/CodeQuality.md b/audit/2026-03-01-01/pass4/CodeQuality.md new file mode 100644 index 0000000..9af9ffe --- /dev/null +++ b/audit/2026-03-01-01/pass4/CodeQuality.md @@ -0,0 +1,209 @@ +# Pass 4: Code Quality Audit + +**Auditor:** Claude Code (Opus 4.6) +**Date:** 2026-03-01 +**Scope:** All source files, interface files, and config files in `rain.interpreter.interface` + +--- + +## Files Reviewed (Evidence of Thorough Reading) + +### Source Libraries + +**`src/lib/bytecode/LibBytecode.sol`** -- library `LibBytecode` +- `sourceCount(bytes memory bytecode)` -- line 44 +- `checkNoOOBPointers(bytes memory bytecode)` -- line 70 +- `sourceRelativeOffset(bytes memory bytecode, uint256 sourceIndex)` -- line 188 +- `sourcePointer(bytes memory bytecode, uint256 sourceIndex)` -- line 209 +- `sourceOpsCount(bytes memory bytecode, uint256 sourceIndex)` -- line 227 +- `sourceStackAllocation(bytes memory bytecode, uint256 sourceIndex)` -- line 246 +- `sourceInputsOutputsLength(bytes memory bytecode, uint256 sourceIndex)` -- line 272 +- `bytecodeToSources(bytes memory bytecode)` -- line 291 + +**`src/lib/caller/LibContext.sol`** -- library `LibContext` +- `base()` -- line 63 +- `hash(SignedContextV1 memory signedContext)` -- line 79 +- `hash(SignedContextV1[] memory signedContexts)` -- line 108 +- `build(bytes32[][] memory baseContext, SignedContextV1[] memory signedContexts)` -- line 166 + +**`src/lib/caller/LibEvaluable.sol`** -- library `LibEvaluable` +- `hash(EvaluableV4 memory evaluable)` -- line 23 + +**`src/lib/codegen/LibGenParseMeta.sol`** -- library `LibGenParseMeta` +- `findBestExpander(AuthoringMetaV2[] memory metas)` -- line 49 +- `buildParseMetaV2(AuthoringMetaV2[] memory authoringMeta, uint8 maxDepth)` -- line 126 +- `parseMetaConstantString(Vm vm, bytes memory authoringMetaBytes, uint8 buildDepth)` -- line 241 + +**`src/lib/ns/LibNamespace.sol`** -- library `LibNamespace` +- `qualifyNamespace(StateNamespace stateNamespace, address sender)` -- line 24 + +**`src/lib/parse/LibParseMeta.sol`** -- library `LibParseMeta` +- `wordBitmapped(uint256 seed, bytes32 word)` -- line 45 +- `lookupWord(bytes memory meta, bytes32 word)` -- line 67 + +### Error Files + +**`src/error/ErrBytecode.sol`** -- error declarations only +- `SourceIndexOutOfBounds` (line 8), `UnexpectedSources` (line 12), `UnexpectedTrailingOffsetBytes` (line 16), `TruncatedSource` (line 21), `TruncatedHeader` (line 26), `TruncatedHeaderOffsets` (line 30), `StackSizingsNotMonotonic` (line 36) + +**`src/error/ErrExtern.sol`** -- error declarations only +- `NotAnExternContract` (line 6), `BadInputs` (line 12) + +**`src/error/ErrIntegrity.sol`** -- error declarations only +- `BadOpInputsLength` (line 9), `BadOpOutputsLength` (line 15) + +### Interface Files (Non-Deprecated) + +**`src/interface/IInterpreterCallerV4.sol`** -- struct `EvaluableV4`, interface `IInterpreterCallerV4` +- event `ContextV2(address sender, bytes32[][] context)` -- line 47 + +**`src/interface/IInterpreterExternV4.sol`** -- types `EncodedExternDispatchV2`, `ExternDispatchV2`; interface `IInterpreterExternV4` +- `externIntegrity(ExternDispatchV2 dispatch, uint256 expectedInputs, uint256 expectedOutputs)` -- line 33 +- `extern(ExternDispatchV2 dispatch, StackItem[] calldata inputs)` -- line 43 + +**`src/interface/IInterpreterStoreV3.sol`** -- interface `IInterpreterStoreV3` +- event `Set(FullyQualifiedNamespace namespace, bytes32 key, bytes32 value)` -- line 36 +- `set(StateNamespace namespace, bytes32[] calldata kvs)` -- line 48 +- `get(FullyQualifiedNamespace namespace, bytes32 key)` -- line 65 + +**`src/interface/IInterpreterV4.sol`** -- types `OperandV2`, `StackItem`; struct `EvalV4`; interface `IInterpreterV4` +- `eval4(EvalV4 calldata eval)` -- line 101 + +**`src/interface/IParserV2.sol`** -- interface `IParserV2` +- `parse2(bytes calldata data)` -- line 10 + +**`src/interface/ISubParserV4.sol`** -- interface `ISubParserV4` +- `subParseLiteral2(bytes calldata data)` -- line 40 +- `subParseWord2(bytes calldata data)` -- line 68 + +**`src/interface/IParserPragmaV1.sol`** -- struct `PragmaV1`; interface `IParserPragmaV1` +- `parsePragma1(bytes calldata data)` -- line 10 + +### Config Files + +**`foundry.toml`** -- solc 0.8.25, optimizer enabled with 1M runs, cancun EVM, one remapping +**`slither.config.json`** -- excludes assembly-usage, solc-version, unused-imports, pragma detectors + +--- + +## Findings + +### A01 -- CRITICAL: Operator Precedence Bug in `LibParseMeta.lookupWord` + +**File:** `src/lib/parse/LibParseMeta.sol`, line 110 + +```solidity +if (wordFingerprint == posData & FINGERPRINT_MASK) { +``` + +In Solidity, the bitwise AND operator `&` has **lower** precedence than the equality operator `==`. This means the expression is parsed as: + +```solidity +if ((wordFingerprint == posData) & FINGERPRINT_MASK) { +``` + +rather than the clearly intended: + +```solidity +if (wordFingerprint == (posData & FINGERPRINT_MASK)) { +``` + +The `==` comparison returns a `bool` (0 or 1), which is then bitwise ANDed with `FINGERPRINT_MASK` (0xFFFFFF). Since `FINGERPRINT_MASK` has its lowest bit set, this means the result of `(wordFingerprint == posData) & FINGERPRINT_MASK` will be `1` when `wordFingerprint == posData` (full equality), and `0` otherwise. In Solidity, a non-zero `uint256` is truthy when used in an `if` condition. + +The practical consequence is that this match condition requires `wordFingerprint == posData` (full 256-bit equality) rather than only matching on the lower 3 bytes (fingerprint). This could cause valid lookups to fail if the upper bytes of `posData` differ from `wordFingerprint`, because `posData` contains the opcode index in byte 28 (the upper byte of the 4-byte item) while `wordFingerprint` only has the lower 3 bytes set. + +**However**, closer inspection reveals that `wordFingerprint` is set to `hashed & FINGERPRINT_MASK` (line 102), so its upper bytes are always zero. Meanwhile `posData` is loaded from the meta data where the upper byte (byte 28) contains the opcode index. So when the fingerprints match, `wordFingerprint != posData` because the opcode index byte differs. This means **lookups will almost always fail** (the only exception being opcode index 0 with a matching fingerprint). + +**Note:** It is possible the Solidity compiler catches this at compile time and emits a type error or warning since `==` returns `bool` and `&` expects integer operands, which would prevent compilation. If the code compiles successfully, the compiler may be implicitly converting the `bool` to `uint256` for the bitwise AND, which would exhibit the bug described above. Alternatively, the Solidity compiler for version 0.8.25 may error on this construct entirely. This needs to be verified with a build, but forge was not available in this environment. If the code does compile, this is a critical bug in word lookup. + +--- + +### 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). + +--- + +### A05 -- INFO: Inconsistent Pragma Versions Between Interface and Library Files + +**Files:** All files in `src/interface/` vs all files in `src/lib/` and `src/error/` + +Interface files use two different pragma versions: +- `pragma solidity ^0.8.18;` -- `IInterpreterCallerV4.sol`, `IInterpreterExternV4.sol`, `IInterpreterStoreV3.sol`, `IParserV2.sol`, `ISubParserV4.sol`, `IParserPragmaV1.sol` +- `pragma solidity ^0.8.25;` -- `IInterpreterV4.sol` + +Library and error files uniformly use: +- `pragma solidity ^0.8.25;` + +The `foundry.toml` pins compilation to `solc = "0.8.25"`. + +This is an intentional design pattern where interfaces use a lower minimum pragma to maximize compatibility for downstream consumers who import these interfaces, while libraries that are compiled directly use the pinned version. `IInterpreterV4.sol` is the exception among interfaces at `^0.8.25`, which breaks the otherwise consistent pattern across all other non-deprecated interface files. + +--- + +### A06 -- INFO: Minor Typos in Documentation Comments + +Multiple files contain minor typos in NatSpec/comments: + +| File | Line | Typo | Correction | +|------|------|------|------------| +| `src/lib/bytecode/LibBytecode.sol` | 169 | "implicity" | "implicitly" | +| `src/lib/bytecode/LibBytecode.sol` | 264 | "togther" | "together" | +| `src/lib/bytecode/LibBytecode.sol` | 302 | "legacly" | "legacy" | +| `src/lib/parse/LibParseMeta.sol` | 14 | "targetting" | "targeting" | +| `src/interface/IInterpreterV4.sol` | 60 | "prepoluated" | "prepopulated" | +| `src/error/ErrBytecode.sol` | 18 | "doesnt" | "doesn't" | + +--- + +## Summary + +| ID | Severity | Title | +|----|----------|-------| +| A01 | CRITICAL | Operator precedence bug in `LibParseMeta.lookupWord` -- `==` evaluated before `&` | +| A02 | LOW | Unused import and `using` statement for `LibUint256Array` in `LibContext.sol` | +| A03 | LOW | Stale `IInterpreterStoreV2` reference in `IInterpreterCallerV4` NatSpec | +| A04 | LOW | Discarded return values via no-op expressions in `LibGenParseMeta.sol` | +| A05 | INFO | Inconsistent pragma version on `IInterpreterV4.sol` vs other interface files | +| A06 | INFO | Minor typos in documentation comments across multiple files | diff --git a/audit/2026-03-01-01/triage.md b/audit/2026-03-01-01/triage.md new file mode 100644 index 0000000..6a61452 --- /dev/null +++ b/audit/2026-03-01-01/triage.md @@ -0,0 +1,65 @@ +# Triage — Audit 2026-03-01-01 + +## Pass 0: Process Review + +| ID | Severity | Title | Status | +|----|----------|-------|--------| +| A00-1 | LOW | CLAUDE.md says "Requires NixOS" but only Nix package manager is required | FIXED — Changed to "Requires the Nix package manager". | +| A00-2 | LOW | CLAUDE.md does not mention revert style conventions | FIXED — Added custom error convention note. | +| A00-3 | INFO | README.md says "Install `nix develop`" which is misleading | FIXED — Changed to "Install Nix". | +| A00-4 | INFO | CLAUDE.md doesn't state policy on modifying deprecated interfaces | FIXED — Added policy: deprecated interfaces should not be modified unless undeprecating. | +| A00-5 | INFO | No instruction on Solidity version policy for new vs. existing files | FIXED — Added pragma version policy for interfaces vs libraries. | + +## Pass 1: Security Review + +| ID | Severity | Title | Status | +|----|----------|-------|--------| +| P1-A01-2 | LOW | `bytecodeToSources` does not call `checkNoOOBPointers` internally | DOCUMENTED — Added natspec documenting that callers MUST checkNoOOBPointers before calling, plus @param/@return tags. | +| P1-A01-6 | LOW | `checkNoOOBPointers` does not enforce monotonically increasing offsets explicitly | DOCUMENTED — Added natspec noting contiguity implicitly enforces monotonicity. | +| P1-A02-4 | LOW | Reference implementation mismatch for zero signed contexts | FIXED — Fixed `buildStructureSlow` to conditionally omit signers column when zero signed contexts. Added zero-signed tests with empty and non-empty base. | +| P1-A03-1 | LOW | No input validation on `meta` bytes in `lookupWord` | DOCUMENTED — Added natspec documenting meta must be well-formed from buildParseMetaV2. Also fixes stale "io fn pointer" text (P3-A01-8). | +| P1-A03-2 | LOW | No bounds check on computed `pos` before data read in `lookupWord` | DISMISSED — Duplicate of P1-A03-1; same underlying issue, already covered by natspec documenting well-formedness precondition. | +| P1-A04-1 | LOW | Seed search loop skips seed value 255 | FIXED — Changed `<` to `<=` in loop condition. Added reference impl `LibGenParseMetaSlow.findBestExpanderSlow` and fuzz test verifying agreement across all 256 seeds. | +| P1-A04-2 | LOW | Opcode index silently truncated for word lists exceeding 255 entries | FIXED — Added `AuthoringMetaTooLarge` custom error and length check. Tests: boundary at 256 (succeeds), 257 (reverts), fuzz for 257-512 range. | +| P1-A06-1 | LOW | Stale documentation reference to `IInterpreterStoreV2` | FIXED — Changed `IInterpreterStoreV2` to `IInterpreterStoreV3` in natspec. | +| P1-A06-3 | LOW | `SourceIndexV2` uses `uint256` while V4 type system uses `bytes32` | DISMISSED — `SourceIndexV2` is defined in deprecated code; not actionable. | +| P1-A06-5 | MEDIUM | `EvalV4.namespace` is caller-controlled FullyQualifiedNamespace with no validation guidance | FIXED — Added natspec to `EvalV4` struct documenting that the interpreter MUST qualify the namespace itself via `LibNamespace.qualifyNamespace`. | +| P1-A06-8 | LOW | `PragmaV1.usingWordsFrom` has no length or duplication constraints | DOCUMENTED — Added natspec to PragmaV1 struct documenting field semantics and validation guidance. | +| P1-A06-9 | LOW | No interface guidance on `EvalV4.bytecode` validation or maximum size | DOCUMENTED — Added natspec recommending implementations validate bytecode via `LibBytecode.checkNoOOBPointers`. | + +## Pass 2: Test Coverage Review + +| ID | Severity | Title | Status | +|----|----------|-------|--------| +| P2-A01-1 | HIGH | `bytecodeToSources` has zero test coverage | FIXED — Added test file `test/src/lib/bytecode/LibBytecode.bytecodeToSources.t.sol` with 5 concrete tests + 1 fuzz test. All pass. | +| P2-A02-1 | LOW | No test triggers the `InvalidSignature` revert path in `build()` | FIXED — Added 4 tests: zero signature, second-index revert, wrong context data, empty signature bytes. All use external wrapper for vm.expectRevert. | +| P2-A02-2 | LOW | No test for `build()` with non-empty base context and zero signed contexts | FIXED — Addressed by P1-A02-4 fix. Added `testBuildStructureZeroSignedNonEmptyBase` fuzz test. | +| P2-A02-3 | LOW | Reference implementation `buildStructureSlow` disagrees with `build()` for zero signed contexts | FIXED — Addressed by P1-A02-4 fix. | +| P2-A03-1 | MEDIUM | `DuplicateFingerprint` revert path in `buildParseMetaV2` is never tested | FIXED — Added `testBuildMetaDuplicateFingerprint` test with external wrapper to trigger and verify the revert. | +| P2-A03-2 | MEDIUM | `parseMetaConstantString` function has no test coverage | FIXED — Added `testParseMetaConstantString` test exercising the abi.decode path and LibCodeGen integration. | +| P2-A03-3 | LOW | `lookupWord` has no dedicated test file and lacks edge-case coverage | FIXED — Created `LibParseMeta.lookupWord.t.sol` with 6 tests: known words, not-found, single-depth, fuzz not-found, fuzz roundtrip, and large (50-word) multi-depth set. | +| P2-A03-4 | LOW | `findBestExpander` is not tested with empty input or large collision-heavy input | FIXED — Added tests for empty, single word, large (64 elements), invariant fuzz, and reference comparison fuzz. | + +## Pass 3: Documentation Review + +| ID | Severity | Title | Status | +|----|----------|-------|--------| +| P3-A01-2 | LOW | `bytecodeToSources` missing `@param` and `@return` documentation | FIXED — Addressed by P1-A01-2 fix. Added @param and @return tags. | +| P3-A01-4 | LOW | `build` missing `@return` documentation | FIXED — Added @return tag describing the context matrix structure. | +| P3-A01-8 | LOW | `lookupWord` natspec mentions stale "io fn pointer" | FIXED — Addressed by P1-A03-1 fix. Replaced stale text with accurate description. | +| P3-A02-1 | LOW | `OperandV2` type has no natspec documentation | FIXED — Added @dev natspec. | +| P3-A02-2 | LOW | `StackItem` type has no natspec documentation | FIXED — Added @dev natspec. | +| P3-A02-3 | LOW | `EvalV4` struct has no natspec documentation | FIXED — Addressed by P1-A06-5 fix. Added full natspec with `@dev` and `@param` tags for all 7 fields. | +| P3-A02-4 | LOW | `eval4` function lacks `@param` and `@return` natspec tags | FIXED — Added @param eval, @return stack, @return writes tags. | +| P3-A02-5 | LOW | `IParserV2` interface and `parse2` function have no natspec | FIXED — Added @title, @notice, @param, @return natspec. | +| P3-A02-6 | LOW | `IParserPragmaV1` interface and `parsePragma1` have no natspec | FIXED — Added @title, @notice, @param, @return natspec. | +| P3-A02-7 | LOW | `PragmaV1` struct has no natspec documentation | FIXED — Addressed by P1-A06-8 fix. | + +## Pass 4: Code Quality Review + +| ID | Severity | Title | Status | +|----|----------|-------|--------| +| P4-A01 | ~~CRITICAL~~ | ~~Operator precedence bug in `lookupWord`~~ | DISMISSED — Verified incorrect: Solidity `&` (level 7) binds tighter than `==` (level 11). Expression is parsed as intended. | +| P4-A02 | LOW | Unused import and `using` for `LibUint256Array` in `LibContext.sol` | FIXED — Removed unused import and using directive. | +| P4-A03 | LOW | Stale `IInterpreterStoreV2` reference in `IInterpreterCallerV4` NatSpec | FIXED — Duplicate of P1-A06-1, addressed by same fix. | +| P4-A04 | LOW | Discarded return values via no-op `(hashed);` in `LibGenParseMeta.sol` | FIXED — Changed to idiomatic `(uint256 shifted,)` destructuring pattern. | diff --git a/src/interface/IInterpreterCallerV4.sol b/src/interface/IInterpreterCallerV4.sol index 66c88f0..d66fe80 100644 --- a/src/interface/IInterpreterCallerV4.sol +++ b/src/interface/IInterpreterCallerV4.sol @@ -35,7 +35,7 @@ struct EvaluableV4 { /// - Provide the context, which can be built in a standard way by `LibContext` /// - Handle the stack array returned from `eval4` /// - OPTIONALLY emit the `Context` event -/// - OPTIONALLY set state on the associated `IInterpreterStoreV2`. +/// - OPTIONALLY set state on the associated `IInterpreterStoreV3`. interface IInterpreterCallerV4 { /// Calling contracts SHOULD emit `Context` before calling `eval4` if they /// are able. Notably `eval4` MAY be called within a static call which means diff --git a/src/interface/IInterpreterV4.sol b/src/interface/IInterpreterV4.sol index 5571bb1..f7a4059 100644 --- a/src/interface/IInterpreterV4.sol +++ b/src/interface/IInterpreterV4.sol @@ -36,10 +36,27 @@ import { } from "./deprecated/v2/IInterpreterV3.sol"; import {IInterpreterStoreV3} from "./IInterpreterStoreV3.sol"; +/// @dev Operand for an opcode in the interpreter. Encoded as `bytes32` to +/// allow opcode-specific interpretation of the full 32-byte value. type OperandV2 is bytes32; +/// @dev A single item on the interpreter stack. Encoded as `bytes32` to hold +/// arbitrary 32-byte values, including packed Rain decimal floats. type StackItem is bytes32; +/// @dev Parameters for a single evaluation of Rainlang bytecode. +/// @param store The store to read/write state from/to. +/// @param namespace The fully qualified namespace for state reads during eval. +/// The interpreter MUST qualify this namespace itself using +/// `LibNamespace.qualifyNamespace` with `msg.sender`. Implementations MUST NOT +/// trust caller-provided values as pre-qualified — the type name is descriptive +/// of the output, not an assertion about the input. +/// @param bytecode The Rainlang bytecode to evaluate. +/// @param sourceIndex The index of the source within the bytecode to evaluate. +/// @param context The context matrix available to the evaluated logic. +/// @param inputs Pre-populated stack items available to the evaluated logic. +/// @param stateOverlay State overrides applied before evaluation for "what if" +/// analysis. Format is implementation-defined (e.g. pairwise key/value). struct EvalV4 { IInterpreterStoreV3 store; FullyQualifiedNamespace namespace; @@ -92,11 +109,18 @@ interface IInterpreterV4 { /// Pass Rainlang bytecode in calldata, and get back the stack and storage /// writes. /// + /// Implementations SHOULD validate bytecode structure (e.g. via + /// `LibBytecode.checkNoOOBPointers`) before execution. + /// /// Key differences in `eval4`: /// - Supports state overlays to facilitate "what if" analysis. Each item /// of state in the overlay will override corresponding gets from the store /// unless/until they are set to something else in the evaluated logic. /// - Numbers are treated as packed Rain decimal floats, NOT fixed point /// decimals. + /// @param eval The eval configuration specifying bytecode, store, namespace, + /// context, inputs, and state overlay. + /// @return stack The output stack items from evaluating the specified source. + /// @return writes Key-value pairs to be applied to the store by the caller. function eval4(EvalV4 calldata eval) external view returns (StackItem[] calldata stack, bytes32[] calldata writes); } diff --git a/src/interface/IParserPragmaV1.sol b/src/interface/IParserPragmaV1.sol index a9f0a5d..bc2ee9f 100644 --- a/src/interface/IParserPragmaV1.sol +++ b/src/interface/IParserPragmaV1.sol @@ -2,10 +2,19 @@ // SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd pragma solidity ^0.8.18; +/// @dev Pragma configuration for a parser. +/// @param usingWordsFrom Addresses of sub-parser contracts whose words should +/// be available during parsing. Implementations SHOULD validate entries +/// (reject zero addresses, handle duplicates). struct PragmaV1 { address[] usingWordsFrom; } +/// @title IParserPragmaV1 +/// @notice Interface for extracting pragma directives from Rainlang source. interface IParserPragmaV1 { + /// Parses pragma directives from Rainlang source data. + /// @param data The Rainlang source to extract pragmas from. + /// @return The parsed pragma containing any `usingWordsFrom` addresses. function parsePragma1(bytes calldata data) external view returns (PragmaV1 calldata); } diff --git a/src/interface/IParserV2.sol b/src/interface/IParserV2.sol index 08b1b5b..690297d 100644 --- a/src/interface/IParserV2.sol +++ b/src/interface/IParserV2.sol @@ -6,6 +6,12 @@ pragma solidity ^0.8.18; //forge-lint: disable-next-line(unused-import) import {AuthoringMetaV2} from "./deprecated/v1/IParserV1.sol"; +/// @title IParserV2 +/// @notice Interface for parsing Rainlang source into interpreter bytecode. interface IParserV2 { + /// Parses Rainlang source data into bytecode compatible with + /// `IInterpreterV4.eval4`. MUST revert if the input is not valid Rainlang. + /// @param data The Rainlang source to parse. + /// @return bytecode The compiled bytecode ready for evaluation. function parse2(bytes calldata data) external view returns (bytes calldata bytecode); } diff --git a/src/lib/bytecode/LibBytecode.sol b/src/lib/bytecode/LibBytecode.sol index 39609bd..ef15641 100644 --- a/src/lib/bytecode/LibBytecode.sol +++ b/src/lib/bytecode/LibBytecode.sol @@ -66,6 +66,8 @@ library LibBytecode { /// - The number of opcodes specified in the header of each source locates /// the end of the source exactly at either the offset of the next source /// or the end of the bytecode `bytes`. + /// Monotonically increasing offsets are implicitly enforced by contiguity: + /// each source must end exactly where the next begins. //forge-lint: disable-next-line(mixed-case-function) function checkNoOOBPointers(bytes memory bytecode) internal pure { unchecked { @@ -288,6 +290,10 @@ library LibBytecode { /// Requires allocation and copying so it isn't particularly efficient, but /// allows us to use the new bytecode format with old interpreter code. Not /// recommended for production code but useful for testing. + /// Callers MUST `checkNoOOBPointers` BEFORE calling this function, + /// otherwise memory outside the bytecode `bytes` MAY be read. + /// @param bytecode The bytecode to convert to legacy source format. + /// @return The individual source arrays in legacy format. function bytecodeToSources(bytes memory bytecode) internal pure returns (bytes[] memory) { unchecked { uint256 count = sourceCount(bytecode); diff --git a/src/lib/caller/LibContext.sol b/src/lib/caller/LibContext.sol index 5e1fc69..fa8913a 100644 --- a/src/lib/caller/LibContext.sol +++ b/src/lib/caller/LibContext.sol @@ -2,7 +2,6 @@ // SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd pragma solidity ^0.8.25; -import {LibUint256Array} from "rain.solmem/lib/LibUint256Array.sol"; import {LibHashNoAlloc, HASH_NIL} from "rain.lib.hash/LibHashNoAlloc.sol"; import {SignatureChecker} from "openzeppelin-contracts/contracts/utils/cryptography/SignatureChecker.sol"; @@ -43,8 +42,6 @@ uint256 constant CONTEXT_BASE_ROW_CALLING_CONTRACT = 1; /// closer to compatibility and portability, inheriting network effects of what /// has already been authored elsewhere. library LibContext { - using LibUint256Array for uint256[]; - /// The base context is the `msg.sender` and address of the calling contract. /// As the interpreter itself is called via an external interface and may be /// statically calling itself, it MAY NOT have any ability to inspect either @@ -163,6 +160,10 @@ library LibContext { /// position that would force signed context to be provided in the "correct" /// order, rather than relying on the `msg.sender` to honestly present data /// in any particular structure/order. + /// @return The fully assembled context matrix. Column 0 is the base context + /// from `LibContext.base()`. Subsequent columns are from `baseContext`, + /// followed by a signers column (each signer address cast to `bytes32`), + /// followed by each signed context's data as individual columns. function build(bytes32[][] memory baseContext, SignedContextV1[] memory signedContexts) internal view diff --git a/src/lib/codegen/LibGenParseMeta.sol b/src/lib/codegen/LibGenParseMeta.sol index c75fefb..cffe9d0 100644 --- a/src/lib/codegen/LibGenParseMeta.sol +++ b/src/lib/codegen/LibGenParseMeta.sol @@ -20,6 +20,10 @@ uint256 constant META_ITEM_MASK = (1 << META_ITEM_SIZE) - 1; /// @dev For metadata builder. error DuplicateFingerprint(); +/// @dev Thrown when the authoring meta exceeds 256 words. Opcode indices are +/// stored in a single byte, so more than 256 words cannot be represented. +error AuthoringMetaTooLarge(uint256 length); + /// @title LibGenParseMeta /// @notice Library for building parse meta from authoring meta, and generating /// constant strings for the parse meta to be used in generated code. The parse @@ -54,11 +58,10 @@ library LibGenParseMeta { unchecked { { uint256 bestCt = 0; - for (uint256 seed = 0; seed < type(uint8).max; seed++) { + for (uint256 seed = 0; seed <= type(uint8).max; seed++) { uint256 expansion = 0; for (uint256 i = 0; i < metas.length; i++) { - (uint256 shifted, uint256 hashed) = LibParseMeta.wordBitmapped(seed, metas[i].word); - (hashed); + (uint256 shifted,) = LibParseMeta.wordBitmapped(seed, metas[i].word); expansion = shifted | expansion; } uint256 ct = LibCtPop.ctpop(expansion); @@ -85,8 +88,7 @@ library LibGenParseMeta { uint256 usedExpansion = 0; uint256 j = 0; for (uint256 i = 0; i < metas.length; i++) { - (uint256 shifted, uint256 hashed) = LibParseMeta.wordBitmapped(bestSeed, metas[i].word); - (hashed); + (uint256 shifted,) = LibParseMeta.wordBitmapped(bestSeed, metas[i].word); if ((shifted & usedExpansion) == 0) { usedExpansion = shifted | usedExpansion; } else { @@ -129,6 +131,11 @@ library LibGenParseMeta { returns (bytes memory parseMeta) { unchecked { + // Opcode index is stored in a single byte, so we cannot handle + // more than 256 words. + if (authoringMeta.length > 256) { + revert AuthoringMetaTooLarge(authoringMeta.length); + } // Write out expansions. uint8[] memory seeds; uint256[] memory expansions; diff --git a/src/lib/parse/LibParseMeta.sol b/src/lib/parse/LibParseMeta.sol index 8d03894..0481ea2 100644 --- a/src/lib/parse/LibParseMeta.sol +++ b/src/lib/parse/LibParseMeta.sol @@ -57,9 +57,12 @@ library LibParseMeta { } } - /// Given the parse meta and a word, return the index and io fn pointer for - /// the word. If the word is not found, then `exists` will be false. The - /// caller MUST check `exists` before using the other return values. + /// Given the parse meta and a word, return whether the word exists and its + /// index. If the word is not found, then `exists` will be false. The caller + /// MUST check `exists` before using the other return values. + /// The `meta` parameter MUST be well-formed as produced by + /// `LibGenParseMeta.buildParseMetaV2`. Behavior is undefined for malformed + /// meta — no bounds checking is performed on the meta structure. /// @param meta The parser meta. /// @param word The word to lookup. /// @return True if the word exists in the parse meta. diff --git a/test/src/lib/bytecode/LibBytecode.bytecodeToSources.t.sol b/test/src/lib/bytecode/LibBytecode.bytecodeToSources.t.sol new file mode 100644 index 0000000..0dfe6a8 --- /dev/null +++ b/test/src/lib/bytecode/LibBytecode.bytecodeToSources.t.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {BytecodeTest} from "test/abstract/BytecodeTest.sol"; +import {LibBytecode} from "src/lib/bytecode/LibBytecode.sol"; + +contract LibBytecodeBytecodeToSourcesTest is BytecodeTest { + /// Zero sources should return an empty array. + function testBytecodeToSourcesZeroSources() external pure { + bytes[] memory sources = LibBytecode.bytecodeToSources(hex"00"); + assertEq(sources.length, 0); + } + + /// Empty bytecode should return an empty array. + function testBytecodeToSourcesEmpty() external pure { + bytes[] memory sources = LibBytecode.bytecodeToSources(hex""); + assertEq(sources.length, 0); + } + + /// Single source with zero opcodes should return one empty bytes. + function testBytecodeToSourcesOneSourceZeroOps() external pure { + // 1 source, offset 0, header: 0 ops, 0 alloc, 0 inputs, 0 outputs + bytes[] memory sources = LibBytecode.bytecodeToSources(hex"01000000000000"); + assertEq(sources.length, 1); + assertEq(sources[0].length, 0); + } + + /// Single source with one opcode. Verify the byte-shuffling: + /// new format byte 0 (opcode index) moves to byte 1, byte 0 becomes 0. + function testBytecodeToSourcesOneSourceOneOp() external pure { + // 1 source, offset 0, header: 1 op, 1 alloc, 0 inputs, 1 output + // opcode: [0xAB, 0xCD, 0xEF, 0x12] + bytes[] memory sources = LibBytecode.bytecodeToSources(hex"01000001010001ABCDEF12"); + assertEq(sources.length, 1); + assertEq(sources[0].length, 4); + // After shuffle: byte 0 (0xAB) moves to byte 1, byte 0 becomes 0 + assertEq(uint8(sources[0][0]), 0x00); + assertEq(uint8(sources[0][1]), 0xAB); + assertEq(uint8(sources[0][2]), 0xEF); + assertEq(uint8(sources[0][3]), 0x12); + } + + /// Multiple sources with varying op counts. + function testBytecodeToSourcesMultipleSources() external pure { + // 2 sources + // source 0: offset 0, 1 op, header [01, 01, 00, 01], opcode [AA, BB, CC, DD] + // source 1: offset 8, 0 ops, header [00, 00, 00, 00] + bytes[] memory sources = LibBytecode.bytecodeToSources(hex"020000000801010001AABBCCDD00000000"); + assertEq(sources.length, 2); + assertEq(sources[0].length, 4); + assertEq(sources[1].length, 0); + // Verify byte-shuffling on source 0 + assertEq(uint8(sources[0][0]), 0x00); + assertEq(uint8(sources[0][1]), 0xAA); + } + + /// Fuzz: bytecodeToSources should produce one source per sourceCount, + /// and each source length should equal opsCount * 4. + function testBytecodeToSourcesFuzz(bytes memory bytecode, uint256 sourceCount, bytes32 seed) external pure { + conformBytecode(bytecode, sourceCount, seed); + LibBytecode.checkNoOOBPointers(bytecode); + sourceCount = LibBytecode.sourceCount(bytecode); + + bytes[] memory sources = LibBytecode.bytecodeToSources(bytecode); + assertEq(sources.length, sourceCount); + + for (uint256 i = 0; i < sourceCount; i++) { + assertEq(sources[i].length, LibBytecode.sourceOpsCount(bytecode, i) * 4); + } + } +} diff --git a/test/src/lib/caller/LibContext.t.sol b/test/src/lib/caller/LibContext.t.sol index f800cbd..a4f58d8 100644 --- a/test/src/lib/caller/LibContext.t.sol +++ b/test/src/lib/caller/LibContext.t.sol @@ -3,10 +3,24 @@ pragma solidity =0.8.25; import {Test} from "forge-std/Test.sol"; -import {LibContext, MessageHashUtils, LibHashNoAlloc, SignedContextV1} from "src/lib/caller/LibContext.sol"; +import { + LibContext, + MessageHashUtils, + LibHashNoAlloc, + SignedContextV1, + InvalidSignature +} from "src/lib/caller/LibContext.sol"; import {LibContextSlow} from "./LibContextSlow.sol"; contract LibContextTest is Test { + function buildExternal(bytes32[][] memory baseContext, SignedContextV1[] memory signedContexts) + external + view + returns (bytes32[][] memory) + { + return LibContext.build(baseContext, signedContexts); + } + function testBase() public view { bytes32[] memory baseContext = LibContext.base(); @@ -48,8 +62,116 @@ contract LibContextTest is Test { } } + /// Zero signed contexts with empty base: reference must match production. + function testBuildStructureZeroSignedEmptyBase() public view { + bytes32[][] memory base = new bytes32[][](0); + SignedContextV1[] memory signedContexts = new SignedContextV1[](0); + + bytes32[][] memory expected = LibContextSlow.buildStructureSlow(base, signedContexts); + bytes32[][] memory actual = LibContext.build(base, signedContexts); + assertEq(expected.length, actual.length, "length mismatch"); + + for (uint256 i = 0; i < expected.length; i++) { + assertEq(expected[i], actual[i]); + } + } + + /// Zero signed contexts with non-empty base: reference must match + /// production. The result should have 1 (base context) + base.length + /// columns and no signers column. + /// forge-config: default.fuzz.runs = 100 + function testBuildStructureZeroSignedNonEmptyBase(bytes32[][] memory base) public view { + SignedContextV1[] memory signedContexts = new SignedContextV1[](0); + + bytes32[][] memory expected = LibContextSlow.buildStructureSlow(base, signedContexts); + bytes32[][] memory actual = LibContext.build(base, signedContexts); + assertEq(expected.length, actual.length, "length mismatch"); + assertEq(actual.length, 1 + base.length, "should be base context + base columns only"); + + for (uint256 i = 0; i < expected.length; i++) { + assertEq(expected[i], actual[i]); + } + } + + /// A completely invalid signature should revert with InvalidSignature(0). + function testBuildInvalidSignatureReverts() public { + bytes32[] memory ctx = new bytes32[](1); + ctx[0] = bytes32(uint256(42)); + + SignedContextV1[] memory signedContexts = new SignedContextV1[](1); + signedContexts[0] = SignedContextV1({ + signer: address(0xdead), + context: ctx, + signature: hex"0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" + }); + + vm.expectRevert(abi.encodeWithSelector(InvalidSignature.selector, uint256(0))); + this.buildExternal(new bytes32[][](0), signedContexts); + } + + /// First signature valid, second invalid — should revert with + /// InvalidSignature(1), not InvalidSignature(0). + function testBuildInvalidSignatureSecondIndex() public { + // Use vm.sign for a proper first signature. + uint256 signerPk = 0xA11CE; + address signer = vm.addr(signerPk); + + bytes32[] memory ctx = new bytes32[](1); + ctx[0] = bytes32(uint256(1)); + + bytes32 digest = MessageHashUtils.toEthSignedMessageHash(LibHashNoAlloc.hashWords(ctx)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPk, digest); + bytes memory validSig = abi.encodePacked(r, s, v); + + SignedContextV1[] memory signedContexts = new SignedContextV1[](2); + signedContexts[0] = SignedContextV1({signer: signer, context: ctx, signature: validSig}); + signedContexts[1] = SignedContextV1({ + signer: address(0xdead), + context: ctx, + signature: hex"0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" + }); + + vm.expectRevert(abi.encodeWithSelector(InvalidSignature.selector, uint256(1))); + this.buildExternal(new bytes32[][](0), signedContexts); + } + + /// Valid signature but for wrong context data — signer signed different + /// data than what is presented. + function testBuildInvalidSignatureWrongContext() public { + uint256 signerPk = 0xB0B; + address signer = vm.addr(signerPk); + + bytes32[] memory signedCtx = new bytes32[](1); + signedCtx[0] = bytes32(uint256(1)); + + bytes32 digest = MessageHashUtils.toEthSignedMessageHash(LibHashNoAlloc.hashWords(signedCtx)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPk, digest); + bytes memory sig = abi.encodePacked(r, s, v); + + // Present different context than what was signed. + bytes32[] memory differentCtx = new bytes32[](1); + differentCtx[0] = bytes32(uint256(999)); + + SignedContextV1[] memory signedContexts = new SignedContextV1[](1); + signedContexts[0] = SignedContextV1({signer: signer, context: differentCtx, signature: sig}); + + vm.expectRevert(abi.encodeWithSelector(InvalidSignature.selector, uint256(0))); + this.buildExternal(new bytes32[][](0), signedContexts); + } + + /// Empty signature bytes should revert. + function testBuildInvalidSignatureEmpty() public { + bytes32[] memory ctx = new bytes32[](1); + ctx[0] = bytes32(uint256(1)); + + SignedContextV1[] memory signedContexts = new SignedContextV1[](1); + signedContexts[0] = SignedContextV1({signer: address(0xdead), context: ctx, signature: ""}); + + vm.expectRevert(abi.encodeWithSelector(InvalidSignature.selector, uint256(0))); + this.buildExternal(new bytes32[][](0), signedContexts); + } + function testBuild0() public view { - // @todo test this better. bytes32[][] memory expected = new bytes32[][](1); expected[0] = LibContext.base(); bytes32[][] memory built = LibContext.build(new bytes32[][](0), new SignedContextV1[](0)); diff --git a/test/src/lib/caller/LibContextSlow.sol b/test/src/lib/caller/LibContextSlow.sol index a60e546..33d964a 100644 --- a/test/src/lib/caller/LibContextSlow.sol +++ b/test/src/lib/caller/LibContextSlow.sol @@ -36,7 +36,8 @@ library LibContextSlow { view returns (bytes32[][] memory) { - bytes32[][] memory context = new bytes32[][](1 + baseContext.length + 1 + signedContexts.length); + uint256 signedLen = signedContexts.length > 0 ? signedContexts.length + 1 : 0; + bytes32[][] memory context = new bytes32[][](1 + baseContext.length + signedLen); context[0] = new bytes32[](2); context[0][0] = bytes32(uint256(uint160(address(msg.sender)))); context[0][1] = bytes32(uint256(uint160(address(this)))); @@ -48,16 +49,18 @@ library LibContextSlow { } offset = offset + i; - bytes32[] memory signers = new bytes32[](signedContexts.length); - for (i = 0; i < signedContexts.length; i++) { - signers[i] = bytes32(uint256(uint160(signedContexts[i].signer))); - } - context[offset] = signers; - offset = offset + 1; + if (signedContexts.length > 0) { + bytes32[] memory signers = new bytes32[](signedContexts.length); + for (i = 0; i < signedContexts.length; i++) { + signers[i] = bytes32(uint256(uint160(signedContexts[i].signer))); + } + context[offset] = signers; + offset = offset + 1; - i = 0; - for (; i < signedContexts.length; i++) { - context[i + offset] = signedContexts[i].context; + i = 0; + for (; i < signedContexts.length; i++) { + context[i + offset] = signedContexts[i].context; + } } return context; diff --git a/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol b/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol index d0ba092..2fa56bf 100644 --- a/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol +++ b/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol @@ -6,10 +6,18 @@ import {Test} from "forge-std/Test.sol"; import {LibParseMeta} from "src/lib/parse/LibParseMeta.sol"; import {LibAuthoringMeta, AuthoringMetaV2} from "test/lib/meta/LibAuthoringMeta.sol"; -import {LibGenParseMeta} from "src/lib/codegen/LibGenParseMeta.sol"; +import {LibGenParseMeta, DuplicateFingerprint, AuthoringMetaTooLarge} from "src/lib/codegen/LibGenParseMeta.sol"; import {LibBloom} from "test/lib/bloom/LibBloom.sol"; contract LibGenParseMetaBuildMetaTest is Test { + function buildParseMetaV2External(AuthoringMetaV2[] memory authoringMeta, uint8 maxDepth) + external + pure + returns (bytes memory) + { + return LibGenParseMeta.buildParseMetaV2(authoringMeta, maxDepth); + } + /// This is super loose from limited empirical testing. function expanderDepth(uint256 n) internal pure returns (uint8) { // Number of fully saturated expanders @@ -70,4 +78,128 @@ contract LibGenParseMetaBuildMetaTest is Test { assertTrue(!notExists, "notExists"); assertEq(0, l, "l"); } + + function testBuildMetaDuplicateFingerprint() external { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](2); + metas[0] = AuthoringMetaV2({word: bytes32(uint256(1)), description: "a"}); + metas[1] = AuthoringMetaV2({word: bytes32(uint256(1)), description: "b"}); + + vm.expectRevert(abi.encodeWithSelector(DuplicateFingerprint.selector)); + this.buildParseMetaV2External(metas, 3); + } + + /// 257 words should revert with AuthoringMetaTooLarge. + function testBuildMetaTooLarge257() external { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](257); + for (uint256 i = 0; i < 257; i++) { + metas[i] = AuthoringMetaV2({word: bytes32(i), description: ""}); + } + vm.expectRevert(abi.encodeWithSelector(AuthoringMetaTooLarge.selector, 257)); + this.buildParseMetaV2External(metas, 8); + } + + /// Fuzz: any length above 256 should revert with AuthoringMetaTooLarge. + function testBuildMetaTooLargeFuzz(uint256 length) external { + length = bound(length, 257, 512); + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](length); + for (uint256 i = 0; i < length; i++) { + metas[i] = AuthoringMetaV2({word: bytes32(i), description: ""}); + } + vm.expectRevert(abi.encodeWithSelector(AuthoringMetaTooLarge.selector, length)); + this.buildParseMetaV2External(metas, 8); + } + + /// Exactly 256 words should succeed (boundary). + function testBuildMeta256Words() external pure { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](256); + for (uint256 i = 0; i < 256; i++) { + metas[i] = AuthoringMetaV2({word: bytes32(i), description: ""}); + } + bytes memory meta = LibGenParseMeta.buildParseMetaV2(metas, 8); + assertTrue(meta.length > 0); + } + + /// The generated parse meta from parseMetaConstantString should be + /// functionally equivalent to calling buildParseMetaV2 directly. Verify + /// by building both ways and checking all words can be looked up with + /// correct indices. + function testParseMetaConstantStringRoundtrip() external pure { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](3); + metas[0] = AuthoringMetaV2({word: bytes32("add"), description: "Add two numbers"}); + metas[1] = AuthoringMetaV2({word: bytes32("sub"), description: "Subtract"}); + metas[2] = AuthoringMetaV2({word: bytes32("mul"), description: "Multiply"}); + + bytes memory encoded = abi.encode(metas); + string memory result = LibGenParseMeta.parseMetaConstantString(vm, encoded, 3); + + // The function should produce non-empty output. + assertTrue(bytes(result).length > 0); + + // Verify the underlying parse meta is functional by building it + // directly and checking all words resolve. + bytes memory parseMeta = LibGenParseMeta.buildParseMetaV2(metas, 3); + for (uint256 i = 0; i < metas.length; i++) { + (bool exists, uint256 index) = LibParseMeta.lookupWord(parseMeta, metas[i].word); + assertTrue(exists, "word should exist"); + assertEq(index, i, "word index mismatch"); + } + } + + /// Empty authoring meta should produce a valid (non-empty) constant string. + function testParseMetaConstantStringEmpty() external pure { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](0); + bytes memory encoded = abi.encode(metas); + string memory result = LibGenParseMeta.parseMetaConstantString(vm, encoded, 1); + + assertTrue(bytes(result).length > 0); + } + + /// Single word should produce a valid constant string and the parse meta + /// built internally should correctly look up that word. + function testParseMetaConstantStringSingleWord() external pure { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](1); + metas[0] = AuthoringMetaV2({word: bytes32("only"), description: "The only word"}); + + bytes memory encoded = abi.encode(metas); + string memory result = LibGenParseMeta.parseMetaConstantString(vm, encoded, 1); + + assertTrue(bytes(result).length > 0); + + bytes memory parseMeta = LibGenParseMeta.buildParseMetaV2(metas, 1); + (bool exists, uint256 index) = LibParseMeta.lookupWord(parseMeta, bytes32("only")); + assertTrue(exists); + assertEq(index, 0); + } + + /// Different build depths should all produce valid output for the same + /// input, and the underlying parse meta should remain functional. + function testParseMetaConstantStringBuildDepths() external pure { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](2); + metas[0] = AuthoringMetaV2({word: bytes32("foo"), description: ""}); + metas[1] = AuthoringMetaV2({word: bytes32("bar"), description: ""}); + + bytes memory encoded = abi.encode(metas); + + for (uint8 depth = 1; depth <= 5; depth++) { + string memory result = LibGenParseMeta.parseMetaConstantString(vm, encoded, depth); + assertTrue(bytes(result).length > 0); + + bytes memory parseMeta = LibGenParseMeta.buildParseMetaV2(metas, depth); + for (uint256 i = 0; i < metas.length; i++) { + (bool exists, uint256 index) = LibParseMeta.lookupWord(parseMeta, metas[i].word); + assertTrue(exists); + assertEq(index, i); + } + } + } + + /// Fuzz: parseMetaConstantString should not revert for any valid + /// (no duplicate words) authoring meta. + function testParseMetaConstantStringFuzz(AuthoringMetaV2[] memory authoringMeta) external pure { + vm.assume(!LibBloom.bloomFindsDupes(LibAuthoringMeta.copyWordsFromAuthoringMeta(authoringMeta))); + bytes memory encoded = abi.encode(authoringMeta); + string memory result = + LibGenParseMeta.parseMetaConstantString(vm, encoded, expanderDepth(authoringMeta.length)); + assertTrue(bytes(result).length > 0); + } } diff --git a/test/src/lib/codegen/LibGenParseMeta.findExpander.t.sol b/test/src/lib/codegen/LibGenParseMeta.findExpander.t.sol index b9c33db..c94eb06 100644 --- a/test/src/lib/codegen/LibGenParseMeta.findExpander.t.sol +++ b/test/src/lib/codegen/LibGenParseMeta.findExpander.t.sol @@ -8,6 +8,7 @@ import {LibBloom} from "test/lib/bloom/LibBloom.sol"; import {LibCtPop} from "rain.math.binary/lib/LibCtPop.sol"; import {LibAuthoringMeta, AuthoringMetaV2} from "test/lib/meta/LibAuthoringMeta.sol"; import {LibGenParseMeta} from "src/lib/codegen/LibGenParseMeta.sol"; +import {LibGenParseMetaSlow} from "test/src/lib/codegen/LibGenParseMetaSlow.sol"; /// @title LibGenParseMetaFindExpanderTest /// Test that we can find reasonable expansions in a reasonable number of @@ -31,4 +32,61 @@ contract LibGenParseMetaFindExpanderTest is Test { assertEq(LibCtPop.ctpop(expansion), authoringMeta.length); assertEq(remaining.length, 0); } + + /// Empty input should return bestSeed 0, empty expansion, empty remaining. + function testFindExpanderEmpty() external pure { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](0); + (uint8 bestSeed, uint256 bestExpansion, AuthoringMetaV2[] memory remaining) = + LibGenParseMeta.findBestExpander(metas); + assertEq(bestSeed, 0); + assertEq(bestExpansion, 0); + assertEq(remaining.length, 0); + } + + /// Large input (64 elements) forces non-empty remaining array due to + /// bloom filter collisions. + function testFindExpanderLarge() external pure { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](64); + for (uint256 i = 0; i < 64; i++) { + metas[i] = AuthoringMetaV2({word: bytes32(i), description: ""}); + } + (, uint256 bestExpansion, AuthoringMetaV2[] memory remaining) = + LibGenParseMeta.findBestExpander(metas); + uint256 expandedCount = LibCtPop.ctpop(bestExpansion); + assertEq(remaining.length, 64 - expandedCount); + } + + /// Single word should always get a perfect expansion with no remaining. + function testFindExpanderSingleWord(bytes32 word) external pure { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](1); + metas[0] = AuthoringMetaV2({word: word, description: ""}); + (, uint256 bestExpansion, AuthoringMetaV2[] memory remaining) = + LibGenParseMeta.findBestExpander(metas); + assertEq(LibCtPop.ctpop(bestExpansion), 1); + assertEq(remaining.length, 0); + } + + /// Fuzz: the invariant expandedCount + remaining.length == metas.length + /// must always hold. + function testFindExpanderInvariant(AuthoringMetaV2[] memory authoringMeta) external pure { + vm.assume(!LibBloom.bloomFindsDupes(LibAuthoringMeta.copyWordsFromAuthoringMeta(authoringMeta))); + (, uint256 bestExpansion, AuthoringMetaV2[] memory remaining) = + LibGenParseMeta.findBestExpander(authoringMeta); + uint256 expandedCount = LibCtPop.ctpop(bestExpansion); + assertEq(expandedCount + remaining.length, authoringMeta.length); + } + + /// Fuzz: findBestExpander must agree with the reference implementation + /// that searches all 256 seeds. + function testFindExpanderMatchesReference(AuthoringMetaV2[] memory authoringMeta) external pure { + vm.assume(authoringMeta.length > 0); + vm.assume(authoringMeta.length <= 0x20); + vm.assume(!LibBloom.bloomFindsDupes(LibAuthoringMeta.copyWordsFromAuthoringMeta(authoringMeta))); + + (uint8 refBestSeed, uint256 refBestCt) = LibGenParseMetaSlow.findBestExpanderSlow(authoringMeta); + + (uint8 bestSeed, uint256 bestExpansion,) = LibGenParseMeta.findBestExpander(authoringMeta); + assertEq(bestSeed, refBestSeed, "seed mismatch"); + assertEq(LibCtPop.ctpop(bestExpansion), refBestCt, "expansion count mismatch"); + } } diff --git a/test/src/lib/codegen/LibGenParseMetaSlow.sol b/test/src/lib/codegen/LibGenParseMetaSlow.sol new file mode 100644 index 0000000..e13b7e3 --- /dev/null +++ b/test/src/lib/codegen/LibGenParseMetaSlow.sol @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity ^0.8.25; + +import {LibParseMeta} from "src/lib/parse/LibParseMeta.sol"; +import {LibCtPop} from "rain.math.binary/lib/LibCtPop.sol"; +import {AuthoringMetaV2} from "src/interface/IParserV2.sol"; + +library LibGenParseMetaSlow { + /// Reference implementation of findBestExpander that searches all 256 seeds + /// (0 through 255 inclusive). Returns only the best seed and its popcount + /// so tests can compare against the optimised implementation. + function findBestExpanderSlow(AuthoringMetaV2[] memory metas) + internal + pure + returns (uint8 bestSeed, uint256 bestCt) + { + for (uint256 seed = 0; seed <= type(uint8).max; seed++) { + uint256 expansion = 0; + for (uint256 i = 0; i < metas.length; i++) { + (uint256 shifted,) = LibParseMeta.wordBitmapped(seed, metas[i].word); + expansion = shifted | expansion; + } + uint256 ct = LibCtPop.ctpop(expansion); + if (ct > bestCt) { + bestCt = ct; + //forge-lint: disable-next-line(unsafe-typecast) + bestSeed = uint8(seed); + } + // Perfect expansion — no need to keep searching. + if (ct == metas.length) { + break; + } + } + } +} diff --git a/test/src/lib/parse/LibParseMeta.lookupWord.t.sol b/test/src/lib/parse/LibParseMeta.lookupWord.t.sol new file mode 100644 index 0000000..8946257 --- /dev/null +++ b/test/src/lib/parse/LibParseMeta.lookupWord.t.sol @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {Test} from "forge-std/Test.sol"; +import {LibParseMeta} from "src/lib/parse/LibParseMeta.sol"; +import {LibGenParseMeta} from "src/lib/codegen/LibGenParseMeta.sol"; +import {LibAuthoringMeta, AuthoringMetaV2} from "test/lib/meta/LibAuthoringMeta.sol"; +import {LibBloom} from "test/lib/bloom/LibBloom.sol"; + +contract LibParseMetaLookupWordTest is Test { + /// Build meta from known words, look them all up, verify indices. + function testLookupWordKnown() external pure { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](3); + metas[0] = AuthoringMetaV2({word: bytes32("add"), description: ""}); + metas[1] = AuthoringMetaV2({word: bytes32("sub"), description: ""}); + metas[2] = AuthoringMetaV2({word: bytes32("mul"), description: ""}); + + bytes memory meta = LibGenParseMeta.buildParseMetaV2(metas, 8); + + for (uint256 i = 0; i < metas.length; i++) { + (bool exists, uint256 index) = LibParseMeta.lookupWord(meta, metas[i].word); + assertTrue(exists, "word should exist"); + assertEq(index, i, "word index mismatch"); + } + } + + /// Looking up a word not in meta should return false with index 0. + function testLookupWordNotFound() external pure { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](1); + metas[0] = AuthoringMetaV2({word: bytes32("add"), description: ""}); + + bytes memory meta = LibGenParseMeta.buildParseMetaV2(metas, 8); + + (bool exists, uint256 index) = LibParseMeta.lookupWord(meta, bytes32("notaword")); + assertFalse(exists); + assertEq(index, 0); + } + + /// Single-depth meta with a single word. + function testLookupWordSingleDepth() external pure { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](1); + metas[0] = AuthoringMetaV2({word: bytes32("only"), description: ""}); + + bytes memory meta = LibGenParseMeta.buildParseMetaV2(metas, 1); + + (bool exists, uint256 index) = LibParseMeta.lookupWord(meta, bytes32("only")); + assertTrue(exists); + assertEq(index, 0); + + // Not-found on single-depth meta. + (bool notExists,) = LibParseMeta.lookupWord(meta, bytes32("other")); + assertFalse(notExists); + } + + /// Multiple not-found lookups should all return false. + function testLookupWordMultipleNotFound(bytes32 a, bytes32 b, bytes32 c) external pure { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](1); + metas[0] = AuthoringMetaV2({word: bytes32("known"), description: ""}); + + // Ensure fuzzed words differ from the known word. + vm.assume(a != bytes32("known")); + vm.assume(b != bytes32("known")); + vm.assume(c != bytes32("known")); + + bytes memory meta = LibGenParseMeta.buildParseMetaV2(metas, 3); + + (bool existsA,) = LibParseMeta.lookupWord(meta, a); + assertFalse(existsA); + (bool existsB,) = LibParseMeta.lookupWord(meta, b); + assertFalse(existsB); + (bool existsC,) = LibParseMeta.lookupWord(meta, c); + assertFalse(existsC); + } + + /// Fuzz: every word in the authoring meta should be found at its correct + /// index, and a random word not in the meta should not be found. + function testLookupWordRoundtripFuzz(AuthoringMetaV2[] memory authoringMeta, bytes32 notFound) external pure { + vm.assume(authoringMeta.length > 0); + vm.assume(authoringMeta.length <= 64); + vm.assume(!LibBloom.bloomFindsDupes(LibAuthoringMeta.copyWordsFromAuthoringMeta(authoringMeta))); + for (uint256 i = 0; i < authoringMeta.length; i++) { + vm.assume(authoringMeta[i].word != notFound); + } + + uint8 depth = uint8(authoringMeta.length / type(uint8).max + 3); + bytes memory meta = LibGenParseMeta.buildParseMetaV2(authoringMeta, depth); + + // Every word should be found at its index. + for (uint256 i = 0; i < authoringMeta.length; i++) { + (bool exists, uint256 index) = LibParseMeta.lookupWord(meta, authoringMeta[i].word); + assertTrue(exists, "word should exist"); + assertEq(index, i, "index mismatch"); + } + + // A word not in meta should not be found. + (bool notExists,) = LibParseMeta.lookupWord(meta, notFound); + assertFalse(notExists, "unknown word should not exist"); + } + + /// Larger word set forcing multi-depth bloom — verify all words still + /// resolve correctly. + function testLookupWordLargeSet() external pure { + uint256 count = 50; + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](count); + for (uint256 i = 0; i < count; i++) { + metas[i] = AuthoringMetaV2({word: bytes32(i + 1), description: ""}); + } + + bytes memory meta = LibGenParseMeta.buildParseMetaV2(metas, 5); + + for (uint256 i = 0; i < count; i++) { + (bool exists, uint256 index) = LibParseMeta.lookupWord(meta, bytes32(i + 1)); + assertTrue(exists, "word should exist"); + assertEq(index, i, "index mismatch"); + } + + // Zero was not added. + (bool notExists,) = LibParseMeta.lookupWord(meta, bytes32(uint256(0))); + assertFalse(notExists); + } +} From cec1d83c761ff247ab86ace4fff9f1a726dace03 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sun, 1 Mar 2026 21:17:31 +0400 Subject: [PATCH 02/13] fix: add bloom filter bit check in lookupWord (M01) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/lib/parse/LibParseMeta.sol | 8 ++ .../lib/parse/LibParseMeta.lookupWord.t.sol | 81 +++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/src/lib/parse/LibParseMeta.sol b/src/lib/parse/LibParseMeta.sol index 0481ea2..5dd4ed7 100644 --- a/src/lib/parse/LibParseMeta.sol +++ b/src/lib/parse/LibParseMeta.sol @@ -101,6 +101,14 @@ library LibParseMeta { } (uint256 shifted, uint256 hashed) = wordBitmapped(seed, word); + + // If the word's bit is not set in the expansion, the word + // is not in the set. No word was mapped to this bit, so + // there is nothing to collide with at any depth. + if (expansion & shifted == 0) { + return (false, 0); + } + uint256 pos = LibCtPop.ctpop(expansion & (shifted - 1)) + cumulativeCt; wordFingerprint = hashed & FINGERPRINT_MASK; uint256 metaItemSize = META_ITEM_SIZE; diff --git a/test/src/lib/parse/LibParseMeta.lookupWord.t.sol b/test/src/lib/parse/LibParseMeta.lookupWord.t.sol index 8946257..34120eb 100644 --- a/test/src/lib/parse/LibParseMeta.lookupWord.t.sol +++ b/test/src/lib/parse/LibParseMeta.lookupWord.t.sol @@ -7,8 +7,89 @@ import {LibParseMeta} from "src/lib/parse/LibParseMeta.sol"; import {LibGenParseMeta} from "src/lib/codegen/LibGenParseMeta.sol"; import {LibAuthoringMeta, AuthoringMetaV2} from "test/lib/meta/LibAuthoringMeta.sol"; import {LibBloom} from "test/lib/bloom/LibBloom.sol"; +import { + META_ITEM_SIZE, + FINGERPRINT_MASK, + META_EXPANSION_SIZE, + META_PREFIX_SIZE +} from "src/lib/parse/LibParseMeta.sol"; contract LibParseMetaLookupWordTest is Test { + /// Demonstrates M01: lookupWord does not check whether the word's bit is + /// actually set in the bloom filter expansion before comparing fingerprints. + /// We construct raw meta bytes where: + /// - The expansion has a single bit set (NOT the lookup word's bit) + /// - The item at position 0 has a fingerprint matching the lookup word + /// This should return (false, 0) but currently returns (true, fakeIndex) + /// because the bit-set check is missing. + function testLookupWordMissingBitCheck() external pure { + bytes32 word = bytes32("notinmeta"); + uint8 seed = 0; + + // Compute word's bitmap and fingerprint, then build crafted meta. + bytes memory meta = _buildM01Meta(word, seed); + + // lookupWord should return (false, 0) because the word's bit is NOT + // set in the expansion. But due to M01, it returns (true, 42). + (bool exists, uint256 index) = LibParseMeta.lookupWord(meta, word); + assertFalse(exists, "M01: word bit not set in expansion, should not match"); + assertEq(index, 0, "M01: index should be 0 for not-found"); + } + + /// Fuzz variant of M01: any word should fail to match when its bit is not + /// set in the expansion, regardless of fingerprint. + function testLookupWordMissingBitCheckFuzz(bytes32 word, uint8 seed) external pure { + bytes memory meta = _buildM01Meta(word, seed); + + (bool exists, uint256 index) = LibParseMeta.lookupWord(meta, word); + assertFalse(exists, "M01 fuzz: word bit not set, should not match"); + assertEq(index, 0, "M01 fuzz: index should be 0 for not-found"); + } + + /// Constructs a crafted meta that triggers M01. The meta has one bloom + /// layer with a single bit set that is NOT the lookup word's bit. The item + /// at the position lookupWord will compute has a fingerprint matching the + /// word. Without the bit-set check, lookupWord returns a false positive. + function _buildM01Meta(bytes32 word, uint8 seed) internal pure returns (bytes memory meta) { + (uint256 shifted, uint256 hashed) = LibParseMeta.wordBitmapped(seed, word); + uint256 wordFingerprint = hashed & FINGERPRINT_MASK; + + // Find the word's bit position. + uint256 bitPos; + for (uint256 i = 0; i < 256; i++) { + if (shifted == (1 << i)) { + bitPos = i; + break; + } + } + + // Pick a different bit ABOVE the word's bit so ctpop gives pos = 0. + // Wrap around if needed — the key requirement is the bit differs. + uint256 fakeBitPos = (bitPos + 128) % 256; + uint256 fakeExpansion = 1 << fakeBitPos; + + // Determine what pos lookupWord will compute: + // pos = ctpop(expansion & (shifted - 1)) + uint256 expectedPos = (fakeBitPos < bitPos) ? uint256(1) : uint256(0); + + uint256 numItems = expectedPos + 1; + meta = new bytes(META_PREFIX_SIZE + META_EXPANSION_SIZE + numItems * META_ITEM_SIZE); + + // Write depth, seed, expansion. + meta[0] = bytes1(uint8(1)); + meta[1] = bytes1(seed); + for (uint256 i = 0; i < 32; i++) { + meta[2 + i] = bytes1(uint8((fakeExpansion >> (8 * (31 - i))) & 0xFF)); + } + + // Write item at expectedPos with the word's fingerprint and a fake + // opcode index of 42. + uint256 itemOffset = META_PREFIX_SIZE + META_EXPANSION_SIZE + expectedPos * META_ITEM_SIZE; + meta[itemOffset] = bytes1(uint8(42)); + meta[itemOffset + 1] = bytes1(uint8((wordFingerprint >> 16) & 0xFF)); + meta[itemOffset + 2] = bytes1(uint8((wordFingerprint >> 8) & 0xFF)); + meta[itemOffset + 3] = bytes1(uint8(wordFingerprint & 0xFF)); + } /// Build meta from known words, look them all up, verify indices. function testLookupWordKnown() external pure { AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](3); From 0083204b1a054bc5d618277dc41ac937a3dba63c Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sun, 1 Mar 2026 21:32:01 +0400 Subject: [PATCH 03/13] fix: prevent zero-fingerprint sentinel collision (L02) 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 --- src/lib/parse/LibParseMeta.sol | 6 ++++++ .../src/lib/parse/LibParseMeta.wordBitmapped.t.sol | 14 +++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/lib/parse/LibParseMeta.sol b/src/lib/parse/LibParseMeta.sol index 5dd4ed7..edbc789 100644 --- a/src/lib/parse/LibParseMeta.sol +++ b/src/lib/parse/LibParseMeta.sol @@ -54,6 +54,12 @@ library LibParseMeta { // low 3 bytes for the fingerprint. //slither-disable-next-line incorrect-shift bitmap := shl(byte(0, hashed), 1) + // Fingerprint 0 is reserved as the empty-slot sentinel in + // buildParseMetaV2. If the low 3 bytes are 0, set to 1. + // This introduces a small bias on fingerprint 1 (2 in 2^24 + // instead of 1 in 2^24) which is negligible. + // bitmap is already computed so the high bytes don't matter. + if iszero(and(hashed, 0xFFFFFF)) { hashed := 1 } } } diff --git a/test/src/lib/parse/LibParseMeta.wordBitmapped.t.sol b/test/src/lib/parse/LibParseMeta.wordBitmapped.t.sol index 6195deb..e708b3f 100644 --- a/test/src/lib/parse/LibParseMeta.wordBitmapped.t.sol +++ b/test/src/lib/parse/LibParseMeta.wordBitmapped.t.sol @@ -4,7 +4,7 @@ pragma solidity =0.8.25; import {Test} from "forge-std/Test.sol"; -import {LibParseMeta} from "src/lib/parse/LibParseMeta.sol"; +import {LibParseMeta, FINGERPRINT_MASK} from "src/lib/parse/LibParseMeta.sol"; contract LitParseMetaTest is Test { function referenceWordBitmapped(uint256 seed, bytes32 word) public pure returns (uint256 bitmap, uint256 hashed) { @@ -14,6 +14,11 @@ contract LitParseMetaTest is Test { // Taking the type byte of hashed only. //forge-lint: disable-next-line(unsafe-typecast, incorrect-shift) bitmap = 1 << uint256(uint8(uint256(hashed) >> 0xF8)); + // Fingerprint 0 is reserved as the empty-slot sentinel, so force + // it to 1 when the low 3 bytes are zero. + if (hashed & FINGERPRINT_MASK == 0) { + hashed = 1; + } } function testWordBitmapped(uint256 seed, bytes32 word) public pure { @@ -22,4 +27,11 @@ contract LitParseMetaTest is Test { assertEq(bitmap, refBitmap, "bitmap"); assertEq(hashed, refHashed, "hashed"); } + + /// The fingerprint (low 3 bytes of hashed) must never be zero, because + /// zero is the empty-slot sentinel in buildParseMetaV2. + function testWordBitmappedFingerprintNonZero(uint256 seed, bytes32 word) public pure { + (, uint256 hashed) = LibParseMeta.wordBitmapped(seed, word); + assertTrue(hashed & FINGERPRINT_MASK != 0, "fingerprint must not be zero"); + } } From a3cb1c76027fe4a2cd4ebbec42808b57ca78b5fa Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sun, 1 Mar 2026 21:43:59 +0400 Subject: [PATCH 04/13] fix: add MaxDepthExceeded error for bloom filter overflow (L03) 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 --- src/lib/codegen/LibGenParseMeta.sol | 7 +++++ .../codegen/LibGenParseMeta.buildMeta.t.sol | 29 ++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/lib/codegen/LibGenParseMeta.sol b/src/lib/codegen/LibGenParseMeta.sol index cffe9d0..244a5db 100644 --- a/src/lib/codegen/LibGenParseMeta.sol +++ b/src/lib/codegen/LibGenParseMeta.sol @@ -24,6 +24,10 @@ error DuplicateFingerprint(); /// stored in a single byte, so more than 256 words cannot be represented. error AuthoringMetaTooLarge(uint256 length); +/// @dev Thrown when the authoring meta cannot be compressed within the +/// provided maxDepth bloom filter layers. +error MaxDepthExceeded(uint8 maxDepth); + /// @title LibGenParseMeta /// @notice Library for building parse meta from authoring meta, and generating /// constant strings for the parse meta to be used in generated code. The parse @@ -147,6 +151,9 @@ library LibGenParseMeta { { AuthoringMetaV2[] memory remainingAuthoringMeta = authoringMeta; while (remainingAuthoringMeta.length > 0) { + if (depth >= maxDepth) { + revert MaxDepthExceeded(maxDepth); + } uint8 seed; uint256 expansion; (seed, expansion, remainingAuthoringMeta) = findBestExpander(remainingAuthoringMeta); diff --git a/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol b/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol index 2fa56bf..c8588d9 100644 --- a/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol +++ b/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol @@ -6,7 +6,7 @@ import {Test} from "forge-std/Test.sol"; import {LibParseMeta} from "src/lib/parse/LibParseMeta.sol"; import {LibAuthoringMeta, AuthoringMetaV2} from "test/lib/meta/LibAuthoringMeta.sol"; -import {LibGenParseMeta, DuplicateFingerprint, AuthoringMetaTooLarge} from "src/lib/codegen/LibGenParseMeta.sol"; +import {LibGenParseMeta, DuplicateFingerprint, AuthoringMetaTooLarge, MaxDepthExceeded} from "src/lib/codegen/LibGenParseMeta.sol"; import {LibBloom} from "test/lib/bloom/LibBloom.sol"; contract LibGenParseMetaBuildMetaTest is Test { @@ -109,6 +109,33 @@ contract LibGenParseMetaBuildMetaTest is Test { this.buildParseMetaV2External(metas, 8); } + /// maxDepth=1 with enough words to force multiple bloom layers should + /// revert with MaxDepthExceeded. + function testBuildMetaMaxDepthExceeded() external { + // 256 unique words will need more than 1 bloom layer. + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](256); + for (uint256 i = 0; i < 256; i++) { + metas[i] = AuthoringMetaV2({word: bytes32(i), description: ""}); + } + vm.expectRevert(abi.encodeWithSelector(MaxDepthExceeded.selector, 1)); + this.buildParseMetaV2External(metas, 1); + } + + /// Fuzz: maxDepth=1 with more than 1 word that collides should revert. + /// Even 2 words can collide at depth 1 if they share a bit position for + /// every seed — but with 256+ words, collision is guaranteed. + function testBuildMetaMaxDepthExceededFuzz(uint8 maxDepth) external { + // Use enough words that the required depth exceeds maxDepth. + // 256 words need at least 2 layers; bound maxDepth to 1. + vm.assume(maxDepth < 2); + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](256); + for (uint256 i = 0; i < 256; i++) { + metas[i] = AuthoringMetaV2({word: bytes32(i), description: ""}); + } + vm.expectRevert(abi.encodeWithSelector(MaxDepthExceeded.selector, maxDepth)); + this.buildParseMetaV2External(metas, maxDepth); + } + /// Exactly 256 words should succeed (boundary). function testBuildMeta256Words() external pure { AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](256); From 71196ab912154fe8c74df4da1fd6e5e62edf0a65 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sun, 1 Mar 2026 21:48:52 +0400 Subject: [PATCH 05/13] docs: document ramifications of fingerprint 0 bias (L02) Co-Authored-By: Claude Opus 4.6 --- src/lib/parse/LibParseMeta.sol | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/lib/parse/LibParseMeta.sol b/src/lib/parse/LibParseMeta.sol index edbc789..7112aed 100644 --- a/src/lib/parse/LibParseMeta.sol +++ b/src/lib/parse/LibParseMeta.sol @@ -57,7 +57,12 @@ library LibParseMeta { // Fingerprint 0 is reserved as the empty-slot sentinel in // buildParseMetaV2. If the low 3 bytes are 0, set to 1. // This introduces a small bias on fingerprint 1 (2 in 2^24 - // instead of 1 in 2^24) which is negligible. + // instead of 1 in 2^24) which is negligible. Overall collision + // probability changes from 1/2^24 to (2^24 + 2)/2^48 which is + // effectively identical. The only concrete effect is that two + // words which both independently hash to fingerprint 0 (~1 in + // 16.7M each) would both map to 1 and appear as a + // DuplicateFingerprint during generation — a ~1 in 2^46 event. // bitmap is already computed so the high bytes don't matter. if iszero(and(hashed, 0xFFFFFF)) { hashed := 1 } } From 501ee0dab973d1d81749714b6581ecf603a674d1 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sun, 1 Mar 2026 21:58:13 +0400 Subject: [PATCH 06/13] fix: correct META_ITEM_MASK from 4-bit to 32-bit (I01) 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 --- src/lib/codegen/LibGenParseMeta.sol | 3 +-- .../lib/codegen/LibGenParseMeta.buildMeta.t.sol | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/lib/codegen/LibGenParseMeta.sol b/src/lib/codegen/LibGenParseMeta.sol index 244a5db..2f97482 100644 --- a/src/lib/codegen/LibGenParseMeta.sol +++ b/src/lib/codegen/LibGenParseMeta.sol @@ -14,8 +14,7 @@ import {LibCtPop} from "rain.math.binary/lib/LibCtPop.sol"; import {Vm} from "forge-std/Vm.sol"; import {LibCodeGen} from "rain.sol.codegen/lib/LibCodeGen.sol"; -//forge-lint: disable-next-line(incorrect-shift) -uint256 constant META_ITEM_MASK = (1 << META_ITEM_SIZE) - 1; +uint256 constant META_ITEM_MASK = type(uint32).max; /// @dev For metadata builder. error DuplicateFingerprint(); diff --git a/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol b/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol index c8588d9..cd1415b 100644 --- a/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol +++ b/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol @@ -6,10 +6,24 @@ import {Test} from "forge-std/Test.sol"; import {LibParseMeta} from "src/lib/parse/LibParseMeta.sol"; import {LibAuthoringMeta, AuthoringMetaV2} from "test/lib/meta/LibAuthoringMeta.sol"; -import {LibGenParseMeta, DuplicateFingerprint, AuthoringMetaTooLarge, MaxDepthExceeded} from "src/lib/codegen/LibGenParseMeta.sol"; +import { + LibGenParseMeta, + DuplicateFingerprint, + AuthoringMetaTooLarge, + MaxDepthExceeded, + META_ITEM_MASK +} from "src/lib/codegen/LibGenParseMeta.sol"; +import {META_ITEM_SIZE} from "src/lib/parse/LibParseMeta.sol"; import {LibBloom} from "test/lib/bloom/LibBloom.sol"; contract LibGenParseMetaBuildMetaTest is Test { + /// META_ITEM_MASK must be a full META_ITEM_SIZE-byte mask (32 bits for + /// 4-byte items). Previously the constant was (1 << 4) - 1 = 0xF which + /// is only 4 bits. + function testMetaItemMask() external pure { + assertEq(META_ITEM_MASK, (1 << (META_ITEM_SIZE * 8)) - 1); + assertEq(META_ITEM_MASK, type(uint32).max); + } function buildParseMetaV2External(AuthoringMetaV2[] memory authoringMeta, uint8 maxDepth) external pure From c68f9aee4463400b24e8b6650ccd993445ab4d70 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sun, 1 Mar 2026 22:13:43 +0400 Subject: [PATCH 07/13] feat: add checkParseMetaStructure validation function (L04) 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 --- src/lib/parse/LibParseMeta.sol | 40 +++++++++++- .../lib/parse/LibParseMeta.lookupWord.t.sol | 65 ++++++++++++++++++- 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/src/lib/parse/LibParseMeta.sol b/src/lib/parse/LibParseMeta.sol index 7112aed..8cb21f8 100644 --- a/src/lib/parse/LibParseMeta.sol +++ b/src/lib/parse/LibParseMeta.sol @@ -25,11 +25,48 @@ uint256 constant FINGERPRINT_MASK = 0xFFFFFF; /// @dev 33 = 32 bytes for expansion + 1 byte for seed uint256 constant META_EXPANSION_SIZE = 0x21; +/// @dev Thrown by `checkParseMetaStructure` when the meta bytes do not match +/// the expected length derived from its depth and expansion data. +/// @param expected The expected byte length. +/// @param actual The actual byte length. +error InvalidParseMeta(uint256 expected, uint256 actual); + /// @title LibParseMeta /// @notice Common logic for working with parse meta, which is the data structure /// used to store information about the words in a parser. The parse meta is /// designed to be compact and efficient to lookup. library LibParseMeta { + /// Validates that the parse meta has a structurally consistent length. + /// Reads the depth and all expansions to compute the expected total byte + /// count, then reverts if `meta.length` does not match. Intended to be + /// called once at build time so that `lookupWord` can trust the meta + /// without per-call bounds checks. + /// @param meta The parse meta bytes to validate. + function checkParseMetaStructure(bytes memory meta) internal pure { + unchecked { + uint256 depth; + uint256 totalItems = 0; + assembly ("memory-safe") { + depth := and(mload(add(meta, 1)), 0xFF) + } + uint256 cursor; + assembly ("memory-safe") { + cursor := add(meta, 1) + } + for (uint256 i = 0; i < depth; i++) { + uint256 expansion; + assembly ("memory-safe") { + cursor := add(cursor, 0x21) + expansion := mload(cursor) + } + totalItems += LibCtPop.ctpop(expansion); + } + uint256 expected = META_PREFIX_SIZE + depth * META_EXPANSION_SIZE + totalItems * META_ITEM_SIZE; + if (meta.length != expected) { + revert InvalidParseMeta(expected, meta.length); + } + } + } /// @dev Given a word and a seed, return the bitmap and fingerprint for the /// word. The bitmap is a uint256 with a single bit set, which can be used /// to check if the word is present in an expansion. The fingerprint is a @@ -73,7 +110,8 @@ library LibParseMeta { /// MUST check `exists` before using the other return values. /// The `meta` parameter MUST be well-formed as produced by /// `LibGenParseMeta.buildParseMetaV2`. Behavior is undefined for malformed - /// meta — no bounds checking is performed on the meta structure. + /// meta — no bounds checking is performed on the meta structure. Use + /// `checkParseMetaStructure` to validate meta at build time. /// @param meta The parser meta. /// @param word The word to lookup. /// @return True if the word exists in the parse meta. diff --git a/test/src/lib/parse/LibParseMeta.lookupWord.t.sol b/test/src/lib/parse/LibParseMeta.lookupWord.t.sol index 34120eb..465445b 100644 --- a/test/src/lib/parse/LibParseMeta.lookupWord.t.sol +++ b/test/src/lib/parse/LibParseMeta.lookupWord.t.sol @@ -11,10 +11,73 @@ import { META_ITEM_SIZE, FINGERPRINT_MASK, META_EXPANSION_SIZE, - META_PREFIX_SIZE + META_PREFIX_SIZE, + InvalidParseMeta } from "src/lib/parse/LibParseMeta.sol"; contract LibParseMetaLookupWordTest is Test { + function checkParseMetaStructureExternal(bytes memory meta) external pure { + LibParseMeta.checkParseMetaStructure(meta); + } + + /// buildParseMetaV2 output must always pass structural validation. + function testCheckParseMetaStructureBuildOutput() external pure { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](3); + metas[0] = AuthoringMetaV2({word: bytes32("add"), description: ""}); + metas[1] = AuthoringMetaV2({word: bytes32("sub"), description: ""}); + metas[2] = AuthoringMetaV2({word: bytes32("mul"), description: ""}); + bytes memory meta = LibGenParseMeta.buildParseMetaV2(metas, 8); + LibParseMeta.checkParseMetaStructure(meta); + } + + /// Fuzz: any well-formed buildParseMetaV2 output passes validation. + function testCheckParseMetaStructureFuzz(AuthoringMetaV2[] memory authoringMeta) external pure { + vm.assume(authoringMeta.length > 0); + vm.assume(authoringMeta.length <= 64); + vm.assume(!LibBloom.bloomFindsDupes(LibAuthoringMeta.copyWordsFromAuthoringMeta(authoringMeta))); + uint8 depth = uint8(authoringMeta.length / type(uint8).max + 3); + bytes memory meta = LibGenParseMeta.buildParseMetaV2(authoringMeta, depth); + LibParseMeta.checkParseMetaStructure(meta); + } + + /// Truncated meta should fail validation. + function testCheckParseMetaStructureTruncated() external { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](1); + metas[0] = AuthoringMetaV2({word: bytes32("add"), description: ""}); + bytes memory meta = LibGenParseMeta.buildParseMetaV2(metas, 8); + + // Truncate by 1 byte. + bytes memory truncated = new bytes(meta.length - 1); + for (uint256 i = 0; i < truncated.length; i++) { + truncated[i] = meta[i]; + } + vm.expectRevert(); + this.checkParseMetaStructureExternal(truncated); + } + + /// Extra trailing bytes should fail validation. + function testCheckParseMetaStructureExtraBytes() external { + AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](1); + metas[0] = AuthoringMetaV2({word: bytes32("add"), description: ""}); + bytes memory meta = LibGenParseMeta.buildParseMetaV2(metas, 8); + + // Append 1 extra byte. + bytes memory extended = new bytes(meta.length + 1); + for (uint256 i = 0; i < meta.length; i++) { + extended[i] = meta[i]; + } + vm.expectRevert(); + this.checkParseMetaStructureExternal(extended); + } + + /// Empty meta (zero length) should fail validation. + function testCheckParseMetaStructureEmpty() external { + bytes memory meta = new bytes(0); + // depth=0, 0 expansions, 0 items → expected length = 1. + vm.expectRevert(); + this.checkParseMetaStructureExternal(meta); + } + /// Demonstrates M01: lookupWord does not check whether the word's bit is /// actually set in the bloom filter expansion before comparing fingerprints. /// We construct raw meta bytes where: From 3f25e80a239500aad22c06e82e6f717bfd81a593 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sun, 1 Mar 2026 22:19:38 +0400 Subject: [PATCH 08/13] fix: self-check parse meta structure in buildParseMetaV2 Co-Authored-By: Claude Opus 4.6 --- src/lib/codegen/LibGenParseMeta.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lib/codegen/LibGenParseMeta.sol b/src/lib/codegen/LibGenParseMeta.sol index 2f97482..08bbf98 100644 --- a/src/lib/codegen/LibGenParseMeta.sol +++ b/src/lib/codegen/LibGenParseMeta.sol @@ -239,6 +239,8 @@ library LibGenParseMeta { break; } } + + LibParseMeta.checkParseMetaStructure(parseMeta); } } From 76210eb344f7344599c66725c0af97f5e217b22c Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sun, 1 Mar 2026 22:25:57 +0400 Subject: [PATCH 09/13] docs: update natspec for wordBitmapped and buildParseMetaV2 - 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 --- src/lib/codegen/LibGenParseMeta.sol | 13 +++++++++---- src/lib/parse/LibParseMeta.sol | 9 ++++++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/lib/codegen/LibGenParseMeta.sol b/src/lib/codegen/LibGenParseMeta.sol index 08bbf98..489293e 100644 --- a/src/lib/codegen/LibGenParseMeta.sol +++ b/src/lib/codegen/LibGenParseMeta.sol @@ -111,15 +111,20 @@ library LibGenParseMeta { /// byte is its opcode index, the remaining 3 bytes are the word /// fingerprint. /// The parse meta is used to lookup word definitions. To do a lookup, the - /// word is hashed with the seed, then the first byte of the hash is compared - /// against the bloom filter. If there is a hit then we count the number of - /// 1 bits in the bloom filter up to this item's 1 bit. We then treat this - /// as the index of the item in the items array. We then compare the word + /// word is hashed with the seed, then the first byte of the hash selects a + /// bit in the bloom filter. If the bit is not set, the word is not in the + /// set — return immediately. If the bit is set, we count the number of 1 + /// bits in the bloom filter below this item's bit. We then treat this as + /// the index of the item in the items array. We then compare the word /// fingerprint against the fingerprint of the item at this index. If the /// fingerprints equal then we have a match, else we increment the seed and /// try again with the next bloom filter, offsetting all the indexes by the /// total bit count of the previous bloom filter. If we reach the end of the /// bloom filters then we have a miss. + /// The output is validated via `LibParseMeta.checkParseMetaStructure` + /// before returning. Reverts with `MaxDepthExceeded` if the words cannot + /// be compressed within `maxDepth` layers, or `AuthoringMetaTooLarge` if + /// there are more than 256 words. /// @param authoringMeta The authoring meta to build the parse meta from. /// @param maxDepth The maximum depth of the bloom filters to use. This is a /// tradeoff between the size of the parse meta and the speed of lookups. The diff --git a/src/lib/parse/LibParseMeta.sol b/src/lib/parse/LibParseMeta.sol index 8cb21f8..358b634 100644 --- a/src/lib/parse/LibParseMeta.sol +++ b/src/lib/parse/LibParseMeta.sol @@ -71,14 +71,17 @@ library LibParseMeta { /// word. The bitmap is a uint256 with a single bit set, which can be used /// to check if the word is present in an expansion. The fingerprint is a /// uint256 with the low 3 bytes set, which can be used to check for - /// collisions when a word is found in an expansion. + /// collisions when a word is found in an expansion. The fingerprint is + /// guaranteed to be non-zero (fingerprint 0 is remapped to 1) because + /// zero is used as the empty-slot sentinel in `buildParseMetaV2`. /// @param seed The seed to use for the bitmap, which should be a byte value /// between 0 and 255. /// @param word The word to generate the bitmap and fingerprint for. /// @return bitmap A uint256 with a single bit set, which can be used to /// check if the word is present in an expansion. - /// @return hashed A uint256 with the low 3 bytes set, which can be used to - /// check for collisions when a word is found in an expansion. + /// @return hashed A uint256 with the low 3 bytes guaranteed non-zero, + /// which can be used to check for collisions when a word is found in an + /// expansion. function wordBitmapped(uint256 seed, bytes32 word) internal pure returns (uint256 bitmap, uint256 hashed) { assembly ("memory-safe") { mstore(0, word) From 183c68e0b26499e24a46aa2a306f5ca5becc1056 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sun, 1 Mar 2026 22:34:48 +0400 Subject: [PATCH 10/13] chore: add CLAUDE.md and audit/ to REUSE.toml Co-Authored-By: Claude Opus 4.6 --- REUSE.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/REUSE.toml b/REUSE.toml index f49a274..9fd8130 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -6,7 +6,9 @@ path = [ ".github/workflows/**/", ".gitignore", ".gitmodules", + "CLAUDE.md", "README.md", + "audit/**/", "flake.lock", "flake.nix", "foundry.toml", From d667e656c54c8eaf1162f7886c309e58728c949e Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sun, 1 Mar 2026 22:38:39 +0400 Subject: [PATCH 11/13] chore: fix slither warnings and forge fmt Added slither-disable-next-line for intentionally unused return values in findBestExpander. Applied forge fmt formatting fixes. Co-Authored-By: Claude Opus 4.6 --- src/lib/codegen/LibGenParseMeta.sol | 2 ++ src/lib/parse/LibParseMeta.sol | 1 + test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol | 4 ++-- test/src/lib/codegen/LibGenParseMeta.findExpander.t.sol | 9 +++------ test/src/lib/parse/LibParseMeta.lookupWord.t.sol | 1 + 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/lib/codegen/LibGenParseMeta.sol b/src/lib/codegen/LibGenParseMeta.sol index 489293e..b29d52b 100644 --- a/src/lib/codegen/LibGenParseMeta.sol +++ b/src/lib/codegen/LibGenParseMeta.sol @@ -64,6 +64,7 @@ library LibGenParseMeta { for (uint256 seed = 0; seed <= type(uint8).max; seed++) { uint256 expansion = 0; for (uint256 i = 0; i < metas.length; i++) { + //slither-disable-next-line unused-return (uint256 shifted,) = LibParseMeta.wordBitmapped(seed, metas[i].word); expansion = shifted | expansion; } @@ -91,6 +92,7 @@ library LibGenParseMeta { uint256 usedExpansion = 0; uint256 j = 0; for (uint256 i = 0; i < metas.length; i++) { + //slither-disable-next-line unused-return (uint256 shifted,) = LibParseMeta.wordBitmapped(bestSeed, metas[i].word); if ((shifted & usedExpansion) == 0) { usedExpansion = shifted | usedExpansion; diff --git a/src/lib/parse/LibParseMeta.sol b/src/lib/parse/LibParseMeta.sol index 358b634..28a6224 100644 --- a/src/lib/parse/LibParseMeta.sol +++ b/src/lib/parse/LibParseMeta.sol @@ -67,6 +67,7 @@ library LibParseMeta { } } } + /// @dev Given a word and a seed, return the bitmap and fingerprint for the /// word. The bitmap is a uint256 with a single bit set, which can be used /// to check if the word is present in an expansion. The fingerprint is a diff --git a/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol b/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol index cd1415b..0e15d7a 100644 --- a/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol +++ b/test/src/lib/codegen/LibGenParseMeta.buildMeta.t.sol @@ -24,6 +24,7 @@ contract LibGenParseMetaBuildMetaTest is Test { assertEq(META_ITEM_MASK, (1 << (META_ITEM_SIZE * 8)) - 1); assertEq(META_ITEM_MASK, type(uint32).max); } + function buildParseMetaV2External(AuthoringMetaV2[] memory authoringMeta, uint8 maxDepth) external pure @@ -239,8 +240,7 @@ contract LibGenParseMetaBuildMetaTest is Test { function testParseMetaConstantStringFuzz(AuthoringMetaV2[] memory authoringMeta) external pure { vm.assume(!LibBloom.bloomFindsDupes(LibAuthoringMeta.copyWordsFromAuthoringMeta(authoringMeta))); bytes memory encoded = abi.encode(authoringMeta); - string memory result = - LibGenParseMeta.parseMetaConstantString(vm, encoded, expanderDepth(authoringMeta.length)); + string memory result = LibGenParseMeta.parseMetaConstantString(vm, encoded, expanderDepth(authoringMeta.length)); assertTrue(bytes(result).length > 0); } } diff --git a/test/src/lib/codegen/LibGenParseMeta.findExpander.t.sol b/test/src/lib/codegen/LibGenParseMeta.findExpander.t.sol index c94eb06..60ea203 100644 --- a/test/src/lib/codegen/LibGenParseMeta.findExpander.t.sol +++ b/test/src/lib/codegen/LibGenParseMeta.findExpander.t.sol @@ -50,8 +50,7 @@ contract LibGenParseMetaFindExpanderTest is Test { for (uint256 i = 0; i < 64; i++) { metas[i] = AuthoringMetaV2({word: bytes32(i), description: ""}); } - (, uint256 bestExpansion, AuthoringMetaV2[] memory remaining) = - LibGenParseMeta.findBestExpander(metas); + (, uint256 bestExpansion, AuthoringMetaV2[] memory remaining) = LibGenParseMeta.findBestExpander(metas); uint256 expandedCount = LibCtPop.ctpop(bestExpansion); assertEq(remaining.length, 64 - expandedCount); } @@ -60,8 +59,7 @@ contract LibGenParseMetaFindExpanderTest is Test { function testFindExpanderSingleWord(bytes32 word) external pure { AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](1); metas[0] = AuthoringMetaV2({word: word, description: ""}); - (, uint256 bestExpansion, AuthoringMetaV2[] memory remaining) = - LibGenParseMeta.findBestExpander(metas); + (, uint256 bestExpansion, AuthoringMetaV2[] memory remaining) = LibGenParseMeta.findBestExpander(metas); assertEq(LibCtPop.ctpop(bestExpansion), 1); assertEq(remaining.length, 0); } @@ -70,8 +68,7 @@ contract LibGenParseMetaFindExpanderTest is Test { /// must always hold. function testFindExpanderInvariant(AuthoringMetaV2[] memory authoringMeta) external pure { vm.assume(!LibBloom.bloomFindsDupes(LibAuthoringMeta.copyWordsFromAuthoringMeta(authoringMeta))); - (, uint256 bestExpansion, AuthoringMetaV2[] memory remaining) = - LibGenParseMeta.findBestExpander(authoringMeta); + (, uint256 bestExpansion, AuthoringMetaV2[] memory remaining) = LibGenParseMeta.findBestExpander(authoringMeta); uint256 expandedCount = LibCtPop.ctpop(bestExpansion); assertEq(expandedCount + remaining.length, authoringMeta.length); } diff --git a/test/src/lib/parse/LibParseMeta.lookupWord.t.sol b/test/src/lib/parse/LibParseMeta.lookupWord.t.sol index 465445b..b12ebe0 100644 --- a/test/src/lib/parse/LibParseMeta.lookupWord.t.sol +++ b/test/src/lib/parse/LibParseMeta.lookupWord.t.sol @@ -153,6 +153,7 @@ contract LibParseMetaLookupWordTest is Test { meta[itemOffset + 2] = bytes1(uint8((wordFingerprint >> 8) & 0xFF)); meta[itemOffset + 3] = bytes1(uint8(wordFingerprint & 0xFF)); } + /// Build meta from known words, look them all up, verify indices. function testLookupWordKnown() external pure { AuthoringMetaV2[] memory metas = new AuthoringMetaV2[](3); From 8c133bbf358788f21d7d83d3a3445f33d419b195 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sun, 1 Mar 2026 22:40:49 +0400 Subject: [PATCH 12/13] chore: add .coderabbit.yaml to ignore audit folder Co-Authored-By: Claude Opus 4.6 --- .coderabbit.yaml | 3 +++ REUSE.toml | 1 + 2 files changed, 4 insertions(+) create mode 100644 .coderabbit.yaml diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 0000000..9c7500c --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,3 @@ +reviews: + path_filters: + - "!audit/**" diff --git a/REUSE.toml b/REUSE.toml index 9fd8130..f37536a 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -2,6 +2,7 @@ version = 1 [[annotations]] path = [ + ".coderabbit.yaml", ".gas-snapshot", ".github/workflows/**/", ".gitignore", From db43bf9299fec891003f80a453d470dd0e70254f Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sun, 1 Mar 2026 22:48:36 +0400 Subject: [PATCH 13/13] docs: clarify @return for build when no signed contexts The signers column is only present when signedContexts.length > 0. Co-Authored-By: Claude Opus 4.6 --- src/lib/caller/LibContext.sol | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/lib/caller/LibContext.sol b/src/lib/caller/LibContext.sol index fa8913a..5b430a2 100644 --- a/src/lib/caller/LibContext.sol +++ b/src/lib/caller/LibContext.sol @@ -161,9 +161,10 @@ library LibContext { /// order, rather than relying on the `msg.sender` to honestly present data /// in any particular structure/order. /// @return The fully assembled context matrix. Column 0 is the base context - /// from `LibContext.base()`. Subsequent columns are from `baseContext`, - /// followed by a signers column (each signer address cast to `bytes32`), - /// followed by each signed context's data as individual columns. + /// from `LibContext.base()`. Subsequent columns are from `baseContext`. + /// If signed contexts are provided, those are followed by a signers column + /// (each signer address cast to `bytes32`) and then each signed context's + /// data as individual columns. function build(bytes32[][] memory baseContext, SignedContextV1[] memory signedContexts) internal view