Skip to content

Commit

Permalink
refactor: gets rid of unencrypted emit in private_context (#7236)
Browse files Browse the repository at this point in the history
This pr took a stab at something we had discussed related to #7161: 
- remove the unencrypted emit functions from the private context
- have the logic reside in the contracts that need it
  - `contract_instance_deployer`
  - `contract_class_registerer`
 
Since having unencrypted emits easily available in the private context
is a huge footgun privacy-wise, we decided that it would be better to
get rid of it there. However, as we still needed it for the deployer (as
we need something to deploy the "first" public code), we left the public
inputs and just insert directly into those instead of using a neat
function.

The setup is still slightly different from what we are doing in private,
because it is dealing with `event_type_id` slightly odd, and doing a lot
of inefficient things. So it needs to be revisited at some point for
optimisations.
When the event macros are refined to also handle structs with non-field
elements we should be able to use a `to_be_bytes` value from in there to
more cleanly emit the event, and also update the "listener" such that we
could get rid of the current
`DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE` and instead be looking
at just the `event_type_id` and the contract address to match it.
  • Loading branch information
LHerskind authored Jul 1, 2024
1 parent 88d43e7 commit 3e6d88e
Show file tree
Hide file tree
Showing 11 changed files with 94 additions and 199 deletions.
51 changes: 3 additions & 48 deletions noir-projects/aztec-nr/aztec/src/context/private_context.nr
Original file line number Diff line number Diff line change
@@ -1,17 +1,11 @@
use crate::{
context::{inputs::PrivateContextInputs, packed_returns::PackedReturns},
messaging::process_l1_to_l2_message,
hash::{hash_args_array, ArgsHasher, compute_unencrypted_log_hash},
messaging::process_l1_to_l2_message, hash::{hash_args_array, ArgsHasher},
keys::constants::{NULLIFIER_INDEX, OUTGOING_INDEX, NUM_KEY_TYPES, sk_generators},
note::note_interface::NoteInterface,
oracle::{
key_validation_request::get_key_validation_request, arguments, returns::pack_returns,
call_private_function::call_private_function_internal, header::get_header_at,
logs::{
emit_encrypted_note_log, emit_encrypted_event_log,
emit_contract_class_unencrypted_log_private_internal, emit_unencrypted_log_private_internal
},
logs_traits::{LensForEncryptedLog, ToBytesForUnencryptedLog},
logs::{emit_encrypted_note_log, emit_encrypted_event_log},
enqueue_public_function_call::{
enqueue_public_function_call_internal, set_public_teardown_function_call_internal,
parse_public_call_stack_item_from_oracle
Expand All @@ -36,10 +30,7 @@ use dep::protocol_types::{
MAX_KEY_VALIDATION_REQUESTS_PER_CALL, MAX_ENCRYPTED_LOGS_PER_CALL, MAX_UNENCRYPTED_LOGS_PER_CALL,
MAX_NOTE_ENCRYPTED_LOGS_PER_CALL
},
contrakt::{storage_read::StorageRead, storage_update_request::StorageUpdateRequest},
grumpkin_private_key::GrumpkinPrivateKey, grumpkin_point::GrumpkinPoint, header::Header,
messaging::l2_to_l1_message::L2ToL1Message, utils::reader::Reader, traits::{is_empty, Empty},
utils::arrays::find_index
header::Header, messaging::l2_to_l1_message::L2ToL1Message, utils::reader::Reader, traits::Empty
};

// When finished, one can call .finish() to convert back to the abi
Expand Down Expand Up @@ -270,42 +261,6 @@ impl PrivateContext {
}
// docs:end:consume_l1_to_l2_message

// TODO: We might want to remove this since emitting unencrypted logs from private functions is violating privacy.
// --> might be a better approach to force devs to make a public function call that emits the log if needed then
// it would be less easy to accidentally leak information.
// If we decide to keep this function around would make sense to wait for traits and then merge it with emit_unencrypted_log.
pub fn emit_unencrypted_log<T, N, M>(&mut self, log: T) where T: ToBytesForUnencryptedLog<N, M> {
let event_selector = 5; // TODO: compute actual event selector.
let contract_address = self.this_address();
let counter = self.next_counter();
let log_slice = log.to_be_bytes_arr();
let log_hash = compute_unencrypted_log_hash(contract_address, event_selector, log);
// 44 = addr (32) + selector (4) + raw log len (4) + processed log len (4)
let len = 44 + log_slice.len().to_field();
let side_effect = LogHash { value: log_hash, counter, length: len };
self.unencrypted_logs_hashes.push(side_effect);
// call oracle
let _void = emit_unencrypted_log_private_internal(contract_address, event_selector, log, counter);
}

// 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
pub fn emit_contract_class_unencrypted_log<N>(&mut self, log: [Field; N]) {
let event_selector = 5; // TODO: compute actual event selector.
let contract_address = self.this_address();
let counter = self.next_counter();
let log_hash = emit_contract_class_unencrypted_log_private_internal(contract_address, event_selector, log, counter);
// 44 = addr (32) + selector (4) + raw log len (4) + processed log len (4)
let len = 44 + N * 32;
let side_effect = LogHash { value: log_hash, counter, length: len };
self.unencrypted_logs_hashes.push(side_effect);
}

// NB: A randomness value of 0 signals that the kernels should not mask the contract address
// used in siloing later on e.g. 'handshaking' contract w/ known address.
pub fn emit_raw_event_log_with_masked_address<M>(&mut self, randomness: Field, encrypted_log: [u8; M]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ contract ContractClassRegisterer {
ARTIFACT_FUNCTION_TREE_MAX_HEIGHT, FUNCTION_TREE_HEIGHT,
MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS, REGISTERER_CONTRACT_CLASS_REGISTERED_MAGIC_VALUE
},
traits::Serialize
traits::Serialize, abis::log_hash::LogHash
};

use dep::aztec::{context::PrivateContext, oracle::logs::emit_contract_class_unencrypted_log_private_internal};

use crate::events::{
class_registered::ContractClassRegistered,
private_function_broadcasted::{ClassPrivateFunctionBroadcasted, PrivateFunction},
Expand Down Expand Up @@ -52,7 +54,7 @@ contract ContractClassRegisterer {
public_bytecode_commitment
]
);
context.emit_contract_class_unencrypted_log(event.serialize());
emit_contract_class_unencrypted_log(&mut context, event.serialize());
}

#[aztec(private)]
Expand Down Expand Up @@ -87,7 +89,7 @@ contract ContractClassRegisterer {
function_data.metadata_hash
]
);
context.emit_contract_class_unencrypted_log(event.serialize());
emit_contract_class_unencrypted_log(&mut context, event.serialize());
}

