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.

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

refactor: plugin redesign #125

Merged
merged 2 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
108 changes: 72 additions & 36 deletions src/PRBProxyRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ contract PRBProxyRegistry is IPRBProxyRegistry {
INTERNAL STORAGE
//////////////////////////////////////////////////////////////////////////*/

mapping(address owner => mapping(IPRBProxyPlugin plugin => bytes4[] methods)) internal _methods;

mapping(address owner => mapping(address envoy => mapping(address target => bool permission))) internal _permissions;

mapping(address owner => mapping(bytes4 method => IPRBProxyPlugin plugin)) internal _plugins;
Expand Down Expand Up @@ -76,6 +78,23 @@ contract PRBProxyRegistry is IPRBProxyRegistry {
USER-FACING CONSTANT FUNCTIONS
//////////////////////////////////////////////////////////////////////////*/

/// @inheritdoc IPRBProxyRegistry
function getMethodsByOwner(address owner, IPRBProxyPlugin plugin) external view returns (bytes4[] memory methods) {
methods = _methods[owner][plugin];
}

/// @inheritdoc IPRBProxyRegistry
function getMethodsByProxy(
IPRBProxy proxy,
IPRBProxyPlugin plugin
)
external
view
returns (bytes4[] memory methods)
{
methods = _methods[proxy.owner()][plugin];
}

/// @inheritdoc IPRBProxyRegistry
function getPermissionByOwner(
address owner,
Expand Down Expand Up @@ -159,35 +178,13 @@ contract PRBProxyRegistry is IPRBProxyRegistry {

/// @inheritdoc IPRBProxyRegistry
function installPlugin(IPRBProxyPlugin plugin) external override onlyCallerWithProxy {
// Get the method list to install.
bytes4[] memory methodList = plugin.methodList();

// The plugin must have at least one listed method.
uint256 length = methodList.length;
if (length == 0) {
revert PRBProxyRegistry_PluginEmptyMethodList(plugin);
}

// Install every method in the list.
address owner = msg.sender;
for (uint256 i = 0; i < length;) {
// Check for collisions.
bytes4 method = methodList[i];
if (address(_plugins[owner][method]) != address(0)) {
revert PRBProxyRegistry_PluginMethodCollision({
currentPlugin: _plugins[owner][method],
newPlugin: plugin,
method: method
});
}
_plugins[owner][method] = plugin;
unchecked {
i += 1;
}
}
_installPlugin(plugin);
}

// Log the plugin installation.
emit InstallPlugin(owner, _proxies[owner], plugin);
/// @inheritdoc IPRBProxyRegistry
function deployAndInstallPlugin(IPRBProxyPlugin plugin) external returns (IPRBProxy proxy) {
proxy = _deploy({ owner: msg.sender });
_installPlugin(plugin);
}

/// @inheritdoc IPRBProxyRegistry
Expand All @@ -199,26 +196,29 @@ contract PRBProxyRegistry is IPRBProxyRegistry {

/// @inheritdoc IPRBProxyRegistry
function uninstallPlugin(IPRBProxyPlugin plugin) external override onlyCallerWithProxy {
// Get the method list to uninstall.
bytes4[] memory methodList = plugin.methodList();
// Retrieve the methods originally installed by this plugin.
address owner = msg.sender;
bytes4[] memory methods = _methods[owner][plugin];

// The plugin must have at least one listed method.
uint256 length = methodList.length;
// The plugin must be a known, previously installed plugin.
uint256 length = methods.length;
if (length == 0) {
revert PRBProxyRegistry_PluginEmptyMethodList(plugin);
revert PRBProxyRegistry_PluginUnknown(plugin);
}

// Uninstall every method in the list.
address owner = msg.sender;
for (uint256 i = 0; i < length;) {
delete _plugins[owner][methodList[i]];
delete _plugins[owner][methods[i]];
unchecked {
i += 1;
}
}

// Remove the methods from the reverse mapping.
delete _methods[owner][plugin];

// Log the plugin uninstallation.
emit UninstallPlugin(owner, _proxies[owner], plugin);
emit UninstallPlugin(owner, _proxies[owner], plugin, methods);
}

/*//////////////////////////////////////////////////////////////////////////
Expand All @@ -243,4 +243,40 @@ contract PRBProxyRegistry is IPRBProxyRegistry {
// Log the creation of the proxy.
emit DeployProxy({ operator: msg.sender, owner: owner, proxy: proxy });
}

/// @dev See the documentation for the user-facing functions that call this internal function.
function _installPlugin(IPRBProxyPlugin plugin) internal {
// Retrieve the methods to install.
bytes4[] memory methods = plugin.getMethods();

// The plugin must implement at least one method.
uint256 length = methods.length;
if (length == 0) {
revert PRBProxyRegistry_PluginWithZeroMethods(plugin);
}

// Install every method in the list.
address owner = msg.sender;
for (uint256 i = 0; i < length;) {
// Check for collisions.
bytes4 method = methods[i];
if (address(_plugins[owner][method]) != address(0)) {
revert PRBProxyRegistry_PluginMethodCollision({
currentPlugin: _plugins[owner][method],
newPlugin: plugin,
method: method
});
}
_plugins[owner][method] = plugin;
unchecked {
i += 1;
}
}

// Set the methods in the reverse mapping.
_methods[owner][plugin] = methods;

// Log the plugin installation.
emit InstallPlugin(owner, _proxies[owner], plugin, methods);
}
Comment on lines +248 to +281

Check notice

Code scanning / Slither

Reentrancy vulnerabilities

Reentrancy in PRBProxyRegistry._installPlugin(IPRBProxyPlugin) (src/PRBProxyRegistry.sol#248-281): External calls: - methods = plugin.getMethods() (src/PRBProxyRegistry.sol#250) State variables written after the call(s): - _methods[owner][plugin] = methods (src/PRBProxyRegistry.sol#277) - _plugins[owner][method] = plugin (src/PRBProxyRegistry.sol#270)
}
10 changes: 5 additions & 5 deletions src/interfaces/IPRBProxyPlugin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ pragma solidity >=0.8.4;
/// @title IPRBProxyPlugin
/// @notice Interface for plugin contracts that can be installed on a proxy.
/// @dev Plugins are contracts that enable the proxy to interact with and respond to calls from other contracts. These
/// plugins are run in the proxy's fallback function.
/// plugins are run via the proxy's fallback function.
///
/// This interface is meant to be directly inherited by plugin implementations.
interface IPRBProxyPlugin {
/// @notice Enumerates the methods implemented by the plugin.
/// @dev These methods can be installed and uninstalled.
/// @notice Retrieves the methods implemented by the plugin.
/// @dev The registry pulls these methods when installing the plugin.
///
/// Requirements:
/// - The plugin must implement at least one method.
///
/// @return methods An array of the methods implemented by the plugin.
function methodList() external returns (bytes4[] memory methods);
/// @return methods The array of the methods implemented by the plugin.
function getMethods() external returns (bytes4[] memory methods);
}
Loading