From 9bf721b69b30c801e9de50b756524d5b7b052da4 Mon Sep 17 00:00:00 2001 From: Antonio Morrone Date: Fri, 17 Feb 2023 15:50:26 +0100 Subject: [PATCH 1/8] feat(WIP): allow and remove tokens from nativeHolderSmartWallet --- tasks/allowTokens.ts | 28 +++++ tasks/deploy.ts | 20 +++- tasks/removeTokens.ts | 28 +++++ .../nativeHolderSmartWallet.test.ts | 101 ++++++++++++++++++ utils/scripts/types.ts | 2 +- 5 files changed, 177 insertions(+), 2 deletions(-) diff --git a/tasks/allowTokens.ts b/tasks/allowTokens.ts index 126f5439..9c51c539 100644 --- a/tasks/allowTokens.ts +++ b/tasks/allowTokens.ts @@ -39,7 +39,12 @@ export const allowTokens = async ( contractAddressesDeployed.CustomSmartWalletDeployVerifier; const customRelayVerifierAddress = contractAddressesDeployed.CustomSmartWalletRelayVerifier; + const nativeDeployVerifierAddress = + contractAddressesDeployed.NativeHolderSmartWalletDeployVerifier; + const nativeRelayVerifierAddress = + contractAddressesDeployed.NativeHolderSmartWalletRelayVerifier; + // TODO: This need to be refactored if (!deployVerifierAddress) { throw new Error('Could not obtain deploy verifier address'); } @@ -56,6 +61,18 @@ export const allowTokens = async ( throw new Error('Could not obtain custom deploy verifier address'); } + if (!nativeDeployVerifierAddress) { + throw new Error( + 'Could not obtain deploy verifier address for the NativeHolderSmartWallet' + ); + } + + if (!nativeRelayVerifierAddress) { + throw new Error( + 'Could not obtain relay verifier address for the NativeHolderSmartWallet' + ); + } + const deployVerifier = await ethers.getContractAt( 'DeployVerifier', deployVerifierAddress @@ -74,6 +91,15 @@ export const allowTokens = async ( customRelayVerifierAddress ); + const nativeHolderDeployVerifier = await ethers.getContractAt( + 'DeployVerifier', + nativeDeployVerifierAddress + ); + const nativeHolderRelayVerifier = await ethers.getContractAt( + 'RelayVerifier', + nativeRelayVerifierAddress + ); + const verifierMap: Map< string, { acceptToken: (tokenAddress: string) => Promise } @@ -82,6 +108,8 @@ export const allowTokens = async ( verifierMap.set('relayVerifier', relayVerifier); verifierMap.set('customDeployVerifier', customDeployVerifier); verifierMap.set('customRelayVerifier', customRelayVerifier); + verifierMap.set('nativeHolderDeployVerifier', nativeHolderDeployVerifier); + verifierMap.set('nativeHolderRelayVerifier', nativeHolderRelayVerifier); for (const tokenAddress of tokenAddresses) { for (const [key, verifier] of verifierMap) { diff --git a/tasks/deploy.ts b/tasks/deploy.ts index a720b25f..4bb25f21 100644 --- a/tasks/deploy.ts +++ b/tasks/deploy.ts @@ -71,6 +71,9 @@ export const deployContracts = async ( const customSmartWalletDeployVerifierF = await ethers.getContractFactory( 'CustomSmartWalletDeployVerifier' ); + const nativeHolderSmartWalletF = await ethers.getContractFactory( + 'NativeHolderSmartWallet' + ); const versionRegistryFactory = await ethers.getContractFactory( 'VersionRegistry' @@ -103,7 +106,18 @@ export const deployContracts = async ( customSmartWalletFactoryAddress ); const { address: customRelayVerifierAddress } = await relayVerifierF.deploy( - smartWalletFactoryAddress + customSmartWalletFactoryAddress + ); + + const { address: nativeHolderSmartWalletAddress } = + await nativeHolderSmartWalletF.deploy(); + const { address: nativeHolderSmartWalletFactoryAddress } = + await smartWalletFactoryF.deploy(nativeHolderSmartWalletAddress); + const { address: nativeDeployVerifierAddress } = await deployVerifierF.deploy( + nativeHolderSmartWalletFactoryAddress + ); + const { address: nativeRelayVerifierAddress } = await relayVerifierF.deploy( + nativeHolderSmartWalletFactoryAddress ); const { address: versionRegistryAddress } = @@ -126,6 +140,10 @@ export const deployContracts = async ( CustomSmartWalletFactory: customSmartWalletFactoryAddress, CustomSmartWalletDeployVerifier: customDeployVerifierAddress, CustomSmartWalletRelayVerifier: customRelayVerifierAddress, + NativeHolderSmartWallet: nativeHolderSmartWalletAddress, + NativeHolderSmartWalletFactory: nativeHolderSmartWalletFactoryAddress, + NativeHolderSmartWalletDeployVerifier: nativeDeployVerifierAddress, + NativeHolderSmartWalletRelayVerifier: nativeRelayVerifierAddress, UtilToken: utilTokenAddress, VersionRegistry: versionRegistryAddress, }; diff --git a/tasks/removeTokens.ts b/tasks/removeTokens.ts index ad0aed5f..28e4aa82 100644 --- a/tasks/removeTokens.ts +++ b/tasks/removeTokens.ts @@ -39,7 +39,12 @@ export const removeTokens = async ( contractAddressesDeployed.CustomSmartWalletDeployVerifier; const customRelayVerifierAddress = contractAddressesDeployed.CustomSmartWalletRelayVerifier; + const nativeDeployVerifierAddress = + contractAddressesDeployed.NativeHolderSmartWalletDeployVerifier; + const nativeRelayVerifierAddress = + contractAddressesDeployed.NativeHolderSmartWalletDeployVerifier; + // TODO: This need to be refactored if (!deployVerifierAddress) { throw new Error('Could not obtain deploy verifier address'); } @@ -56,6 +61,18 @@ export const removeTokens = async ( throw new Error('Could not obtain custom deploy verifier address'); } + if (!nativeDeployVerifierAddress) { + throw new Error( + 'Could not obtain deploy verifier address for the NativeHolderSmartWallet' + ); + } + + if (!nativeRelayVerifierAddress) { + throw new Error( + 'Could not obtain relay verifier address for the NativeHolderSmartWallet' + ); + } + const deployVerifier = await ethers.getContractAt( 'DeployVerifier', deployVerifierAddress @@ -73,6 +90,15 @@ export const removeTokens = async ( customRelayVerifierAddress ); + const nativeHolderDeployVerifier = await ethers.getContractAt( + 'DeployVerifier', + nativeDeployVerifierAddress + ); + const nativeHolderRelayVerifier = await ethers.getContractAt( + 'RelayVerifier', + nativeRelayVerifierAddress + ); + const verifierMap: Map< string, { @@ -88,6 +114,8 @@ export const removeTokens = async ( verifierMap.set('relayVerifier', relayVerifier); verifierMap.set('customDeployVerifier', customDeployVerifier); verifierMap.set('customRelayVerifier', customRelayVerifier); + verifierMap.set('nativeHolderDeployVerifier', nativeHolderDeployVerifier); + verifierMap.set('nativeHolderRelayVerifier', nativeHolderRelayVerifier); for (const tokenAddress of tokenAddresses) { for (const [key, verifier] of verifierMap) { diff --git a/test/smartwallet/nativeHolderSmartWallet.test.ts b/test/smartwallet/nativeHolderSmartWallet.test.ts index 8b8756d0..84d3eb52 100644 --- a/test/smartwallet/nativeHolderSmartWallet.test.ts +++ b/test/smartwallet/nativeHolderSmartWallet.test.ts @@ -1,3 +1,104 @@ +import { FakeContract, MockContract, smock } from '@defi-wonderland/smock'; +import { BaseProvider } from '@ethersproject/providers'; +import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'; +import chai, { expect } from 'chai'; +import chaiAsPromised from 'chai-as-promised'; +import { BigNumber, ethers, Wallet } from 'ethers'; +import { ethers as hardhat } from 'hardhat'; +import { + ERC20, + NativeHolderSmartWallet, + NativeHolderSmartWallet__factory, +} from '../../typechain-types'; + +chai.use(smock.matchers); +chai.use(chaiAsPromised); + +describe.only('SmartWallet contract', function () { + describe('Function directExecuteWithValue() mocked', function () { + let mockSmartWallet: MockContract; + let provider: BaseProvider; + let owner: Wallet; + let fakeToken: FakeContract; + let fundedAccount: SignerWithAddress; + + beforeEach(async function () { + [, fundedAccount] = (await hardhat.getSigners()) as [ + SignerWithAddress, + SignerWithAddress, + SignerWithAddress + ]; + + const mockSmartWalletFactory = + await smock.mock( + 'NativeHolderSmartWallet' + ); + + provider = hardhat.provider; + owner = hardhat.Wallet.createRandom().connect(provider); + + //Fund the owner + await fundedAccount.sendTransaction({ + to: owner.address, + value: hardhat.utils.parseEther('1'), + }); + mockSmartWallet = await mockSmartWalletFactory.connect(owner).deploy(); + + fakeToken = await smock.fake('ERC20'); + fakeToken.transfer.returns(true); + }); + + it('Should transfer native currency without executing a transaction', async function () { + const recipient = Wallet.createRandom(); + await fundedAccount.sendTransaction({ + to: mockSmartWallet.address, + value: hardhat.utils.parseEther('5'), + }); + + const recipientBalancePriorExecution = await provider.getBalance( + recipient.address + ); + const swBalancePriorExecution = await provider.getBalance( + mockSmartWallet.address + ); + + const value = ethers.utils.parseEther('2'); + + await expect( + mockSmartWallet.directExecuteWithValue( + recipient.address, + value, + '0x00' + ), + 'Execution failed' + ).not.to.be.rejected; + + const recipientBalanceAfterExecution = await provider.getBalance( + recipient.address + ); + const swBalanceAfterExecution = await provider.getBalance( + mockSmartWallet.address + ); + + const valueBN = BigNumber.from(value); + + const expectedRecipientBalance = BigNumber.from( + recipientBalancePriorExecution + ).add(valueBN); + const expectedSwBalance = BigNumber.from(swBalancePriorExecution).sub( + valueBN + ); + + expect(expectedRecipientBalance.toString()).to.be.equal( + recipientBalanceAfterExecution.toString() + ); + expect(expectedSwBalance.toString()).to.be.equal( + swBalanceAfterExecution.toString() + ); + }); + }); +}); + /* import { use, expect } from 'chai'; import chaiAsPromised from 'chai-as-promised'; import { BN } from 'ethereumjs-util'; diff --git a/utils/scripts/types.ts b/utils/scripts/types.ts index 179875e9..0dc9bef1 100644 --- a/utils/scripts/types.ts +++ b/utils/scripts/types.ts @@ -16,7 +16,7 @@ const factoryList = { : never ; export type ContractAddresses = { - [key in (ContractName | 'CustomSmartWalletRelayVerifier')]: string | undefined; + [key in (ContractName | 'CustomSmartWalletRelayVerifier' | 'NativeHolderSmartWalletFactory' | 'NativeHolderSmartWalletDeployVerifier' | 'NativeHolderSmartWalletRelayVerifier')]: string | undefined; }; export type NetworkConfig = { From 197e8c1672836430b146b4379366ba3d610afa6b Mon Sep 17 00:00:00 2001 From: Antonio Morrone Date: Mon, 20 Feb 2023 15:07:07 +0100 Subject: [PATCH 2/8] test: add missing unit tests for NativeHolderSmartWallet --- .../nativeHolderSmartWallet.test.ts | 815 ++++++++---------- 1 file changed, 352 insertions(+), 463 deletions(-) diff --git a/test/smartwallet/nativeHolderSmartWallet.test.ts b/test/smartwallet/nativeHolderSmartWallet.test.ts index 84d3eb52..3356cada 100644 --- a/test/smartwallet/nativeHolderSmartWallet.test.ts +++ b/test/smartwallet/nativeHolderSmartWallet.test.ts @@ -3,500 +3,389 @@ import { BaseProvider } from '@ethersproject/providers'; import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'; import chai, { expect } from 'chai'; import chaiAsPromised from 'chai-as-promised'; -import { BigNumber, ethers, Wallet } from 'ethers'; +import { + BaseContract, + BigNumber, + ContractTransaction, + ethers, + PayableOverrides, + Wallet, +} from 'ethers'; import { ethers as hardhat } from 'hardhat'; import { ERC20, NativeHolderSmartWallet, NativeHolderSmartWallet__factory, } from '../../typechain-types'; +import { PromiseOrValue } from '../../typechain-types/common'; +import { + getLocalEip712Signature, + TypedRequestData, +} from '../utils/EIP712Utils'; +import { + buildDomainSeparator, + createRequest, + getSuffixData, + HARDHAT_CHAIN_ID, +} from './utils'; chai.use(smock.matchers); chai.use(chaiAsPromised); -describe.only('SmartWallet contract', function () { - describe('Function directExecuteWithValue() mocked', function () { - let mockSmartWallet: MockContract; - let provider: BaseProvider; - let owner: Wallet; - let fakeToken: FakeContract; - let fundedAccount: SignerWithAddress; - - beforeEach(async function () { - [, fundedAccount] = (await hardhat.getSigners()) as [ - SignerWithAddress, - SignerWithAddress, - SignerWithAddress - ]; - - const mockSmartWalletFactory = - await smock.mock( - 'NativeHolderSmartWallet' - ); +const recipientABI = [ + { + inputs: [ + { + internalType: 'string', + name: 'message', + type: 'string', + }, + ], + name: 'emitMessage', + outputs: [ + { + internalType: 'string', + name: '', + type: 'string', + }, + ], + stateMutability: 'payable', + type: 'function', + }, +]; +interface TestRecipient extends BaseContract { + functions: { + emitMessage( + message: PromiseOrValue, + overrides?: PayableOverrides & { from?: PromiseOrValue } + ): Promise; + }; +} + +const INITIAL_SW_BALANCE = hardhat.utils.parseEther('5'); +const TWO_ETHERS = ethers.utils.parseEther('2'); + +describe('NativeHolderSmartWallet contract', function () { + let smartWallet: MockContract; + let provider: BaseProvider; + let owner: Wallet; + let fakeToken: FakeContract; + let fundedAccount: SignerWithAddress; + let relayHub: SignerWithAddress; + let worker: SignerWithAddress; + let privateKey: Buffer; + let recipientContract: FakeContract; + let encodedFunctionData: string; + + beforeEach(async function () { + [relayHub, fundedAccount, worker] = (await hardhat.getSigners()) as [ + SignerWithAddress, + SignerWithAddress, + SignerWithAddress + ]; + + const mockSmartWalletFactory = + await smock.mock( + 'NativeHolderSmartWallet' + ); + + provider = hardhat.provider; + owner = hardhat.Wallet.createRandom().connect(provider); + + //Fund the owner + await fundedAccount.sendTransaction({ + to: owner.address, + value: hardhat.utils.parseEther('1'), + }); + smartWallet = await mockSmartWalletFactory.connect(owner).deploy(); + const domainSeparator = buildDomainSeparator(smartWallet.address); + await smartWallet.setVariable('domainSeparator', domainSeparator); + + fakeToken = await smock.fake('ERC20'); + fakeToken.transfer.returns(true); + + privateKey = Buffer.from(owner.privateKey.substring(2, 66), 'hex'); + + recipientContract = await smock.fake(recipientABI); + encodedFunctionData = recipientContract.interface.encodeFunctionData( + 'emitMessage', + ['hello'] + ); + + await fundedAccount.sendTransaction({ + to: smartWallet.address, + value: INITIAL_SW_BALANCE, + }); + }); - provider = hardhat.provider; - owner = hardhat.Wallet.createRandom().connect(provider); + async function expectBalanceToBeRight( + execution: () => Promise, + recipientAddress: string, + expectedRecipientBalance: BigNumber, + expectedSwBalance: BigNumber + ) { + await execution(); - //Fund the owner - await fundedAccount.sendTransaction({ - to: owner.address, - value: hardhat.utils.parseEther('1'), + const recipientBalanceAfterExecution = await provider.getBalance( + recipientAddress + ); + const swBalanceAfterExecution = await provider.getBalance( + smartWallet.address + ); + + expect(expectedRecipientBalance.toString()).to.be.equal( + recipientBalanceAfterExecution.toString() + ); + expect(expectedSwBalance.toString()).to.be.equal( + swBalanceAfterExecution.toString() + ); + } + + async function expectBalanceAfterSuccessExecution( + recipientAddress: string, + value: BigNumber, + execution: () => Promise + ) { + const recipientBalancePriorExecution = await provider.getBalance( + recipientAddress + ); + const swBalancePriorExecution = await provider.getBalance( + smartWallet.address + ); + + const valueBN = BigNumber.from(value); + + const expectedRecipientBalance = BigNumber.from( + recipientBalancePriorExecution + ).add(valueBN); + const expectedSwBalance = BigNumber.from(swBalancePriorExecution).sub( + valueBN + ); + await expectBalanceToBeRight( + execution, + recipientAddress, + expectedRecipientBalance, + expectedSwBalance + ); + } + + describe('execute', function () { + type PrepareRequestParams = { + data: string; + value?: string; + }; + + function prepareRequest({ + data = '0x', + value = TWO_ETHERS.toString(), + }: PrepareRequestParams) { + const relayRequest = createRequest( + { + relayHub: relayHub.address, + from: owner.address, + to: recipientContract.address, + tokenAmount: '10', + tokenGas: '40000', + tokenContract: fakeToken.address, + data, + value + }, + { + callForwarder: smartWallet.address, + } + ); + + const typedRequestData = new TypedRequestData( + HARDHAT_CHAIN_ID, + smartWallet.address, + relayRequest + ); + const signature = getLocalEip712Signature(typedRequestData, privateKey); + const suffixData = getSuffixData(typedRequestData); + + return { + relayRequest, + suffixData, + signature, + }; + } + + it('Should transfer native currency without executing transactions', async function () { + const { relayRequest, suffixData, signature } = prepareRequest({ + data: '0x', + value: TWO_ETHERS.toString(), }); - mockSmartWallet = await mockSmartWalletFactory.connect(owner).deploy(); - fakeToken = await smock.fake('ERC20'); - fakeToken.transfer.returns(true); + const execution = async () => { + await expect( + smartWallet + .connect(relayHub) + .execute( + suffixData, + relayRequest.request, + worker.address, + signature + ) + ).not.to.be.rejected; + + expect(fakeToken.transfer, 'Token.transfer() was not called').to.be + .called; + }; + + await expectBalanceAfterSuccessExecution( + recipientContract.address, + TWO_ETHERS, + execution + ); }); - it('Should transfer native currency without executing a transaction', async function () { - const recipient = Wallet.createRandom(); - await fundedAccount.sendTransaction({ - to: mockSmartWallet.address, - value: hardhat.utils.parseEther('5'), + it('Should transfer native currency and execute transaction', async function () { + const { relayRequest, suffixData, signature } = prepareRequest({ + data: encodedFunctionData, }); + const execution = async () => { + await expect( + smartWallet + .connect(relayHub) + .execute( + suffixData, + relayRequest.request, + worker.address, + signature + ) + ).not.to.be.rejected; + + expect(fakeToken.transfer, 'Token.transfer() was not called').to.be + .called; + expect(recipientContract.emitMessage, 'Recipient method was not called') + .to.be.called; + }; + + await expectBalanceAfterSuccessExecution( + recipientContract.address, + TWO_ETHERS, + execution + ); + }); + + it('Should fail if the smart wallet cannot pay native currency', async function () { + // we try to transfer a value higher than the SW balance + const valueToTransfer = INITIAL_SW_BALANCE.add( + ethers.utils.parseEther('1') + ).toString(); + const { relayRequest, suffixData, signature } = prepareRequest({ + data: encodedFunctionData, + value: valueToTransfer.toString(), + }); + + const execution = async function () { + // Use the static call to check the returned values + const tx = await smartWallet + .connect(relayHub) + .callStatic.execute( + suffixData, + relayRequest.request, + worker.address, + signature + ); + expect(tx.success, 'Success is true').to.be.false; + + await expect( + smartWallet + .connect(relayHub) + .execute( + suffixData, + relayRequest.request, + worker.address, + signature + ), + 'Execute transaction rejected' + ).not.to.be.rejected; + }; + const recipientBalancePriorExecution = await provider.getBalance( - recipient.address + recipientContract.address ); const swBalancePriorExecution = await provider.getBalance( - mockSmartWallet.address + smartWallet.address ); - const value = ethers.utils.parseEther('2'); + await expectBalanceToBeRight(execution, recipientContract.address, recipientBalancePriorExecution, swBalancePriorExecution); + }); + }); - await expect( - mockSmartWallet.directExecuteWithValue( - recipient.address, - value, - '0x00' - ), - 'Execution failed' - ).not.to.be.rejected; - - const recipientBalanceAfterExecution = await provider.getBalance( - recipient.address - ); - const swBalanceAfterExecution = await provider.getBalance( - mockSmartWallet.address + describe('Function directExecuteWithValue()', function () { + + it('Should transfer native currency without executing transactions', async function () { + const execution = async () => { + await expect( + smartWallet.directExecuteWithValue( + recipientContract.address, + TWO_ETHERS, + '0x00' + ), + 'Execution failed' + ).not.to.be.rejected; + }; + await expectBalanceAfterSuccessExecution( + recipientContract.address, + TWO_ETHERS, + execution ); + }); - const valueBN = BigNumber.from(value); - - const expectedRecipientBalance = BigNumber.from( - recipientBalancePriorExecution - ).add(valueBN); - const expectedSwBalance = BigNumber.from(swBalancePriorExecution).sub( - valueBN + it('Should transfer native currency and execute transactions', async function () { + const value = ethers.utils.parseEther('2'); + const execution = async () => { + await expect( + smartWallet.directExecuteWithValue( + recipientContract.address, + value, + encodedFunctionData + ), + 'Execution failed' + ).not.to.be.rejected; + }; + await expectBalanceAfterSuccessExecution( + recipientContract.address, + value, + execution ); + }); - expect(expectedRecipientBalance.toString()).to.be.equal( - recipientBalanceAfterExecution.toString() + it('Should fail if the smart wallet cannot pay native currency', async function () { + const recipientBalancePriorExecution = await provider.getBalance( + recipientContract.address ); - expect(expectedSwBalance.toString()).to.be.equal( - swBalanceAfterExecution.toString() + const swBalancePriorExecution = await provider.getBalance( + smartWallet.address ); - }); - }); -}); -/* import { use, expect } from 'chai'; -import chaiAsPromised from 'chai-as-promised'; -import { BN } from 'ethereumjs-util'; -import { toWei } from 'web3-utils'; -import { - NativeHolderSmartWalletInstance, - SmartWalletFactoryInstance, - TestRecipientInstance -} from '../../types/truffle-contracts'; -import { - createRequest, - createSmartWallet, - createSmartWalletFactory, - getGaslessAccount, - getTestingEnvironment, - signRequest -} from '../utils'; - -use(chaiAsPromised); - -const NativeSmartWallet = artifacts.require('NativeHolderSmartWallet'); -const TestRecipient = artifacts.require('TestRecipient'); - -contract('NativeHolderSmartWallet', ([worker, sender, fundedAccount]) => { - const senderPrivateKey: Buffer = Buffer.from( - '0c06818f82e04c564290b32ab86b25676731fc34e9a546108bf109194c8e3aae', - 'hex' - ); - describe('execute', () => { - let nativeSw: NativeHolderSmartWalletInstance; - let factory: SmartWalletFactoryInstance; - let chainId: number; - let recipientContract: TestRecipientInstance; - - beforeEach(async () => { - chainId = (await getTestingEnvironment()).chainId; - const nativeSwTemplate = await NativeSmartWallet.new(); - factory = await createSmartWalletFactory(nativeSwTemplate); - nativeSw = await createSmartWallet( - { - relayHub: worker, - ownerEOA: sender, - factory, - privKey: senderPrivateKey, - chainId, - smartWalletContractName: 'NativeHolderSmartWallet' - } - ); - recipientContract = await TestRecipient.new(); - }); - - it('Should transfer native currency without executing a transaction', async () => { - const initialNonce = await nativeSw.nonce(); - - await web3.eth.sendTransaction({ - from: fundedAccount, - to: nativeSw.address, - value: toWei('5', 'ether') - }); - - const recipient = await getGaslessAccount(); - - const recipientBalancePriorExecution = await web3.eth.getBalance( - recipient.address - ); - const swBalancePriorExecution = await web3.eth.getBalance( - nativeSw.address - ); - - const value = toWei('2', 'ether'); - - const relayRequest = createRequest( - { - data: '0x00', - to: recipient.address, - nonce: initialNonce.toString(), - relayHub: worker, - from: sender, - value - }, - { - callForwarder: nativeSw.address - } - ); - - const { signature, suffixData } = signRequest( - senderPrivateKey, - relayRequest, - chainId - ); - - await nativeSw.execute( - suffixData, - relayRequest.request, - worker, - signature, - { from: worker } - ); - - const recipientBalanceAfterExecution = await web3.eth.getBalance( - recipient.address - ); - const swBalanceAfterExecution = await web3.eth.getBalance( - nativeSw.address - ); - - const valueBN = new BN(value); - - const expectedRecipientBalance = new BN( - recipientBalancePriorExecution - ).add(valueBN); - const expectedSwBalance = new BN(swBalancePriorExecution).sub( - valueBN - ); - - expect(expectedRecipientBalance.toString()).to.be.equal( - recipientBalanceAfterExecution.toString() - ); - expect(expectedSwBalance.toString()).to.be.equal( - swBalanceAfterExecution.toString() - ); - }); - - it('Should transfer native currency and execute transaction', async () => { - const initialNonce = await nativeSw.nonce(); - - await web3.eth.sendTransaction({ - from: fundedAccount, - to: nativeSw.address, - value: toWei('5', 'ether') - }); - - const recipientBalancePriorExecution = await web3.eth.getBalance( - recipientContract.address - ); - const swBalancePriorExecution = await web3.eth.getBalance( - nativeSw.address - ); - - const value = toWei('2', 'ether'); - - const encodedData = recipientContract.contract.methods - .emitMessage('hello') - .encodeABI(); - - const relayRequest = createRequest( - { - data: encodedData, - to: recipientContract.address, - nonce: initialNonce.toString(), - relayHub: worker, - from: sender, - value - }, - { - callForwarder: nativeSw.address - } - ); - - const { signature, suffixData } = signRequest( - senderPrivateKey, - relayRequest, - chainId - ); - - await nativeSw.execute( - suffixData, - relayRequest.request, - worker, - signature, - { from: worker } - ); - - // @ts-ignore - const logs = await recipientContract.getPastEvents( - 'SampleRecipientEmitted' - ); - expect(logs.length).to.be.equal(1, 'TestRecipient should emit'); - - const recipientBalanceAfterExecution = await web3.eth.getBalance( - recipientContract.address - ); - const swBalanceAfterExecution = await web3.eth.getBalance( - nativeSw.address - ); - - const valueBN = new BN(value); - - const expectedRecipientBalance = new BN( - recipientBalancePriorExecution - ).add(valueBN); - const expectedSwBalance = new BN(swBalancePriorExecution).sub( - valueBN - ); - - expect(expectedRecipientBalance.toString()).to.be.equal( - recipientBalanceAfterExecution.toString() - ); - expect(expectedSwBalance.toString()).to.be.equal( - swBalanceAfterExecution.toString() - ); - }); - - it('Should fail if smart wallet cannot pay native currency', async () => { - const initialNonce = await nativeSw.nonce(); - - const value = toWei('2', 'ether'); - - const encodedData = recipientContract.contract.methods - .emitMessage('hello') - .encodeABI(); - - const relayRequest = createRequest( - { - data: encodedData, - to: recipientContract.address, - nonce: initialNonce.toString(), - relayHub: worker, - from: sender, - value - }, - { - callForwarder: nativeSw.address - } - ); - - const { signature, suffixData } = signRequest( - senderPrivateKey, - relayRequest, - chainId - ); - - await nativeSw.execute( - suffixData, - relayRequest.request, - worker, - signature, - { from: worker } - ); - - // @ts-ignore - const logs = await recipientContract.getPastEvents( - 'SampleRecipientEmitted' - ); - expect(logs.length).to.be.equal(0, 'TestRecipient should not emit'); - }); - }); + const valueToTransfer = INITIAL_SW_BALANCE.add( + ethers.utils.parseEther('1') + ); - describe('directExecuteWithValue', () => { - let nativeSw: NativeHolderSmartWalletInstance; - let factory: SmartWalletFactoryInstance; - let chainId: number; - - let recipientContract: TestRecipientInstance; - - beforeEach(async () => { - chainId = (await getTestingEnvironment()).chainId; - const nativeSwTemplate = await NativeSmartWallet.new(); - factory = await createSmartWalletFactory(nativeSwTemplate); - nativeSw = await createSmartWallet( - { - relayHub: worker, - ownerEOA: sender, - factory, - privKey: senderPrivateKey, - chainId, - smartWalletContractName: 'NativeHolderSmartWallet' - } - ); - recipientContract = await TestRecipient.new(); - }); - - it('Should transfer native currency without executing a transaction', async () => { - await web3.eth.sendTransaction({ - from: fundedAccount, - to: nativeSw.address, - value: toWei('5', 'ether') - }); - - const recipient = await getGaslessAccount(); - - const recipientBalancePriorExecution = await web3.eth.getBalance( - recipient.address - ); - const swBalancePriorExecution = await web3.eth.getBalance( - nativeSw.address - ); - - const value = toWei('2', 'ether'); - - await nativeSw.directExecuteWithValue( - recipient.address, - value, - '0x00', - { from: sender } - ); - - const recipientBalanceAfterExecution = await web3.eth.getBalance( - recipient.address - ); - const swBalanceAfterExecution = await web3.eth.getBalance( - nativeSw.address - ); - - const valueBN = new BN(value); - - const expectedRecipientBalance = new BN( - recipientBalancePriorExecution - ).add(valueBN); - const expectedSwBalance = new BN(swBalancePriorExecution).sub( - valueBN - ); - - expect(expectedRecipientBalance.toString()).to.be.equal( - recipientBalanceAfterExecution.toString() - ); - expect(expectedSwBalance.toString()).to.be.equal( - swBalanceAfterExecution.toString() - ); - }); - - it('Should transfer native currency and execute transaction', async () => { - await web3.eth.sendTransaction({ - from: fundedAccount, - to: nativeSw.address, - value: toWei('5', 'ether') - }); - - const recipientBalancePriorExecution = await web3.eth.getBalance( - recipientContract.address - ); - const swBalancePriorExecution = await web3.eth.getBalance( - nativeSw.address - ); - - const value = toWei('2', 'ether'); - - const encodedData = recipientContract.contract.methods - .emitMessage('hello') - .encodeABI(); - - await nativeSw.directExecuteWithValue( - recipientContract.address, - value, - encodedData, - { from: sender } - ); - - // @ts-ignore - const logs = await recipientContract.getPastEvents( - 'SampleRecipientEmitted' - ); - expect(logs.length).to.be.equal(1, 'TestRecipient should emit'); - - const recipientBalanceAfterExecution = await web3.eth.getBalance( - recipientContract.address - ); - const swBalanceAfterExecution = await web3.eth.getBalance( - nativeSw.address - ); - - const valueBN = new BN(value); - - const expectedRecipientBalance = new BN( - recipientBalancePriorExecution - ).add(valueBN); - const expectedSwBalance = new BN(swBalancePriorExecution).sub( - valueBN - ); - - expect(expectedRecipientBalance.toString()).to.be.equal( - recipientBalanceAfterExecution.toString() - ); - expect(expectedSwBalance.toString()).to.be.equal( - swBalanceAfterExecution.toString() - ); - }); - - it('Should fail if smart wallet cannot pay native currency', async () => { - const value = toWei('2', 'ether'); - - const encodedData = recipientContract.contract.methods - .emitMessage('hello') - .encodeABI(); - - await nativeSw.directExecuteWithValue( - recipientContract.address, - value, - encodedData, - { from: sender } - ); - - // @ts-ignore - const logs = await recipientContract.getPastEvents( - 'SampleRecipientEmitted' - ); - expect(logs.length).to.be.equal(0, 'TestRecipient should not emit'); - }); + const execution = async () => { + // Use the static call to check the returned values + const tx = await smartWallet.callStatic.directExecuteWithValue( + recipientContract.address, + valueToTransfer, + encodedFunctionData + ); + expect(tx.success, 'Success is true').to.be.false; + + await expect( + smartWallet.directExecuteWithValue( + recipientContract.address, + valueToTransfer, + encodedFunctionData + ), + 'Execution failed' + ).not.to.be.rejected; + } + + await expectBalanceToBeRight(execution, recipientContract.address, recipientBalancePriorExecution, swBalancePriorExecution) }); + }); }); - */ From d707bad6ca6cb850d171cc9ed893de8f53c5e49d Mon Sep 17 00:00:00 2001 From: Antonio Morrone Date: Mon, 20 Feb 2023 15:51:21 +0100 Subject: [PATCH 3/8] test: add missing unit tests for NativeHolderSmartWallet --- test/smartwallet/smartWallet.test.ts | 95 +++------------------------- test/smartwallet/utils.ts | 75 ++++++++++++++++++++++ 2 files changed, 85 insertions(+), 85 deletions(-) create mode 100644 test/smartwallet/utils.ts diff --git a/test/smartwallet/smartWallet.test.ts b/test/smartwallet/smartWallet.test.ts index 08b76451..d7e2fcc6 100644 --- a/test/smartwallet/smartWallet.test.ts +++ b/test/smartwallet/smartWallet.test.ts @@ -1,41 +1,29 @@ -import { ethers as hardhat } from 'hardhat'; +import { FakeContract, MockContract, smock } from '@defi-wonderland/smock'; +import { BaseProvider } from '@ethersproject/providers'; import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'; import chai, { expect } from 'chai'; -import { FakeContract, smock } from '@defi-wonderland/smock'; import chaiAsPromised from 'chai-as-promised'; -import { TypedDataUtils } from '@metamask/eth-sig-util'; -import { - getLocalEip712Signature, - TypedRequestData, - TypedDeployRequestData, - getLocalEip712DeploySignature, -} from '../utils/EIP712Utils'; -import { SignTypedDataVersion } from '@metamask/eth-sig-util'; import { Wallet } from 'ethers'; +import { ethers as hardhat } from 'hardhat'; import { - SmartWallet, - SmartWalletFactory, - ERC20, - SmartWallet__factory, + ERC20, SmartWallet, + SmartWalletFactory, SmartWallet__factory } from 'typechain-types'; -import { BaseProvider } from '@ethersproject/providers'; import { EnvelopingTypes, - IForwarder, + IForwarder } from 'typechain-types/contracts/RelayHub'; -import { MockContract } from '@defi-wonderland/smock'; import { createValidPersonalSignSignature } from '../utils/createValidPersonalSignSignature'; +import { + getLocalEip712DeploySignature, getLocalEip712Signature, TypedDeployRequestData, TypedRequestData +} from '../utils/EIP712Utils'; +import { buildDomainSeparator, createRequest, getSuffixData, HARDHAT_CHAIN_ID } from './utils'; chai.use(smock.matchers); chai.use(chaiAsPromised); const ZERO_ADDRESS = hardhat.constants.AddressZero; -const ONE_FIELD_IN_BYTES = 32; -const HARDHAT_CHAIN_ID = 31337; -type ForwardRequest = IForwarder.ForwardRequestStruct; -type RelayData = EnvelopingTypes.RelayDataStruct; -type RelayRequest = EnvelopingTypes.RelayRequestStruct; type DeployRequest = EnvelopingTypes.DeployRequestStruct; type DeployRequestInternal = IForwarder.DeployRequestStruct; @@ -46,44 +34,6 @@ describe('SmartWallet contract', function () { let relayHub: SignerWithAddress; let fakeToken: FakeContract; - function createRequest( - request: Partial, - relayData?: Partial - ): RelayRequest { - const baseRequest: RelayRequest = { - request: { - relayHub: ZERO_ADDRESS, - from: ZERO_ADDRESS, - to: ZERO_ADDRESS, - tokenContract: ZERO_ADDRESS, - value: '0', - gas: '10000', - nonce: '0', - tokenAmount: '0', - tokenGas: '50000', - validUntilTime: '0', - data: '0x', - }, - relayData: { - gasPrice: '1', - feesReceiver: ZERO_ADDRESS, - callForwarder: ZERO_ADDRESS, - callVerifier: ZERO_ADDRESS, - }, - }; - - return { - request: { - ...baseRequest.request, - ...request, - }, - relayData: { - ...baseRequest.relayData, - ...relayData, - }, - }; - } - function createDeployRequest( request: Partial, relayData?: Partial @@ -123,31 +73,6 @@ describe('SmartWallet contract', function () { }; } - function buildDomainSeparator(address: string) { - const domainSeparator = { - name: 'RSK Enveloping Transaction', - version: '2', - chainId: HARDHAT_CHAIN_ID, - verifyingContract: address, - }; - - return hardhat.utils._TypedDataEncoder.hashDomain(domainSeparator); - } - - function getSuffixData(typedRequestData: TypedRequestData): string { - const encoded = TypedDataUtils.encodeData( - typedRequestData.primaryType, - typedRequestData.message, - typedRequestData.types, - SignTypedDataVersion.V4 - ); - - const messageSize = Object.keys(typedRequestData.message).length; - - return ( - '0x' + encoded.slice(messageSize * ONE_FIELD_IN_BYTES).toString('hex') - ); - } async function createSmartWalletFactory(owner: Wallet) { const smartWalletTemplateFactory = await hardhat.getContractFactory( diff --git a/test/smartwallet/utils.ts b/test/smartwallet/utils.ts new file mode 100644 index 00000000..8847d6e8 --- /dev/null +++ b/test/smartwallet/utils.ts @@ -0,0 +1,75 @@ +import { TypedDataUtils, SignTypedDataVersion } from '@metamask/eth-sig-util'; +import { ethers } from 'hardhat'; +import { IForwarder } from '../../typechain-types'; +import { EnvelopingTypes } from '../../typechain-types/contracts/RelayHub'; +import { TypedRequestData } from '../utils/EIP712Utils'; + +const ZERO_ADDRESS = ethers.constants.AddressZero; +export const HARDHAT_CHAIN_ID = 31337; +const ONE_FIELD_IN_BYTES = 32; + +export type ForwardRequest = IForwarder.ForwardRequestStruct; +export type RelayRequest = EnvelopingTypes.RelayRequestStruct; +export type RelayData = EnvelopingTypes.RelayDataStruct; + +export function createRequest( + request: Partial, + relayData?: Partial +): RelayRequest { + const baseRequest: RelayRequest = { + request: { + relayHub: ZERO_ADDRESS, + from: ZERO_ADDRESS, + to: ZERO_ADDRESS, + tokenContract: ZERO_ADDRESS, + value: '0', + gas: '10000', + nonce: '0', + tokenAmount: '0', + tokenGas: '50000', + validUntilTime: '0', + data: '0x', + }, + relayData: { + gasPrice: '1', + feesReceiver: ZERO_ADDRESS, + callForwarder: ZERO_ADDRESS, + callVerifier: ZERO_ADDRESS, + }, + }; + + return { + request: { + ...baseRequest.request, + ...request, + }, + relayData: { + ...baseRequest.relayData, + ...relayData, + }, + }; +} + +export function getSuffixData(typedRequestData: TypedRequestData): string { + const encoded = TypedDataUtils.encodeData( + typedRequestData.primaryType, + typedRequestData.message, + typedRequestData.types, + SignTypedDataVersion.V4 + ); + + const messageSize = Object.keys(typedRequestData.message).length; + + return '0x' + encoded.slice(messageSize * ONE_FIELD_IN_BYTES).toString('hex'); +} + +export function buildDomainSeparator(address: string) { + const domainSeparator = { + name: 'RSK Enveloping Transaction', + version: '2', + chainId: HARDHAT_CHAIN_ID, + verifyingContract: address, + }; + + return ethers.utils._TypedDataEncoder.hashDomain(domainSeparator); +} From 0cc5206d035f9622560d0a0b3ea4d5315f555ad4 Mon Sep 17 00:00:00 2001 From: Antonio Morrone Date: Mon, 20 Feb 2023 16:04:47 +0100 Subject: [PATCH 4/8] refactor: avoid code duplication in tasks --- tasks/allowTokens.ts | 101 +++--------------------- tasks/deploy.ts | 14 +--- tasks/getAllowedTokens.ts | 75 +++--------------- tasks/removeTokens.ts | 100 +++--------------------- tasks/utils.ts | 115 ++++++++++++++++++++++++++++ test/tasks/allowTokens.test.ts | 26 +------ test/tasks/deploy.test.ts | 4 + test/tasks/getAllowedTokens.test.ts | 25 +----- test/tasks/removeTokens.test.ts | 26 +------ test/tasks/utils.ts | 35 +++++++++ 10 files changed, 191 insertions(+), 330 deletions(-) create mode 100644 test/tasks/utils.ts diff --git a/tasks/allowTokens.ts b/tasks/allowTokens.ts index 9c51c539..948384b5 100644 --- a/tasks/allowTokens.ts +++ b/tasks/allowTokens.ts @@ -1,6 +1,6 @@ import { ContractTransaction } from 'ethers'; import { HardhatRuntimeEnvironment } from 'hardhat/types'; -import { getExistingConfig } from './deploy'; +import { getVerifiers } from './utils'; export const allowTokens = async ( taskArgs: { tokenlist: string }, @@ -8,97 +8,14 @@ export const allowTokens = async ( ) => { const tokenAddresses = taskArgs.tokenlist.split(','); - const { ethers, network } = hre; - - if (!network) { - throw new Error('Unknown Network'); - } - - const { chainId } = network.config; - - if (!chainId) { - throw new Error('Unknown Chain Id'); - } - - const contractAddresses = getExistingConfig(); - - if (!contractAddresses) { - throw new Error('No contracts deployed'); - } - - const networkChainKey = `${network.name}.${chainId}`; - const contractAddressesDeployed = contractAddresses[networkChainKey]; - - if (!contractAddressesDeployed) { - throw new Error(`Contracts not deployed for chain ID ${chainId}`); - } - - const deployVerifierAddress = contractAddressesDeployed.DeployVerifier; - const relayVerifierAddress = contractAddressesDeployed.RelayVerifier; - const customDeployVerifierAddress = - contractAddressesDeployed.CustomSmartWalletDeployVerifier; - const customRelayVerifierAddress = - contractAddressesDeployed.CustomSmartWalletRelayVerifier; - const nativeDeployVerifierAddress = - contractAddressesDeployed.NativeHolderSmartWalletDeployVerifier; - const nativeRelayVerifierAddress = - contractAddressesDeployed.NativeHolderSmartWalletRelayVerifier; - - // TODO: This need to be refactored - if (!deployVerifierAddress) { - throw new Error('Could not obtain deploy verifier address'); - } - - if (!relayVerifierAddress) { - throw new Error('Could not obtain relay verifier address'); - } - - if (!customDeployVerifierAddress) { - throw new Error('Could not obtain custom deploy verifier address'); - } - - if (!customRelayVerifierAddress) { - throw new Error('Could not obtain custom deploy verifier address'); - } - - if (!nativeDeployVerifierAddress) { - throw new Error( - 'Could not obtain deploy verifier address for the NativeHolderSmartWallet' - ); - } - - if (!nativeRelayVerifierAddress) { - throw new Error( - 'Could not obtain relay verifier address for the NativeHolderSmartWallet' - ); - } - - const deployVerifier = await ethers.getContractAt( - 'DeployVerifier', - deployVerifierAddress - ); - const relayVerifier = await ethers.getContractAt( - 'RelayVerifier', - relayVerifierAddress - ); - const customDeployVerifier = await ethers.getContractAt( - 'CustomSmartWalletDeployVerifier', - customDeployVerifierAddress - ); - - const customRelayVerifier = await ethers.getContractAt( - 'RelayVerifier', - customRelayVerifierAddress - ); - - const nativeHolderDeployVerifier = await ethers.getContractAt( - 'DeployVerifier', - nativeDeployVerifierAddress - ); - const nativeHolderRelayVerifier = await ethers.getContractAt( - 'RelayVerifier', - nativeRelayVerifierAddress - ); + const { + deployVerifier, + relayVerifier, + customDeployVerifier, + customRelayVerifier, + nativeHolderDeployVerifier, + nativeHolderRelayVerifier, + } = await getVerifiers(hre); const verifierMap: Map< string, diff --git a/tasks/deploy.ts b/tasks/deploy.ts index 4bb25f21..8d52e018 100644 --- a/tasks/deploy.ts +++ b/tasks/deploy.ts @@ -1,22 +1,12 @@ import { HardhatEthersHelpers, HardhatRuntimeEnvironment } from 'hardhat/types'; import fs from 'node:fs'; import { ContractAddresses, NetworkConfig } from '../utils/scripts/types'; -import { parseJsonFile } from './utils'; +import { getExistingConfig } from './utils'; const ADDRESS_FILE = process.env['ADDRESS_FILE'] || 'contract-addresses.json'; export type AddressesConfig = { [key: string]: ContractAddresses }; -export const getExistingConfig = () => { - try { - return parseJsonFile(ADDRESS_FILE); - } catch (error) { - console.warn(error); - } - - return undefined; -}; - export const writeConfigToDisk = (config: NetworkConfig) => { fs.writeFileSync(ADDRESS_FILE, JSON.stringify(config)); console.log(`address file available at: ${ADDRESS_FILE}`); @@ -43,7 +33,7 @@ export const updateConfig = ( } return { - ...getExistingConfig(), + ...getExistingConfig(ADDRESS_FILE), [`${network}.${chainId}`]: contractAddresses, }; }; diff --git a/tasks/getAllowedTokens.ts b/tasks/getAllowedTokens.ts index 4680b714..f0bd043f 100644 --- a/tasks/getAllowedTokens.ts +++ b/tasks/getAllowedTokens.ts @@ -1,71 +1,16 @@ import { HardhatRuntimeEnvironment } from 'hardhat/types'; -import { getExistingConfig } from './deploy'; +import { getVerifiers } from './utils'; export const getAllowedTokens = async (hre: HardhatRuntimeEnvironment) => { - const { ethers, network } = hre; - if (!network) { - throw new Error('Unknown Network'); - } - - const { chainId } = network.config; - - if (!chainId) { - throw new Error('Unknown Chain Id'); - } - - const contractAddresses = getExistingConfig(); - - if (!contractAddresses) { - throw new Error('No contracts deployed'); - } - - const networkChainKey = `${network.name}.${chainId}`; - const contractAddressesDeployed = contractAddresses[networkChainKey]; - if (!contractAddressesDeployed) { - throw new Error(`Contracts not deployed for chain ID ${chainId}`); - } - - const deployVerifierAddress = contractAddressesDeployed.DeployVerifier; - const relayVerifierAddress = contractAddressesDeployed.RelayVerifier; - const customDeployVerifierAddress = - contractAddressesDeployed.CustomSmartWalletDeployVerifier; - const customRelayVerifierAddress = - contractAddressesDeployed.CustomSmartWalletRelayVerifier; - - if (!deployVerifierAddress) { - throw new Error('Could not obtain deploy verifier address'); - } - - if (!relayVerifierAddress) { - throw new Error('Could not obtain relay verifier address'); - } - - if (!customDeployVerifierAddress) { - throw new Error('Could not obtain custom deploy verifier address'); - } - - if (!customRelayVerifierAddress) { - throw new Error('Could not obtain custom deploy verifier address'); - } - - const deployVerifier = await ethers.getContractAt( - 'DeployVerifier', - deployVerifierAddress - ); - const relayVerifier = await ethers.getContractAt( - 'RelayVerifier', - relayVerifierAddress - ); - const customDeployVerifier = await ethers.getContractAt( - 'CustomSmartWalletDeployVerifier', - customDeployVerifierAddress - ); - - const customRelayVerifier = await ethers.getContractAt( - 'RelayVerifier', - customRelayVerifierAddress - ); + const { + deployVerifier, + relayVerifier, + customDeployVerifier, + customRelayVerifier, + nativeHolderDeployVerifier, + nativeHolderRelayVerifier, + } = await getVerifiers(hre); const verifierMap: Map< string, @@ -75,6 +20,8 @@ export const getAllowedTokens = async (hre: HardhatRuntimeEnvironment) => { verifierMap.set('relayVerifier', relayVerifier); verifierMap.set('customDeployVerifier', customDeployVerifier); verifierMap.set('customRelayVerifier', customRelayVerifier); + verifierMap.set('nativeHolderDeployVerifier', nativeHolderDeployVerifier); + verifierMap.set('nativeHolderRelayVerifier', nativeHolderRelayVerifier); for (const [key, verifier] of verifierMap) { try { diff --git a/tasks/removeTokens.ts b/tasks/removeTokens.ts index 28e4aa82..b19e2c30 100644 --- a/tasks/removeTokens.ts +++ b/tasks/removeTokens.ts @@ -1,6 +1,6 @@ import { ContractTransaction } from 'ethers'; import { HardhatRuntimeEnvironment } from 'hardhat/types'; -import { getExistingConfig } from './deploy'; +import { getVerifiers } from './utils'; export const removeTokens = async ( taskArgs: { tokenlist: string }, @@ -8,96 +8,14 @@ export const removeTokens = async ( ) => { const tokenAddresses = taskArgs.tokenlist.split(','); - const { ethers, network } = hre; - - if (!network) { - throw new Error('Unknown Network'); - } - - const { chainId } = network.config; - - if (!chainId) { - throw new Error('Unknown Chain Id'); - } - - const contractAddresses = getExistingConfig(); - - if (!contractAddresses) { - throw new Error('No contracts deployed'); - } - - const networkChainKey = `${network.name}.${chainId}`; - const contractAddressesDeployed = contractAddresses[networkChainKey]; - - if (!contractAddressesDeployed) { - throw new Error(`Contracts not deployed for chain ID ${chainId}`); - } - - const deployVerifierAddress = contractAddressesDeployed.DeployVerifier; - const relayVerifierAddress = contractAddressesDeployed.RelayVerifier; - const customDeployVerifierAddress = - contractAddressesDeployed.CustomSmartWalletDeployVerifier; - const customRelayVerifierAddress = - contractAddressesDeployed.CustomSmartWalletRelayVerifier; - const nativeDeployVerifierAddress = - contractAddressesDeployed.NativeHolderSmartWalletDeployVerifier; - const nativeRelayVerifierAddress = - contractAddressesDeployed.NativeHolderSmartWalletDeployVerifier; - - // TODO: This need to be refactored - if (!deployVerifierAddress) { - throw new Error('Could not obtain deploy verifier address'); - } - - if (!relayVerifierAddress) { - throw new Error('Could not obtain relay verifier address'); - } - - if (!customDeployVerifierAddress) { - throw new Error('Could not obtain custom deploy verifier address'); - } - - if (!customRelayVerifierAddress) { - throw new Error('Could not obtain custom deploy verifier address'); - } - - if (!nativeDeployVerifierAddress) { - throw new Error( - 'Could not obtain deploy verifier address for the NativeHolderSmartWallet' - ); - } - - if (!nativeRelayVerifierAddress) { - throw new Error( - 'Could not obtain relay verifier address for the NativeHolderSmartWallet' - ); - } - - const deployVerifier = await ethers.getContractAt( - 'DeployVerifier', - deployVerifierAddress - ); - const relayVerifier = await ethers.getContractAt( - 'RelayVerifier', - relayVerifierAddress - ); - const customDeployVerifier = await ethers.getContractAt( - 'CustomSmartWalletDeployVerifier', - customDeployVerifierAddress - ); - const customRelayVerifier = await ethers.getContractAt( - 'RelayVerifier', - customRelayVerifierAddress - ); - - const nativeHolderDeployVerifier = await ethers.getContractAt( - 'DeployVerifier', - nativeDeployVerifierAddress - ); - const nativeHolderRelayVerifier = await ethers.getContractAt( - 'RelayVerifier', - nativeRelayVerifierAddress - ); + const { + deployVerifier, + relayVerifier, + customDeployVerifier, + customRelayVerifier, + nativeHolderDeployVerifier, + nativeHolderRelayVerifier, + } = await getVerifiers(hre); const verifierMap: Map< string, diff --git a/tasks/utils.ts b/tasks/utils.ts index b6d07042..b274d39d 100644 --- a/tasks/utils.ts +++ b/tasks/utils.ts @@ -1,4 +1,7 @@ import fs from 'fs'; +import { HardhatRuntimeEnvironment } from 'hardhat/types'; +import { AddressesConfig } from './deploy'; + // TODO: we may convert this function to return a promise export const parseJsonFile = (filePath: string) => { @@ -7,3 +10,115 @@ export const parseJsonFile = (filePath: string) => { } throw new Error(`The file ${filePath} doesn't exist`); }; + + +export const getExistingConfig = (addressFile: string): AddressesConfig | undefined => { + try { + return parseJsonFile(addressFile); + } catch (error) { + console.warn(error); + } + + return undefined; +}; + +export async function getVerifiers(hre: HardhatRuntimeEnvironment) { + const { ethers, network } = hre; + + if (!network) { + throw new Error('Unknown Network'); + } + + const { chainId } = network.config; + + if (!chainId) { + throw new Error('Unknown Chain Id'); + } + + const contractAddresses = getExistingConfig(); + + if (!contractAddresses) { + throw new Error('No contracts deployed'); + } + + const networkChainKey = `${network.name}.${chainId}`; + const contractAddressesDeployed = contractAddresses[networkChainKey]; + if (!contractAddressesDeployed) { + throw new Error(`Contracts not deployed for chain ID ${chainId}`); + } + + const deployVerifierAddress = contractAddressesDeployed.DeployVerifier; + const relayVerifierAddress = contractAddressesDeployed.RelayVerifier; + const customDeployVerifierAddress = + contractAddressesDeployed.CustomSmartWalletDeployVerifier; + const customRelayVerifierAddress = + contractAddressesDeployed.CustomSmartWalletRelayVerifier; + const nativeDeployVerifierAddress = + contractAddressesDeployed.NativeHolderSmartWalletDeployVerifier; + const nativeRelayVerifierAddress = + contractAddressesDeployed.NativeHolderSmartWalletRelayVerifier; + + if (!deployVerifierAddress) { + throw new Error('Could not obtain deploy verifier address'); + } + + if (!relayVerifierAddress) { + throw new Error('Could not obtain relay verifier address'); + } + + if (!customDeployVerifierAddress) { + throw new Error('Could not obtain custom deploy verifier address'); + } + + if (!customRelayVerifierAddress) { + throw new Error('Could not obtain custom deploy verifier address'); + } + + if (!nativeDeployVerifierAddress) { + throw new Error( + 'Could not obtain native deploy verifier address for the NativeHolderSmartWallet' + ); + } + + if (!nativeRelayVerifierAddress) { + throw new Error( + 'Could not obtain native relay verifier address for the NativeHolderSmartWallet' + ); + } + + const deployVerifier = await ethers.getContractAt( + 'DeployVerifier', + deployVerifierAddress + ); + const relayVerifier = await ethers.getContractAt( + 'RelayVerifier', + relayVerifierAddress + ); + const customDeployVerifier = await ethers.getContractAt( + 'CustomSmartWalletDeployVerifier', + customDeployVerifierAddress + ); + + const customRelayVerifier = await ethers.getContractAt( + 'RelayVerifier', + customRelayVerifierAddress + ); + + const nativeHolderDeployVerifier = await ethers.getContractAt( + 'DeployVerifier', + nativeDeployVerifierAddress + ); + const nativeHolderRelayVerifier = await ethers.getContractAt( + 'RelayVerifier', + nativeRelayVerifierAddress + ); + + return { + deployVerifier, + relayVerifier, + customDeployVerifier, + customRelayVerifier, + nativeHolderDeployVerifier, + nativeHolderRelayVerifier, + }; +} diff --git a/test/tasks/allowTokens.test.ts b/test/tasks/allowTokens.test.ts index 5eebbd5e..343f7235 100644 --- a/test/tasks/allowTokens.test.ts +++ b/test/tasks/allowTokens.test.ts @@ -6,6 +6,7 @@ import * as hre from 'hardhat'; import { ethers } from 'hardhat'; import sinon from 'sinon'; import { allowTokens } from '../../tasks/allowTokens'; +import { stubReadFileSync } from './utils'; use(chaiAsPromised); @@ -15,32 +16,9 @@ describe('Allow Tokens Script', function () { tokenlist: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', }; - const contractAddresses = { - Penalizer: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - RelayHub: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - SmartWallet: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - SmartWalletFactory: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - DeployVerifier: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - RelayVerifier: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - CustomSmartWallet: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - CustomSmartWalletFactory: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - CustomSmartWalletDeployVerifier: - '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - CustomSmartWalletRelayVerifier: - '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - VersionRegistry: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - UtilToken: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - }; - - const chainContractAddresses = { - 'hardhat.33': contractAddresses, - }; - beforeEach(function () { sinon.stub(fs, 'existsSync').returns(true); - sinon - .stub(fs, 'readFileSync') - .returns(JSON.stringify(chainContractAddresses)); + stubReadFileSync() hre.network.config.chainId = 33; }); diff --git a/test/tasks/deploy.test.ts b/test/tasks/deploy.test.ts index 3270506a..e39e9610 100644 --- a/test/tasks/deploy.test.ts +++ b/test/tasks/deploy.test.ts @@ -37,6 +37,10 @@ describe('Deploy Script', function () { 'CustomSmartWalletFactory', 'CustomSmartWalletDeployVerifier', 'CustomSmartWalletRelayVerifier', + 'NativeHolderSmartWallet', + 'NativeHolderSmartWalletFactory', + 'NativeHolderSmartWalletDeployVerifier', + 'NativeHolderSmartWalletRelayVerifier', 'VersionRegistry', 'UtilToken' ); diff --git a/test/tasks/getAllowedTokens.test.ts b/test/tasks/getAllowedTokens.test.ts index f003685f..4b3bec85 100644 --- a/test/tasks/getAllowedTokens.test.ts +++ b/test/tasks/getAllowedTokens.test.ts @@ -6,37 +6,16 @@ import * as hre from 'hardhat'; import { ethers } from 'hardhat'; import sinon from 'sinon'; import { getAllowedTokens } from '../../tasks/getAllowedTokens'; +import { stubReadFileSync } from './utils'; use(chaiAsPromised); describe('Get Allowed Tokens Script', function () { describe('getAllowedTokens', function () { - const contractAddresses = { - Penalizer: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - RelayHub: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - SmartWallet: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - SmartWalletFactory: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - DeployVerifier: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - RelayVerifier: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - CustomSmartWallet: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - CustomSmartWalletFactory: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - CustomSmartWalletDeployVerifier: - '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - CustomSmartWalletRelayVerifier: - '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - VersionRegistry: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - UtilToken: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - }; - - const chainContractAddresses = { - 'hardhat.33': contractAddresses, - }; beforeEach(function () { sinon.stub(fs, 'existsSync').returns(true); - sinon - .stub(fs, 'readFileSync') - .returns(JSON.stringify(chainContractAddresses)); + stubReadFileSync() hre.network.config.chainId = 33; }); diff --git a/test/tasks/removeTokens.test.ts b/test/tasks/removeTokens.test.ts index f02da3a8..033426ed 100644 --- a/test/tasks/removeTokens.test.ts +++ b/test/tasks/removeTokens.test.ts @@ -6,6 +6,7 @@ import * as hre from 'hardhat'; import { ethers } from 'hardhat'; import sinon from 'sinon'; import { removeTokens } from '../../tasks/removeTokens'; +import { stubReadFileSync } from './utils'; use(chaiAsPromised); @@ -15,32 +16,9 @@ describe('Remove Tokens Script', function () { tokenlist: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', }; - const contractAddresses = { - Penalizer: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - RelayHub: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - SmartWallet: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - SmartWalletFactory: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - DeployVerifier: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - RelayVerifier: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - CustomSmartWallet: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - CustomSmartWalletFactory: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - CustomSmartWalletDeployVerifier: - '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - CustomSmartWalletRelayVerifier: - '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - VersionRegistry: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - UtilToken: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', - }; - - const chainContractAddresses = { - 'hardhat.33': contractAddresses, - }; - beforeEach(function () { sinon.stub(fs, 'existsSync').returns(true); - sinon - .stub(fs, 'readFileSync') - .returns(JSON.stringify(chainContractAddresses)); + stubReadFileSync(); hre.network.config.chainId = 33; }); diff --git a/test/tasks/utils.ts b/test/tasks/utils.ts new file mode 100644 index 00000000..840c7bc2 --- /dev/null +++ b/test/tasks/utils.ts @@ -0,0 +1,35 @@ +import fs from 'fs'; +import sinon from 'sinon'; + +const defaultContractAddresses = { + Penalizer: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + RelayHub: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + SmartWallet: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + SmartWalletFactory: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + DeployVerifier: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + RelayVerifier: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + CustomSmartWallet: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + CustomSmartWalletFactory: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + CustomSmartWalletDeployVerifier: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + CustomSmartWalletRelayVerifier: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + NativeHolderSmartWallet: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + NativeHolderSmartWalletFactory: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + NativeHolderSmartWalletDeployVerifier: + '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + NativeHolderSmartWalletRelayVerifier: + '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + VersionRegistry: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', + UtilToken: '0x145845fd06c85B7EA1AA2d030E1a747B3d8d15D7', +}; + +const defaultChainContractAddresses = { + 'hardhat.33': defaultContractAddresses, +}; + +export const stubReadFileSync = ( + chainContractAddresses = defaultChainContractAddresses +) => { + sinon + .stub(fs, 'readFileSync') + .returns(JSON.stringify(chainContractAddresses)); +}; From 7fa4a0481fe0dc484262f0de22a54409e655b459 Mon Sep 17 00:00:00 2001 From: Antonio Morrone Date: Mon, 20 Feb 2023 16:13:16 +0100 Subject: [PATCH 5/8] fix: fix getExistingConfig to use env variable ADDRESS_FILE --- tasks/utils.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tasks/utils.ts b/tasks/utils.ts index b274d39d..7ca9d4e2 100644 --- a/tasks/utils.ts +++ b/tasks/utils.ts @@ -11,10 +11,11 @@ export const parseJsonFile = (filePath: string) => { throw new Error(`The file ${filePath} doesn't exist`); }; +const ADDRESS_FILE = process.env['ADDRESS_FILE'] || 'contract-addresses.json'; -export const getExistingConfig = (addressFile: string): AddressesConfig | undefined => { +export const getExistingConfig = (addressFile?: string): AddressesConfig | undefined => { try { - return parseJsonFile(addressFile); + return parseJsonFile(addressFile||ADDRESS_FILE); } catch (error) { console.warn(error); } From 46c8abf17316fe59db1b354d1c953337b28639f5 Mon Sep 17 00:00:00 2001 From: Antonio Morrone Date: Mon, 20 Feb 2023 16:16:56 +0100 Subject: [PATCH 6/8] style: fix linting and format errors --- tasks/getAllowedTokens.ts | 1 - tasks/utils.ts | 7 +++--- .../nativeHolderSmartWallet.test.ts | 21 +++++++++++++----- test/smartwallet/smartWallet.test.ts | 22 ++++++++++++++----- test/tasks/allowTokens.test.ts | 2 +- test/tasks/getAllowedTokens.test.ts | 3 +-- 6 files changed, 37 insertions(+), 19 deletions(-) diff --git a/tasks/getAllowedTokens.ts b/tasks/getAllowedTokens.ts index f0bd043f..8bfcfe3d 100644 --- a/tasks/getAllowedTokens.ts +++ b/tasks/getAllowedTokens.ts @@ -2,7 +2,6 @@ import { HardhatRuntimeEnvironment } from 'hardhat/types'; import { getVerifiers } from './utils'; export const getAllowedTokens = async (hre: HardhatRuntimeEnvironment) => { - const { deployVerifier, relayVerifier, diff --git a/tasks/utils.ts b/tasks/utils.ts index 7ca9d4e2..ef0103dc 100644 --- a/tasks/utils.ts +++ b/tasks/utils.ts @@ -2,7 +2,6 @@ import fs from 'fs'; import { HardhatRuntimeEnvironment } from 'hardhat/types'; import { AddressesConfig } from './deploy'; - // TODO: we may convert this function to return a promise export const parseJsonFile = (filePath: string) => { if (fs.existsSync(filePath)) { @@ -13,9 +12,11 @@ export const parseJsonFile = (filePath: string) => { const ADDRESS_FILE = process.env['ADDRESS_FILE'] || 'contract-addresses.json'; -export const getExistingConfig = (addressFile?: string): AddressesConfig | undefined => { +export const getExistingConfig = ( + addressFile?: string +): AddressesConfig | undefined => { try { - return parseJsonFile(addressFile||ADDRESS_FILE); + return parseJsonFile(addressFile || ADDRESS_FILE); } catch (error) { console.warn(error); } diff --git a/test/smartwallet/nativeHolderSmartWallet.test.ts b/test/smartwallet/nativeHolderSmartWallet.test.ts index 3356cada..32b538c2 100644 --- a/test/smartwallet/nativeHolderSmartWallet.test.ts +++ b/test/smartwallet/nativeHolderSmartWallet.test.ts @@ -188,7 +188,7 @@ describe('NativeHolderSmartWallet contract', function () { tokenGas: '40000', tokenContract: fakeToken.address, data, - value + value, }, { callForwarder: smartWallet.address, @@ -311,12 +311,16 @@ describe('NativeHolderSmartWallet contract', function () { smartWallet.address ); - await expectBalanceToBeRight(execution, recipientContract.address, recipientBalancePriorExecution, swBalancePriorExecution); + await expectBalanceToBeRight( + execution, + recipientContract.address, + recipientBalancePriorExecution, + swBalancePriorExecution + ); }); }); describe('Function directExecuteWithValue()', function () { - it('Should transfer native currency without executing transactions', async function () { const execution = async () => { await expect( @@ -374,7 +378,7 @@ describe('NativeHolderSmartWallet contract', function () { encodedFunctionData ); expect(tx.success, 'Success is true').to.be.false; - + await expect( smartWallet.directExecuteWithValue( recipientContract.address, @@ -383,9 +387,14 @@ describe('NativeHolderSmartWallet contract', function () { ), 'Execution failed' ).not.to.be.rejected; - } + }; - await expectBalanceToBeRight(execution, recipientContract.address, recipientBalancePriorExecution, swBalancePriorExecution) + await expectBalanceToBeRight( + execution, + recipientContract.address, + recipientBalancePriorExecution, + swBalancePriorExecution + ); }); }); }); diff --git a/test/smartwallet/smartWallet.test.ts b/test/smartwallet/smartWallet.test.ts index d7e2fcc6..0a5d2eb3 100644 --- a/test/smartwallet/smartWallet.test.ts +++ b/test/smartwallet/smartWallet.test.ts @@ -6,18 +6,29 @@ import chaiAsPromised from 'chai-as-promised'; import { Wallet } from 'ethers'; import { ethers as hardhat } from 'hardhat'; import { - ERC20, SmartWallet, - SmartWalletFactory, SmartWallet__factory + ERC20, + SmartWallet, + SmartWalletFactory, + SmartWallet__factory, } from 'typechain-types'; import { EnvelopingTypes, - IForwarder + IForwarder, } from 'typechain-types/contracts/RelayHub'; import { createValidPersonalSignSignature } from '../utils/createValidPersonalSignSignature'; import { - getLocalEip712DeploySignature, getLocalEip712Signature, TypedDeployRequestData, TypedRequestData + getLocalEip712DeploySignature, + getLocalEip712Signature, + TypedDeployRequestData, + TypedRequestData, } from '../utils/EIP712Utils'; -import { buildDomainSeparator, createRequest, getSuffixData, HARDHAT_CHAIN_ID } from './utils'; +import { + buildDomainSeparator, + createRequest, + getSuffixData, + HARDHAT_CHAIN_ID, + RelayData, +} from './utils'; chai.use(smock.matchers); chai.use(chaiAsPromised); @@ -73,7 +84,6 @@ describe('SmartWallet contract', function () { }; } - async function createSmartWalletFactory(owner: Wallet) { const smartWalletTemplateFactory = await hardhat.getContractFactory( 'SmartWallet' diff --git a/test/tasks/allowTokens.test.ts b/test/tasks/allowTokens.test.ts index 343f7235..0a6ca4e6 100644 --- a/test/tasks/allowTokens.test.ts +++ b/test/tasks/allowTokens.test.ts @@ -18,7 +18,7 @@ describe('Allow Tokens Script', function () { beforeEach(function () { sinon.stub(fs, 'existsSync').returns(true); - stubReadFileSync() + stubReadFileSync(); hre.network.config.chainId = 33; }); diff --git a/test/tasks/getAllowedTokens.test.ts b/test/tasks/getAllowedTokens.test.ts index 4b3bec85..92213459 100644 --- a/test/tasks/getAllowedTokens.test.ts +++ b/test/tasks/getAllowedTokens.test.ts @@ -12,10 +12,9 @@ use(chaiAsPromised); describe('Get Allowed Tokens Script', function () { describe('getAllowedTokens', function () { - beforeEach(function () { sinon.stub(fs, 'existsSync').returns(true); - stubReadFileSync() + stubReadFileSync(); hre.network.config.chainId = 33; }); From d27e4f2f4af8d48eb1eedca63c986ae674f41fa1 Mon Sep 17 00:00:00 2001 From: Antonio Morrone Date: Mon, 20 Feb 2023 16:27:31 +0100 Subject: [PATCH 7/8] fix: rename directExecuteWithValue to directExecute --- .../smartwallet/NativeHolderSmartWallet.sol | 2 +- test/smartwallet/nativeHolderSmartWallet.test.ts | 16 +++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/contracts/smartwallet/NativeHolderSmartWallet.sol b/contracts/smartwallet/NativeHolderSmartWallet.sol index 8e95ac6c..172a8abe 100644 --- a/contracts/smartwallet/NativeHolderSmartWallet.sol +++ b/contracts/smartwallet/NativeHolderSmartWallet.sol @@ -8,7 +8,7 @@ import "./SmartWallet.sol"; /* solhint-disable avoid-low-level-calls */ contract NativeHolderSmartWallet is SmartWallet { - function directExecuteWithValue( + function directExecute( address to, uint256 value, bytes calldata data diff --git a/test/smartwallet/nativeHolderSmartWallet.test.ts b/test/smartwallet/nativeHolderSmartWallet.test.ts index 32b538c2..2f3b1292 100644 --- a/test/smartwallet/nativeHolderSmartWallet.test.ts +++ b/test/smartwallet/nativeHolderSmartWallet.test.ts @@ -320,11 +320,11 @@ describe('NativeHolderSmartWallet contract', function () { }); }); - describe('Function directExecuteWithValue()', function () { + describe('Function directExecute(address to,uint256 value,bytes calldata data)', function () { it('Should transfer native currency without executing transactions', async function () { const execution = async () => { await expect( - smartWallet.directExecuteWithValue( + smartWallet['directExecute(address,uint256,bytes)']( recipientContract.address, TWO_ETHERS, '0x00' @@ -343,7 +343,7 @@ describe('NativeHolderSmartWallet contract', function () { const value = ethers.utils.parseEther('2'); const execution = async () => { await expect( - smartWallet.directExecuteWithValue( + smartWallet['directExecute(address,uint256,bytes)']( recipientContract.address, value, encodedFunctionData @@ -372,15 +372,13 @@ describe('NativeHolderSmartWallet contract', function () { const execution = async () => { // Use the static call to check the returned values - const tx = await smartWallet.callStatic.directExecuteWithValue( - recipientContract.address, - valueToTransfer, - encodedFunctionData - ); + const tx = await smartWallet.callStatic[ + 'directExecute(address,uint256,bytes)' + ](recipientContract.address, valueToTransfer, encodedFunctionData); expect(tx.success, 'Success is true').to.be.false; await expect( - smartWallet.directExecuteWithValue( + smartWallet['directExecute(address,uint256,bytes)']( recipientContract.address, valueToTransfer, encodedFunctionData From 0996aafe896371bc588ddad668a6fc97ed70fff3 Mon Sep 17 00:00:00 2001 From: Antonio Morrone Date: Wed, 22 Feb 2023 13:28:13 +0100 Subject: [PATCH 8/8] test: apply PR suggestions --- .../nativeHolderSmartWallet.test.ts | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/test/smartwallet/nativeHolderSmartWallet.test.ts b/test/smartwallet/nativeHolderSmartWallet.test.ts index 2f3b1292..b700e358 100644 --- a/test/smartwallet/nativeHolderSmartWallet.test.ts +++ b/test/smartwallet/nativeHolderSmartWallet.test.ts @@ -64,6 +64,7 @@ interface TestRecipient extends BaseContract { const INITIAL_SW_BALANCE = hardhat.utils.parseEther('5'); const TWO_ETHERS = ethers.utils.parseEther('2'); +const EMIT_MESSAGE_ARGUMENT = 'hello'; describe('NativeHolderSmartWallet contract', function () { let smartWallet: MockContract; @@ -109,7 +110,7 @@ describe('NativeHolderSmartWallet contract', function () { recipientContract = await smock.fake(recipientABI); encodedFunctionData = recipientContract.interface.encodeFunctionData( 'emitMessage', - ['hello'] + [EMIT_MESSAGE_ARGUMENT] ); await fundedAccount.sendTransaction({ @@ -143,7 +144,7 @@ describe('NativeHolderSmartWallet contract', function () { async function expectBalanceAfterSuccessExecution( recipientAddress: string, - value: BigNumber, + transferValue: BigNumber, execution: () => Promise ) { const recipientBalancePriorExecution = await provider.getBalance( @@ -153,14 +154,9 @@ describe('NativeHolderSmartWallet contract', function () { smartWallet.address ); - const valueBN = BigNumber.from(value); - - const expectedRecipientBalance = BigNumber.from( - recipientBalancePriorExecution - ).add(valueBN); - const expectedSwBalance = BigNumber.from(swBalancePriorExecution).sub( - valueBN - ); + const expectedRecipientBalance = + recipientBalancePriorExecution.add(transferValue); + const expectedSwBalance = swBalancePriorExecution.sub(transferValue); await expectBalanceToBeRight( execution, recipientAddress, @@ -258,8 +254,10 @@ describe('NativeHolderSmartWallet contract', function () { expect(fakeToken.transfer, 'Token.transfer() was not called').to.be .called; - expect(recipientContract.emitMessage, 'Recipient method was not called') - .to.be.called; + expect( + recipientContract.emitMessage, + 'Recipient method was not called' + ).to.be.calledWith(EMIT_MESSAGE_ARGUMENT); }; await expectBalanceAfterSuccessExecution( @@ -350,6 +348,11 @@ describe('NativeHolderSmartWallet contract', function () { ), 'Execution failed' ).not.to.be.rejected; + + expect( + recipientContract.emitMessage, + 'Recipient method was not called' + ).to.have.been.calledWith(EMIT_MESSAGE_ARGUMENT); }; await expectBalanceAfterSuccessExecution( recipientContract.address,