#[aztec(private)]
Expand Down Expand Up @@ -117,6 +119,25 @@ contract ContractClassRegisterer {
function_data.metadata_hash
]
);
context.emit_contract_class_unencrypted_log(event.serialize());
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<N>(context: &mut PrivateContext, log: [Field; N]) {
let event_selector = 5; // TODO: compute actual event selector.
let contract_address = context.this_address();
let counter = context.next_counter();
let log_hash = emit_contract_class_unencrypted_log_private_internal(contract_address, event_selector, log, counter);
// 44 = addr (32) + selector (4) + raw log len (4) + processed log len (4)
let len = 44 + N * 32;
let side_effect = LogHash { value: log_hash, counter, length: len };
context.unencrypted_logs_hashes.push(side_effect);
}
}

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,13 +1,42 @@
mod events;

contract ContractInstanceDeployer {
use dep::aztec::protocol_types::{
address::{AztecAddress, EthAddress, PublicKeysHash, PartialAddress},
contract_class_id::ContractClassId, constants::DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE,
traits::Serialize
traits::Serialize, abis::log_hash::LogHash
};
use dep::aztec::{
context::PrivateContext, hash::compute_unencrypted_log_hash,
oracle::logs::emit_unencrypted_log_private_internal
};

use crate::events::{instance_deployed::ContractInstanceDeployed};
// @todo This should be using an event, but currently we only support fields in the struct.
// #[aztec(event)]
struct ContractInstanceDeployed {
address: AztecAddress,
version: u8,
salt: Field,
contract_class_id: ContractClassId,
initialization_hash: Field,
public_keys_hash: PublicKeysHash,
deployer: AztecAddress,
}

global CONTRACT_INSTANCE_DEPLOYED_SERIALIZED_SIZE: Field = 8;

impl Serialize<CONTRACT_INSTANCE_DEPLOYED_SERIALIZED_SIZE> for ContractInstanceDeployed {
fn serialize(self: Self) -> [Field; CONTRACT_INSTANCE_DEPLOYED_SERIALIZED_SIZE] {
[
DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE,
self.address.to_field(),
self.version as Field,
self.salt,
self.contract_class_id.to_field(),
self.initialization_hash,
self.public_keys_hash.to_field(),
self.deployer.to_field(),
]
}
}

#[aztec(private)]
fn deploy(
Expand All @@ -34,8 +63,25 @@ contract ContractInstanceDeployer {

// Broadcast the event
let event = ContractInstanceDeployed { contract_class_id, address, public_keys_hash, initialization_hash, salt, deployer, version: 1 };
let event_payload = event.serialize();
dep::aztec::oracle::debug_log::debug_log_format("ContractInstanceDeployed: {}", event_payload);
context.emit_unencrypted_log(event_payload);

let payload = event.serialize();
dep::aztec::oracle::debug_log::debug_log_format("ContractInstanceDeployed: {}", payload);

let contract_address = context.this_address();
let counter = context.next_counter();

// The event_type_id is not strictly needed. So i'm setting it to 0 here, and we can then purge it
// later on.
let event_type_id = 0;

// @todo This is very inefficient, we are doing a lot of back and forth conversions.
let log_slice = payload.to_be_bytes_arr();
let log_hash = compute_unencrypted_log_hash(contract_address, event_type_id, payload);
// 44 = addr (32) + selector (4) + raw log len (4) + processed log len (4)
let len = 44 + log_slice.len().to_field();
let side_effect = LogHash { value: log_hash, counter, length: len };
context.unencrypted_logs_hashes.push(side_effect);

let _void = emit_unencrypted_log_private_internal(contract_address, event_type_id, payload, counter);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@ contract Crowdfunding {

// docs:start:all-deps
use dep::aztec::{
protocol_types::{
abis::function_selector::FunctionSelector, address::AztecAddress, traits::Serialize,
grumpkin_point::GrumpkinPoint
},
protocol_types::address::AztecAddress,
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note,
state_vars::{PrivateSet, PublicImmutable, SharedImmutable}
};
use dep::aztec::unencrypted_logs::unencrypted_event_emission::encode_event;
use dep::value_note::value_note::ValueNote;
use dep::token::Token;
// docs:end:all-deps
Expand Down Expand Up @@ -95,10 +93,14 @@ contract Crowdfunding {

// 2) Transfer the donation tokens from this contract to the operator
Token::at(storage.donation_token.read_private()).transfer(operator_address, amount as Field).call(&mut context);

// 3) Emit an unencrypted event so that anyone can audit how much the operator has withdrawn
let event = WithdrawalProcessed { amount: amount as Field, who: operator_address.to_field() };
context.emit_unencrypted_log(event.serialize());
Crowdfunding::at(context.this_address())._publish_donation_receipts(amount, operator_address).enqueue(&mut context);
}
// docs:end:operator-withdrawals

#[aztec(public)]
#[aztec(internal)]
fn _publish_donation_receipts(amount: u64, to: AztecAddress) {
WithdrawalProcessed { amount: amount as Field, who: to.to_field() }.emit(encode_event(&mut context));
}
}
19 changes: 0 additions & 19 deletions noir-projects/noir-contracts/contracts/test_contract/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -299,25 +299,6 @@ contract Test {
}
}

#[aztec(private)]
fn emit_msg_sender() {
context.emit_unencrypted_log(context.msg_sender());
}

#[aztec(private)]
fn emit_unencrypted_logs(fields: [Field; 5], nest: bool) {
// Merged two fns to avoid hitting max #functions limit:
// nest -> emit_unencrypted_logs_nested
// else -> emit_array_as_unencrypted_log
if nest {
Test::at(context.this_address()).emit_msg_sender().call(&mut context);
Test::at(context.this_address()).emit_unencrypted_logs(fields, false).call(&mut context);
context.emit_unencrypted_log("test");
} else {
context.emit_unencrypted_log(fields);
}
}

#[aztec(private)]
fn emit_encrypted_logs_nested(value: Field, owner: AztecAddress, outgoing_viewer: AztecAddress) {
let mut storage_slot = storage.example_constant.get_storage_slot() + 1;
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/circuits.js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,4 @@
]
]
}
}
}
18 changes: 0 additions & 18 deletions yarn-project/end-to-end/src/e2e_block_building.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,24 +246,6 @@ describe('e2e_block_building', () => {
testContract = await TestContract.deploy(owner).send().deployed();
}, 60_000);

