Skip to content

chore: pin pragma, rename word-count constant, use relative test imports - #194

Open
thedavidmeister wants to merge 7 commits into
mainfrom
2026-06-18-issue-28-55-136-146-149-pragma-imports
Open

chore: pin pragma, rename word-count constant, use relative test imports#194
thedavidmeister wants to merge 7 commits into
mainfrom
2026-06-18-issue-28-55-136-146-149-pragma-imports

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

No bytecode change (bytecode_hash = "none", cbor_metadata = false).

Test plan

  • forge build passes
  • forge test --no-match-test Fork passes (35/35)
  • copy-artifacts CI passes (pragma/rename/imports are all bytecode-neutral)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores

    • Standardized Solidity compiler settings to require version 0.8.25 exactly across the ERC4626 components and test utilities.
    • Consolidated length handling around a single shared word-count constant for ERC4626 pointer generation.
  • Tests

    • Updated ERC4626 tests to match the new word-count constant and adjusted length/revert expectations accordingly.

… use relative imports

Pin all src/ and test/ files to =0.8.25 (matching ERC4626Words.sol and all test
contracts). Rename OPCODE_FUNCTION_POINTERS_LENGTH to ERC4626_WORD_COUNT in
ERC4626Extern.sol to reflect shared word count rather than opcode-only size.
Replace bare src/ and test/ import paths in test files with relative paths so
tests remain resolvable when the repo is consumed as a soldeer dependency.

Closes #28, #55, #136, #146, #149

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Jun 18, 2026
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5bfde4e5-2191-4457-a04c-dc1a3284a2c7

📥 Commits

Reviewing files that changed from the base of the PR and between 7d1eb1c and 13c361b.

📒 Files selected for processing (10)
  • src/abstract/ERC4626Extern.sol
  • src/abstract/ERC4626SubParser.sol
  • src/lib/erc4626/LibERC4626.sol
  • src/lib/op/erc4626/LibOpERC4626ConvertToAssets.sol
  • src/lib/op/erc4626/LibOpERC4626ConvertToShares.sol
  • src/lib/parse/LibERC4626SubParser.sol
  • test/src/concrete/ERC4626Words.extern.t.sol
  • test/src/concrete/ERC4626Words.indexAlignment.t.sol
  • test/src/concrete/ERC4626WordsLengthConstants.t.sol
  • test/utils/MockERC4626.sol

Walkthrough

This PR pins Solidity pragma directives to exact version =0.8.25 across multiple source and test files, and renames the misleading OPCODE_FUNCTION_POINTERS_LENGTH constant in ERC4626Extern.sol to ERC4626_WORD_COUNT, updating all references in source and test files accordingly.

Changes

Pragma pin and constant rename

Layer / File(s) Summary
Rename length constant and update pointer builders
src/abstract/ERC4626Extern.sol
Replaces OPCODE_FUNCTION_POINTERS_LENGTH with ERC4626_WORD_COUNT = 2, updates opcode and integrity function-pointer array allocations to use it, and pins the pragma to =0.8.25.
Update tests for renamed constant
test/src/concrete/ERC4626Words.extern.t.sol, test/src/concrete/ERC4626Words.indexAlignment.t.sol, test/src/concrete/ERC4626WordsLengthConstants.t.sol
Test imports and assertions switch from OPCODE_FUNCTION_POINTERS_LENGTH to ERC4626_WORD_COUNT, covering length checks and out-of-range revert expectations.
Pin remaining pragmas
src/abstract/ERC4626SubParser.sol, src/lib/erc4626/LibERC4626.sol, src/lib/op/erc4626/LibOpERC4626ConvertToAssets.sol, src/lib/op/erc4626/LibOpERC4626ConvertToShares.sol, src/lib/parse/LibERC4626SubParser.sol, test/utils/MockERC4626.sol
Solidity pragma changed from caret ^0.8.25 to exact =0.8.25 with no logic changes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Related issues: #55 (pragma consistency), #28 (mis-named length constant)

Suggested labels: chore, consistency

Suggested reviewers: none identified from available context

Poem

A rabbit hopped through pragma lines,
Pinning versions, tidy signs.
Words renamed from length to count,
Consistency now paramount.
Hop, hop, done — the build's aligned! 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive The pragma pinning and constant rename align with #55 and #28, but the summary gives no evidence that the relative-import fixes for #136/#146/#149 were applied. Confirm the affected test files now use relative imports, or include the diff for those files so the linked-issue compliance can be verified.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the pragma pinning, constant rename, and test import changes in the PR.
Out of Scope Changes check ✅ Passed The described changes stay within the stated pragma, constant rename, and test-import cleanup objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 2026-06-18-issue-28-55-136-146-149-pragma-imports

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

thedavidmeister and others added 6 commits June 18, 2026 03:07
Split MockERC20 into test/utils/MockERC20.sol; update base imports.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…]: fix renamed constant import and forge fmt

Replace OPCODE_FUNCTION_POINTERS_LENGTH with ERC4626_WORD_COUNT in
ERC4626Words.extern.t.sol (import + 2 usages) to match the rename in
ERC4626Extern.sol from the previous commit. Add forgefmt: disable-next-line
comments to both function-pointer array declarations in ERC4626Extern.sol to
stabilize the oscillating forge formatter for this specific syntax pattern.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@thedavidmeister thedavidmeister added the ai:needs-work Needs rework — the producer's inbox (vetter verdict or human ruling) label Jul 6, 2026
@thedavidmeister

thedavidmeister commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

🤖 ai:vetter
Reviewed 13c361b: reject — six lib/abstract source files pinned to =0.8.25 violating org convention (^ for libs/abstract); rena
cost 150 — pragma convention violation on lib/abstract files

@thedavidmeister

Copy link
Copy Markdown
Contributor Author

🤖 ai:vetter
vet-protocol 4
lens source@13c361b2fd392f9b2e959c5b3029465bafbc450c + audit skill invoked at pr:194
Reviewed 13c361b: needs-work — Closes #28/#55/#136: the #28 rename and #136 test-import fixes are sound, but the #55 answer is inverted — six library/abstract files (ERC4626Extern, ERC4626SubParser, LibERC4626, both LibOp*, LibERC4626SubParser) are pinned =0.8.25 where the org convention floats ^ for lib/abstract, while the two concrete test mocks the convention does pin (test/utils/MaliciousERC4626.sol:3, MockERC20.sol:3) are left floating; pragma is answered per file kind, never by mass-pinning; QA evidence block missing.
cost 185 — repo-wide pragma sweep plus rename

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:needs-work Needs rework — the producer's inbox (vetter verdict or human ruling)

Projects

None yet

1 participant