-
Notifications
You must be signed in to change notification settings - Fork 12.3k
Merge release-v5.5 branch #6034
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: ernestognw <[email protected]> Co-authored-by: Hadrien Croubois <[email protected]>
…itionalContext (#5961) Co-authored-by: Hadrien Croubois <[email protected]> Co-authored-by: ernestognw <[email protected]> Signed-off-by: Hadrien Croubois <[email protected]>
Signed-off-by: Hadrien Croubois <[email protected]>
Signed-off-by: Hadrien Croubois <[email protected]>
Co-authored-by: ernestognw <[email protected]> Signed-off-by: Hadrien Croubois <[email protected]>
…#5976) Co-authored-by: Hadrien Croubois <[email protected]> Signed-off-by: Hadrien Croubois <[email protected]>
Signed-off-by: Hadrien Croubois <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Ernesto García <[email protected]> Co-authored-by: Hadrien Croubois <[email protected]>
|
WalkthroughThis PR consolidates and releases OpenZeppelin Contracts version 5.5.0 by removing intermediate changeset entries and finalizing CHANGELOG.md. Key technical changes include: updating the Account validation flow to pass user operation signatures as a parameter to enable custom handling, adding slice and splice utility functions for value-type arrays (address[], bytes32[], uint256[]), implementing bounds checks in AccountERC7579 for insufficient calldata, tightening ERC1271 returndata validation, refactoring ECDSA.parse to handle both 64-byte and 65-byte signatures, and updating Solidity compiler version requirements from ^0.8.20 to ^0.8.24 across multiple files. OpenZeppelin contract version references are updated to v5.5.0 throughout the codebase. Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (1)
contracts/utils/Bytes.sol (1)
100-106: Clarify “replicates JS Array.splice” wording. JS splice returns removed elements; this function returns the modified buffer. Suggest making that explicit to avoid confusion.- * NOTE: replicates the behavior of ... [Javascript's `Array.splice`] + * NOTE: replicates the in-place effect of ... [JavaScript's `Array.splice`] on content/length. + * Unlike JavaScript, this function returns the modified buffer (not the removed segment).Also applies to: 114-116
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (153)
.changeset/afraid-chicken-attack.md(0 hunks).changeset/all-geese-stand.md(0 hunks).changeset/angry-waves-film.md(0 hunks).changeset/clear-tools-refuse.md(0 hunks).changeset/dull-students-eat.md(0 hunks).changeset/eight-radios-check.md(0 hunks).changeset/fast-beans-pull.md(0 hunks).changeset/funny-donuts-follow.md(0 hunks).changeset/itchy-turkeys-allow.md(0 hunks).changeset/loose-lamps-bake.md(0 hunks).changeset/major-feet-write.md(0 hunks).changeset/modern-moments-raise.md(0 hunks).changeset/new-days-tease.md(0 hunks).changeset/old-memes-dress.md(0 hunks).changeset/petite-seas-shake.md(0 hunks).changeset/plain-times-itch.md(0 hunks).changeset/public-crabs-heal.md(0 hunks).changeset/quick-pianos-press.md(0 hunks).changeset/rich-cows-repair.md(0 hunks).changeset/ripe-bears-hide.md(0 hunks).changeset/sharp-scissors-drum.md(0 hunks).changeset/shiny-dolphins-lick.md(0 hunks).changeset/silent-zebras-press.md(0 hunks).changeset/solid-cobras-talk.md(0 hunks).changeset/ten-steaks-try.md(0 hunks).changeset/tender-dolls-nail.md(0 hunks).changeset/three-parents-argue.md(0 hunks).changeset/violet-turtles-like.md(0 hunks).changeset/whole-cats-find.md(0 hunks).changeset/whole-plums-speak.md(0 hunks).changeset/wild-baths-buy.md(0 hunks).changeset/wise-webs-fly.md(0 hunks).changeset/witty-hats-flow.md(0 hunks)CHANGELOG.md(3 hunks)certora/specs/Account.spec(1 hunks)contracts/access/extensions/AccessControlDefaultAdminRules.sol(1 hunks)contracts/access/extensions/AccessControlEnumerable.sol(1 hunks)contracts/access/extensions/IAccessControlDefaultAdminRules.sol(1 hunks)contracts/access/manager/AccessManager.sol(1 hunks)contracts/access/manager/IAccessManager.sol(1 hunks)contracts/account/Account.sol(3 hunks)contracts/account/extensions/draft-AccountERC7579.sol(6 hunks)contracts/account/extensions/draft-ERC7821.sol(1 hunks)contracts/account/utils/EIP7702Utils.sol(1 hunks)contracts/account/utils/draft-ERC7579Utils.sol(1 hunks)contracts/crosschain/ERC7786Recipient.sol(1 hunks)contracts/finance/VestingWallet.sol(1 hunks)contracts/governance/Governor.sol(1 hunks)contracts/governance/IGovernor.sol(1 hunks)contracts/governance/TimelockController.sol(1 hunks)contracts/governance/extensions/GovernorProposalGuardian.sol(1 hunks)contracts/governance/extensions/GovernorStorage.sol(1 hunks)contracts/governance/extensions/GovernorSuperQuorum.sol(1 hunks)contracts/governance/extensions/GovernorTimelockCompound.sol(1 hunks)contracts/governance/extensions/GovernorTimelockControl.sol(1 hunks)contracts/governance/extensions/GovernorVotesQuorumFraction.sol(1 hunks)contracts/governance/extensions/GovernorVotesSuperQuorumFraction.sol(1 hunks)contracts/governance/utils/IVotes.sol(1 hunks)contracts/governance/utils/Votes.sol(1 hunks)contracts/governance/utils/VotesExtended.sol(1 hunks)contracts/interfaces/IERC3156FlashLender.sol(1 hunks)contracts/interfaces/IERC4626.sol(1 hunks)contracts/interfaces/IERC6909.sol(1 hunks)contracts/interfaces/IERC7751.sol(1 hunks)contracts/interfaces/draft-IERC6093.sol(1 hunks)contracts/interfaces/draft-IERC7579.sol(1 hunks)contracts/interfaces/draft-IERC7786.sol(1 hunks)contracts/interfaces/draft-IERC7802.sol(1 hunks)contracts/metatx/ERC2771Context.sol(1 hunks)contracts/metatx/ERC2771Forwarder.sol(1 hunks)contracts/mocks/Stateless.sol(1 hunks)contracts/mocks/account/AccountMock.sol(1 hunks)contracts/package.json(1 hunks)contracts/proxy/Clones.sol(1 hunks)contracts/proxy/Proxy.sol(1 hunks)contracts/proxy/transparent/TransparentUpgradeableProxy.sol(1 hunks)contracts/proxy/utils/UUPSUpgradeable.sol(1 hunks)contracts/token/ERC1155/ERC1155.sol(1 hunks)contracts/token/ERC1155/extensions/ERC1155Burnable.sol(1 hunks)contracts/token/ERC1155/extensions/ERC1155Pausable.sol(1 hunks)contracts/token/ERC1155/extensions/ERC1155Supply.sol(1 hunks)contracts/token/ERC1155/extensions/ERC1155URIStorage.sol(1 hunks)contracts/token/ERC1155/utils/ERC1155Holder.sol(1 hunks)contracts/token/ERC20/ERC20.sol(1 hunks)contracts/token/ERC20/extensions/ERC20Permit.sol(1 hunks)contracts/token/ERC20/extensions/ERC20Votes.sol(1 hunks)contracts/token/ERC20/extensions/ERC4626.sol(1 hunks)contracts/token/ERC20/extensions/IERC20Permit.sol(1 hunks)contracts/token/ERC20/utils/SafeERC20.sol(1 hunks)contracts/token/ERC6909/ERC6909.sol(1 hunks)contracts/token/ERC6909/extensions/ERC6909ContentURI.sol(1 hunks)contracts/token/ERC6909/extensions/ERC6909Metadata.sol(1 hunks)contracts/token/ERC6909/extensions/ERC6909TokenSupply.sol(1 hunks)contracts/token/ERC721/ERC721.sol(1 hunks)contracts/token/ERC721/extensions/ERC721Burnable.sol(1 hunks)contracts/token/ERC721/extensions/ERC721Consecutive.sol(1 hunks)contracts/token/ERC721/extensions/ERC721Enumerable.sol(1 hunks)contracts/token/ERC721/extensions/ERC721Pausable.sol(1 hunks)contracts/token/ERC721/extensions/ERC721Royalty.sol(1 hunks)contracts/token/ERC721/extensions/ERC721URIStorage.sol(1 hunks)contracts/token/ERC721/extensions/ERC721Votes.sol(1 hunks)contracts/token/ERC721/extensions/ERC721Wrapper.sol(1 hunks)contracts/token/ERC721/utils/ERC721Holder.sol(1 hunks)contracts/token/ERC721/utils/ERC721Utils.sol(1 hunks)contracts/utils/Address.sol(1 hunks)contracts/utils/Arrays.sol(2 hunks)contracts/utils/Base58.sol(1 hunks)contracts/utils/Base64.sol(1 hunks)contracts/utils/Blockhash.sol(1 hunks)contracts/utils/Bytes.sol(4 hunks)contracts/utils/Create2.sol(1 hunks)contracts/utils/LowLevelCall.sol(1 hunks)contracts/utils/Memory.sol(1 hunks)contracts/utils/Multicall.sol(1 hunks)contracts/utils/RLP.sol(1 hunks)contracts/utils/ReentrancyGuard.sol(1 hunks)contracts/utils/ReentrancyGuardTransient.sol(1 hunks)contracts/utils/RelayedCall.sol(1 hunks)contracts/utils/ShortStrings.sol(1 hunks)contracts/utils/SlotDerivation.sol(1 hunks)contracts/utils/Strings.sol(1 hunks)contracts/utils/cryptography/ECDSA.sol(2 hunks)contracts/utils/cryptography/EIP712.sol(1 hunks)contracts/utils/cryptography/MessageHashUtils.sol(1 hunks)contracts/utils/cryptography/SignatureChecker.sol(2 hunks)contracts/utils/cryptography/WebAuthn.sol(1 hunks)contracts/utils/cryptography/draft-ERC7739Utils.sol(1 hunks)contracts/utils/cryptography/signers/SignerEIP7702.sol(1 hunks)contracts/utils/cryptography/signers/SignerWebAuthn.sol(1 hunks)contracts/utils/cryptography/signers/draft-ERC7739.sol(1 hunks)contracts/utils/cryptography/verifiers/ERC7913P256Verifier.sol(1 hunks)contracts/utils/cryptography/verifiers/ERC7913RSAVerifier.sol(1 hunks)contracts/utils/cryptography/verifiers/ERC7913WebAuthnVerifier.sol(2 hunks)contracts/utils/draft-InteroperableAddress.sol(1 hunks)contracts/utils/introspection/ERC165Checker.sol(1 hunks)contracts/utils/math/Math.sol(1 hunks)contracts/utils/structs/Accumulators.sol(1 hunks)contracts/utils/structs/Checkpoints.sol(1 hunks)contracts/utils/structs/CircularBuffer.sol(1 hunks)contracts/utils/structs/EnumerableMap.sol(1 hunks)contracts/utils/structs/EnumerableSet.sol(1 hunks)contracts/utils/structs/Heap.sol(1 hunks)contracts/utils/structs/MerkleTree.sol(1 hunks)contracts/utils/types/Time.sol(1 hunks)package.json(1 hunks)scripts/generate/templates/Arrays.js(3 hunks)scripts/generate/templates/EnumerableMap.js(1 hunks)scripts/generate/templates/EnumerableSet.js(1 hunks)scripts/release/workflow/github-release.js(1 hunks)test/account/extensions/AccountERC7579.behavior.js(3 hunks)test/utils/Arrays.t.sol(2 hunks)test/utils/Arrays.test.js(6 hunks)test/utils/Bytes.t.sol(1 hunks)
💤 Files with no reviewable changes (33)
- .changeset/old-memes-dress.md
- .changeset/fast-beans-pull.md
- .changeset/silent-zebras-press.md
- .changeset/modern-moments-raise.md
- .changeset/wise-webs-fly.md
- .changeset/major-feet-write.md
- .changeset/loose-lamps-bake.md
- .changeset/wild-baths-buy.md
- .changeset/shiny-dolphins-lick.md
- .changeset/solid-cobras-talk.md
- .changeset/funny-donuts-follow.md
- .changeset/ripe-bears-hide.md
- .changeset/plain-times-itch.md
- .changeset/dull-students-eat.md
- .changeset/tender-dolls-nail.md
- .changeset/violet-turtles-like.md
- .changeset/all-geese-stand.md
- .changeset/ten-steaks-try.md
- .changeset/public-crabs-heal.md
- .changeset/three-parents-argue.md
- .changeset/whole-plums-speak.md
- .changeset/whole-cats-find.md
- .changeset/quick-pianos-press.md
- .changeset/petite-seas-shake.md
- .changeset/eight-radios-check.md
- .changeset/new-days-tease.md
- .changeset/afraid-chicken-attack.md
- .changeset/witty-hats-flow.md
- .changeset/angry-waves-film.md
- .changeset/rich-cows-repair.md
- .changeset/clear-tools-refuse.md
- .changeset/sharp-scissors-drum.md
- .changeset/itchy-turkeys-allow.md
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-08-28T16:58:18.879Z
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5904
File: contracts/mocks/crosschain/ERC7786GatewayMock.sol:27-31
Timestamp: 2025-08-28T16:58:18.879Z
Learning: In the OpenZeppelin ERC7786 implementation, the `UnsupportedAttribute(bytes4 selector)` error is defined in the `IERC7786GatewaySource` interface, making it directly accessible to inheriting contracts without needing interface qualification.
Applied to files:
contracts/interfaces/draft-IERC7786.solcontracts/interfaces/draft-IERC7579.solcontracts/interfaces/draft-IERC6093.solcontracts/interfaces/draft-IERC7802.solcontracts/interfaces/IERC7751.solcontracts/account/extensions/draft-AccountERC7579.sol
📚 Learning: 2025-08-29T13:16:08.640Z
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5904
File: contracts/mocks/crosschain/ERC7786RecipientMock.sol:12-14
Timestamp: 2025-08-29T13:16:08.640Z
Learning: In OpenZeppelin contracts, mock contracts (like ERC7786RecipientMock) don't require input validation such as zero-address checks in constructors, as they are only used for testing purposes in controlled environments.
Applied to files:
contracts/interfaces/draft-IERC7786.solcontracts/interfaces/draft-IERC7579.solcontracts/crosschain/ERC7786Recipient.solcontracts/interfaces/draft-IERC7802.sol
📚 Learning: 2025-09-04T09:13:21.278Z
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5904
File: contracts/crosschain/ERC7786Recipient.sol:3-3
Timestamp: 2025-09-04T09:13:21.278Z
Learning: In OpenZeppelin contracts, hardhat.config.js uses a sophisticated yargs-based configuration where the Solidity compiler version is set via argv.compiler (line 77) with a default of '0.8.27' defined in the yargs options (line 21), allowing flexible command-line overrides while maintaining a consistent default.
Applied to files:
package.jsonscripts/generate/templates/Arrays.js
📚 Learning: 2025-10-15T02:52:05.027Z
Learnt from: ernestognw
Repo: OpenZeppelin/openzeppelin-contracts PR: 5891
File: test/account/modules/ERC7579Module.behavior.js:56-61
Timestamp: 2025-10-15T02:52:05.027Z
Learning: In ERC7579 validator tests for `isValidSignatureWithSender`, using `this.mock` (not bound to a specific account) is valid when testing signature validation with any arbitrary sender, while `this.mockFromAccount` is used when testing account-specific validation scenarios.
Applied to files:
contracts/mocks/Stateless.solcontracts/mocks/account/AccountMock.solCHANGELOG.mdcontracts/account/extensions/draft-AccountERC7579.sol
📚 Learning: 2025-10-03T13:14:57.679Z
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5914
File: contracts/crosschain/bridges/BridgeERC20.sol:57-58
Timestamp: 2025-10-03T13:14:57.679Z
Learning: In cross-chain bridge contracts like BridgeERC20, when processing incoming messages in _processMessage, avoid validation checks that revert on malformed addresses. Reverting would create cross-chain inconsistency where tokens are locked/burned on the source chain but never minted on the destination. Instead, use best-effort address extraction (e.g., address(bytes20(toBinary))) to maintain atomicity across chains. If tokens are minted to an incorrect address due to user error, recovery may be possible through admin controls rather than leaving funds permanently locked.
Applied to files:
CHANGELOG.mdcontracts/utils/draft-InteroperableAddress.sol
📚 Learning: 2025-08-28T16:58:18.879Z
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5904
File: contracts/mocks/crosschain/ERC7786GatewayMock.sol:27-31
Timestamp: 2025-08-28T16:58:18.879Z
Learning: When a Solidity contract inherits from an interface, it has direct access to all declarations in that interface including errors, without needing to qualify them with the interface name.
Applied to files:
contracts/interfaces/draft-IERC6093.sol
📚 Learning: 2025-09-04T09:13:21.278Z
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5904
File: contracts/crosschain/ERC7786Recipient.sol:3-3
Timestamp: 2025-09-04T09:13:21.278Z
Learning: In OpenZeppelin contracts, hardhat.config.js uses yargs to configure the Solidity compiler version dynamically via command line arguments, with a default of '0.8.27' set on line 21 and the solidity configuration using argv.compile around lines 76-87, rather than hardcoded version strings.
Applied to files:
scripts/generate/templates/Arrays.js
📚 Learning: 2025-09-02T16:12:17.728Z
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5892
File: contracts/utils/ReentrancyGuard.sol:114-116
Timestamp: 2025-09-02T16:12:17.728Z
Learning: In diamond proxy architectures, `address(this)` returns the proxy address (same for all modules), not individual module addresses, so it's ineffective for storage slot isolation between modules. Function signatures (`msg.sig`) provide better isolation and are available in pure functions.
Applied to files:
contracts/account/extensions/draft-AccountERC7579.sol
🧬 Code graph analysis (3)
test/utils/Arrays.test.js (1)
test/helpers/random.js (1)
generators(3-10)
test/account/extensions/AccountERC7579.behavior.js (1)
test/helpers/erc7579.js (1)
MODULE_TYPE_FALLBACK(5-5)
scripts/generate/templates/Arrays.js (1)
scripts/generate/templates/Arrays.opts.js (1)
TYPES(1-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - solidity-contracts
- GitHub Check: Header rules - solidity-contracts
- GitHub Check: Pages changed - solidity-contracts
- GitHub Check: tests
- GitHub Check: slither
- GitHub Check: coverage
- GitHub Check: tests-upgradeable
- GitHub Check: tests-foundry
🔇 Additional comments (148)
contracts/token/ERC721/utils/ERC721Utils.sol (1)
2-2: Version header updated correctly for v5.5.0 release.This is a straightforward documentation update as part of the release process. No functional changes.
contracts/token/ERC20/utils/SafeERC20.sol (1)
2-2: Version bump for release v5.5.0.Version comment correctly updated as part of the v5.5.0 release.
contracts/interfaces/IERC3156FlashLender.sol (1)
2-2: LGTM!The header version bump from v5.4.0 to v5.5.0 is correct and aligns with the PR objective for the release consolidation. The interface definition and all method signatures remain unchanged.
contracts/token/ERC721/extensions/ERC721Consecutive.sol (1)
2-2: Version header and pragma updates are aligned with release v5.5.0.The header comment correctly references v5.5.0, and the Solidity pragma ^0.8.24 is consistent with the PR's stated compiler version updates. This is an appropriate metadata-only change for the release consolidation.
Also applies to: 4-4
contracts/interfaces/IERC4626.sol (1)
2-2: Version metadata updated for release.The header comment correctly reflects the version bump to v5.5.0, aligning with the release consolidation PR objectives. The interface definition itself remains unchanged.
contracts/utils/RelayedCall.sol (1)
2-2: Version header updated correctly for v5.5.0 release.The header comment has been updated to reference the v5.5.0 version as part of the release consolidation. This documentation change aligns with the PR objectives and follows OpenZeppelin's standard header conventions.
contracts/utils/cryptography/MessageHashUtils.sol (1)
2-2: Version header correctly updated to v5.5.0.The header comment reflects the new version as part of the v5.5.0 release consolidation. No functional changes in this file—all cryptography utility functions remain intact and correctly implemented.
contracts/interfaces/draft-IERC7786.sol (1)
2-2: Version header update for v5.5.0 release – LGTM!The header comment update aligns with the PR's objective to consolidate OpenZeppelin Contracts v5.5.0. The interface definitions and public APIs remain unchanged.
contracts/interfaces/draft-IERC7802.sol (1)
2-2: Version metadata updated correctly for v5.5.0 release.The header comment has been appropriately updated to reflect the release version without any changes to the interface logic or signatures.
contracts/access/extensions/IAccessControlDefaultAdminRules.sol (1)
2-2: Version metadata update is correct.The header comment version has been properly updated from v5.4.0 to v5.5.0 as part of the v5.5.0 release consolidation. No interface changes, no functional updates.
contracts/finance/VestingWallet.sol (2)
2-2: Header version update is appropriate.The OpenZeppelin version in the contract header has been correctly bumped to v5.5.0 as part of the release consolidation.
3-3: Verify pragma version consistency with other PR changes.The AI summary indicates this PR updates Solidity compiler version requirements from
^0.8.20to^0.8.24across multiple files. However,VestingWallet.solstill specifies^0.8.20. Confirm whether this file should also be updated to^0.8.24for consistency with the broader version bump in this release.contracts/interfaces/draft-IERC6093.sol (1)
2-2: Version update is correct.The header comment has been appropriately updated from v5.4.0 to v5.5.0 to reflect the release version. The file contains well-defined error interfaces for ERC-20, ERC-721, and ERC-1155 tokens consistent with EIP-6093.
contracts/utils/introspection/ERC165Checker.sol (1)
2-2: LGTM!The header comment version update to v5.5.0 is appropriate for this release consolidation. No functional changes to review.
contracts/utils/types/Time.sol (1)
2-2: ✓ Version header update approved.The version header has been correctly updated from v5.1.0 to v5.5.0 as part of the release merge. No functional changes in this file.
contracts/utils/ShortStrings.sol (1)
2-2: ✓ Version header updated appropriately for release.The version annotation has been correctly updated to v5.5.0 as part of the release merge. No functional changes to this file.
contracts/utils/Create2.sol (2)
2-2: Header version updated appropriately for v5.5.0 release.The OpenZeppelin Contracts version in the header comment has been correctly updated from v5.1.0 to v5.5.0, aligning with the release consolidation.
4-4: Verify pragma solidity version consistency with release requirements.The AI summary indicates that Solidity compiler version requirements are being updated to
^0.8.24across multiple files in this release. However, this file's pragma on line 4 remains at^0.8.20.Please confirm whether:
- This file intentionally should remain at
^0.8.20, or- The pragma solidity version should also be updated to
^0.8.24for consistency with the v5.5.0 release.contracts/utils/LowLevelCall.sol (1)
2-2: Version header update looks good.The header comment is correctly updated to reflect the v5.5.0 release as per the PR objective.
contracts/token/ERC721/extensions/ERC721URIStorage.sol (1)
2-2: Version header correctly updated to v5.5.0.The header comment bump is consistent with the release consolidation and requires no further action.
contracts/utils/structs/Accumulators.sol (1)
2-2: Version header update is correct.This routine version bump to v5.5.0 is appropriate for the release merge and aligns with pragma updates across the PR. No functional changes.
contracts/interfaces/IERC7751.sol (2)
2-2: Header metadata added correctly.The OpenZeppelin version header follows the standard convention for tracking versioning information across the codebase.
10-12: Clean ERC-7751 interface definition.The
WrappedErrorerror captures the necessary context for wrapping bubbled-up reverts: target address, function selector, reason bytes, and additional details. This aligns with the ERC-7751 standard for custom error handling.contracts/utils/cryptography/EIP712.sol (2)
2-2: Version header update for release v5.5.0 looks good.The version annotation update from v5.4.0 to v5.5.0 is appropriate and consistent with the release consolidation. This is a standard metadata change for a release merge.
4-4: No action needed—pragma already at target version.The pragma in EIP712.sol is correctly set to
^0.8.24, which aligns with the PR's stated updates. Verification confirms the file is already at the intended version. No changes are required for this file.Note: The codebase still contains ~170+ files on
^0.8.20, indicating the pragma updates are partial. Confirm this aligns with the PR's intended scope if not already covered in related changes.contracts/utils/structs/EnumerableSet.sol (3)
2-2: Version and pragma updates look good.Header and compiler pragma have been bumped as part of the v5.5.0 release cycle.
Also applies to: 5-5
133-139: Logic integrity preserved with new utility functions.The _clear and _values slice functions maintain their existing safety properties:
- Bounds are properly checked using Math.min before unsafe access
- Unchecked blocks contain only arithmetic that has been pre-validated
- Position deletion still occurs before array truncation
Also applies to: 189-201, 637-649, 779-791
138-138: No issues found - Array utility functions are properly implemented and available.The
unsafeAccess()andunsafeSetLength()utilities referenced across all six lines (138, 197, 586, 645, 728, 787) are documented functions in OpenZeppelin Contracts v5.5.0. These functions return storage slot structs (AddressSlot, Bytes32Slot, Uint256Slot) that provide a.valueaccessor, which correctly matches their usage pattern in the EnumerableSet code.contracts/utils/Base64.sol (1)
2-2: LGTM! Version header updated for v5.5.0 release.Standard version header update consistent with the release merge.
contracts/utils/ReentrancyGuard.sol (1)
2-2: LGTM! Version header updated for v5.5.0 release.contracts/token/ERC721/extensions/ERC721Wrapper.sol (1)
2-2: LGTM! Version header updated for v5.5.0 release.contracts/token/ERC6909/ERC6909.sol (1)
2-2: LGTM! Version header updated for v5.5.0 release.package.json (1)
4-4: LGTM! Package version bumped to 5.5.0.Version bump aligns with the release merge.
contracts/governance/extensions/GovernorTimelockCompound.sol (1)
2-2: LGTM! Version header updated for v5.5.0 release.contracts/access/extensions/AccessControlEnumerable.sol (2)
2-2: LGTM! Version header updated for v5.5.0 release.
4-4: Verify pragma update is consistent with rollout plan.This file's pragma is updated to ^0.8.24, while other files in this PR (Base64.sol, ReentrancyGuard.sol, ERC6909.sol, Checkpoints.sol) remain on ^0.8.20. The AI summary mentions selective updates to ^0.8.24. Confirm this is intentional and aligns with the project's phased upgrade strategy.
contracts/utils/structs/Checkpoints.sol (1)
2-2: LGTM! Version header updated for v5.5.0 release.contracts/access/extensions/AccessControlDefaultAdminRules.sol (1)
2-2: Version header update approved.The OpenZeppelin Contracts version reference has been correctly updated to v5.5.0 as part of the release consolidation.
contracts/governance/extensions/GovernorVotesQuorumFraction.sol (1)
2-2: Version header update approved.The OpenZeppelin Contracts version reference has been correctly updated to v5.5.0.
contracts/token/ERC20/ERC20.sol (1)
2-2: Version header update approved.The OpenZeppelin Contracts version reference has been correctly updated to v5.5.0 for the core ERC20 implementation.
contracts/metatx/ERC2771Context.sol (1)
2-2: Version header update approved.The OpenZeppelin Contracts version reference has been correctly updated to v5.5.0.
contracts/token/ERC721/utils/ERC721Holder.sol (1)
2-2: Version header update approved.The OpenZeppelin Contracts version reference has been correctly updated to v5.5.0.
contracts/token/ERC721/extensions/ERC721Burnable.sol (1)
2-2: Version header update approved.The OpenZeppelin Contracts version reference has been correctly updated to v5.5.0.
contracts/governance/extensions/GovernorSuperQuorum.sol (1)
2-2: Version header update approved.The OpenZeppelin Contracts version reference has been correctly updated to v5.5.0.
contracts/governance/extensions/GovernorTimelockControl.sol (1)
2-2: Version header update approved.The OpenZeppelin Contracts version reference has been correctly updated to v5.5.0 as part of the release consolidation.
contracts/utils/Multicall.sol (1)
2-2: LGTM! Version header updated for v5.5.0 release.The OpenZeppelin header comment has been appropriately updated to reflect the v5.5.0 release. No functional changes.
contracts/utils/RLP.sol (2)
2-2: LGTM! Version header updated for v5.5.0 release.
4-4: The review comment characterization of this pragma change as "unusual" is incorrect; however, the downgrade claim requires PR diff verification.The codebase exhibits an intentional multi-tier pragma strategy across 11 files at
^0.8.26(including RLP.sol, draft-InteroperableAddress.sol, MultiSignerERC7913.sol, and others), so this is not isolated or anomalous. However, the review's claim about a downgrade from^0.8.27cannot be confirmed without seeing the original pragma in the PR diff. Additionally, the PR summary claims "updating from^0.8.20to^0.8.24" but the actual codebase contains pragmas spanning 20–27 across different modules, suggesting either the summary is incomplete or RLP.sol's change contradicts stated goals.Verify:
- The original pragma version of RLP.sol before this change (check PR diff).
- Whether the multi-tier pragma distribution (20, 21, 22, 24, 26, 27) aligns with the stated PR objectives.
contracts/token/ERC20/extensions/ERC4626.sol (1)
2-2: LGTM! Version header updated for v5.5.0 release.The OpenZeppelin header comment has been appropriately updated to reflect the v5.5.0 release. No functional changes.
contracts/crosschain/ERC7786Recipient.sol (2)
2-2: LGTM! Version header updated for v5.5.0 release.
4-4: No pragma change occurred—the review comment is incorrect.The git diff confirms that
ERC7786Recipient.solhas maintainedpragma solidity ^0.8.20;from the master branch to this PR. The only modification is adding an OpenZeppelin header comment. There is no downgrade from ^0.8.27 or any other version change.Likely an incorrect or invalid review comment.
contracts/governance/extensions/GovernorStorage.sol (1)
2-2: LGTM! Version header updated for v5.5.0 release.The OpenZeppelin header comment has been appropriately updated to reflect the v5.5.0 release. No functional changes.
contracts/account/utils/EIP7702Utils.sol (1)
2-2: LGTM! Version header updated for v5.5.0 release.The OpenZeppelin header comment has been appropriately updated to reflect the v5.5.0 release. No functional changes.
contracts/token/ERC721/extensions/ERC721Enumerable.sol (1)
2-2: LGTM! Version header updated for v5.5.0 release.The OpenZeppelin header comment has been appropriately updated to reflect the v5.5.0 release. No functional changes.
scripts/release/workflow/github-release.js (1)
12-12: LGTM! Improved release targeting using commit SHA.The change from
github.ref_nametocontext.shaensures the release is created targeting a specific commit SHA rather than a branch/tag reference. This provides better determinism and reliability for the release process.contracts/token/ERC1155/extensions/ERC1155URIStorage.sol (1)
2-2: Version metadata update looks good.The header comment correctly reflects the v5.5.0 release. This is a standard metadata update with no functional changes.
contracts/utils/cryptography/signers/SignerEIP7702.sol (1)
2-2: Version metadata update looks good.The header comment correctly reflects the v5.5.0 release. This is a standard metadata update with no functional changes.
contracts/governance/extensions/GovernorProposalGuardian.sol (1)
2-2: Version metadata update looks good.The header comment correctly reflects the v5.5.0 release. This is a standard metadata update with no functional changes.
contracts/utils/Strings.sol (1)
2-2: Version metadata update looks good.The header comment correctly reflects the v5.5.0 release. This is a standard metadata update with no functional changes.
contracts/utils/Blockhash.sol (1)
2-2: Version metadata update looks good.The header comment correctly reflects the v5.5.0 release. This is a standard metadata update with no functional changes.
contracts/token/ERC6909/extensions/ERC6909Metadata.sol (1)
2-2: Version metadata update looks good.The header comment correctly reflects the v5.5.0 release. This is a standard metadata update with no functional changes.
contracts/token/ERC1155/utils/ERC1155Holder.sol (1)
2-2: Version metadata update looks good.The header comment correctly reflects the v5.5.0 release. This is a standard metadata update with no functional changes.
contracts/proxy/utils/UUPSUpgradeable.sol (1)
2-2: Version metadata update looks good.The header comment correctly reflects the v5.5.0 release. This is a standard metadata update with no functional changes.
contracts/token/ERC6909/extensions/ERC6909TokenSupply.sol (1)
2-2: LGTM - Version metadata update for v5.5.0 release.The header comment update to v5.5.0 is appropriate for this release consolidation.
contracts/utils/cryptography/draft-ERC7739Utils.sol (1)
2-2: LGTM - Version metadata update for v5.5.0 release.The header comment update to v5.5.0 is appropriate for this release consolidation.
contracts/metatx/ERC2771Forwarder.sol (1)
2-2: LGTM - Version metadata update for v5.5.0 release.The header comment update to v5.5.0 is appropriate for this release consolidation.
contracts/utils/ReentrancyGuardTransient.sol (1)
2-2: LGTM - Version metadata update for v5.5.0 release.The header comment update to v5.5.0 is appropriate for this release consolidation.
contracts/token/ERC721/extensions/ERC721Pausable.sol (1)
2-2: LGTM - Version metadata update for v5.5.0 release.The header comment update to v5.5.0 is appropriate for this release consolidation.
scripts/generate/templates/EnumerableSet.js (1)
6-6: LGTM - Solidity version update to ^0.8.24.The pragma update is consistent with the broader compiler requirement changes mentioned in the PR summary.
contracts/access/manager/AccessManager.sol (1)
2-2: LGTM - Version metadata update for v5.5.0 release.The header comment update to v5.5.0 is appropriate for this release consolidation.
contracts/token/ERC20/extensions/ERC20Votes.sol (1)
2-2: LGTM - Version metadata update for v5.5.0 release.The header comment update to v5.5.0 is appropriate for this release consolidation.
contracts/governance/extensions/GovernorVotesSuperQuorumFraction.sol (1)
2-2: Header version bump looks good. Metadata update aligns with the 5.5.0 release tag; no logic changes involved.contracts/utils/Address.sol (1)
2-2: Header version bump looks good. Comment now reflects v5.5.0 while code stays untouched, which is consistent with the release merge.contracts/proxy/transparent/TransparentUpgradeableProxy.sol (1)
2-2: Header version bump looks good. Updated metadata matches the release consolidation; no functional adjustments needed.contracts/utils/cryptography/signers/draft-ERC7739.sol (1)
2-2: Header version bump looks good. Comment update to v5.5.0 is consistent with the release merge and introduces no behavioral change.contracts/utils/math/Math.sol (1)
2-2: Header version bump looks good. Metadata now points to v5.5.0, with the remainder of the file unchanged.contracts/utils/cryptography/WebAuthn.sol (1)
2-2: LGTM: Version header updated.Metadata update consistent with the v5.5.0 release.
contracts/proxy/Proxy.sol (1)
2-2: LGTM: Version header updated.Metadata update consistent with the v5.5.0 release.
contracts/token/ERC721/extensions/ERC721Votes.sol (1)
2-2: LGTM: Version header updated.Metadata update consistent with the v5.5.0 release.
scripts/generate/templates/EnumerableMap.js (1)
6-6: LGTM: Pragma version updated in template.Template updated to generate contracts with ^0.8.24, aligning with the updated compiler requirements for v5.5.0.
contracts/utils/Memory.sol (1)
2-2: LGTM: Version header added.Metadata addition consistent with the v5.5.0 release.
contracts/utils/structs/MerkleTree.sol (1)
2-4: LGTM: Version header and pragma updated.Metadata and compiler version updates consistent with the v5.5.0 release requirements.
contracts/token/ERC721/extensions/ERC721Royalty.sol (1)
2-2: LGTM: Version header updated.Metadata update consistent with the v5.5.0 release.
contracts/proxy/Clones.sol (1)
2-2: LGTM: Version header updated.Metadata update consistent with the v5.5.0 release.
contracts/interfaces/IERC6909.sol (1)
2-2: LGTM: Version header updated for v5.5.0 release.The metadata update is consistent with the version bump.
contracts/token/ERC721/ERC721.sol (1)
2-2: LGTM: Version header updated for v5.5.0 release.The metadata update is consistent with the version bump.
contracts/utils/structs/Heap.sol (1)
2-4: LGTM: Version header and compiler pragma updated for v5.5.0 release.The metadata and pragma updates are consistent with the version bump.
contracts/package.json (1)
4-4: LGTM: Package version updated to 5.5.0.The version bump is consistent with the v5.5.0 release.
contracts/access/manager/IAccessManager.sol (1)
2-2: LGTM: Version header updated for v5.5.0 release.The metadata update is consistent with the version bump.
contracts/token/ERC6909/extensions/ERC6909ContentURI.sol (2)
2-2: LGTM: Version header updated for v5.5.0 release.The metadata update is consistent with the version bump.
4-4: Inconsistency confirmed; verify intentionality with developer.The pragma version inconsistency is real:
ERC6909ContentURI.solhas^0.8.20whileHeap.solandCircularBuffer.soluse^0.8.24. However, sibling files in the ERC6909 extensions directory (ERC6909TokenSupply.sol,ERC6909Metadata.sol) also remain at^0.8.20, suggesting the ERC6909 module may have been intentionally excluded from the ^0.8.24 upgrade. Confirm whether the ERC6909 extensions should be updated as part of this PR or if the selective version upgrade was deliberate.contracts/utils/structs/CircularBuffer.sol (1)
2-4: LGTM: Version header and compiler pragma updated for v5.5.0 release.The metadata and pragma updates are consistent with the version bump.
contracts/utils/SlotDerivation.sol (2)
2-2: LGTM: Version header updated for v5.5.0 release.The metadata update is consistent with the version bump.
5-5: Verify pragma version consistency across the codebase.Similar to ERC6909ContentURI.sol, this file retains
pragma solidity ^0.8.20while other files in the PR have been updated to^0.8.24. Ensure this is intentional and consistent with the project's compiler version strategy.contracts/token/ERC1155/extensions/ERC1155Burnable.sol (1)
2-4: LGTM! Consistent version metadata updates.The header and pragma updates align with the v5.5.0 release requirements.
contracts/token/ERC1155/extensions/ERC1155Supply.sol (1)
2-4: LGTM! Consistent version metadata updates.The header and pragma updates align with the v5.5.0 release requirements.
contracts/token/ERC1155/ERC1155.sol (1)
2-4: LGTM! Consistent version metadata updates.The header and pragma updates align with the v5.5.0 release requirements.
contracts/token/ERC1155/extensions/ERC1155Pausable.sol (1)
2-4: LGTM! Consistent version metadata updates.The header and pragma updates align with the v5.5.0 release requirements.
contracts/utils/draft-InteroperableAddress.sol (1)
2-4: LGTM! Version metadata updated appropriately.The higher Solidity pragma requirement (
^0.8.26) for this draft contract is acceptable, as draft contracts may require newer language features.contracts/utils/structs/EnumerableMap.sol (1)
2-5: LGTM! Consistent version metadata updates.The header and pragma updates align with the v5.5.0 release requirements.
contracts/utils/Bytes.sol (3)
2-2: Version header bump only. Looks good.
86-95: Bounds clamping in slice is correct. end/start are saturated to buffer.length and each other before mcopy; zero-length safe.
119-126: In-place splice overlap behavior verified. MCOPY/Yul's mcopy is specified to allow overlapping source/destination regions and behaves like memmove, as documented in EIP-5656 and Solidity's release notes. The code's use of mcopy for overlapping buffer regions is safe.scripts/generate/templates/Arrays.js (3)
6-6: Pragma aligned with features. ^0.8.24 is consistent with memory-safe Yul usage.
362-392: Value-type slice generator LGTM. Proper end/start clamping and byte-length calculation; mcopy offsets correct for 32-byte slots.
445-447: Generation scope is right. Emits slice/splice only for value types; avoids bytes/string pitfalls.contracts/account/utils/draft-ERC7579Utils.sol (1)
2-2: Version header bump only. No logic changes.contracts/governance/TimelockController.sol (1)
2-2: Version header bump only. No changes to behavior.contracts/governance/Governor.sol (1)
2-2: Version header bump only. No functional diffs.contracts/mocks/Stateless.sol (1)
36-36: Import added for test coverage. Keeping verifiers reachable post-transpile is fine.contracts/governance/utils/Votes.sol (1)
2-2: Version header bump only. All good.contracts/interfaces/draft-IERC7579.sol (1)
2-2: LGTM! Version metadata updated for v5.5.0 release.The header comment version bump is consistent with the release process.
contracts/utils/cryptography/verifiers/ERC7913P256Verifier.sol (1)
2-2: LGTM! Version metadata updated for v5.5.0 release.The header comment version bump is consistent with the release process.
contracts/governance/utils/VotesExtended.sol (1)
2-2: LGTM! Version metadata updated for v5.5.0 release.The header comment version bump is consistent with the release process.
test/utils/Bytes.t.sol (1)
3-3: LGTM! Compiler version requirement updated for v5.5.0 release.The pragma update from
^0.8.20to^0.8.24aligns with the broader compiler version requirement changes across the codebase.contracts/governance/utils/IVotes.sol (1)
2-2: LGTM! Version metadata updated for v5.5.0 release.The header comment version bump is consistent with the release process.
contracts/utils/cryptography/signers/SignerWebAuthn.sol (1)
2-2: LGTM! Version metadata updated for v5.5.0 release.The header comment version bump is consistent with the release process.
contracts/utils/cryptography/verifiers/ERC7913RSAVerifier.sol (1)
2-2: LGTM! Version metadata updated for v5.5.0 release.The header comment version bump is consistent with the release process.
contracts/governance/IGovernor.sol (1)
2-2: LGTM! Version metadata updated for v5.5.0 release.The header comment version bump is consistent with the release process.
contracts/token/ERC20/extensions/IERC20Permit.sol (1)
2-2: LGTM! Version header updated correctly.The version reference has been updated to v5.5.0, consistent with the release consolidation.
contracts/utils/cryptography/verifiers/ERC7913WebAuthnVerifier.sol (1)
2-2: LGTM! Documentation enhancements.The addition of the OpenZeppelin version header and the
@custom:statelesstag improve contract documentation without affecting functionality.Also applies to: 20-21
contracts/token/ERC20/extensions/ERC20Permit.sol (1)
2-2: LGTM! Version header updated.Correctly updated to v5.5.0 as part of the release consolidation.
certora/specs/Account.spec (1)
208-208: LGTM! Specification strengthened with bounds check.The added guard ensures
initDatahas at least 4 bytes before decoding the function selector, preventing out-of-bounds access. This aligns the formal specification with the implementation's safety checks indraft-AccountERC7579.sol.contracts/utils/cryptography/SignatureChecker.sol (2)
2-2: LGTM! Version header updated.Correctly updated to v5.5.0.
86-86: LGTM! Tightened ERC-1271 returndata validation.The change from
0x19(25 bytes) to0x1f(31 bytes) ensures at least 32 bytes of returndata are present before comparing the returned selector. This strengthens validation against truncated or malformed ERC-1271 responses.test/account/extensions/AccountERC7579.behavior.js (3)
94-108: LGTM! Comprehensive edge case testing.The tests verify that
isModuleInstalledhandles empty and short calldata gracefully by returningfalsewithout reverting. This validates the bounds checking logic for insufficient data to decode a function selector.
170-179: LGTM! Installation error handling validated.The test confirms that installing a fallback module with initData too short to encode a function selector properly reverts with the
ERC7579CannotDecodeFallbackDataerror.
254-263: LGTM! Uninstallation error handling validated.The test ensures that uninstalling a fallback module with insufficient initData also properly reverts with
ERC7579CannotDecodeFallbackData, providing symmetric error handling with the installation flow.contracts/utils/cryptography/ECDSA.sol (3)
2-2: LGTM! Version header updated.Correctly updated to v5.5.0.
209-216: LGTM! Improved documentation clarity.The enhanced documentation clearly explains the behavior for both 64-byte (ERC-2098) and 65-byte signatures, including the important distinction that
vis automatically normalized for 64-byte signatures but must already be correct for 65-byte signatures.
217-240: LGTM! Robust signature parsing implementation.The refactored
parse()function correctly handles both signature formats:
- 65-byte: Standard r, s, v extraction
- 64-byte (ERC-2098): Properly extracts r and s from the compact format and normalizes v to 27/28
- Invalid: Gracefully returns zeros
The implementation aligns with the enhanced documentation and enables ERC-2098 compact signature support.
contracts/account/Account.sol (4)
2-2: LGTM! Version header updated.Correctly updated to v5.5.0.
78-78: LGTM! Cleaner signature parameter passing.Extracting the signature at the
validateUserOplevel and passing it explicitly to_validateUserOpimproves separation of concerns and enables derived contracts to customize signature handling.
87-90: LGTM! Clear documentation of design intent.The added documentation effectively explains the purpose of the signature parameter and how derived contracts can leverage it for custom signature handling patterns.
95-104: LGTM! Internal API enhanced for extensibility.The updated
_validateUserOpsignature accepts the signature as a separate parameter, enabling derived contracts to implement custom signature processing logic while maintaining the core validation flow. The implementation correctly uses the new parameter.test/utils/Arrays.test.js (2)
101-115: LGTM - Modern async testing pattern.The updates to use
.to.eventually.equal()are consistent with modern async testing practices and align with the rest of the test suite.
179-234: Excellent test coverage for slice/splice functionality.The test suite comprehensively covers edge cases including empty arrays, out-of-bounds indices, empty ranges, and single elements. The structure mirrors JavaScript's native Array.slice behavior, which aids in understanding.
contracts/utils/Arrays.sol (3)
2-5: LGTM - Version and pragma updates.The pragma update to ^0.8.24 is necessary for the
mcopyopcode used in the new slice/splice implementations, and the version bump to v5.5.0 correctly reflects this release.
378-466: Well-implemented slice utilities.The slice functions correctly implement JavaScript Array.slice semantics with proper input sanitization using
Math.min. The use ofmcopyprovides gas-efficient memory copying, and thememory-safeannotations are appropriate.
468-559: LGTM - Splice utilities correctly modify in place.The splice functions properly implement in-place array modification by moving content and updating the length. The approach is gas-efficient and correctly mirrors JavaScript's Array.splice behavior.
contracts/account/extensions/draft-AccountERC7579.sol (4)
71-72: LGTM - Clear error declaration.The
ERC7579CannotDecodeFallbackDataerror appropriately signals when initData/deInitData is insufficient to extract a selector, improving error clarity.
155-157: Good defensive bounds checking.The length check prevents out-of-bounds access when extracting the fallback selector from
additionalContext. Returningfalserather than reverting correctly complies with ERC-7579's requirement thatisModuleInstallednever revert.
211-220: LGTM - Signature parameter enables custom validation.The addition of the
signatureparameter to_validateUserOpenables custom signature handling logic in the baseAccountclass while maintaining compatibility with module-based validation.
388-393: LGTM - Improved signature extraction and bounds checking.The use of
address(bytes20(signature))is more explicit than slicing for address extraction. The length check in_decodeFallbackDataprevents panics when data is too short to contain a selector, with a descriptive error message.Also applies to: 408-409
contracts/mocks/account/AccountMock.sol (1)
103-108: LGTM - Consistent signature propagation.The update to
_validateUserOpcorrectly propagates the newsignatureparameter to the parent implementation, maintaining consistency with the changes inAccountERC7579.test/utils/Arrays.t.sol (3)
3-8: LGTM - Necessary pragma and import updates.The pragma update to ^0.8.24 aligns with the Arrays library requirements, and the Math import is needed for the sanitization logic used in test assertions.
27-183: Excellent fuzz test coverage for slice and splice.The test suite thoroughly exercises all overloads with proper verification of:
- Non-mutating behavior for
slice(original buffer unchanged)- In-place mutation for
splice(result equals input reference)- Correct element-wise copying across all value types
The use of helper functions keeps tests DRY and maintainable.
193-247: LGTM - Well-designed test helpers.The assertion and copy helpers properly handle type-specific logic with overloading, providing clear, reusable utilities that enhance test readability and maintainability.
| /** | ||
| * @dev Moves the content of \`array\`, from \`start\` (included) to the end of \`array\` to the start of that array. | ||
| * | ||
| * NOTE: This function modifies the provided array in place. If you need to preserve the original array, use {slice} instead. | ||
| * NOTE: replicates the behavior of https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice[Javascript's \`Array.splice\`] | ||
| */ | ||
| function splice(${type.name}[] memory array, uint256 start) internal pure returns (${type.name}[] memory) { | ||
| return splice(array, start, array.length); | ||
| } | ||
| /** | ||
| * @dev Moves the content of \`array\`, from \`start\` (included) to \`end\` (excluded) to the start of that array. The | ||
| * \`end\` argument is truncated to the length of the \`array\`. | ||
| * | ||
| * NOTE: This function modifies the provided array in place. If you need to preserve the original array, use {slice} instead. | ||
| * NOTE: replicates the behavior of https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice[Javascript's \`Array.splice\`] | ||
| */ | ||
| function splice(${type.name}[] memory array, uint256 start, uint256 end) internal pure returns (${type.name}[] memory) { | ||
| // sanitize | ||
| end = Math.min(end, array.length); | ||
| start = Math.min(start, end); | ||
| // move and resize | ||
| assembly ("memory-safe") { | ||
| mcopy(add(array, 0x20), add(add(array, 0x20), mul(start, 0x20)), mul(sub(end, start), 0x20)) | ||
| mstore(array, sub(end, start)) | ||
| } | ||
| return array; | ||
| } | ||
| `; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Splice generator: doc clarity and overlap copy.
- Doc: like Bytes, clarify return value vs JS splice.
- Overlap: confirm reliance on Yul mcopy being overlap-safe.
- * NOTE: replicates the behavior of ... [Javascript's `Array.splice`]
+ * NOTE: replicates the in-place effect of ... [JavaScript's `Array.splice`] on content/length.
+ * Unlike JavaScript, this function returns the modified array (not the removed segment).🌐 Web query:
Does Solidity/Yul's `mcopy` guarantee correct behavior when src/dst overlap (memmove semantics)?
💡 Result:
Yes — the MCOPY / Yul mcopy semantics guarantee memmove-style behavior (safe for overlapping src/dst). The EIP specifies that copying occurs "as if an intermediate buffer was used, allowing the destination and source to overlap," and Solidity's release notes confirm the mcopy builtin / MCOPY use. [1][2]
Sources:
[1] EIP-5656 discussion / spec (MCOPY: copy as if via intermediate buffer).
[2] Solidity 0.8.24/0.8.25 release notes (introducing mcopy and MCOPY usage).
Clarify splice documentation to distinguish from JavaScript behavior.
The mcopy overlap concern is resolved: Solidity/Yul's mcopy guarantees memmove-style behavior with safe handling of overlapping src/dst regions, so the current implementation is correct.
However, the documentation needs clarification. The function returns the modified array, which differs fundamentally from JavaScript's Array.splice (which returns the removed segment). Update the note to avoid misleading developers:
- * NOTE: replicates the behavior of ... [Javascript's `Array.splice`]
+ * NOTE: replicates the in-place effect of ... [JavaScript's `Array.splice`] on content and length.
+ * Unlike JavaScript, this function returns the modified array (not the removed segment).🤖 Prompt for AI Agents
In scripts/generate/templates/Arrays.js around lines 395 to 425, the docstring
currently references JavaScript's Array.splice in a way that misleads readers:
this Solidity function moves a slice to the start and returns the modified array
(not the removed segment like JS splice), and the mcopy overlap concern is
already safe; update the notes to clearly state that the function mutates the
input, returns the modified array (not the removed elements like JS), and
optionally mention that Yul/Solidity mcopy behaves like memmove and handles
overlapping regions safely so the implementation is correct.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Jamq2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
g
No description provided.