Skip to content

Commit

Permalink
chore: add end_gas_used to avm public inputs (#9910)
Browse files Browse the repository at this point in the history
Please read [contributing guidelines](CONTRIBUTING.md) and remove this
line.
  • Loading branch information
LeilaWang authored Nov 13, 2024
1 parent f462657 commit 3889def
Show file tree
Hide file tree
Showing 10 changed files with 60 additions and 22 deletions.
1 change: 1 addition & 0 deletions barretenberg/cpp/src/barretenberg/vm/aztec_constants.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#define PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH 866
#define PUBLIC_CONTEXT_INPUTS_LENGTH 41
#define AVM_ACCUMULATED_DATA_LENGTH 318
#define AVM_CIRCUIT_PUBLIC_INPUTS_LENGTH 1006
#define AVM_VERIFICATION_KEY_LENGTH_IN_FIELDS 86
#define AVM_PROOF_LENGTH_IN_FIELDS 4291
#define AVM_PUBLIC_COLUMN_MAX_SIZE 1024
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use dep::types::{
abis::{
accumulated_data::CombinedAccumulatedData,
combined_constant_data::CombinedConstantData,
gas::Gas,
log_hash::{LogHash, ScopedLogHash},
},
constants::{
Expand Down Expand Up @@ -105,7 +104,7 @@ impl PublicBaseRollupInputs {
end,
start_state,
revert_code,
gas_used: Gas::empty(), // gas_used is not used in rollup circuits.
gas_used: from_public.end_gas_used,
fee_payer: from_private.fee_payer,
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pub struct AvmCircuitPublicInputs {
///////////////////////////////////
// Outputs.
end_tree_snapshots: TreeSnapshots,
end_gas_used: Gas,
accumulated_data: AvmAccumulatedData,
transaction_fee: Field,
reverted: bool,
Expand All @@ -55,6 +56,7 @@ impl Empty for AvmCircuitPublicInputs {
previous_non_revertible_accumulated_data: PrivateToAvmAccumulatedData::empty(),
previous_revertible_accumulated_data: PrivateToAvmAccumulatedData::empty(),
end_tree_snapshots: TreeSnapshots::empty(),
end_gas_used: Gas::empty(),
accumulated_data: AvmAccumulatedData::empty(),
transaction_fee: 0,
reverted: false,
Expand Down Expand Up @@ -88,6 +90,7 @@ impl Eq for AvmCircuitPublicInputs {
== other.previous_revertible_accumulated_data
)
& (self.end_tree_snapshots == other.end_tree_snapshots)
& (self.end_gas_used == other.end_gas_used)
& (self.accumulated_data == other.accumulated_data)
& (self.transaction_fee == other.transaction_fee)
& (self.reverted == other.reverted)
Expand Down Expand Up @@ -116,6 +119,7 @@ impl Serialize<AVM_CIRCUIT_PUBLIC_INPUTS_LENGTH> for AvmCircuitPublicInputs {
fields.extend_from_array(self.previous_non_revertible_accumulated_data.serialize());
fields.extend_from_array(self.previous_revertible_accumulated_data.serialize());
fields.extend_from_array(self.end_tree_snapshots.serialize());
fields.extend_from_array(self.end_gas_used.serialize());
fields.extend_from_array(self.accumulated_data.serialize());
fields.push(self.transaction_fee);
fields.push(self.reverted as Field);
Expand Down Expand Up @@ -156,6 +160,7 @@ impl Deserialize<AVM_CIRCUIT_PUBLIC_INPUTS_LENGTH> for AvmCircuitPublicInputs {
PrivateToAvmAccumulatedData::deserialize,
),
end_tree_snapshots: reader.read_struct(TreeSnapshots::deserialize),
end_gas_used: reader.read_struct(Gas::deserialize),
accumulated_data: reader.read_struct(AvmAccumulatedData::deserialize),
transaction_fee: reader.read(),
reverted: reader.read() as bool,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -476,10 +476,10 @@ pub global AVM_CIRCUIT_PUBLIC_INPUTS_LENGTH: u32 = GLOBAL_VARIABLES_LENGTH
+ PRIVATE_TO_AVM_ACCUMULATED_DATA_LENGTH /* previous_non_revertible_accumulated_data */
+ PRIVATE_TO_AVM_ACCUMULATED_DATA_LENGTH /* previous_revertible_accumulated_data */
+ TREE_SNAPSHOTS_LENGTH /* end_tree_snapshots */
+ GAS_LENGTH /* end_gas_used */
+ AVM_ACCUMULATED_DATA_LENGTH
+ 1 /* transaction_fee */
+ 1 /* reverted */
;
+ 1 /* reverted */;

pub global CONSTANT_ROLLUP_DATA_LENGTH: u32 = APPEND_ONLY_TREE_SNAPSHOT_LENGTH
+ 1 /* vk_tree_root */
Expand Down
1 change: 1 addition & 0 deletions yarn-project/circuits.js/src/constants.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ export const PRIVATE_TO_PUBLIC_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 1196;
export const PUBLIC_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2931;
export const VM_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2340;
export const KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 600;
export const AVM_CIRCUIT_PUBLIC_INPUTS_LENGTH = 1006;
export const CONSTANT_ROLLUP_DATA_LENGTH = 13;
export const BASE_OR_MERGE_PUBLIC_INPUTS_LENGTH = 30;
export const BLOCK_ROOT_OR_BLOCK_MERGE_PUBLIC_INPUTS_LENGTH = 90;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export class AvmCircuitPublicInputs {
public previousNonRevertibleAccumulatedData: PrivateToAvmAccumulatedData,
public previousRevertibleAccumulatedData: PrivateToAvmAccumulatedData,
public endTreeSnapshots: TreeSnapshots,
public endGasUsed: Gas,
public accumulatedData: AvmAccumulatedData,
public transactionFee: Fr,
public reverted: boolean,
Expand All @@ -50,6 +51,7 @@ export class AvmCircuitPublicInputs {
reader.readObject(PrivateToAvmAccumulatedData),
reader.readObject(PrivateToAvmAccumulatedData),
reader.readObject(TreeSnapshots),
reader.readObject(Gas),
reader.readObject(AvmAccumulatedData),
reader.readObject(Fr),
reader.readBoolean(),
Expand All @@ -70,6 +72,7 @@ export class AvmCircuitPublicInputs {
this.previousNonRevertibleAccumulatedData,
this.previousRevertibleAccumulatedData,
this.endTreeSnapshots,
this.endGasUsed,
this.accumulatedData,
this.transactionFee,
this.reverted,
Expand Down Expand Up @@ -99,6 +102,7 @@ export class AvmCircuitPublicInputs {
PrivateToAvmAccumulatedData.fromFields(reader),
PrivateToAvmAccumulatedData.fromFields(reader),
TreeSnapshots.fromFields(reader),
Gas.fromFields(reader),
AvmAccumulatedData.fromFields(reader),
reader.readField(),
reader.readBoolean(),
Expand All @@ -119,6 +123,7 @@ export class AvmCircuitPublicInputs {
PrivateToAvmAccumulatedData.empty(),
PrivateToAvmAccumulatedData.empty(),
TreeSnapshots.empty(),
Gas.empty(),
AvmAccumulatedData.empty(),
Fr.zero(),
false,
Expand Down Expand Up @@ -147,6 +152,7 @@ export class AvmCircuitPublicInputs {
previousNonRevertibleAccumulatedData: ${inspect(this.previousNonRevertibleAccumulatedData)},
previousRevertibleAccumulatedData: ${inspect(this.previousRevertibleAccumulatedData)},
endTreeSnapshots: ${inspect(this.endTreeSnapshots)},
endGasUsed: ${inspect(this.endGasUsed)},
accumulatedData: ${inspect(this.accumulatedData)},
transactionFee: ${inspect(this.transactionFee)},
reverted: ${this.reverted},
Expand Down
5 changes: 3 additions & 2 deletions yarn-project/circuits.js/src/tests/factories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ function makeAvmCircuitPublicInputs(seed = 1) {
return new AvmCircuitPublicInputs(
makeGlobalVariables(seed),
makeTreeSnapshots(seed + 0x10),
makeGas(),
makeGas(seed + 0x20),
makeGasSettings(),
makeTuple(MAX_ENQUEUED_CALLS_PER_TX, makePublicCallRequest, seed + 0x100),
makeTuple(MAX_ENQUEUED_CALLS_PER_TX, makePublicCallRequest, seed + 0x200),
Expand All @@ -659,8 +659,9 @@ function makeAvmCircuitPublicInputs(seed = 1) {
makePrivateToAvmAccumulatedData(seed + 0x500),
makePrivateToAvmAccumulatedData(seed + 0x600),
makeTreeSnapshots(seed + 0x700),
makeGas(seed + 0x750),
makeAvmAccumulatedData(seed + 0x800),
fr(seed + 0x700),
fr(seed + 0x900),
false,
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2126,6 +2126,7 @@ function mapAvmCircuitPublicInputsToNoir(inputs: AvmCircuitPublicInputs): AvmCir
inputs.previousRevertibleAccumulatedData,
),
end_tree_snapshots: mapTreeSnapshotsToNoir(inputs.endTreeSnapshots),
end_gas_used: mapGasToNoir(inputs.endGasUsed),
accumulated_data: mapAvmAccumulatedDataToNoir(inputs.accumulatedData),
transaction_fee: mapFieldToNoir(inputs.transactionFee),
reverted: inputs.reverted,
Expand Down
29 changes: 19 additions & 10 deletions yarn-project/simulator/src/public/enqueued_calls_processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,9 @@ describe('enqueued_calls_processor', () => {

const output = txResult.avmProvingRequest!.inputs.output;

const expectedGasUsedForFee = expectedTotalGas;
const expectedTxFee = expectedTotalGas.computeFee(gasFees);
expect(output.endGasUsed).toEqual(expectedGasUsedForFee);
expect(output.transactionFee).toEqual(expectedTxFee);

// We keep all data.
Expand Down Expand Up @@ -235,7 +237,9 @@ describe('enqueued_calls_processor', () => {

const output = txResult.avmProvingRequest!.inputs.output;

const expectedGasUsedForFee = expectedTotalGas;
const expectedTxFee = expectedTotalGas.computeFee(gasFees);
expect(output.endGasUsed).toEqual(expectedGasUsedForFee);
expect(output.transactionFee).toEqual(expectedTxFee);

// We keep all data.
Expand Down Expand Up @@ -266,8 +270,9 @@ describe('enqueued_calls_processor', () => {

const output = txResult.avmProvingRequest!.inputs.output;

const expectedTotalGasForFee = expectedTotalGas.sub(expectedTeardownGasUsed).add(teardownGasLimits);
const expectedTxFee = expectedTotalGasForFee.computeFee(gasFees);
const expectedGasUsedForFee = expectedTotalGas.sub(expectedTeardownGasUsed).add(teardownGasLimits);
const expectedTxFee = expectedGasUsedForFee.computeFee(gasFees);
expect(output.endGasUsed).toEqual(expectedGasUsedForFee);
expect(output.transactionFee).toEqual(expectedTxFee);

// We keep all data.
Expand Down Expand Up @@ -313,8 +318,9 @@ describe('enqueued_calls_processor', () => {

const output = txResult.avmProvingRequest!.inputs.output;

const expectedTotalGasForFee = expectedTotalGas.sub(expectedTeardownGasUsed).add(teardownGasLimits);
const expectedTxFee = expectedTotalGasForFee.computeFee(gasFees);
const expectedGasUsedForFee = expectedTotalGas.sub(expectedTeardownGasUsed).add(teardownGasLimits);
const expectedTxFee = expectedGasUsedForFee.computeFee(gasFees);
expect(output.endGasUsed).toEqual(expectedGasUsedForFee);
expect(output.transactionFee).toEqual(expectedTxFee);

// We keep all data.
Expand Down Expand Up @@ -459,8 +465,9 @@ describe('enqueued_calls_processor', () => {

const output = txResult.avmProvingRequest!.inputs.output;

const expectedTotalGasForFee = expectedTotalGas.sub(expectedTeardownGasUsed).add(teardownGasLimits);
const expectedTxFee = expectedTotalGasForFee.computeFee(gasFees);
const expectedGasUsedForFee = expectedTotalGas.sub(expectedTeardownGasUsed).add(teardownGasLimits);
const expectedTxFee = expectedGasUsedForFee.computeFee(gasFees);
expect(output.endGasUsed).toEqual(expectedGasUsedForFee);
expect(output.transactionFee).toEqual(expectedTxFee);

// we keep the non-revertible data.
Expand Down Expand Up @@ -535,8 +542,9 @@ describe('enqueued_calls_processor', () => {
const output = txResult.avmProvingRequest!.inputs.output;

// Should still charge the full teardownGasLimits for fee even though teardown reverted.
const expectedTotalGasForFee = expectedTotalGas.sub(expectedTeardownGasUsed).add(teardownGasLimits);
const expectedTxFee = expectedTotalGasForFee.computeFee(gasFees);
const expectedGasUsedForFee = expectedTotalGas.sub(expectedTeardownGasUsed).add(teardownGasLimits);
const expectedTxFee = expectedGasUsedForFee.computeFee(gasFees);
expect(output.endGasUsed).toEqual(expectedGasUsedForFee);
expect(output.transactionFee).toEqual(expectedTxFee);

// We keep the non-revertible data.
Expand Down Expand Up @@ -618,8 +626,9 @@ describe('enqueued_calls_processor', () => {
const output = txResult.avmProvingRequest!.inputs.output;

// Should still charge the full teardownGasLimits for fee even though teardown reverted.
const expectedTotalGasForFee = expectedTotalGas.sub(expectedTeardownGasUsed).add(teardownGasLimits);
const expectedTxFee = expectedTotalGasForFee.computeFee(gasFees);
const expectedGasUsedForFee = expectedTotalGas.sub(expectedTeardownGasUsed).add(teardownGasLimits);
const expectedTxFee = expectedGasUsedForFee.computeFee(gasFees);
expect(output.endGasUsed).toEqual(expectedGasUsedForFee);
expect(output.transactionFee).toEqual(expectedTxFee);

// we keep the non-revertible data
Expand Down
27 changes: 21 additions & 6 deletions yarn-project/simulator/src/public/enqueued_calls_processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,14 @@ export class EnqueuedCallsProcessor {
},
);

const gasUsedForFee = this.getGasUsedForFee(tx, phaseGasUsed);
const transactionFee = this.getTransactionFee(tx, phaseGasUsed);
avmProvingRequest!.inputs.output = this.generateAvmCircuitPublicInputs(tx, tailKernelOutput, transactionFee);
avmProvingRequest!.inputs.output = this.generateAvmCircuitPublicInputs(
tx,
tailKernelOutput,
gasUsedForFee,
transactionFee,
);

const gasUsed = {
totalGas: this.getActualGasUsed(tx, phaseGasUsed),
Expand Down Expand Up @@ -412,16 +418,19 @@ export class EnqueuedCallsProcessor {

private getTransactionFee(tx: Tx, phaseGasUsed: PhaseGasUsed): Fr {
const gasFees = this.globalVariables.gasFees;
const txFee = tx.data.gasUsed // This should've included teardown gas limits.
.add(phaseGasUsed[TxExecutionPhase.SETUP])
.add(phaseGasUsed[TxExecutionPhase.APP_LOGIC])
.computeFee(gasFees);
const txFee = this.getGasUsedForFee(tx, phaseGasUsed).computeFee(gasFees);

this.log.debug(`Computed tx fee`, { txFee, gasUsed: inspect(phaseGasUsed), gasFees: inspect(gasFees) });

return txFee;
}

private getGasUsedForFee(tx: Tx, phaseGasUsed: PhaseGasUsed) {
return tx.data.gasUsed // This should've included teardown gas limits.
.add(phaseGasUsed[TxExecutionPhase.SETUP])
.add(phaseGasUsed[TxExecutionPhase.APP_LOGIC]);
}

private getActualGasUsed(tx: Tx, phaseGasUsed: PhaseGasUsed) {
const requireTeardown = tx.data.hasTeardownPublicCallRequest();
const teardownGasLimits = tx.data.constants.txContext.gasSettings.teardownGasLimits;
Expand Down Expand Up @@ -488,7 +497,12 @@ export class EnqueuedCallsProcessor {
}

// Temporary hack to create the AvmCircuitPublicInputs from public tail's public inputs.
private generateAvmCircuitPublicInputs(tx: Tx, tailOutput: KernelCircuitPublicInputs, transactionFee: Fr) {
private generateAvmCircuitPublicInputs(
tx: Tx,
tailOutput: KernelCircuitPublicInputs,
gasUsedForFee: Gas,
transactionFee: Fr,
) {
const startTreeSnapshots = new TreeSnapshots(
tailOutput.constants.historicalHeader.state.l1ToL2MessageTree,
tailOutput.startState.noteHashTree,
Expand Down Expand Up @@ -532,6 +546,7 @@ export class EnqueuedCallsProcessor {
convertAccumulatedData(tx.data.forPublic!.nonRevertibleAccumulatedData),
convertAccumulatedData(tx.data.forPublic!.revertibleAccumulatedData),
endTreeSnapshots,
gasUsedForFee,
convertAvmAccumulatedData(tailOutput.end),
transactionFee,
!tailOutput.revertCode.equals(RevertCode.OK),
Expand Down

1 comment on commit 3889def

@AztecBot
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.

Benchmark suite Current: 3889def Previous: 9325f6f Ratio
nativeconstruct_proof_ultrahonk_power_of_2/20 5778.846983999998 ms/iter 5342.71464199999 ms/iter 1.08

This comment was automatically generated by workflow using github-action-benchmark.

CC: @ludamad @codygunton

Please sign in to comment.