From 5236166cf0526eeef4abb167e056b48a985e48a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1n=20Jakub=20Nani=C5=A1ta?= Date: Tue, 9 Jan 2024 16:08:07 -0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=AA=9A=20Error=20parser=20(#173)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .changeset/fair-nails-suffer.md | 6 + packages/devtools-evm-hardhat/package.json | 2 + .../src/{ => errors}/errors.ts | 0 .../devtools-evm-hardhat/src/errors/index.ts | 2 + .../devtools-evm-hardhat/src/errors/parser.ts | 40 ++++ packages/devtools-evm-hardhat/src/index.ts | 1 + packages/devtools-evm/src/omnigraph/sdk.ts | 8 +- pnpm-lock.yaml | 12 + .../contracts/Thrower.sol | 75 +++++++ tests/devtools-evm-hardhat-test/package.json | 3 + .../test/errors/parser.test.ts | 210 ++++++++++++++++++ 11 files changed, 355 insertions(+), 4 deletions(-) create mode 100644 .changeset/fair-nails-suffer.md rename packages/devtools-evm-hardhat/src/{ => errors}/errors.ts (100%) create mode 100644 packages/devtools-evm-hardhat/src/errors/index.ts create mode 100644 packages/devtools-evm-hardhat/src/errors/parser.ts create mode 100644 tests/devtools-evm-hardhat-test/contracts/Thrower.sol create mode 100644 tests/devtools-evm-hardhat-test/test/errors/parser.test.ts diff --git a/.changeset/fair-nails-suffer.md b/.changeset/fair-nails-suffer.md new file mode 100644 index 000000000..af0387ff9 --- /dev/null +++ b/.changeset/fair-nails-suffer.md @@ -0,0 +1,6 @@ +--- +"@layerzerolabs/devtools-evm-hardhat-test": patch +"@layerzerolabs/devtools-evm-hardhat": patch +--- + +Add a generic contract error parser for hardhat projects diff --git a/packages/devtools-evm-hardhat/package.json b/packages/devtools-evm-hardhat/package.json index d04b743c7..e8a359785 100644 --- a/packages/devtools-evm-hardhat/package.json +++ b/packages/devtools-evm-hardhat/package.json @@ -41,6 +41,7 @@ "zod": "^3.22.4" }, "devDependencies": { + "@ethersproject/abi": "^5.7.0", "@ethersproject/abstract-signer": "^5.7.0", "@ethersproject/contracts": "^5.7.0", "@ethersproject/providers": "^5.7.2", @@ -66,6 +67,7 @@ "typescript": "^5.3.3" }, "peerDependencies": { + "@ethersproject/abi": "^5.7.0", "@ethersproject/abstract-signer": "^5.7.0", "@ethersproject/contracts": "^5.7.0", "@ethersproject/providers": "^5.7.0", diff --git a/packages/devtools-evm-hardhat/src/errors.ts b/packages/devtools-evm-hardhat/src/errors/errors.ts similarity index 100% rename from packages/devtools-evm-hardhat/src/errors.ts rename to packages/devtools-evm-hardhat/src/errors/errors.ts diff --git a/packages/devtools-evm-hardhat/src/errors/index.ts b/packages/devtools-evm-hardhat/src/errors/index.ts new file mode 100644 index 000000000..f73769133 --- /dev/null +++ b/packages/devtools-evm-hardhat/src/errors/index.ts @@ -0,0 +1,2 @@ +export * from './errors' +export * from './parser' diff --git a/packages/devtools-evm-hardhat/src/errors/parser.ts b/packages/devtools-evm-hardhat/src/errors/parser.ts new file mode 100644 index 000000000..4d58407fd --- /dev/null +++ b/packages/devtools-evm-hardhat/src/errors/parser.ts @@ -0,0 +1,40 @@ +import { getDefaultRuntimeEnvironment } from '@/runtime' +import { Contract } from '@ethersproject/contracts' +import { + createErrorParser as createErrorParserBase, + makeZeroAddress, + type OmniContractFactory, +} from '@layerzerolabs/devtools-evm' + +/** + * Helper function that combines all the available ABIs into a one giant + * interface (only containing the error fragments) used for error decoding. + * + * TODO This function is not memoized at the moment, if the performance turns out to be a bottleneck we can memoize + * + * @returns {OmniContractFactory} + */ +const createCombinedContractFactory = + (): OmniContractFactory => + async ({ eid }) => { + // We're getting the artifacts so it does not really matter which environment we get them from + const env = getDefaultRuntimeEnvironment() + + // We'll grab all the artifacts from the environment + const artifactNames = await env.artifacts.getAllFullyQualifiedNames() + const artifacts = artifactNames.map((name) => env.artifacts.readArtifactSync(name)) + + // Now we combine the ABIs and keep only the errors + const abi = artifacts.flatMap((artifact) => artifact.abi).filter(({ type }) => type === 'error') + + // Even though duplicated fragments don't throw errors, they still pollute the interface with warning console.logs + // To prevent this, we'll run a simple deduplication algorithm - use JSON encoded values as hashes + const deduplicatedAbi = Object.values(Object.fromEntries(abi.map((abi) => [JSON.stringify(abi), abi]))) + + return { eid, contract: new Contract(makeZeroAddress(), deduplicatedAbi) } + } + +/** + * Creates a generic error parser based on all the artifacts found in your hardhat project + */ +export const createErrorParser = () => createErrorParserBase(createCombinedContractFactory()) diff --git a/packages/devtools-evm-hardhat/src/index.ts b/packages/devtools-evm-hardhat/src/index.ts index 1bf375c68..139834a89 100644 --- a/packages/devtools-evm-hardhat/src/index.ts +++ b/packages/devtools-evm-hardhat/src/index.ts @@ -3,6 +3,7 @@ import './type-extensions' // Regular exports export * from './config' +export * from './errors' export * from './internal' export * from './omnigraph' export * from './provider' diff --git a/packages/devtools-evm/src/omnigraph/sdk.ts b/packages/devtools-evm/src/omnigraph/sdk.ts index 80073fa1c..6587eae3e 100644 --- a/packages/devtools-evm/src/omnigraph/sdk.ts +++ b/packages/devtools-evm/src/omnigraph/sdk.ts @@ -1,9 +1,9 @@ -import { OmniPoint, OmniTransaction } from '@layerzerolabs/devtools' -import { IOmniSDK, OmniContract } from './types' +import type { OmniPoint, OmniTransaction } from '@layerzerolabs/devtools' +import type { IOmniSDK, OmniContract } from './types' import { omniContractToPoint } from './coordinates' import { createContractErrorParser } from '@/errors/parser' -import { OmniContractErrorParser } from '@/errors/types' -import { ContractError } from '..' +import type { OmniContractErrorParser } from '@/errors/types' +import type { ContractError } from '@/errors/errors' /** * Base class for all EVM SDKs, providing some common functionality diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7df00760f..87a010f73 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -326,6 +326,9 @@ importers: specifier: ^3.22.4 version: 3.22.4 devDependencies: + '@ethersproject/abi': + specifier: ^5.7.0 + version: 5.7.0 '@ethersproject/abstract-signer': specifier: ^5.7.0 version: 5.7.0 @@ -935,9 +938,15 @@ importers: '@ethersproject/abstract-signer': specifier: ^5.7.0 version: 5.7.0 + '@ethersproject/bignumber': + specifier: ^5.7.0 + version: 5.7.0 '@ethersproject/constants': specifier: ^5.7.0 version: 5.7.0 + '@ethersproject/contracts': + specifier: ^5.7.0 + version: 5.7.0 '@ethersproject/providers': specifier: ^5.7.2 version: 5.7.2 @@ -989,6 +998,9 @@ importers: '@layerzerolabs/protocol-devtools-evm': specifier: ~0.0.1 version: link:../../packages/protocol-devtools-evm + '@layerzerolabs/test-devtools': + specifier: ~0.0.1 + version: link:../test-devtools '@layerzerolabs/toolbox-hardhat': specifier: ~0.0.1 version: link:../../packages/toolbox-hardhat diff --git a/tests/devtools-evm-hardhat-test/contracts/Thrower.sol b/tests/devtools-evm-hardhat-test/contracts/Thrower.sol new file mode 100644 index 000000000..2f174a64f --- /dev/null +++ b/tests/devtools-evm-hardhat-test/contracts/Thrower.sol @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.22; + +contract Thrower { + error CustomErrorWithNoArguments(); + error CustomErrorWithAnArgument(string message); + + // This error is duplicated in the NestedThrower + error CommonErrorWithNoArguments(); + + // This error is duplicated in the NestedThrower but with a different argument + error CommonErrorWithAnArgument(string message); + + NestedThrower private nested; + + constructor() { + nested = new NestedThrower(); + } + + function throwWithCustomErrorAndNoArguments() external pure { + revert CustomErrorWithNoArguments(); + } + + function throwWithCustomErrorAndArgument(string calldata message) external pure { + revert CustomErrorWithAnArgument(message); + } + + function throwWithCommonErrorAndNoArguments() external pure { + revert CommonErrorWithNoArguments(); + } + + function throwWithCommonErrorAndArgument(string calldata message) external pure { + revert CommonErrorWithAnArgument(message); + } + + function throwNestedWithCustomErrorAndNoArguments() external view { + nested.throwWithCustomErrorAndNoArguments(); + } + + function throwNestedWithCustomErrorAndArgument(string calldata message) external view { + nested.throwWithCustomErrorAndArgument(message); + } + + function throwNestedWithCommonErrorAndNoArguments() external view { + nested.throwWithCommonErrorAndNoArguments(); + } + + function throwNestedWithCommonErrorAndArgument(uint256 code) external view { + nested.throwWithCommonErrorAndArgument(code); + } +} + +contract NestedThrower { + error NestedCustomErrorWithNoArguments(); + error NestedCustomErrorWithAnArgument(string message); + + error CommonErrorWithNoArguments(); + error CommonErrorWithAnArgument(uint256 code); + + function throwWithCustomErrorAndNoArguments() external pure { + revert NestedCustomErrorWithNoArguments(); + } + + function throwWithCustomErrorAndArgument(string calldata message) external pure { + revert NestedCustomErrorWithAnArgument(message); + } + + function throwWithCommonErrorAndNoArguments() external pure { + revert CommonErrorWithNoArguments(); + } + + function throwWithCommonErrorAndArgument(uint256 code) external pure { + revert CommonErrorWithAnArgument(code); + } +} diff --git a/tests/devtools-evm-hardhat-test/package.json b/tests/devtools-evm-hardhat-test/package.json index c25f7fc25..c2dfecf73 100644 --- a/tests/devtools-evm-hardhat-test/package.json +++ b/tests/devtools-evm-hardhat-test/package.json @@ -18,7 +18,9 @@ "devDependencies": { "@babel/core": "^7.23.7", "@ethersproject/abstract-signer": "^5.7.0", + "@ethersproject/bignumber": "^5.7.0", "@ethersproject/constants": "^5.7.0", + "@ethersproject/contracts": "^5.7.0", "@ethersproject/providers": "^5.7.2", "@ethersproject/wallet": "^5.7.0", "@layerzerolabs/devtools": "~0.0.1", @@ -36,6 +38,7 @@ "@layerzerolabs/omnicounter-devtools-evm": "~0.0.1", "@layerzerolabs/protocol-devtools": "~0.0.1", "@layerzerolabs/protocol-devtools-evm": "~0.0.1", + "@layerzerolabs/test-devtools": "~0.0.1", "@layerzerolabs/toolbox-hardhat": "~0.0.1", "@nomicfoundation/hardhat-ethers": "^3.0.5", "@openzeppelin/contracts": "^4.9.5", diff --git a/tests/devtools-evm-hardhat-test/test/errors/parser.test.ts b/tests/devtools-evm-hardhat-test/test/errors/parser.test.ts new file mode 100644 index 000000000..bf3e4e78f --- /dev/null +++ b/tests/devtools-evm-hardhat-test/test/errors/parser.test.ts @@ -0,0 +1,210 @@ +import fc from 'fast-check' +import 'hardhat' +import { BigNumber } from '@ethersproject/bignumber/lib/bignumber' +import { Contract } from '@ethersproject/contracts' +import { CustomError, UnknownError } from '@layerzerolabs/devtools-evm' +import { createErrorParser, createSignerFactory } from '@layerzerolabs/devtools-evm-hardhat' +import { OmniError } from '@layerzerolabs/devtools' +import { pointArbitrary } from '@layerzerolabs/test-devtools' +import { getHreByNetworkName, getEidForNetworkName } from '@layerzerolabs/devtools-evm-hardhat' +import assert from 'assert' + +describe('errors/parser', () => { + describe('createErrorParser', () => { + let contract: Contract + + /** + * Helper utility that swaps the promise resolution for rejection and other way around + * + * This is useful for the below tests since we are testing that promises reject + * and want to get their rejection values. + * + * @param promise `Promise` + * + * @returns `Promise` + */ + const assertFailed = async (promise: Promise): Promise => + promise.then( + (result) => { + fail(`Expected a promise to always reject but it resolved with ${JSON.stringify(result)}`) + }, + (error) => error + ) + + beforeAll(async () => { + // Get the environment + const env = await getHreByNetworkName('britney') + const eid = getEidForNetworkName('britney') + const { signer } = await createSignerFactory()(eid) + + // Get the deployer account + const [deployer] = await env.getUnnamedAccounts() + assert(deployer, 'Missing deployer account') + + // Deploy the Thrower contract + const deployment = await env.deployments.deploy('Thrower', { + from: deployer, + }) + + contract = new Contract(deployment.address, deployment.abi, signer) + }) + + it('should parse a custom an error with no arguments coming from the contract itself', async () => { + const errorParser = createErrorParser() + + await fc.assert( + fc.asyncProperty(pointArbitrary, async (point) => { + const error = await assertFailed(contract.throwWithCustomErrorAndNoArguments()) + const omniError: OmniError = { error, point } + const parsedError = await errorParser(omniError) + + expect(parsedError.point).toEqual(point) + expect(parsedError.error).toBeInstanceOf(CustomError) + expect(parsedError.error.reason).toEqual('CustomErrorWithNoArguments') + expect((parsedError.error as CustomError).args).toEqual([]) + }) + ) + }) + + it('should parse a custom an error with an argument coming from the contract itself', async () => { + const errorParser = createErrorParser() + + await fc.assert( + fc.asyncProperty(pointArbitrary, fc.string(), async (point, arg) => { + const error = await assertFailed(contract.throwWithCustomErrorAndArgument(arg)) + const omniError: OmniError = { error, point } + const parsedError = await errorParser(omniError) + + expect(parsedError.point).toEqual(point) + expect(parsedError.error).toBeInstanceOf(CustomError) + expect(parsedError.error.reason).toEqual('CustomErrorWithAnArgument') + expect((parsedError.error as CustomError).args).toEqual([arg]) + }) + ) + }) + + it('should parse a custom an error with no arguments coming from a nested contract', async () => { + const errorParser = createErrorParser() + + await fc.assert( + fc.asyncProperty(pointArbitrary, async (point) => { + const error = await assertFailed(contract.throwNestedWithCustomErrorAndNoArguments()) + const omniError: OmniError = { error, point } + const parsedError = await errorParser(omniError) + + expect(parsedError.point).toEqual(point) + expect(parsedError.error).toBeInstanceOf(CustomError) + expect(parsedError.error.reason).toEqual('NestedCustomErrorWithNoArguments') + expect((parsedError.error as CustomError).args).toEqual([]) + }) + ) + }) + + it('should parse a custom an error with an argument coming from a nested contract', async () => { + const errorParser = createErrorParser() + + await fc.assert( + fc.asyncProperty(pointArbitrary, fc.string(), async (point, arg) => { + const error = await assertFailed(contract.throwNestedWithCustomErrorAndArgument(arg)) + const omniError: OmniError = { error, point } + const parsedError = await errorParser(omniError) + + expect(parsedError.point).toEqual(point) + expect(parsedError.error).toBeInstanceOf(CustomError) + expect(parsedError.error.reason).toEqual('NestedCustomErrorWithAnArgument') + expect((parsedError.error as CustomError).args).toEqual([arg]) + }) + ) + }) + + it('should parse a custom an error with no arguments defined in more contracts coming from the contract itself', async () => { + const errorParser = createErrorParser() + + await fc.assert( + fc.asyncProperty(pointArbitrary, async (point) => { + const error = await assertFailed(contract.throwWithCommonErrorAndNoArguments()) + const omniError: OmniError = { error, point } + const parsedError = await errorParser(omniError) + + expect(parsedError.point).toEqual(point) + expect(parsedError.error).toBeInstanceOf(CustomError) + expect(parsedError.error.reason).toEqual('CommonErrorWithNoArguments') + expect((parsedError.error as CustomError).args).toEqual([]) + }) + ) + }) + + it('should parse a custom an error with an different arguments defined in more contracts coming from the contract itself', async () => { + const errorParser = createErrorParser() + + await fc.assert( + fc.asyncProperty(pointArbitrary, fc.string(), async (point, arg) => { + const error = await assertFailed(contract.throwWithCommonErrorAndArgument(arg)) + const omniError: OmniError = { error, point } + const parsedError = await errorParser(omniError) + + expect(parsedError.point).toEqual(point) + expect(parsedError.error).toBeInstanceOf(CustomError) + expect(parsedError.error.reason).toEqual('CommonErrorWithAnArgument') + expect((parsedError.error as CustomError).args).toEqual([arg]) + }) + ) + }) + + it('should parse a custom an error with an different arguments defined in more contracts coming from a nested contract', async () => { + const errorParser = createErrorParser() + + await fc.assert( + fc.asyncProperty(pointArbitrary, async (point) => { + const error = await assertFailed(contract.throwNestedWithCommonErrorAndNoArguments()) + const omniError: OmniError = { error, point } + const parsedError = await errorParser(omniError) + + expect(parsedError.point).toEqual(point) + expect(parsedError.error).toBeInstanceOf(CustomError) + expect(parsedError.error.reason).toEqual('CommonErrorWithNoArguments') + expect((parsedError.error as CustomError).args).toEqual([]) + }) + ) + }) + + it('should parse a custom an error with an different arguments defined in more contracts coming from a nested contract', async () => { + const errorParser = createErrorParser() + + await fc.assert( + fc.asyncProperty(pointArbitrary, fc.integer({ min: 0 }), async (point, arg) => { + const error = await assertFailed(contract.throwNestedWithCommonErrorAndArgument(arg)) + const omniError: OmniError = { error, point } + const parsedError = await errorParser(omniError) + + expect(parsedError.point).toEqual(point) + expect(parsedError.error).toBeInstanceOf(CustomError) + expect(parsedError.error.reason).toEqual('CommonErrorWithAnArgument') + expect((parsedError.error as CustomError).args).toEqual([BigNumber.from(arg)]) + }) + ) + }) + + it('should never reject', async () => { + const errorParser = createErrorParser() + + await fc.assert( + fc.asyncProperty(pointArbitrary, fc.anything(), async (point, error) => { + const omniError: OmniError = { error, point } + const parsedError = await errorParser(omniError) + + expect(parsedError.point).toEqual(point) + expect(parsedError.error).toBeInstanceOf(UnknownError) + expect(parsedError.error.reason).toBeUndefined() + expect(parsedError.error.message).toMatch(/Unknown error: /) + }), + // Test case for when toString method of the error is not defined + { + seed: 223418789, + path: '40:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:1:77:77', + endOnFailure: true, + } + ) + }) + }) +})