Skip to content

Commit

Permalink
Revert "feat: new per-enqueued-call gas limit (#9033)"
Browse files Browse the repository at this point in the history
This reverts commit 6ef0895.
  • Loading branch information
ludamad authored Oct 9, 2024
1 parent 18d24fb commit c481a09
Show file tree
Hide file tree
Showing 15 changed files with 29 additions and 92 deletions.
25 changes: 3 additions & 22 deletions barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1760,30 +1760,11 @@ TEST_F(AvmExecutionTests, daGasLeft)
validate_trace(std::move(trace), public_inputs);
}

TEST_F(AvmExecutionTests, ExecutorThrowsWithTooMuchGasAllocated)
{
std::string bytecode_hex = to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16(sender)
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(EnvironmentVariable::SENDER)) + "0007"; // addr 7

std::vector<FF> calldata = {};
std::vector<FF> returndata = {};
std::vector<FF> public_inputs_vec(PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH, 0);
public_inputs_vec[L2_START_GAS_LEFT_PCPI_OFFSET] = MAX_L2_GAS_PER_ENQUEUED_CALL + 1;

auto bytecode = hex_to_bytes(bytecode_hex);
auto instructions = Deserialization::parse(bytecode);

EXPECT_THROW_WITH_MESSAGE(
Execution::gen_trace(instructions, returndata, calldata, public_inputs_vec),
"Cannot allocate more than MAX_L2_GAS_PER_ENQUEUED_CALL to the AVM for execution of an enqueued call");
}

// Should throw whenever the wrong number of public inputs are provided
TEST_F(AvmExecutionTests, ExecutorThrowsWithIncorrectNumberOfPublicInputs)
{
std::string bytecode_hex = to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16(sender)
"00" // Indirect flag
std::string bytecode_hex = to_hex(OpCode::GETENVVAR_16) + // opcode SENDER
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(EnvironmentVariable::SENDER)) + "0007"; // addr 7

std::vector<FF> calldata = {};
Expand All @@ -1793,7 +1774,7 @@ TEST_F(AvmExecutionTests, ExecutorThrowsWithIncorrectNumberOfPublicInputs)
auto bytecode = hex_to_bytes(bytecode_hex);
auto instructions = Deserialization::parse(bytecode);

EXPECT_THROW_WITH_MESSAGE(Execution::gen_trace(instructions, returndata, calldata, public_inputs_vec),
EXPECT_THROW_WITH_MESSAGE(Execution::gen_trace(instructions, calldata, returndata, public_inputs_vec),
"Public inputs vector is not of PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH");
}

Expand Down
15 changes: 0 additions & 15 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,6 @@ template <typename FF_> VmPublicInputs<FF_> convert_public_inputs(std::vector<FF
throw_or_abort("Public inputs vector is not of PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH");
}

// WARNING: this must be constrained by the kernel!
// Here this is just a sanity check to prevent generation of proofs that
// will be thrown out by the kernel anyway.
if constexpr (IsAnyOf<FF_, bb::fr>) {
if (public_inputs_vec[L2_START_GAS_LEFT_PCPI_OFFSET] > MAX_L2_GAS_PER_ENQUEUED_CALL) {
throw_or_abort(
"Cannot allocate more than MAX_L2_GAS_PER_ENQUEUED_CALL to the AVM for execution of an enqueued call");
}
} else {
if (public_inputs_vec[L2_START_GAS_LEFT_PCPI_OFFSET].get_value() > MAX_L2_GAS_PER_ENQUEUED_CALL) {
throw_or_abort(
"Cannot allocate more than MAX_L2_GAS_PER_ENQUEUED_CALL to the AVM for execution of an enqueued call");
}
}

std::array<FF_, KERNEL_INPUTS_LENGTH>& kernel_inputs = std::get<KERNEL_INPUTS>(public_inputs);

