Conversation
🦋 Changeset detectedLatest commit: d45215b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
This PR extends the Clones library with two new fetchCloneArgs overloads that return a slice of the immutable args appended to a clone’s runtime code, allowing callers to avoid allocating/copying the full args when only a subset is needed.
Changes:
- Added
Clones.fetchCloneArgs(address,uint256)(slice fromstartto end) andClones.fetchCloneArgs(address,uint256,uint256)(slice of at mostlengthbytes fromstart), with truncation semantics instead of reverting on out-of-range. - Refactored the existing
fetchCloneArgs(address)implementation to delegate to the new slicing overload for a single shared implementation. - Added Hardhat + Foundry coverage for in-range slicing and truncation behavior, and introduced a changeset entry for the new API.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
contracts/proxy/Clones.sol |
Adds the two slicing overloads and refactors the original fetchCloneArgs(address) to reuse the shared slicing implementation. |
test/proxy/Clones.test.js |
Updates overload invocation syntax and adds tests covering slice reads, “slice to end”, and truncation behavior. |
test/proxy/Clones.t.sol |
Adds fuzz tests validating truncation semantics for both new overloads. |
.changeset/clones-fetch-slice.md |
Documents the new overloads as a minor change in the release notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Walkthrough
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/proxy/Clones.t.sol`:
- Around line 88-90: Update both fuzz-test assumptions surrounding
Clones.cloneDeterministicWithImmutableArgs to bound args.length by the
deployable 0x5fd3 maximum instead of 0xbfd3, ensuring generated inputs cannot
cause deployment to revert before the slice assertions.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1d58fdca-c2fe-41a8-9008-b854e9ba28ff
📒 Files selected for processing (4)
.changeset/clones-fetch-slice.mdcontracts/proxy/Clones.soltest/proxy/Clones.t.soltest/proxy/Clones.test.js
f920335 to
6c04982
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
test/proxy/Clones.t.sol:88
- The clone helpers revert with
CloneArgumentsTooLong()whenargs.length > 0x5fd3(seeClones._cloneCodeWithImmutableArgs). Using0xbfd3here doesn't exclude those reverting cases and can make the fuzz test fragile if Foundry’s generatedbyteslengths increase in the future. Consider constraining the assume to the actual maximum supported args length.
vm.assume(args.length < 0xbfd3);
test/proxy/Clones.t.sol:104
- Same as above:
Clonesreverts forargs.length > 0x5fd3, so the fuzz precondition should cap to that limit to avoid generating reverting deployments.
vm.assume(args.length < 0xbfd3);
Adds two overloads of
Clones.fetchCloneArgsthat read only part of the immutable args attached to a clone, avoiding allocation/copy of the full array when only a portion is needed:fetchCloneArgs(address instance, uint256 start)— copies fromstart(included) to the end.fetchCloneArgs(address instance, uint256 start, uint256 length)— copies at mostlengthbytes starting atstart.Out-of-range arguments are truncated to the length of the immutable args (the returned array may be shorter than requested, or empty), matching the capping semantics of
Bytes.slice/Bytes.splicerather than reverting. The existingfetchCloneArgs(address)is refactored to delegate to the new slicing overload, so all three share a single implementation.PR Checklist