Skip to content

Commit

Permalink
feat: reducing usage of default gas settings
Browse files Browse the repository at this point in the history
  • Loading branch information
LHerskind committed Nov 21, 2024
1 parent c87ce1c commit 2062468
Show file tree
Hide file tree
Showing 16 changed files with 67 additions and 63 deletions.
1 change: 0 additions & 1 deletion l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ library Constants {
uint256 internal constant DEFAULT_GAS_LIMIT = 1000000000;
uint256 internal constant DEFAULT_TEARDOWN_GAS_LIMIT = 12000000;
uint256 internal constant MAX_L2_GAS_PER_ENQUEUED_CALL = 12000000;
uint256 internal constant DEFAULT_MAX_FEE_PER_GAS = 10;
uint256 internal constant DA_BYTES_PER_FIELD = 32;
uint256 internal constant DA_GAS_PER_BYTE = 16;
uint256 internal constant FIXED_DA_GAS = 512;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ unconstrained fn setup_refund_success() {
// Gas used to compute transaction fee
// TXE oracle uses gas_used = Gas(1,1) when crafting TX
let txe_expected_gas_used = Gas::new(1, 1);
// TXE oracle uses default gas fees
let txe_gas_fees = GasFees::default();
// TXE oracle uses gas fees of (1, 1)
let txe_gas_fees = GasFees::new(1, 1);
let expected_tx_fee = txe_expected_gas_used.compute_fee(txe_gas_fees);

// Fund account with enough to cover tx fee plus some
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ use crate::components::{
},
};
use dep::types::{
abis::{gas_settings::GasSettings, kernel_circuit_public_inputs::KernelCircuitPublicInputs},
abis::{
gas::Gas, gas_fees::GasFees, gas_settings::GasSettings,
kernel_circuit_public_inputs::KernelCircuitPublicInputs,
},
constants::{DEFAULT_GAS_LIMIT, DEFAULT_TEARDOWN_GAS_LIMIT},
tests::fixture_builder::FixtureBuilder,
};

Expand All @@ -22,10 +26,16 @@ pub struct TailOutputValidatorBuilder {

impl TailOutputValidatorBuilder {
pub fn new() -> Self {
let gas_settings = GasSettings::new(
Gas::new(DEFAULT_GAS_LIMIT, DEFAULT_GAS_LIMIT),
Gas::new(DEFAULT_TEARDOWN_GAS_LIMIT, DEFAULT_TEARDOWN_GAS_LIMIT),
GasFees::new(10, 10),
);

let mut output = FixtureBuilder::new();
let mut previous_kernel = FixtureBuilder::new();
output.tx_context.gas_settings = GasSettings::default();
previous_kernel.tx_context.gas_settings = GasSettings::default();
output.tx_context.gas_settings = gas_settings;
previous_kernel.tx_context.gas_settings = gas_settings;
output.set_first_nullifier();
previous_kernel.set_first_nullifier();
TailOutputValidatorBuilder { output, previous_kernel }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ impl PublicBaseRollupInputs {
// self.avm_proof_data.vk_data.validate_in_vk_tree([AVM_VK_INDEX]);
// }
// TODO: Validate tube_data.public_inputs vs avm_proof_data.public_inputs

let reverted = self.avm_proof_data.public_inputs.reverted;

let combined_accumulated_data = self.generate_combined_accumulated_data(reverted);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ impl GasFees {
Self { fee_per_da_gas, fee_per_l2_gas }
}

pub fn default() -> Self {
GasFees::new(1, 1)
}

pub fn is_empty(self) -> bool {
(self.fee_per_da_gas == 0) & (self.fee_per_l2_gas == 0)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use crate::{
abis::{gas::Gas, gas_fees::GasFees},
constants::{
DEFAULT_GAS_LIMIT, DEFAULT_MAX_FEE_PER_GAS, DEFAULT_TEARDOWN_GAS_LIMIT, GAS_SETTINGS_LENGTH,
},
constants::{DEFAULT_GAS_LIMIT, DEFAULT_TEARDOWN_GAS_LIMIT, GAS_SETTINGS_LENGTH},
traits::{Deserialize, Empty, Serialize},
utils::reader::Reader,
};
Expand All @@ -17,14 +15,6 @@ impl GasSettings {
pub fn new(gas_limits: Gas, teardown_gas_limits: Gas, max_fees_per_gas: GasFees) -> Self {
Self { gas_limits, teardown_gas_limits, max_fees_per_gas }
}

pub fn default() -> Self {
GasSettings::new(
Gas::new(DEFAULT_GAS_LIMIT, DEFAULT_GAS_LIMIT),
Gas::new(DEFAULT_TEARDOWN_GAS_LIMIT, DEFAULT_TEARDOWN_GAS_LIMIT),
GasFees::new(DEFAULT_MAX_FEE_PER_GAS, DEFAULT_MAX_FEE_PER_GAS),
)
}
}

impl Eq for GasSettings {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ pub global DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE =
pub global DEFAULT_GAS_LIMIT: u32 = 1_000_000_000;
pub global DEFAULT_TEARDOWN_GAS_LIMIT: u32 = 12_000_000;
pub global MAX_L2_GAS_PER_ENQUEUED_CALL: u32 = 12_000_000;
pub global DEFAULT_MAX_FEE_PER_GAS: Field = 10;
pub global DA_BYTES_PER_FIELD: u32 = 32;
pub global DA_GAS_PER_BYTE: u32 = 16;
// pays for preamble information in TX Effects
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/circuit-types/src/interfaces/aztec-node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ describe('AztecNodeApiSchema', () => {

it('getCurrentBaseFees', async () => {
const response = await context.client.getCurrentBaseFees();
expect(response).toEqual(GasFees.default());
expect(response).toEqual(GasFees.empty());
});

it('getBlockNumber', async () => {
Expand Down Expand Up @@ -442,7 +442,7 @@ class MockAztecNode implements AztecNode {
return Promise.resolve(L2Block.random(number));
}
getCurrentBaseFees(): Promise<GasFees> {
return Promise.resolve(GasFees.default());
return Promise.resolve(GasFees.empty());
}
getBlockNumber(): Promise<number> {
return Promise.resolve(1);
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/circuit-types/src/interfaces/pxe.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ describe('PXESchema', () => {

it('getCurrentBaseFees', async () => {
const result = await context.client.getCurrentBaseFees();
expect(result).toEqual(GasFees.default());
expect(result).toEqual(GasFees.empty());
});

it('simulateUnconstrained', async () => {
Expand Down Expand Up @@ -450,7 +450,7 @@ class MockPXE implements PXE {
return Promise.resolve(L2Block.random(number));
}
getCurrentBaseFees(): Promise<GasFees> {
return Promise.resolve(GasFees.default());
return Promise.resolve(GasFees.empty());
}
simulateUnconstrained(
_functionName: string,
Expand Down
1 change: 0 additions & 1 deletion yarn-project/circuits.js/src/constants.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ export const DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE =
export const DEFAULT_GAS_LIMIT = 1000000000;
export const DEFAULT_TEARDOWN_GAS_LIMIT = 12000000;
export const MAX_L2_GAS_PER_ENQUEUED_CALL = 12000000;
export const DEFAULT_MAX_FEE_PER_GAS = 10;
export const DA_BYTES_PER_FIELD = 32;
export const DA_GAS_PER_BYTE = 16;
export const FIXED_DA_GAS = 512;
Expand Down
5 changes: 0 additions & 5 deletions yarn-project/circuits.js/src/structs/gas_fees.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,6 @@ export class GasFees {
return new GasFees(Fr.ZERO, Fr.ZERO);
}

/** Fixed gas fee values used until we define how gas fees in the protocol are computed. */
static default() {
return new GasFees(Fr.ONE, Fr.ONE);
}

isEmpty() {
return this.feePerDaGas.isZero() && this.feePerL2Gas.isZero();
}
Expand Down
12 changes: 5 additions & 7 deletions yarn-project/circuits.js/src/structs/gas_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,7 @@ import { type FieldsOf } from '@aztec/foundation/types';

import { z } from 'zod';

import {
DEFAULT_GAS_LIMIT,
DEFAULT_MAX_FEE_PER_GAS,
DEFAULT_TEARDOWN_GAS_LIMIT,
GAS_SETTINGS_LENGTH,
} from '../constants.gen.js';
import { DEFAULT_GAS_LIMIT, DEFAULT_TEARDOWN_GAS_LIMIT, GAS_SETTINGS_LENGTH } from '../constants.gen.js';
import { Gas, GasDimensions } from './gas.js';
import { GasFees } from './gas_fees.js';

Expand Down Expand Up @@ -66,11 +61,14 @@ export class GasSettings {
}

/** Default gas settings to use when user has not provided them. */
// @todo @lherskind The `MAX_FEES_PER_GAS` should be not be set as a default.
// deleting the default values, and trying to figure out the plumbing.
// Issue: #10104
static default(overrides: Partial<FieldsOf<GasSettings>> = {}) {
return GasSettings.from({
gasLimits: { l2Gas: DEFAULT_GAS_LIMIT, daGas: DEFAULT_GAS_LIMIT },
teardownGasLimits: { l2Gas: DEFAULT_TEARDOWN_GAS_LIMIT, daGas: DEFAULT_TEARDOWN_GAS_LIMIT },
maxFeesPerGas: { feePerL2Gas: new Fr(DEFAULT_MAX_FEE_PER_GAS), feePerDaGas: new Fr(DEFAULT_MAX_FEE_PER_GAS) },
maxFeesPerGas: { feePerL2Gas: new Fr(10), feePerDaGas: new Fr(10) },
...compact(overrides),
});
}
Expand Down
8 changes: 4 additions & 4 deletions yarn-project/cli-wallet/src/cmds/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export function injectCommands(program: Command, log: LogFn, debugLogger: DebugL
skipInitialization,
publicDeploy,
wait,
FeeOpts.fromCli(options, log, db),
await FeeOpts.fromCli(options, client, log, db),
json,
debugLogger,
log,
Expand Down Expand Up @@ -131,7 +131,7 @@ export function injectCommands(program: Command, log: LogFn, debugLogger: DebugL
const client = await createCompatibleClient(rpcUrl, debugLogger);
const account = await createOrRetrieveAccount(client, parsedFromAddress, db);

await deployAccount(account, wait, FeeOpts.fromCli(options, log, db), json, debugLogger, log);
await deployAccount(account, wait, await FeeOpts.fromCli(options, client, log, db), json, debugLogger, log);
});

const deployCommand = program
Expand Down Expand Up @@ -206,7 +206,7 @@ export function injectCommands(program: Command, log: LogFn, debugLogger: DebugL
typeof init === 'string' ? false : init,
universal,
wait,
FeeOpts.fromCli(options, log, db),
await FeeOpts.fromCli(options, client, log, db),
debugLogger,
log,
logJson(log),
Expand Down Expand Up @@ -266,7 +266,7 @@ export function injectCommands(program: Command, log: LogFn, debugLogger: DebugL
contractAddress,
wait,
cancel,
FeeOpts.fromCli(options, log, db),
await FeeOpts.fromCli(options, client, log, db),
log,
);
if (db && sentTx) {
Expand Down
53 changes: 36 additions & 17 deletions yarn-project/cli-wallet/src/utils/options/fees.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import {
FeeJuicePaymentMethodWithClaim,
type FeePaymentMethod,
NoFeePaymentMethod,
type PXE,
PrivateFeePaymentMethod,
PublicFeePaymentMethod,
type SendMethodOptions,
} from '@aztec/aztec.js';
import { AztecAddress, Fr, Gas, GasFees, GasSettings } from '@aztec/circuits.js';
import { parseBigint } from '@aztec/cli/utils';
import { type LogFn } from '@aztec/foundation/log';

import { Option } from 'commander';
Expand All @@ -21,6 +21,7 @@ export type CliFeeArgs = {
estimateGasOnly: boolean;
gasLimits?: string;
payment?: string;
maxFeesPerGas?: string;
estimateGas?: boolean;
};

Expand All @@ -35,9 +36,8 @@ export function printGasEstimates(
gasEstimates: Pick<GasSettings, 'gasLimits' | 'teardownGasLimits'>,
log: LogFn,
) {
log(`Maximum total tx fee: ${getEstimatedCost(gasEstimates, GasSettings.default().maxFeesPerGas)}`);
log(`Estimated total tx fee: ${getEstimatedCost(gasEstimates, GasFees.default())}`);
log(`Estimated gas usage: ${formatGasEstimate(gasEstimates)}`);
log(`Maximum total tx fee: ${getEstimatedCost(gasEstimates, feeOpts.gasSettings.maxFeesPerGas)}`);
}

function formatGasEstimate(estimate: Pick<GasSettings, 'gasLimits' | 'teardownGasLimits'>) {
Expand All @@ -62,7 +62,7 @@ export class FeeOpts implements IFeeOpts {
return {
estimateGas: this.estimateGas,
fee: {
gasSettings: this.gasSettings ?? GasSettings.default(),
gasSettings: this.gasSettings,
paymentMethod: await this.paymentMethodFactory(sender),
},
};
Expand All @@ -77,24 +77,30 @@ export class FeeOpts implements IFeeOpts {

static getOptions() {
return [
new Option('--inclusion-fee <value>', 'Inclusion fee to pay for the tx.').argParser(parseBigint),
new Option('--gas-limits <da=100,l2=100,teardownDA=10,teardownL2=10>', 'Gas limits for the tx.'),
FeeOpts.paymentMethodOption(),
new Option('--max-fee-per-gas <da=100,l2=100>', 'Maximum fee per gas unit for DA and L2 computation.'),
new Option('--no-estimate-gas', 'Whether to automatically estimate gas limits for the tx.'),
new Option('--estimate-gas-only', 'Only report gas estimation for the tx, do not send it.'),
];
}

static fromCli(args: CliFeeArgs, log: LogFn, db?: WalletDB) {
static async fromCli(args: CliFeeArgs, pxe: PXE, log: LogFn, db?: WalletDB) {
const estimateOnly = args.estimateGasOnly;
if (!args.gasLimits && !args.payment) {
return new NoFeeOpts(estimateOnly);
}
const gasSettings = GasSettings.from({
const gasFees = args.maxFeesPerGas
? parseGasFees(args.maxFeesPerGas)
: { maxFeesPerGas: await pxe.getCurrentBaseFees() };
const input = {
...GasSettings.default(),
...(args.gasLimits ? parseGasLimits(args.gasLimits) : {}),
maxFeesPerGas: GasFees.default(),
});
...gasFees,
};
const gasSettings = GasSettings.from(input);

if (!args.gasLimits && !args.payment) {
return new NoFeeOpts(estimateOnly, gasSettings);
}

return new FeeOpts(
estimateOnly,
gasSettings,
Expand All @@ -105,11 +111,7 @@ export class FeeOpts implements IFeeOpts {
}

class NoFeeOpts implements IFeeOpts {
constructor(public estimateOnly: boolean) {}

get gasSettings(): GasSettings {
return GasSettings.default();
}
constructor(public estimateOnly: boolean, public gasSettings: GasSettings) {}

toSendOpts(): Promise<SendMethodOptions> {
return Promise.resolve({});
Expand Down Expand Up @@ -211,3 +213,20 @@ function parseGasLimits(gasLimits: string): { gasLimits: Gas; teardownGasLimits:
teardownGasLimits: new Gas(parsed.teardownDA, parsed.teardownL2),
};
}

function parseGasFees(gasFees: string): { maxFeesPerGas: GasFees } {
const parsed = gasFees.split(',').reduce((acc, fee) => {
const [dimension, value] = fee.split('=');
acc[dimension] = parseInt(value, 10);
return acc;
}, {} as Record<string, number>);

const expected = ['da', 'l2'];
for (const dimension of expected) {
if (!(dimension in parsed)) {
throw new Error(`Missing gas fee for ${dimension}`);
}
}

return { maxFeesPerGas: new GasFees(parsed.da, parsed.l2) };
}
2 changes: 1 addition & 1 deletion yarn-project/simulator/src/public/fixtures/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export async function simulateAvmTestContractGenerateCircuitInputs(
calldata = [functionSelector.toField(), ...calldata];

const globalVariables = GlobalVariables.empty();
globalVariables.gasFees = GasFees.default();
globalVariables.gasFees = GasFees.empty();
globalVariables.timestamp = TIMESTAMP;

const worldStateDB = mock<WorldStateDB>();
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/txe/src/oracle/txe_oracle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ export class TXE implements TypedOracle {
globalVariables.chainId = this.chainId;
globalVariables.version = this.version;
globalVariables.blockNumber = new Fr(this.blockNumber);
globalVariables.gasFees = GasFees.default();
globalVariables.gasFees = new GasFees(1, 1);

const simulator = new PublicTxSimulator(
db,
Expand Down

0 comments on commit 2062468

Please sign in to comment.