feat(v2): use solady's minimal ERC-1967 proxy and UUPSUpgradeable #35
Conversation
jaypaik
force-pushed
the
03-10-style_consistent_natspec_style
branch
from
a79c6dc
to
8dac3c6
Compare
jaypaik
force-pushed
the
03-10-feat_use_solady_s_minimal_ERC-1967_proxy_and_UUPSUpgradeable
branch
from
1d1c48d
to
fa694c6
Compare
| @@ -0,0 +1,1210 @@ | |||
| // SPDX-License-Identifier: MIT | |||
| pragma solidity ^0.8.4; | |||
|
|
|||
There was a problem hiding this comment.
Could we add a comment (either here or elsewhere in the repo) indicating the commit hash from Solady at which this file was pulled, to reference later?
There was a problem hiding this comment.
Yup, good idea. Will do for all of these.
There was a problem hiding this comment.
Cut out a task for this, will address upstack.
| @@ -0,0 +1,142 @@ | |||
| // SPDX-License-Identifier: MIT | |||
| pragma solidity ^0.8.4; | |||
|
|
|||
There was a problem hiding this comment.
Same here for the solady commit hash
src/LightAccountFactory.sol
Outdated
| ); | ||
| } | ||
|
|
||
| function _getCombinedSalt(address owner, uint256 salt) internal pure returns (bytes32) { | ||
| return keccak256(abi.encode(owner, salt)); |
There was a problem hiding this comment.
this can be optimized to avoid memory expansion as:
function _getCombinedSalt(address owner, uint256 salt) internal pure returns (bytes32 combinedSalt) {
assembly ("memory-safe") {
mstore(0x00, owner)
mstore(0x20, salt)
combinedSalt := keccak256(0x00, 0x40)
}
}This will use the scratch space in the first two bytes of memory, rather than allocalte a new bytes buffer for the result of abi.encode.
This will have the caveat that the upper bits of address need to be cleared elsewhere, but that's fine here because we only pass post-sanitization external function parameters.
| ); | ||
| } | ||
|
|
||
| function _getCombinedSalt(address[] memory owners, uint256 salt) internal pure returns (bytes32) { | ||
| return keccak256(abi.encode(owners, salt)); |
There was a problem hiding this comment.
For this one, we can't avoid memory expansion without a large refactor.
| return LightAccount(payable(addr)); | ||
| /// @return account The address of either the newly deployed account or an existing account with this owner and salt. | ||
| function createAccount(address owner, uint256 salt) public returns (LightAccount account) { | ||
| (bool alreadyDeployed, address accountAddress) = |
There was a problem hiding this comment.
Returning (bool alreadyDeployed, address accountAddress) is a very nice DevEx from solady :)
jaypaik
force-pushed
the
03-10-style_consistent_natspec_style
branch
from
8dac3c6
to
34d89e9
Compare
jaypaik
force-pushed
the
03-10-feat_use_solady_s_minimal_ERC-1967_proxy_and_UUPSUpgradeable
branch
from
fa694c6
to
9836dde
Compare
jaypaik
force-pushed
the
03-10-style_consistent_natspec_style
branch
from
34d89e9
to
f37599d
Compare
jaypaik
force-pushed
the
03-10-feat_use_solady_s_minimal_ERC-1967_proxy_and_UUPSUpgradeable
branch
from
9836dde
to
775df0e
Compare
jaypaik
force-pushed
the
03-10-style_consistent_natspec_style
branch
from
f37599d
to
e0cb7cf
Compare
jaypaik
force-pushed
the
03-10-feat_use_solady_s_minimal_ERC-1967_proxy_and_UUPSUpgradeable
branch
from
775df0e
to
4774a7f
Compare
jaypaik
force-pushed
the
03-10-feat_use_solady_s_minimal_ERC-1967_proxy_and_UUPSUpgradeable
branch
from
ab666c3
to
b4f2a46
Compare
jaypaik
deleted the
03-10-feat_use_solady_s_minimal_ERC-1967_proxy_and_UUPSUpgradeable
branch
Note
This changes the way we generate counterfactuals. I think this is okay, but let me know if you see any problems.