-
Notifications
You must be signed in to change notification settings - Fork 12.3k
Rename SignerERC7702 as SignerEIP7702 #5932
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
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR systematically renames the SignerERC7702 contract to SignerEIP7702 across the codebase, updating the implementation, mocks, inheritance clauses, internal overrides, changelog entry, and documentation to align with the new EIP naming convention. Class diagram for renaming SignerERC7702 to SignerEIP7702classDiagram
class AbstractSigner
class SignerEIP7702 {
+_rawSignatureValidation(hash, signature)
}
AbstractSigner <|-- SignerEIP7702
class Account
class AccountERC7702Mock {
}
Account <|-- AccountERC7702Mock
SignerEIP7702 <|-- AccountERC7702Mock
class AccountERC7702WithModulesMock {
+_rawSignatureValidation(hash, signature)
}
Account <|-- AccountERC7702WithModulesMock
SignerEIP7702 <|-- AccountERC7702WithModulesMock
AccountERC7579 <|-- AccountERC7702WithModulesMock
class MyAccountERC7702 {
+_erc7821AuthorizedExecutor(caller, selector)
}
Account <|-- MyAccountERC7702
SignerEIP7702 <|-- MyAccountERC7702
ERC7821 <|-- MyAccountERC7702
ERC721Holder <|-- MyAccountERC7702
ERC1155Holder <|-- MyAccountERC7702
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
WalkthroughThis PR renames the signer utility from SignerERC7702 to SignerEIP7702 across code and docs. Changes include updating the contract name in contracts/utils/cryptography/signers/SignerERC7702.sol, imports and inheritance in mocks (AccountMock.sol, MyAccountERC7702.sol), and adjusting _rawSignatureValidation override references. Documentation updates span README.adoc, accounts.adoc, eoa-delegation.adoc, and CHANGELOG.md, including import paths and example inheritance. No behavioral or logic changes are introduced; updates are limited to symbol and reference renames. Possibly related issues
Pre-merge checks (5 passed)✅ Passed checks (5 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches🧪 Generate unit tests
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: 2
🧹 Nitpick comments (3)
contracts/utils/cryptography/README.adoc (1)
59-59: Fix inconsistent reference: replace remaining "SignerERC7702" mention in this doc.Line 59 correctly uses {{SignerEIP7702}}, but earlier in this file the bullet list still references {SignerERC7702} (Line 20). Update that bullet to {SignerEIP7702} to keep anchors and cross-refs consistent.
Would you like me to scan the docs for any other lingering "SignerERC7702" references?
CHANGELOG.md (1)
48-48: Use “EIP-7702”, not “ERC-7702”, in the entry text.Nit: naming consistency across the repo uses EIP-7702.
Apply this diff:
- - `SignerEIP7702`: Implementation of `AbstractSigner` for Externally Owned Accounts (EOAs). Useful with ERC-7702. ([#5657](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/5657)) + - `SignerEIP7702`: Implementation of `AbstractSigner` for Externally Owned Accounts (EOAs). Useful with EIP-7702. ([#5657](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/5657))docs/modules/ROOT/pages/eoa-delegation.adoc (1)
13-13: LGTM on the rename; align terminology elsewhere in this page.This line correctly references SignerEIP7702. Consider also changing earlier occurrences of “ERC-7702” to “EIP-7702” (e.g., Line 9) for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md(1 hunks)contracts/mocks/account/AccountMock.sol(4 hunks)contracts/mocks/docs/account/MyAccountERC7702.sol(1 hunks)contracts/utils/cryptography/README.adoc(1 hunks)contracts/utils/cryptography/signers/SignerERC7702.sol(1 hunks)docs/modules/ROOT/pages/accounts.adoc(2 hunks)docs/modules/ROOT/pages/eoa-delegation.adoc(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T13:16:08.640Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#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/mocks/account/AccountMock.sol
🪛 GitHub Actions: checks
contracts/mocks/docs/account/MyAccountERC7702.sol
[error] 9-9: forge test -vvv: Source 'contracts/utils/cryptography/signers/SignerEIP7702.sol' not found: File not found. Searched the following locations: '/home/runner/work/openzeppelin-contracts/openzeppelin-contracts'.
contracts/mocks/account/AccountMock.sol
[error] 1-1: npm run compile (hardhat compile) failed: HH404: File ../../utils/cryptography/signers/SignerEIP7702.sol, imported from AccountMock.sol, not found.
🪛 GitHub Actions: formal verification
contracts/mocks/docs/account/MyAccountERC7702.sol
[error] 9-9: Source 'contracts/utils/cryptography/signers/SignerEIP7702.sol' not found.
contracts/mocks/account/AccountMock.sol
[error] 19-19: Source 'contracts/utils/cryptography/signers/SignerEIP7702.sol' not found.
⏰ 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). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (6)
contracts/utils/cryptography/signers/SignerERC7702.sol (1)
14-24: Cannot verify — repo path 'contracts/' missing; confirm SignerERC7702.sol vs SignerEIP7702.sol filename mismatch**Verification script output: "contracts/**: No such file or directory (os error 2)"; I couldn't inspect the code.
- If the file exists, ensure filename matches imports: git mv contracts/utils/cryptography/signers/SignerERC7702.sol contracts/utils/cryptography/signers/SignerEIP7702.sol (or update imports that reference SignerEIP7702.sol).
docs/modules/ROOT/pages/eoa-delegation.adoc (1)
89-90: LGTM.Reference and anchor for SignerEIP7702 look correct.
docs/modules/ROOT/pages/accounts.adoc (2)
26-26: LGTM.Renamed reference to SignerEIP7702 is consistent with the new entity.
99-101: LGTM; ensure import path exists post-rename.Example imports SignerEIP7702.sol; this will be valid once the file is renamed in the repo.
contracts/mocks/account/AccountMock.sol (2)
121-127: Override list correctly updated to include SignerEIP7702.The delegation to SignerEIP7702._rawSignatureValidation is accurate.
84-93: LGTM.Inheritance updated to SignerEIP7702 as intended.
| import {SignerRSA} from "../../utils/cryptography/signers/SignerRSA.sol"; | ||
| import {SignerWebAuthn} from "../../utils/cryptography/signers/SignerWebAuthn.sol"; | ||
| import {SignerERC7702} from "../../utils/cryptography/signers/SignerERC7702.sol"; | ||
| import {SignerEIP7702} from "../../utils/cryptography/signers/SignerEIP7702.sol"; |
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.
Import fails: SignerEIP7702.sol not found (rename source file).
Same root cause as other failures: the signer file wasn’t renamed. Rename the source file to SignerEIP7702.sol to unblock compilation.
🧰 Tools
🪛 GitHub Actions: formal verification
[error] 19-19: Source 'contracts/utils/cryptography/signers/SignerEIP7702.sol' not found.
🤖 Prompt for AI Agents
In contracts/mocks/account/AccountMock.sol around line 19, the import of
SignerEIP7702.sol fails because the actual signer source file was not renamed to
match this import; rename the signer source file in
../../utils/cryptography/signers/ to SignerEIP7702.sol (or alternatively update
the import to the actual filename) so the import path matches the file name and
compilation can succeed.
| import {SignerEIP7702} from "../../../utils/cryptography/signers/SignerEIP7702.sol"; | ||
|
|
||
| contract MyAccountERC7702 is Account, SignerERC7702, ERC7821, ERC721Holder, ERC1155Holder { | ||
| contract MyAccountERC7702 is Account, SignerEIP7702, ERC7821, ERC721Holder, ERC1155Holder { |
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.
💡 Verification agent
🧩 Analysis chain
Import fails: SignerEIP7702.sol not found (rename source file).
CI error shows this import path doesn’t exist yet. After renaming the signer file to SignerEIP7702.sol (in the signers folder), this will compile.
🏁 Script executed:
#!/bin/bash
# Confirm the import target exists after the rename
test -f contracts/utils/cryptography/signers/SignerEIP7702.sol && echo "OK: SignerEIP7702.sol present" || (echo "Missing SignerEIP7702.sol" && exit 1)Length of output: 25
Fix missing import: add or rename SignerEIP7702.sol or update the import
contracts/mocks/docs/account/MyAccountERC7702.sol (lines 9-11) imports contracts/utils/cryptography/signers/SignerEIP7702.sol, but that file is missing (local check returned "Missing SignerEIP7702.sol") — either create/rename the signer file at that path or change the import to the correct filename.
🧰 Tools
🪛 GitHub Actions: checks
[error] 9-9: forge test -vvv: Source 'contracts/utils/cryptography/signers/SignerEIP7702.sol' not found: File not found. Searched the following locations: '/home/runner/work/openzeppelin-contracts/openzeppelin-contracts'.
🪛 GitHub Actions: formal verification
[error] 9-9: Source 'contracts/utils/cryptography/signers/SignerEIP7702.sol' not found.
🤖 Prompt for AI Agents
In contracts/mocks/docs/account/MyAccountERC7702.sol around lines 9-11 the
import for "../../../utils/cryptography/signers/SignerEIP7702.sol" points to a
missing file; either add or rename the signer source to that exact path and
filename (SignerEIP7702.sol) exporting the SignerEIP7702 contract, or update
this import to the actual existing signer filename/path (and adjust the contract
inheritance if the contract name differs) so the compiler can locate the signer
implementation.
|
This is a breaking change. We cannot do that before v6.0 |
Got it! So this will be merged as part of v6.0, right? |
|
Update: we decided to do some "import related" breaking changes in 5.5 |
ernestognw
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.
Looks good. Thanks 🙌
Fixes #5930
Renamed SignerERC7702 as SignerEIP7702
PR Checklist
npx changeset add)Summary by Sourcery
Rename the SignerERC7702 abstract contract to SignerEIP7702 and update all related references
Enhancements:
Documentation:
Chores: