Skip to content
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.

Sign up for GitHub

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

fix: generate domain separator per 712 spec #23

Merged
merged 4 commits into from
Jan 8, 2024
Merged

fix: generate domain separator per 712 spec #23

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c9fd11a
fix: generate domain separator per 712 spec
moldy530 Dec 13, 2023
682ecbb
style: standardize private vars in LightAccount
howydev Dec 23, 2023
9eb4891
test: update init code check test
howydev Dec 23, 2023
cdd5d3d
fix: consistency
jaypaik Jan 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions script/Deploy_LightAccountFactory.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ contract Deploy_LightAccountFactory is Script {
console.log("******** Deploy ...... *********");
console.log("********************************");

LightAccountFactory factory =
new LightAccountFactory{salt: 0x4e59b44847b379578588920ca78fbf26c0b4956c3406f3bdc271500000c2f72f}(entryPoint);
LightAccountFactory factory = new LightAccountFactory{
salt: 0x4e59b44847b379578588920ca78fbf26c0b4956c3406f3bdc271500000c2f72f
}(entryPoint);

// Deployed address check
if (address(factory) != 0x00000055C0b4fA41dde26A74435ff03692292FBD) {
howydev marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
24 changes: 13 additions & 11 deletions src/LightAccount.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,14 @@ contract LightAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, Cus
0x33e4b41198cc5b8053630ed667ea7c0c4c873f7fc8d9a478b5d7259cec0a4a00;
// bytes4(keccak256("isValidSignature(bytes32,bytes)"))
bytes4 internal constant _1271_MAGIC_VALUE = 0x1626ba7e;
IEntryPoint private immutable _entryPoint;
IEntryPoint private immutable _ENTRY_POINT;
// keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)")
bytes32 private constant DOMAIN_SEPARATOR_TYPEHASH =
bytes32 private constant _DOMAIN_SEPARATOR_TYPEHASH =
0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f;
// keccak256("LightAccountMessage(bytes message)");
bytes32 private constant LA_MSG_TYPEHASH = 0x5e3baca2936049843f06038876a12f03627b5edc98025751ecf2ac7562640199;
bytes32 private constant _LA_MSG_TYPEHASH = 0x5e3baca2936049843f06038876a12f03627b5edc98025751ecf2ac7562640199;
bytes32 private constant _NAME_HASH = keccak256(bytes("LightAccount"));
bytes32 private constant _VERSION_HASH = keccak256(bytes("1"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: keccak256(bytes("foo")) yields the same result as keccak256("foo") and our comments above omit the bytes wrapper. But the ERC and OZ implementations follow this pattern so I think it's fine to keep it this way too.

Also, might be good to have consistent usage between precomputed values vs. inline expressions. It seems we can assign the commented expressions to _DOMAIN_SEPARATOR_TYPEHASH and _LA_MSG_TYPEHASH (they won't get recomputed at runtime). It actually might save gas on deployment too (double check me on this).


struct LightAccountStorage {
address owner;
Expand Down Expand Up @@ -105,7 +107,7 @@ contract LightAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, Cus
}

constructor(IEntryPoint anEntryPoint) CustomSlotInitializable(_INITIALIZABLE_STORAGE_POSITION) {
_entryPoint = anEntryPoint;
_ENTRY_POINT = anEntryPoint;
_disableInitializers();
}

Expand Down Expand Up @@ -183,7 +185,7 @@ contract LightAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, Cus
/**
* @notice Called once as part of initialization, either during initial deployment or when first upgrading to
* this contract.
* @dev The _entryPoint member is immutable, to reduce gas consumption. To upgrade EntryPoint,
* @dev The _ENTRY_POINT member is immutable, to reduce gas consumption. To upgrade EntryPoint,
* a new implementation of LightAccount must be deployed with the new EntryPoint address, then upgrading
* the implementation by calling `upgradeTo()`
* @param anOwner The initial owner of the account
Expand All @@ -210,7 +212,7 @@ contract LightAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, Cus

/// @inheritdoc BaseAccount
function entryPoint() public view virtual override returns (IEntryPoint) {
return _entryPoint;
return _ENTRY_POINT;
}

/**
Expand All @@ -236,9 +238,9 @@ contract LightAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, Cus
function domainSeparator() public view returns (bytes32) {
return keccak256(
abi.encode(
DOMAIN_SEPARATOR_TYPEHASH,
abi.encode("LightAccount"), // name
abi.encode("1"), // version
_DOMAIN_SEPARATOR_TYPEHASH,
_NAME_HASH, // name
_VERSION_HASH, // version
block.chainid, // chainId
address(this) // verifying contract
)
Expand All @@ -251,7 +253,7 @@ contract LightAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, Cus
* @return Encoded message.
*/
function encodeMessageData(bytes memory message) public view returns (bytes memory) {
bytes32 messageHash = keccak256(abi.encode(LA_MSG_TYPEHASH, keccak256(message)));
bytes32 messageHash = keccak256(abi.encode(_LA_MSG_TYPEHASH, keccak256(message)));
return abi.encodePacked("\x19\x01", domainSeparator(), messageHash);
}

Expand Down Expand Up @@ -287,7 +289,7 @@ contract LightAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, Cus
revert InvalidOwner(address(0));
}
_getStorage().owner = anOwner;
emit LightAccountInitialized(_entryPoint, anOwner);
emit LightAccountInitialized(_ENTRY_POINT, anOwner);
emit OwnershipTransferred(address(0), anOwner);
}

Expand Down
5 changes: 2 additions & 3 deletions src/LightAccountFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,8 @@ contract LightAccountFactory {
}
ret = LightAccount(
payable(
new ERC1967Proxy{salt : bytes32(salt)}(
address(accountImplementation),
abi.encodeCall(LightAccount.initialize, (owner))
new ERC1967Proxy{salt: bytes32(salt)}(
address(accountImplementation), abi.encodeCall(LightAccount.initialize, (owner))
)
)
);
Expand Down
2 changes: 1 addition & 1 deletion test/LightAccount.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ contract LightAccountTest is Test {
bytes32(uint256(uint160(0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789)))
)
),
0x2ad62a8bb3850247ef0c4f04e30b584e6eee7caa0e063745e90956653b90eb84
0x72db05fba86adbc64040d8949a885cd2a59aa1604d3c84bec19870bd9a7db0d5
);
}

Expand Down