// Copy items from PublicCircuitPublicInputs vector to public input columns
Expand Down
1 change: 0 additions & 1 deletion barretenberg/cpp/src/barretenberg/vm/aztec_constants.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#define MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL 16
#define MAX_L1_TO_L2_MSG_READ_REQUESTS_PER_CALL 16
#define MAX_UNENCRYPTED_LOGS_PER_CALL 4
#define MAX_L2_GAS_PER_ENQUEUED_CALL 5000000
#define AZTEC_ADDRESS_LENGTH 1
#define GAS_FEES_LENGTH 2
#define GAS_LENGTH 2
Expand Down
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 @@ -117,7 +117,6 @@ library Constants {
14061769416655647708490531650437236735160113654556896985372298487345;
uint256 internal constant DEFAULT_GAS_LIMIT = 1000000000;
uint256 internal constant DEFAULT_TEARDOWN_GAS_LIMIT = 100000000;
uint256 internal constant MAX_L2_GAS_PER_ENQUEUED_CALL = 5000000;
uint256 internal constant DEFAULT_MAX_FEE_PER_GAS = 10;
uint256 internal constant DEFAULT_INCLUSION_FEE = 0;
uint256 internal constant DA_BYTES_PER_FIELD = 32;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use dep::types::{
kernel_circuit_public_inputs::PublicKernelCircuitPublicInputs, enqueued_call_data::EnqueuedCallData,
public_call_request::PublicCallRequest, validation_requests::PublicValidationRequestArrayLengths
},
constants::MAX_L2_GAS_PER_ENQUEUED_CALL, utils::arrays::array_length
utils::arrays::array_length
};

