From 7677ca5d9280ac9615a92be36d1958960dbd7353 Mon Sep 17 00:00:00 2001 From: ludamad Date: Wed, 9 Oct 2024 23:07:36 +0100 Subject: [PATCH] fix: Revert "feat: new per-enqueued-call gas limit" (#9139) Reverts AztecProtocol/aztec-packages#9033 broke uniswap tests --- .../vm/avm/tests/execution.test.cpp | 25 +++---------------- .../src/barretenberg/vm/avm/trace/helper.hpp | 15 ----------- .../src/barretenberg/vm/aztec_constants.hpp | 1 - .../src/core/libraries/ConstantsGen.sol | 1 - .../enqueued_call_data_validator.nr | 16 ++++-------- .../crates/types/src/constants.nr | 1 - yarn-project/circuits.js/src/constants.gen.ts | 1 - .../circuits.js/src/scripts/constants.in.ts | 1 - yarn-project/circuits.js/src/structs/gas.ts | 3 +-- .../simulator/src/avm/avm_simulator.ts | 5 ---- .../simulator/src/avm/fixtures/index.ts | 4 +-- .../src/public/enqueued_call_simulator.ts | 11 ++------ .../simulator/src/public/execution.ts | 6 +++-- yarn-project/simulator/src/public/executor.ts | 10 ++++---- .../src/public/public_processor.test.ts | 21 ++++++---------- 15 files changed, 29 insertions(+), 92 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp index d78d5efa1f0..9e43b804768 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp @@ -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(EnvironmentVariable::SENDER)) + "0007"; // addr 7 - - std::vector calldata = {}; - std::vector returndata = {}; - std::vector 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(EnvironmentVariable::SENDER)) + "0007"; // addr 7 std::vector calldata = {}; @@ -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"); } diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp index a3051d91235..946feea07c1 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp @@ -37,21 +37,6 @@ template VmPublicInputs convert_public_inputs(std::vector) { - 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& kernel_inputs = std::get(public_inputs); // Copy items from PublicCircuitPublicInputs vector to public input columns diff --git a/barretenberg/cpp/src/barretenberg/vm/aztec_constants.hpp b/barretenberg/cpp/src/barretenberg/vm/aztec_constants.hpp index 2b923e6a713..3f6c828a6e6 100644 --- a/barretenberg/cpp/src/barretenberg/vm/aztec_constants.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/aztec_constants.hpp @@ -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 diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol index bbea41984b2..1a3d9eb9c1f 100644 --- a/l1-contracts/src/core/libraries/ConstantsGen.sol +++ b/l1-contracts/src/core/libraries/ConstantsGen.sol @@ -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; diff --git a/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/components/enqueued_call_data_validator.nr b/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/components/enqueued_call_data_validator.nr index a66ea0e5169..7688f780885 100644 --- a/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/components/enqueued_call_data_validator.nr +++ b/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/components/enqueued_call_data_validator.nr @@ -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 { @@ -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" ); } } diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr index aaa531ae2b3..58f914aabfd 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -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; diff --git a/yarn-project/circuits.js/src/constants.gen.ts b/yarn-project/circuits.js/src/constants.gen.ts index f379c07941c..dbc2e1fb465 100644 --- a/yarn-project/circuits.js/src/constants.gen.ts +++ b/yarn-project/circuits.js/src/constants.gen.ts @@ -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; diff --git a/yarn-project/circuits.js/src/scripts/constants.in.ts b/yarn-project/circuits.js/src/scripts/constants.in.ts index a4783b95183..ce909deba97 100644 --- a/yarn-project/circuits.js/src/scripts/constants.in.ts +++ b/yarn-project/circuits.js/src/scripts/constants.in.ts @@ -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[] = []; diff --git a/yarn-project/circuits.js/src/structs/gas.ts b/yarn-project/circuits.js/src/structs/gas.ts index 7e1f7469492..e913190032b 100644 --- a/yarn-project/circuits.js/src/structs/gas.ts +++ b/yarn-project/circuits.js/src/structs/gas.ts @@ -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'; @@ -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() { diff --git a/yarn-project/simulator/src/avm/avm_simulator.ts b/yarn-project/simulator/src/avm/avm_simulator.ts index 96c905cc58b..6c036ecface 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.ts @@ -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'; @@ -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()})`); } diff --git a/yarn-project/simulator/src/avm/fixtures/index.ts b/yarn-project/simulator/src/avm/fixtures/index.ts index fb401245b66..084767f0317 100644 --- a/yarn-project/simulator/src/avm/fixtures/index.ts +++ b/yarn-project/simulator/src/avm/fixtures/index.ts @@ -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'; @@ -91,7 +91,7 @@ export function initGlobalVariables(overrides?: Partial): Globa */ export function initMachineState(overrides?: Partial): AvmMachineState { return AvmMachineState.fromState({ - l2GasLeft: overrides?.l2GasLeft ?? MAX_L2_GAS_PER_ENQUEUED_CALL, + l2GasLeft: overrides?.l2GasLeft ?? 1e8, daGasLeft: overrides?.daGasLeft ?? 1e8, }); } diff --git a/yarn-project/simulator/src/public/enqueued_call_simulator.ts b/yarn-project/simulator/src/public/enqueued_call_simulator.ts index a37c1b2066f..cd588fbe644 100644 --- a/yarn-project/simulator/src/public/enqueued_call_simulator.ts +++ b/yarn-project/simulator/src/public/enqueued_call_simulator.ts @@ -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, @@ -121,12 +120,6 @@ export class EnqueuedCallSimulator { transactionFee: Fr, phase: PublicKernelPhase, ): Promise { - // 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; @@ -147,7 +140,7 @@ export class EnqueuedCallSimulator { const result = await this.publicExecutor.simulate( executionRequest, constants, - allocatedGas, + availableGas, tx.data.constants.txContext, pendingNullifiers, transactionFee, @@ -174,7 +167,7 @@ export class EnqueuedCallSimulator { accumulatedData, startSideEffectCounter, startSideEffectCounter, - allocatedGas, + availableGas, result.transactionFee, result.reverted, ); diff --git a/yarn-project/simulator/src/public/execution.ts b/yarn-project/simulator/src/public/execution.ts index 7dc41776095..6c688c28df9 100644 --- a/yarn-project/simulator/src/public/execution.ts +++ b/yarn-project/simulator/src/public/execution.ts @@ -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. */ @@ -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; diff --git a/yarn-project/simulator/src/public/executor.ts b/yarn-project/simulator/src/public/executor.ts index d9f381f5018..cb4c3ec7692 100644 --- a/yarn-project/simulator/src/public/executor.ts +++ b/yarn-project/simulator/src/public/executor.ts @@ -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. @@ -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, @@ -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(); @@ -111,7 +111,7 @@ export class PublicExecutor { const publicExecutionResult = trace.toPublicExecutionResult( avmExecutionEnv, - /*startGasLeft=*/ allocatedGas, + /*startGasLeft=*/ availableGas, /*endGasLeft=*/ Gas.from(avmContext.machineState.gasLeft), bytecode, avmResult, @@ -127,7 +127,7 @@ export class PublicExecutor { const _vmCircuitPublicInputs = enqueuedCallTrace.toVMCircuitPublicInputs( constants, avmExecutionEnv, - /*startGasLeft=*/ allocatedGas, + /*startGasLeft=*/ availableGas, /*endGasLeft=*/ Gas.from(avmContext.machineState.gasLeft), avmResult, ); diff --git a/yarn-project/simulator/src/public/public_processor.test.ts b/yarn-project/simulator/src/public/public_processor.test.ts index 60d24624aaa..4e48c4166e5 100644 --- a/yarn-project/simulator/src/public/public_processor.test.ts +++ b/yarn-project/simulator/src/public/public_processor.test.ts @@ -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, @@ -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[] = [