From fb1168f2c2329f718407544fcf411a9239ae4740 Mon Sep 17 00:00:00 2001 From: Sam Goldman Date: Sat, 19 Nov 2022 16:25:17 -0500 Subject: [PATCH] fix(ex): make executor most robust to errors and cancelled bundles --- .changeset/selfish-islands-shop.md | 7 + packages/core/src/fund.ts | 85 +++++++++ packages/core/src/index.ts | 1 + packages/core/src/utils.ts | 10 + packages/executor/src/index.ts | 191 ++++++++++++-------- packages/plugins/src/hardhat/deployments.ts | 8 +- packages/plugins/src/hardhat/execution.ts | 2 +- packages/plugins/src/hardhat/fund.ts | 59 ------ packages/plugins/src/hardhat/tasks.ts | 32 ++-- 9 files changed, 246 insertions(+), 149 deletions(-) create mode 100644 .changeset/selfish-islands-shop.md create mode 100644 packages/core/src/fund.ts delete mode 100644 packages/plugins/src/hardhat/fund.ts diff --git a/.changeset/selfish-islands-shop.md b/.changeset/selfish-islands-shop.md new file mode 100644 index 000000000..ce2c91af2 --- /dev/null +++ b/.changeset/selfish-islands-shop.md @@ -0,0 +1,7 @@ +--- +'@chugsplash/core': patch +'@chugsplash/executor': patch +'@chugsplash/plugins': patch +--- + +Make executor most robust to errors and cancelled bundles. Ensure that executor receives payment. diff --git a/packages/core/src/fund.ts b/packages/core/src/fund.ts new file mode 100644 index 000000000..8634d416b --- /dev/null +++ b/packages/core/src/fund.ts @@ -0,0 +1,85 @@ +import { ChugSplashManagerABI } from '@chugsplash/contracts' +import { ethers } from 'ethers' + +import { getChugSplashManagerProxyAddress } from './utils' +import { ChugSplashConfig } from './config/types' + +/** + * Gets the amount ETH in the ChugSplashManager that can be used to execute a deployment. + * This equals the ChugSplashManager's balance minus the total debt owed to executors. + */ +export const getOwnerBalanceInChugSplashManager = async ( + provider: ethers.providers.JsonRpcProvider, + projectName: string +): Promise => { + const ChugSplashManager = new ethers.Contract( + getChugSplashManagerProxyAddress(projectName), + ChugSplashManagerABI, + provider + ) + + const managerBalance = await provider.getBalance(ChugSplashManager.address) + const totalDebt = await ChugSplashManager.totalDebt() + return managerBalance.sub(totalDebt) +} + +/** + * Gets the minimum amount that must be sent to the ChugSplashManager in order to execute the + * ChugSplash config. If this function returns zero, then there is already a sufficient amount of + * funds. + * + * @param provider JSON RPC provider. + * @param parsedConfig Parsed ChugSplash config. + * @returns The minimum amount to send to the ChugSplashManager in order to execute the config + * (denominated in wei). + */ +export const getExecutionAmountToSend = async ( + provider: ethers.providers.JsonRpcProvider, + parsedConfig: ChugSplashConfig +): Promise => { + const totalExecutionAmount = await simulateExecution(provider, parsedConfig) + const availableExecutionAmount = await getOwnerBalanceInChugSplashManager( + provider, + parsedConfig.options.projectName + ) + const executionAmount = totalExecutionAmount.sub(availableExecutionAmount) + return executionAmount.gt(0) ? executionAmount : ethers.BigNumber.from(0) +} + +export const simulateExecution = async ( + provider: ethers.providers.JsonRpcProvider, + parsedConfig: ChugSplashConfig +) => { + provider + parsedConfig + + // TODO + return ethers.utils.parseEther('0.25') +} + +/** + * Returns the amount to send to the ChugSplashManager to execute a bundle, plus a buffer in case + * the gas price increases during execution. If this returns zero, there is already a sufficient + * amount of funds in the ChugSplashManager. + * + * @param provider JSON RPC provider. + * @param parsedConfig Parsed ChugSplash config. + * @returns The amount required to fund a bundle, plus a buffer. Denominated in wei. + */ +export const getExecutionAmountToSendPlusBuffer = async ( + provider: ethers.providers.JsonRpcProvider, + parsedConfig: ChugSplashConfig +) => { + const executionAmount = await getExecutionAmountToSend(provider, parsedConfig) + return executionAmount.mul(15).div(10) +} + +export const hasSufficientFundsForExecution = async ( + provider: ethers.providers.JsonRpcProvider, + parsedConfig: ChugSplashConfig +): Promise => { + // Get the amount of funds that must be sent to the ChugSplashManager in order to execute the + // bundle. + const executionAmount = await getExecutionAmountToSend(provider, parsedConfig) + return executionAmount.eq(0) +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 5531b29d2..506ad4e7a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -2,3 +2,4 @@ export * from './actions' export * from './config' export * from './languages' export * from './utils' +export * from './fund' diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts index 861d91938..b7e5c5e96 100644 --- a/packages/core/src/utils.ts +++ b/packages/core/src/utils.ts @@ -215,3 +215,13 @@ export const displayDeploymentTable = ( console.table(deployments) } } + +export const claimExecutorPayment = async ( + executor: Signer, + ChugSplashManager: Contract +) => { + const executorDebt = await ChugSplashManager.debt(await executor.getAddress()) + if (executorDebt.gt(0)) { + await (await ChugSplashManager.claimExecutorPayment()).wait() + } +} diff --git a/packages/executor/src/index.ts b/packages/executor/src/index.ts index 17c5e5620..e0664a682 100644 --- a/packages/executor/src/index.ts +++ b/packages/executor/src/index.ts @@ -8,6 +8,10 @@ import { ChugSplashRegistryABI, CHUGSPLASH_REGISTRY_PROXY_ADDRESS, } from '@chugsplash/contracts' +import { + claimExecutorPayment, + hasSufficientFundsForExecution, +} from '@chugsplash/core' import { getChainId } from '@eth-optimism/core-utils' import * as Amplitude from '@amplitude/node' @@ -22,8 +26,9 @@ type Options = { type Metrics = {} type State = { + events: ethers.Event[] registry: ethers.Contract - wallet: ethers.Wallet + provider: ethers.providers.JsonRpcProvider lastBlockNumber: number amplitudeClient: Amplitude.NodeClient } @@ -66,111 +71,153 @@ export class ChugSplashExecutor extends BaseServiceV2 { } const reg = CHUGSPLASH_REGISTRY_PROXY_ADDRESS - const provider = ethers.getDefaultProvider(this.options.network) + this.state.provider = new ethers.providers.JsonRpcProvider( + this.options.network + ) this.state.registry = new ethers.Contract( reg, ChugSplashRegistryABI, - provider + this.state.provider ) this.state.lastBlockNumber = -1 - this.state.wallet = new ethers.Wallet(this.options.privateKey, provider) + this.state.events = [] } async main() { - // Find all active upgrades that have not yet been executed in blocks after the stored hash - const approvalAnnouncementEvents = await this.state.registry.queryFilter( + const wallet = new ethers.Wallet( + this.options.privateKey, + this.state.provider + ) + + const latestBlockNumber = await this.state.provider.getBlockNumber() + + // Get approval events in blocks after the stored block number + const newApprovalEvents = await this.state.registry.queryFilter( this.state.registry.filters.EventAnnounced('ChugSplashBundleApproved'), - this.state.lastBlockNumber + 1 + this.state.lastBlockNumber + 1, + latestBlockNumber ) + // Concatenate the new approval events to the array + this.state.events = this.state.events.concat(newApprovalEvents) + + // store last block number + this.state.lastBlockNumber = latestBlockNumber + // If none found, return - if (approvalAnnouncementEvents.length === 0) { + if (this.state.events.length === 0) { this.logger.info('no events found') return } - this.logger.info(`${approvalAnnouncementEvents.length} events found`) + this.logger.info( + `total number of events: ${this.state.events.length}. new events: ${newApprovalEvents.length}` + ) - // store last block number - this.state.lastBlockNumber = approvalAnnouncementEvents.at(-1).blockNumber + const eventsCopy = this.state.events.slice() // execute all approved bundles - for (const approvalAnnouncementEvent of approvalAnnouncementEvents) { + for (const approvalAnnouncementEvent of eventsCopy) { + // Remove the current event from the front of the events array and put it at the end + this.state.events.shift() + this.state.events.push(approvalAnnouncementEvent) + // fetch manager for relevant project - const signer = this.state.wallet const manager = new ethers.Contract( approvalAnnouncementEvent.args.manager, ChugSplashManagerABI, - signer + wallet ) // get active bundle id for this project const activeBundleId = await manager.activeBundleId() - if (activeBundleId === ethers.constants.HashZero) { - this.logger.error(`Error: No active bundle id found in manager`) - continue - } - - // get proposal event and compile - const proposalEvents = await manager.queryFilter( - manager.filters.ChugSplashBundleProposed(activeBundleId) - ) - const proposalEvent = proposalEvents[0] - const { bundle, canonicalConfig } = await compileRemoteBundle( - hre, - proposalEvent.args.configUri - ) + if (activeBundleId !== ethers.constants.HashZero) { + // Retrieve the corresponding proposal event to get the config URI. + const [proposalEvent] = await manager.queryFilter( + manager.filters.ChugSplashBundleProposed(activeBundleId) + ) - // ensure compiled bundle matches proposed bundle - if (bundle.root !== proposalEvent.args.bundleRoot) { - // log error and continue - this.logger.error( - 'Error: Compiled bundle root does not match proposal event bundle root', - canonicalConfig.options + // Compile the bundle using the config URI. + const { bundle, canonicalConfig } = await compileRemoteBundle( + hre, + proposalEvent.args.configUri ) - continue - } - // execute bundle - try { - await hre.run('chugsplash-execute', { - chugSplashManager: manager, - bundleId: activeBundleId, - bundle, - parsedConfig: canonicalConfig, - executor: signer, - hide: false, - }) - this.logger.info('Successfully executed') - } catch (e) { - // log error and continue - this.logger.error('Error: execution error', e, canonicalConfig.options) - continue - } + // ensure compiled bundle matches proposed bundle + if (bundle.root !== proposalEvent.args.bundleRoot) { + // We cannot execute this bundle, so we remove it from the events array. + this.state.events.pop() + + // log error and continue + this.logger.error( + 'Error: Compiled bundle root does not match proposal event bundle root', + canonicalConfig.options + ) + continue + } - // verify on etherscan - try { - if ((await getChainId(this.state.wallet.provider)) !== 31337) { - await verifyChugSplashConfig(hre, proposalEvent.args.configUri) - this.logger.info('Successfully verified') + if ( + await hasSufficientFundsForExecution( + this.state.provider, + canonicalConfig + ) + ) { + // execute bundle + try { + await hre.run('chugsplash-execute', { + chugSplashManager: manager, + bundleId: activeBundleId, + bundle, + parsedConfig: canonicalConfig, + executor: wallet, + hide: false, + }) + this.logger.info('Successfully executed') + } catch (e) { + // log error and continue + this.logger.error( + 'Error: execution error', + e, + canonicalConfig.options + ) + continue + } + + // verify on etherscan + try { + if ((await getChainId(this.state.provider)) !== 31337) { + await verifyChugSplashConfig(hre, proposalEvent.args.configUri) + this.logger.info('Successfully verified') + } + } catch (e) { + this.logger.error( + 'Error: verification error', + e, + canonicalConfig.options + ) + } + + if (this.options.amplitudeKey !== 'disabled') { + this.state.amplitudeClient.logEvent({ + event_type: 'ChugSplash Executed', + user_id: canonicalConfig.options.projectOwner, + event_properties: { + projectName: canonicalConfig.options.projectName, + }, + }) + } + } else { + // Continue to the next bundle if there is an insufficient amount of funds in the + // ChugSplashManager. + continue } - } catch (e) { - this.logger.error( - 'Error: verification error', - e, - canonicalConfig.options - ) } - if (this.options.amplitudeKey !== 'disabled') { - this.state.amplitudeClient.logEvent({ - event_type: 'ChugSplash Executed', - user_id: canonicalConfig.options.projectOwner, - event_properties: { - projectName: canonicalConfig.options.projectName, - }, - }) - } + // Withdraw any debt owed to the executor. + await claimExecutorPayment(wallet, manager) + + // Remove the current event from the events array. + this.state.events.pop() } } } diff --git a/packages/plugins/src/hardhat/deployments.ts b/packages/plugins/src/hardhat/deployments.ts index 37c23190b..520c15fea 100644 --- a/packages/plugins/src/hardhat/deployments.ts +++ b/packages/plugins/src/hardhat/deployments.ts @@ -15,6 +15,8 @@ import { ChugSplashActionBundle, computeBundleId, getChugSplashManager, + claimExecutorPayment, + getExecutionAmountToSendPlusBuffer } from '@chugsplash/core' import { getChainId } from '@eth-optimism/core-utils' import { HardhatRuntimeEnvironment } from 'hardhat/types' @@ -33,7 +35,6 @@ import { monitorTask, TASK_CHUGSPLASH_VERIFY_BUNDLE, } from './tasks' -import { getExecutionAmountPlusBuffer } from './fund' /** * TODO @@ -178,8 +179,8 @@ export const deployChugSplashConfig = async ( if (currBundleStatus === ChugSplashBundleStatus.PROPOSED) { spinner.start('Funding the deployment...') // Get the amount necessary to fund the deployment. - const executionAmountPlusBuffer = await getExecutionAmountPlusBuffer( - hre, + const executionAmountPlusBuffer = await getExecutionAmountToSendPlusBuffer( + hre.ethers.provider, parsedConfig ) // Approve and fund the deployment. @@ -220,6 +221,7 @@ export const deployChugSplashConfig = async ( }, hre ) + await claimExecutorPayment(signer, ChugSplashManager) } spinner.succeed(`${parsedConfig.options.projectName} deployed!`) diff --git a/packages/plugins/src/hardhat/execution.ts b/packages/plugins/src/hardhat/execution.ts index 33e1e80a2..bf6b7773f 100644 --- a/packages/plugins/src/hardhat/execution.ts +++ b/packages/plugins/src/hardhat/execution.ts @@ -4,13 +4,13 @@ import { ChugSplashConfig, getChugSplashRegistry, getChugSplashManager, + getOwnerBalanceInChugSplashManager, } from '@chugsplash/core' import { HardhatRuntimeEnvironment } from 'hardhat/types' import { ethers } from 'ethers' import { ChugSplashManagerABI } from '@chugsplash/contracts' import { sleep } from '@eth-optimism/core-utils' -import { getOwnerBalanceInChugSplashManager } from './fund' import { getFinalDeploymentTxnHash } from './deployments' export const monitorRemoteExecution = async ( diff --git a/packages/plugins/src/hardhat/fund.ts b/packages/plugins/src/hardhat/fund.ts deleted file mode 100644 index 62f5a2e75..000000000 --- a/packages/plugins/src/hardhat/fund.ts +++ /dev/null @@ -1,59 +0,0 @@ -import { ChugSplashManagerABI } from '@chugsplash/contracts' -import { - ChugSplashConfig, - getChugSplashManagerProxyAddress, -} from '@chugsplash/core' -import { Provider } from '@ethersproject/abstract-provider' -import { ethers } from 'ethers' -import { HardhatRuntimeEnvironment } from 'hardhat/types' - -/** - * Gets the amount ETH in the ChugSplashManager that can be used to execute a deployment. - * This equals the ChugSplashManager's balance minus the total debt owed to executors. - */ -export const getOwnerBalanceInChugSplashManager = async ( - provider: Provider, - projectName: string -): Promise => { - const ChugSplashManager = new ethers.Contract( - getChugSplashManagerProxyAddress(projectName), - ChugSplashManagerABI, - provider - ) - - const managerBalance = await provider.getBalance(ChugSplashManager.address) - const totalDebt = await ChugSplashManager.totalDebt() - return managerBalance.sub(totalDebt) -} - -export const getExecutionAmount = async ( - hre: HardhatRuntimeEnvironment, - parsedConfig: ChugSplashConfig -): Promise => { - const totalExecutionAmount = await simulateExecution(hre, parsedConfig) - const availableExecutionAmount = await getOwnerBalanceInChugSplashManager( - hre.ethers.provider, - parsedConfig.options.projectName - ) - const executionAmount = totalExecutionAmount.sub(availableExecutionAmount) - return executionAmount.gt(0) ? executionAmount : ethers.BigNumber.from(0) -} - -export const simulateExecution = async ( - hre: HardhatRuntimeEnvironment, - parsedConfig: ChugSplashConfig -) => { - hre - parsedConfig - - // TODO - return ethers.utils.parseEther('0.25') -} - -export const getExecutionAmountPlusBuffer = async ( - hre: HardhatRuntimeEnvironment, - parsedConfig: ChugSplashConfig -) => { - const executionAmount = await getExecutionAmount(hre, parsedConfig) - return executionAmount.mul(15).div(10) -} diff --git a/packages/plugins/src/hardhat/tasks.ts b/packages/plugins/src/hardhat/tasks.ts index 24ddbaef8..c33d6bca0 100644 --- a/packages/plugins/src/hardhat/tasks.ts +++ b/packages/plugins/src/hardhat/tasks.ts @@ -29,6 +29,8 @@ import { getChugSplashManager, getProjectOwnerAddress, isDeployImplementationAction, + getExecutionAmountToSendPlusBuffer, + getOwnerBalanceInChugSplashManager, } from '@chugsplash/core' import { ChugSplashManagerABI, @@ -66,10 +68,6 @@ import { errorProjectNotRegistered, successfulProposalMessage, } from '../messages' -import { - getExecutionAmountPlusBuffer, - getOwnerBalanceInChugSplashManager, -} from './fund' import { monitorRemoteExecution, postExecutionActions } from './execution' // Load environment variables from .env @@ -397,8 +395,8 @@ with a name other than ${parsedConfig.options.projectName}` // Get the amount that the user must send in order to execute the bundle including a buffer in // case the gas price increases during execution. - const executionAmountPlusBuffer = await getExecutionAmountPlusBuffer( - hre, + const executionAmountPlusBuffer = await getExecutionAmountToSendPlusBuffer( + hre.ethers.provider, parsedConfig ) @@ -567,12 +565,6 @@ export const executeTask = async ( bundleId ) - // Withdraw any debt owed to the executor. - const executorDebt = await chugSplashManager.debt(executorAddress) - if (executorDebt.gt(0)) { - await (await chugSplashManager.claimExecutorPayment()).wait() - } - if (isLocalExecution) { await postExecutionActions(hre.ethers.provider, parsedConfig) await createDeploymentArtifacts(hre, parsedConfig, finalDeploymentTxnHash) @@ -1192,16 +1184,18 @@ task(TASK_TEST_REMOTE_EXECUTION) 'configPath', 'Path to the ChugSplash config file to deploy' ) + .addFlag('silent', "Hide all of ChugSplash's output") .addFlag('noCompile', "Don't compile when running this task") .setAction( async ( args: { configPath: string noCompile: boolean + silent: boolean }, hre: HardhatRuntimeEnvironment ) => { - const { configPath, noCompile } = args + const { configPath, noCompile, silent } = args if (hre.network.name !== 'localhost') { throw new Error( @@ -1209,9 +1203,19 @@ task(TASK_TEST_REMOTE_EXECUTION) ) } + const spinner = ora({ isSilent: silent }) + const signer = hre.ethers.provider.getSigner() await deployChugSplashPredeploys(hre, signer) - await deployChugSplashConfig(hre, configPath, false, true, '', noCompile) + await deployChugSplashConfig( + hre, + configPath, + false, + true, + '', + noCompile, + spinner + ) } )