pub struct EnqueuedCallDataValidator {
Expand Down Expand Up @@ -89,23 +89,17 @@ impl EnqueuedCallDataValidator {
// Validates that the start gas injected into the vm circuit matches the remaining gas.
fn validate_start_gas(self, previous_kernel: PublicKernelCircuitPublicInputs) {
let enqueued_call_start_gas = self.enqueued_call.data.start_gas_left;
// NOTE: the AVM circuit will fail to generate a proof if its "start gas" is > MAX_L2_GAS_PER_ENQUEUED_CALL,
// so the kernel never allocates more than that maximum to one enqueued call.
if self.phase != PublicKernelPhase.TEARDOWN {
// An enqueued call's start gas is the remaining gas left in the transaction after the previous kernel.
let tx_gas_limits = previous_kernel.constants.tx_context.gas_settings.gas_limits;
let mut computed_start_gas = tx_gas_limits.sub(previous_kernel.end.gas_used).sub(previous_kernel.end_non_revertible.gas_used);
// Keep L2 gas below max
computed_start_gas.l2_gas = std::cmp::min(computed_start_gas.l2_gas, MAX_L2_GAS_PER_ENQUEUED_CALL);
let computed_start_gas = tx_gas_limits.sub(previous_kernel.end.gas_used).sub(previous_kernel.end_non_revertible.gas_used);
assert_eq(
enqueued_call_start_gas, computed_start_gas, "Start gas for enqueued call does not match transaction gas left (with MAX_L2_GAS_PER_ENQUEUED_CALL applied)"
enqueued_call_start_gas, computed_start_gas, "Start gas for enqueued call does not match transaction gas left"
);
} else {
let mut teardown_gas_limit = previous_kernel.constants.tx_context.gas_settings.teardown_gas_limits;
// Keep L2 gas below max
teardown_gas_limit.l2_gas = std::cmp::min(teardown_gas_limit.l2_gas, MAX_L2_GAS_PER_ENQUEUED_CALL);
let teardown_gas_limit = previous_kernel.constants.tx_context.gas_settings.teardown_gas_limits;
assert_eq(
enqueued_call_start_gas, teardown_gas_limit, "Start gas for enqueued call does not match teardown gas allocation (with MAX_L2_GAS_PER_ENQUEUED_CALL applied)"
enqueued_call_start_gas, teardown_gas_limit, "Start gas for enqueued call does not match teardown gas allocation"
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ global DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE = 0x85864497636cf755ae7bd
// GAS DEFAULTS
global DEFAULT_GAS_LIMIT: u32 = 1_000_000_000;
global DEFAULT_TEARDOWN_GAS_LIMIT: u32 = 100_000_000;
global MAX_L2_GAS_PER_ENQUEUED_CALL: u32 = 5_000_000;
global DEFAULT_MAX_FEE_PER_GAS: Field = 10;
global DEFAULT_INCLUSION_FEE: Field = 0;
global DA_BYTES_PER_FIELD: u32 = 32;
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 =
14061769416655647708490531650437236735160113654556896985372298487345n;
export const DEFAULT_GAS_LIMIT = 1000000000;
export const DEFAULT_TEARDOWN_GAS_LIMIT = 100000000;
export const MAX_L2_GAS_PER_ENQUEUED_CALL = 5000000;
export const DEFAULT_MAX_FEE_PER_GAS = 10;
export const DEFAULT_INCLUSION_FEE = 0;
export const DA_BYTES_PER_FIELD = 32;
Expand Down
1 change: 0 additions & 1 deletion yarn-project/circuits.js/src/scripts/constants.in.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ const CPP_CONSTANTS = [
'MEM_TAG_U64',
'MEM_TAG_U128',
'MEM_TAG_FF',
'MAX_L2_GAS_PER_ENQUEUED_CALL',
];

const CPP_GENERATORS: string[] = [];
Expand Down
3 changes: 1 addition & 2 deletions yarn-project/circuits.js/src/structs/gas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { type FieldsOf } from '@aztec/foundation/types';

import { inspect } from 'util';

import { MAX_L2_GAS_PER_ENQUEUED_CALL } from '../constants.gen.js';
import { type GasFees } from './gas_fees.js';
import { type UInt32 } from './shared.js';

Expand Down Expand Up @@ -37,7 +36,7 @@ export class Gas {

/** Returns large enough gas amounts for testing purposes. */
static test() {
return new Gas(1e9, MAX_L2_GAS_PER_ENQUEUED_CALL);
return new Gas(1e9, 1e9);
}

isEmpty() {
Expand Down
5 changes: 0 additions & 5 deletions yarn-project/simulator/src/avm/avm_simulator.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { MAX_L2_GAS_PER_ENQUEUED_CALL } from '@aztec/circuits.js';
import { type DebugLogger, createDebugLogger } from '@aztec/foundation/log';

import { strict as assert } from 'assert';
Expand All @@ -22,10 +21,6 @@ export class AvmSimulator {
private bytecode: Buffer | undefined;

constructor(private context: AvmContext) {
assert(
context.machineState.gasLeft.l2Gas <= MAX_L2_GAS_PER_ENQUEUED_CALL,
`Cannot allocate more than ${MAX_L2_GAS_PER_ENQUEUED_CALL} to the AVM for execution of an enqueued call`,
);
this.log = createDebugLogger(`aztec:avm_simulator:core(f:${context.environment.functionSelector.toString()})`);
}

Expand Down
4 changes: 2 additions & 2 deletions yarn-project/simulator/src/avm/fixtures/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { isNoirCallStackUnresolved } from '@aztec/circuit-types';
import { GasFees, GlobalVariables, MAX_L2_GAS_PER_ENQUEUED_CALL } from '@aztec/circuits.js';
import { GasFees, GlobalVariables } from '@aztec/circuits.js';
import { FunctionSelector, getFunctionDebugMetadata } from '@aztec/foundation/abi';
import { AztecAddress } from '@aztec/foundation/aztec-address';
import { EthAddress } from '@aztec/foundation/eth-address';
Expand Down Expand Up @@ -91,7 +91,7 @@ export function initGlobalVariables(overrides?: Partial<GlobalVariables>): Globa
*/
export function initMachineState(overrides?: Partial<AvmMachineState>): AvmMachineState {
return AvmMachineState.fromState({
l2GasLeft: overrides?.l2GasLeft ?? MAX_L2_GAS_PER_ENQUEUED_CALL,
l2GasLeft: overrides?.l2GasLeft ?? 1e8,
daGasLeft: overrides?.daGasLeft ?? 1e8,
});
}
Expand Down
11 changes: 2 additions & 9 deletions yarn-project/simulator/src/public/enqueued_call_simulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
L2ToL1Message,
LogHash,
MAX_L1_TO_L2_MSG_READ_REQUESTS_PER_CALL,
MAX_L2_GAS_PER_ENQUEUED_CALL,
MAX_L2_TO_L1_MSGS_PER_CALL,
MAX_NOTE_HASHES_PER_CALL,
MAX_NOTE_HASH_READ_REQUESTS_PER_CALL,
Expand Down Expand Up @@ -121,12 +120,6 @@ export class EnqueuedCallSimulator {
transactionFee: Fr,
phase: PublicKernelPhase,
): Promise<EnqueuedCallResult> {
// Gas allocated to an enqueued call can be different from the available gas
// if there is more gas available than the max allocation per enqueued call.
const allocatedGas = new Gas(
/*daGas=*/ availableGas.daGas,
/*l2Gas=*/ Math.min(availableGas.l2Gas, MAX_L2_GAS_PER_ENQUEUED_CALL),
);
const pendingNullifiers = this.getSiloedPendingNullifiers(previousPublicKernelOutput);
const startSideEffectCounter = previousPublicKernelOutput.endSideEffectCounter + 1;

Expand All @@ -147,7 +140,7 @@ export class EnqueuedCallSimulator {
const result = await this.publicExecutor.simulate(
executionRequest,
constants,
allocatedGas,
availableGas,
tx.data.constants.txContext,
pendingNullifiers,
transactionFee,
Expand All @@ -174,7 +167,7 @@ export class EnqueuedCallSimulator {
accumulatedData,
startSideEffectCounter,
startSideEffectCounter,
allocatedGas,
availableGas,
result.transactionFee,
result.reverted,
);
Expand Down
6 changes: 4 additions & 2 deletions yarn-project/simulator/src/public/execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {
} from '@aztec/circuits.js';
import { computeVarArgsHash } from '@aztec/circuits.js/hash';

import { type Gas as AvmGas } from '../avm/avm_gas.js';

/**
* The public function execution result.
*/
Expand All @@ -34,9 +36,9 @@ export interface PublicExecutionResult {
/** The side effect counter after executing this function call */
endSideEffectCounter: Fr;
/** How much gas was available for this public execution. */
startGasLeft: Gas;
startGasLeft: AvmGas;
/** How much gas was left after this public execution. */
endGasLeft: Gas;
endGasLeft: AvmGas;
/** Transaction fee set for this tx. */
transactionFee: Fr;

Expand Down
10 changes: 5 additions & 5 deletions yarn-project/simulator/src/public/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class PublicExecutor {
* Executes a public execution request.
* @param executionRequest - The execution to run.
* @param constants - The constants (including global variables) to use.
* @param allocatedGas - The gas available at the start of this enqueued call.
* @param availableGas - The gas available at the start of this enqueued call.
* @param txContext - Transaction context.
* @param pendingSiloedNullifiers - The pending nullifier set from earlier parts of this TX.
* @param transactionFee - Fee offered for this TX.
Expand All @@ -55,7 +55,7 @@ export class PublicExecutor {
public async simulate(
executionRequest: PublicExecutionRequest,
constants: CombinedConstantData,
allocatedGas: Gas,
availableGas: Gas,
_txContext: TxContext,
pendingSiloedNullifiers: Nullifier[],
transactionFee: Fr = Fr.ZERO,
Expand Down Expand Up @@ -85,7 +85,7 @@ export class PublicExecutor {

const avmExecutionEnv = createAvmExecutionEnvironment(executionRequest, constants.globalVariables, transactionFee);

const avmMachineState = new AvmMachineState(allocatedGas);
const avmMachineState = new AvmMachineState(availableGas);
const avmContext = new AvmContext(avmPersistableState, avmExecutionEnv, avmMachineState);
const simulator = new AvmSimulator(avmContext);
const avmResult = await simulator.execute();
Expand All @@ -111,7 +111,7 @@ export class PublicExecutor {

const publicExecutionResult = trace.toPublicExecutionResult(
avmExecutionEnv,
/*startGasLeft=*/ allocatedGas,
/*startGasLeft=*/ availableGas,
/*endGasLeft=*/ Gas.from(avmContext.machineState.gasLeft),
bytecode,
avmResult,
Expand All @@ -127,7 +127,7 @@ export class PublicExecutor {
const _vmCircuitPublicInputs = enqueuedCallTrace.toVMCircuitPublicInputs(
constants,
avmExecutionEnv,
/*startGasLeft=*/ allocatedGas,
/*startGasLeft=*/ availableGas,
/*endGasLeft=*/ Gas.from(avmContext.machineState.gasLeft),
avmResult,
);
Expand Down
21 changes: 7 additions & 14 deletions yarn-project/simulator/src/public/public_processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -724,9 +724,8 @@ describe('public_processor', () => {
const revertibleRequests = tx.getRevertiblePublicExecutionRequests();
const teardownRequest = tx.getPublicTeardownExecutionRequest()!;

// Keep gas numbers MAX_L2_GAS_PER_ENQUEUED_CALL or the logic below has to get weird
const gasLimits = Gas.from({ l2Gas: 1e6, daGas: 1e6 });
const teardownGas = Gas.from({ l2Gas: 1e5, daGas: 1e5 });
const gasLimits = Gas.from({ l2Gas: 1e9, daGas: 1e9 });
const teardownGas = Gas.from({ l2Gas: 1e7, daGas: 1e7 });
tx.data.constants.txContext.gasSettings = GasSettings.from({
gasLimits: gasLimits,
teardownGasLimits: teardownGas,
Expand All @@ -751,26 +750,20 @@ describe('public_processor', () => {

let simulatorCallCount = 0;

// Keep gas numbers below MAX_L2_GAS_PER_ENQUEUED_CALL or we need
// to separately compute available start gas and "effective" start gas
// for each enqueued call after applying that max.
const initialGas = gasLimits.sub(teardownGas);
const setupGasUsed = Gas.from({ l2Gas: 1e4 });
const appGasUsed = Gas.from({ l2Gas: 2e4, daGas: 2e4 });
const teardownGasUsed = Gas.from({ l2Gas: 3e4, daGas: 3e4 });
const setupGasUsed = Gas.from({ l2Gas: 1e6 });
const appGasUsed = Gas.from({ l2Gas: 2e6, daGas: 2e6 });
const teardownGasUsed = Gas.from({ l2Gas: 3e6, daGas: 3e6 });
const afterSetupGas = initialGas.sub(setupGasUsed);
const afterAppGas = afterSetupGas.sub(appGasUsed);
const afterTeardownGas = teardownGas.sub(teardownGasUsed);

// Total gas used is the sum of teardown gas allocation plus all expenditures along the way,
// without including the gas used in the teardown phase (since that's consumed entirely up front).
const expectedTotalGasUsed = teardownGas.add(setupGasUsed).add(appGasUsed);
const expectedTotalGasUsed = { l2Gas: 1e7 + 1e6 + 2e6, daGas: 1e7 + 2e6 };

// Inclusion fee plus block gas fees times total gas used
const expectedTxFee =
tx.data.constants.txContext.gasSettings.inclusionFee.toNumber() +
expectedTotalGasUsed.l2Gas * 1 +
expectedTotalGasUsed.daGas * 1;
const expectedTxFee = 1e4 + (1e7 + 1e6 + 2e6) * 1 + (1e7 + 2e6) * 1;
const transactionFee = new Fr(expectedTxFee);

const simulatorResults: PublicExecutionResult[] = [
Expand Down

0 comments on commit c481a09

Please sign in to comment.