From 8c6399892ca9697fb13ae77e8bef60953e2dffa4 Mon Sep 17 00:00:00 2001 From: Mitchell Tracy Date: Tue, 18 Jun 2024 05:35:33 -0400 Subject: [PATCH] do not discard logs on revert since the kernel has pruned revertible logs. add test of private fee payment method with revert. --- .../circuit-types/src/logs/tx_l2_logs.ts | 21 ++-- .../circuit-types/src/tx/processed_tx.ts | 6 +- .../end-to-end/src/e2e_fees/failures.test.ts | 99 +++++++++++++++++++ 3 files changed, 114 insertions(+), 12 deletions(-) diff --git a/yarn-project/circuit-types/src/logs/tx_l2_logs.ts b/yarn-project/circuit-types/src/logs/tx_l2_logs.ts index f17004efa7c..eca90bfbdb2 100644 --- a/yarn-project/circuit-types/src/logs/tx_l2_logs.ts +++ b/yarn-project/circuit-types/src/logs/tx_l2_logs.ts @@ -157,16 +157,17 @@ export class UnencryptedTxL2Logs extends TxL2Logs { * for more details. */ public hash(): Buffer { - if (this.unrollLogs().length == 0) { + const unrolledLogs = this.unrollLogs(); + if (unrolledLogs.length == 0) { return Buffer.alloc(32); } let flattenedLogs = Buffer.alloc(0); - for (const logsFromSingleFunctionCall of this.unrollLogs()) { + for (const logsFromSingleFunctionCall of unrolledLogs) { flattenedLogs = Buffer.concat([flattenedLogs, logsFromSingleFunctionCall.getSiloedHash()]); } // pad the end of logs with 0s - for (let i = 0; i < MAX_UNENCRYPTED_LOGS_PER_TX - this.unrollLogs().length; i++) { + for (let i = 0; i < MAX_UNENCRYPTED_LOGS_PER_TX - unrolledLogs.length; i++) { flattenedLogs = Buffer.concat([flattenedLogs, Buffer.alloc(32)]); } @@ -235,16 +236,17 @@ export class EncryptedNoteTxL2Logs extends TxL2Logs { * for more details. */ public hash(): Buffer { - if (this.unrollLogs().length == 0) { + const unrolledLogs = this.unrollLogs(); + if (unrolledLogs.length == 0) { return Buffer.alloc(32); } let flattenedLogs = Buffer.alloc(0); - for (const logsFromSingleFunctionCall of this.unrollLogs()) { + for (const logsFromSingleFunctionCall of unrolledLogs) { flattenedLogs = Buffer.concat([flattenedLogs, logsFromSingleFunctionCall.hash()]); } // pad the end of logs with 0s - for (let i = 0; i < MAX_NOTE_ENCRYPTED_LOGS_PER_TX - this.unrollLogs().length; i++) { + for (let i = 0; i < MAX_NOTE_ENCRYPTED_LOGS_PER_TX - unrolledLogs.length; i++) { flattenedLogs = Buffer.concat([flattenedLogs, Buffer.alloc(32)]); } @@ -313,16 +315,17 @@ export class EncryptedTxL2Logs extends TxL2Logs { * for more details. */ public hash(): Buffer { - if (this.unrollLogs().length == 0) { + const unrolledLogs = this.unrollLogs(); + if (unrolledLogs.length == 0) { return Buffer.alloc(32); } let flattenedLogs = Buffer.alloc(0); - for (const logsFromSingleFunctionCall of this.unrollLogs()) { + for (const logsFromSingleFunctionCall of unrolledLogs) { flattenedLogs = Buffer.concat([flattenedLogs, logsFromSingleFunctionCall.getSiloedHash()]); } // pad the end of logs with 0s - for (let i = 0; i < MAX_ENCRYPTED_LOGS_PER_TX - this.unrollLogs().length; i++) { + for (let i = 0; i < MAX_ENCRYPTED_LOGS_PER_TX - unrolledLogs.length; i++) { flattenedLogs = Buffer.concat([flattenedLogs, Buffer.alloc(32)]); } diff --git a/yarn-project/circuit-types/src/tx/processed_tx.ts b/yarn-project/circuit-types/src/tx/processed_tx.ts index 130cc4a6a6d..5622ccc2004 100644 --- a/yarn-project/circuit-types/src/tx/processed_tx.ts +++ b/yarn-project/circuit-types/src/tx/processed_tx.ts @@ -159,9 +159,9 @@ export function makeProcessedTx( data: kernelOutput, proof, // TODO(4712): deal with non-revertible logs here - noteEncryptedLogs: revertReason ? EncryptedNoteTxL2Logs.empty() : tx.noteEncryptedLogs, - encryptedLogs: revertReason ? EncryptedTxL2Logs.empty() : tx.encryptedLogs, - unencryptedLogs: revertReason ? UnencryptedTxL2Logs.empty() : tx.unencryptedLogs, + noteEncryptedLogs: tx.noteEncryptedLogs, + encryptedLogs: tx.encryptedLogs, + unencryptedLogs: tx.unencryptedLogs, isEmpty: false, revertReason, publicProvingRequests, diff --git a/yarn-project/end-to-end/src/e2e_fees/failures.test.ts b/yarn-project/end-to-end/src/e2e_fees/failures.test.ts index 160c49e6d2c..1357c3b0e6b 100644 --- a/yarn-project/end-to-end/src/e2e_fees/failures.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees/failures.test.ts @@ -4,9 +4,11 @@ import { Fr, type FunctionCall, FunctionSelector, + PrivateFeePaymentMethod, PublicFeePaymentMethod, TxStatus, computeAuthWitMessageHash, + computeSecretHash, } from '@aztec/aztec.js'; import { Gas, GasSettings } from '@aztec/circuits.js'; import { FunctionType } from '@aztec/foundation/abi'; @@ -120,6 +122,103 @@ describe('e2e_fees failures', () => { // Can't do presently because all logs are "revertible" so we lose notes that get broadcasted during unshielding. }); + it('reverts transactions but still pays fees using PrivateFeePaymentMethod', async () => { + const OutrageousPublicAmountAliceDoesNotHave = BigInt(1e8); + const PrivateMintedAlicePrivateBananas = BigInt(1e15); + + const [initialAlicePrivateBananas, initialFPCPrivateBananas] = await t.bananaPrivateBalances( + aliceAddress, + bananaFPC.address, + ); + const [initialAlicePublicBananas, initialFPCPublicBananas] = await t.bananaPublicBalances( + aliceAddress, + bananaFPC.address, + ); + const [initialAliceGas, initialFPCGas, initialSequencerGas] = await t.gasBalances( + aliceAddress, + bananaFPC.address, + sequencerAddress, + ); + + await t.mintPrivateBananas(PrivateMintedAlicePrivateBananas, aliceAddress); + + // if we simulate locally, it throws an error + await expect( + bananaCoin.methods + // still use a public transfer so as to fail in the public app logic phase + .transfer_public(aliceAddress, sequencerAddress, OutrageousPublicAmountAliceDoesNotHave, 0) + .send({ + fee: { + gasSettings, + paymentMethod: new PrivateFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet), + }, + }) + .wait(), + ).rejects.toThrow(/attempt to subtract with underflow 'hi == high'/); + + // we did not pay the fee, because we did not submit the TX + await expectMapping( + t.bananaPrivateBalances, + [aliceAddress, bananaFPC.address], + [initialAlicePrivateBananas + PrivateMintedAlicePrivateBananas, initialFPCPrivateBananas], + ); + await expectMapping( + t.bananaPublicBalances, + [aliceAddress, bananaFPC.address], + [initialAlicePublicBananas, initialFPCPublicBananas], + ); + await expectMapping(t.gasBalances, [aliceAddress, bananaFPC.address], [initialAliceGas, initialFPCGas]); + + // if we skip simulation, it includes the failed TX + const rebateSecret = Fr.random(); + const currentSequencerL1Gas = await t.getCoinbaseBalance(); + const txReceipt = await bananaCoin.methods + .transfer_public(aliceAddress, sequencerAddress, OutrageousPublicAmountAliceDoesNotHave, 0) + .send({ + skipPublicSimulation: true, + fee: { + gasSettings, + paymentMethod: new PrivateFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet, rebateSecret), + }, + }) + .wait({ dontThrowOnRevert: true }); + + expect(txReceipt.status).toBe(TxStatus.APP_LOGIC_REVERTED); + const feeAmount = txReceipt.transactionFee!; + const newSequencerL1Gas = await t.getCoinbaseBalance(); + expect(newSequencerL1Gas).toEqual(currentSequencerL1Gas + feeAmount); + + // and thus we paid the fee + await expectMapping( + t.bananaPrivateBalances, + [aliceAddress, bananaFPC.address], + [ + // alice paid the maximum amount in private bananas + initialAlicePrivateBananas + PrivateMintedAlicePrivateBananas - gasSettings.getFeeLimit().toBigInt(), + initialFPCPrivateBananas, + ], + ); + await expectMapping( + t.bananaPublicBalances, + [aliceAddress, bananaFPC.address], + [initialAlicePublicBananas, initialFPCPublicBananas + feeAmount], + ); + await expectMapping(t.gasBalances, [aliceAddress, bananaFPC.address], [initialAliceGas, initialFPCGas - feeAmount]); + + // Alice can redeem her shield to get the rebate + const refund = gasSettings.getFeeLimit().toBigInt() - feeAmount; + expect(refund).toBeGreaterThan(0n); + const secretHashForRebate = computeSecretHash(rebateSecret); + await t.addPendingShieldNoteToPXE(t.aliceWallet, refund, secretHashForRebate, txReceipt.txHash); + await bananaCoin.methods.redeem_shield(aliceAddress, refund, rebateSecret).send().wait(); + + await expectMapping( + t.bananaPrivateBalances, + [aliceAddress, bananaFPC.address], + [initialAlicePrivateBananas + PrivateMintedAlicePrivateBananas - feeAmount, initialFPCPrivateBananas], + ); + }); + it('fails transaction that error in setup', async () => { const OutrageousPublicAmountAliceDoesNotHave = BigInt(100e12);