-
Notifications
You must be signed in to change notification settings - Fork 73
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
100% Code Coverage on Signer Launchpad #411
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,15 +25,15 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage { | |
* @notice The EIP-712 type-hash for the domain separator used for verifying Safe initialization signatures. | ||
* @custom:computed-as keccak256("EIP712Domain(uint256 chainId,address verifyingContract)") | ||
*/ | ||
bytes32 private constant DOMAIN_SEPARATOR_TYPEHASH = 0x47e79534a245952e8b16893a336b85a3d9ea9fa8c573f3d803afb92a79469218; | ||
bytes32 private constant _DOMAIN_SEPARATOR_TYPEHASH = 0x47e79534a245952e8b16893a336b85a3d9ea9fa8c573f3d803afb92a79469218; | ||
|
||
/** | ||
* @notice The storage slot used for the target Safe singleton address to promote to. | ||
* @custom:computed-as keccak256("SafeSignerLaunchpad.targetSingleton") - 1 | ||
* @dev This value is intentionally computed to be a hash -1 as a precaution to avoid any potential issues from | ||
* unintended hash collisions. | ||
*/ | ||
uint256 private constant TARGET_SINGLETON_SLOT = 0x610b07c5cf4b478e92ab041de73a412736c750f1bf07a613600b24b3a8bd597e; | ||
uint256 private constant _TARGET_SINGLETON_SLOT = 0x610b07c5cf4b478e92ab041de73a412736c750f1bf07a613600b24b3a8bd597e; | ||
|
||
/** | ||
* @notice The keccak256 hash of the EIP-712 SafeInitOp struct, representing the user operation to execute alongside initialization. | ||
|
@@ -43,7 +43,7 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage { | |
* {address} entryPoint - The address of the entry point that will execute the user operation. | ||
* @custom:computed-as keccak256("SafeInitOp(bytes32 userOpHash,uint48 validAfter,uint48 validUntil,address entryPoint)") | ||
*/ | ||
bytes32 private constant SAFE_INIT_OP_TYPEHASH = 0x25838d3914a61e3531f21f12b8cd3110a5f9d478292d07dd197859a5c4eaacb2; | ||
bytes32 private constant _SAFE_INIT_OP_TYPEHASH = 0x25838d3914a61e3531f21f12b8cd3110a5f9d478292d07dd197859a5c4eaacb2; | ||
|
||
/** | ||
* @notice An error indicating that the entry point used when deploying a new module instance is invalid. | ||
|
@@ -67,10 +67,9 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage { | |
error AlreadyInitialized(); | ||
|
||
/** | ||
* @notice An error indicating an attempt to execute a user operation on an account that has already been promoted | ||
* to a Safe singleton. | ||
* @notice An error indicating an attempt to execute a user operation on an account that has not been initialized. | ||
*/ | ||
error AlreadyPromoted(); | ||
error NotInitialized(); | ||
|
||
/** | ||
* @notice An error indicating that the account was initialized with an invalid Safe singleton address. | ||
|
@@ -201,6 +200,14 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage { | |
} | ||
} | ||
|
||
/** | ||
* @notice Computes the EIP-712 domain separator for Safe launchpad operations. | ||
* @return domainSeparatorHash The EIP-712 domain separator hash for this contract. | ||
*/ | ||
function domainSeparator() public view returns (bytes32) { | ||
return keccak256(abi.encode(_DOMAIN_SEPARATOR_TYPEHASH, block.chainid, _SELF)); | ||
} | ||
|
||
/** | ||
* @notice Compute the {SafeInitOp} hash of the first user operation that initializes the Safe. | ||
* @dev The hash is generated using the keccak256 hash function and the EIP-712 standard. It is signed by the Safe | ||
|
@@ -216,7 +223,7 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage { | |
abi.encodePacked( | ||
bytes2(0x1901), | ||
domainSeparator(), | ||
keccak256(abi.encode(SAFE_INIT_OP_TYPEHASH, userOpHash, validAfter, validUntil, SUPPORTED_ENTRYPOINT)) | ||
keccak256(abi.encode(_SAFE_INIT_OP_TYPEHASH, userOpHash, validAfter, validUntil, SUPPORTED_ENTRYPOINT)) | ||
) | ||
); | ||
} | ||
|
@@ -280,7 +287,7 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage { | |
) external onlySupportedEntryPoint { | ||
address singleton = _targetSingleton(); | ||
if (singleton == address(0)) { | ||
revert AlreadyPromoted(); | ||
revert NotInitialized(); | ||
} | ||
|
||
SafeStorage.singleton = singleton; | ||
|
@@ -303,6 +310,7 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage { | |
// See <https://docs.soliditylang.org/en/latest/control-structures.html#panic-via-assert-and-error-via-require>. | ||
mstore(0x00, hex"4e487b71") | ||
mstore(0x04, 0x21) | ||
revert(0, 0x24) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug caught by the unit tests. |
||
} | ||
} | ||
|
||
|
@@ -311,14 +319,6 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage { | |
} | ||
} | ||
|
||
/** | ||
* @notice Computes the EIP-712 domain separator for Safe launchpad operations. | ||
* @return domainSeparatorHash The EIP-712 domain separator hash for this contract. | ||
*/ | ||
function domainSeparator() public view returns (bytes32) { | ||
return keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, block.chainid, _SELF)); | ||
} | ||
|
||
/** | ||
* @dev Validates that the user operation is correctly signed and returns an ERC-4337 packed validation data | ||
* of `validAfter || validUntil || authorizer`: | ||
|
@@ -378,7 +378,7 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage { | |
assembly ("memory-safe") { | ||
// Note that we explicitly don't mask the address, as Solidity will generate masking code for every time | ||
// the variable is read. | ||
value := sload(TARGET_SINGLETON_SLOT) | ||
value := sload(_TARGET_SINGLETON_SLOT) | ||
} | ||
} | ||
|
||
|
@@ -389,7 +389,7 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage { | |
function _setTargetSingleton(address value) internal { | ||
// solhint-disable-next-line no-inline-assembly | ||
assembly ("memory-safe") { | ||
sstore(TARGET_SINGLETON_SLOT, value) | ||
sstore(_TARGET_SINGLETON_SLOT, value) | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Consistent naming.