From 1de7700225d2f65d6def06234ff2500330a6e477 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 26 Feb 2025 15:24:34 -0300 Subject: [PATCH] chore: Fix and reenable fees-settings test Fixes #12258 --- yarn-project/end-to-end/bootstrap.sh | 1 + .../end-to-end/src/e2e_fees/fee_settings.test.ts | 12 ++++++------ .../src/tx_validator/gas_validator.ts | 11 +++++++---- yarn-project/stdlib/src/gas/gas_fees.ts | 9 ++++++++- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/yarn-project/end-to-end/bootstrap.sh b/yarn-project/end-to-end/bootstrap.sh index bbf169f7f623..6aff3b4cae88 100755 --- a/yarn-project/end-to-end/bootstrap.sh +++ b/yarn-project/end-to-end/bootstrap.sh @@ -60,6 +60,7 @@ function test_cmds { echo "$prefix simple e2e_fees/account_init" echo "$prefix simple e2e_fees/failures" echo "$prefix simple e2e_fees/fee_juice_payments" + echo "$prefix simple e2e_fees/fee_settings" echo "$prefix simple e2e_fees/gas_estimation" echo "$prefix simple e2e_fees/private_payments" echo "$prefix simple e2e_fees/public_payments" diff --git a/yarn-project/end-to-end/src/e2e_fees/fee_settings.test.ts b/yarn-project/end-to-end/src/e2e_fees/fee_settings.test.ts index 8eb08821955b..4c06339f9910 100644 --- a/yarn-project/end-to-end/src/e2e_fees/fee_settings.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees/fee_settings.test.ts @@ -41,14 +41,15 @@ describe('e2e_fees fee settings', () => { describe('setting max fee per gas', () => { const bumpL2Fees = async () => { const before = await aztecNode.getCurrentBaseFees(); - t.logger.info(`Initial L2 base fees are ${inspect(before)}`, { baseFees: before }); + t.logger.warn(`Initial L2 base fees are ${inspect(before)}`, { baseFees: before.toInspect() }); // Bumps L1 base fee, updates the L1 fee oracle, and advances slots to update L2 base fees. // Do we need all these advance and upgrade calls? Probably not, but these calls are blazing fast, // so it's not big deal if we're throwing some unnecessary calls. We just want higher L2 base fees. - t.logger.info(`Bumping L1 base fee per gas`); + const { baseFeePerGas } = await t.context.deployL1ContractsValues.publicClient.getBlock(); + t.logger.info(`Doubling current L1 base fee per gas ${baseFeePerGas}`); await cheatCodes.rollup.updateL1GasFeeOracle(); - await cheatCodes.eth.setNextBlockBaseFeePerGas(1e11); + await cheatCodes.eth.setNextBlockBaseFeePerGas(baseFeePerGas! * 2n); await cheatCodes.eth.mine(); await cheatCodes.rollup.advanceSlots(6); await cheatCodes.rollup.updateL1GasFeeOracle(); @@ -56,7 +57,7 @@ describe('e2e_fees fee settings', () => { await cheatCodes.rollup.updateL1GasFeeOracle(); const after = await aztecNode.getCurrentBaseFees(); - t.logger.info(`L2 base fees after L1 gas spike are ${inspect(after)}`, { baseFees: after }); + t.logger.warn(`L2 base fees after L1 gas spike are ${inspect(after)}`, { baseFees: after.toInspect() }); expect(after.feePerL2Gas.toBigInt()).toBeGreaterThan(before.feePerL2Gas.toBigInt()); }; @@ -70,8 +71,7 @@ describe('e2e_fees fee settings', () => { return tx; }; - // TODO(#12258): This test is currently broken on master. Fix it. - it.skip('handles base fee spikes with default padding', async () => { + it('handles base fee spikes with default padding', async () => { // Prepare two txs using the current L2 base fees: one with no padding and one with default padding const txWithNoPadding = await sendTx(0); const txWithDefaultPadding = await sendTx(undefined); diff --git a/yarn-project/sequencer-client/src/tx_validator/gas_validator.ts b/yarn-project/sequencer-client/src/tx_validator/gas_validator.ts index 626c8ed21bb9..281eea52c9ca 100644 --- a/yarn-project/sequencer-client/src/tx_validator/gas_validator.ts +++ b/yarn-project/sequencer-client/src/tx_validator/gas_validator.ts @@ -24,8 +24,8 @@ export class GasTxValidator implements TxValidator { this.#gasFees = gasFees; } - validateTx(tx: Tx): Promise { - if (this.#shouldSkip(tx)) { + async validateTx(tx: Tx): Promise { + if (await this.#shouldSkip(tx)) { return Promise.resolve({ result: 'skipped', reason: ['Insufficient fee per gas'] }); } return this.#validateTxFee(tx); @@ -37,7 +37,7 @@ export class GasTxValidator implements TxValidator { * Note that circuits check max fees even if fee payer is unset, so we * keep this validation even if the tx does not pay fees. */ - #shouldSkip(tx: Tx): boolean { + async #shouldSkip(tx: Tx): Promise { const gasSettings = tx.data.constants.txContext.gasSettings; // Skip the tx if its max fees are not enough for the current block's gas fees. @@ -47,7 +47,10 @@ export class GasTxValidator implements TxValidator { maxFeesPerGas.feePerL2Gas.lt(this.#gasFees.feePerL2Gas); if (notEnoughMaxFees) { - this.#log.warn(`Skipping transaction ${tx.getTxHash()} due to insufficient fee per gas`); + this.#log.warn(`Skipping transaction ${await tx.getTxHash()} due to insufficient fee per gas`, { + txMaxFeesPerGas: maxFeesPerGas.toInspect(), + currentGasFees: this.#gasFees.toInspect(), + }); } return notEnoughMaxFees; } diff --git a/yarn-project/stdlib/src/gas/gas_fees.ts b/yarn-project/stdlib/src/gas/gas_fees.ts index 015d7d127494..0e9523847545 100644 --- a/yarn-project/stdlib/src/gas/gas_fees.ts +++ b/yarn-project/stdlib/src/gas/gas_fees.ts @@ -91,7 +91,14 @@ export class GasFees { return serializeToFields(this.feePerDaGas, this.feePerL2Gas); } + toInspect() { + return { + feePerDaGas: this.feePerDaGas.toNumberUnsafe(), + feePerL2Gas: this.feePerL2Gas.toNumberUnsafe(), + }; + } + [inspect.custom]() { - return `GasFees { feePerDaGas=${this.feePerDaGas} feePerL2Gas=${this.feePerL2Gas} }`; + return `GasFees { feePerDaGas=${this.feePerDaGas.toBigInt()} feePerL2Gas=${this.feePerL2Gas.toBigInt()} }`; } }