From 699ced40612323386dbda28ab08a67d955780938 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 2 Oct 2024 21:13:35 +0000 Subject: [PATCH 1/2] Add comments to log oracles --- .../aztec-nr/aztec/src/oracle/logs.nr | 43 ++++++++----------- .../src/main.nr | 29 +++++++------ .../src/main.nr | 9 ++-- 3 files changed, 40 insertions(+), 41 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/oracle/logs.nr b/noir-projects/aztec-nr/aztec/src/oracle/logs.nr index f0a4e457e95..af0a5d5e756 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/logs.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/logs.nr @@ -1,38 +1,29 @@ use dep::protocol_types::address::AztecAddress; -// This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. - -pub fn emit_encrypted_note_log( - note_hash_counter: u32, - encrypted_note: [u8; M], - counter: u32 -) { +/// Informs the simulator that an encrypted note log has been emitted, helping it keep track of side-effects and easing +/// debugging. +pub fn emit_encrypted_note_log(note_hash_counter: u32, encrypted_note: [u8; M], counter: u32) { // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. unsafe { emit_encrypted_note_log_oracle_wrapper(note_hash_counter, encrypted_note, counter) } } -pub fn emit_encrypted_event_log( - contract_address: AztecAddress, - randomness: Field, - encrypted_event: [u8; M], - counter: u32 -) { +/// Informs the simulator that an encrypted event log has been emitted, helping it keep track of side-effects and easing +/// debugging. +pub fn emit_encrypted_event_log(contract_address: AztecAddress, randomness: Field, encrypted_event: [u8; M], counter: u32) { // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. unsafe { emit_encrypted_event_log_oracle_wrapper(contract_address, randomness, encrypted_event, counter) } } -pub fn emit_unencrypted_log_private_internal( - contract_address: AztecAddress, - message: T, - counter: u32 -) { +/// Informs the simulator that an unencrypted log has been emitted, helping it keep track of side-effects and easing +/// debugging. +pub fn emit_unencrypted_log_private(contract_address: AztecAddress, message: T, counter: u32) { // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. unsafe { - emit_unencrypted_log_private_internal_oracle_wrapper(contract_address, message, counter) + emit_unencrypted_log_private_oracle_wrapper(contract_address, message, counter) } } @@ -49,16 +40,18 @@ unconstrained fn emit_encrypted_event_log_oracle_wrapper( emit_encrypted_event_log_oracle(contract_address, randomness, encrypted_event, counter) } -unconstrained fn emit_unencrypted_log_private_internal_oracle_wrapper(contract_address: AztecAddress, message: T, counter: u32) { - let _ = emit_unencrypted_log_oracle_private(contract_address, message, counter); +unconstrained fn emit_unencrypted_log_private_oracle_wrapper(contract_address: AztecAddress, message: T, counter: u32) { + let _ = emit_unencrypted_log_private_oracle(contract_address, message, counter); } -unconstrained pub fn emit_contract_class_unencrypted_log_private_internal( +/// Temporary substitute for `emit_unencrypted_log_private` that is used for handling contract class registration. This +/// variant returns the log hash, which would be too large to compute inside a circuit. +unconstrained pub fn emit_contract_class_unencrypted_log_private( contract_address: AztecAddress, message: [Field; N], counter: u32 ) -> Field { - emit_contract_class_unencrypted_log_private(contract_address, message, counter) + emit_contract_class_unencrypted_log_private_oracle(contract_address, message, counter) } // = 480 + 32 * N bytes @@ -78,14 +71,14 @@ unconstrained fn emit_encrypted_event_log_oracle( ) {} #[oracle(emitUnencryptedLog)] -unconstrained fn emit_unencrypted_log_oracle_private( +unconstrained fn emit_unencrypted_log_private_oracle( _contract_address: AztecAddress, _message: T, _counter: u32 ) -> Field {} #[oracle(emitContractClassUnencryptedLog)] -unconstrained fn emit_contract_class_unencrypted_log_private( +unconstrained fn emit_contract_class_unencrypted_log_private_oracle( contract_address: AztecAddress, message: [Field; N], counter: u32 diff --git a/noir-projects/noir-contracts/contracts/contract_class_registerer_contract/src/main.nr b/noir-projects/noir-contracts/contracts/contract_class_registerer_contract/src/main.nr index a4ecf782c50..3112cd5dbcf 100644 --- a/noir-projects/noir-contracts/contracts/contract_class_registerer_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/contract_class_registerer_contract/src/main.nr @@ -17,7 +17,7 @@ contract ContractClassRegisterer { }; use dep::aztec::{ - context::PrivateContext, oracle::logs::emit_contract_class_unencrypted_log_private_internal, + context::PrivateContext, oracle::logs::emit_contract_class_unencrypted_log_private, macros::functions::private }; @@ -141,21 +141,24 @@ contract ContractClassRegisterer { emit_contract_class_unencrypted_log(&mut context, event.serialize()); } - // This fn exists separately from emit_unencrypted_log because sha hashing the preimage - // is too large to compile (16,200 fields, 518,400 bytes) => the oracle hashes it - // It is ONLY used with contract_class_registerer_contract since we already assert correctness: - // - Contract class -> we will commit to the packed bytecode (currently a TODO) - // - Private function -> we provide a membership proof - // - Unconstrained function -> we provide a membership proof - // Ordinary logs are not protected by the above so this fn shouldn't be called by anything else #[contract_library_method] - pub fn emit_contract_class_unencrypted_log(context: &mut PrivateContext, log: [Field; N]) { + fn emit_contract_class_unencrypted_log(context: &mut PrivateContext, log: [Field; N]) { let contract_address = context.this_address(); let counter = context.next_counter(); - let log_hash = emit_contract_class_unencrypted_log_private_internal(contract_address, log, counter); + + // The log preimage is too large for the hash to compile (16,200 fields, 518,400 bytes), so we do this via a + // specialized oracle. + // A malicious oracle cannot force us to use an incorrect bytecode: + // - Contract class -> we will commit to the packed bytecode (currently a TODO) + // - Private function -> we provide a membership proof + // - Unconstrained function -> we provide a membership proof + // However, the sequencer will be required to know a contract's preimage if it is called and the sequencer + // cannot prove non-registration. Therefore, it is possible that a malicious oracle might prevent sequencers + // from including transactions with calls to certain badly-broadcasted contracts. + // TODO(#8978): review correctness + let log_hash = emit_contract_class_unencrypted_log_private(contract_address, log, counter); + // 40 = addr (32) + raw log len (4) + processed log len (4) - let len = 40 + N * 32; - let side_effect = LogHash { value: log_hash, counter, length: len as Field }; - context.unencrypted_logs_hashes.push(side_effect); + context.unencrypted_logs_hashes.push(LogHash { value: log_hash, counter, length: 40 + (N as Field) * 32 }); } } diff --git a/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr b/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr index c6855fd9d68..4afcabfc4bd 100644 --- a/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr @@ -8,7 +8,7 @@ contract ContractInstanceDeployer { traits::Serialize }; use dep::aztec::{ - hash::compute_unencrypted_log_hash, oracle::logs::emit_unencrypted_log_private_internal, + hash::compute_unencrypted_log_hash, oracle::logs::emit_unencrypted_log_private, macros::{events::event, functions::private}, utils::to_bytes::arr_to_be_bytes_arr }; use std::meta::derive; @@ -73,8 +73,11 @@ contract ContractInstanceDeployer { // 40 = addr (32) + raw log len (4) + processed log len (4) let len = 40 + serialized_log.len().to_field(); let side_effect = LogHash { value: log_hash, counter, length: len }; - context.unencrypted_logs_hashes.push(side_effect); - emit_unencrypted_log_private_internal(contract_address, payload, counter); + // We manually push the unencrypted log to the context and inform the execution environment about this because + // PrivateContext does not expose a `emit_unencrypted_log` method - this mechanism is considered error-prone + // and only some canonical contracts use it. + context.unencrypted_logs_hashes.push(side_effect); + emit_unencrypted_log_private(contract_address, payload, counter); } } From 13855cc12b2c0281cd7c2a044ef2839046cce577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 3 Oct 2024 16:10:54 -0300 Subject: [PATCH 2/2] Update noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr [no ci] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jan Beneš --- .../contracts/contract_instance_deployer_contract/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr b/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr index 4afcabfc4bd..98b576ce140 100644 --- a/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr @@ -75,7 +75,7 @@ contract ContractInstanceDeployer { let side_effect = LogHash { value: log_hash, counter, length: len }; // We manually push the unencrypted log to the context and inform the execution environment about this because - // PrivateContext does not expose a `emit_unencrypted_log` method - this mechanism is considered error-prone + // PrivateContext does not expose an `emit_unencrypted_log` method - this mechanism is considered error-prone // and only some canonical contracts use it. context.unencrypted_logs_hashes.push(side_effect); emit_unencrypted_log_private(contract_address, payload, counter);