-
Notifications
You must be signed in to change notification settings - Fork 21
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
Update safe integration test #11
Conversation
JohnGuilding
commented
Jun 27, 2024
•
edited
Loading
edited
- Pulls latest Safe 7579 changes
- Updates Safe integration tests
- Updates Safe unit tests
- Runs multi-account tests in CI (default, Safe, Kernel)
- Runs simulate tests in CI (Checks against 4337 storage access rules)
5a334cc
to
951231b
Compare
f45a202
to
ca8f535
Compare
script/DeploySafeRecovery.s.sol
Outdated
address verifier = 0xEdC642bbaD91E21cCE6cd436Fdc6F040FD0fF998; | ||
address dkimRegistry = 0xC83256CCf7B94d310e49edA05077899ca036eb78; | ||
address emailAuthImpl = 0x1C76Aa365c17B40c7E944DcCdE4dC6e6D2A7b748; | ||
bytes32 salt = bytes32(uint256(0)); |
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.
Why is the salt fixed here?
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.
Yeah maybe that is inconsistent with the other code. Deployed other contracts with factory which uses salts.
The code to deploy the launchpad and safe7579 contract had a salt like this, so I guess I copied it over
script/DeploySafeRecovery.s.sol
Outdated
address safe7579 = address(new Safe7579{ salt: salt }()); | ||
address safe7579Launchpad = | ||
address(new Safe7579Launchpad{ salt: salt }(entryPoint, registry)); |
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.
Where/when does this safe account install necessary modules?
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.
For this script, it is to deploy the contracts for the frontend demo. The module is installed via the frontend.