From ee435f93bf03ce1dcc7db8b50bafe3a61d125c0f Mon Sep 17 00:00:00 2001 From: Maurelian Date: Wed, 19 Jan 2022 14:22:25 -0500 Subject: [PATCH 1/7] Add revert on failed call in Dummy contract --- .../contracts/test/DummyContract.sol | 9 +- .../contracts/test/L1/DepositFeed.spec.ts | 135 ++++++++++-------- 2 files changed, 84 insertions(+), 60 deletions(-) diff --git a/packages/contracts/contracts/test/DummyContract.sol b/packages/contracts/contracts/test/DummyContract.sol index af6c38bd..8d8aadfe 100644 --- a/packages/contracts/contracts/test/DummyContract.sol +++ b/packages/contracts/contracts/test/DummyContract.sol @@ -5,15 +5,18 @@ pragma solidity ^0.8.10; * For use in testing with a call from a contract rather than an EOA. */ contract Dummy { + error Failed(); + /** * Forwards a call. * @param _target Address to call * @param _data Data to forward */ function forward(address _target, bytes calldata _data) external payable { - (bool success, bytes memory ret) = _target.call{ value: msg.value }(_data); + (bool success, ) = _target.call{ value: msg.value }(_data); // Silence the 'Return value of low-level calls not used' warning. - success; - ret; + if (!success) { + revert Failed(); + } } } diff --git a/packages/contracts/test/L1/DepositFeed.spec.ts b/packages/contracts/test/L1/DepositFeed.spec.ts index 665fce2d..d3dd7122 100644 --- a/packages/contracts/test/L1/DepositFeed.spec.ts +++ b/packages/contracts/test/L1/DepositFeed.spec.ts @@ -26,6 +26,8 @@ const decodeDepositEvent = async ( const events = await depositFeed.queryFilter( depositFeed.filters.TransactionDeposited() ) + console.log(events.length); + const eventArgs = events[events.length - 1].args return { @@ -63,13 +65,16 @@ describe('DepositFeed', () => { describe('Should emit the correct log values...', async () => { it('when an EOA deposits a transaction with 0 value.', async () => { - await depositFeed.depositTransaction( - ZERO_ADDRESS, - ZERO_BIGNUMBER, - NON_ZERO_GASLIMIT, - false, - NON_ZERO_DATA - ) + const receipt = await ( + await depositFeed.depositTransaction( + ZERO_ADDRESS, + ZERO_BIGNUMBER, + NON_ZERO_GASLIMIT, + false, + NON_ZERO_DATA + ) + ).wait() + await expect(receipt.status).to.equal(1) const eventArgs = await decodeDepositEvent(depositFeed) @@ -85,13 +90,16 @@ describe('DepositFeed', () => { }) it('when an EOA deposits a contract creation with 0 value.', async () => { - await depositFeed.depositTransaction( - ZERO_ADDRESS, - ZERO_BIGNUMBER, - NON_ZERO_GASLIMIT, - true, - NON_ZERO_DATA - ) + const receipt = await ( + await depositFeed.depositTransaction( + ZERO_ADDRESS, + ZERO_BIGNUMBER, + NON_ZERO_GASLIMIT, + true, + NON_ZERO_DATA + ) + ).wait() + await expect(receipt.status).to.equal(1) const eventArgs = await decodeDepositEvent(depositFeed) @@ -111,16 +119,19 @@ describe('DepositFeed', () => { const dummy = await (await ethers.getContractFactory('Dummy')).deploy() await dummy.deployed() - await dummy.forward( - depositFeed.address, - depositFeed.interface.encodeFunctionData('depositTransaction', [ - ZERO_ADDRESS, - ZERO_BIGNUMBER, - NON_ZERO_GASLIMIT, - true, - NON_ZERO_DATA, - ]) - ) + const receipt = await ( + await dummy.forward( + depositFeed.address, + depositFeed.interface.encodeFunctionData('depositTransaction', [ + ZERO_ADDRESS, + ZERO_BIGNUMBER, + NON_ZERO_GASLIMIT, + true, + NON_ZERO_DATA, + ]) + ) + ).wait() + await expect(receipt.status).to.equal(1) const eventArgs = await decodeDepositEvent(depositFeed) @@ -138,16 +149,20 @@ describe('DepositFeed', () => { describe('and increase its eth balance...', async () => { it('when an EOA deposits a transaction with an ETH value.', async () => { const balBefore = await ethers.provider.getBalance(depositFeed.address) - await depositFeed.depositTransaction( - NON_ZERO_ADDRESS, - ZERO_BIGNUMBER, - NON_ZERO_GASLIMIT, - false, - '0x', - { - value: NON_ZERO_VALUE, - } - ) + const receipt = await ( + await depositFeed.depositTransaction( + NON_ZERO_ADDRESS, + ZERO_BIGNUMBER, + NON_ZERO_GASLIMIT, + false, + '0x', + { + value: NON_ZERO_VALUE, + } + ) + ).wait() + await expect(receipt.status).to.equal(1) + const balAfter = await ethers.provider.getBalance(depositFeed.address) const eventArgs = await decodeDepositEvent(depositFeed) @@ -166,16 +181,19 @@ describe('DepositFeed', () => { it('when an EOA deposits a contract creation with an ETH value.', async () => { const balBefore = await ethers.provider.getBalance(depositFeed.address) - await depositFeed.depositTransaction( - ZERO_ADDRESS, - ZERO_BIGNUMBER, - NON_ZERO_GASLIMIT, - true, - '0x', - { - value: NON_ZERO_VALUE, - } - ) + const receipt = await ( + await depositFeed.depositTransaction( + ZERO_ADDRESS, + ZERO_BIGNUMBER, + NON_ZERO_GASLIMIT, + true, + '0x', + { + value: NON_ZERO_VALUE, + } + ) + ).wait() + await expect(receipt.status).to.equal(1) const balAfter = await ethers.provider.getBalance(depositFeed.address) const eventArgs = await decodeDepositEvent(depositFeed) @@ -198,19 +216,22 @@ describe('DepositFeed', () => { await dummy.deployed() const balBefore = await ethers.provider.getBalance(depositFeed.address) - await dummy.forward( - depositFeed.address, - depositFeed.interface.encodeFunctionData('depositTransaction', [ - ZERO_ADDRESS, - ZERO_BIGNUMBER, - NON_ZERO_GASLIMIT, - true, - NON_ZERO_DATA, - ]), - { - value: NON_ZERO_VALUE, - } - ) + const receipt = await ( + await dummy.forward( + depositFeed.address, + depositFeed.interface.encodeFunctionData('depositTransaction', [ + ZERO_ADDRESS, + ZERO_BIGNUMBER, + NON_ZERO_GASLIMIT, + true, + NON_ZERO_DATA, + ]), + { + value: NON_ZERO_VALUE, + } + ) + ).wait() + await expect(receipt.status).to.equal(1) const balAfter = await ethers.provider.getBalance(depositFeed.address) const eventArgs = await decodeDepositEvent(depositFeed) From 20a3ec9502d6e16c19ef08ddfea34b97f2d8f67a Mon Sep 17 00:00:00 2001 From: Maurelian Date: Wed, 19 Jan 2022 14:30:18 -0500 Subject: [PATCH 2/7] Add missing test cases --- .../contracts/test/L1/DepositFeed.spec.ts | 68 ++++++++++++++++++- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/packages/contracts/test/L1/DepositFeed.spec.ts b/packages/contracts/test/L1/DepositFeed.spec.ts index d3dd7122..edc6b2c4 100644 --- a/packages/contracts/test/L1/DepositFeed.spec.ts +++ b/packages/contracts/test/L1/DepositFeed.spec.ts @@ -26,7 +26,6 @@ const decodeDepositEvent = async ( const events = await depositFeed.queryFilter( depositFeed.filters.TransactionDeposited() ) - console.log(events.length); const eventArgs = events[events.length - 1].args @@ -65,7 +64,7 @@ describe('DepositFeed', () => { describe('Should emit the correct log values...', async () => { it('when an EOA deposits a transaction with 0 value.', async () => { - const receipt = await ( + const receipt = await( await depositFeed.depositTransaction( ZERO_ADDRESS, ZERO_BIGNUMBER, @@ -89,6 +88,37 @@ describe('DepositFeed', () => { }) }) + it('when a contract deposits a transaction with 0 value.', async () => { + // Deploy a dummy contract so we can impersonate it + const dummy = await (await ethers.getContractFactory('Dummy')).deploy() + await dummy.deployed() + + await expect( + dummy.forward( + depositFeed.address, + depositFeed.interface.encodeFunctionData('depositTransaction', [ + NON_ZERO_ADDRESS, + ZERO_BIGNUMBER, + NON_ZERO_GASLIMIT, + false, + NON_ZERO_DATA, + ]) + ) + ).to.not.be.reverted + + const eventArgs = await decodeDepositEvent(depositFeed) + + expect(eventArgs).to.deep.equal({ + from: applyL1ToL2Alias(dummy.address), + to: NON_ZERO_ADDRESS, + value: ZERO_BIGNUMBER, + mint: ZERO_BIGNUMBER, + gasLimit: NON_ZERO_GASLIMIT, + isCreation: false, + data: NON_ZERO_DATA, + }) + }) + it('when an EOA deposits a contract creation with 0 value.', async () => { const receipt = await ( await depositFeed.depositTransaction( @@ -179,6 +209,40 @@ describe('DepositFeed', () => { }) }) + it('when a contract deposits a transaction with an ETH value.', async () => { + // Deploy a dummy contract so we can impersonate it + const dummy = await (await ethers.getContractFactory('Dummy')).deploy() + await dummy.deployed() + // this is not emitting an event! + await expect( + dummy.forward( + depositFeed.address, + depositFeed.interface.encodeFunctionData('depositTransaction', [ + NON_ZERO_ADDRESS, + ZERO_BIGNUMBER, + NON_ZERO_GASLIMIT, + false, + NON_ZERO_DATA, + ]), + { + value: NON_ZERO_VALUE, + } + ) + ).to.not.be.reverted + + const eventArgs = await decodeDepositEvent(depositFeed) + + expect(eventArgs).to.deep.equal({ + from: applyL1ToL2Alias(dummy.address), + to: NON_ZERO_ADDRESS, + value: ZERO_BIGNUMBER, + mint: NON_ZERO_VALUE, + gasLimit: NON_ZERO_GASLIMIT, + isCreation: false, + data: NON_ZERO_DATA, + }) + }) + it('when an EOA deposits a contract creation with an ETH value.', async () => { const balBefore = await ethers.provider.getBalance(depositFeed.address) const receipt = await ( From 45b782b94c67a71bb44afd5fbbce6dc6dcd948a8 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Wed, 19 Jan 2022 14:33:16 -0500 Subject: [PATCH 3/7] Fix broken test --- packages/contracts/test/L1/DepositFeed.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts/test/L1/DepositFeed.spec.ts b/packages/contracts/test/L1/DepositFeed.spec.ts index edc6b2c4..ed325b84 100644 --- a/packages/contracts/test/L1/DepositFeed.spec.ts +++ b/packages/contracts/test/L1/DepositFeed.spec.ts @@ -66,7 +66,7 @@ describe('DepositFeed', () => { it('when an EOA deposits a transaction with 0 value.', async () => { const receipt = await( await depositFeed.depositTransaction( - ZERO_ADDRESS, + NON_ZERO_ADDRESS, ZERO_BIGNUMBER, NON_ZERO_GASLIMIT, false, @@ -79,7 +79,7 @@ describe('DepositFeed', () => { expect(eventArgs).to.deep.equal({ from: signerAddress, - to: ZERO_ADDRESS, + to: NON_ZERO_ADDRESS, mint: ZERO_BIGNUMBER, value: ZERO_BIGNUMBER, gasLimit: NON_ZERO_GASLIMIT, From a0f52018fde7a6c0e9fad2193e4496f9bd0aff5b Mon Sep 17 00:00:00 2001 From: Maurelian Date: Thu, 20 Jan 2022 10:04:59 -0500 Subject: [PATCH 4/7] Ignore .deps folder --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 8bba35cd..070c0468 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,5 @@ node_modules yarn-error.log coverage.out +# hardhat automatically puts console.sol in here when you use it +.deps From 55bf74ba0b9a48d536b98bdfff8c4d23d8a10b1d Mon Sep 17 00:00:00 2001 From: protolambda Date: Fri, 21 Jan 2022 19:15:05 +0100 Subject: [PATCH 5/7] deposit feed test: rm old comment --- packages/contracts/test/L1/DepositFeed.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/contracts/test/L1/DepositFeed.spec.ts b/packages/contracts/test/L1/DepositFeed.spec.ts index ed325b84..11a95f26 100644 --- a/packages/contracts/test/L1/DepositFeed.spec.ts +++ b/packages/contracts/test/L1/DepositFeed.spec.ts @@ -213,7 +213,6 @@ describe('DepositFeed', () => { // Deploy a dummy contract so we can impersonate it const dummy = await (await ethers.getContractFactory('Dummy')).deploy() await dummy.deployed() - // this is not emitting an event! await expect( dummy.forward( depositFeed.address, From 0fe5e2e7058fdb88718ceda14e87f44e6270f1c4 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Fri, 21 Jan 2022 13:47:22 -0500 Subject: [PATCH 6/7] Forward full balance from Dummy contract --- packages/contracts/contracts/test/DummyContract.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/contracts/contracts/test/DummyContract.sol b/packages/contracts/contracts/test/DummyContract.sol index 8d8aadfe..6ce8e40f 100644 --- a/packages/contracts/contracts/test/DummyContract.sol +++ b/packages/contracts/contracts/test/DummyContract.sol @@ -13,7 +13,8 @@ contract Dummy { * @param _data Data to forward */ function forward(address _target, bytes calldata _data) external payable { - (bool success, ) = _target.call{ value: msg.value }(_data); + uint256 amount = address(this).balance; + (bool success, ) = _target.call{ value: amount }(_data); // Silence the 'Return value of low-level calls not used' warning. if (!success) { revert Failed(); From 819b3688f0cc0adca94f4c14148b441c44950739 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Fri, 21 Jan 2022 13:47:45 -0500 Subject: [PATCH 7/7] Lint test script --- packages/contracts/test/L1/DepositFeed.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts/test/L1/DepositFeed.spec.ts b/packages/contracts/test/L1/DepositFeed.spec.ts index 11a95f26..f355d66d 100644 --- a/packages/contracts/test/L1/DepositFeed.spec.ts +++ b/packages/contracts/test/L1/DepositFeed.spec.ts @@ -64,7 +64,7 @@ describe('DepositFeed', () => { describe('Should emit the correct log values...', async () => { it('when an EOA deposits a transaction with 0 value.', async () => { - const receipt = await( + const receipt = await ( await depositFeed.depositTransaction( NON_ZERO_ADDRESS, ZERO_BIGNUMBER,