Skip to content

Commit

Permalink
fix: Pad base fee in aztec.js (#11370)
Browse files Browse the repository at this point in the history
When sending a tx, aztecjs would pick a `maxFeePerGas` equivalent to the
network's current base fee. This means that if the network fees
increased before the tx was mined, it became ineligible.

This adds a padding (defaulting to an extra 50%) to the fees set.

Fixes #11358

---------

Co-authored-by: spypsy <spypsy@users.noreply.github.com>
  • Loading branch information
spalladino and spypsy authored Jan 21, 2025
1 parent bca7052 commit d0e9a55
Show file tree
Hide file tree
Showing 11 changed files with 140 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,11 @@ export abstract class BaseContractInteraction {
* @param fee - User-provided fee options.
*/
protected async getDefaultFeeOptions(fee: UserFeeOptions | undefined): Promise<FeeOptions> {
const maxFeesPerGas = fee?.gasSettings?.maxFeesPerGas ?? (await this.wallet.getCurrentBaseFees());
const maxFeesPerGas =
fee?.gasSettings?.maxFeesPerGas ?? (await this.wallet.getCurrentBaseFees()).mul(1 + (fee?.baseFeePadding ?? 0.5));
const paymentMethod = fee?.paymentMethod ?? new NoFeePaymentMethod();
const gasSettings: GasSettings = GasSettings.default({ ...fee?.gasSettings, maxFeesPerGas });
this.log.debug(`Using L2 gas settings`, gasSettings);
return { gasSettings, paymentMethod };
}

Expand Down
2 changes: 2 additions & 0 deletions yarn-project/aztec.js/src/entrypoint/payload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ export type UserFeeOptions = {
paymentMethod?: FeePaymentMethod;
/** The gas settings */
gasSettings?: Partial<FieldsOf<GasSettings>>;
/** Percentage to pad the base fee by, if empty, defaults to 0.5 */
baseFeePadding?: number;
/** Whether to run an initial simulation of the tx with high gas limit to figure out actual gas settings. */
estimateGas?: boolean;
/** Percentage to pad the estimated gas limits by, if empty, defaults to 0.1. Only relevant if estimateGas is set. */
Expand Down
8 changes: 8 additions & 0 deletions yarn-project/aztec.js/src/utils/cheat_codes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,14 @@ export class RollupCheatCodes {
await action(owner, this.rollup);
await this.ethCheatCodes.stopImpersonating(owner);
}

/** Directly calls the L1 gas fee oracle. */
public async updateL1GasFeeOracle() {
await this.asOwner(async (account, rollup) => {
await rollup.write.updateL1GasFeeOracle({ account, chain: this.client.chain });
this.logger.warn(`Updated L1 gas fee oracle`);
});
}
}

/**
Expand Down
3 changes: 2 additions & 1 deletion yarn-project/bot/src/bot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ export class Bot {
estimateGas = true;
this.log.verbose(`Estimating gas for transaction`);
}
const baseFeePadding = 2; // Send 3x the current base fee
this.log.verbose(skipPublicSimulation ? `Skipping public simulation` : `Simulating public transfers`);
return { fee: { estimateGas, paymentMethod, gasSettings }, skipPublicSimulation };
return { fee: { estimateGas, paymentMethod, gasSettings, baseFeePadding }, skipPublicSimulation };
}
}
14 changes: 10 additions & 4 deletions yarn-project/circuits.js/src/structs/gas_fees.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,16 @@ export class GasFees {
}

mul(scalar: number | bigint) {
return new GasFees(
new Fr(this.feePerDaGas.toBigInt() * BigInt(scalar)),
new Fr(this.feePerL2Gas.toBigInt() * BigInt(scalar)),
);
if (scalar === 1 || scalar === 1n) {
return this.clone();
} else if (typeof scalar === 'bigint') {
return new GasFees(new Fr(this.feePerDaGas.toBigInt() * scalar), new Fr(this.feePerL2Gas.toBigInt() * scalar));
} else {
return new GasFees(
new Fr(this.feePerDaGas.toNumberUnsafe() * scalar),
new Fr(this.feePerL2Gas.toNumberUnsafe() * scalar),
);
}
}

static from(fields: FieldsOf<GasFees>) {
Expand Down
90 changes: 90 additions & 0 deletions yarn-project/end-to-end/src/e2e_fees/fee_settings.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import {
type AccountWallet,
type AztecAddress,
type AztecNode,
type CheatCodes,
FeeJuicePaymentMethod,
} from '@aztec/aztec.js';
import { Fr, type GasSettings } from '@aztec/circuits.js';
import { TestContract } from '@aztec/noir-contracts.js/Test';

import { inspect } from 'util';

import { FeesTest } from './fees_test.js';

describe('e2e_fees fee settings', () => {
let aztecNode: AztecNode;
let cheatCodes: CheatCodes;
let aliceAddress: AztecAddress;
let aliceWallet: AccountWallet;
let gasSettings: Partial<GasSettings>;
let paymentMethod: FeeJuicePaymentMethod;
let testContract: TestContract;

const t = new FeesTest('fee_juice');

beforeAll(async () => {
await t.applyBaseSnapshots();
await t.applyFundAliceWithFeeJuice();

({ aliceAddress, aliceWallet, gasSettings, cheatCodes, aztecNode } = await t.setup());

testContract = await TestContract.deploy(aliceWallet).send().deployed();
gasSettings = { ...gasSettings, maxFeesPerGas: undefined };
paymentMethod = new FeeJuicePaymentMethod(aliceAddress);
}, 60_000);

afterAll(async () => {
await t.teardown();
});

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 });

// 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`);
await cheatCodes.rollup.updateL1GasFeeOracle();
await cheatCodes.eth.setNextBlockBaseFeePerGas(1e11);
await cheatCodes.eth.mine();
await cheatCodes.rollup.advanceSlots(6);
await cheatCodes.rollup.updateL1GasFeeOracle();
await cheatCodes.rollup.advanceSlots(6);
await cheatCodes.rollup.updateL1GasFeeOracle();

const after = await aztecNode.getCurrentBaseFees();
t.logger.info(`L2 base fees after L1 gas spike are ${inspect(after)}`, { baseFees: after });
expect(after.feePerL2Gas.toBigInt()).toBeGreaterThan(before.feePerL2Gas.toBigInt());
};

const sendTx = async (baseFeePadding: number | undefined) => {
t.logger.info(`Preparing tx to be sent with base fee padding ${baseFeePadding}`);
const tx = await testContract.methods
.emit_nullifier_public(Fr.random())
.prove({ fee: { gasSettings, paymentMethod, baseFeePadding } });
const { maxFeesPerGas } = tx.data.constants.txContext.gasSettings;
t.logger.info(`Tx with hash ${tx.getTxHash()} ready with max fees ${inspect(maxFeesPerGas)}`);
return tx;
};

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);

// Now bump the L2 fees before we actually send them
await bumpL2Fees();

// And check that the no-padding does not get mined, but the default padding is good enough
t.logger.info(`Sendings txs`);
const sentWithNoPadding = txWithNoPadding.send();
const sentWithDefaultPadding = txWithDefaultPadding.send();
t.logger.info(`Awaiting txs`);
await expect(sentWithNoPadding.wait({ timeout: 30 })).rejects.toThrow(/dropped./i);
await sentWithDefaultPadding.wait({ timeout: 30 });
});
});
});
3 changes: 3 additions & 0 deletions yarn-project/end-to-end/src/e2e_fees/fees_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
type AccountWallet,
type AztecAddress,
type AztecNode,
CheatCodes,
type Logger,
type PXE,
SignerlessWallet,
Expand Down Expand Up @@ -52,6 +53,7 @@ export class FeesTest {
public logger: Logger;
public pxe!: PXE;
public aztecNode!: AztecNode;
public cheatCodes!: CheatCodes;

public aliceWallet!: AccountWallet;
public aliceAddress!: AztecAddress;
Expand Down Expand Up @@ -133,6 +135,7 @@ export class FeesTest {
this.pxe = pxe;
this.aztecNode = aztecNode;
this.gasSettings = GasSettings.default({ maxFeesPerGas: (await this.aztecNode.getCurrentBaseFees()).mul(2) });
this.cheatCodes = await CheatCodes.create(aztecNodeConfig.l1RpcUrl, pxe);
const accountManagers = accountKeys.map(ak => getSchnorrAccount(pxe, ak[0], ak[1], 1));
await Promise.all(accountManagers.map(a => a.register()));
this.wallets = await Promise.all(accountManagers.map(a => a.getWallet()));
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/ethereum/src/eth_cheat_codes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export class EthCheatCodes {
* Set the next block base fee per gas
* @param baseFee - The base fee to set
*/
public async setNextBlockBaseFeePerGas(baseFee: bigint): Promise<void> {
public async setNextBlockBaseFeePerGas(baseFee: bigint | number): Promise<void> {
const res = await this.rpcCall('anvil_setNextBlockBaseFeePerGas', [baseFee.toString()]);
if (res.error) {
throw new Error(`Error setting next block base fee per gas: ${res.error.message}`);
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/ethereum/src/l1_tx_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ export class L1TxUtils {

let priorityFee: bigint;
if (gasConfig.fixedPriorityFeePerGas) {
this.logger?.debug('Using fixed priority fee per gas', {
this.logger?.debug('Using fixed priority fee per L1 gas', {
fixedPriorityFeePerGas: gasConfig.fixedPriorityFeePerGas,
});
// try to maintain precision up to 1000000 wei
Expand Down Expand Up @@ -514,7 +514,7 @@ export class L1TxUtils {
maxFeePerBlobGas = maxFeePerBlobGas > minBlobFee ? maxFeePerBlobGas : minBlobFee;
}

this.logger?.debug(`Computed gas price`, {
this.logger?.debug(`Computed L1 gas price`, {
attempt,
baseFee: formatGwei(baseFee),
maxFeePerGas: formatGwei(maxFeePerGas),
Expand Down
13 changes: 13 additions & 0 deletions yarn-project/foundation/src/fields/fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ abstract class BaseField {
return Boolean(this.toBigInt());
}

/**
* Converts this field to a number.
* Throws if the underlying value is greater than MAX_SAFE_INTEGER.
*/
toNumber(): number {
const value = this.toBigInt();
if (value > Number.MAX_SAFE_INTEGER) {
Expand All @@ -107,6 +111,15 @@ abstract class BaseField {
return Number(value);
}

/**
* Converts this field to a number.
* May cause loss of precision if the underlying value is greater than MAX_SAFE_INTEGER.
*/
toNumberUnsafe(): number {
const value = this.toBigInt();
return Number(value);
}

toShortString(): string {
const str = this.toString();
return `${str.slice(0, 10)}...${str.slice(-4)}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ export class GasTxValidator implements TxValidator<Tx> {
return this.#validateTxFee(tx);
}

/**
* Check whether the tx's max fees are valid for the current block, and skip if not.
* We skip instead of invalidating since the tx may become elligible later.
* 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 {
const gasSettings = tx.data.constants.txContext.gasSettings;

Expand Down

0 comments on commit d0e9a55

Please sign in to comment.