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

feat: using current gas prices in cli-wallet #10105

Merged
merged 1 commit into from
Nov 26, 2024
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
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 @@ -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;
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 @@ -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
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 @@ -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;
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'];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure we want to expect DA gas here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not, were mainly for completeness.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will probably shake out as a single issue against aztec.js like "chore: remove mention of DA gas/fees from user facing code"

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
Loading