chore: allowed targets -> allowed calls, emit prev value for change fncs

sense-finance · Jun 25, 2024 · 3775e51 · 3775e51
1 parent 695a4ec
commit 3775e51
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 29 deletions.
diff --git a/src/RumpelGuard.sol b/src/RumpelGuard.sol
@@ -12,20 +12,20 @@ contract RumpelGuard is AccessControl, IGuard {
     event SetCallAllowed(address indexed target, bytes4 indexed functionSig, bool allow);
     event SetCallPermenantlyAllowed(address indexed target, bytes4 indexed functionSig);
 
-    mapping(address => mapping(bytes4 => bool)) public allowedTargets;
-    mapping(address => mapping(bytes4 => bool)) public permanentlyAllowedTargets;
+    mapping(address => mapping(bytes4 => bool)) public allowedCalls;
+    mapping(address => mapping(bytes4 => bool)) public permanentlyAllowedCalls;
 
     constructor() {
         _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
     }
 
     function setCallAllowed(address target, bytes4 functionSig, bool allow) public onlyRole(DEFAULT_ADMIN_ROLE) {
-        allowedTargets[target][functionSig] = allow;
+        allowedCalls[target][functionSig] = allow;
         emit SetCallAllowed(target, functionSig, allow);
     }
 
     function setCallPermenantlyAllowed(address target, bytes4 functionSig) public onlyRole(DEFAULT_ADMIN_ROLE) {
-        permanentlyAllowedTargets[target][functionSig] = true; // One way, only true
+        permanentlyAllowedCalls[target][functionSig] = true; // One way, only true
         emit SetCallPermenantlyAllowed(target, functionSig);
     }
 
@@ -46,7 +46,7 @@ contract RumpelGuard is AccessControl, IGuard {
         bytes4 functionSig = bytes4(data);
         // TODO: check value?
 
-        if (!allowedTargets[to][functionSig] && !permanentlyAllowedTargets[to][functionSig]) {
+        if (!allowedCalls[to][functionSig] && !permanentlyAllowedCalls[to][functionSig]) {
             revert CallNotAllowed();
         }
     }

diff --git a/src/RumpelModule.sol b/src/RumpelModule.sol
@@ -13,7 +13,7 @@ contract RumpelModule is AccessControl {
 
     event ExecutionFromModule(address indexed safe, address indexed target, uint256 value, bytes data);
     event TokensSwepped(address indexed safe, address indexed token, uint256 amount);
-    event RumpelVaultUpdated(address indexed newVault);
+    event RumpelVaultUpdated(address prevVault, address newVault);
     event BlockedCallAdded(address indexed target, bytes4 indexed data);
 
     mapping(address => mapping(bytes4 => bool)) public blockedCalls;
@@ -89,7 +89,7 @@ contract RumpelModule is AccessControl {
     }
 
     function setRumpelVault(address _rumpelVault) public virtual onlyRole(DEFAULT_ADMIN_ROLE) {
+        emit RumpelVaultUpdated(rumpelVault, _rumpelVault);
         rumpelVault = _rumpelVault;
-        emit RumpelVaultUpdated(_rumpelVault);
     }
 }
diff --git a/src/RumpelWalletFactory.sol b/src/RumpelWalletFactory.sol
@@ -10,17 +10,16 @@ import {InitializationScript} from "./InitializationScript.sol";
 import {RumpelModule} from "./RumpelModule.sol";
 
 // delegate claiming fnc in the vault?
-// restrict what we can do with the wallet (top 20 no approve and transfer)
 // upgradable?'
 // TODO: do we handle safe contract upgrades?
 
 contract RumpelWalletFactory is Ownable, Pausable {
-    event SafeCreated(address indexed safe, address[] owners, uint256 threshold);
-    event RumpelGuardUpdated(address indexed newGuard);
-    event RumpelModuleUpdated(address indexed newModule);
-    event InitializationScriptUpdated(address indexed newScript);
-    event SafeSingletonUpdated(address indexed newSafeSingleton);
-    event ProxyFactoryUpdated(address indexed newProxyFactory);
+    event SafeCreated(address indexed safe, address[] indexed owners, uint256 threshold);
+    event RumpelGuardUpdated(address prevGuard, address newGuard);
+    event RumpelModuleUpdated(address prevModule, address newModule);
+    event InitializationScriptUpdated(address prevScript, address newScript);
+    event SafeSingletonUpdated(address prevSafeSingleton, address newSafeSingleton);
+    event ProxyFactoryUpdated(address prevProxyFactory, address newProxyFactory);
 
     uint256 public saltNonce;
 
@@ -46,14 +45,15 @@ contract RumpelWalletFactory is Ownable, Pausable {
         rumpelGuard = _rumpelGuard;
     }
 
+    // Address can be predicted off-chain
     function createWallet(address[] calldata owners, uint256 threshold) public whenNotPaused returns (address) {
         address safe = proxyFactory.createProxyWithNonce(
             safeSingleton,
-            abi.encodeWithSelector( // initializer
+            abi.encodeWithSelector(
                 ISafe.setup.selector,
                 owners,
                 threshold,
-                initializationScript, // Target contract we delegatecall to for initialization
+                initializationScript, // Contract with initialization logic
                 abi.encodeWithSelector(InitializationScript.initialize.selector, rumpelModule, rumpelGuard), // Initializing call to enable module and guard
                 address(0), // fallbackHandler TODO: do we want to set the default compatibility fallback handler? will any UIs be harder without this?
                 address(0), // paymentToken
@@ -71,40 +71,34 @@ contract RumpelWalletFactory is Ownable, Pausable {
     // Admin ----
 
     function setRumpelGuard(address _rumpelGuard) public onlyOwner {
+        emit RumpelGuardUpdated(rumpelGuard, _rumpelGuard);
         rumpelGuard = _rumpelGuard;
-        emit RumpelGuardUpdated(_rumpelGuard);
     }
 
     function setRumpelModule(address _rumpelModule) public onlyOwner {
+        emit RumpelModuleUpdated(rumpelModule, _rumpelModule);
         rumpelModule = _rumpelModule;
-        emit RumpelModuleUpdated(_rumpelModule);
     }
 
     function setInitializationScript(address _initializationScript) public onlyOwner {
+        emit InitializationScriptUpdated(initializationScript, _initializationScript);
         initializationScript = _initializationScript;
-        emit InitializationScriptUpdated(_initializationScript);
     }
 
     function setSafeSingleton(address _safeSingleton) public onlyOwner {
+        emit SafeSingletonUpdated(safeSingleton, _safeSingleton);
         safeSingleton = _safeSingleton;
-        emit SafeSingletonUpdated(_safeSingleton);
     }
 
     function setProxyFactory(ISafeProxyFactory _proxyFactory) public onlyOwner {
+        emit ProxyFactoryUpdated(address(proxyFactory), address(_proxyFactory));
         proxyFactory = _proxyFactory;
-        emit ProxyFactoryUpdated(address(_proxyFactory));
     }
 
-    /**
-     * @dev Pauses wallet creation. Can only be called by the owner.
-     */
     function pauseWalletCreation() public onlyOwner {
         _pause();
     }
 
-    /**
-     * @dev Unpauses wallet creation. Can only be called by the owner.
-     */
     function unpauseWalletCreation() public onlyOwner {
         _unpause();
     }

diff --git a/test/RumpelWallet.t.sol b/test/RumpelWallet.t.sol
@@ -162,11 +162,11 @@ contract RumpelWalletTest is Test {
     function testFuzz_GuardAllowAndDisallowCalls(address target, bytes4 functionSig) public {
         vm.prank(admin);
         rumpelGuard.setCallAllowed(target, functionSig, true);
-        assertTrue(rumpelGuard.allowedTargets(target, functionSig));
+        assertTrue(rumpelGuard.allowedCalls(target, functionSig));
 
         vm.prank(admin);
         rumpelGuard.setCallAllowed(target, functionSig, false);
-        assertFalse(rumpelGuard.allowedTargets(target, functionSig));
+        assertFalse(rumpelGuard.allowedCalls(target, functionSig));
     }
 
     function test_rumpelWalletIsGuarded() public {