From 5a7c3fa0a1a39be1cd05df54ed9799fcd768e82c Mon Sep 17 00:00:00 2001 From: LHerskind Date: Thu, 21 Nov 2024 22:17:40 +0000 Subject: [PATCH] feat: reducing usage of default gas settings --- .../src/core/libraries/ConstantsGen.sol | 1 - .../token_contract/src/test/refunds.nr | 4 +- .../tail_output_validator_builder/mod.nr | 16 ++++-- .../rollup-lib/src/base/public_base_rollup.nr | 1 - .../crates/types/src/abis/gas_fees.nr | 4 -- .../crates/types/src/abis/gas_settings.nr | 12 +---- .../crates/types/src/constants.nr | 1 - .../src/interfaces/aztec-node.test.ts | 4 +- .../circuit-types/src/interfaces/pxe.test.ts | 4 +- yarn-project/circuits.js/src/constants.gen.ts | 1 - .../circuits.js/src/structs/gas_fees.ts | 5 -- .../circuits.js/src/structs/gas_settings.ts | 12 ++--- yarn-project/cli-wallet/src/cmds/index.ts | 8 +-- .../cli-wallet/src/utils/options/fees.ts | 53 +++++++++++++------ .../simulator/src/public/fixtures/index.ts | 2 +- yarn-project/txe/src/oracle/txe_oracle.ts | 2 +- 16 files changed, 67 insertions(+), 63 deletions(-) diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol index 6f272bc1c3b2..362808524e14 100644 --- a/l1-contracts/src/core/libraries/ConstantsGen.sol +++ b/l1-contracts/src/core/libraries/ConstantsGen.sol @@ -116,7 +116,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; diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr index 2fa5a00240c2..d8797b3ac8cc 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr @@ -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 diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/tests/tail_output_validator_builder/mod.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/tests/tail_output_validator_builder/mod.nr index a624732fde93..61d4290dd00f 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/tests/tail_output_validator_builder/mod.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/tests/tail_output_validator_builder/mod.nr @@ -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, }; @@ -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 } diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/public_base_rollup.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/public_base_rollup.nr index 1fff4bb07ffe..05ad943e2b11 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/public_base_rollup.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/public_base_rollup.nr @@ -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); diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_fees.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_fees.nr index 7c8b9f84996e..43fee1f907e1 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_fees.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_fees.nr @@ -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) } diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_settings.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_settings.nr index 764d05e4a423..457f1fb5a8af 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_settings.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_settings.nr @@ -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, }; @@ -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 { diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr index 7525aa94663e..6c189046ce76 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -175,7 +175,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 diff --git a/yarn-project/circuit-types/src/interfaces/aztec-node.test.ts b/yarn-project/circuit-types/src/interfaces/aztec-node.test.ts index 2f2b6a0ab069..ddb1aa3ba77f 100644 --- a/yarn-project/circuit-types/src/interfaces/aztec-node.test.ts +++ b/yarn-project/circuit-types/src/interfaces/aztec-node.test.ts @@ -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 () => { @@ -442,7 +442,7 @@ class MockAztecNode implements AztecNode { return Promise.resolve(L2Block.random(number)); } getCurrentBaseFees(): Promise { - return Promise.resolve(GasFees.default()); + return Promise.resolve(GasFees.empty()); } getBlockNumber(): Promise { return Promise.resolve(1); diff --git a/yarn-project/circuit-types/src/interfaces/pxe.test.ts b/yarn-project/circuit-types/src/interfaces/pxe.test.ts index 625106421019..294ba5199885 100644 --- a/yarn-project/circuit-types/src/interfaces/pxe.test.ts +++ b/yarn-project/circuit-types/src/interfaces/pxe.test.ts @@ -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 () => { @@ -450,7 +450,7 @@ class MockPXE implements PXE { return Promise.resolve(L2Block.random(number)); } getCurrentBaseFees(): Promise { - return Promise.resolve(GasFees.default()); + return Promise.resolve(GasFees.empty()); } simulateUnconstrained( _functionName: string, diff --git a/yarn-project/circuits.js/src/constants.gen.ts b/yarn-project/circuits.js/src/constants.gen.ts index 7a85b60a3308..b03bffa47fcd 100644 --- a/yarn-project/circuits.js/src/constants.gen.ts +++ b/yarn-project/circuits.js/src/constants.gen.ts @@ -102,7 +102,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; diff --git a/yarn-project/circuits.js/src/structs/gas_fees.ts b/yarn-project/circuits.js/src/structs/gas_fees.ts index 6cab097c0cec..b8d37fa4064b 100644 --- a/yarn-project/circuits.js/src/structs/gas_fees.ts +++ b/yarn-project/circuits.js/src/structs/gas_fees.ts @@ -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(); } diff --git a/yarn-project/circuits.js/src/structs/gas_settings.ts b/yarn-project/circuits.js/src/structs/gas_settings.ts index 9ea1dbea30a6..03a38401ea18 100644 --- a/yarn-project/circuits.js/src/structs/gas_settings.ts +++ b/yarn-project/circuits.js/src/structs/gas_settings.ts @@ -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'; @@ -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> = {}) { 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), }); } diff --git a/yarn-project/cli-wallet/src/cmds/index.ts b/yarn-project/cli-wallet/src/cmds/index.ts index eaa78803c342..a286ad586038 100644 --- a/yarn-project/cli-wallet/src/cmds/index.ts +++ b/yarn-project/cli-wallet/src/cmds/index.ts @@ -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, @@ -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 @@ -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), @@ -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) { diff --git a/yarn-project/cli-wallet/src/utils/options/fees.ts b/yarn-project/cli-wallet/src/utils/options/fees.ts index 00226cfe64b8..154be27e1c20 100644 --- a/yarn-project/cli-wallet/src/utils/options/fees.ts +++ b/yarn-project/cli-wallet/src/utils/options/fees.ts @@ -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'; @@ -21,6 +21,7 @@ export type CliFeeArgs = { estimateGasOnly: boolean; gasLimits?: string; payment?: string; + maxFeesPerGas?: string; estimateGas?: boolean; }; @@ -35,9 +36,8 @@ export function printGasEstimates( gasEstimates: Pick, 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) { @@ -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), }, }; @@ -77,24 +77,30 @@ export class FeeOpts implements IFeeOpts { static getOptions() { return [ - new Option('--inclusion-fee ', 'Inclusion fee to pay for the tx.').argParser(parseBigint), new Option('--gas-limits ', 'Gas limits for the tx.'), FeeOpts.paymentMethodOption(), + new Option('--max-fee-per-gas ', '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, @@ -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 { return Promise.resolve({}); @@ -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); + + 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) }; +} diff --git a/yarn-project/simulator/src/public/fixtures/index.ts b/yarn-project/simulator/src/public/fixtures/index.ts index 8c0f53fab32d..133727b89b9f 100644 --- a/yarn-project/simulator/src/public/fixtures/index.ts +++ b/yarn-project/simulator/src/public/fixtures/index.ts @@ -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(); diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index de40b91e8f4c..670bf6b1b770 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -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,