diff --git a/src/PRBProxy.sol b/src/PRBProxy.sol index 5c8dad0..9cdb775 100644 --- a/src/PRBProxy.sol +++ b/src/PRBProxy.sol @@ -132,6 +132,7 @@ contract PRBProxy is IPRBProxy { bool permission ) external override onlyOwner { permissions[envoy][target][selector] = permission; + emit SetPermission(envoy, target, selector, permission); } /// @inheritdoc IPRBProxy diff --git a/src/interfaces/IPRBProxy.sol b/src/interfaces/IPRBProxy.sol index ed95225..d4c0f04 100644 --- a/src/interfaces/IPRBProxy.sol +++ b/src/interfaces/IPRBProxy.sol @@ -30,6 +30,8 @@ interface IPRBProxy { event Execute(address indexed target, bytes data, bytes response); + event SetPermission(address indexed envoy, address indexed target, bytes4 indexed selector, bool permission); + event TransferOwnership(address indexed oldOwner, address indexed newOwner); /*////////////////////////////////////////////////////////////////////////// diff --git a/test/prb-proxy/PRBProxy.t.sol b/test/prb-proxy/PRBProxy.t.sol index 1980853..cd7dddc 100644 --- a/test/prb-proxy/PRBProxy.t.sol +++ b/test/prb-proxy/PRBProxy.t.sol @@ -33,6 +33,8 @@ abstract contract PRBProxy_Test is Base_Test { event Execute(address indexed target, bytes data, bytes response); + event SetPermission(address indexed envoy, address indexed target, bytes4 indexed selector, bool permission); + event TransferOwnership(address indexed oldOwner, address indexed newOwner); /*////////////////////////////////////////////////////////////////////////// diff --git a/test/prb-proxy/set-permission/setPermission.t.sol b/test/prb-proxy/set-permission/setPermission.t.sol index 10bb7be..7603361 100644 --- a/test/prb-proxy/set-permission/setPermission.t.sol +++ b/test/prb-proxy/set-permission/setPermission.t.sol @@ -6,8 +6,6 @@ import { PRBProxy_Test } from "../PRBProxy.t.sol"; import { TargetDummy } from "../../helpers/targets/TargetDummy.t.sol"; contract SetPermission_Test is PRBProxy_Test { - bytes4 internal constant selector = TargetDummy.foo.selector; - /// @dev it should revert. function test_RevertWhen_CallerNotOwner() external { // Make Eve the caller in this test. @@ -16,7 +14,12 @@ contract SetPermission_Test is PRBProxy_Test { // Run the test. vm.expectRevert(abi.encodeWithSelector(IPRBProxy.PRBProxy_NotOwner.selector, owner, caller)); - proxy.setPermission(caller, address(targets.dummy), selector, true); + proxy.setPermission({ + envoy: caller, + target: address(targets.dummy), + selector: TargetDummy.foo.selector, + permission: true + }); } modifier callerOwner() { @@ -25,27 +28,87 @@ contract SetPermission_Test is PRBProxy_Test { /// @dev it should set the permission. function test_SetPermission_PermissionNotSet() external callerOwner { - proxy.setPermission(users.envoy, address(targets.dummy), selector, true); - bool permission = proxy.getPermission(users.envoy, address(targets.dummy), selector); + proxy.setPermission({ + envoy: users.envoy, + target: address(targets.dummy), + selector: TargetDummy.foo.selector, + permission: true + }); + bool permission = proxy.getPermission({ + envoy: users.envoy, + target: address(targets.dummy), + selector: TargetDummy.foo.selector + }); assertTrue(permission); } modifier permissionSet() { - proxy.setPermission(users.envoy, address(targets.dummy), selector, true); + proxy.setPermission({ + envoy: users.envoy, + target: address(targets.dummy), + selector: TargetDummy.foo.selector, + permission: true + }); _; } /// @dev it should do nothing when re-setting the permission. function test_SetPermission_PermissionSet_ResetPermission() external callerOwner permissionSet { - proxy.setPermission(users.envoy, address(targets.dummy), selector, true); - bool permission = proxy.getPermission(users.envoy, address(targets.dummy), selector); + proxy.setPermission({ + envoy: users.envoy, + target: address(targets.dummy), + selector: TargetDummy.foo.selector, + permission: true + }); + bool permission = proxy.getPermission({ + envoy: users.envoy, + target: address(targets.dummy), + selector: TargetDummy.foo.selector + }); assertTrue(permission); } + /// @dev it should do nothing when re-setting the permission. + function test_SetPermission_PermissionSet_ResetPermission_Event() external callerOwner permissionSet { + vm.expectEmit({ checkTopic1: true, checkTopic2: true, checkTopic3: true, checkData: true }); + emit SetPermission({ + envoy: users.envoy, + target: address(targets.dummy), + selector: TargetDummy.foo.selector, + permission: true + }); + proxy.setPermission({ + envoy: users.envoy, + target: address(targets.dummy), + selector: TargetDummy.foo.selector, + permission: true + }); + } + /// @dev it should unset the permission. function test_SetPermission_PermissionSet_UnsetPermission() external callerOwner permissionSet { - proxy.setPermission(users.envoy, address(targets.dummy), selector, false); - bool permission = proxy.getPermission(users.envoy, address(targets.dummy), selector); - assertFalse(permission); + proxy.setPermission({ + envoy: users.envoy, + target: address(targets.dummy), + selector: TargetDummy.foo.selector, + permission: false + }); + } + + /// @dev it should unset the permission. + function test_SetPermission_PermissionSet_UnsetPermission_Event() external callerOwner permissionSet { + vm.expectEmit({ checkTopic1: true, checkTopic2: true, checkTopic3: true, checkData: true }); + emit SetPermission({ + envoy: users.envoy, + target: address(targets.dummy), + selector: TargetDummy.foo.selector, + permission: false + }); + proxy.setPermission({ + envoy: users.envoy, + target: address(targets.dummy), + selector: TargetDummy.foo.selector, + permission: false + }); } } diff --git a/test/prb-proxy/set-permission/setPermission.tree b/test/prb-proxy/set-permission/setPermission.tree index b80c478..fffaaa9 100644 --- a/test/prb-proxy/set-permission/setPermission.tree +++ b/test/prb-proxy/set-permission/setPermission.tree @@ -5,5 +5,9 @@ setPermission.t.sol ├── when the permission was not set │ └── it should set the permission └── when the permission was set - ├── it should do nothing when re-setting the permission - └── it should unset the permission + ├── when resetting the permission + │ ├── it should reset the permission + │ └── it should emit a {SetPermission} event + └── when unsetting the permission + ├── it should unset the permission + └── it should emit a {SetPermission} event