-
Notifications
You must be signed in to change notification settings - Fork 554
Many accounts per EOA #561
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
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.
LG, just that comment on the encode fn
@@ -189,6 +189,11 @@ contract AccountCore is IAccountCore, Initializable, Multicall, BaseAccount, Acc | |||
Internal functions | |||
//////////////////////////////////////////////////////////////*/ | |||
|
|||
/// @dev Returns the salt used when deploying an Account. | |||
function _generateSalt(address _admin, bytes memory _data) internal view virtual returns (bytes32) { | |||
return keccak256(abi.encodePacked(_admin, _data)); |
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.
I think we should use encode instead of encode packed here ? From what I read online it's better suited for dynamic types like bytes
function _generateSalt(address _admin, bytes memory) internal view virtual returns (bytes32) { | ||
return keccak256(abi.encode(_admin)); | ||
function _generateSalt(address _admin, bytes memory _data) internal view virtual returns (bytes32) { | ||
return keccak256(abi.encodePacked(_admin, _data)); |
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.
Same here
Codecov ReportAttention:
📢 Thoughts on this report? Let us know!. |
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.
lg. We'll have to update the AccountExtension impl address so just a reminder that it will need to be redeployed on all chains deterministically.
and with the new function selectors added 🙏
} | ||
|
||
/// @notice Withdraw funds for this account from Entrypoint. | ||
function withdrawDepositTo(address payable withdrawAddress, uint256 amount) public { |
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.
remember to add those 2 to the AccountExtension struct in the factory when publishing
No description provided.