From 99bbaee6282ec9d7e6d853e43653d43eb68bf408 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Thu, 29 Feb 2024 14:00:51 +0100 Subject: [PATCH] test: addressing flakiness of `e2e_public_cross_chain_messaging` (#4853) The flakiness of the `e2e_public_cross_chain_messaging` was caused by me not waiting for the tx to be mined by anvil. Instead I just tried to fetch the logs immediately which then sometimes led to there being no logs even though exactly 1 log was expected. This PR fixes this by waiting for tx receipt. --- .../e2e_public_cross_chain_messaging.test.ts | 61 ++++++++++--------- .../src/integration_l1_publisher.test.ts | 32 +++++----- yarn-project/noir-contracts.js/package.json | 2 +- 3 files changed, 49 insertions(+), 46 deletions(-) diff --git a/yarn-project/end-to-end/src/e2e_public_cross_chain_messaging.test.ts b/yarn-project/end-to-end/src/e2e_public_cross_chain_messaging.test.ts index d415d7a3db9..8fc15d8de80 100644 --- a/yarn-project/end-to-end/src/e2e_public_cross_chain_messaging.test.ts +++ b/yarn-project/end-to-end/src/e2e_public_cross_chain_messaging.test.ts @@ -18,7 +18,7 @@ import { TokenContract } from '@aztec/noir-contracts.js/Token'; import { TokenBridgeContract } from '@aztec/noir-contracts.js/TokenBridge'; import { Hex } from 'viem'; -import { getAbiItem, getAddress } from 'viem/utils'; +import { decodeEventLog } from 'viem/utils'; import { publicDeployAccounts, setup } from './fixtures/utils.js'; import { CrossChainTestHarness } from './shared/cross_chain_test_harness.js'; @@ -235,24 +235,25 @@ describe('e2e_public_cross_chain_messaging', () => { }; const txHash = await outbox.write.consume([l2ToL1Message] as const, {} as any); - - const abiItem = getAbiItem({ - abi: OutboxAbi, - name: 'MessageConsumed', + const txReceipt = await crossChainTestHarness.publicClient.waitForTransactionReceipt({ + hash: txHash, }); - const events = await crossChainTestHarness.publicClient.getLogs({ - address: getAddress(outbox.address.toString()), - event: abiItem, - fromBlock: 0n, - }); + // Exactly 1 event should be emitted in the transaction + expect(txReceipt.logs.length).toBe(1); - // We get the event just for the relevant transaction - const txEvents = events.filter(event => event.transactionHash === txHash); + // We decode the event log before checking it + const txLog = txReceipt.logs[0]; + const topics = decodeEventLog({ + abi: OutboxAbi, + data: txLog.data, + topics: txLog.topics, + }); - // We check that exactly 1 MessageConsumed event was emitted with the expected recipient - expect(txEvents.length).toBe(1); - expect(txEvents[0].args.recipient).toBe(recipient.toChecksumString()); + // We check that MessageConsumed event was emitted with the expected recipient + // Note: For whatever reason, viem types "think" that there is no recipient on topics.args. I hack around this + // by casting the args to "any" + expect((topics.args as any).recipient).toBe(recipient.toChecksumString()); }, 60_000, ); @@ -290,24 +291,28 @@ describe('e2e_public_cross_chain_messaging', () => { // for later use let msgKey!: Fr; { - const events = await crossChainTestHarness.publicClient.getLogs({ - address: getAddress(inbox.address.toString()), - event: getAbiItem({ - abi: InboxAbi, - name: 'MessageAdded', - }), - fromBlock: 0n, + const txReceipt = await crossChainTestHarness.publicClient.waitForTransactionReceipt({ + hash: txHash, }); - // We get the event just for the relevant transaction - const txEvents = events.filter(event => event.transactionHash === txHash); + // Exactly 1 event should be emitted in the transaction + expect(txReceipt.logs.length).toBe(1); + + // We decode the event log before checking it + const txLog = txReceipt.logs[0]; + const topics = decodeEventLog({ + abi: InboxAbi, + data: txLog.data, + topics: txLog.topics, + }); - // We check that exactly 1 MessageAdded event was emitted with the expected recipient - expect(txEvents.length).toBe(1); - expect(txEvents[0].args.recipient).toBe(recipient); + // We check that MessageAdded event was emitted with the expected recipient + // Note: For whatever reason, viem types "think" that there is no recipient on topics.args. I hack around this + // by casting the args to "any" + expect((topics.args as any).recipient).toBe(recipient); // TODO(#4678): Unify naming of message key/entry key - msgKey = Fr.fromString(txEvents[0].args.entryKey!); + msgKey = Fr.fromString(topics.args.entryKey!); } // We wait for the archiver to process the message and we push a block for the message to be confirmed diff --git a/yarn-project/end-to-end/src/integration_l1_publisher.test.ts b/yarn-project/end-to-end/src/integration_l1_publisher.test.ts index 2a1c3d7e0e6..d282e0795a3 100644 --- a/yarn-project/end-to-end/src/integration_l1_publisher.test.ts +++ b/yarn-project/end-to-end/src/integration_l1_publisher.test.ts @@ -55,6 +55,7 @@ import { HttpTransport, PublicClient, WalletClient, + decodeEventLog, encodeFunctionData, getAbiItem, getAddress, @@ -78,7 +79,6 @@ describe('L1Publisher integration', () => { let publicClient: PublicClient; let deployerAccount: PrivateKeyAccount; - let availabilityOracleAddress: Address; let rollupAddress: Address; let inboxAddress: Address; let outboxAddress: Address; @@ -113,7 +113,6 @@ describe('L1Publisher integration', () => { } = await setupL1Contracts(config.rpcUrl, deployerAccount, logger); publicClient = publicClient_; - availabilityOracleAddress = getAddress(l1ContractAddresses.availabilityOracleAddress.toString()); rollupAddress = getAddress(l1ContractAddresses.rollupAddress.toString()); inboxAddress = getAddress(l1ContractAddresses.inboxAddress.toString()); outboxAddress = getAddress(l1ContractAddresses.outboxAddress.toString()); @@ -329,24 +328,23 @@ describe('L1Publisher integration', () => { const body = Body.random(); // `sendPublishTx` function is private so I am hacking around TS here. I think it's ok for test purposes. const txHash = await (publisher as any).sendPublishTx(body.toBuffer()); - - // We verify that the body was successfully published by fetching TxsPublished events from the AvailabilityOracle - // and checking if the txsHash in the event is as expected - const events = await publicClient.getLogs({ - address: availabilityOracleAddress, - event: getAbiItem({ - abi: AvailabilityOracleAbi, - name: 'TxsPublished', - }), - fromBlock: 0n, + const txReceipt = await publicClient.waitForTransactionReceipt({ + hash: txHash, }); - // We get the event just for the relevant transaction - const txEvents = events.filter(event => event.transactionHash === txHash); + // Exactly 1 event should be emitted in the transaction + expect(txReceipt.logs.length).toBe(1); + + // We decode the event log before checking it + const txLog = txReceipt.logs[0]; + const topics = decodeEventLog({ + abi: AvailabilityOracleAbi, + data: txLog.data, + topics: txLog.topics, + }); - // We check that exactly 1 TxsPublished event was emitted and txsHash is as expected - expect(txEvents.length).toBe(1); - expect(txEvents[0].args.txsHash).toEqual(`0x${body.getCalldataHash().toString('hex')}`); + // We check that the txsHash in the TxsPublished event is as expected + expect(topics.args.txsHash).toEqual(`0x${body.getCalldataHash().toString('hex')}`); }); it(`Build ${numberOfConsecutiveBlocks} blocks of 4 bloated txs building on each other`, async () => { diff --git a/yarn-project/noir-contracts.js/package.json b/yarn-project/noir-contracts.js/package.json index 8efbdbd1bc4..cdfa4bd68ea 100644 --- a/yarn-project/noir-contracts.js/package.json +++ b/yarn-project/noir-contracts.js/package.json @@ -49,4 +49,4 @@ "engines": { "node": ">=18" } -} \ No newline at end of file +}