From 9ebd31e086ae9eb4735f6f3e5b842c5225476528 Mon Sep 17 00:00:00 2001 From: Luke Tchang Date: Wed, 12 May 2021 18:06:05 -0400 Subject: [PATCH] fix: remove proveAndProcess dependency on test ordering (#317) * fix: optics message sender and recipients determined in test/utils.js rather than in test file * fix: sender and mockRecipient init * fix: uses singleton object rather than awaiting promise explicitly * fix: rename MockRecipient test contract TestRecipient to avoid confusion w/ actual mocks --- .../{MockRecipient.sol => TestRecipient.sol} | 2 +- solidity/optics-core/test/Replica.test.js | 31 +++++++------------ .../test/cross-chain/GovernanceRouter.test.js | 12 +++---- solidity/optics-core/test/utils.js | 21 +++++++++++++ vectors/proveAndProcessTestCases.json | 2 +- 5 files changed, 41 insertions(+), 27 deletions(-) rename solidity/optics-core/contracts/test/{MockRecipient.sol => TestRecipient.sol} (93%) diff --git a/solidity/optics-core/contracts/test/MockRecipient.sol b/solidity/optics-core/contracts/test/TestRecipient.sol similarity index 93% rename from solidity/optics-core/contracts/test/MockRecipient.sol rename to solidity/optics-core/contracts/test/TestRecipient.sol index feeff522b4..b4e553bde4 100644 --- a/solidity/optics-core/contracts/test/MockRecipient.sol +++ b/solidity/optics-core/contracts/test/TestRecipient.sol @@ -3,7 +3,7 @@ pragma solidity >=0.6.11; import {IMessageRecipient} from "../../interfaces/IMessageRecipient.sol"; -contract MockRecipient is IMessageRecipient { +contract TestRecipient is IMessageRecipient { fallback() external { revert("Fallback"); } diff --git a/solidity/optics-core/test/Replica.test.js b/solidity/optics-core/test/Replica.test.js index aee967a0b3..1b44350481 100644 --- a/solidity/optics-core/test/Replica.test.js +++ b/solidity/optics-core/test/Replica.test.js @@ -1,9 +1,7 @@ const { waffle, ethers } = require('hardhat'); -const { provider, deployMockContract } = waffle; +const { provider } = waffle; const { expect } = require('chai'); - const testUtils = require('./utils'); -const MockRecipient = require('../artifacts/contracts/test/MockRecipient.sol/MockRecipient.json'); const { testCases: homeDomainHashTestCases, @@ -305,11 +303,8 @@ describe('Replica', async () => { }); it('Processes a proved message', async () => { - const [sender, recipient] = provider.getWallets(); - const mockRecipient = await deployMockContract( - recipient, - MockRecipient.abi, - ); + const sender = testUtils.opticsMessageSender; + const mockRecipient = await testUtils.opticsMessageMockRecipient.getRecipient(); const mockVal = '0x1234abcd'; await mockRecipient.mock.handle.returns(mockVal); @@ -420,11 +415,8 @@ describe('Replica', async () => { }); it('Returns false when processing message for bad handler function', async () => { - const [sender, recipient] = provider.getWallets(); - const mockRecipient = await deployMockContract( - recipient, - MockRecipient.abi, - ); + const sender = testUtils.opticsMessageSender; + const mockRecipient = await testUtils.opticsMessageMockRecipient.getRecipient(); // Recipient handler function reverts await mockRecipient.mock.handle.reverts(); @@ -448,11 +440,8 @@ describe('Replica', async () => { }); it('Proves and processes a message', async () => { - const [sender, recipient] = provider.getWallets(); - const mockRecipient = await deployMockContract( - recipient, - MockRecipient.abi, - ); + const sender = testUtils.opticsMessageSender; + const mockRecipient = await testUtils.opticsMessageMockRecipient.getRecipient(); const mockVal = '0x1234abcd'; await mockRecipient.mock.handle.returns(mockVal); @@ -475,7 +464,11 @@ describe('Replica', async () => { const messageLeaf = optics.messageToLeaf(opticsMessage); expect(messageLeaf).to.equal(leaf); - // Set replica's current root to match root given by proof + // Set replica's current root to match newly computed root that includes + // the new leaf (normally root will have already been computed and path + // simply verifies leaf is in tree but because it is cryptographically + // impossible to find the inputs that create a pre-determined root, we + // simply recalculate root with the leaf using branchRoot) const proofRoot = await replica.testBranchRoot(leaf, path, index); await replica.setCurrentRoot(proofRoot); diff --git a/solidity/optics-core/test/cross-chain/GovernanceRouter.test.js b/solidity/optics-core/test/cross-chain/GovernanceRouter.test.js index d7bc23c020..517b2d887d 100644 --- a/solidity/optics-core/test/cross-chain/GovernanceRouter.test.js +++ b/solidity/optics-core/test/cross-chain/GovernanceRouter.test.js @@ -227,14 +227,14 @@ describe('GovernanceRouter', async () => { it('Accepts valid call messages', async () => { // Create address for router to enroll and domain for router - const mockRecipient = await optics.deployImplementation('MockRecipient'); + const testRecipient = await optics.deployImplementation('TestRecipient'); - const MockRecipient = await ethers.getContractFactory('MockRecipient'); + const TestRecipient = await ethers.getContractFactory('TestRecipient'); const string = 'String!'; - const receiveStringFunction = MockRecipient.interface.getFunction( + const receiveStringFunction = TestRecipient.interface.getFunction( 'receiveString', ); - const receiveStringEncoded = MockRecipient.interface.encodeFunctionData( + const receiveStringEncoded = TestRecipient.interface.encodeFunctionData( receiveStringFunction, [string], ); @@ -243,12 +243,12 @@ describe('GovernanceRouter', async () => { ); const callData = { - to: optics.ethersAddressToBytes32(mockRecipient.address), + to: optics.ethersAddressToBytes32(testRecipient.address), dataLen: receiveStringEncodedLength, data: receiveStringEncoded, }; - // Create Call message to mockRecipient that calls receiveString + // Create Call message to test recipient that calls receiveString const callMessage = optics.GovernanceRouter.formatCalls([ callData, callData, diff --git a/solidity/optics-core/test/utils.js b/solidity/optics-core/test/utils.js index f613feeaf0..2d0c350343 100644 --- a/solidity/optics-core/test/utils.js +++ b/solidity/optics-core/test/utils.js @@ -1,3 +1,22 @@ +const { provider, deployMockContract } = waffle; +const TestRecipient = require('../artifacts/contracts/test/TestRecipient.sol/TestRecipient.json'); + +const [opticsMessageSender] = provider.getWallets(); + +class MockRecipientObject { + constructor() { + const [opticsMessageRecipient] = provider.getWallets(); + this.mockRecipient = deployMockContract( + opticsMessageRecipient, + TestRecipient.abi, + ); + } + + async getRecipient() { + return await this.mockRecipient; + } +} + const increaseTimestampBy = async (provider, increaseTime) => { await provider.send('evm_increaseTime', [increaseTime]); await provider.send('evm_mine'); @@ -16,6 +35,8 @@ function getUnusedSigner(provider, numUsedSigners) { const testUtils = { increaseTimestampBy, getUnusedSigner, + opticsMessageSender, + opticsMessageMockRecipient: new MockRecipientObject(), }; module.exports = testUtils; diff --git a/vectors/proveAndProcessTestCases.json b/vectors/proveAndProcessTestCases.json index 0ce83dddae..57e3f05248 100644 --- a/vectors/proveAndProcessTestCases.json +++ b/vectors/proveAndProcessTestCases.json @@ -1,7 +1,7 @@ { "testCases": [ { - "leaf": "0x157e45c45cd4178436a5d5671cbdb153742b7e8fbd0eb48e09989ee4c3dad8b3", + "leaf": "0x7cabd6c58bb5bb408f8ebac7ad1d2a8415ed7b345c71a5a86a52ff0aaedfe832", "index": 0, "path": [ "0x65ad6b7c39c687dad3edc05bec09300b742363f5c1f42db586bdce40c9fc5eef",