pi constant word#535
Conversation
Add a `pi` standard op mirroring the existing `e` word (LibOpE), stacking the mathematical constant pi as a Float. - New `LibOpPi` library with integrity/run/referenceFn, encoding pi as a Float at the same precision/convention as `LibDecimalFloat.FLOAT_E` (67-digit coefficient * 10^-66; round-trips to pi to 66 decimal places). - Register `pi` in `LibAllStandardOps` between `mul` and `power` across all four parallel arrays (authoring meta, operand handlers, integrity pointers, opcode pointers); bump ALL_STANDARD_OPS_LENGTH 72 -> 73. - Update the word-ordering assertions in LibAllStandardOps.t.sol. - Add `LibOpPi.t.sol` mirroring `LibOpE.t.sol` (parse + eval to pi, integrity, operand-disallowed, bad input/output arity). - Regenerate bytecode pointer artifacts: interpreter, parser, expression deployer, and Rainlang `.pointers.sol` (creation/runtime code, deploy addresses, codehashes, parse-meta, function pointer tables) plus the CBOR authoring meta. Converged to a stable fixed point. This is a deployed-interpreter-bytecode change: the pinned deploy addresses and codehashes change, so landing REQUIRES the multi-network interpreter redeploy workflow (per #237 / the LibOpE class of change). Closes #225 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds a Changespi opcode implementation and wiring
Shared test helper extraction
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The pi opcode changes the interpreter bytecode, which cascades through the expression deployer, parser, and Rainlang registry. Refresh the committed forge ABI artifacts so the copy-artifacts determinism check passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/op/math/LibOpPi.sol`:
- Around line 11-12: The NatSpec documentation block in LibOpPi.sol for the pi
constant has an explicit `@dev` tag on line 11 but line 12 (containing the
hexadecimal representation) is untagged. According to NatSpec formatting rules,
when a block uses explicit tags, all lines must be explicitly tagged. Add the
`@dev` tag to line 12 to properly tag the continuation of the pi constant
documentation.
- Line 3: The pragma declaration in LibOpPi.sol uses a caret range (^0.8.25)
which allows multiple compiler versions and violates the requirement for
deterministic builds. Change the pragma solidity statement from using the caret
operator (^) to an equals operator (=) to pin the exact version. Replace pragma
solidity ^0.8.25; with pragma solidity =0.8.25; to enforce exact version
matching as required by the coding guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 379c9863-5c0c-4155-9707-a529c59b7d1f
⛔ Files ignored due to path filters (4)
src/generated/Rainlang.pointers.solis excluded by!**/generated/**src/generated/RainlangExpressionDeployer.pointers.solis excluded by!**/generated/**src/generated/RainlangInterpreter.pointers.solis excluded by!**/generated/**src/generated/RainlangParser.pointers.solis excluded by!**/generated/**
📒 Files selected for processing (11)
crates/bindings/abi/Rainlang.jsoncrates/test_fixtures/abi/Rainlang.jsoncrates/test_fixtures/abi/RainlangExpressionDeployer.jsoncrates/test_fixtures/abi/RainlangInterpreter.jsoncrates/test_fixtures/abi/RainlangParser.jsonmeta/AuthoringMeta.rain.metameta/RainlangExpressionDeployer.rain.metasrc/lib/op/LibAllStandardOps.solsrc/lib/op/math/LibOpPi.soltest/src/lib/op/LibAllStandardOps.t.soltest/src/lib/op/math/LibOpPi.t.sol
| @@ -0,0 +1,49 @@ | |||
| // SPDX-License-Identifier: LicenseRef-DCL-1.0 | |||
| // SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd | |||
| pragma solidity ^0.8.25; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Pragma in changed Solidity file:"
rg -n '^pragma solidity' src/lib/op/math/LibOpPi.sol
echo
echo "Checking toolchain config for Cancun target (if foundry.toml exists):"
fd -H 'foundry.toml' | xargs -r rg -n '^\s*(evm_version|solc_version)\s*='Repository: rainlanguage/rainlang
Length of output: 222
Pin pragma to exact 0.8.25.
Line 3 uses pragma solidity ^0.8.25; (caret range), but the repository requires exact version pinning to ensure deterministic builds and compiler behavior consistency.
Suggested fix
-pragma solidity ^0.8.25;
+pragma solidity =0.8.25;Per coding guidelines: "**/*.sol: Solidity version must be exactly 0.8.25 with EVM target cancun." The EVM target is correctly configured in foundry.toml, but the pragma declaration must use = instead of ^.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/op/math/LibOpPi.sol` at line 3, The pragma declaration in LibOpPi.sol
uses a caret range (^0.8.25) which allows multiple compiler versions and
violates the requirement for deterministic builds. Change the pragma solidity
statement from using the caret operator (^) to an equals operator (=) to pin the
exact version. Replace pragma solidity ^0.8.25; with pragma solidity =0.8.25; to
enforce exact version matching as required by the coding guidelines.
Source: Coding guidelines
| /// @dev The mathematical constant pi as a `Float`. | ||
| /// 3.141592653589793238462643383279502884197169399375105820974944592308e66, -66 |
There was a problem hiding this comment.
Fix tagged NatSpec block formatting.
Line 12 is untagged inside a block that already uses an explicit tag (@dev), which violates the NatSpec tagging rule.
Suggested fix
/// `@dev` The mathematical constant pi as a `Float`.
-/// 3.141592653589793238462643383279502884197169399375105820974944592308e66, -66
+/// `@dev` Encoded as coefficient/exponent: 3.141592653589793238462643383279502884197169399375105820974944592308e66, -66.As per coding guidelines: "In NatSpec doc blocks containing explicit tags (e.g., @title), all entries must be explicitly tagged; untagged lines do not continue the previous tag."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// @dev The mathematical constant pi as a `Float`. | |
| /// 3.141592653589793238462643383279502884197169399375105820974944592308e66, -66 | |
| /// `@dev` The mathematical constant pi as a `Float`. | |
| /// `@dev` Encoded as coefficient/exponent: 3.141592653589793238462643383279502884197169399375105820974944592308e66, -66. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/op/math/LibOpPi.sol` around lines 11 - 12, The NatSpec documentation
block in LibOpPi.sol for the pi constant has an explicit `@dev` tag on line 11 but
line 12 (containing the hexadecimal representation) is untagged. According to
NatSpec formatting rules, when a block uses explicit tags, all lines must be
explicitly tagged. Add the `@dev` tag to line 12 to properly tag the continuation
of the pi constant documentation.
Source: Coding guidelines
The prior red was transient: CI staged rainix@main during the ~3.5h window when rainix main HEAD (c54864d1) had the dangling LICENSE symlink that broke GitHub action-staging. That is now fixed on rainix main (LICENSE -> LICENSES/ LicenseRef-DCL-1.0.txt); this retriggers CI to pick it up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
All five conflicts were generated artifacts (crates/*/abi/*.json, src/generated/*.pointers.sol); resolved by regenerating from the merged tree via rainlang-prelude + CopyArtifacts. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
test/src/lib/state/LibInterpreterStateDataContractExtern.sol (2)
11-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winIncomplete NatSpec on public functions.
deserializehas no doc comment (only the contract-level@dev, itself split across two untagged lines at 11-12), anddeserializeStackLengths's comment (lines 25-27) has no explicit tags at all. Per repo audit-skill guideline for test files, NatSpec should be complete (e.g.,@param/@returnon each external function).As per coding guidelines: "All test files must meet audit requirements from the /audit skill, including tested error paths, complete NatSpec, and no audit findings."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/lib/state/LibInterpreterStateDataContractExtern.sol` around lines 11 - 28, Complete the NatSpec for the external functions in LibInterpreterStateDataContractExtern: add a proper function-level doc block for deserialize and replace the loose contract comment with tagged NatSpec, then add explicit `@param` entries for every argument and an `@return` description. Also update deserializeStackLengths to include full NatSpec with `@param` serialized and `@return`, keeping the documentation aligned with the function names so the test file satisfies audit requirements.Source: Coding guidelines
36-49: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUnbounded backward memory scan has no fallback/bound.
The
for { let offset := 2 } 1 { ... }loop has no upper bound and relies entirely on the assumption that every stack slot is zero-initialized memory (true today, since this runs immediately after a freshunsafeDeserializein the same call). If that invariant is ever violated (e.g., helper reused in a different call flow, or future changes touch memory before this scan), the loop will keep walking backward indefinitely, expanding memory and burning gas rather than failing fast with a clear error.Also worth a comment: the loop deliberately starts at
offset := 2, not1, because slotN-1(offset 1) is always zero and would falsely matchlen == 0for anyN >= 1. That's a non-obvious invariant that should be called out explicitly rather than left implicit.Since this is a test-only helper and the invariant currently holds, this is not urgent, but a bound (e.g., cap iterations at a sane max stack size and revert/assert otherwise) would make failures loud instead of silently burning gas.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/lib/state/LibInterpreterStateDataContractExtern.sol` around lines 36 - 49, The backward scan in the helper inside LibInterpreterStateDataContractExtern is unbounded and should fail fast if the expected zero-initialized stack layout is not found. Update the assembly loop in the array-length recovery logic to add a sane iteration cap or explicit revert/assert path, and keep the special offset := 2 start with a clarifying comment explaining why offset 1 is skipped. Use the LibInterpreterStateDataContractExtern helper and its length-detection loop as the target for the fix.test/src/lib/integrity/IntegrityHighwater.sol (1)
9-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNatSpec incomplete / mistagged.
Two issues per repo guidelines for
test/**/*.{sol,rs}(audit skill requires complete NatSpec) and**/*.sol(explicit-tag rule):
- Lines 9-11: the
@devblock is followed by two untagged continuation lines. Per guideline, untagged lines don't continue the previous tag, so this doc block is effectively incomplete.zeroInputTwoOutput,twoInputOneOutput,buildIntegrityPointers, andrunIntegrityCheckhave no NatSpec at all.📝 Suggested fix
-/// `@dev` Contract with 2 opcodes for testing StackUnderflowHighwater. -/// Opcode 0: 0 inputs, 2 outputs (advances highwater). -/// Opcode 1: 2 inputs, 1 output (drops stack below highwater). +/// `@dev` Contract with 2 opcodes for testing StackUnderflowHighwater. +/// `@dev` Opcode 0: 0 inputs, 2 outputs (advances highwater). +/// `@dev` Opcode 1: 2 inputs, 1 output (drops stack below highwater). contract IntegrityHighwater { + /// `@dev` Always returns (0, 2). function zeroInputTwoOutput(IntegrityCheckState memory, OperandV2) internal pure returns (uint256, uint256) {The rest of the extraction (function-pointer table construction,
runIntegrityCheckdelegation) matches the original inline usage shown in the downstream test snippet, so no functional concerns there.As per coding guidelines: "In NatSpec doc blocks containing explicit tags (e.g.,
@title), all entries must be explicitly tagged; untagged lines do not continue the previous tag" and "All test files must meet audit requirements from the /audit skill, including tested error paths, complete NatSpec, and no audit findings."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/lib/integrity/IntegrityHighwater.sol` around lines 9 - 45, The NatSpec on IntegrityHighwater is incomplete and has mistagged continuation lines; update the contract docs so every descriptive line in the top comment is explicitly tagged instead of relying on untagged continuation. Add complete NatSpec for zeroInputTwoOutput, twoInputOneOutput, buildIntegrityPointers, and runIntegrityCheck, using proper tags like `@dev/`@return where applicable. Keep the documentation consistent with the existing IntegrityHighwater and LibIntegrityCheck behavior so the test contract satisfies the repo’s audit and explicit-tag requirements.Source: Coding guidelines
test/src/lib/integrity/IntegritySingleOp.sol (1)
9-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame NatSpec gaps as
IntegrityHighwater.sol.Line 9-10's
@devblock continues onto an untagged line, andoneInputOneOutput,buildIntegrityPointers,runIntegrityCheckhave no doc comments at all. Same guideline applies here.Functional extraction otherwise matches the downstream
testStackUnderflowusage shown in the context snippet, no logic issues.As per coding guidelines: "In NatSpec doc blocks containing explicit tags (e.g.,
@title), all entries must be explicitly tagged; untagged lines do not continue the previous tag" and "All test files must meet audit requirements from the /audit skill, including tested error paths, complete NatSpec, and no audit findings."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/lib/integrity/IntegritySingleOp.sol` around lines 9 - 40, The NatSpec in IntegritySingleOp is incomplete and contains an untagged continuation line in the existing `@dev` block, so update the contract comment to make every line explicitly tagged and add proper doc comments for oneInputOneOutput, buildIntegrityPointers, and runIntegrityCheck. Use the contract name IntegritySingleOp and the three function names to locate the missing documentation, and keep the comments audit-ready and consistent with the same NatSpec requirements applied in IntegrityHighwater.sol.Source: Coding guidelines
test/src/abstract/ChildRainlangExtern.sol (1)
10-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicates
TestableExtern's wrapper functions.
buildIntegrityFunctionPointers/buildOpcodeFunctionPointershere (lines 19-25) are identical in purpose to the ones already extracted intoTestableExtern.sol. ExtendingTestableExterninstead ofBaseRainlangExterndirectly would remove this duplication — the inheritedviewwrapper still legally calls thepureoverrides sincepurenarrowsview.♻️ Proposed refactor to reuse TestableExtern
-import {BaseRainlangExtern} from "../../../src/abstract/BaseRainlangExtern.sol"; +import {TestableExtern} from "./TestableExtern.sol"; /// `@dev` We need a contract that is deployable in order to test the abstract /// base contract. Must override the function pointer virtuals to return /// non-empty, equal-length bytes so the constructor validation passes. -contract ChildRainlangExtern is BaseRainlangExtern { +contract ChildRainlangExtern is TestableExtern { function opcodeFunctionPointers() internal pure override returns (bytes memory) { return hex"0000"; } function integrityFunctionPointers() internal pure override returns (bytes memory) { return hex"0000"; } - - function buildIntegrityFunctionPointers() external pure returns (bytes memory) { - return integrityFunctionPointers(); - } - - function buildOpcodeFunctionPointers() external pure returns (bytes memory) { - return opcodeFunctionPointers(); - } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/abstract/ChildRainlangExtern.sol` around lines 10 - 26, ChildRainlangExtern duplicates the wrapper methods already provided by TestableExtern. Update ChildRainlangExtern to inherit from TestableExtern instead of BaseRainlangExtern so it can reuse the existing buildIntegrityFunctionPointers and buildOpcodeFunctionPointers wrappers, while keeping the pure overrides for integrityFunctionPointers and opcodeFunctionPointers intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/src/abstract/ChildRainlangSubParser.sol`:
- Around line 10-24: Add complete NatSpec comments to the fixture override
functions in ChildRainlangSubParser: describedByMetaV1,
buildLiteralParserFunctionPointers, buildOperandHandlerFunctionPointers, and
buildSubParserWordParsers. Update each function with the required documentation
tags so the test fixture satisfies the “complete NatSpec” guideline without
changing the function behavior.
- Around line 7-8: The doc-block in ChildRainlangSubParser’s file uses an
untagged continuation line under the `@dev` comment, which breaks the repo’s
convention. Update the comment so the second line is either folded into a single
tagged `@dev` line or rewritten as its own properly tagged doc-block text, keeping
the documentation style consistent with the existing comment format.
In `@test/src/abstract/HappyPathLiteralSubParser.sol`:
- Around line 38-52: Add complete NatSpec comments to the fixture override
functions in HappyPathLiteralSubParser: describedByMetaV1,
buildLiteralParserFunctionPointers, buildOperandHandlerFunctionPointers, and
buildSubParserWordParsers. Keep the existing behavior unchanged, but document
each function with appropriate `@notice/`@dev/@return tags so the test fixture
satisfies the project’s complete NatSpec requirement.
- Around line 13-15: The doc comment in HappyPathLiteralSubParser should not use
untagged continuation lines for the `@dev` note; rewrite the comment so each
wrapped line is properly tagged or otherwise formatted per the doc-block
convention. Update the comment above the parser fixture to keep the full
description in a compliant tagged form rather than letting the text on
subsequent lines implicitly continue `@dev`.
In `@test/src/abstract/TwoOpExtern.sol`:
- Around line 9-23: Add complete NatSpec doc comments to the fixture methods in
TwoOpExtern: opcodeFunctionPointers, integrityFunctionPointers,
buildIntegrityFunctionPointers, and buildOpcodeFunctionPointers. Keep the
existing behavior unchanged and add clear `@return` descriptions (and any relevant
`@notice/`@dev if needed) so these test helpers satisfy the audit/NatSpec
requirement.
In `@test/src/lib/parse/BadLengthSubParser.sol`:
- Around line 10-28: Add missing NatSpec to all public/external members in
BadLengthSubParser: the constructor, supportsInterface, subParseLiteral2, and
subParseWord2. Document each function’s purpose, parameters, and return values
in the same style used elsewhere in the test Solidity files, and ensure the
contract-level comment is not the only documentation present.
In `@test/src/lib/parse/ConstantReturningSubParser.sol`:
- Around line 9-10: Update ConstantReturningSubParser so each NatSpec block is
fully tagged: make the second line of the existing `@dev` and `@notice` descriptions
part of the same tagged comment instead of leaving it untagged, and add missing
NatSpec for supportsInterface and subParseLiteral2. Keep the documentation
attached to the relevant symbols in ConstantReturningSubParser so every
public/external function and the parser description satisfy the complete NatSpec
requirement.
In `@test/src/lib/parse/MultiConstantSubParser.sol`:
- Around line 14-16: Add complete NatSpec to MultiConstantSubParser so all
public/external functions and the split notice text are fully documented. In the
parser contract, update the documentation around supportsInterface and
subParseLiteral2 to include complete `@notice/`@param/@return tags, and rewrite
the two-line notice so each continued line is explicitly tagged rather than
relying on untagged continuation. Use the existing ConstantReturningSubParser
style as the reference for the required coverage and formatting.
---
Nitpick comments:
In `@test/src/abstract/ChildRainlangExtern.sol`:
- Around line 10-26: ChildRainlangExtern duplicates the wrapper methods already
provided by TestableExtern. Update ChildRainlangExtern to inherit from
TestableExtern instead of BaseRainlangExtern so it can reuse the existing
buildIntegrityFunctionPointers and buildOpcodeFunctionPointers wrappers, while
keeping the pure overrides for integrityFunctionPointers and
opcodeFunctionPointers intact.
In `@test/src/lib/integrity/IntegrityHighwater.sol`:
- Around line 9-45: The NatSpec on IntegrityHighwater is incomplete and has
mistagged continuation lines; update the contract docs so every descriptive line
in the top comment is explicitly tagged instead of relying on untagged
continuation. Add complete NatSpec for zeroInputTwoOutput, twoInputOneOutput,
buildIntegrityPointers, and runIntegrityCheck, using proper tags like
`@dev/`@return where applicable. Keep the documentation consistent with the
existing IntegrityHighwater and LibIntegrityCheck behavior so the test contract
satisfies the repo’s audit and explicit-tag requirements.
In `@test/src/lib/integrity/IntegritySingleOp.sol`:
- Around line 9-40: The NatSpec in IntegritySingleOp is incomplete and contains
an untagged continuation line in the existing `@dev` block, so update the contract
comment to make every line explicitly tagged and add proper doc comments for
oneInputOneOutput, buildIntegrityPointers, and runIntegrityCheck. Use the
contract name IntegritySingleOp and the three function names to locate the
missing documentation, and keep the comments audit-ready and consistent with the
same NatSpec requirements applied in IntegrityHighwater.sol.
In `@test/src/lib/state/LibInterpreterStateDataContractExtern.sol`:
- Around line 11-28: Complete the NatSpec for the external functions in
LibInterpreterStateDataContractExtern: add a proper function-level doc block for
deserialize and replace the loose contract comment with tagged NatSpec, then add
explicit `@param` entries for every argument and an `@return` description. Also
update deserializeStackLengths to include full NatSpec with `@param` serialized
and `@return`, keeping the documentation aligned with the function names so the
test file satisfies audit requirements.
- Around line 36-49: The backward scan in the helper inside
LibInterpreterStateDataContractExtern is unbounded and should fail fast if the
expected zero-initialized stack layout is not found. Update the assembly loop in
the array-length recovery logic to add a sane iteration cap or explicit
revert/assert path, and keep the special offset := 2 start with a clarifying
comment explaining why offset 1 is skipped. Use the
LibInterpreterStateDataContractExtern helper and its length-detection loop as
the target for the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fd9f961c-80e3-4011-a0bd-bf2defb10ada
⛔ Files ignored due to path filters (2)
src/generated/Rainlang.pointers.solis excluded by!**/generated/**src/generated/RainlangInterpreter.pointers.solis excluded by!**/generated/**
📒 Files selected for processing (39)
crates/bindings/abi/Rainlang.jsoncrates/test_fixtures/abi/Rainlang.jsoncrates/test_fixtures/abi/RainlangInterpreter.jsontest/src/abstract/BaseRainlangExtern.construction.t.soltest/src/abstract/BaseRainlangExtern.ierc165.t.soltest/src/abstract/BaseRainlangExtern.integrityOpcodeRange.t.soltest/src/abstract/BaseRainlangSubParser.ierc165.t.soltest/src/abstract/BaseRainlangSubParser.subParseLiteral2.t.soltest/src/abstract/BaseRainlangSubParser.subParseWord2.t.soltest/src/abstract/ChildRainlangExtern.soltest/src/abstract/ChildRainlangSubParser.soltest/src/abstract/EmptyPointersExtern.soltest/src/abstract/EmptyWordParsersSubParser.soltest/src/abstract/HappyPathLiteralSubParser.soltest/src/abstract/MismatchedExternMoreIntegrity.soltest/src/abstract/MismatchedExternMoreOpcodes.soltest/src/abstract/MismatchedLiteralSubParser.soltest/src/abstract/MismatchedWordSubParser.soltest/src/abstract/NoMatchLiteralSubParser.soltest/src/abstract/TestableExtern.soltest/src/abstract/TwoOpExtern.soltest/src/concrete/MockExternBadLiteralIndex.soltest/src/concrete/ModifierTestParser.soltest/src/concrete/RainlangInterpreter.zeroFunctionPointers.t.soltest/src/concrete/RainlangParser.parseMemoryOverflow.t.soltest/src/concrete/RainlangReferenceExtern.subParserIndexOutOfBounds.t.soltest/src/concrete/ZeroFPRainlangInterpreter.soltest/src/lib/integrity/IntegrityHighwater.soltest/src/lib/integrity/IntegritySingleOp.soltest/src/lib/integrity/LibIntegrityCheck.t.soltest/src/lib/parse/BadLengthSubParser.soltest/src/lib/parse/ConstantReturningSubParser.soltest/src/lib/parse/ContextReturningSubParser.soltest/src/lib/parse/LibSubParse.badSubParserResult.t.soltest/src/lib/parse/LibSubParse.constantAccumulation.t.soltest/src/lib/parse/LibSubParse.subParseWords.t.soltest/src/lib/parse/MultiConstantSubParser.soltest/src/lib/state/LibInterpreterStateDataContract.t.soltest/src/lib/state/LibInterpreterStateDataContractExtern.sol
✅ Files skipped from review due to trivial changes (5)
- test/src/abstract/EmptyPointersExtern.sol
- test/src/lib/parse/ContextReturningSubParser.sol
- test/src/concrete/ZeroFPRainlangInterpreter.sol
- test/src/lib/parse/LibSubParse.subParseWords.t.sol
- crates/test_fixtures/abi/Rainlang.json
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/bindings/abi/Rainlang.json
| /// @dev We need a contract that is deployable in order to test the abstract | ||
| /// base contract. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Untagged continuation line breaks the doc-block convention.
Line 8 continues the @dev text from line 7 without its own tag; per this repo's convention, untagged lines don't continue the previous tag.
As per coding guidelines, "untagged lines do not continue the previous tag".
📝 Proposed fix
-/// `@dev` We need a contract that is deployable in order to test the abstract
-/// base contract.
+/// `@dev` We need a contract that is deployable in order to test the abstract base contract.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// @dev We need a contract that is deployable in order to test the abstract | |
| /// base contract. | |
| /// `@dev` We need a contract that is deployable in order to test the abstract base contract. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/src/abstract/ChildRainlangSubParser.sol` around lines 7 - 8, The
doc-block in ChildRainlangSubParser’s file uses an untagged continuation line
under the `@dev` comment, which breaks the repo’s convention. Update the comment
so the second line is either folded into a single tagged `@dev` line or rewritten
as its own properly tagged doc-block text, keeping the documentation style
consistent with the existing comment format.
Source: Coding guidelines
| function describedByMetaV1() external pure override returns (bytes32) { | ||
| return 0; | ||
| } | ||
|
|
||
| function buildLiteralParserFunctionPointers() external pure returns (bytes memory) { | ||
| return new bytes(0); | ||
| } | ||
|
|
||
| function buildOperandHandlerFunctionPointers() external pure returns (bytes memory) { | ||
| return new bytes(0); | ||
| } | ||
|
|
||
| function buildSubParserWordParsers() external pure returns (bytes memory) { | ||
| return new bytes(0); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add NatSpec to the fixture's override functions.
describedByMetaV1, buildLiteralParserFunctionPointers, buildOperandHandlerFunctionPointers, and buildSubParserWordParsers have no doc comments.
As per coding guidelines, test files must include "complete NatSpec".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/src/abstract/ChildRainlangSubParser.sol` around lines 10 - 24, Add
complete NatSpec comments to the fixture override functions in
ChildRainlangSubParser: describedByMetaV1, buildLiteralParserFunctionPointers,
buildOperandHandlerFunctionPointers, and buildSubParserWordParsers. Update each
function with the required documentation tags so the test fixture satisfies the
“complete NatSpec” guideline without changing the function behavior.
Source: Coding guidelines
| /// @dev Sub parser where matchSubParseLiteralDispatch always succeeds at | ||
| /// index 0, returning a known dispatch value. subParserLiteralParsers has a | ||
| /// single valid function pointer to echoLiteralParser. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Untagged continuation lines break the doc-block convention.
Lines 14-15 continue the @dev text from line 13 without repeating the tag.
As per coding guidelines, "untagged lines do not continue the previous tag".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/src/abstract/HappyPathLiteralSubParser.sol` around lines 13 - 15, The
doc comment in HappyPathLiteralSubParser should not use untagged continuation
lines for the `@dev` note; rewrite the comment so each wrapped line is properly
tagged or otherwise formatted per the doc-block convention. Update the comment
above the parser fixture to keep the full description in a compliant tagged form
rather than letting the text on subsequent lines implicitly continue `@dev`.
Source: Coding guidelines
| function describedByMetaV1() external pure override returns (bytes32) { | ||
| return bytes32(0); | ||
| } | ||
|
|
||
| function buildLiteralParserFunctionPointers() external pure returns (bytes memory) { | ||
| return ""; | ||
| } | ||
|
|
||
| function buildOperandHandlerFunctionPointers() external pure returns (bytes memory) { | ||
| return ""; | ||
| } | ||
|
|
||
| function buildSubParserWordParsers() external pure returns (bytes memory) { | ||
| return ""; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add NatSpec to the fixture's override functions.
describedByMetaV1, buildLiteralParserFunctionPointers, buildOperandHandlerFunctionPointers, and buildSubParserWordParsers have no doc comments.
As per coding guidelines, test files must include "complete NatSpec".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/src/abstract/HappyPathLiteralSubParser.sol` around lines 38 - 52, Add
complete NatSpec comments to the fixture override functions in
HappyPathLiteralSubParser: describedByMetaV1,
buildLiteralParserFunctionPointers, buildOperandHandlerFunctionPointers, and
buildSubParserWordParsers. Keep the existing behavior unchanged, but document
each function with appropriate `@notice/`@dev/@return tags so the test fixture
satisfies the project’s complete NatSpec requirement.
Source: Coding guidelines
| function opcodeFunctionPointers() internal pure override returns (bytes memory) { | ||
| return hex"00010002"; | ||
| } | ||
|
|
||
| function integrityFunctionPointers() internal pure override returns (bytes memory) { | ||
| return hex"00010002"; | ||
| } | ||
|
|
||
| function buildIntegrityFunctionPointers() external pure returns (bytes memory) { | ||
| return integrityFunctionPointers(); | ||
| } | ||
|
|
||
| function buildOpcodeFunctionPointers() external pure returns (bytes memory) { | ||
| return opcodeFunctionPointers(); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add NatSpec to the fixture's functions.
None of opcodeFunctionPointers, integrityFunctionPointers, buildIntegrityFunctionPointers, or buildOpcodeFunctionPointers carry any doc comments.
As per coding guidelines, "All test files must meet audit requirements from the /audit skill, including tested error paths, complete NatSpec, and no audit findings".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/src/abstract/TwoOpExtern.sol` around lines 9 - 23, Add complete NatSpec
doc comments to the fixture methods in TwoOpExtern: opcodeFunctionPointers,
integrityFunctionPointers, buildIntegrityFunctionPointers, and
buildOpcodeFunctionPointers. Keep the existing behavior unchanged and add clear
`@return` descriptions (and any relevant `@notice/`@dev if needed) so these test
helpers satisfy the audit/NatSpec requirement.
Source: Coding guidelines
| contract BadLengthSubParser is ISubParserV4, IERC165 { | ||
| bytes public badBytecode; | ||
|
|
||
| constructor(bytes memory badBytecode_) { | ||
| badBytecode = badBytecode_; | ||
| } | ||
|
|
||
| function supportsInterface(bytes4 interfaceId) external pure override returns (bool) { | ||
| return interfaceId == type(ISubParserV4).interfaceId || interfaceId == type(IERC165).interfaceId; | ||
| } | ||
|
|
||
| function subParseLiteral2(bytes calldata) external pure override returns (bool, bytes32) { | ||
| return (false, 0); | ||
| } | ||
|
|
||
| function subParseWord2(bytes calldata) external view override returns (bool, bytes memory, bytes32[] memory) { | ||
| return (true, badBytecode, new bytes32[](0)); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add NatSpec to public members.
supportsInterface, subParseLiteral2, subParseWord2, and the constructor have no NatSpec, only the contract has a brief comment. As per coding guidelines, test/**/*.{sol,rs} files "must meet audit requirements from the /audit skill, including tested error paths, complete NatSpec, and no audit findings."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/src/lib/parse/BadLengthSubParser.sol` around lines 10 - 28, Add missing
NatSpec to all public/external members in BadLengthSubParser: the constructor,
supportsInterface, subParseLiteral2, and subParseWord2. Document each function’s
purpose, parameters, and return values in the same style used elsewhere in the
test Solidity files, and ensure the contract-level comment is not the only
documentation present.
Source: Coding guidelines
| /// @dev A sub parser that resolves any word by returning a constant opcode | ||
| /// with a known constant value. Each call returns exactly one constant. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
NatSpec continuation lines are untagged; some functions lack docs entirely.
Lines 9-10 and 22-23 each spread a single @dev/@notice description across two lines, but only the first line carries the tag. As per coding guidelines, "untagged lines do not continue the previous tag," so the second line of each block is effectively undocumented. Additionally, supportsInterface (Line 14) and subParseLiteral2 (Line 18) have no NatSpec at all, which conflicts with the "complete NatSpec" audit requirement for test files.
📝 Proposed fix for tag continuation
- /// `@dev` A sub parser that resolves any word by returning a constant opcode
- /// with a known constant value. Each call returns exactly one constant.
+ /// `@dev` A sub parser that resolves any word by returning a constant opcode
+ /// `@dev` with a known constant value. Each call returns exactly one constant.Also applies to: 14-16, 18-20, 22-23
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/src/lib/parse/ConstantReturningSubParser.sol` around lines 9 - 10,
Update ConstantReturningSubParser so each NatSpec block is fully tagged: make
the second line of the existing `@dev` and `@notice` descriptions part of the same
tagged comment instead of leaving it untagged, and add missing NatSpec for
supportsInterface and subParseLiteral2. Keep the documentation attached to the
relevant symbols in ConstantReturningSubParser so every public/external function
and the parser description satisfy the complete NatSpec requirement.
Source: Coding guidelines
| function supportsInterface(bytes4 interfaceId) external pure override returns (bool) { | ||
| return interfaceId == type(ISubParserV4).interfaceId || interfaceId == type(IERC165).interfaceId; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Same NatSpec tag-continuation and missing-doc gap as ConstantReturningSubParser.sol.
Lines 22-23 split a @notice description across two lines with only the first tagged, and supportsInterface/subParseLiteral2 (Lines 14-20) have no NatSpec. As per coding guidelines, "untagged lines do not continue the previous tag" and test files must have "complete NatSpec."
Also applies to: 18-20, 22-23
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/src/lib/parse/MultiConstantSubParser.sol` around lines 14 - 16, Add
complete NatSpec to MultiConstantSubParser so all public/external functions and
the split notice text are fully documented. In the parser contract, update the
documentation around supportsInterface and subParseLiteral2 to include complete
`@notice/`@param/@return tags, and rewrite the two-line notice so each continued
line is explicitly tagged rather than relying on untagged continuation. Use the
existing ConstantReturningSubParser style as the reference for the required
coverage and formatting.
Source: Coding guidelines
…d fixture abis The post-merge-update artifact regen (BuildPointers, forge build, CopyArtifacts, forge fmt in the CI-identical rainix sol-shell) drifts five committed artifacts; committing the regen clears the copy-artifacts diff and rs-test's commands::eval::tests::test_execute, which consumes the fixture abis (verified locally: cargo test green, 11/11 in rainlang-cli). Co-Authored-By: Claude <noreply@anthropic.com>
|
🤖 ai:producer |
|
🤖 ai:producer |
thedavidmeister
added
the
ai:ready
|
🤖 ai:vetter |
1 participant
Adds a
piconstant standard op, mirroring the existingeword (LibOpE).Closes #225
This is a deployed-interpreter-bytecode change (the same class as the
eword / #237). Adding the op shifts the interpreter/parser/expression-deployer/Rainlang bytecode, so the pinned deploy addresses and codehashes change. The fork-gatedLibInterpreterDeployTest/LibInterpreterDeployProdTesttests assert the live on-chain bytecode against these pins and will stay red on every network until the contracts are redeployed via the manual interpreter-redeploy workflow. Do not merge without human review + the redeploy at land.What changed (mirrors the
ePRs #246 / #375)LibOpEmirror:src/lib/op/math/LibOpPi.sol—integrity(0 in / 1 out),run(pushes the constant),referenceFn, identical in structure/gas toLibOpE.piis encoded as aFloatat the same precision/convention asLibDecimalFloat.FLOAT_E: a 67-digit coefficient × 10⁻⁶⁶, packedFloat.wrap(bytes32(0xffffffbe1dd4c9e873614f593bba9c6007d9a7ac8d03a4b6c700a65cb537a1b4)). (The same packing reproduces the publishedFLOAT_Ebyte-for-byte; thepivalue round-trips to π accurate to 66 decimal places.rain-math-float 0.1.1has noFLOAT_PI, so the constant is defined locally inLibOpPi, matching how the lib definesFLOAT_E.)piinsrc/lib/op/LibAllStandardOps.solbetweenmulandpoweracross all four parallel arrays (authoring meta, operand handlers, integrity pointers, opcode pointers);ALL_STANDARD_OPS_LENGTH72 → 73.test/src/lib/op/LibAllStandardOps.t.sol(piat index 59; later indices shifted).test/src/lib/op/math/LibOpPi.t.solmirroringLibOpE.t.sol(parse_: pi();+ eval to π, integrity, operand-disallowed, bad input/output arity).Regenerated artifacts (staged):
src/generated/RainlangInterpreter.pointers.solsrc/generated/RainlangParser.pointers.solsrc/generated/RainlangExpressionDeployer.pointers.solsrc/generated/Rainlang.pointers.solmeta/AuthoringMeta.rain.meta,meta/RainlangExpressionDeployer.rain.meta(
RainlangStore.pointers.solunchanged — store bytecode doesn't depend on opcodes. Pointer generation was iterated to a stable fixed point;rainix-sol-artifactsproduces no further diff and leaves the crate ABIs untouched.)New pinned values after regeneration:
0x7E694007Bb400ab2147a2060D2e48703F3FA1cef0x3069f10fd2956d0681277d4834e35e9fefaf6d3a079af0a44ccab979ffba052d0x088Bdb2B554E3CdedA4137bEeF92ffEe1e9F4DBf0x844ca2709f34283f566cee50abb1b948bf0e46fc62e1ff46df5659570186eaea0xC0BFfCcb665d3ebCa5Aba3816d94F35C8467d6f70x65c7c20b94c1878de8d25c6c0bd22ba470a9983719e274626e87e679b8da1df70x7c08C22E0700E9bB51082907514F8F68806fEEA00xe6cdd994c12e5626c2c0d6e0ad7c0fdf0bd290ea4c8a474e1f32ae040b12df6f0x1Aa775533E28B1D843e1A589034984E3a62005DC0xdaa0024dc105c6a9ea0838604bad0a5e662743eca97789c2cecfdf8667d0bf9bVerification
forge buildclean (only pre-existingboolean-cstlint warnings in unrelated files).forge fmt --checkclean; one-contract-per-file respected.*_RPC_URLenv vars (LibInterpreterDeploy*, plus fork-constructor math op tests) — environment-gated, identical onmain, and exactly the codehash checks the redeploy resolves.LibOpPiTest(7) +LibOpETest+LibAllStandardOpsTest+ filesystem-ordering all green.LibOpPi.runatFLOAT_Einstead ofFLOAT_PImakestestOpPiRunfail (run≠referenceFn); restored to π and re-confirmed green.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes