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: Use gas estimation in aztecjs contract function interactions #6260

Merged
merged 4 commits into from
May 14, 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
53 changes: 40 additions & 13 deletions yarn-project/aztec.js/src/contract/base_contract_interaction.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
import { type PXE, type Tx, type TxExecutionRequest } from '@aztec/circuit-types';
import { type Tx, type TxExecutionRequest } from '@aztec/circuit-types';
import { GasSettings } from '@aztec/circuits.js';

import { type FeeOptions } from '../entrypoint/entrypoint.js';
import { type Wallet } from '../account/wallet.js';
import { type ExecutionRequestInit, type FeeOptions } from '../entrypoint/entrypoint.js';
import { getGasLimits } from './get_gas_limits.js';
import { SentTx } from './sent_tx.js';

/**
* Represents options for calling a (constrained) function in a contract.
* Allows the user to specify the sender address and nonce for a transaction.
*/
export type SendMethodOptions = {
/**
* Wether to skip the simulation of the public part of the transaction.
*/
/** Wether to skip the simulation of the public part of the transaction. */
skipPublicSimulation?: boolean;

/**
* The fee options for the transaction.
*/
/** 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;
};

/**
Expand All @@ -27,7 +27,7 @@ export abstract class BaseContractInteraction {
protected tx?: Tx;
protected txRequest?: TxExecutionRequest;

constructor(protected pxe: PXE) {}
constructor(protected wallet: Wallet) {}

Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we need to make this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just noted that all derived classes were requiring a Wallet and not a PXE, so I lifted that to the base class.

/**
* Create a transaction execution request ready to be simulated.
Expand All @@ -43,7 +43,7 @@ export abstract class BaseContractInteraction {
*/
public async prove(options: SendMethodOptions = {}): Promise<Tx> {
const txRequest = this.txRequest ?? (await this.create(options));
this.tx = await this.pxe.proveTx(txRequest, !options.skipPublicSimulation);
this.tx = await this.wallet.proveTx(txRequest, !options.skipPublicSimulation);
return this.tx;
}

Expand All @@ -59,9 +59,36 @@ export abstract class BaseContractInteraction {
public send(options: SendMethodOptions = {}) {
const promise = (async () => {
const tx = this.tx ?? (await this.prove(options));
return this.pxe.sendTx(tx);
return this.wallet.sendTx(tx);
})();

return new SentTx(this.pxe, promise);
return new SentTx(this.wallet, promise);
}

/**
* Estimates gas for a given tx request and returns defaults gas settings for it.
* @param txRequest - Transaction execution request to process.
* @returns Gas settings.
*/
protected async estimateGas(txRequest: TxExecutionRequest) {
const simulationResult = await this.wallet.simulateTx(txRequest, true);
const { totalGas: gasLimits, teardownGas: teardownGasLimits } = getGasLimits(simulationResult);
return GasSettings.default({ gasLimits, teardownGasLimits });
}

/**
* Helper method to return fee options based on the user opts, estimating tx gas if needed.
* @param request - Request to execute for this interaction.
* @returns Fee options for the actual transaction.
*/
protected async getFeeOptions(request: ExecutionRequestInit) {
const fee = request.fee;
if (fee) {
const txRequest = await this.wallet.createTxExecutionRequest(request);
const { gasLimits, teardownGasLimits } = await this.estimateGas(txRequest);
const gasSettings = GasSettings.default({ ...fee.gasSettings, gasLimits, teardownGasLimits });
return { ...fee, gasSettings };
}
return fee;
}
}
9 changes: 4 additions & 5 deletions yarn-project/aztec.js/src/contract/batch_call.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { BaseContractInteraction, type SendMethodOptions } from './base_contract

/** A batch of function calls to be sent as a single transaction through a wallet. */
export class BatchCall extends BaseContractInteraction {
constructor(protected wallet: Wallet, protected calls: FunctionCall[]) {
constructor(wallet: Wallet, protected calls: FunctionCall[]) {
super(wallet);
}

Expand All @@ -17,10 +17,9 @@ export class BatchCall extends BaseContractInteraction {
*/
public async create(opts?: SendMethodOptions): Promise<TxExecutionRequest> {
if (!this.txRequest) {
this.txRequest = await this.wallet.createTxExecutionRequest({
calls: this.calls,
fee: opts?.fee,
});
const calls = this.calls;
const fee = opts?.estimateGas ? await this.getFeeOptions({ calls, fee: opts?.fee }) : opts?.fee;
this.txRequest = await this.wallet.createTxExecutionRequest({ calls, fee });
}
return this.txRequest;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export type SimulateMethodOptions = {
*/
export class ContractFunctionInteraction extends BaseContractInteraction {
constructor(
protected wallet: Wallet,
wallet: Wallet,
protected contractAddress: AztecAddress,
protected functionDao: FunctionAbi,
protected args: any[],
Expand All @@ -47,10 +47,9 @@ export class ContractFunctionInteraction extends BaseContractInteraction {
throw new Error("Can't call `create` on an unconstrained function.");
}
if (!this.txRequest) {
this.txRequest = await this.wallet.createTxExecutionRequest({
calls: [this.request()],
fee: opts?.fee,
});
const calls = [this.request()];
const fee = opts?.estimateGas ? await this.getFeeOptions({ calls, fee: opts?.fee }) : opts?.fee;
this.txRequest = await this.wallet.createTxExecutionRequest({ calls, fee });
}
return this.txRequest;
}
Expand Down Expand Up @@ -98,12 +97,12 @@ export class ContractFunctionInteraction extends BaseContractInteraction {
argsOfCalls: [packedArgs],
authWitnesses: [],
});
const simulatedTx = await this.pxe.simulateTx(txRequest, true, options.from ?? this.wallet.getAddress());
const simulatedTx = await this.wallet.simulateTx(txRequest, true, options.from ?? this.wallet.getAddress());
const flattened = simulatedTx.privateReturnValues;
return flattened ? decodeReturnValues(this.functionDao, flattened) : [];
} else {
const txRequest = await this.create();
const simulatedTx = await this.pxe.simulateTx(txRequest, true);
const simulatedTx = await this.wallet.simulateTx(txRequest, true);
this.txRequest = undefined;
const flattened = simulatedTx.publicOutput?.publicReturnValues;
return flattened ? decodeReturnValues(this.functionDao, flattened) : [];
Expand Down
20 changes: 15 additions & 5 deletions yarn-project/aztec.js/src/contract/deploy_method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class DeployMethod<TContract extends ContractBase = Contract> extends Bas

constructor(
private publicKeysHash: Fr,
protected wallet: Wallet,
wallet: Wallet,
private artifact: ContractArtifact,
private postDeployCtor: (address: AztecAddress, wallet: Wallet) => Promise<TContract>,
private args: any[] = [],
Expand All @@ -80,11 +80,14 @@ export class DeployMethod<TContract extends ContractBase = Contract> extends Bas
if (!this.txRequest) {
this.txRequest = await this.wallet.createTxExecutionRequest(await this.request(options));
// TODO: Should we add the contracts to the DB here, or once the tx has been sent or mined?
await this.pxe.registerContract({ artifact: this.artifact, instance: this.instance! });
await this.wallet.registerContract({ artifact: this.artifact, instance: this.instance! });
}
return this.txRequest;
}

// REFACTOR: Having a `request` method with different semantics than the ones in the other
// derived ContractInteractions is confusing. We should unify the flow of all ContractInteractions.

/**
* Returns an array of function calls that represent this operation. Useful as a building
* block for constructing batch requests.
Expand All @@ -102,13 +105,20 @@ export class DeployMethod<TContract extends ContractBase = Contract> extends Bas
throw new Error(`No function calls needed to deploy contract ${this.artifact.name}`);
}

this.functionCalls = {
const request = {
calls: [...deployment.calls, ...bootstrap.calls],
authWitnesses: [...(deployment.authWitnesses ?? []), ...(bootstrap.authWitnesses ?? [])],
packedArguments: [...(deployment.packedArguments ?? []), ...(bootstrap.packedArguments ?? [])],
fee: options.fee,
};

if (options.estimateGas) {
request.fee = await this.getFeeOptions(request);
}

this.functionCalls = request;
}

return this.functionCalls;
}

Expand All @@ -134,7 +144,7 @@ export class DeployMethod<TContract extends ContractBase = Contract> extends Bas

// Register the contract class if it hasn't been published already.
if (!options.skipClassRegistration) {
if (await this.pxe.isContractClassPubliclyRegistered(contractClass.id)) {
if (await this.wallet.isContractClassPubliclyRegistered(contractClass.id)) {
this.log.debug(
`Skipping registration of already registered contract class ${contractClass.id.toString()} for ${instance.address.toString()}`,
);
Expand Down Expand Up @@ -192,7 +202,7 @@ export class DeployMethod<TContract extends ContractBase = Contract> extends Bas
this.log.debug(
`Sent deployment tx of ${this.artifact.name} contract with deployment address ${instance.address.toString()}`,
);
return new DeploySentTx(this.pxe, txHashPromise, this.postDeployCtor, instance);
return new DeploySentTx(this.wallet, txHashPromise, this.postDeployCtor, instance);
}

/**
Expand Down
41 changes: 41 additions & 0 deletions yarn-project/aztec.js/src/contract/get_gas_limits.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { PublicKernelType, type SimulatedTx, mockSimulatedTx } from '@aztec/circuit-types';
import { Gas } from '@aztec/circuits.js';

import { getGasLimits } from './get_gas_limits.js';

describe('getGasLimits', () => {
let simulatedTx: SimulatedTx;

beforeEach(() => {
simulatedTx = mockSimulatedTx();
simulatedTx.tx.data.publicInputs.end.gasUsed = Gas.from({ daGas: 100, l2Gas: 200 });
simulatedTx.publicOutput!.gasUsed = {
[PublicKernelType.SETUP]: Gas.from({ daGas: 10, l2Gas: 20 }),
[PublicKernelType.APP_LOGIC]: Gas.from({ daGas: 20, l2Gas: 40 }),
[PublicKernelType.TEARDOWN]: Gas.from({ daGas: 10, l2Gas: 20 }),
};
});

it('returns gas limits from private gas usage only', () => {
simulatedTx.publicOutput = undefined;
// Should be 110 and 220 but oh floating point
expect(getGasLimits(simulatedTx)).toEqual({
totalGas: Gas.from({ daGas: 111, l2Gas: 221 }),
teardownGas: Gas.empty(),
});
});

it('returns gas limits for private and public', () => {
expect(getGasLimits(simulatedTx)).toEqual({
totalGas: Gas.from({ daGas: 154, l2Gas: 308 }),
teardownGas: Gas.from({ daGas: 11, l2Gas: 22 }),
});
});

it('pads gas limits', () => {
expect(getGasLimits(simulatedTx, 1)).toEqual({
totalGas: Gas.from({ daGas: 280, l2Gas: 560 }),
teardownGas: Gas.from({ daGas: 20, l2Gas: 40 }),
});
});
});
24 changes: 24 additions & 0 deletions yarn-project/aztec.js/src/contract/get_gas_limits.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { PublicKernelType, type SimulatedTx } from '@aztec/circuit-types';
import { Gas } from '@aztec/circuits.js';

/**
* Returns suggested total and teardown gas limits for a simulated tx.
* Note that public gas usage is only accounted for if the publicOutput is present.
* @param pad - Percentage to pad the suggested gas limits by, defaults to 10%.
*/
export function getGasLimits(simulatedTx: SimulatedTx, pad = 0.1) {
const privateGasUsed = simulatedTx.tx.data.publicInputs.end.gasUsed;
if (simulatedTx.publicOutput) {
const publicGasUsed = Object.values(simulatedTx.publicOutput.gasUsed)
.filter(Boolean)
.reduce((total, current) => total.add(current), Gas.empty());
const teardownGas = simulatedTx.publicOutput.gasUsed[PublicKernelType.TEARDOWN] ?? Gas.empty();

return {
totalGas: privateGasUsed.add(publicGasUsed).mul(1 + pad),
teardownGas: teardownGas.mul(1 + pad),
};
}

return { totalGas: privateGasUsed.mul(1 + pad), teardownGas: Gas.empty() };
}
37 changes: 0 additions & 37 deletions yarn-project/circuit-types/src/tx/simulated_tx.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import { Gas } from '@aztec/circuits.js';

import { mockSimulatedTx } from '../mocks.js';
import { PublicKernelType } from './processed_tx.js';
import { SimulatedTx } from './simulated_tx.js';

describe('simulated_tx', () => {
Expand All @@ -22,38 +19,4 @@ describe('simulated_tx', () => {
expect(SimulatedTx.fromJSON(simulatedTx.toJSON())).toEqual(simulatedTx);
});
});

describe('getGasLimits', () => {
beforeEach(() => {
simulatedTx.tx.data.publicInputs.end.gasUsed = Gas.from({ daGas: 100, l2Gas: 200 });
simulatedTx.publicOutput!.gasUsed = {
[PublicKernelType.SETUP]: Gas.from({ daGas: 10, l2Gas: 20 }),
[PublicKernelType.APP_LOGIC]: Gas.from({ daGas: 20, l2Gas: 40 }),
[PublicKernelType.TEARDOWN]: Gas.from({ daGas: 10, l2Gas: 20 }),
};
});

it('returns gas limits from private gas usage only', () => {
simulatedTx.publicOutput = undefined;
// Should be 110 and 220 but oh floating point
expect(simulatedTx.getGasLimits()).toEqual({
totalGas: Gas.from({ daGas: 111, l2Gas: 221 }),
teardownGas: Gas.empty(),
});
});

it('returns gas limits for private and public', () => {
expect(simulatedTx.getGasLimits()).toEqual({
totalGas: Gas.from({ daGas: 154, l2Gas: 308 }),
teardownGas: Gas.from({ daGas: 11, l2Gas: 22 }),
});
});

it('pads gas limits', () => {
expect(simulatedTx.getGasLimits(1)).toEqual({
totalGas: Gas.from({ daGas: 280, l2Gas: 560 }),
teardownGas: Gas.from({ daGas: 20, l2Gas: 40 }),
});
});
});
});
26 changes: 1 addition & 25 deletions yarn-project/circuit-types/src/tx/simulated_tx.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Fr, Gas } from '@aztec/circuits.js';
import { Fr } from '@aztec/circuits.js';

import { PublicKernelType } from './processed_tx.js';
import { type ProcessReturnValues, PublicSimulationOutput } from './public_simulation_output.js';
import { Tx } from './tx.js';

Expand All @@ -17,29 +16,6 @@ export class SimulatedTx {
public publicOutput?: PublicSimulationOutput,
) {}

/**
* Returns suggested total and teardown gas limits for the simulated tx.
* Note that public gas usage is only accounted for if the publicOutput is present.
* @param pad - Percentage to pad the suggested gas limits by, defaults to 10%.
*/
public getGasLimits(pad = 0.1) {
const privateGasUsed = this.tx.data.publicInputs.end.gasUsed;
if (this.publicOutput) {
const publicGasUsed = Object.values(this.publicOutput.gasUsed).reduce(
(total, current) => total.add(current),
Gas.empty(),
);
const teardownGas = this.publicOutput.gasUsed[PublicKernelType.TEARDOWN] ?? Gas.empty();

return {
totalGas: privateGasUsed.add(publicGasUsed).mul(1 + pad),
teardownGas: teardownGas.mul(1 + pad),
};
}

return { totalGas: privateGasUsed.mul(1 + pad), teardownGas: Gas.empty() };
}

/**
* Convert a SimulatedTx class object to a plain JSON object.
* @returns A plain object with SimulatedTx properties.
Expand Down
15 changes: 8 additions & 7 deletions yarn-project/circuits.js/src/structs/gas_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,14 @@ export class GasSettings {
}

/** Default gas settings to use when user has not provided them. */
static default() {
return new GasSettings(
new Gas(DEFAULT_GAS_LIMIT, DEFAULT_GAS_LIMIT),
new Gas(DEFAULT_TEARDOWN_GAS_LIMIT, DEFAULT_TEARDOWN_GAS_LIMIT),
new GasFees(new Fr(DEFAULT_MAX_FEE_PER_GAS), new Fr(DEFAULT_MAX_FEE_PER_GAS)),
new Fr(DEFAULT_INCLUSION_FEE),
);
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) },
inclusionFee: new Fr(DEFAULT_INCLUSION_FEE),
...overrides,
});
}

/** Gas settings to use for simulating a contract call. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe('e2e_fees dapp_subscription', () => {

beforeAll(async () => {
await t.applyBaseSnapshots();
await t.applyFundAlice();
await t.applyFundAliceWithBananas();
await t.applySetupSubscription();

({
Expand Down
Loading
Loading