docs(account/AccountERC7579): clarify ERC-1271 behavior and no native fallback #5925
Conversation
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates documentation in the AccountERC7579 extension to accurately describe ERC-1271 behavior: signature checks are delegated exclusively to IERC7579Validator modules (returning 0xffffffff if none validate), native/raw signature validation is disabled by default, and the _validateUserOp fallback path will only work when explicitly overridden or composed with other extensions. Class diagram for updated AccountERC7579 extension documentationclassDiagram
class AccountERC7579 {
<<abstract>>
+isValidSignature(data, signature) : bytes4
+_validateUserOp(userOp, userOpHash, missingAccountFunds) : uint256
Note: isValidSignature delegates to installed IERC7579Validator modules
Note: Returns 0xffffffff if no module validates
Note: Native/raw signature validation is disabled by default
}
class IERC1271 {
<<interface>>
+isValidSignature(data, signature) : bytes4
}
class IERC7579Validator {
<<interface>>
+isValidSignatureWithSender(sender, data, signature) : bytes4
}
class Account {
<<abstract>>
+_validateUserOp(userOp, userOpHash, missingAccountFunds) : uint256
}
AccountERC7579 --|> Account
AccountERC7579 --|> IERC1271
AccountERC7579 --|> IERC7579Validator
AccountERC7579 : - native/raw signature validation is disabled
AccountERC7579 : - fallback to Account-_validateUserOp only works if overridden or composed
AccountERC7579 : - developers can override or compose with ERC7739 for native validation
class ERC7739 {
<<extension>>
}
Flow diagram for isValidSignature delegation in AccountERC7579flowchart TD
A["isValidSignature(data, signature)"] --> B["Delegate to installed IERC7579Validator modules"]
B -->|"Module validates"| C["Return module's result"]
B -->|"No module validates"| D["Return 0xffffffff"]
D --> E["No native/raw validation performed"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughDocumentation in contracts/account/extensions/draft-AccountERC7579.sol was updated to clarify ERC-1271 signature validation delegation to IERC7579Validator modules, default disabling of raw/native signature validation, guidance on combining with ERC7739 and multiple inheritance, and notes that _validateUserOp depends on overrides. No code or API signatures changed. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
contracts/account/extensions/draft-AccountERC7579.sol (1)
175-182: Nit: clarify short signatures and module return valueSuggest mentioning that invalid also covers signatures shorter than 20 bytes, and that modules MUST return the ERC‑1271 magic (
0x1626ba7e); this function forwards the module’s return unchanged.- * function returns `0xffffffff`. + * function returns `0xffffffff` (including when the signature is shorter than 20 bytes). + * + * Modules MUST return the ERC-1271 magic value (`0x1626ba7e`) on success; this function forwards + * the module's return value unchanged.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
contracts/account/extensions/draft-AccountERC7579.sol(2 hunks)
⏰ 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). (6)
- GitHub Check: tests-foundry
- GitHub Check: tests-upgradeable
- GitHub Check: tests
- GitHub Check: coverage
- GitHub Check: slither
- GitHub Check: halmos
🔇 Additional comments (2)
contracts/account/extensions/draft-AccountERC7579.sol (2)
175-182: Docs correctly state ERC-1271 delegation and no native fallbackAccurately reflects the implementation and the
_rawSignatureValidationoverride.
208-211: Docs about _validateUserOp fallback and disabled raw validation are accurateMatches behavior: fallback to {Account-_validateUserOp} won’t pass here unless another ancestor enables raw validation (e.g., via ERC-7739).
| * Note: this contract disables raw signature validation (see {_rawSignatureValidation}), so unless | ||
| * overridden or complemented via multiple inheritance, the fallback path to {Account-_validateUserOp} | ||
| * will not succeed by itself. | ||
| * |
There was a problem hiding this comment.
This part is incorrect. If no installed validation module is extracted from the user operation, we fallback to super._validateUserOp. In the absence of other overrides, this fallback goes to Account._validateUserOp which does a _rawSignatureValidation check.
There was a problem hiding this comment.
This part is incorrect. If no installed validation module is extracted from the user operation, we fallback to
super._validateUserOp. In the absence of other overrides, this fallback goes toAccount._validateUserOpwhich does a_rawSignatureValidationcheck.
corrected
Amxx
added
documentation
This change updates isValidSignature documentation to reflect the actual behavior: validation is delegated exclusively to installed ERC-7579 validator modules and returns 0xffffffff when none validate; native/raw signature validation is disabled by default in this extension. The _validateUserOp comment is also clarified to note that while it falls back to {Account-_validateUserOp}, raw validation is disabled here, so the fallback will not succeed unless overridden or composed with other extensions (e.g., ERC-7739). These updates remove confusing guidance that suggested a native fallback existed, aligning docs with the implemented and intended behavior.
Summary by Sourcery
Clarify in AccountERC7579 that ERC-1271 signature validation is exclusively delegated to installed validator modules with no native/raw fallback by default, and update _validateUserOp comments accordingly
Documentation:
Summary by CodeRabbit