Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Pad base fee in aztec.js #11370

Merged
merged 3 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading