safe-global / safe-smart-account

Safe allows secure management of blockchain assets.
https://safe.global
GNU Lesser General Public License v3.0
1.84k stars 907 forks source link

Safely add itself as a new module on safe creation. #610

Closed skeletor-spaceman closed 1 year ago

skeletor-spaceman commented 1 year ago

regarding Test4337ModuleAndHandler even if this is just a test mock contract I feel this is a somewhat sensitive operation, and if used as an example might generate some issues down the road for other modules. taking into account that enableModule functionality might change on newer implementations it'd be safer to use the standard logic and do:

    function enableMyself() public {
        GnosisSafe(payable(address(this))).enableModule(myAddress);
    }

also removing the need to inherit from:

import "../../libraries/SafeStorage.sol";
...
contract Test4337ModuleAndHandler is SafeStorage {

original code for reference. https://github.com/safe-global/safe-contracts/blob/2b7e4c4f8c548dc1b36e06154b0b26c8b5fefb16/contracts/test/4337/Test4337ModuleAndHandler.sol#L57-L64

mmv08 commented 1 year ago

Thanks! That's a great suggestion, I don't know what I was thinking 😂