Skip to content
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.

Sign up for GitHub

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

Implement IPRBProxyStorage interface #84

Merged
merged 2 commits into from
Mar 8, 2023
Merged

Implement IPRBProxyStorage interface #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5400f2
feat: add "IPRBProxyStorage" interface
andreivladbrg Mar 8, 2023
e16f82a
refactor: make mappings public
andreivladbrg Mar 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 2 additions & 35 deletions src/PRBProxy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,18 @@ pragma solidity >=0.8.18;
import { IPRBProxy } from "./interfaces/IPRBProxy.sol";
import { IPRBProxyPlugin } from "./interfaces/IPRBProxyPlugin.sol";
import { IPRBProxyRegistry } from "./interfaces/IPRBProxyRegistry.sol";
import { PRBProxyStorage } from "./PRBProxyStorage.sol";

/// @title PRBProxy
/// @dev This contract implements the {IPRBProxy} interface.
contract PRBProxy is IPRBProxy {
contract PRBProxy is IPRBProxy, PRBProxyStorage {
/*//////////////////////////////////////////////////////////////////////////
CONSTANTS
//////////////////////////////////////////////////////////////////////////*/

/// @inheritdoc IPRBProxy
IPRBProxyRegistry public immutable override registry;

/*//////////////////////////////////////////////////////////////////////////
PUBLIC STORAGE
//////////////////////////////////////////////////////////////////////////*/

/// @inheritdoc IPRBProxy
address public override owner;

/// @inheritdoc IPRBProxy
uint256 public override minGasReserve;

/*//////////////////////////////////////////////////////////////////////////
INTERNAL STORAGE
//////////////////////////////////////////////////////////////////////////*/

/// @dev Maps plugin methods to plugin implementation.
mapping(bytes4 method => IPRBProxyPlugin plugin) internal plugins;

/// @dev Maps envoys to target contracts to function selectors to boolean flags.
mapping(address envoy => mapping(address target => bool permission)) internal permissions;

/*//////////////////////////////////////////////////////////////////////////
CONSTRUCTOR
//////////////////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -84,20 +65,6 @@ contract PRBProxy is IPRBProxy {
/// @dev Called when the call data is empty.
receive() external payable { }

/*//////////////////////////////////////////////////////////////////////////
PUBLIC CONSTANT FUNCTIONS
//////////////////////////////////////////////////////////////////////////*/

/// @inheritdoc IPRBProxy
function getPermission(address envoy, address target) external view override returns (bool permission) {
permission = permissions[envoy][target];
}

/// @inheritdoc IPRBProxy
function getPluginForMethod(bytes4 method) external view override returns (IPRBProxyPlugin plugin) {
plugin = plugins[method];
}

/*/////////////////////////////////////////////////////////////////////////
PUBLIC NON-CONSTANT FUNCTIONS
//////////////////////////////////////////////////////////////////////////*/
Expand Down
30 changes: 12 additions & 18 deletions src/PRBProxyStorage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,24 @@
pragma solidity >=0.8.18;

import { IPRBProxyPlugin } from "./interfaces/IPRBProxyPlugin.sol";
import { IPRBProxyStorage } from "./interfaces/IPRBProxyStorage.sol";

/// @notice Abstract contract with the storage layout of the {PRBProxy} contract.
/// @dev This contract is an exact replica of the storage layout of {PRBProxy}, and it exists so that it can
/// be inherited in target contracts. However, to avoid overcomplicating the inheritance structure, this is
/// not inherited by the {PRBProxy} contract itself.
abstract contract PRBProxyStorage {
/// @title PRBProxyStorage
/// @dev This contract implements the {IPRBProxyStorage} interface.
abstract contract PRBProxyStorage is IPRBProxyStorage {
/*//////////////////////////////////////////////////////////////////////////
PUBLIC STORAGE
//////////////////////////////////////////////////////////////////////////*/

/// @notice The address of the owner account or contract.
address public owner;
/// @inheritdoc IPRBProxyStorage
address public override owner;

Check warning

Code scanning / Slither

State variables that could be declared constant

PRBProxyStorage.owner (src/PRBProxyStorage.sol#15) should be constant

Check warning

Code scanning / Slither

State variables that could be declared immutable

PRBProxyStorage.owner (src/PRBProxyStorage.sol#15) should be immutable

/// @notice How much gas to reserve for running the remainder of the "execute" function after the DELEGATECALL.
/// @dev This prevents the proxy from becoming unusable if EVM opcode gas costs change in the future.
uint256 public minGasReserve;
/// @inheritdoc IPRBProxyStorage
uint256 public override minGasReserve;

Check warning

Code scanning / Slither

State variables that could be declared immutable

PRBProxyStorage.minGasReserve (src/PRBProxyStorage.sol#18) should be immutable

/*//////////////////////////////////////////////////////////////////////////
INTERNAL STORAGE
//////////////////////////////////////////////////////////////////////////*/

/// @dev Maps plugin methods to plugin implementation.
mapping(bytes4 method => IPRBProxyPlugin plugin) internal plugins;
/// @inheritdoc IPRBProxyStorage
mapping(address envoy => mapping(address target => bool permission)) public permissions;

Check failure

Code scanning / Slither

Uninitialized state variables

PRBProxyStorage.permissions (src/PRBProxyStorage.sol#21) is never initialized. It is used in: - PRBProxy.execute(address,bytes) (src/PRBProxy.sol#73-104)

/// @dev Maps envoys to target contracts to function selectors to boolean flags.
mapping(address envoy => mapping(address target => bool permission)) internal permissions;
/// @inheritdoc IPRBProxyStorage
mapping(bytes4 method => IPRBProxyPlugin plugin) public plugins;

Check failure

Code scanning / Slither

Uninitialized state variables

PRBProxyStorage.plugins (src/PRBProxyStorage.sol#24) is never initialized. It is used in: - PRBProxy.fallback(bytes) (src/PRBProxy.sol#37-63)
}
21 changes: 3 additions & 18 deletions src/interfaces/IPRBProxy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ pragma solidity >=0.8.4;

import { IPRBProxyPlugin } from "./IPRBProxyPlugin.sol";
import { IPRBProxyRegistry } from "./IPRBProxyRegistry.sol";
import { IPRBProxyStorage } from "./IPRBProxyStorage.sol";

/// @title IPRBProxy
/// @notice Proxy contract to compose transactions on owner's behalf.
interface IPRBProxy {
interface IPRBProxy is IPRBProxyStorage {
/*//////////////////////////////////////////////////////////////////////////
ERRORS
//////////////////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -43,25 +44,9 @@ interface IPRBProxy {
event RunPlugin(IPRBProxyPlugin indexed plugin, bytes data, bytes response);

/*//////////////////////////////////////////////////////////////////////////
PUBLIC CONSTANT FUNCTIONS
PUBLIC CONSTANT FUNCTIONS
//////////////////////////////////////////////////////////////////////////*/

/// @notice Returns a boolean flag that indicates whether the envoy has permission to call the provided target
/// contract.
function getPermission(address envoy, address target) external view returns (bool permission);

/// @notice Returns the address of the plugin installed for the the provided method.
/// @dev Returns the zero address if no plugin is installed.
/// @param method The signature of the method to make the query for.
function getPluginForMethod(bytes4 method) external view returns (IPRBProxyPlugin plugin);

/// @notice How much gas to reserve for running the remainder of the "execute" function after the DELEGATECALL.
/// @dev This prevents the proxy from becoming unusable if EVM opcode gas costs change in the future.
function minGasReserve() external view returns (uint256);

/// @notice The address of the owner account or contract.
function owner() external view returns (address);

/// @notice The address of the registry that has deployed this proxy.
function registry() external view returns (IPRBProxyRegistry);

Expand Down
28 changes: 28 additions & 0 deletions src/interfaces/IPRBProxyStorage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.8.4;

import { IPRBProxyPlugin } from "./IPRBProxyPlugin.sol";

/// @title IPRBProxyStorage
/// @dev Storage contract so that it can be inherited in target contracts.
interface IPRBProxyStorage {
/*//////////////////////////////////////////////////////////////////////////
PUBLIC CONSTANT FUNCTIONS
//////////////////////////////////////////////////////////////////////////*/

/// @notice How much gas to reserve for running the remainder of the "execute" function after the DELEGATECALL.
/// @dev This prevents the proxy from becoming unusable if EVM opcode gas costs change in the future.
function minGasReserve() external view returns (uint256);

/// @notice The address of the owner account or contract.
function owner() external view returns (address);

/// @notice Returns a boolean flag that indicates whether the envoy has permission to call the provided target
/// contract.
function permissions(address envoy, address target) external view returns (bool permission);

/// @notice Returns the address of the plugin installed for the the provided method.
/// @dev Returns the zero address if no plugin is installed.
/// @param method The signature of the method to make the query for.
function plugins(bytes4 method) external view returns (IPRBProxyPlugin plugin);
}
4 changes: 2 additions & 2 deletions test/helpers/install-plugin/installPlugin.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ contract InstallPlugin_Test is Helpers_Test {
// Assert that every plugin method has been installed.
bytes4[] memory pluginMethods = plugins.dummy.methodList();
for (uint256 i = 0; i < pluginMethods.length; ++i) {
IPRBProxyPlugin actualPlugin = proxy.getPluginForMethod(pluginMethods[i]);
IPRBProxyPlugin actualPlugin = proxy.plugins(pluginMethods[i]);
IPRBProxyPlugin expectedPlugin = plugins.dummy;
assertEq(actualPlugin, expectedPlugin, "Plugin method not installed");
}
Expand All @@ -46,7 +46,7 @@ contract InstallPlugin_Test is Helpers_Test {
// Assert that every plugin method has been installed.
bytes4[] memory pluginMethods = plugins.dummy.methodList();
for (uint256 i = 0; i < pluginMethods.length; ++i) {
IPRBProxyPlugin actualPlugin = proxy.getPluginForMethod(pluginMethods[i]);
IPRBProxyPlugin actualPlugin = proxy.plugins(pluginMethods[i]);
IPRBProxyPlugin expectedPlugin = plugins.dummy;
assertEq(actualPlugin, expectedPlugin, "Plugin method not installed");
}
Expand Down
4 changes: 2 additions & 2 deletions test/helpers/set-permission/setPermission.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ contract SetPermission_Test is Helpers_Test {
/// @dev it should set the permission.
function test_SetPermission_PermissionNotSet() external {
setPermission({ envoy: users.envoy, target: address(targets.dummy), permission: true });
bool permission = proxy.getPermission({ envoy: users.envoy, target: address(targets.dummy) });
bool permission = proxy.permissions({ envoy: users.envoy, target: address(targets.dummy) });
assertTrue(permission);
}

Expand All @@ -19,7 +19,7 @@ contract SetPermission_Test is Helpers_Test {
/// @dev it should do nothing when re-setting the permission.
function test_SetPermission_PermissionSet_ResetPermission() external permissionSet {
setPermission({ envoy: users.envoy, target: address(targets.dummy), permission: true });
bool permission = proxy.getPermission({ envoy: users.envoy, target: address(targets.dummy) });
bool permission = proxy.permissions({ envoy: users.envoy, target: address(targets.dummy) });
assertTrue(permission);
}

Expand Down
4 changes: 2 additions & 2 deletions test/helpers/uninstall-plugin/uninstallPlugin.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ contract UninstallPlugin_Test is Helpers_Test {
// Assert that every plugin method has been uninstalled.
bytes4[] memory pluginMethods = plugins.dummy.methodList();
for (uint256 i = 0; i < pluginMethods.length; ++i) {
IPRBProxyPlugin actualPlugin = proxy.getPluginForMethod(pluginMethods[i]);
IPRBProxyPlugin actualPlugin = proxy.plugins(pluginMethods[i]);
IPRBProxyPlugin expectedPlugin = IPRBProxyPlugin(address(0));
assertEq(actualPlugin, expectedPlugin, "Plugin method installed");
}
Expand All @@ -45,7 +45,7 @@ contract UninstallPlugin_Test is Helpers_Test {
// Assert that every plugin method has been uninstalled.
bytes4[] memory pluginMethods = plugins.dummy.methodList();
for (uint256 i = 0; i < pluginMethods.length; ++i) {
IPRBProxyPlugin actualPlugin = proxy.getPluginForMethod(pluginMethods[i]);
IPRBProxyPlugin actualPlugin = proxy.plugins(pluginMethods[i]);
IPRBProxyPlugin expectedPlugin = IPRBProxyPlugin(address(0));
assertEq(actualPlugin, expectedPlugin, "Plugin method installed");
}
Expand Down
Loading