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

fix: Revert "feat: new per-enqueued-call gas limit" #9139

Merged
merged 1 commit into from
Oct 9, 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
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
Loading