From 60d7adc9ebbb3b321eaff555539ecde6e72029e0 Mon Sep 17 00:00:00 2001 From: Sam Goldman Date: Mon, 16 Jan 2023 16:55:54 -0500 Subject: [PATCH] fix(ct): make executors permissioned --- .changeset/old-badgers-whisper.md | 8 + .../contracts/ChugSplashBootLoader.sol | 5 +- .../contracts/contracts/ChugSplashManager.sol | 185 +++------- .../contracts/ChugSplashRegistry.sol | 79 ++++- .../contracts/contracts/ProxyInitializer.sol | 7 +- packages/contracts/src/constants.ts | 7 +- packages/contracts/src/ifaces.ts | 2 +- .../contracts/test/ChugSplashManager.t.sol | 319 ++++++------------ .../contracts/test/ChugSplashRegistry.t.sol | 65 +++- .../contracts/test/integration-tests.t.sol | 3 +- packages/core/src/actions/execute.ts | 19 -- packages/core/src/fund.ts | 8 +- .../core/src/languages/solidity/predeploys.ts | 48 ++- packages/core/src/tasks/index.ts | 6 +- packages/core/src/utils.ts | 2 +- packages/executor/src/index.ts | 1 + packages/executor/src/utils/etherscan.ts | 2 +- packages/plugins/src/hardhat/tasks.ts | 25 +- 18 files changed, 357 insertions(+), 434 deletions(-) create mode 100644 .changeset/old-badgers-whisper.md diff --git a/.changeset/old-badgers-whisper.md b/.changeset/old-badgers-whisper.md new file mode 100644 index 000000000..87bfb7f27 --- /dev/null +++ b/.changeset/old-badgers-whisper.md @@ -0,0 +1,8 @@ +--- +'@chugsplash/contracts': patch +'@chugsplash/core': patch +'@chugsplash/executor': patch +'@chugsplash/plugins': patch +--- + +Make executors permissioned diff --git a/packages/contracts/contracts/ChugSplashBootLoader.sol b/packages/contracts/contracts/ChugSplashBootLoader.sol index cb71f95bd..325fa9b87 100644 --- a/packages/contracts/contracts/ChugSplashBootLoader.sol +++ b/packages/contracts/contracts/ChugSplashBootLoader.sol @@ -40,7 +40,6 @@ contract ChugSplashBootLoader is Initializable { * using ChugSplash! * * @param _owner Address of the owner of the ChugSplash contracts. - * @param _executorBondAmount Executor bond amount in ETH. * @param _executionLockTime Amount of time for an executor to completely execute a * bundle after claiming it. * @param _ownerBondAmount Amount that must be deposited in this contract in order to @@ -54,7 +53,6 @@ contract ChugSplashBootLoader is Initializable { */ function initialize( address _owner, - uint256 _executorBondAmount, uint256 _executionLockTime, uint256 _ownerBondAmount, uint256 _executorPaymentPercentage, @@ -84,10 +82,11 @@ contract ChugSplashBootLoader is Initializable { registryImplementation = new ChugSplashRegistry{ salt: _salt }( address(proxyUpdater), _ownerBondAmount, - _executorBondAmount, _executionLockTime, _executorPaymentPercentage, _managerImplementation ); + + registryImplementation.initialize(_owner, new address[](0)); } } diff --git a/packages/contracts/contracts/ChugSplashManager.sol b/packages/contracts/contracts/ChugSplashManager.sol index 9278c8875..b010d8ac4 100644 --- a/packages/contracts/contracts/ChugSplashManager.sol +++ b/packages/contracts/contracts/ChugSplashManager.sol @@ -105,14 +105,6 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { string target ); - /** - * @notice Emitted when a bundle is claimed by an executor. - * - * @param bundleId ID of the bundle that was claimed. - * @param executor Address of the executor that claimed the bundle ID for the project. - */ - event ChugSplashBundleClaimed(bytes32 indexed bundleId, address indexed executor); - /** * @notice Emitted when an executor claims a payment. * @@ -202,17 +194,11 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { * @notice Amount that must be deposited in this contract in order to execute a bundle. The * project owner can withdraw this amount whenever a bundle is not active. This bond * will be forfeited if the project owner cancels a bundle that is in progress, which is - * necessary to prevent owners from trolling executors by immediately cancelling and + * necessary to prevent owners from trolling the executor by immediately cancelling and * withdrawing funds. */ uint256 public immutable ownerBondAmount; - /** - * @notice Amount in ETH that the executor must send to this contract to claim a bundle for - * `executionLockTime`. - */ - uint256 public immutable executorBondAmount; - /** * @notice Amount of time for an executor to finish executing a bundle once they have claimed * it. If the executor fails to completely execute the bundle in this amount of time, @@ -221,9 +207,9 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { uint256 public immutable executionLockTime; /** - * @notice Amount that executors are paid, denominated as a percentage of the cost of execution. - * For example: if a bundle costs 1 gwei to execute and the executorPaymentPercentage is - * 10, then the executor will profit 0.1 gwei. + * @notice Amount that the executor is paid, denominated as a percentage of the cost of + * execution. For example: if a bundle costs 1 gwei to execute and the + * executorPaymentPercentage is 10, then the executor will profit 0.1 gwei. */ uint256 public immutable executorPaymentPercentage; @@ -233,12 +219,6 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { */ mapping(string => address payable) public proxies; - /** - * @notice Mapping of executor addresses to the ETH amount stored in this contract that is - * owed to them. - */ - mapping(address => uint256) public debt; - /** * @notice Mapping of targets to proxy types. If a target is using the default proxy, * then its value in this mapping is bytes32(0). @@ -279,16 +259,26 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { bytes32 public activeBundleId; /** - * @notice Total ETH amount that is owed to executors. + * @notice ETH amount that is owed to the executor. + */ + uint256 public debt; + + /** + * @notice Modifier that restricts access to the executor. */ - uint256 public totalDebt; + modifier onlyExecutor() { + require( + registry.executors(msg.sender) == true, + "ChugSplashManager: caller is not an executor" + ); + _; + } /** * @param _registry Address of the ChugSplashRegistry. * @param _name Name of the project this contract is managing. * @param _owner Address of the project owner. * @param _proxyUpdater Address of the ProxyUpdater. - * @param _executorBondAmount Executor bond amount in ETH. * @param _executionLockTime Amount of time for an executor to completely execute a * bundle after claiming it. * @param _ownerBondAmount Amount that must be deposited in this contract in order to @@ -301,14 +291,12 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { string memory _name, address _owner, address _proxyUpdater, - uint256 _executorBondAmount, uint256 _executionLockTime, uint256 _ownerBondAmount, uint256 _executorPaymentPercentage ) { registry = _registry; proxyUpdater = _proxyUpdater; - executorBondAmount = _executorBondAmount; executionLockTime = _executionLockTime; ownerBondAmount = _ownerBondAmount; executorPaymentPercentage = _executorPaymentPercentage; @@ -344,18 +332,6 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { return keccak256(abi.encode(_bundleRoot, _bundleSize, _configUri)); } - /** - * @notice Queries the selected executor for a given project/bundle. - * - * @param _bundleId ID of the bundle currently being executed. - * - * @return Address of the selected executor. - */ - function getSelectedExecutor(bytes32 _bundleId) public view returns (address) { - ChugSplashBundleState storage bundle = _bundles[_bundleId]; - return bundle.selectedExecutor; - } - /** * @notice Gets the ChugSplashBundleState struct for a given bundle ID. Note that we explicitly * define this function because the getter function that is automatically generated by @@ -436,7 +412,7 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { */ function approveChugSplashBundle(bytes32 _bundleId) public onlyOwner { require( - address(this).balance - totalDebt >= ownerBondAmount, + address(this).balance - debt >= ownerBondAmount, "ChugSplashManager: insufficient balance in manager" ); @@ -464,7 +440,8 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { * executor doesn't need to send as many transactions to execute a bundle. Note that * this function only accepts SetStorage and DeployImplementation actions. * SetImplementation actions must be sent separately to `completeChugSplashBundle` after - * the SetStorage and DeployImplementation actions have been executed. + * the SetStorage and DeployImplementation actions have been executed. Only callable by + * the executor. * * @param _actions Array of SetStorage/DeployImplementation actions to execute. * @param _actionIndexes Array of action indexes. @@ -474,7 +451,7 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { ChugSplashAction[] memory _actions, uint256[] memory _actionIndexes, bytes32[][] memory _proofs - ) public { + ) public onlyExecutor { for (uint256 i = 0; i < _actions.length; i++) { executeChugSplashAction(_actions[i], _actionIndexes[i], _proofs[i]); } @@ -484,7 +461,7 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { * @notice Executes a specific action within the current active bundle for a project. Actions * can only be executed once. A re-entrancy guard is added to prevent an implementation * contract's constructor from calling another contract which in turn calls back into - * this function. + * this function. Only callable by the executor * * @param _action Action to execute. * @param _actionIndex Index of the action in the bundle. @@ -494,7 +471,7 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { ChugSplashAction memory _action, uint256 _actionIndex, bytes32[] memory _proof - ) public nonReentrant { + ) public nonReentrant onlyExecutor { uint256 initialGasLeft = gasleft(); require( @@ -509,12 +486,6 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { "ChugSplashManager: action has already been executed" ); - address executor = getSelectedExecutor(activeBundleId); - require( - executor == msg.sender, - "ChugSplashManager: caller is not approved executor for active bundle ID" - ); - require( MerkleTree.verify( bundle.merkleRoot, @@ -559,9 +530,6 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { emit DefaultProxyDeployed(_action.target, proxy, activeBundleId, _action.target); registry.announce("DefaultProxyDeployed"); - } else if (_getProxyImplementation(proxy, adapter) != address(0)) { - // Set the proxy's implementation to address(0). - _setProxyStorage(proxy, adapter, EIP1967_IMPLEMENTATION_KEY, bytes32(0)); } } else { // We intend to support alternative proxy types in the future, but doing so requires @@ -572,6 +540,12 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { // proxy = proxies[_action.target]; } + if (_getProxyImplementation(proxy, adapter) != address(0)) { + // Set the proxy's implementation to address(0). This ensures that end-users can't + // accidentally interact with a proxy that is in the process of being upgraded. + _setProxyStorage(proxy, adapter, EIP1967_IMPLEMENTATION_KEY, bytes32(0)); + } + // Mark the action as executed and update the total number of executed actions. bundle.actionsExecuted++; bundle.executions[_actionIndex] = true; @@ -592,15 +566,14 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { // Estimate the amount of gas used in this call by subtracting the current gas left from the // initial gas left. We add 152778 to this amount to account for the intrinsic gas cost // (21k), the calldata usage, and the subsequent opcodes that occur when we add the - // executorPayment to the totalDebt and debt. Unfortunately, there is a wide variance in the + // executorPayment to the debt and debt. Unfortunately, there is a wide variance in the // gas costs of these last opcodes due to the variable cost of SSTORE. Also, gas refunds // might be contributing to the difficulty of getting a good estimate. For now, we err on // the side of safety by adding a larger value. // TODO: Get a better estimate than 152778. uint256 gasUsed = 152778 + initialGasLeft - gasleft(); - // Calculate the executor's payment and add it to the total debt and the current executor's - // debt. + // Calculate the executor's payment and add it to the debt owed to the executor. uint256 executorPayment; if (block.chainid != 10 && block.chainid != 420) { // Use the gas price for any network that isn't Optimism. @@ -615,8 +588,7 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { executorPayment = (gasUsed * (100 + executorPaymentPercentage)) / 100; } - totalDebt += executorPayment; - debt[msg.sender] += executorPayment; + debt += executorPayment; } /** @@ -624,6 +596,7 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { * single transaction to ensure that all contracts are initialized at the same time. * Note that this function will revert if it is called before all of the SetCode and * DeployImplementation actions have been executed in `executeChugSplashAction`. + * Only callable by the executor. * * @param _actions Array of ChugSplashActions, where each action type must be * `SET_IMPLEMENTATION`. @@ -634,7 +607,7 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { ChugSplashAction[] memory _actions, uint256[] memory _actionIndexes, bytes32[][] memory _proofs - ) public { + ) public onlyExecutor { uint256 initialGasLeft = gasleft(); require( @@ -642,12 +615,6 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { "ChugSplashManager: no bundle has been approved for execution" ); - address executor = getSelectedExecutor(activeBundleId); - require( - executor == msg.sender, - "ChugSplashManager: caller is not approved executor for active bundle ID" - ); - ChugSplashBundleState storage bundle = _bundles[activeBundleId]; for (uint256 i = 0; i < _actions.length; i++) { @@ -724,10 +691,8 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { executorPayment = (gasUsed * (100 + executorPaymentPercentage)) / 100; } - // Add the executor's payment to the total debt. - totalDebt += executorPayment; - // Add the executor's payment and the executor's bond to their debt. - debt[msg.sender] += executorPayment + executorBondAmount; + // Add the executor's payment to the debt. + debt += executorPayment; } /** @@ -745,19 +710,10 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { ChugSplashBundleState storage bundle = _bundles[activeBundleId]; - if (bundle.selectedExecutor != address(0)) { - if (bundle.timeClaimed + executionLockTime >= block.timestamp) { - // Give the owner's bond to the current executor if the bundle is cancelled within - // the `executionLockTime` window. Also return the executor's bond. - debt[bundle.selectedExecutor] += ownerBondAmount + executorBondAmount; - // We don't add the `executorBondAmount` to the `totalDebt` here because we already - // did this in `claimBundle`. - totalDebt += ownerBondAmount; - } else { - // Give the executor's bond to the owner if the `executionLockTime` window has - // passed. - totalDebt -= executorBondAmount; - } + if (bundle.timeClaimed + executionLockTime >= block.timestamp) { + // Give the owner's bond to the executor if the bundle is cancelled within + // the `executionLockTime` window. + debt += ownerBondAmount; } bytes32 cancelledBundleId = activeBundleId; @@ -769,56 +725,19 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { } /** - * @notice Allows an executor to post a bond of `executorBondAmount` to claim the sole right to - * execute actions for a bundle over a period of `executionLockTime`. Only the first - * executor to post a bond gains this right. Executors must finish executing the bundle - * within `executionLockTime` or else their bond is forfeited to this contract and - * another executor may claim the bundle. Note that this strategy creates a PGA for the - * transaction to claim the bundle but removes PGAs during the execution process. - */ - function claimBundle() external payable { - require(activeBundleId != bytes32(0), "ChugSplashManager: no bundle is currently active"); - require( - executorBondAmount == msg.value, - "ChugSplashManager: incorrect executor bond amount" - ); - - ChugSplashBundleState storage bundle = _bundles[activeBundleId]; - - require( - block.timestamp > bundle.timeClaimed + executionLockTime, - "ChugSplashManager: bundle is currently claimed by an executor" - ); - - address prevExecutor = bundle.selectedExecutor; - bundle.timeClaimed = block.timestamp; - bundle.selectedExecutor = msg.sender; - - // Add the new executor's bond to the `totalDebt` if there was no previous executor. We skip - // this if there was a previous executor because this allows the owner to claim the previous - // executor's forfeited bond. - if (prevExecutor == address(0)) { - totalDebt += executorBondAmount; - } - - emit ChugSplashBundleClaimed(activeBundleId, msg.sender); - registry.announce("ChugSplashBundleClaimed"); - } - - /** - * @notice Allows executors to claim their ETH payments and bond. Executors may only withdraw - * ETH that is owed to them by this contract. + * @notice Allows the executor to claim their ETH payments and bond. The executor may only + * withdraw ETH that is owed to it by this contract. */ - function claimExecutorPayment() external { - uint256 amount = debt[msg.sender]; + function claimExecutorPayment() external onlyExecutor { + require(debt > 0, "ChugSplashManager: no debt to withdraw"); - debt[msg.sender] -= amount; - totalDebt -= amount; + uint256 amountToWithdraw = debt; + debt = 0; - (bool success, ) = payable(msg.sender).call{ value: amount }(new bytes(0)); - require(success, "ChugSplashManager: call to withdraw owner funds failed"); + (bool success, ) = payable(msg.sender).call{ value: amountToWithdraw }(new bytes(0)); + require(success, "ChugSplashManager: call to withdraw executor funds failed"); - emit ExecutorPaymentClaimed(msg.sender, amount); + emit ExecutorPaymentClaimed(msg.sender, amountToWithdraw); registry.announce("ExecutorPaymentClaimed"); } @@ -857,8 +776,8 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { } /** - * @notice Allows the project owner to withdraw all funds in this contract minus the total debt - * owed to the executors. Cannot be called when there is an active bundle. + * @notice Allows the project owner to withdraw all funds in this contract minus the debt + * owed to the executor. Cannot be called when there is an active bundle. */ function withdrawOwnerETH() external onlyOwner { require( @@ -866,7 +785,7 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable { "ChugSplashManager: cannot withdraw funds while bundle is active" ); - uint256 amount = address(this).balance - totalDebt; + uint256 amount = address(this).balance - debt; (bool success, ) = payable(msg.sender).call{ value: amount }(new bytes(0)); require(success, "ChugSplashManager: call to withdraw owner funds failed"); diff --git a/packages/contracts/contracts/ChugSplashRegistry.sol b/packages/contracts/contracts/ChugSplashRegistry.sol index 81c1b4399..21eada1b2 100644 --- a/packages/contracts/contracts/ChugSplashRegistry.sol +++ b/packages/contracts/contracts/ChugSplashRegistry.sol @@ -4,6 +4,9 @@ pragma solidity ^0.8.9; import { ChugSplashManager } from "./ChugSplashManager.sol"; import { ChugSplashManagerProxy } from "./ChugSplashManagerProxy.sol"; import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import { + OwnableUpgradeable +} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; import { Proxy } from "./libraries/Proxy.sol"; /** @@ -13,7 +16,7 @@ import { Proxy } from "./libraries/Proxy.sol"; * find and index these deployments. Deployment names are unique and are reserved on a * first-come, first-served basis. */ -contract ChugSplashRegistry is Initializable { +contract ChugSplashRegistry is Initializable, OwnableUpgradeable { /** * @notice Emitted whenever a new project is registered. * @@ -74,6 +77,20 @@ contract ChugSplashRegistry is Initializable { */ event ProxyTypeAdded(bytes32 proxyType, address adapter); + /** + * @notice Emitted when an executor is added. + * + * @param executor Address of the added executor. + */ + event ExecutorAdded(address indexed executor); + + /** + * @notice Emitted when an executor is removed. + * + * @param executor Address of the removed executor. + */ + event ExecutorRemoved(address indexed executor); + /** * @notice Mapping of project names to ChugSplashManager contracts. */ @@ -89,6 +106,11 @@ contract ChugSplashRegistry is Initializable { */ mapping(bytes32 => address) public adapters; + /** + * @notice Addresses that can execute bundles. + */ + mapping(address => bool) public executors; + /** * @notice Address of the ProxyUpdater. */ @@ -99,11 +121,6 @@ contract ChugSplashRegistry is Initializable { */ uint256 public immutable ownerBondAmount; - /** - * @notice Amount that an executor must send to the ChugSplashManager to claim a bundle. - */ - uint256 public immutable executorBondAmount; - /** * @notice Amount of time for an executor to completely execute a bundle after claiming it. */ @@ -124,8 +141,6 @@ contract ChugSplashRegistry is Initializable { * @param _proxyUpdater Address of the ProxyUpdater. * @param _ownerBondAmount Amount that must be deposited in the ChugSplashManager in * order to execute a bundle. - * @param _executorBondAmount Amount that an executor must send to the ChugSplashManager - * to claim a bundle. * @param _executionLockTime Amount of time for an executor to completely execute a * bundle after claiming it. * @param _executorPaymentPercentage Amount that an executor will earn from completing a bundle, @@ -135,19 +150,30 @@ contract ChugSplashRegistry is Initializable { constructor( address _proxyUpdater, uint256 _ownerBondAmount, - uint256 _executorBondAmount, uint256 _executionLockTime, uint256 _executorPaymentPercentage, address _managerImplementation ) { proxyUpdater = _proxyUpdater; ownerBondAmount = _ownerBondAmount; - executorBondAmount = _executorBondAmount; executionLockTime = _executionLockTime; executorPaymentPercentage = _executorPaymentPercentage; managerImplementation = _managerImplementation; } + /** + * @param _owner Initial owner of this contract. + * @param _executors Array of executors to add. + */ + function initialize(address _owner, address[] memory _executors) public initializer { + __Ownable_init(); + _transferOwnership(_owner); + + for (uint i = 0; i < _executors.length; i++) { + _setExecutor(_executors[i], true); + } + } + /** * @notice Registers a new project. * @@ -228,4 +254,37 @@ contract ChugSplashRegistry is Initializable { emit ProxyTypeAdded(_proxyType, _adapter); } + + /** + * @notice Add an executor, which can execute bundles on behalf of users. Only callable by the + * owner of this contract. + * + * @param _executor Address of the executor to add. + */ + function addExecutor(address _executor) public onlyOwner { + require(executors[_executor] == false, "ChugSplashRegistry: executor already added"); + _setExecutor(_executor, true); + emit ExecutorAdded(_executor); + } + + /** + * @notice Remove an executor. Only callable by the owner of this contract. + * + * @param _executor Address of the executor to remove. + */ + function removeExecutor(address _executor) public onlyOwner { + require(executors[_executor] == true, "ChugSplashRegistry: executor already removed"); + _setExecutor(_executor, false); + emit ExecutorRemoved(_executor); + } + + /** + * @notice Internal function that adds or removes an executor. + * + * @param _executor Address of the executor to add or remove. + * @param _isExecutor Boolean indicating if the executor is being added or removed. + */ + function _setExecutor(address _executor, bool _isExecutor) internal { + executors[_executor] = _isExecutor; + } } diff --git a/packages/contracts/contracts/ProxyInitializer.sol b/packages/contracts/contracts/ProxyInitializer.sol index 8e78d562c..41efc1ef9 100644 --- a/packages/contracts/contracts/ProxyInitializer.sol +++ b/packages/contracts/contracts/ProxyInitializer.sol @@ -46,10 +46,11 @@ contract ProxyInitializer is Initializable { * new owner specified in the constructor. * * @param _implementation The proxy's implementation address. + * @param _data Data to delegatecall the new implementation with. */ - function initialize(address _implementation) external initializer { - // Set the proxy's implementation contract. - proxy.upgradeTo(_implementation); + function initialize(address _implementation, bytes memory _data) external initializer { + // Set the proxy's implementation contract and delegatecall it with the supplied data. + proxy.upgradeToAndCall(_implementation, _data); // Transfer ownership of the proxy to the new owner. proxy.changeAdmin(newOwner); diff --git a/packages/contracts/src/constants.ts b/packages/contracts/src/constants.ts index 82a73a6d0..93b72ea95 100644 --- a/packages/contracts/src/constants.ts +++ b/packages/contracts/src/constants.ts @@ -16,6 +16,7 @@ import { export const OWNER_MULTISIG_ADDRESS = '0xF2a21e4E9F22AAfD7e8Bf47578a550b4102732a9' +export const EXECUTOR = '0x42761facf5e6091fca0e38f450adfb1e22bd8c3c' export const CHUGSPLASH_SALT = '0x' + '11'.repeat(32) @@ -48,7 +49,6 @@ const chugsplashManagerConstructorArgTypes = export const DETERMINISTIC_DEPLOYMENT_PROXY_ADDRESS = '0x4e59b44847b379578588920ca78fbf26c0b4956c' export const OWNER_BOND_AMOUNT = ethers.utils.parseEther('0.001') -export const EXECUTOR_BOND_AMOUNT = ethers.utils.parseEther('0.01') export const EXECUTION_LOCK_TIME = 15 * 60 export const EXECUTOR_PAYMENT_PERCENTAGE = 20 @@ -125,7 +125,6 @@ const chugsplashManagerConstructorArgValues = [ 'Root Manager', OWNER_MULTISIG_ADDRESS, PROXY_UPDATER_ADDRESS, - EXECUTOR_BOND_AMOUNT, EXECUTION_LOCK_TIME, OWNER_BOND_AMOUNT, EXECUTOR_PAYMENT_PERCENTAGE, @@ -154,11 +153,10 @@ export const CHUGSPLASH_REGISTRY_ADDRESS = ethers.utils.getCreate2Address( [ ChugSplashRegistryArtifact.bytecode, ethers.utils.defaultAbiCoder.encode( - ['address', 'uint256', 'uint256', 'uint256', 'uint256', 'address'], + ['address', 'uint256', 'uint256', 'uint256', 'address'], [ PROXY_UPDATER_ADDRESS, OWNER_BOND_AMOUNT, - EXECUTOR_BOND_AMOUNT, EXECUTION_LOCK_TIME, EXECUTOR_PAYMENT_PERCENTAGE, CHUGSPLASH_MANAGER_ADDRESS, @@ -178,7 +176,6 @@ export const CHUGSPLASH_CONSTRUCTOR_ARGS = {} CHUGSPLASH_CONSTRUCTOR_ARGS[chugsplashRegistrySourceName] = [ PROXY_UPDATER_ADDRESS, OWNER_BOND_AMOUNT, - EXECUTOR_BOND_AMOUNT, EXECUTION_LOCK_TIME, EXECUTOR_PAYMENT_PERCENTAGE, CHUGSPLASH_MANAGER_ADDRESS, diff --git a/packages/contracts/src/ifaces.ts b/packages/contracts/src/ifaces.ts index 66034cc45..ddbb8dd36 100644 --- a/packages/contracts/src/ifaces.ts +++ b/packages/contracts/src/ifaces.ts @@ -8,7 +8,7 @@ export const DefaultAdapterArtifact = require('../artifacts/contracts/adapters/D export const ProxyArtifact = require('../artifacts/contracts/libraries/Proxy.sol/Proxy.json') export const ProxyInitializerArtifact = require('../artifacts/contracts/ProxyInitializer.sol/ProxyInitializer.json') -export const buildInfo = require('../artifacts/build-info/6523e78f87a5803afa0b08ab43b0b969.json') +export const buildInfo = require('../artifacts/build-info/af0bd9270cded1139bee6d9130cd340f.json') export const ChugSplashRegistryABI = ChugSplashRegistryArtifact.abi export const ChugSplashBootLoaderABI = ChugSplashBootLoaderArtifact.abi diff --git a/packages/contracts/test/ChugSplashManager.t.sol b/packages/contracts/test/ChugSplashManager.t.sol index 0f9693552..80dfa2d32 100644 --- a/packages/contracts/test/ChugSplashManager.t.sol +++ b/packages/contracts/test/ChugSplashManager.t.sol @@ -42,8 +42,6 @@ contract ChugSplashManager_Test is Test { string target ); - event ChugSplashBundleClaimed(bytes32 indexed bundleId, address indexed executor); - event ChugSplashActionExecuted( bytes32 indexed bundleId, address indexed proxy, @@ -107,14 +105,12 @@ contract ChugSplashManager_Test is Test { address proposer = address(64); address owner = address(128); address nonOwner = address(256); - address executor1 = address(512); - address executor2 = address(1024); + address executor = address(512); bytes32 salt = bytes32(hex"11"); uint256 initialTimestamp = 1641070800; uint256 bundleExecutionCost = 2 ether; string projectName = 'TestProject'; uint256 ownerBondAmount = 10e8 gwei; // 0.1 ETH - uint256 executorBondAmount = 1 ether; uint256 executionLockTime = 15 minutes; uint256 executorPaymentPercentage = 20; uint256 bundleSize = actionIndexes.length; @@ -181,7 +177,6 @@ contract ChugSplashManager_Test is Test { projectName, owner, proxyUpdaterAddress, - executorBondAmount, executionLockTime, ownerBondAmount, executorPaymentPercentage @@ -189,7 +184,6 @@ contract ChugSplashManager_Test is Test { bootloader.initialize( owner, - executorBondAmount, executionLockTime, ownerBondAmount, executorPaymentPercentage, @@ -208,6 +202,13 @@ contract ChugSplashManager_Test is Test { registry = ChugSplashRegistry(address(registryProxy)); registry.register(projectName, owner); + + vm.startPrank(owner); + address[] memory executors = new address[](1); + executors[0] = executor; + registry.initialize(owner, executors); + vm.stopPrank(); + manager = registry.projects(projectName); adapter = new DefaultAdapter(); @@ -219,7 +220,6 @@ contract ChugSplashManager_Test is Test { function test_constructor_success() external { assertEq(address(manager.registry()), address(registry)); assertEq(address(manager.proxyUpdater()), address(bootloader.proxyUpdater())); - assertEq(manager.executorBondAmount(), executorBondAmount); assertEq(manager.executionLockTime(), executionLockTime); assertEq(manager.ownerBondAmount(), ownerBondAmount); assertEq(manager.executorPaymentPercentage(), executorPaymentPercentage); @@ -241,14 +241,9 @@ contract ChugSplashManager_Test is Test { assertEq(manager.computeBundleId(bundleRoot, bundleSize, configUri), expectedBundleId); } - function test_getSelectedExecutor_success() external { - helper_proposeThenApproveThenFundThenClaimBundle(); - assertEq(manager.getSelectedExecutor(bundleId), executor1); - } - function test_proposeChugSplashBundle_revert_notProposerOrOwner() external { vm.expectRevert("ChugSplashManager: caller must be proposer or owner"); - vm.prank(executor1); + vm.prank(executor); manager.proposeChugSplashBundle(bundleRoot, bundleSize, configUri); } @@ -304,16 +299,16 @@ contract ChugSplashManager_Test is Test { } // approveChugSplashBundle: - // - reverts if the manager's balance minus the totalDebt is less than the owner bond amount + // - reverts if the manager's balance minus the debt is less than the owner bond amount function test_approveChugSplashBundle_revert_balance() external { assertEq(address(manager).balance, 0); - uint256 totalDebt = 1 gwei; - uint256 insufficientAmount = ownerBondAmount + totalDebt - 1; + uint256 debt = 1 gwei; + uint256 insufficientAmount = ownerBondAmount + debt - 1; stdstore .target(address(manager)) - .sig("totalDebt()") - .checked_write(totalDebt); + .sig("debt()") + .checked_write(debt); (bool success, ) = address(manager).call{ value: insufficientAmount }(new bytes(0)); assertTrue(success); @@ -376,51 +371,51 @@ contract ChugSplashManager_Test is Test { function test_executeChugSplashAction_revert_noActiveBundle() external { vm.expectRevert("ChugSplashManager: no bundle has been approved for execution"); + vm.prank(executor); manager.executeChugSplashAction( firstAction, actionIndexes[0], proofs[0] ); } function test_executeChugSplashAction_revert_alreadyExecuted() external { - helper_proposeThenApproveThenFundThenClaimBundle(); + helper_proposeThenApproveThenFundBundle(); helper_executeFirstAction(); vm.expectRevert("ChugSplashManager: action has already been executed"); + vm.prank(executor); manager.executeChugSplashAction(firstAction, actionIndexes[0], proofs[0]); } - function test_executeChugSplashAction_revert_wrongExecutor() external { - helper_proposeThenApproveThenFundThenClaimBundle(); - - vm.prank(executor2); - vm.expectRevert("ChugSplashManager: caller is not approved executor for active bundle ID"); + function test_executeChugSplashAction_revert_onlyExecutor() external { + vm.prank(owner); + vm.expectRevert("ChugSplashManager: caller is not an executor"); manager.executeChugSplashAction(firstAction, actionIndexes[0], proofs[0]); } function test_executeChugSplashAction_revert_invalidProof() external { - helper_proposeThenApproveThenFundThenClaimBundle(); + helper_proposeThenApproveThenFundBundle(); uint256 incorrectActionIndex = actionIndexes[0] + 1; - hoax(executor1); + hoax(executor); vm.expectRevert("ChugSplashManager: invalid bundle action proof"); manager.executeChugSplashAction(firstAction, incorrectActionIndex, proofs[0]); } function test_executeChugSplashAction_revert_noAdapter() external { - helper_proposeThenApproveThenFundThenClaimBundle(); + helper_proposeThenApproveThenFundBundle(); vm.mockCall( address(registry), abi.encodeWithSelector(registry.adapters.selector, bytes32(0)), abi.encode(address(0)) ); - hoax(executor1); + hoax(executor); vm.expectRevert("ChugSplashManager: proxy type has no adapter"); manager.executeChugSplashAction(firstAction, actionIndexes[0], proofs[0]); } function test_executeChugSplashAction_success_deployProxyAndImplementation() external { - helper_proposeThenApproveThenFundThenClaimBundle(); + helper_proposeThenApproveThenFundBundle(); address payable proxyAddress = manager.getDefaultProxyAddress(firstAction.target); assertEq(proxyAddress.code.length, 0); address implementationAddress = Create2.compute( @@ -429,8 +424,7 @@ contract ChugSplashManager_Test is Test { firstAction.data ); assertEq(implementationAddress.code.length, 0); - uint256 initialTotalDebt = manager.totalDebt(); - uint256 initialExecutorDebt = manager.debt(executor1); + uint256 initialDebt = manager.debt(); vm.expectCall( address(registry), @@ -458,11 +452,10 @@ contract ChugSplashManager_Test is Test { vm.expectEmit(true, true, true, true); emit ImplementationDeployed(firstAction.target, implementationAddress, bundleId, firstAction.target); vm.expectEmit(true, true, true, true); - emit ChugSplashActionExecuted(bundleId, proxyAddress, executor1, actionIndexes[0]); + emit ChugSplashActionExecuted(bundleId, proxyAddress, executor, actionIndexes[0]); helper_executeFirstAction(); - uint256 finalTotalDebt = manager.totalDebt(); - uint256 finalExecutorDebt = manager.debt(executor1); + uint256 finalDebt = manager.debt(); ChugSplashBundleState memory bundle = manager.bundles(bundleId); uint256 executionGasUsed = 760437; @@ -472,17 +465,15 @@ contract ChugSplashManager_Test is Test { assertGt(implementationAddress.code.length, 0); assertEq(bundle.actionsExecuted, 1); assertTrue(bundle.executions[actionIndexes[0]]); - bytes32 implemenetationSalt = keccak256(abi.encode(bundleId, bytes(firstAction.target))); - assertEq(manager.implementations(implemenetationSalt), implementationAddress); - assertGt(finalTotalDebt, estExecutorPayment + initialTotalDebt); - assertGt(finalExecutorDebt, estExecutorPayment + initialExecutorDebt); + bytes32 implementationSalt = keccak256(abi.encode(bundleId, bytes(firstAction.target))); + assertEq(manager.implementations(implementationSalt), implementationAddress); + assertGt(finalDebt, estExecutorPayment + initialDebt); } function test_executeChugSplashAction_success_setStorage() external { - helper_proposeThenApproveThenFundThenClaimBundle(); + helper_proposeThenApproveThenFundBundle(); helper_executeFirstAction(); - uint256 initialTotalDebt = manager.totalDebt(); - uint256 initialExecutorDebt = manager.debt(executor1); + uint256 initialDebt = manager.debt(); address payable proxyAddress = manager.getDefaultProxyAddress(firstAction.target); vm.expectCall( @@ -493,10 +484,9 @@ contract ChugSplashManager_Test is Test { ) ); vm.expectEmit(true, true, true, true); - emit ChugSplashActionExecuted(bundleId, proxyAddress, executor1, actionIndexes[1]); + emit ChugSplashActionExecuted(bundleId, proxyAddress, executor, actionIndexes[1]); helper_executeSecondAction(); - uint256 finalTotalDebt = manager.totalDebt(); - uint256 finalExecutorDebt = manager.debt(executor1); + uint256 finalDebt = manager.debt(); ChugSplashBundleState memory bundle = manager.bundles(bundleId); vm.prank(address(manager)); @@ -510,15 +500,13 @@ contract ChugSplashManager_Test is Test { assertTrue(bundle.executions[actionIndexes[1]]); assertEq(implementationAddress, address(0)); assertEq(storageValue, expectedStorageValue); - assertGt(finalTotalDebt, estExecutorPayment + initialTotalDebt); - assertGt(finalExecutorDebt, estExecutorPayment + initialExecutorDebt); + assertGt(finalDebt, estExecutorPayment + initialDebt); } function test_executeChugSplashAction_success_setImplementationToZeroAddress() external { - helper_proposeThenApproveThenFundThenClaimBundle(); + helper_proposeThenApproveThenFundBundle(); helper_executeFirstAction(); - uint256 initialTotalDebt = manager.totalDebt(); - uint256 initialExecutorDebt = manager.debt(executor1); + uint256 initialDebt = manager.debt(); vm.startPrank(address(manager)); address payable proxyAddress = manager.getDefaultProxyAddress(firstAction.target); @@ -531,8 +519,7 @@ contract ChugSplashManager_Test is Test { bytes32 newImplementationBytes = vm.load(proxyAddress, EIP1967_IMPLEMENTATION_KEY); (bytes32 storageKey, bytes32 expectedStorageValue) = abi.decode(secondAction.data, (bytes32, bytes32)); bytes32 storageValue = vm.load(proxyAddress, storageKey); - uint256 finalTotalDebt = manager.totalDebt(); - uint256 finalExecutorDebt = manager.debt(executor1); + uint256 finalDebt = manager.debt(); uint256 executionGasUsed = 72301; uint256 estExecutorPayment = tx.gasprice * executionGasUsed * (100 + executorPaymentPercentage) / 100; @@ -540,42 +527,39 @@ contract ChugSplashManager_Test is Test { assertTrue(bundle.executions[actionIndexes[1]]); assertEq(newImplementationBytes, bytes32(0)); assertEq(storageValue, expectedStorageValue); - assertGt(finalTotalDebt, estExecutorPayment + initialTotalDebt); - assertGt(finalExecutorDebt, estExecutorPayment + initialExecutorDebt); + assertGt(finalDebt, estExecutorPayment + initialDebt); } function test_completeChugSplashBundle_revert_noActiveBundle() external { vm.expectRevert("ChugSplashManager: no bundle has been approved for execution"); - helper_completeBundle(executor1); + helper_completeBundle(executor); } - function test_completeChugSplashBundle_revert_wrongExecutor() external { - helper_proposeThenApproveThenFundThenClaimBundle(); - vm.expectRevert("ChugSplashManager: caller is not approved executor for active bundle ID"); - helper_completeBundle(executor2); + function test_completeChugSplashBundle_revert_onlyExecutor() external { + vm.expectRevert("ChugSplashManager: caller is not an executor"); + helper_completeBundle(owner); } function test_completeChugSplashBundle_revert_invalidProof() external { - helper_proposeThenApproveThenFundThenClaimBundle(); + helper_proposeThenApproveThenFundBundle(); setImplementationProofArray[0][0] = bytes32(0); vm.expectRevert("ChugSplashManager: invalid bundle action proof"); - helper_completeBundle(executor1); + helper_completeBundle(executor); } function test_completeChugSplashBundle_revert_incompleteBundle() external { - helper_proposeThenApproveThenFundThenClaimBundle(); + helper_proposeThenApproveThenFundBundle(); helper_executeFirstAction(); vm.expectRevert("ChugSplashManager: bundle was not completed"); - helper_completeBundle(executor1); + helper_completeBundle(executor); } function test_completeChugSplashBundle_success() external { - helper_proposeThenApproveThenFundThenClaimBundle(); + helper_proposeThenApproveThenFundBundle(); helper_executeMultipleActions(); ChugSplashBundleState memory prevBundle = manager.bundles(bundleId); address payable proxyAddress = manager.getDefaultProxyAddress(firstAction.target); - uint256 initialTotalDebt = manager.totalDebt(); - uint256 initialExecutorDebt = manager.debt(executor1); + uint256 initialDebt = manager.debt(); uint256 actionIndex = setImplementationActionIndexArray[0]; uint256 numActions = actionIndex + 1; @@ -587,7 +571,7 @@ contract ChugSplashManager_Test is Test { ) ); vm.expectEmit(true, true, true, true); - emit ChugSplashActionExecuted(bundleId, proxyAddress, executor1, actionIndex); + emit ChugSplashActionExecuted(bundleId, proxyAddress, executor, actionIndex); vm.expectCall( address(registry), abi.encodeCall( @@ -596,11 +580,10 @@ contract ChugSplashManager_Test is Test { ) ); vm.expectEmit(true, true, true, true); - emit ChugSplashBundleCompleted(bundleId, executor1, numActions); - helper_completeBundle(executor1); + emit ChugSplashBundleCompleted(bundleId, executor, numActions); + helper_completeBundle(executor); - uint256 finalTotalDebt = manager.totalDebt(); - uint256 finalExecutorDebt = manager.debt(executor1); + uint256 finalDebt = manager.debt(); bytes32 implementationSalt = keccak256(abi.encode(bundleId, bytes(firstAction.target))); address expectedImplementation = manager.implementations(implementationSalt); ChugSplashBundleState memory bundle = manager.bundles(bundleId); @@ -614,9 +597,7 @@ contract ChugSplashManager_Test is Test { assertEq(implementation, expectedImplementation); assertEq(uint8(bundle.status), uint8(ChugSplashBundleStatus.COMPLETED)); assertEq(manager.activeBundleId(), bytes32(0)); - assertGt(finalTotalDebt, estExecutorPayment + initialTotalDebt); - assertGt(finalExecutorDebt, estExecutorPayment + initialExecutorDebt); - assertEq(finalTotalDebt, finalExecutorDebt); + assertGt(finalDebt, estExecutorPayment + initialDebt); } // cancelActiveChugSplashBundle: @@ -636,12 +617,11 @@ contract ChugSplashManager_Test is Test { } function test_cancelActiveChugSplashBundle_success_withinExecutionLockTime() external { - helper_proposeThenApproveThenFundThenClaimBundle(); + helper_proposeThenApproveThenFundBundle(); helper_executeFirstAction(); uint256 timeClaimed = manager.bundles(bundleId).timeClaimed; uint256 actionsExecuted = manager.bundles(bundleId).actionsExecuted; - uint256 initialTotalDebt = manager.totalDebt(); - uint256 initialExecutorDebt = manager.debt(executor1); + uint256 initialDebt = manager.debt(); vm.warp(executionLockTime + timeClaimed); vm.expectCall( @@ -656,26 +636,24 @@ contract ChugSplashManager_Test is Test { vm.prank(owner); manager.cancelActiveChugSplashBundle(); - assertEq(manager.debt(executor1), initialExecutorDebt + ownerBondAmount + executorBondAmount); - assertEq(manager.totalDebt(), initialTotalDebt + ownerBondAmount); + assertEq(manager.debt(), initialDebt + ownerBondAmount); assertEq(manager.activeBundleId(), bytes32(0)); assertEq(uint8(manager.bundles(bundleId).status), uint8(ChugSplashBundleStatus.CANCELLED)); } // cancelActiveChugSplashBundle: // - if bundle is NOT cancelled within the `executionLockTime` window and there is an executor: - // - decreases the `totalDebt` by `executorBondAmount` + // - does not decrease `debt` // - removes active bundle id // - sets bundle status to `CANCELLED` // - emits ChugSplashBundleCancelled // - calls registry.announce with ChugSplashBundleCancelled function test_cancelActiveChugSplashBundle_success_afterExecutionLockTime() external { - helper_proposeThenApproveThenFundThenClaimBundle(); + helper_proposeThenApproveThenFundBundle(); helper_executeFirstAction(); uint256 timeClaimed = manager.bundles(bundleId).timeClaimed; uint256 actionsExecuted = manager.bundles(bundleId).actionsExecuted; - uint256 initialTotalDebt = manager.totalDebt(); - uint256 initialExecutorDebt = manager.debt(executor1); + uint256 initialDebt = manager.debt(); vm.warp(executionLockTime + timeClaimed + 1); vm.expectCall( @@ -690,132 +668,35 @@ contract ChugSplashManager_Test is Test { vm.prank(owner); manager.cancelActiveChugSplashBundle(); - assertEq(manager.debt(executor1), initialExecutorDebt); - assertEq(manager.totalDebt(), initialTotalDebt - executorBondAmount); + assertEq(manager.debt(), initialDebt); assertEq(manager.activeBundleId(), bytes32(0)); assertEq(uint8(manager.bundles(bundleId).status), uint8(ChugSplashBundleStatus.CANCELLED)); } - // cancelActiveChugSplashBundle: - // - if an executor has not claimed the bundle: - // - no debt is incremented - // - removes active bundle id - // - sets bundle status to `CANCELLED` - // - emits ChugSplashBundleCancelled - // - calls registry.announce with ChugSplashBundleCancelled - function test_cancelActiveChugSplashBundle_success_noExecutor() external { - helper_proposeThenApproveThenFundBundle(); - uint256 initialTotalDebt = manager.totalDebt(); - - vm.expectCall( - address(registry), - abi.encodeCall( - ChugSplashRegistry.announce, - ("ChugSplashBundleCancelled") - ) - ); - vm.expectEmit(true, true, true, true); - emit ChugSplashBundleCancelled(bundleId, owner, 0); - vm.startPrank(owner); - manager.cancelActiveChugSplashBundle(); - manager.withdrawOwnerETH(); - - assertEq(manager.totalDebt(), initialTotalDebt); - assertEq(manager.debt(address(0)), 0); - assertEq(manager.activeBundleId(), bytes32(0)); - assertEq(uint8(manager.bundles(bundleId).status), uint8(ChugSplashBundleStatus.CANCELLED)); - } - - // claimBundle: - // - reverts if there is no active bundle - function test_claimBundle_revert_noActiveBundle() external { - vm.expectRevert('ChugSplashManager: no bundle is currently active'); - manager.claimBundle(); - } - - // claimBundle: - // - reverts if callvalue is less than the `executorBondAmount` - function test_claimBundle_revert_insufficientBond() external { - helper_proposeThenApproveBundle(); - vm.expectRevert('ChugSplashManager: incorrect executor bond amount'); - manager.claimBundle{ value: executorBondAmount - 1}(); - } - - // claimBundle: - // - reverts if bundle is currently claimed by another executor - function test_claimBundle_revert_alreadyClaimed() external { - helper_proposeThenApproveBundle(); - helper_claimBundle(executor1); - - vm.warp(initialTimestamp + executionLockTime); - vm.expectRevert("ChugSplashManager: bundle is currently claimed by an executor"); - helper_claimBundle(executor2); - } - - // claimBundle: - // - see helper_claimBundle - // - if there was no previous executor: - // - increases `totalDebt` by `executorBondAmount` - function test_claimBundle_success_noPreviousExecutor() external { - helper_proposeThenApproveBundle(); - - vm.expectCall( - address(registry), - abi.encodeCall( - ChugSplashRegistry.announce, - ("ChugSplashBundleClaimed") - ) - ); - vm.expectEmit(true, true, true, true); - emit ChugSplashBundleClaimed(bundleId, executor1); - helper_claimBundle(executor1); - - ChugSplashBundleState memory bundle = manager.bundles(bundleId); - - assertEq(bundle.timeClaimed, block.timestamp); - assertEq(bundle.selectedExecutor, executor1); - assertEq(manager.totalDebt(), executorBondAmount); + function test_claimExecutorPayment_revert_onlyExecutor() external { + vm.expectRevert("ChugSplashManager: caller is not an executor"); + vm.prank(owner); + manager.claimExecutorPayment(); } - // claimBundle: - // - see helper_claimBundle - // - if there was a previous executor: - // - `totalDebt` remains the same - function test_claimBundle_success_withPreviousExecutor() external { - helper_proposeThenApproveBundle(); - helper_claimBundle(executor1); - uint256 initialTotalDebt = manager.totalDebt(); - uint256 secondClaimedBundleTimestamp = initialTimestamp + executionLockTime + 1; - vm.warp(secondClaimedBundleTimestamp); - - vm.expectCall( - address(registry), - abi.encodeCall( - ChugSplashRegistry.announce, - ("ChugSplashBundleClaimed") - ) - ); - vm.expectEmit(true, true, true, true); - emit ChugSplashBundleClaimed(bundleId, executor2); - helper_claimBundle(executor2); - - ChugSplashBundleState memory bundle = manager.bundles(bundleId); - - assertEq(bundle.timeClaimed, secondClaimedBundleTimestamp); - assertEq(bundle.selectedExecutor, executor2); - assertEq(manager.totalDebt(), initialTotalDebt); + function test_claimExecutorPayment_revert_noDebt() external { + vm.expectRevert("ChugSplashManager: no debt to withdraw"); + vm.prank(executor); + manager.claimExecutorPayment(); } // claimExecutorPayment: - // - decreases `debt` and `totalDebt` by the withdrawn amount + // - sets debt to 0 + // - increases executor's balance by `debt` + // - decreases ChugSplashManager balance by `debt` // - emits ExecutorPaymentClaimed // - calls registry.announce with ExecutorPaymentClaimed function test_claimExecutorPayment_success() external { - helper_proposeThenApproveThenFundThenClaimBundle(); + helper_proposeThenApproveThenFundBundle(); helper_executeFirstAction(); - uint256 executorDebt = manager.debt(executor1); - uint256 initialTotalDebt = manager.totalDebt(); - uint256 initialExecutorBalance = address(executor1).balance; + uint256 debt = manager.debt(); + uint256 initialExecutorBalance = address(executor).balance; + uint256 initialManagerBalance = address(manager).balance; vm.expectCall( address(registry), @@ -825,13 +706,13 @@ contract ChugSplashManager_Test is Test { ) ); vm.expectEmit(true, true, true, true); - emit ExecutorPaymentClaimed(executor1, executorDebt); - vm.prank(executor1); + emit ExecutorPaymentClaimed(executor, debt); + vm.prank(executor); manager.claimExecutorPayment(); - assertEq(address(executor1).balance, executorDebt + initialExecutorBalance); - assertEq(manager.debt(executor1), 0); - assertEq(manager.totalDebt(), initialTotalDebt - executorDebt); + assertEq(manager.debt(), 0); + assertEq(address(manager).balance, initialManagerBalance - debt); + assertEq(address(executor).balance, debt + initialExecutorBalance); } // transferProxyOwnership: @@ -857,9 +738,9 @@ contract ChugSplashManager_Test is Test { // - emits ProxyOwnershipTransferred // - calls registry.announce with ProxyOwnershipTransferred function test_transferProxyOwnership_success() external { - helper_proposeThenApproveThenFundThenClaimBundle(); + helper_proposeThenApproveThenFundBundle(); helper_executeMultipleActions(); - helper_completeBundle(executor1); + helper_completeBundle(executor); address payable proxyAddress = manager.getDefaultProxyAddress(firstAction.target); vm.prank(address(manager)); assertEq(Proxy(proxyAddress).admin(), address(manager)); @@ -872,12 +753,12 @@ contract ChugSplashManager_Test is Test { ) ); vm.expectEmit(true, true, true, true); - emit ProxyOwnershipTransferred(firstAction.target, proxyAddress, bytes32(0), executor1, firstAction.target); + emit ProxyOwnershipTransferred(firstAction.target, proxyAddress, bytes32(0), executor, firstAction.target); vm.prank(owner); - manager.transferProxyOwnership(firstAction.target, executor1); + manager.transferProxyOwnership(firstAction.target, executor); - vm.prank(executor1); - assertEq(Proxy(proxyAddress).admin(), executor1); + vm.prank(executor); + assertEq(Proxy(proxyAddress).admin(), executor); } function test_addProposer_revert_nonOwner() external { @@ -963,13 +844,13 @@ contract ChugSplashManager_Test is Test { function test_withdrawOwnerETH_success() external { uint256 managerBalance = 1 ether; - uint256 totalDebt = 1 gwei; - uint256 amountWithdrawn = managerBalance - totalDebt; + uint256 debt = 1 gwei; + uint256 amountWithdrawn = managerBalance - debt; helper_fundChugSplashManager(managerBalance); stdstore .target(address(manager)) - .sig("totalDebt()") - .checked_write(totalDebt); + .sig("debt()") + .checked_write(debt); uint256 prevOwnerBalance = address(owner).balance; vm.expectEmit(true, true, true, true); @@ -1016,7 +897,7 @@ contract ChugSplashManager_Test is Test { } function helper_executeMultipleActions() internal { - startHoax(executor1); + startHoax(executor); manager.executeMultipleActions(actions, actionIndexes, proofs); vm.stopPrank(); } @@ -1027,7 +908,7 @@ contract ChugSplashManager_Test is Test { } function helper_executeSecondAction() internal { - hoax(executor1); + hoax(executor); manager.executeChugSplashAction(secondAction, actionIndexes[1], proofs[1]); } @@ -1036,23 +917,13 @@ contract ChugSplashManager_Test is Test { helper_fundChugSplashManager(bundleExecutionCost); } - function helper_proposeThenApproveThenFundThenClaimBundle() internal { - helper_proposeThenApproveThenFundBundle(); - helper_claimBundle(executor1); - } - function helper_fundChugSplashManager(uint256 _amount) internal { (bool success, ) = address(manager).call{ value: _amount }(new bytes(0)); assertTrue(success); } function helper_executeFirstAction() internal { - hoax(executor1); + hoax(executor); manager.executeChugSplashAction(firstAction, actionIndexes[0], proofs[0]); } - - function helper_claimBundle(address _executor) internal { - hoax(_executor); - manager.claimBundle{ value: executorBondAmount }(); - } } diff --git a/packages/contracts/test/ChugSplashRegistry.t.sol b/packages/contracts/test/ChugSplashRegistry.t.sol index 735a8a7dc..6ac6f4827 100644 --- a/packages/contracts/test/ChugSplashRegistry.t.sol +++ b/packages/contracts/test/ChugSplashRegistry.t.sol @@ -30,14 +30,19 @@ contract ChugSplashRegistry_Test is Test { event ProxyTypeAdded(bytes32 proxyType, address adapter); + event ExecutorAdded(address indexed executor); + + event ExecutorRemoved(address indexed executor); + address owner = address(128); address adapter = address(256); + address executor = address(512); + address nonOwner = address(1024); bytes32 proxyType = bytes32(hex"1337"); bytes32 salt = bytes32(hex"11"); address dummyRegistryProxyAddress = address(1); string projectName = 'TestProject'; uint256 ownerBondAmount = 10e8 gwei; // 0.1 ETH - uint256 executorBondAmount = 1 ether; uint256 executionLockTime = 15 minutes; uint256 executorPaymentPercentage = 20; @@ -53,7 +58,6 @@ contract ChugSplashRegistry_Test is Test { projectName, owner, address(proxyUpdater), - executorBondAmount, executionLockTime, ownerBondAmount, executorPaymentPercentage @@ -62,20 +66,22 @@ contract ChugSplashRegistry_Test is Test { registry = new ChugSplashRegistry{ salt: salt }( address(proxyUpdater), ownerBondAmount, - executorBondAmount, executionLockTime, executorPaymentPercentage, address(manager) ); + + registry.initialize(owner, new address[](0)); } - function test_constructor_success() external { + function test_initialize_success() external { assertEq(address(registry.proxyUpdater()), address(proxyUpdater)); - assertEq(registry.executorBondAmount(), executorBondAmount); assertEq(registry.executionLockTime(), executionLockTime); assertEq(registry.ownerBondAmount(), ownerBondAmount); assertEq(registry.executorPaymentPercentage(), executorPaymentPercentage); assertEq(registry.managerImplementation(), address(manager)); + + assertEq(registry.owner(), owner); } function test_register_revert_nameAlreadyRegistered() external { @@ -151,6 +157,55 @@ contract ChugSplashRegistry_Test is Test { registry.announceWithData(announcedEvent, data); } + function test_addExecutor_revert_nonOwner() external { + vm.prank(nonOwner); + vm.expectRevert('Ownable: caller is not the owner'); + registry.addExecutor(executor); + } + + function test_addExecutor_revert_alreadyAdded() external { + vm.startPrank(owner); + registry.addExecutor(executor); + vm.expectRevert('ChugSplashRegistry: executor already added'); + registry.addExecutor(executor); + } + + function test_addExecutor_success() external { + assertFalse(registry.executors(executor)); + + vm.expectEmit(true, true, true, true); + emit ExecutorAdded(executor); + vm.prank(owner); + registry.addExecutor(executor); + + assertTrue(registry.executors(executor)); + } + + function test_removeExecutor_revert_nonOwner() external { + vm.prank(nonOwner); + vm.expectRevert('Ownable: caller is not the owner'); + registry.removeExecutor(executor); + } + + function test_removeExecutor_revert_alreadyRemoved() external { + vm.prank(owner); + vm.expectRevert('ChugSplashRegistry: executor already removed'); + registry.removeExecutor(executor); + } + + function test_removeExecutor_success() external { + vm.startPrank(owner); + registry.addExecutor(executor); + + assertTrue(registry.executors(executor)); + + vm.expectEmit(true, true, true, true); + emit ExecutorRemoved(executor); + registry.removeExecutor(executor); + + assertFalse(registry.executors(executor)); + } + function test_addProxyType_revert_existingAdapter() external { registry.addProxyType(proxyType, adapter); vm.expectRevert("ChugSplashRegistry: proxy type has an existing adapter"); diff --git a/packages/contracts/test/integration-tests.t.sol b/packages/contracts/test/integration-tests.t.sol index 827ec37ea..e48a13a6d 100644 --- a/packages/contracts/test/integration-tests.t.sol +++ b/packages/contracts/test/integration-tests.t.sol @@ -13,10 +13,10 @@ import { Create2 } from "../contracts/libraries/Create2.sol"; contract Integration_Tests is Test { address owner = address(128); + address executor = address(256); bytes32 salt = bytes32(hex"11"); string projectName = 'TestProject'; uint256 ownerBondAmount = 10e8 gwei; // 0.1 ETH - uint256 executorBondAmount = 1 ether; uint256 executionLockTime = 15 minutes; uint256 executorPaymentPercentage = 20; ChugSplashBootLoader bootloader; @@ -34,7 +34,6 @@ contract Integration_Tests is Test { bootloader.initialize( owner, - executorBondAmount, executionLockTime, ownerBondAmount, executorPaymentPercentage, diff --git a/packages/core/src/actions/execute.ts b/packages/core/src/actions/execute.ts index f7edba79e..45bfb1ece 100644 --- a/packages/core/src/actions/execute.ts +++ b/packages/core/src/actions/execute.ts @@ -1,4 +1,3 @@ -import { EXECUTOR_BOND_AMOUNT } from '@chugsplash/contracts' import { ethers } from 'ethers' import { Logger } from '@eth-optimism/common-ts' @@ -34,8 +33,6 @@ export const executeTask = async (args: { logger.info(`[ChugSplash]: preparing to execute the project...`) - const executorAddress = await executor.getAddress() - if ( bundleState.status !== ChugSplashBundleStatus.APPROVED && bundleState.status !== ChugSplashBundleStatus.COMPLETED @@ -48,22 +45,6 @@ export const executeTask = async (args: { if (bundleState.status === ChugSplashBundleStatus.COMPLETED) { logger.info(`[ChugSplash]: already executed: ${projectName}`) } else if (bundleState.status === ChugSplashBundleStatus.APPROVED) { - if (bundleState.selectedExecutor === ethers.constants.AddressZero) { - logger.info( - `[ChugSplash]: claiming the bundle for project: ${projectName}` - ) - await ( - await chugSplashManager.claimBundle( - await getGasPriceOverrides(executor.provider, { - value: EXECUTOR_BOND_AMOUNT, - }) - ) - ).wait() - logger.info(`[ChugSplash]: claimed the bundle`) - } else if (bundleState.selectedExecutor !== executorAddress) { - throw new Error(`another executor has already claimed the bundle`) - } - // We execute all actions in batches to reduce the total number of transactions and reduce the // cost of a deployment in general. Approaching the maximum block gas limit can cause // transactions to be executed slowly as a result of the algorithms that miners use to select diff --git a/packages/core/src/fund.ts b/packages/core/src/fund.ts index 139bff94b..760c69924 100644 --- a/packages/core/src/fund.ts +++ b/packages/core/src/fund.ts @@ -28,8 +28,8 @@ export const availableFundsForExecution = async ( const ChugSplashManager = getChugSplashManagerReadOnly(provider, projectName) const managerBalance = await provider.getBalance(ChugSplashManager.address) - const totalDebt = await ChugSplashManager.totalDebt() - return managerBalance.sub(totalDebt).sub(OWNER_BOND_AMOUNT) + const debt = await ChugSplashManager.debt() + return managerBalance.sub(debt).sub(OWNER_BOND_AMOUNT) } export const getOwnerWithdrawableAmount = async ( @@ -45,8 +45,8 @@ export const getOwnerWithdrawableAmount = async ( } const managerBalance = await provider.getBalance(ChugSplashManager.address) - const totalDebt = await ChugSplashManager.totalDebt() - return managerBalance.sub(totalDebt) + const debt = await ChugSplashManager.debt() + return managerBalance.sub(debt) } export const estimateExecutionGas = async ( diff --git a/packages/core/src/languages/solidity/predeploys.ts b/packages/core/src/languages/solidity/predeploys.ts index ca58aadfb..e813d8b78 100644 --- a/packages/core/src/languages/solidity/predeploys.ts +++ b/packages/core/src/languages/solidity/predeploys.ts @@ -3,7 +3,6 @@ import assert from 'assert' import { Contract, ethers } from 'ethers' import { OWNER_BOND_AMOUNT, - EXECUTOR_BOND_AMOUNT, EXECUTION_LOCK_TIME, EXECUTOR_PAYMENT_PERCENTAGE, CHUGSPLASH_BOOTLOADER_ADDRESS, @@ -24,6 +23,7 @@ import { OWNER_MULTISIG_ADDRESS, PROXY_INITIALIZER_ADDRESS, CHUGSPLASH_SALT, + ChugSplashRegistryABI, } from '@chugsplash/contracts' import { Logger } from '@eth-optimism/common-ts' import { sleep } from '@eth-optimism/core-utils' @@ -40,6 +40,7 @@ import { export const initializeChugSplash = async ( provider: ethers.providers.JsonRpcProvider, deployer: ethers.Signer, + executorAddress: string, logger?: Logger ): Promise => { logger?.info('[ChugSplash]: deploying ChugSplashManager...') @@ -83,7 +84,6 @@ export const initializeChugSplash = async ( await ( await ChugSplashBootLoader.initialize( OWNER_MULTISIG_ADDRESS, - EXECUTOR_BOND_AMOUNT, EXECUTION_LOCK_TIME, OWNER_BOND_AMOUNT, EXECUTOR_PAYMENT_PERCENTAGE, @@ -141,29 +141,50 @@ export const initializeChugSplash = async ( logger?.info('[ChugSplash]: initializing ChugSplashRegistry proxy...') - const ChugSplashRegistryProxy = new Contract( + // Define two contract objects for the ChugSplashRegistry's proxy: one with the Proxy ABI, and the + // other with the ChugSplashRegistry ABI. + const Proxy__ChugSplashRegistry = new Contract( CHUGSPLASH_REGISTRY_PROXY_ADDRESS, ProxyABI, provider ) + const ChugSplashRegistryProxy = new Contract( + CHUGSPLASH_REGISTRY_PROXY_ADDRESS, + ChugSplashRegistryABI, + deployer + ) // Check if the ChugSplashRegistry proxy's owner is the ProxyInitializer. This will only be true // when the ChugSplashRegistry's proxy hasn't been initialized yet. if ( - (await getProxyAdmin(ChugSplashRegistryProxy)) === PROXY_INITIALIZER_ADDRESS + (await getProxyAdmin(Proxy__ChugSplashRegistry)) === + PROXY_INITIALIZER_ADDRESS ) { + logger?.info('[ChugSplash]: initializing ChugSplashRegistry...') + // Initialize the ChugSplashRegistry's proxy. This sets the ChugSplashRegistry proxy's - // implementation and transfers ownership of the proxy to the multisig owner. + // implementation, calls the ChugSplashRegistry's initializer, and transfers ownership of the proxy to the + // multisig owner. await ( await ProxyInitializer.initialize( CHUGSPLASH_REGISTRY_ADDRESS, + ChugSplashRegistryProxy.interface.encodeFunctionData('initialize', [ + await deployer.getAddress(), + [executorAddress], + ]), await getGasPriceOverrides(provider) ) ).wait() + assert( + (await ChugSplashRegistryProxy.executors(executorAddress)) === true, + 'Failed to add executor to ChugSplashRegistry' + ) + // Make sure ownership of the ChugSplashRegistry's proxy has been transferred. assert( - (await getProxyAdmin(ChugSplashRegistryProxy)) === OWNER_MULTISIG_ADDRESS, + (await getProxyAdmin(Proxy__ChugSplashRegistry)) === + OWNER_MULTISIG_ADDRESS, 'ChugSplashRegistry proxy has incorrect owner' ) @@ -176,7 +197,20 @@ export const initializeChugSplash = async ( 'ChugSplashRegistry proxy has incorrect implememtation' ) - logger?.info('[ChugSplash]: ChugSplashRegistry proxy initialized') + // Transfer ownership of the ChugSplashRegistry's proxy from the deployer to the multisig. + await ( + await ChugSplashRegistryProxy.transferOwnership( + OWNER_MULTISIG_ADDRESS, + await getGasPriceOverrides(provider) + ) + ).wait() + + assert( + (await ChugSplashRegistryProxy.owner()) === OWNER_MULTISIG_ADDRESS, + 'Failed to set owner of ChugSplashRegistry via its proxy' + ) + + logger?.info('[ChugSplash]: ChugSplashRegistry initialized') } else { logger?.info( '[ChugSplash]: ChugSplashRegistry proxy was already initialized' diff --git a/packages/core/src/tasks/index.ts b/packages/core/src/tasks/index.ts index 1c23146a2..065afba84 100644 --- a/packages/core/src/tasks/index.ts +++ b/packages/core/src/tasks/index.ts @@ -7,7 +7,7 @@ import ora from 'ora' import { getChainId, remove0x } from '@eth-optimism/core-utils' import Hash from 'ipfs-only-hash' import { create } from 'ipfs-http-client' -import { ProxyABI } from '@chugsplash/contracts' +import { EXECUTOR, ProxyABI } from '@chugsplash/contracts' import { assertValidUpgrade, @@ -76,7 +76,7 @@ export const chugsplashRegisterAbstractTask = async ( stream: NodeJS.WritableStream = process.stderr ) => { const spinner = ora({ isSilent: silent, stream }) - await initializeChugSplash(provider, signer) + await initializeChugSplash(provider, signer, EXECUTOR) for (const parsedConfig of configs) { spinner.start(`Registering ${parsedConfig.options.projectName}...`) @@ -130,7 +130,7 @@ export const chugsplashProposeAbstractTask = async ( spinner.start('Booting up ChugSplash...') } - await initializeChugSplash(provider, signer) + await initializeChugSplash(provider, signer, EXECUTOR) await assertValidUpgrade( provider, diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts index c51a6db32..fb0d69819 100644 --- a/packages/core/src/utils.ts +++ b/packages/core/src/utils.ts @@ -367,7 +367,7 @@ export const claimExecutorPayment = async ( executor: Wallet, ChugSplashManager: Contract ) => { - const executorDebt = await ChugSplashManager.debt(await executor.getAddress()) + const executorDebt = await ChugSplashManager.debt() if (executorDebt.gt(0)) { await ( await ChugSplashManager.claimExecutorPayment( diff --git a/packages/executor/src/index.ts b/packages/executor/src/index.ts index 1e29a8cc6..c02a00e13 100644 --- a/packages/executor/src/index.ts +++ b/packages/executor/src/index.ts @@ -117,6 +117,7 @@ export class ChugSplashExecutor extends BaseServiceV2< await initializeChugSplash( this.state.provider, this.state.wallet, + this.state.wallet.address, this.logger ) diff --git a/packages/executor/src/utils/etherscan.ts b/packages/executor/src/utils/etherscan.ts index 8c4788249..9a349f197 100644 --- a/packages/executor/src/utils/etherscan.ts +++ b/packages/executor/src/utils/etherscan.ts @@ -214,7 +214,7 @@ export const verifyChugSplash = async ( etherscanApiKey, minimumCompilerInput, buildInfo.solcVersion, - CHUGSPLASH_CONSTRUCTOR_ARGS[sourceName] + CHUGSPLASH_CONSTRUCTOR_ARGS[address] ) } diff --git a/packages/plugins/src/hardhat/tasks.ts b/packages/plugins/src/hardhat/tasks.ts index 24cb566d1..3a7d72b5d 100644 --- a/packages/plugins/src/hardhat/tasks.ts +++ b/packages/plugins/src/hardhat/tasks.ts @@ -41,7 +41,7 @@ import { bundleRemote, readUserChugSplashConfig, } from '@chugsplash/core' -import { ChugSplashManagerABI } from '@chugsplash/contracts' +import { ChugSplashManagerABI, EXECUTOR } from '@chugsplash/contracts' import ora from 'ora' import * as dotenv from 'dotenv' import { HardhatRuntimeEnvironment } from 'hardhat/types' @@ -717,10 +717,9 @@ task(TASK_NODE) const spinner = ora({ isSilent: hide }) spinner.start('Booting up ChugSplash...') - await initializeChugSplash( - hre.ethers.provider, - hre.ethers.provider.getSigner() - ) + const signer = hre.ethers.provider.getSigner() + const signerAddress = await signer.getAddress() + await initializeChugSplash(hre.ethers.provider, signer, signerAddress) spinner.succeed('ChugSplash has been initialized.') @@ -753,6 +752,8 @@ task(TASK_TEST) ) => { const { show, noCompile } = args const chainId = await getChainId(hre.ethers.provider) + const signer = hre.ethers.provider.getSigner() + const executor = chainId === 31337 ? await signer.getAddress() : EXECUTOR if (chainId === 31337) { try { const snapshotIdPath = path.join( @@ -769,10 +770,7 @@ task(TASK_TEST) throw new Error('Snapshot failed to be reverted.') } } catch { - await initializeChugSplash( - hre.ethers.provider, - hre.ethers.provider.getSigner() - ) + await initializeChugSplash(hre.ethers.provider, signer, executor) if (!noCompile) { await hre.run(TASK_COMPILE, { quiet: true, @@ -813,12 +811,13 @@ task(TASK_RUN) ) => { const { deployAll, noCompile } = args if (deployAll) { + const signer = hre.ethers.provider.getSigner() const chainId = await getChainId(hre.ethers.provider) + const confirm = chainId === 31337 ? true : args.confirm - await initializeChugSplash( - hre.ethers.provider, - hre.ethers.provider.getSigner() - ) + const executor = + chainId === 31337 ? await signer.getAddress() : EXECUTOR + await initializeChugSplash(hre.ethers.provider, signer, executor) if (!noCompile) { await hre.run(TASK_COMPILE, { quiet: true,