From 5675416250692a75949d63bd6e0a4b7f7064c208 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1n=20Jakub=20Nani=C5=A1ta?= Date: Thu, 8 Feb 2024 14:27:17 -0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=AA=B2=20Fix=20lz:deploy=20task=20not=20b?= =?UTF-8?q?eing=20able=20to=20find=20external=20deployments=20(#372)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .changeset/beige-elephants-attend.md | 7 ++ .../devtools-evm-hardhat/src/tasks/deploy.ts | 39 +++++-- .../src/tasks/oapp/config.init.ts | 7 +- .../src/tasks/oapp/wire/index.ts | 10 +- tests-user/tests/create-lz-oapp.bats | 32 ++++++ .../deploy/002_TestProxy.ts | 5 +- .../test/task/deploy.test.ts | 106 ++++++++++++++++-- turbo.json | 1 - 8 files changed, 173 insertions(+), 34 deletions(-) create mode 100644 .changeset/beige-elephants-attend.md diff --git a/.changeset/beige-elephants-attend.md b/.changeset/beige-elephants-attend.md new file mode 100644 index 000000000..52265081e --- /dev/null +++ b/.changeset/beige-elephants-attend.md @@ -0,0 +1,7 @@ +--- +"@layerzerolabs/ua-devtools-evm-hardhat": patch +"@layerzerolabs/devtools-evm-hardhat-test": patch +"@layerzerolabs/devtools-evm-hardhat": patch +--- + +Fix problems with missing deployments when running lz:deploy diff --git a/packages/devtools-evm-hardhat/src/tasks/deploy.ts b/packages/devtools-evm-hardhat/src/tasks/deploy.ts index a08da672d..0027064e9 100644 --- a/packages/devtools-evm-hardhat/src/tasks/deploy.ts +++ b/packages/devtools-evm-hardhat/src/tasks/deploy.ts @@ -20,12 +20,14 @@ import { promptForText } from '@layerzerolabs/io-devtools' import { Deployment } from 'hardhat-deploy/dist/types' import { assertDefinedNetworks, assertHardhatDeploy } from '@/internal/assertions' import { splitCommaSeparated } from '@layerzerolabs/devtools' +import { isDeepEqual } from '@layerzerolabs/devtools' interface TaskArgs { networks?: string[] tags?: string[] logLevel?: string ci?: boolean + reset?: boolean } /** @@ -55,7 +57,7 @@ type NetworkDeployResult = } const action: ActionType = async ( - { networks: networksArgument, tags: tagsArgument = [], logLevel = 'info', ci = false }, + { networks: networksArgument, tags: tagsArgument = [], logLevel = 'info', ci = false, reset = false }, hre ): Promise => { printLogo() @@ -72,6 +74,7 @@ const action: ActionType = async ( // We only want to be asking users for input if we are not in interactive mode const isInteractive = !ci logger.debug(isInteractive ? 'Running in interactive mode' : 'Running in non-interactive (CI) mode') + logger.debug(reset ? 'Will delete existing deployments' : 'Will not delete existing deployments') // The first thing to do is to ensure that the project is compiled try { @@ -177,11 +180,35 @@ const action: ActionType = async ( // We need to make sure the user has enabled hardhat-deploy assertHardhatDeploy(env) + // We first collect all existing deployments + // + // We do this so that we can diff the state before and after + // running the deployment scripts. + // + // This is, in immediate effect, a workaround for having to set resetMemory + // in the options for the run() function below to false. In near future though + // it opens doors for being able to return partially successful deployment results + const deploymentsBefore = await env.deployments.all() + // The core of this task, running the hardhat deploy scripts - const contracts = await env.deployments.run(selectedTags, { + const deploymentsAfter = await env.deployments.run(selectedTags, { + // If we don't pass resetmemory or set it to true, + // hardhat deploy will erase the database of deployments + // (including the external deployments) + // + // In effect this means the deployments for LayerZero artifacts would not be available + resetMemory: false, writeDeploymentsToFiles: true, + deletePreviousDeployments: reset, }) + // Now we do a little diff on what contracts had been changed + const contracts = Object.fromEntries( + Object.entries(deploymentsAfter).filter( + ([name]) => !isDeepEqual(deploymentsBefore[name], deploymentsAfter[name]) + ) + ) + results[networkName] = { contracts } logger.debug(`Successfully deployed network ${networkName}`) @@ -253,9 +280,5 @@ task(TASK_LZ_DEPLOY, 'Deploy LayerZero contracts', action) true ) .addParam('logLevel', 'Logging level. One of: error, warn, info, verbose, debug, silly', 'info', types.logLevel) - .addParam( - 'ci', - 'Continuous integration (non-interactive) mode. Will not ask for any input from the user', - false, - types.boolean - ) + .addFlag('ci', 'Continuous integration (non-interactive) mode. Will not ask for any input from the user') + .addFlag('reset', 'Delete existing deployments') diff --git a/packages/ua-devtools-evm-hardhat/src/tasks/oapp/config.init.ts b/packages/ua-devtools-evm-hardhat/src/tasks/oapp/config.init.ts index 26d62ab12..569cc0da6 100644 --- a/packages/ua-devtools-evm-hardhat/src/tasks/oapp/config.init.ts +++ b/packages/ua-devtools-evm-hardhat/src/tasks/oapp/config.init.ts @@ -94,10 +94,5 @@ if (process.env.LZ_ENABLE_EXPERIMENTAL_TASK_LZ_OAPP_CONFIG_INIT) { task(TASK_LZ_OAPP_CONFIG_INIT, 'Initialize an OApp configuration file', action) .addParam('oappConfig', 'Path to the new LayerZero OApp config', undefined, types.string) .addParam('logLevel', 'Logging level. One of: error, warn, info, verbose, debug, silly', 'info', types.logLevel) - .addParam( - 'ci', - 'Continuous integration (non-interactive) mode. Will not ask for any input from the user', - false, - types.boolean - ) + .addFlag('ci', 'Continuous integration (non-interactive) mode. Will not ask for any input from the user') } diff --git a/packages/ua-devtools-evm-hardhat/src/tasks/oapp/wire/index.ts b/packages/ua-devtools-evm-hardhat/src/tasks/oapp/wire/index.ts index 478055093..08b788e0a 100644 --- a/packages/ua-devtools-evm-hardhat/src/tasks/oapp/wire/index.ts +++ b/packages/ua-devtools-evm-hardhat/src/tasks/oapp/wire/index.ts @@ -189,13 +189,7 @@ const action: ActionType = async ( return [successfulTransactions, errors, transactionsToSign] } -task(TASK_LZ_OAPP_WIRE, 'Wire LayerZero OApp') +task(TASK_LZ_OAPP_WIRE, 'Wire LayerZero OApp', action) .addParam('oappConfig', 'Path to your LayerZero OApp config', undefined, types.string) .addParam('logLevel', 'Logging level. One of: error, warn, info, verbose, debug, silly', 'info', types.logLevel) - .addParam( - 'ci', - 'Continuous integration (non-interactive) mode. Will not ask for any input from the user', - false, - types.boolean - ) - .setAction(action) + .addFlag('ci', 'Continuous integration (non-interactive) mode. Will not ask for any input from the user') diff --git a/tests-user/tests/create-lz-oapp.bats b/tests-user/tests/create-lz-oapp.bats index 6fe48eb82..6a92bef11 100644 --- a/tests-user/tests/create-lz-oapp.bats +++ b/tests-user/tests/create-lz-oapp.bats @@ -82,6 +82,38 @@ teardown() { assert [ ! -d $DESTINATION ] } +# This test is a bit ridiculous because it basically checks that we error out +# on the fact that we have insufficient funds, not on the fact that the EndpointV2 deployment cannot be found +@test "should find EndpointV2 deployment for oapp in CI mode" { + local DESTINATION="$PROJECTS_DIRECTORY/pnpm-oapp" + local MNEMONIC="test test test test test test test test test test test junk" + + npx --yes create-lz-oapp --ci --example oapp --destination $DESTINATION --package-manager pnpm + cd "$DESTINATION" + + MNEMONIC=$MNEMONIC run pnpm hardhat lz:deploy --ci + assert_output --partial "insufficient funds for intrinsic transaction cost" + + MNEMONIC=$MNEMONIC run pnpm hardhat deploy --network fuji + assert_output --partial "insufficient funds for intrinsic transaction cost" +} + +# This test is a bit ridiculous because it basically checks that we error out +# on the fact that we have insufficient funds, not on the fact that the EndpointV2 deployment cannot be found +@test "should find EndpointV2 deployment for oft in CI mode" { + local DESTINATION="$PROJECTS_DIRECTORY/pnpm-oft" + local MNEMONIC="test test test test test test test test test test test junk" + + npx --yes create-lz-oapp --ci --example oft --destination $DESTINATION --package-manager pnpm + cd "$DESTINATION" + + MNEMONIC=$MNEMONIC run pnpm hardhat lz:deploy --ci + assert_output --partial "insufficient funds for intrinsic transaction cost" + + MNEMONIC=$MNEMONIC run pnpm hardhat deploy --network fuji + assert_output --partial "insufficient funds for intrinsic transaction cost" +} + @test "should work with pnpm & oapp example in CI mode" { local DESTINATION="$PROJECTS_DIRECTORY/pnpm-oapp" diff --git a/tests/devtools-evm-hardhat-test/deploy/002_TestProxy.ts b/tests/devtools-evm-hardhat-test/deploy/002_TestProxy.ts index 3d8569a05..7e42ace8c 100644 --- a/tests/devtools-evm-hardhat-test/deploy/002_TestProxy.ts +++ b/tests/devtools-evm-hardhat-test/deploy/002_TestProxy.ts @@ -10,7 +10,10 @@ const deploy: DeployFunction = async ({ getUnnamedAccounts, deployments, network const [deployer] = await getUnnamedAccounts() assert(deployer, 'Missing deployer') - await deployments.delete('TestProxy') + deployments.delete('TestProxy') + deployments.delete('TestProxy_Implementation') + deployments.delete('TestProxy_Proxy') + deployments.delete('DefaultProxyAdmin') const testProxyDeployment = await deployments.deploy('TestProxy', { from: deployer, proxy: { diff --git a/tests/devtools-evm-hardhat-test/test/task/deploy.test.ts b/tests/devtools-evm-hardhat-test/test/task/deploy.test.ts index 2dfedb9be..8642ca625 100644 --- a/tests/devtools-evm-hardhat-test/test/task/deploy.test.ts +++ b/tests/devtools-evm-hardhat-test/test/task/deploy.test.ts @@ -2,6 +2,7 @@ import hre from 'hardhat' import { TASK_COMPILE } from 'hardhat/builtin-tasks/task-names' +import { DeploymentsManager } from 'hardhat-deploy/dist/src/DeploymentsManager' import { TASK_LZ_DEPLOY, createClearDeployments, @@ -24,7 +25,8 @@ jest.mock('@layerzerolabs/io-devtools', () => { const promptForTextMock = promptForText as jest.Mock const promptToContinueMock = promptToContinue as jest.Mock const promptToSelectMultipleMock = promptToSelectMultiple as jest.Mock -const runMock = jest.spyOn(hre, 'run') +const runSpy = jest.spyOn(hre, 'run') +const runDeploySpy = jest.spyOn(DeploymentsManager.prototype, 'runDeploy') describe(`task ${TASK_LZ_DEPLOY}`, () => { const expectDeployment = expect.objectContaining({ @@ -46,6 +48,8 @@ describe(`task ${TASK_LZ_DEPLOY}`, () => { promptToContinueMock.mockReset() promptToSelectMultipleMock.mockReset() + runDeploySpy.mockClear() + const getHreByEid = createGetHreByEid(hre) const clearDeployments = createClearDeployments(getHreByEid) @@ -71,7 +75,7 @@ describe(`task ${TASK_LZ_DEPLOY}`, () => { // For some reason even though we did not specify any arguments to the compile task, // jest still sees some aarguments being passed so we need to pass those to make this expect work - expect(runMock).toHaveBeenCalledWith(TASK_COMPILE, undefined, {}, undefined) + expect(runSpy).toHaveBeenCalledWith(TASK_COMPILE, undefined, {}, undefined) }) it('should ask for networks & tags', async () => { @@ -166,7 +170,58 @@ describe(`task ${TASK_LZ_DEPLOY}`, () => { promptToSelectMultipleMock.mockResolvedValueOnce(['vengaboys', 'tango']) // Since we provided selected no tags, everything will be deployed + await expect(hre.run(TASK_LZ_DEPLOY, { reset: true })).resolves.toEqual({ + tango: { + contracts: expect.objectContaining({ + Thrower: expectDeployment, + TestProxy: expectDeployment, + }), + }, + vengaboys: { + contracts: expect.objectContaining({ + Thrower: expectDeployment, + TestProxy: expectDeployment, + }), + }, + }) + }) + + it('should not redeploy if reset flag has not been passed', async () => { + // We want to say yes to deployment + promptToContinueMock.mockResolvedValue(true) + // We want to deploy two imaginary tags + promptForTextMock.mockResolvedValue('') + // And we want to select two networks + promptToSelectMultipleMock.mockResolvedValue(['vengaboys', 'tango']) + + // We run the deploy first + await hre.run(TASK_LZ_DEPLOY, {}) + + // Then we run the deploy again and expect nothing to have been deployed + // since we didn't pass the --reset flag await expect(hre.run(TASK_LZ_DEPLOY, {})).resolves.toEqual({ + tango: { + contracts: {}, + }, + vengaboys: { + contracts: {}, + }, + }) + }) + + it('should redeploy if reset flag has been passed', async () => { + // We want to say yes to deployment + promptToContinueMock.mockResolvedValue(true) + // We want to deploy two imaginary tags + promptForTextMock.mockResolvedValue('') + // And we want to select two networks + promptToSelectMultipleMock.mockResolvedValue(['vengaboys', 'tango']) + + // We run the deploy first + await hre.run(TASK_LZ_DEPLOY, {}) + + // Then we run the deploy again + await expect(hre.run(TASK_LZ_DEPLOY, { reset: true })).resolves.toEqual({ tango: { contracts: expect.objectContaining({ Thrower: expectDeployment, @@ -190,10 +245,38 @@ describe(`task ${TASK_LZ_DEPLOY}`, () => { // And we want to select two networks promptToSelectMultipleMock.mockResolvedValueOnce(['vengaboys', 'tango']) - const { tango, vengaboys } = await hre.run(TASK_LZ_DEPLOY, {}) + await hre.run(TASK_LZ_DEPLOY, {}) - expect(Object.keys(tango.contracts)).toEqual(['Thrower']) - expect(Object.keys(vengaboys.contracts)).toEqual(['Thrower']) + expect(runDeploySpy).toHaveBeenCalledTimes(2) + expect(runDeploySpy).toHaveBeenNthCalledWith(1, ['Thrower'], expect.any(Object)) + expect(runDeploySpy).toHaveBeenNthCalledWith(2, ['Thrower'], expect.any(Object)) + }) + + it('should not reset memory on the deployments extension', async () => { + // We want to say yes to deployment + promptToContinueMock.mockResolvedValueOnce(true) + // We want to deploy two imaginary tags + promptForTextMock.mockResolvedValueOnce('') + // And we want to select two networks + promptToSelectMultipleMock.mockResolvedValueOnce(['vengaboys', 'tango']) + + await hre.run(TASK_LZ_DEPLOY, {}) + + expect(runDeploySpy).toHaveBeenCalledTimes(2) + expect(runDeploySpy).toHaveBeenNthCalledWith( + 1, + [], + expect.objectContaining({ + resetMemory: false, + }) + ) + expect(runDeploySpy).toHaveBeenNthCalledWith( + 2, + [], + expect.objectContaining({ + resetMemory: false, + }) + ) }) }) @@ -205,13 +288,14 @@ describe(`task ${TASK_LZ_DEPLOY}`, () => { // For some reason even though we did not specify any arguments to the compile task, // jest still sees some aarguments being passed so we need to pass those to make this expect work - expect(runMock).toHaveBeenCalledWith(TASK_COMPILE, undefined, {}, undefined) + expect(runSpy).toHaveBeenCalledWith(TASK_COMPILE, undefined, {}, undefined) }) it('should use all available networks & tags if networks argument is undefined', async () => { await expect( hre.run(TASK_LZ_DEPLOY, { ci: true, + reset: true, }) ).resolves.toEqual({ britney: { @@ -255,6 +339,7 @@ describe(`task ${TASK_LZ_DEPLOY}`, () => { hre.run(TASK_LZ_DEPLOY, { ci: true, tags: [], + reset: true, }) ).resolves.toEqual({ britney: { @@ -279,11 +364,12 @@ describe(`task ${TASK_LZ_DEPLOY}`, () => { }) it('should deploy only the tags provided', async () => { - const { tango, vengaboys, britney } = await hre.run(TASK_LZ_DEPLOY, { ci: true, tags: ['Thrower'] }) + await hre.run(TASK_LZ_DEPLOY, { ci: true, tags: ['Thrower'] }) - expect(Object.keys(britney.contracts)).toEqual(['Thrower']) - expect(Object.keys(tango.contracts)).toEqual(['Thrower']) - expect(Object.keys(vengaboys.contracts)).toEqual(['Thrower']) + expect(runDeploySpy).toHaveBeenCalledTimes(3) + expect(runDeploySpy).toHaveBeenNthCalledWith(1, ['Thrower'], expect.any(Object)) + expect(runDeploySpy).toHaveBeenNthCalledWith(2, ['Thrower'], expect.any(Object)) + expect(runDeploySpy).toHaveBeenNthCalledWith(3, ['Thrower'], expect.any(Object)) }) }) }) diff --git a/turbo.json b/turbo.json index 0ca632a5c..24d5fd28c 100644 --- a/turbo.json +++ b/turbo.json @@ -38,7 +38,6 @@ }, "globalDependencies": ["tsconfig.json"], "globalPassThroughEnv": [ - "LZ_ENABLE_EXPERIMENTAL_TASK_LZ_DEPLOY", "LZ_ENABLE_EXPERIMENTAL_TASK_LZ_OAPP_CONFIG_INIT", "LAYERZERO_EXAMPLES_REPOSITORY_URL", "LAYERZERO_EXAMPLES_REPOSITORY_REF",