From 589d9cde59bceeb88b33f23df3e22f7ad4e577fd Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Mon, 25 Mar 2024 14:23:23 -0300 Subject: [PATCH 1/2] feat: Throw by default when awaiting a tx that reverted --- yarn-project/aztec.js/src/contract/sent_tx.ts | 6 ++++-- yarn-project/end-to-end/src/e2e_deploy_contract.test.ts | 7 +++++-- yarn-project/end-to-end/src/e2e_fees.test.ts | 2 +- yarn-project/end-to-end/src/guides/dapp_testing.test.ts | 2 +- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/yarn-project/aztec.js/src/contract/sent_tx.ts b/yarn-project/aztec.js/src/contract/sent_tx.ts index 38f35bded7e..03e9da395de 100644 --- a/yarn-project/aztec.js/src/contract/sent_tx.ts +++ b/yarn-project/aztec.js/src/contract/sent_tx.ts @@ -15,6 +15,8 @@ export type WaitOpts = { waitForNotesSync?: boolean; /** Whether to include information useful for debugging/testing in the receipt. */ debug?: boolean; + /** Whether to accept a revert as a status code for the tx when waiting for it. If false, will throw if the tx reverts. */ + dontThrowOnRevert?: boolean; }; export const DefaultWaitOpts: WaitOpts = { @@ -60,10 +62,10 @@ export class SentTx { */ public async wait(opts?: WaitOpts): Promise> { if (opts?.debug && opts.waitForNotesSync === false) { - throw new Error('Cannot set getNotes to true if waitForNotesSync is false'); + throw new Error('Cannot set debug to true if waitForNotesSync is false'); } const receipt = await this.waitForReceipt(opts); - if (receipt.status !== TxStatus.MINED && receipt.status !== TxStatus.REVERTED) { + if (!(receipt.status === TxStatus.MINED || (receipt.status === TxStatus.REVERTED && opts?.dontThrowOnRevert))) { throw new Error( `Transaction ${await this.getTxHash()} was ${receipt.status}. Reason: ${receipt.error ?? 'unknown'}`, ); diff --git a/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts b/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts index 1727b37b987..fdaa62e6490 100644 --- a/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts @@ -134,7 +134,10 @@ describe('e2e_deploy_contract', () => { await Promise.all([goodDeploy.simulate(firstOpts), badDeploy.simulate(secondOpts)]); const [goodTx, badTx] = [goodDeploy.send(firstOpts), badDeploy.send(secondOpts)]; - const [goodTxPromiseResult, badTxReceiptResult] = await Promise.allSettled([goodTx.wait(), badTx.wait()]); + const [goodTxPromiseResult, badTxReceiptResult] = await Promise.allSettled([ + goodTx.wait(), + badTx.wait({ dontThrowOnRevert: true }), + ]); expect(goodTxPromiseResult.status).toBe('fulfilled'); expect(badTxReceiptResult.status).toBe('fulfilled'); // but reverted @@ -396,7 +399,7 @@ describe('e2e_deploy_contract', () => { const receipt = await contract.methods .increment_public_value(whom, 10) .send({ skipPublicSimulation: true }) - .wait(); + .wait({ dontThrowOnRevert: true }); expect(receipt.status).toEqual(TxStatus.REVERTED); // Meanwhile we check we didn't increment the value diff --git a/yarn-project/end-to-end/src/e2e_fees.test.ts b/yarn-project/end-to-end/src/e2e_fees.test.ts index 9dbbc4825e9..d4cdfccab7c 100644 --- a/yarn-project/end-to-end/src/e2e_fees.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees.test.ts @@ -172,7 +172,7 @@ describe('e2e_fees', () => { paymentMethod: new PublicFeePaymentMethod(bananaCoin.address, bananaFPC.address, wallets[0]), }, }) - .wait(); + .wait({ dontThrowOnRevert: true }); expect(txReceipt.status).toBe(TxStatus.REVERTED); // and thus we paid the fee diff --git a/yarn-project/end-to-end/src/guides/dapp_testing.test.ts b/yarn-project/end-to-end/src/guides/dapp_testing.test.ts index 676652fd634..32910fade34 100644 --- a/yarn-project/end-to-end/src/guides/dapp_testing.test.ts +++ b/yarn-project/end-to-end/src/guides/dapp_testing.test.ts @@ -254,7 +254,7 @@ describe('guides/dapp/testing', () => { it('asserts a transaction with a failing public call is included (with no state changes)', async () => { // docs:start:pub-reverted const call = token.methods.transfer_public(owner.getAddress(), recipient.getAddress(), 1000n, 0); - const receipt = await call.send({ skipPublicSimulation: true }).wait(); + const receipt = await call.send({ skipPublicSimulation: true }).wait({ dontThrowOnRevert: true }); expect(receipt.status).toEqual(TxStatus.REVERTED); const ownerPublicBalanceSlot = cheats.aztec.computeSlotInMap(6n, owner.getAddress()); const balance = await pxe.getPublicStorageAt(token.address, ownerPublicBalanceSlot); From e222cd3b61de9e9fd7c5550d90d75b94abc8beb4 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Mon, 25 Mar 2024 17:43:49 -0300 Subject: [PATCH 2/2] Fix e2e deploy test --- yarn-project/end-to-end/src/e2e_deploy_contract.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts b/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts index fdaa62e6490..7638c537499 100644 --- a/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts @@ -439,10 +439,12 @@ describe('e2e_deploy_contract', () => { }, 60_000); it('refuses to initialize the instance with wrong args via a public function', async () => { - // TODO(@spalladino): This tx is mined but reverts, we need to check revert flag once it's available - // Meanwhile, we check that its side effects did not come through as a means to assert it reverted const whom = AztecAddress.random(); - await contract.methods.public_constructor(whom, 43).send({ skipPublicSimulation: true }).wait(); + const receipt = await contract.methods + .public_constructor(whom, 43) + .send({ skipPublicSimulation: true }) + .wait({ dontThrowOnRevert: true }); + expect(receipt.status).toEqual(TxStatus.REVERTED); expect(await contract.methods.get_public_value(whom).view()).toEqual(0n); }, 30_000);