Skip to content

Commit

Permalink
chore: Remove default gas settings (#10163)
Browse files Browse the repository at this point in the history
Deletes default fee in gas settings, and makes `maxFeePerGas` required
for initializing a `GasSettings` instance.

Built on top of
#10105.
  • Loading branch information
spalladino authored Nov 28, 2024
1 parent 9e19244 commit c9a4d88
Show file tree
Hide file tree
Showing 45 changed files with 246 additions and 219 deletions.
3 changes: 1 addition & 2 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,7 @@ library Constants {
uint256 internal constant AZTEC_MAX_EPOCH_DURATION = 32;
uint256 internal constant GENESIS_ARCHIVE_ROOT =
1002640778211850180189505934749257244705296832326768971348723156503780793518;
uint256 internal constant FEE_JUICE_INITIAL_MINT = 20000000000000000000;
uint256 internal constant FEE_FUNDING_FOR_TESTER_ACCOUNT = 100000000000000000000;
uint256 internal constant FEE_JUICE_INITIAL_MINT = 200000000000000000000;
uint256 internal constant PUBLIC_DISPATCH_SELECTOR = 3578010381;
uint256 internal constant MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS = 3000;
uint256 internal constant MAX_PACKED_BYTECODE_SIZE_PER_PRIVATE_FUNCTION_IN_FIELDS = 3000;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,7 @@ pub global GENESIS_ARCHIVE_ROOT: Field =
0x0237797d6a2c04d20d4fa06b74482bd970ccd51a43d9b05b57e9b91fa1ae1cae;
// The following and the value in `deploy_l1_contracts` must match. We should not have the code both places, but
// we are running into circular dependency issues. #3342
global FEE_JUICE_INITIAL_MINT: Field = 20000000000000000000;
global FEE_FUNDING_FOR_TESTER_ACCOUNT: Field = 100000000000000000000; // 100e18
global FEE_JUICE_INITIAL_MINT: Field = 200000000000000000000;
// Last 4 bytes of the Poseidon2 hash of 'public_dispatch(Field)'.
pub global PUBLIC_DISPATCH_SELECTOR: Field = 0xd5441b0d;

Expand Down
3 changes: 2 additions & 1 deletion yarn-project/accounts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"version": "0.1.0",
"type": "module",
"exports": {
"./dapp": "./dest/dapp/index.js",
"./defaults": "./dest/defaults/index.js",
"./ecdsa": "./dest/ecdsa/index.js",
"./schnorr": "./dest/schnorr/index.js",
Expand Down Expand Up @@ -101,4 +102,4 @@
"engines": {
"node": ">=18"
}
}
}
33 changes: 33 additions & 0 deletions yarn-project/accounts/src/dapp/dapp_interface.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { type AccountWallet, type AuthWitnessProvider } from '@aztec/aztec.js';
import { type AztecAddress, type CompleteAddress, type NodeInfo } from '@aztec/circuits.js';
import { DefaultDappEntrypoint } from '@aztec/entrypoints/dapp';

import { DefaultAccountInterface } from '../defaults/account_interface.js';

/**
* Default implementation for an account interface that uses a dapp entrypoint.
*/
export class DefaultDappInterface extends DefaultAccountInterface {
constructor(
authWitnessProvider: AuthWitnessProvider,
userAddress: CompleteAddress,
dappAddress: AztecAddress,
nodeInfo: Pick<NodeInfo, 'l1ChainId' | 'protocolVersion'>,
) {
super(authWitnessProvider, userAddress, nodeInfo);
this.entrypoint = new DefaultDappEntrypoint(
userAddress.address,
authWitnessProvider,
dappAddress,
nodeInfo.l1ChainId,
nodeInfo.protocolVersion,
);
}

static createFromUserWallet(wallet: AccountWallet, dappAddress: AztecAddress): DefaultDappInterface {
return new DefaultDappInterface(wallet, wallet.getCompleteAddress(), dappAddress, {
l1ChainId: wallet.getChainId().toNumber(),
protocolVersion: wallet.getVersion().toNumber(),
});
}
}
1 change: 1 addition & 0 deletions yarn-project/accounts/src/dapp/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './dapp_interface.js';
3 changes: 2 additions & 1 deletion yarn-project/accounts/src/defaults/account_interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import { DefaultAccountEntrypoint } from '@aztec/entrypoints/account';
* entrypoint signature, which accept an AppPayload and a FeePayload as defined in noir-libs/aztec-noir/src/entrypoint module
*/
export class DefaultAccountInterface implements AccountInterface {
private entrypoint: EntrypointInterface;
protected entrypoint: EntrypointInterface;

private chainId: Fr;
private version: Fr;

Expand Down
6 changes: 4 additions & 2 deletions yarn-project/accounts/src/ecdsa/ecdsa_k/artifact.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { type NoirCompiledContract, loadContractArtifact } from '@aztec/aztec.js';
import { type ContractArtifact, type NoirCompiledContract, loadContractArtifact } from '@aztec/aztec.js';

import EcdsaKAccountContractJson from '../../../artifacts/EcdsaKAccount.json' assert { type: 'json' };

export const EcdsaKAccountContractArtifact = loadContractArtifact(EcdsaKAccountContractJson as NoirCompiledContract);
export const EcdsaKAccountContractArtifact: ContractArtifact = loadContractArtifact(
EcdsaKAccountContractJson as NoirCompiledContract,
);
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,16 @@ export class DeployAccountMethod extends DeployMethod {
: feePaymentNameOrArtifact;
}

protected override async getInitializeFunctionCalls(options: DeployOptions): Promise<ExecutionRequestInit> {
protected override async getInitializeFunctionCalls(
options: DeployOptions,
): Promise<Pick<ExecutionRequestInit, 'calls' | 'authWitnesses' | 'packedArguments'>> {
const exec = await super.getInitializeFunctionCalls(options);

if (options.fee && this.#feePaymentArtifact) {
const { address } = this.getInstance();
const emptyAppPayload = EntrypointPayload.fromAppExecution([]);
const feePayload = await EntrypointPayload.fromFeeOptions(address, options?.fee);
const fee = await this.getDefaultFeeOptions(options.fee);
const feePayload = await EntrypointPayload.fromFeeOptions(address, fee);

exec.calls.push({
name: this.#feePaymentArtifact.name,
Expand Down
3 changes: 1 addition & 2 deletions yarn-project/aztec.js/src/account_manager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { DeployAccountSentTx } from './deploy_account_sent_tx.js';
*/
export type DeployAccountOptions = Pick<
DeployOptions,
'fee' | 'skipClassRegistration' | 'skipPublicDeployment' | 'estimateGas' | 'skipInitialization'
'fee' | 'skipClassRegistration' | 'skipPublicDeployment' | 'skipInitialization'
>;

/**
Expand Down Expand Up @@ -166,7 +166,6 @@ export class AccountManager {
skipInitialization: opts?.skipInitialization ?? false,
universalDeploy: true,
fee: opts?.fee,
estimateGas: opts?.estimateGas,
}),
)
.then(tx => tx.getTxHash());
Expand Down
55 changes: 37 additions & 18 deletions yarn-project/aztec.js/src/contract/base_contract_interaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import { type Fr, GasSettings } from '@aztec/circuits.js';
import { createDebugLogger } from '@aztec/foundation/log';

import { type Wallet } from '../account/wallet.js';
import { type ExecutionRequestInit, type FeeOptions } from '../entrypoint/entrypoint.js';
import { type ExecutionRequestInit } from '../entrypoint/entrypoint.js';
import { type FeeOptions, type UserFeeOptions } from '../entrypoint/payload.js';
import { NoFeePaymentMethod } from '../fee/no_fee_payment_method.js';
import { getGasLimits } from './get_gas_limits.js';
import { ProvenTx } from './proven_tx.js';
import { SentTx } from './sent_tx.js';
Expand All @@ -16,11 +18,7 @@ export type SendMethodOptions = {
/** Wether to skip the simulation of the public part of the transaction. */
skipPublicSimulation?: boolean;
/** The fee options for the transaction. */
fee?: FeeOptions;
/** Whether to run an initial simulation of the tx with high gas limit to figure out actual gas settings (will default to true later down the road). */
estimateGas?: boolean;
/** Percentage to pad the suggested gas limits by, if empty, defaults to 10%. */
estimatedGasPad?: number;
fee?: UserFeeOptions;
/** Custom nonce to inject into the app payload of the transaction. Useful when trying to cancel an ongoing transaction by creating a new one with a higher fee */
nonce?: Fr;
/** Whether the transaction can be cancelled. If true, an extra nullifier will be emitted: H(nonce, GENERATOR_INDEX__TX_NULLIFIER) */
Expand Down Expand Up @@ -92,33 +90,54 @@ export abstract class BaseContractInteraction {
public async estimateGas(
opts?: Omit<SendMethodOptions, 'estimateGas' | 'skipPublicSimulation'>,
): Promise<Pick<GasSettings, 'gasLimits' | 'teardownGasLimits'>> {
const txRequest = await this.create({ ...opts, estimateGas: false });
const txRequest = await this.create({ ...opts, fee: { ...opts?.fee, estimateGas: false } });
const simulationResult = await this.wallet.simulateTx(txRequest, true);
const { totalGas: gasLimits, teardownGas: teardownGasLimits } = getGasLimits(
simulationResult,
opts?.estimatedGasPad,
opts?.fee?.estimatedGasPadding,
);
return { gasLimits, teardownGasLimits };
}

/**
* Helper method to return fee options based on the user opts, estimating tx gas if needed.
* Returns default fee options based on the user opts without running a simulation for gas estimation.
* @param fee - User-provided fee options.
*/
protected async getDefaultFeeOptions(fee: UserFeeOptions | undefined): Promise<FeeOptions> {
const maxFeesPerGas = fee?.gasSettings?.maxFeesPerGas ?? (await this.wallet.getCurrentBaseFees());
const paymentMethod = fee?.paymentMethod ?? new NoFeePaymentMethod();
const gasSettings: GasSettings = GasSettings.default({ ...fee?.gasSettings, maxFeesPerGas });
return { gasSettings, paymentMethod };
}

/**
* Return fee options based on the user opts, estimating tx gas if needed.
* @param request - Request to execute for this interaction.
* @param pad - Percentage to pad the suggested gas limits by, as decimal (e.g., 0.10 for 10%).
* @returns Fee options for the actual transaction.
*/
protected async getFeeOptionsFromEstimatedGas(request: ExecutionRequestInit, pad?: number) {
const fee = request.fee;
if (fee) {
const txRequest = await this.wallet.createTxExecutionRequest(request);
protected async getFeeOptions(
request: Omit<ExecutionRequestInit, 'fee'> & { /** User-provided fee options */ fee?: UserFeeOptions },
): Promise<FeeOptions> {
const defaultFeeOptions = await this.getDefaultFeeOptions(request.fee);
const paymentMethod = defaultFeeOptions.paymentMethod;
const maxFeesPerGas = defaultFeeOptions.gasSettings.maxFeesPerGas;

let gasSettings = defaultFeeOptions.gasSettings;
if (request.fee?.estimateGas) {
const feeForEstimation: FeeOptions = { paymentMethod, gasSettings };
const txRequest = await this.wallet.createTxExecutionRequest({ ...request, fee: feeForEstimation });
const simulationResult = await this.wallet.simulateTx(txRequest, true);
const { totalGas: gasLimits, teardownGas: teardownGasLimits } = getGasLimits(simulationResult, pad);
this.log.debug(
const { totalGas: gasLimits, teardownGas: teardownGasLimits } = getGasLimits(
simulationResult,
request.fee?.estimatedGasPadding,
);
gasSettings = GasSettings.from({ maxFeesPerGas, gasLimits, teardownGasLimits });
this.log.verbose(
`Estimated gas limits for tx: DA=${gasLimits.daGas} L2=${gasLimits.l2Gas} teardownDA=${teardownGasLimits.daGas} teardownL2=${teardownGasLimits.l2Gas}`,
);
const gasSettings = GasSettings.default({ ...fee.gasSettings, gasLimits, teardownGasLimits });
return { ...fee, gasSettings };
}
return fee;

return { gasSettings, paymentMethod };
}
}
47 changes: 18 additions & 29 deletions yarn-project/aztec.js/src/contract/batch_call.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ export class BatchCall extends BaseContractInteraction {
*/
public async create(opts?: SendMethodOptions): Promise<TxExecutionRequest> {
const calls = this.calls;
const fee = opts?.estimateGas
? await this.getFeeOptionsFromEstimatedGas({ calls, fee: opts?.fee }, opts?.estimatedGasPad)
: opts?.fee;
return await this.wallet.createTxExecutionRequest({ calls, fee });
const fee = await this.getFeeOptions({ calls, ...opts });
return await this.wallet.createTxExecutionRequest({ calls, ...opts, fee });
}

/**
Expand All @@ -35,48 +33,39 @@ export class BatchCall extends BaseContractInteraction {
* @returns The result of the transaction as returned by the contract function.
*/
public async simulate(options: SimulateMethodOptions = {}): Promise<any> {
const { calls, unconstrained } = this.calls.reduce<{
/**
* Keep track of the number of private calls to retrieve the return values
*/
const { indexedCalls, unconstrained } = this.calls.reduce<{
/** Keep track of the number of private calls to retrieve the return values */
privateIndex: 0;
/**
* Keep track of the number of private calls to retrieve the return values
*/
/** Keep track of the number of public calls to retrieve the return values */
publicIndex: 0;
/**
* The public and private function calls in the batch
*/
calls: [FunctionCall, number, number][];
/**
* The unconstrained function calls in the batch.
*/
/** The public and private function calls in the batch */
indexedCalls: [FunctionCall, number, number][];
/** The unconstrained function calls in the batch. */
unconstrained: [FunctionCall, number][];
}>(
(acc, current, index) => {
if (current.type === FunctionType.UNCONSTRAINED) {
acc.unconstrained.push([current, index]);
} else {
acc.calls.push([
acc.indexedCalls.push([
current,
index,
current.type === FunctionType.PRIVATE ? acc.privateIndex++ : acc.publicIndex++,
]);
}
return acc;
},
{ calls: [], unconstrained: [], publicIndex: 0, privateIndex: 0 },
{ indexedCalls: [], unconstrained: [], publicIndex: 0, privateIndex: 0 },
);

const txRequest = await this.wallet.createTxExecutionRequest({ calls: calls.map(indexedCall => indexedCall[0]) });
const calls = indexedCalls.map(([call]) => call);
const fee = await this.getFeeOptions({ calls, ...options });
const txRequest = await this.wallet.createTxExecutionRequest({ calls, ...options, fee });

const unconstrainedCalls = unconstrained.map(async indexedCall => {
const call = indexedCall[0];
return [
await this.wallet.simulateUnconstrained(call.name, call.args, call.to, options?.from),
indexedCall[1],
] as const;
});
const unconstrainedCalls = unconstrained.map(
async ([call, index]) =>
[await this.wallet.simulateUnconstrained(call.name, call.args, call.to, options?.from), index] as const,
);

const [unconstrainedResults, simulatedTx] = await Promise.all([
Promise.all(unconstrainedCalls),
Expand All @@ -88,7 +77,7 @@ export class BatchCall extends BaseContractInteraction {
unconstrainedResults.forEach(([result, index]) => {
results[index] = result;
});
calls.forEach(([call, callIndex, resultIndex]) => {
indexedCalls.forEach(([call, callIndex, resultIndex]) => {
// As account entrypoints are private, for private functions we retrieve the return values from the first nested call
// since we're interested in the first set of values AFTER the account entrypoint
// For public functions we retrieve the first values directly from the public output.
Expand Down
2 changes: 2 additions & 0 deletions yarn-project/aztec.js/src/contract/contract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
CompleteAddress,
type ContractInstanceWithAddress,
EthAddress,
GasFees,
type NodeInfo,
} from '@aztec/circuits.js';
import { type L1ContractAddresses } from '@aztec/ethereum';
Expand Down Expand Up @@ -153,6 +154,7 @@ describe('Contract Class', () => {
wallet.getNodeInfo.mockResolvedValue(mockNodeInfo);
wallet.proveTx.mockResolvedValue(mockTxProvingResult);
wallet.getRegisteredAccounts.mockResolvedValue([account]);
wallet.getCurrentBaseFees.mockResolvedValue(new GasFees(100, 100));
});

it('should create and send a contract method tx', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,14 @@ export class ContractFunctionInteraction extends BaseContractInteraction {
* @param opts - An optional object containing additional configuration for the transaction.
* @returns A Promise that resolves to a transaction instance.
*/
public async create(opts?: SendMethodOptions): Promise<TxExecutionRequest> {
public async create(opts: SendMethodOptions = {}): Promise<TxExecutionRequest> {
if (this.functionDao.functionType === FunctionType.UNCONSTRAINED) {
throw new Error("Can't call `create` on an unconstrained function.");
}
const calls = [this.request()];
const fee = opts?.estimateGas
? await this.getFeeOptionsFromEstimatedGas({ calls, fee: opts?.fee }, opts?.estimatedGasPad)
: opts?.fee;
const txRequest = await this.wallet.createTxExecutionRequest({
calls,
fee,
nonce: opts?.nonce,
cancellable: opts?.cancellable,
});
return txRequest;
const fee = await this.getFeeOptions({ calls, ...opts });
const { nonce, cancellable } = opts;
return await this.wallet.createTxExecutionRequest({ calls, fee, nonce, cancellable });
}

/**
Expand Down
Loading

0 comments on commit c9a4d88

Please sign in to comment.