it('calls a method with nested unencrypted logs', async () => {
const tx = await testContract.methods.emit_unencrypted_logs([1, 2, 3, 4, 5], true).send().wait();
const logs = (await pxe.getUnencryptedLogs({ txHash: tx.txHash })).logs.map(l => l.log);

// First log should be contract address
expect(logs[0].data).toEqual(testContract.address.toBuffer());

// Second log should be array of fields
let expectedBuffer = Buffer.concat([1, 2, 3, 4, 5].map(num => new Fr(num).toBuffer()));
expect(logs[1].data.subarray(-32 * 5)).toEqual(expectedBuffer);

// Third log should be string "test"
expectedBuffer = Buffer.concat(
['t', 'e', 's', 't'].map(num => Buffer.concat([Buffer.alloc(31), Buffer.from(num)])),
);
expect(logs[2].data.subarray(-32 * 5)).toEqual(expectedBuffer);
}, 60_000);

it('calls a method with nested note encrypted logs', async () => {
// account setup
const privateKey = new Fr(7n);
Expand Down
25 changes: 1 addition & 24 deletions yarn-project/end-to-end/src/e2e_non_contract_account.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,4 @@
import {
type DebugLogger,
ExtendedNote,
Fr,
Note,
type PXE,
SignerlessWallet,
type Wallet,
toBigInt,
} from '@aztec/aztec.js';
import { type DebugLogger, ExtendedNote, Fr, Note, type PXE, SignerlessWallet, type Wallet } from '@aztec/aztec.js';
import { siloNullifier } from '@aztec/circuits.js/hash';
import { TestContract } from '@aztec/noir-contracts.js/Test';

Expand Down Expand Up @@ -50,20 +41,6 @@ describe('e2e_non_contract_account', () => {
expect(siloedNullifier.equals(expectedSiloedNullifier)).toBeTruthy();
});

it('msg.sender is 0 when a non-contract account calls a private function on a contract', async () => {
const contractWithNoContractWallet = await TestContract.at(contract.address, nonContractAccountWallet);

// Send transaction as arbitrary non-contract account
const tx = contractWithNoContractWallet.methods.emit_msg_sender().send();
await tx.wait({ interval: 0.1 });

const logs = (await tx.getUnencryptedLogs()).logs;
expect(logs.length).toBe(1);

const msgSender = toBigInt(logs[0].log.data);
expect(msgSender).toBe(0n);
});

// Note: This test doesn't really belong here as it doesn't have anything to do with non-contract accounts. I needed
// to test the TestNote functionality and it doesn't really fit anywhere else. Creating a separate e2e test for this
// seems wasteful. Move this test if a better place is found.
Expand Down
Loading

0 comments on commit 3e6d88e

Please sign in to comment.