From 2846d956dba88834ba814e933b3de74cc3162129 Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Wed, 2 Oct 2024 18:23:29 +0000 Subject: [PATCH 1/9] wip --- .../src/core/libraries/ConstantsGen.sol | 1 + .../aztec/src/context/private_context.nr | 13 +++++-- .../aztec/src/context/public_context.nr | 24 ++++++++----- .../aztec/src/macros/functions/mod.nr | 3 +- .../oracle/enqueue_public_function_call.nr | 12 +++---- .../crates/types/src/constants.nr | 2 ++ yarn-project/circuits.js/src/constants.gen.ts | 1 + .../simulator/src/acvm/oracle/oracle.ts | 10 +++--- .../simulator/src/acvm/oracle/typed_oracle.ts | 4 +-- .../src/client/client_execution_context.ts | 35 +++++++++++++++---- yarn-project/txe/src/oracle/txe_oracle.ts | 14 +++++--- .../txe/src/txe_service/txe_service.ts | 8 ++--- 12 files changed, 86 insertions(+), 41 deletions(-) diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol index d6a1f7c4778f..91a9ca5a9584 100644 --- a/l1-contracts/src/core/libraries/ConstantsGen.sol +++ b/l1-contracts/src/core/libraries/ConstantsGen.sol @@ -100,6 +100,7 @@ library Constants { uint256 internal constant GENESIS_ARCHIVE_ROOT = 8142738430000951296386584486068033372964809139261822027365426310856631083550; uint256 internal constant FEE_JUICE_INITIAL_MINT = 20000000000; + uint256 internal constant PUBLIC_DISPATCH_SELECTOR = 3578010381; uint256 internal constant MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS = 20000; uint256 internal constant MAX_PACKED_BYTECODE_SIZE_PER_PRIVATE_FUNCTION_IN_FIELDS = 3000; uint256 internal constant MAX_PACKED_BYTECODE_SIZE_PER_UNCONSTRAINED_FUNCTION_IN_FIELDS = 3000; diff --git a/noir-projects/aztec-nr/aztec/src/context/private_context.nr b/noir-projects/aztec-nr/aztec/src/context/private_context.nr index 0b002a3cc67d..557d70f3f4db 100644 --- a/noir-projects/aztec-nr/aztec/src/context/private_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/private_context.nr @@ -26,7 +26,7 @@ use dep::protocol_types::{ MAX_PRIVATE_CALL_STACK_LENGTH_PER_CALL, MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_READ_REQUESTS_PER_CALL, MAX_KEY_VALIDATION_REQUESTS_PER_CALL, MAX_ENCRYPTED_LOGS_PER_CALL, MAX_UNENCRYPTED_LOGS_PER_CALL, - MAX_NOTE_ENCRYPTED_LOGS_PER_CALL + MAX_NOTE_ENCRYPTED_LOGS_PER_CALL, PUBLIC_DISPATCH_SELECTOR }, header::Header, messaging::l2_to_l1_message::L2ToL1Message, traits::Empty }; @@ -468,7 +468,9 @@ impl PrivateContext { let counter = self.next_counter(); let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call; - enqueue_public_function_call_internal( + // TODO(): + // WARNING: + let args_hash = enqueue_public_function_call_internal( contract_address, function_selector, args_hash, @@ -477,6 +479,8 @@ impl PrivateContext { is_delegate_call ); + // Public calls are rerouted through the dispatch function. + let function_selector = FunctionSelector::from_field(PUBLIC_DISPATCH_SELECTOR); let call_context = self.generate_call_context( contract_address, function_selector, @@ -510,7 +514,9 @@ impl PrivateContext { let counter = self.next_counter(); let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call; - set_public_teardown_function_call_internal( + // TODO(): + // WARNING: + let args_hash = set_public_teardown_function_call_internal( contract_address, function_selector, args_hash, @@ -519,6 +525,7 @@ impl PrivateContext { is_delegate_call ); + let function_selector = FunctionSelector::from_field(PUBLIC_DISPATCH_SELECTOR); let call_context = self.generate_call_context( contract_address, function_selector, diff --git a/noir-projects/aztec-nr/aztec/src/context/public_context.nr b/noir-projects/aztec-nr/aztec/src/context/public_context.nr index be95853c7a8f..98188c3bf966 100644 --- a/noir-projects/aztec-nr/aztec/src/context/public_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/public_context.nr @@ -1,6 +1,6 @@ use crate::hash::{compute_secret_hash, compute_message_hash, compute_message_nullifier}; use dep::protocol_types::address::{AztecAddress, EthAddress}; -use dep::protocol_types::constants::MAX_FIELD_VALUE; +use dep::protocol_types::constants::{MAX_FIELD_VALUE, PUBLIC_DISPATCH_SELECTOR}; use dep::protocol_types::traits::{Serialize, Deserialize, Empty}; use dep::protocol_types::abis::function_selector::FunctionSelector; use crate::context::gas::GasOpts; @@ -70,11 +70,12 @@ impl PublicContext { args: [Field], gas_opts: GasOpts ) -> FunctionReturns { + let args = &[function_selector.to_field()].append(args); let results = call( gas_for_call(gas_opts), contract_address, args, - function_selector.to_field() + PUBLIC_DISPATCH_SELECTOR ); let data_to_return: [Field; RETURNS_COUNT] = results.0; let success: u8 = results.1; @@ -90,11 +91,12 @@ impl PublicContext { args: [Field], gas_opts: GasOpts ) -> FunctionReturns { + let args = &[function_selector.to_field()].append(args); let (data_to_return, success): ([Field; RETURNS_COUNT], u8) = call_static( gas_for_call(gas_opts), contract_address, args, - function_selector.to_field() + PUBLIC_DISPATCH_SELECTOR ); assert(success == 1, "Nested static call failed!"); @@ -127,7 +129,9 @@ impl PublicContext { sender() } fn selector(_self: Self) -> FunctionSelector { - FunctionSelector::from_u32(function_selector()) + // The selector is the first element of the calldata when calling a public function through dispatch. + let raw_selector: [Field; 1] = calldata_copy(0, 1); + FunctionSelector::from_field(raw_selector[0]) } fn get_args_hash(mut self) -> Field { if !self.args_hash.is_some() { @@ -215,9 +219,10 @@ unconstrained fn sender() -> AztecAddress { unconstrained fn portal() -> EthAddress { portal_opcode() } -unconstrained fn function_selector() -> u32 { - function_selector_opcode() -} +// UNUSED: Remove. +// unconstrained fn function_selector() -> u32 { +// function_selector_opcode() +// } unconstrained fn transaction_fee() -> Field { transaction_fee_opcode() } @@ -321,8 +326,9 @@ unconstrained fn sender_opcode() -> AztecAddress {} #[oracle(avmOpcodePortal)] unconstrained fn portal_opcode() -> EthAddress {} -#[oracle(avmOpcodeFunctionSelector)] -unconstrained fn function_selector_opcode() -> u32 {} +// UNUSED: Remove. +// #[oracle(avmOpcodeFunctionSelector)] +// unconstrained fn function_selector_opcode() -> u32 {} #[oracle(avmOpcodeTransactionFee)] unconstrained fn transaction_fee_opcode() -> Field {} diff --git a/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr b/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr index 1fc42fbab76b..ceee8c5178cf 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr @@ -247,7 +247,8 @@ comptime fn transform_public(f: FunctionDefinition) -> Quoted { // Unlike in the private case, in public the `context` does not need to receive the hash of the original params. let context_creation = quote { let mut context = dep::aztec::context::public_context::PublicContext::new(|| { - let serialized_args : [Field; $args_len] = dep::aztec::context::public_context::calldata_copy(0, $args_len); + // We start from 1 because we skip the selector for the dispatch function. + let serialized_args : [Field; $args_len] = dep::aztec::context::public_context::calldata_copy(1, $args_len); dep::aztec::hash::hash_args_array(serialized_args) }); }; diff --git a/noir-projects/aztec-nr/aztec/src/oracle/enqueue_public_function_call.nr b/noir-projects/aztec-nr/aztec/src/oracle/enqueue_public_function_call.nr index fabfb4d8e2b1..623b14be2324 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/enqueue_public_function_call.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/enqueue_public_function_call.nr @@ -8,7 +8,7 @@ unconstrained fn enqueue_public_function_call_oracle( _side_effect_counter: u32, _is_static_call: bool, _is_delegate_call: bool -) {} +) -> Field {} unconstrained pub fn enqueue_public_function_call_internal( contract_address: AztecAddress, @@ -17,7 +17,7 @@ unconstrained pub fn enqueue_public_function_call_internal( side_effect_counter: u32, is_static_call: bool, is_delegate_call: bool -) { +) -> Field { enqueue_public_function_call_oracle( contract_address, function_selector, @@ -25,7 +25,7 @@ unconstrained pub fn enqueue_public_function_call_internal( side_effect_counter, is_static_call, is_delegate_call - ); + ) } #[oracle(setPublicTeardownFunctionCall)] @@ -36,7 +36,7 @@ unconstrained fn set_public_teardown_function_call_oracle( _side_effect_counter: u32, _is_static_call: bool, _is_delegate_call: bool -) {} +) -> Field {} unconstrained pub fn set_public_teardown_function_call_internal( contract_address: AztecAddress, @@ -45,7 +45,7 @@ unconstrained pub fn set_public_teardown_function_call_internal( side_effect_counter: u32, is_static_call: bool, is_delegate_call: bool -) { +) -> Field { set_public_teardown_function_call_oracle( contract_address, function_selector, @@ -53,7 +53,7 @@ unconstrained pub fn set_public_teardown_function_call_internal( side_effect_counter, is_static_call, is_delegate_call - ); + ) } pub fn notify_set_min_revertible_side_effect_counter(counter: u32) { 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 cffcda33cd5f..4846842c401e 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -137,6 +137,8 @@ global GENESIS_ARCHIVE_ROOT: Field = 0x1200a06aae1368abe36530b585bd7a4d2ba4de503 // The following and the value in `deploy_l1_contracts` must match. We should not have the code both places, but // we are running into circular dependency issues. #3342 global FEE_JUICE_INITIAL_MINT: Field = 20000000000; +// Last 4 bytes of the Poseidon2 hash of 'public_dispatch(Field)'. +global PUBLIC_DISPATCH_SELECTOR: Field = 0xd5441b0d; // CONTRACT CLASS CONSTANTS global MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS: u32 = 20000; diff --git a/yarn-project/circuits.js/src/constants.gen.ts b/yarn-project/circuits.js/src/constants.gen.ts index 16281453188d..14ff856712f3 100644 --- a/yarn-project/circuits.js/src/constants.gen.ts +++ b/yarn-project/circuits.js/src/constants.gen.ts @@ -85,6 +85,7 @@ export const AZTEC_EPOCH_DURATION = 48; export const AZTEC_TARGET_COMMITTEE_SIZE = 48; export const GENESIS_ARCHIVE_ROOT = 8142738430000951296386584486068033372964809139261822027365426310856631083550n; export const FEE_JUICE_INITIAL_MINT = 20000000000; +export const PUBLIC_DISPATCH_SELECTOR = 3578010381; export const MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS = 20000; export const MAX_PACKED_BYTECODE_SIZE_PER_PRIVATE_FUNCTION_IN_FIELDS = 3000; export const MAX_PACKED_BYTECODE_SIZE_PER_UNCONSTRAINED_FUNCTION_IN_FIELDS = 3000; diff --git a/yarn-project/simulator/src/acvm/oracle/oracle.ts b/yarn-project/simulator/src/acvm/oracle/oracle.ts index cb3bf8041b24..9a93f41e2a41 100644 --- a/yarn-project/simulator/src/acvm/oracle/oracle.ts +++ b/yarn-project/simulator/src/acvm/oracle/oracle.ts @@ -380,8 +380,8 @@ export class Oracle { [sideEffectCounter]: ACVMField[], [isStaticCall]: ACVMField[], [isDelegateCall]: ACVMField[], - ) { - await this.typedOracle.enqueuePublicFunctionCall( + ): Promise { + const newArgsHash = await this.typedOracle.enqueuePublicFunctionCall( AztecAddress.fromString(contractAddress), FunctionSelector.fromField(fromACVMField(functionSelector)), fromACVMField(argsHash), @@ -389,6 +389,7 @@ export class Oracle { frToBoolean(fromACVMField(isStaticCall)), frToBoolean(fromACVMField(isDelegateCall)), ); + return toACVMField(newArgsHash); } async setPublicTeardownFunctionCall( @@ -398,8 +399,8 @@ export class Oracle { [sideEffectCounter]: ACVMField[], [isStaticCall]: ACVMField[], [isDelegateCall]: ACVMField[], - ) { - await this.typedOracle.setPublicTeardownFunctionCall( + ): Promise { + const newArgsHash = await this.typedOracle.setPublicTeardownFunctionCall( AztecAddress.fromString(contractAddress), FunctionSelector.fromField(fromACVMField(functionSelector)), fromACVMField(argsHash), @@ -407,6 +408,7 @@ export class Oracle { frToBoolean(fromACVMField(isStaticCall)), frToBoolean(fromACVMField(isDelegateCall)), ); + return toACVMField(newArgsHash); } notifySetMinRevertibleSideEffectCounter([minRevertibleSideEffectCounter]: ACVMField[]) { diff --git a/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts b/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts index 63dcb34a7354..a4531373cc68 100644 --- a/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts +++ b/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts @@ -229,7 +229,7 @@ export abstract class TypedOracle { _sideEffectCounter: number, _isStaticCall: boolean, _isDelegateCall: boolean, - ): Promise { + ): Promise { throw new OracleMethodNotAvailableError('enqueuePublicFunctionCall'); } @@ -240,7 +240,7 @@ export abstract class TypedOracle { _sideEffectCounter: number, _isStaticCall: boolean, _isDelegateCall: boolean, - ): Promise { + ): Promise { throw new OracleMethodNotAvailableError('setPublicTeardownFunctionCall'); } diff --git a/yarn-project/simulator/src/client/client_execution_context.ts b/yarn-project/simulator/src/client/client_execution_context.ts index 3fc8a5379164..34dfb2aeec1d 100644 --- a/yarn-project/simulator/src/client/client_execution_context.ts +++ b/yarn-project/simulator/src/client/client_execution_context.ts @@ -8,7 +8,14 @@ import { PublicExecutionRequest, type UnencryptedL2Log, } from '@aztec/circuit-types'; -import { CallContext, FunctionSelector, type Header, PrivateContextInputs, type TxContext } from '@aztec/circuits.js'; +import { + CallContext, + FunctionSelector, + type Header, + PUBLIC_DISPATCH_SELECTOR, + PrivateContextInputs, + type TxContext, +} from '@aztec/circuits.js'; import { computeUniqueNoteHash, siloNoteHash } from '@aztec/circuits.js/hash'; import { type FunctionAbi, type FunctionArtifact, type NoteSelector, countArgumentsSize } from '@aztec/foundation/abi'; import { type AztecAddress } from '@aztec/foundation/aztec-address'; @@ -528,16 +535,23 @@ export class ClientExecutionContext extends ViewDataOracle { sideEffectCounter: number, isStaticCall: boolean, isDelegateCall: boolean, - ) { + ): Promise { + // TODO(): + // WARNING: + const newArgsHash = this.packedValuesCache.pack([ + functionSelector.toField(), + ...this.packedValuesCache.unpack(argsHash), + ]); await this.createPublicExecutionRequest( 'enqueued', targetContractAddress, - functionSelector, - argsHash, + FunctionSelector.fromField(new Fr(PUBLIC_DISPATCH_SELECTOR)), + newArgsHash, sideEffectCounter, isStaticCall, isDelegateCall, ); + return newArgsHash; } /** @@ -558,16 +572,23 @@ export class ClientExecutionContext extends ViewDataOracle { sideEffectCounter: number, isStaticCall: boolean, isDelegateCall: boolean, - ) { + ): Promise { + // TODO(): + // WARNING: + const newArgsHash = this.packedValuesCache.pack([ + functionSelector.toField(), + ...this.packedValuesCache.unpack(argsHash), + ]); await this.createPublicExecutionRequest( 'teardown', targetContractAddress, - functionSelector, - argsHash, + FunctionSelector.fromField(new Fr(PUBLIC_DISPATCH_SELECTOR)), + newArgsHash, sideEffectCounter, isStaticCall, isDelegateCall, ); + return newArgsHash; } public override notifySetMinRevertibleSideEffectCounter(minRevertibleSideEffectCounter: number): void { diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index 32791a806b88..8765294c788e 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -21,6 +21,7 @@ import { type NullifierLeafPreimage, PUBLIC_DATA_SUBTREE_HEIGHT, type PUBLIC_DATA_TREE_HEIGHT, + PUBLIC_DISPATCH_SELECTOR, PrivateCircuitPublicInputs, PrivateContextInputs, PublicDataTreeLeaf, @@ -746,7 +747,7 @@ export class TXE implements TypedOracle { sideEffectCounter: number, isStaticCall: boolean, isDelegateCall: boolean, - ) { + ): Promise { // Store and modify env const currentContractAddress = AztecAddress.fromField(this.contractAddress); const currentMessageSender = AztecAddress.fromField(this.msgSender); @@ -757,12 +758,13 @@ export class TXE implements TypedOracle { const callContext = CallContext.empty(); callContext.msgSender = this.msgSender; - callContext.functionSelector = this.functionSelector; + callContext.functionSelector = FunctionSelector.fromField(new Fr(PUBLIC_DISPATCH_SELECTOR)); callContext.storageContractAddress = targetContractAddress; callContext.isStaticCall = isStaticCall; callContext.isDelegateCall = isDelegateCall; - const args = this.packedValuesCache.unpack(argsHash); + const args = [this.functionSelector.toField(), ...this.packedValuesCache.unpack(argsHash)]; + const newArgsHash = this.packedValuesCache.pack(args); const executionResult = await this.executePublicFunction( targetContractAddress, @@ -780,6 +782,8 @@ export class TXE implements TypedOracle { this.setContractAddress(currentContractAddress); this.setMsgSender(currentMessageSender); this.setFunctionSelector(currentFunctionSelector); + + return newArgsHash; } async setPublicTeardownFunctionCall( @@ -789,10 +793,10 @@ export class TXE implements TypedOracle { sideEffectCounter: number, isStaticCall: boolean, isDelegateCall: boolean, - ) { + ): Promise { // Definitely not right, in that the teardown should always be last. // But useful for executing flows. - await this.enqueuePublicFunctionCall( + return await this.enqueuePublicFunctionCall( targetContractAddress, functionSelector, argsHash, diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index f9c0db63d403..425b56c423c3 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -678,7 +678,7 @@ export class TXEService { isStaticCall: ForeignCallSingle, isDelegateCall: ForeignCallSingle, ) { - await this.typedOracle.enqueuePublicFunctionCall( + const newArgsHash = await this.typedOracle.enqueuePublicFunctionCall( fromSingle(targetContractAddress), FunctionSelector.fromField(fromSingle(functionSelector)), fromSingle(argsHash), @@ -686,7 +686,7 @@ export class TXEService { fromSingle(isStaticCall).toBool(), fromSingle(isDelegateCall).toBool(), ); - return toForeignCallResult([]); + return toForeignCallResult([toSingle(newArgsHash)]); } public async setPublicTeardownFunctionCall( @@ -697,7 +697,7 @@ export class TXEService { isStaticCall: ForeignCallSingle, isDelegateCall: ForeignCallSingle, ) { - await this.typedOracle.setPublicTeardownFunctionCall( + const newArgsHash = await this.typedOracle.setPublicTeardownFunctionCall( fromSingle(targetContractAddress), FunctionSelector.fromField(fromSingle(functionSelector)), fromSingle(argsHash), @@ -705,7 +705,7 @@ export class TXEService { fromSingle(isStaticCall).toBool(), fromSingle(isDelegateCall).toBool(), ); - return toForeignCallResult([]); + return toForeignCallResult([toSingle(newArgsHash)]); } public notifySetMinRevertibleSideEffectCounter(minRevertibleSideEffectCounter: ForeignCallSingle) { From fb59a74e1af3fb0267906104c896c0f16754800f Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Wed, 2 Oct 2024 20:33:53 +0000 Subject: [PATCH 2/9] txe changes --- .../aztec-nr/aztec/src/test/helpers/test_environment.nr | 5 ++++- noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr | 7 +++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr index 72533505bf05..a12cf55326ab 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr @@ -12,6 +12,7 @@ use crate::hash::hash_args; use crate::note::{note_header::NoteHeader, note_interface::{NoteInterface, NullifiableNote}}; use crate::oracle::{execution::{get_block_number, get_contract_address}, notes::notify_created_note}; +use protocol_types::constants::PUBLIC_DISPATCH_SELECTOR; pub struct TestEnvironment {} @@ -142,9 +143,11 @@ impl TestEnvironment { let original_contract_address = get_contract_address(); let original_fn_selector = cheatcodes::get_function_selector(); let target_address = call_interface.get_contract_address(); - let fn_selector = call_interface.get_selector(); let calldata = call_interface.get_args(); + // Public functions are routed through the dispatch function. + let fn_selector = FunctionSelector::from_field(PUBLIC_DISPATCH_SELECTOR); + let calldata = &[call_interface.get_selector().to_field()].append(calldata); cheatcodes::set_fn_selector(fn_selector); cheatcodes::set_contract_address(target_address); cheatcodes::set_msg_sender(original_contract_address); diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr index e0bd79bc1d37..fbee5a98a69f 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr @@ -1,6 +1,6 @@ use dep::protocol_types::{ traits::{Deserialize, Serialize}, address::AztecAddress, - abis::{private_circuit_public_inputs::PrivateCircuitPublicInputs}, + abis::{function_selector::FunctionSelector, private_circuit_public_inputs::PrivateCircuitPublicInputs}, contract_instance::ContractInstance }; @@ -10,6 +10,7 @@ use crate::test::helpers::cheatcodes; use crate::keys::public_keys::{PUBLIC_KEYS_LENGTH, PublicKeys}; use crate::oracle::{execution::{get_block_number, get_contract_address}}; +use protocol_types::constants::PUBLIC_DISPATCH_SELECTOR; unconstrained pub fn apply_side_effects_private(contract_address: AztecAddress, public_inputs: PrivateCircuitPublicInputs) { let mut nullifiers = &[]; @@ -83,7 +84,9 @@ impl Deployer { let original_fn_selector = cheatcodes::get_function_selector(); let calldata = call_interface.get_args(); - cheatcodes::set_fn_selector(call_interface.get_selector()); + let fn_selector = FunctionSelector::from_field(PUBLIC_DISPATCH_SELECTOR); + let calldata = &[call_interface.get_selector().to_field()].append(calldata); + cheatcodes::set_fn_selector(fn_selector); cheatcodes::set_contract_address(instance.to_address()); cheatcodes::set_msg_sender(original_contract_address); cheatcodes::set_is_static_call(call_interface.get_is_static()); From bca3ef18751c51f61d93f84eebb52c645aa6974b Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Wed, 2 Oct 2024 20:34:00 +0000 Subject: [PATCH 3/9] fix e2e --- yarn-project/end-to-end/src/e2e_ordering.test.ts | 8 +++++--- yarn-project/sequencer-client/src/config.ts | 9 ++++++--- .../src/tx_validator/gas_validator.ts | 13 ++++++------- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/yarn-project/end-to-end/src/e2e_ordering.test.ts b/yarn-project/end-to-end/src/e2e_ordering.test.ts index 22b256ec4a20..839906f034b8 100644 --- a/yarn-project/end-to-end/src/e2e_ordering.test.ts +++ b/yarn-project/end-to-end/src/e2e_ordering.test.ts @@ -74,14 +74,16 @@ describe('e2e_ordering', () => { }); // The enqueued public calls are in the expected order based on the argument they set (stack is reversed!) - expect(enqueuedPublicCalls.map(c => c.args[0].toBigInt())).toEqual([...expectedOrder].reverse()); + // args[1] is used instead of args[0] because public functions are routed through the public dispatch + // function and args[0] is the target function selector. + expect(enqueuedPublicCalls.map(c => c.args[1].toBigInt())).toEqual([...expectedOrder].reverse()); // Logs are emitted in the expected order await expectLogsFromLastBlockToBe(expectedOrder); // The final value of the child is the last one set const value = await pxe.getPublicStorageAt(child.address, new Fr(1)); - expect(value.value).toBe(expectedOrder[1]); // final state should match last value set + expect(value.toBigInt()).toBe(expectedOrder[1]); // final state should match last value set }, ); }); @@ -106,7 +108,7 @@ describe('e2e_ordering', () => { await child.methods[method]().send().wait(); const value = await pxe.getPublicStorageAt(child.address, new Fr(1)); - expect(value.value).toBe(expectedOrder[expectedOrder.length - 1]); // final state should match last value set + expect(value.toBigInt()).toBe(expectedOrder[expectedOrder.length - 1]); // final state should match last value set }); it.each([ diff --git a/yarn-project/sequencer-client/src/config.ts b/yarn-project/sequencer-client/src/config.ts index 3451fe099ed1..8b89c7f2d0dd 100644 --- a/yarn-project/sequencer-client/src/config.ts +++ b/yarn-project/sequencer-client/src/config.ts @@ -189,16 +189,19 @@ function getDefaultAllowedSetupFunctions(): AllowedElement[] { // needed for claiming on the same tx as a spend { address: FeeJuiceAddress, - selector: FunctionSelector.fromSignature('_increase_public_balance((Field),Field)'), + // We can't restrict the selector because public functions get routed via dispatch. + // selector: FunctionSelector.fromSignature('_increase_public_balance((Field),Field)'), }, // needed for private transfers via FPC { classId: getContractClassFromArtifact(TokenContractArtifact).id, - selector: FunctionSelector.fromSignature('_increase_public_balance((Field),Field)'), + // We can't restrict the selector because public functions get routed via dispatch. + // selector: FunctionSelector.fromSignature('_increase_public_balance((Field),Field)'), }, { classId: getContractClassFromArtifact(FPCContract.artifact).id, - selector: FunctionSelector.fromSignature('prepare_fee((Field),Field,(Field),Field)'), + // We can't restrict the selector because public functions get routed via dispatch. + // selector: FunctionSelector.fromSignature('prepare_fee((Field),Field,(Field),Field)'), }, ]; } diff --git a/yarn-project/sequencer-client/src/tx_validator/gas_validator.ts b/yarn-project/sequencer-client/src/tx_validator/gas_validator.ts index 4173eb3691df..705a9fce7f32 100644 --- a/yarn-project/sequencer-client/src/tx_validator/gas_validator.ts +++ b/yarn-project/sequencer-client/src/tx_validator/gas_validator.ts @@ -1,7 +1,6 @@ import { PublicKernelPhase, type Tx, type TxValidator } from '@aztec/circuit-types'; -import { type AztecAddress, type Fr } from '@aztec/circuits.js'; +import { type AztecAddress, type Fr, FunctionSelector } from '@aztec/circuits.js'; import { createDebugLogger } from '@aztec/foundation/log'; -import { FeeJuiceArtifact } from '@aztec/protocol-contracts/fee-juice'; import { EnqueuedCallsProcessor, computeFeePayerBalanceStorageSlot } from '@aztec/simulator'; /** Provides a view into public contract state */ @@ -64,15 +63,15 @@ export class GasTxValidator implements TxValidator { fn => fn.contractAddress.equals(this.#feeJuiceAddress) && fn.callContext.msgSender.equals(this.#feeJuiceAddress) && - fn.callContext.functionSelector.equals( - FeeJuiceArtifact.functions.find(f => f.name === '_increase_public_balance')!, - ) && - fn.args[0].equals(feePayer) && + fn.args.length > 2 && + // Public functions get routed through the dispatch function, whose first argument is the target function selector. + fn.args[0].equals(FunctionSelector.fromSignature('_increase_public_balance((Field),Field)').toField()) && + fn.args[1].equals(feePayer) && !fn.callContext.isStaticCall && !fn.callContext.isDelegateCall, ); - const balance = claimFunctionCall ? initialBalance.add(claimFunctionCall.args[1]) : initialBalance; + const balance = claimFunctionCall ? initialBalance.add(claimFunctionCall.args[2]) : initialBalance; if (balance.lt(feeLimit)) { this.#log.info(`Rejecting transaction due to not enough fee payer balance`, { feePayer, balance, feeLimit }); return false; From 6245c2800131a68ec7698edf5b54c4c92ecf47bd Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Thu, 3 Oct 2024 08:42:51 +0000 Subject: [PATCH 4/9] minor tweaks --- .../aztec/src/context/private_context.nr | 8 +++++-- .../simulator/src/avm/journal/journal.ts | 10 ++++++--- .../src/client/client_execution_context.ts | 2 +- .../simulator/src/client/private_execution.ts | 4 ++-- .../simulator/src/common/debug_fn_name.ts | 22 +++++++++++++++++++ yarn-project/simulator/src/public/executor.ts | 5 +++-- yarn-project/txe/src/oracle/txe_oracle.ts | 4 ++-- 7 files changed, 43 insertions(+), 12 deletions(-) create mode 100644 yarn-project/simulator/src/common/debug_fn_name.ts diff --git a/noir-projects/aztec-nr/aztec/src/context/private_context.nr b/noir-projects/aztec-nr/aztec/src/context/private_context.nr index 557d70f3f4db..1be1e3da4429 100644 --- a/noir-projects/aztec-nr/aztec/src/context/private_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/private_context.nr @@ -480,7 +480,9 @@ impl PrivateContext { ); // Public calls are rerouted through the dispatch function. - let function_selector = FunctionSelector::from_field(PUBLIC_DISPATCH_SELECTOR); + let function_selector = comptime { + FunctionSelector::from_field(PUBLIC_DISPATCH_SELECTOR) + }; let call_context = self.generate_call_context( contract_address, function_selector, @@ -525,7 +527,9 @@ impl PrivateContext { is_delegate_call ); - let function_selector = FunctionSelector::from_field(PUBLIC_DISPATCH_SELECTOR); + let function_selector = comptime { + FunctionSelector::from_field(PUBLIC_DISPATCH_SELECTOR) + }; let call_context = self.generate_call_context( contract_address, function_selector, diff --git a/yarn-project/simulator/src/avm/journal/journal.ts b/yarn-project/simulator/src/avm/journal/journal.ts index b26c40418381..7594254e099e 100644 --- a/yarn-project/simulator/src/avm/journal/journal.ts +++ b/yarn-project/simulator/src/avm/journal/journal.ts @@ -3,6 +3,7 @@ import { Fr } from '@aztec/foundation/fields'; import { type DebugLogger, createDebugLogger } from '@aztec/foundation/log'; import { SerializableContractInstance } from '@aztec/types/contracts'; +import { getPublicFunctionDebugName } from '../../common/debug_fn_name.js'; import { type WorldStateDB } from '../../public/public_db_sources.js'; import { type TracedContractInstance } from '../../public/side_effect_trace.js'; import { type PublicSideEffectTraceInterface } from '../../public/side_effect_trace_interface.js'; @@ -257,9 +258,12 @@ export class AvmPersistableStateManager { if (!avmCallResults.reverted) { this.acceptNestedCallState(nestedState); } - const functionName = - (await this.worldStateDB.getDebugFunctionName(nestedEnvironment.address, nestedEnvironment.functionSelector)) ?? - `${nestedEnvironment.address}:${nestedEnvironment.functionSelector}`; + const functionName = await getPublicFunctionDebugName( + this.worldStateDB, + nestedEnvironment.address, + nestedEnvironment.functionSelector, + nestedEnvironment.calldata, + ); this.log.verbose(`[AVM] Calling nested function ${functionName}`); diff --git a/yarn-project/simulator/src/client/client_execution_context.ts b/yarn-project/simulator/src/client/client_execution_context.ts index 34dfb2aeec1d..d97efbfeefca 100644 --- a/yarn-project/simulator/src/client/client_execution_context.ts +++ b/yarn-project/simulator/src/client/client_execution_context.ts @@ -501,7 +501,7 @@ export class ClientExecutionContext extends ViewDataOracle { const args = this.packedValuesCache.unpack(argsHash); this.log.verbose( - `Created PublicExecutionRequest of type [${callType}], side-effect counter [${sideEffectCounter}] to ${targetContractAddress}:${functionSelector}(${targetArtifact.name})`, + `Created PublicExecutionRequest to ${targetArtifact.name}@${targetContractAddress}, of type [${callType}], side-effect counter [${sideEffectCounter}]`, ); const request = PublicExecutionRequest.from({ diff --git a/yarn-project/simulator/src/client/private_execution.ts b/yarn-project/simulator/src/client/private_execution.ts index 008f48fbfd99..16283a4ab8cc 100644 --- a/yarn-project/simulator/src/client/private_execution.ts +++ b/yarn-project/simulator/src/client/private_execution.ts @@ -19,10 +19,10 @@ export async function executePrivateFunction( artifact: FunctionArtifact, contractAddress: AztecAddress, functionSelector: FunctionSelector, - log = createDebugLogger('aztec:simulator:secret_execution'), + log = createDebugLogger('aztec:simulator:private_execution'), ): Promise { const functionName = await context.getDebugFunctionName(); - log.verbose(`Executing external function ${contractAddress}:${functionSelector}(${functionName})`); + log.verbose(`Executing external function ${functionName}@${contractAddress}`); const acir = artifact.bytecode; const initialWitness = context.getInitialWitness(artifact); const acvmCallback = new Oracle(context); diff --git a/yarn-project/simulator/src/common/debug_fn_name.ts b/yarn-project/simulator/src/common/debug_fn_name.ts new file mode 100644 index 000000000000..b8d9647d6b76 --- /dev/null +++ b/yarn-project/simulator/src/common/debug_fn_name.ts @@ -0,0 +1,22 @@ +import { type AztecAddress, Fr, FunctionSelector, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js'; + +import { type WorldStateDB } from '../public/public_db_sources.js'; + +export async function getPublicFunctionDebugName( + db: WorldStateDB, + contractAddress: AztecAddress, + fn: FunctionSelector, + calldata: Fr[], +): Promise { + if (fn.equals(FunctionSelector.fromField(new Fr(PUBLIC_DISPATCH_SELECTOR)))) { + // If the function is a dispatch, we need to look up the target function which + // is expected to be the first argument. + const targetFunction = + calldata[0] !== undefined + ? await db.getDebugFunctionName(contractAddress, FunctionSelector.fromField(calldata[0])) + : ``; + return `${targetFunction} (via dispatch)`; + } else { + return (await db.getDebugFunctionName(contractAddress, fn)) ?? `${contractAddress}:${fn}`; + } +} diff --git a/yarn-project/simulator/src/public/executor.ts b/yarn-project/simulator/src/public/executor.ts index 3f99b0368027..95204219af36 100644 --- a/yarn-project/simulator/src/public/executor.ts +++ b/yarn-project/simulator/src/public/executor.ts @@ -10,6 +10,7 @@ import { AvmExecutionEnvironment } from '../avm/avm_execution_environment.js'; import { AvmMachineState } from '../avm/avm_machine_state.js'; import { AvmSimulator } from '../avm/avm_simulator.js'; import { AvmPersistableStateManager } from '../avm/journal/index.js'; +import { getPublicFunctionDebugName } from '../common/debug_fn_name.js'; import { type PublicExecutionResult } from './execution.js'; import { ExecutorMetrics } from './executor_metrics.js'; import { type WorldStateDB } from './public_db_sources.js'; @@ -49,9 +50,9 @@ export class PublicExecutor { ): Promise { const address = executionRequest.contractAddress; const selector = executionRequest.callContext.functionSelector; - const fnName = (await this.worldStateDB.getDebugFunctionName(address, selector)) ?? `${address}:${selector}`; + const fnName = await getPublicFunctionDebugName(this.worldStateDB, address, selector, executionRequest.args); - PublicExecutor.log.verbose(`[AVM] Executing public external function ${fnName}.`); + PublicExecutor.log.verbose(`[AVM] Executing public external function ${fnName}@${address}.`); const timer = new Timer(); const trace = new PublicSideEffectTrace(startSideEffectCounter); diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index 8765294c788e..69f7884c697f 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -557,10 +557,10 @@ export class TXE implements TypedOracle { isDelegateCall: boolean, ) { this.logger.verbose( - `Executing external function ${targetContractAddress}:${functionSelector}(${await this.getDebugFunctionName( + `Executing external function ${await this.getDebugFunctionName( targetContractAddress, functionSelector, - )}) isStaticCall=${isStaticCall} isDelegateCall=${isDelegateCall}`, + )}@${targetContractAddress} isStaticCall=${isStaticCall} isDelegateCall=${isDelegateCall}`, ); // Store and modify env From 696e9ae5b01e98f41f06cc8daa988b05b8c14289 Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Thu, 3 Oct 2024 08:54:16 +0000 Subject: [PATCH 5/9] fix avm_simulator.test.ts --- .../contracts/avm_test_contract/src/main.nr | 5 ++ .../simulator/src/avm/avm_simulator.test.ts | 86 +++++++++++-------- 2 files changed, 54 insertions(+), 37 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr index 122f5ce92b3a..4cacc3920bf4 100644 --- a/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr @@ -480,6 +480,11 @@ contract AvmTest { AvmTest::at(nestedAddress).new_nullifier(nullifier + 1).call(&mut context); } + #[public] + fn nested_call_to_assert_same(arg_a: Field, arg_b: Field) -> pub Field { + AvmTest::at(context.this_address()).assert_same(arg_a, arg_b).call(&mut context) + } + /** * Enqueue a public call from private */ diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index 03078a1b713a..e3820001f6f6 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -101,6 +101,45 @@ describe('AVM simulator: transpiled Noir contracts', () => { expect(results.output).toEqual([new Fr(3)]); }); + it('addition via dispatch', async () => { + const calldata: Fr[] = [ + FunctionSelector.fromSignature('add_args_return(Field,Field)').toField(), + new Fr(1), + new Fr(2), + ]; + const context = initContext({ env: initExecutionEnvironment({ calldata }) }); + + const bytecode = getAvmTestContractBytecode('public_dispatch'); + const results = await new AvmSimulator(context).executeBytecode(bytecode); + + expect(results.reverted).toBe(false); + expect(results.output).toEqual([new Fr(3)]); + }); + + it('get_args_hash via dispatch', async () => { + const calldata = [new Fr(8), new Fr(1), new Fr(2), new Fr(3)]; + const dispatchCalldata = [FunctionSelector.fromSignature('get_args_hash(u8,[Field;3])').toField(), ...calldata]; + + const context = initContext({ env: initExecutionEnvironment({ calldata: dispatchCalldata }) }); + const bytecode = getAvmTestContractBytecode('public_dispatch'); + const results = await new AvmSimulator(context).executeBytecode(bytecode); + + expect(results.reverted).toBe(false); + expect(results.output).toEqual([computeVarArgsHash(calldata)]); + }); + + it('functionSelector getter via dispatch', async () => { + const selector = FunctionSelector.fromSignature('get_function_selector()').toField(); + const dispatchCalldata = [selector]; + + const context = initContext({ env: initExecutionEnvironment({ calldata: dispatchCalldata }) }); + const bytecode = getAvmTestContractBytecode('public_dispatch'); + const results = await new AvmSimulator(context).executeBytecode(bytecode); + + expect(results.reverted).toBe(false); + expect(results.output).toEqual([selector]); + }); + it('modulo and u1', async () => { const calldata: Fr[] = [new Fr(2)]; const context = initContext({ env: initExecutionEnvironment({ calldata }) }); @@ -318,7 +357,6 @@ describe('AVM simulator: transpiled Noir contracts', () => { ['address', address.toField(), 'get_address'], ['storageAddress', storageAddress.toField(), 'get_storage_address'], ['sender', sender.toField(), 'get_sender'], - ['functionSelector', functionSelector.toField(), 'get_function_selector'], ['transactionFee', transactionFee.toField(), 'get_transaction_fee'], ['chainId', chainId.toField(), 'get_chain_id'], ['version', version.toField(), 'get_version'], @@ -337,31 +375,6 @@ describe('AVM simulator: transpiled Noir contracts', () => { }); }); - describe('AvmContextInputs', () => { - it('selector', async () => { - const context = initContext({ - env: initExecutionEnvironment({ - functionSelector: FunctionSelector.fromSignature('check_selector()'), - }), - }); - const bytecode = getAvmTestContractBytecode('check_selector'); - const results = await new AvmSimulator(context).executeBytecode(bytecode); - - expect(results.reverted).toBe(false); - }); - - it('get_args_hash', async () => { - const calldata = [new Fr(8), new Fr(1), new Fr(2), new Fr(3)]; - - const context = initContext({ env: initExecutionEnvironment({ calldata }) }); - const bytecode = getAvmTestContractBytecode('get_args_hash'); - const results = await new AvmSimulator(context).executeBytecode(bytecode); - - expect(results.reverted).toBe(false); - expect(results.output).toEqual([computeVarArgsHash(calldata)]); - }); - }); - it('conversions', async () => { const calldata: Fr[] = [new Fr(0b1011101010100)]; const context = initContext({ env: initExecutionEnvironment({ calldata }) }); @@ -837,8 +850,8 @@ describe('AVM simulator: transpiled Noir contracts', () => { const calldata = [value0, value1]; const context = createContext(calldata); const callBytecode = getAvmTestContractBytecode('nested_call_to_add'); - const addBytecode = getAvmTestContractBytecode('add_args_return'); - mockGetBytecode(worldStateDB, addBytecode); + const nestedBytecode = getAvmTestContractBytecode('public_dispatch'); + mockGetBytecode(worldStateDB, nestedBytecode); const nestedTrace = mock(); mockTraceFork(trace, nestedTrace); @@ -853,8 +866,8 @@ describe('AVM simulator: transpiled Noir contracts', () => { const calldata = [value0, value1]; const context = createContext(calldata); const callBytecode = getAvmTestContractBytecode('nested_static_call_to_add'); - const addBytecode = getAvmTestContractBytecode('add_args_return'); - mockGetBytecode(worldStateDB, addBytecode); + const nestedBytecode = getAvmTestContractBytecode('public_dispatch'); + mockGetBytecode(worldStateDB, nestedBytecode); const nestedTrace = mock(); mockTraceFork(trace, nestedTrace); @@ -870,8 +883,8 @@ describe('AVM simulator: transpiled Noir contracts', () => { const calldata: Fr[] = [value0, value1, ...gas]; const context = createContext(calldata); const callBytecode = getAvmTestContractBytecode('nested_call_to_add_with_gas'); - const addBytecode = getAvmTestContractBytecode('add_args_return'); - mockGetBytecode(worldStateDB, addBytecode); + const nestedBytecode = getAvmTestContractBytecode('public_dispatch'); + mockGetBytecode(worldStateDB, nestedBytecode); mockTraceFork(trace); const results = await new AvmSimulator(context).executeBytecode(callBytecode); @@ -889,7 +902,7 @@ describe('AVM simulator: transpiled Noir contracts', () => { it(`Nested static call which modifies storage (expect failure)`, async () => { const context = createContext(); const callBytecode = getAvmTestContractBytecode('nested_static_call_to_set_storage'); - const nestedBytecode = getAvmTestContractBytecode('set_storage_single'); + const nestedBytecode = getAvmTestContractBytecode('public_dispatch'); mockGetBytecode(worldStateDB, nestedBytecode); mockTraceFork(trace); @@ -911,15 +924,14 @@ describe('AVM simulator: transpiled Noir contracts', () => { it(`Nested calls rethrow exceptions`, async () => { const calldata = [value0, value1]; const context = createContext(calldata); - const callBytecode = getAvmTestContractBytecode('nested_call_to_add'); - // We actually don't pass the function ADD, but it's ok because the signature is the same. - const nestedBytecode = getAvmTestContractBytecode('assert_same'); + const callBytecode = getAvmTestContractBytecode('nested_call_to_assert_same'); + const nestedBytecode = getAvmTestContractBytecode('public_dispatch'); mockGetBytecode(worldStateDB, nestedBytecode); const results = await new AvmSimulator(context).executeBytecode(callBytecode); expect(results.reverted).toBe(true); // The outer call should revert. expect(results.revertReason).toBeDefined(); - expect(resolveAvmTestContractAssertionMessage('assert_same', results.revertReason!)).toMatch( + expect(resolveAvmTestContractAssertionMessage('public_dispatch', results.revertReason!)).toMatch( 'Values are not equal', ); }); From 572cbd1f954dcf500dd0fcccd2882ed3ff88fd9d Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Thu, 3 Oct 2024 09:17:47 +0000 Subject: [PATCH 6/9] fix private_execution.test.ts --- .../simulator/src/client/private_execution.test.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/yarn-project/simulator/src/client/private_execution.test.ts b/yarn-project/simulator/src/client/private_execution.test.ts index a9ab3d33bb44..b6dab5ed0dae 100644 --- a/yarn-project/simulator/src/client/private_execution.test.ts +++ b/yarn-project/simulator/src/client/private_execution.test.ts @@ -20,6 +20,7 @@ import { L1_TO_L2_MSG_TREE_HEIGHT, NOTE_HASH_TREE_HEIGHT, PUBLIC_DATA_TREE_HEIGHT, + PUBLIC_DISPATCH_SELECTOR, PartialStateReference, StateReference, TxContext, @@ -833,13 +834,10 @@ describe('Private Execution test suite', () => { describe('enqueued calls', () => { it.each([false, true])('parent should enqueue call to child (internal %p)', async isInternal => { const parentArtifact = getFunctionArtifact(ParentContractArtifact, 'enqueue_call_to_child'); - const childContractArtifact = ChildContractArtifact.functions.find(fn => fn.name === 'pub_set_value')!; + const childContractArtifact = ChildContractArtifact.functions.find(fn => fn.name === 'public_dispatch')!; expect(childContractArtifact).toBeDefined(); const childAddress = AztecAddress.random(); - const childSelector = FunctionSelector.fromNameAndParameters( - childContractArtifact.name, - childContractArtifact.parameters, - ); + const childSelector = FunctionSelector.fromSignature('pub_set_value(Field)'); const parentAddress = AztecAddress.random(); oracle.getFunctionArtifact.mockImplementation(() => Promise.resolve({ ...childContractArtifact, isInternal })); @@ -855,11 +853,11 @@ describe('Private Execution test suite', () => { const request = new CountedPublicExecutionRequest( PublicExecutionRequest.from({ contractAddress: childAddress, - args: [new Fr(42n)], + args: [childSelector.toField(), new Fr(42n)], callContext: CallContext.from({ msgSender: parentAddress, storageContractAddress: childAddress, - functionSelector: childSelector, + functionSelector: FunctionSelector.fromField(new Fr(PUBLIC_DISPATCH_SELECTOR)), isDelegateCall: false, isStaticCall: false, }), From e8da40eeca9dbc098bb364c4254f8add289452f8 Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Thu, 3 Oct 2024 09:22:48 +0000 Subject: [PATCH 7/9] fix gas_validator.tes.ts --- .../src/tx_validator/gas_validator.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/yarn-project/sequencer-client/src/tx_validator/gas_validator.test.ts b/yarn-project/sequencer-client/src/tx_validator/gas_validator.test.ts index 23f368a4dd93..ba5d76b0a0e6 100644 --- a/yarn-project/sequencer-client/src/tx_validator/gas_validator.test.ts +++ b/yarn-project/sequencer-client/src/tx_validator/gas_validator.test.ts @@ -1,5 +1,5 @@ import { type Tx, mockTx } from '@aztec/circuit-types'; -import { AztecAddress, Fr, FunctionSelector, GasSettings } from '@aztec/circuits.js'; +import { AztecAddress, Fr, FunctionSelector, GasSettings, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js'; import { poseidon2Hash } from '@aztec/foundation/crypto'; import { FeeJuiceContract } from '@aztec/noir-contracts.js'; import { FeeJuiceAddress } from '@aztec/protocol-contracts/fee-juice'; @@ -67,10 +67,11 @@ describe('GasTxValidator', () => { it('allows fee paying txs if fee payer claims enough balance during setup', async () => { mockBalance(TX_FEE - 1n); + const selector = FunctionSelector.fromSignature('_increase_public_balance((Field),Field)'); patchNonRevertibleFn(tx, 0, { address: FeeJuiceAddress, - selector: FunctionSelector.fromSignature('_increase_public_balance((Field),Field)'), - args: [payer, new Fr(1n)], + selector: FunctionSelector.fromField(new Fr(PUBLIC_DISPATCH_SELECTOR)), + args: [selector.toField(), payer, new Fr(1n)], msgSender: FeeJuiceAddress, }); await expectValidateSuccess(tx); From 768b5ecef7887c53c03665e6d34d6c25c2b0419a Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Thu, 3 Oct 2024 09:25:45 +0000 Subject: [PATCH 8/9] update comments --- .../aztec-nr/aztec/src/context/private_context.nr | 12 ++++++++---- .../simulator/src/client/client_execution_context.ts | 12 ++++++++---- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/context/private_context.nr b/noir-projects/aztec-nr/aztec/src/context/private_context.nr index 1be1e3da4429..5374759a4283 100644 --- a/noir-projects/aztec-nr/aztec/src/context/private_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/private_context.nr @@ -468,8 +468,10 @@ impl PrivateContext { let counter = self.next_counter(); let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call; - // TODO(): - // WARNING: + // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Fix this. + // WARNING: This is insecure and should be temporary! + // The oracle repacks the arguments and returns a new args_hash. + // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function. let args_hash = enqueue_public_function_call_internal( contract_address, function_selector, @@ -516,8 +518,10 @@ impl PrivateContext { let counter = self.next_counter(); let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call; - // TODO(): - // WARNING: + // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Fix this. + // WARNING: This is insecure and should be temporary! + // The oracle repacks the arguments and returns a new args_hash. + // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function. let args_hash = set_public_teardown_function_call_internal( contract_address, function_selector, diff --git a/yarn-project/simulator/src/client/client_execution_context.ts b/yarn-project/simulator/src/client/client_execution_context.ts index d97efbfeefca..0f814daaf900 100644 --- a/yarn-project/simulator/src/client/client_execution_context.ts +++ b/yarn-project/simulator/src/client/client_execution_context.ts @@ -536,8 +536,10 @@ export class ClientExecutionContext extends ViewDataOracle { isStaticCall: boolean, isDelegateCall: boolean, ): Promise { - // TODO(): - // WARNING: + // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Fix this. + // WARNING: This is insecure and should be temporary! + // The oracle repacks the arguments and returns a new args_hash. + // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function. const newArgsHash = this.packedValuesCache.pack([ functionSelector.toField(), ...this.packedValuesCache.unpack(argsHash), @@ -573,8 +575,10 @@ export class ClientExecutionContext extends ViewDataOracle { isStaticCall: boolean, isDelegateCall: boolean, ): Promise { - // TODO(): - // WARNING: + // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Fix this. + // WARNING: This is insecure and should be temporary! + // The oracle repacks the arguments and returns a new args_hash. + // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function. const newArgsHash = this.packedValuesCache.pack([ functionSelector.toField(), ...this.packedValuesCache.unpack(argsHash), From 68758d818871e4262809ea5c23e0a44a246656bf Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Thu, 3 Oct 2024 10:03:48 +0000 Subject: [PATCH 9/9] update migration notes --- docs/docs/migration_notes.md | 16 +++++++++++----- .../aztec/src/context/private_context.nr | 4 ++++ .../src/client/client_execution_context.ts | 4 ++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/docs/docs/migration_notes.md b/docs/docs/migration_notes.md index dffb976a0695..1943e4cec73d 100644 --- a/docs/docs/migration_notes.md +++ b/docs/docs/migration_notes.md @@ -8,6 +8,12 @@ Aztec is in full-speed development. Literally every version breaks compatibility ## TBD +### Changes to public calling convention + +Contracts that include public functions (that is, marked with `#[public]`), are required to have a function `public_dispatch(selector: Field)` which acts as an entry point. This will be soon the only public function registered/deployed in contracts. The calling convention is updated so that external calls are made to this function. + +If you are writing your contracts using Aztec-nr, there is nothing you need to change. The `public_dispatch` function is automatically generated by the `#[aztec]` macro. + ### [Aztec.nr] Renamed `unsafe_rand` to `random` Since this is an `unconstrained` function, callers are already supposed to include an `unsafe` block, so this function has been renamed for reduced verbosity. @@ -64,8 +70,8 @@ contract XYZ { - numbers.at(owner).initialize(&mut new_number).emit(encode_and_encrypt_note_with_keys(&mut context, owner_ovpk_m, owner_ivpk_m, owner)); + numbers.at(owner).initialize(&mut new_number).emit(encode_and_encrypt_note(&mut context, owner_ovpk_m, owner_ivpk_m, owner)); - } +``` ## 0.56.0 @@ -208,6 +214,7 @@ export LOG_LEVEL="debug" - assert(verification == true); - true + std::ecdsa_secp256k1::verify_signature(public_key.x, public_key.y, signature, hashed_message) +``` ## 0.49.0 @@ -406,7 +413,7 @@ struct WithdrawalProcessed { ### [Aztec.nr] rename `encode_and_encrypt_with_keys` to `encode_and_encrypt_note_with_keys` -````diff +```diff contract XYZ { - use dep::aztec::encrypted_logs::encrypted_note_emission::encode_and_encrypt_with_keys; + use dep::aztec::encrypted_logs::encrypted_note_emission::encode_and_encrypt_note_with_keys; @@ -414,9 +421,8 @@ contract XYZ { - numbers.at(owner).initialize(&mut new_number).emit(encode_and_encrypt_with_keys(&mut context, owner_ovpk_m, owner_ivpk_m)); + numbers.at(owner).initialize(&mut new_number).emit(encode_and_encrypt_note_with_keys(&mut context, owner_ovpk_m, owner_ivpk_m)); - } - +``` ### [Aztec.nr] changes to `NoteInterface` @@ -469,7 +475,7 @@ These changes were done because having the note hash exposed allowed us to not h + (note_hash_for_nullify, nullifier) + } + } -```` +``` ### [Aztec.nr] `note_getter` returns `BoundedVec` diff --git a/noir-projects/aztec-nr/aztec/src/context/private_context.nr b/noir-projects/aztec-nr/aztec/src/context/private_context.nr index 5374759a4283..5feea1b244ef 100644 --- a/noir-projects/aztec-nr/aztec/src/context/private_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/private_context.nr @@ -472,6 +472,8 @@ impl PrivateContext { // WARNING: This is insecure and should be temporary! // The oracle repacks the arguments and returns a new args_hash. // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function. + // We don't validate or compute it in the circuit because a) it's harder to do with slices, and + // b) this is only temporary. let args_hash = enqueue_public_function_call_internal( contract_address, function_selector, @@ -522,6 +524,8 @@ impl PrivateContext { // WARNING: This is insecure and should be temporary! // The oracle repacks the arguments and returns a new args_hash. // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function. + // We don't validate or compute it in the circuit because a) it's harder to do with slices, and + // b) this is only temporary. let args_hash = set_public_teardown_function_call_internal( contract_address, function_selector, diff --git a/yarn-project/simulator/src/client/client_execution_context.ts b/yarn-project/simulator/src/client/client_execution_context.ts index 0f814daaf900..20ebf40e5c30 100644 --- a/yarn-project/simulator/src/client/client_execution_context.ts +++ b/yarn-project/simulator/src/client/client_execution_context.ts @@ -540,6 +540,8 @@ export class ClientExecutionContext extends ViewDataOracle { // WARNING: This is insecure and should be temporary! // The oracle repacks the arguments and returns a new args_hash. // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function. + // We don't validate or compute it in the circuit because a) it's harder to do with slices, and + // b) this is only temporary. const newArgsHash = this.packedValuesCache.pack([ functionSelector.toField(), ...this.packedValuesCache.unpack(argsHash), @@ -579,6 +581,8 @@ export class ClientExecutionContext extends ViewDataOracle { // WARNING: This is insecure and should be temporary! // The oracle repacks the arguments and returns a new args_hash. // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function. + // We don't validate or compute it in the circuit because a) it's harder to do with slices, and + // b) this is only temporary. const newArgsHash = this.packedValuesCache.pack([ functionSelector.toField(), ...this.packedValuesCache.unpack(argsHash),