Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: separate bytecode logs from unencrypted logs #9891

Merged
merged 11 commits into from
Nov 14, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,6 @@ impl PrivateCallDataValidator {
public_inputs.encrypted_logs_hashes,
self.array_lengths.encrypted_logs_hashes,
);
// TODO(Miranda): remove below as we only have 1 bytecode log?
validate_incrementing_counters_within_range(
counter_start,
counter_end,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ impl PrivateKernelCircuitOutputValidator {
previous_kernel.end.encrypted_logs_hashes,
array_lengths.encrypted_logs_hashes,
);
// TODO(Miranda): remove below as we only have 1 bytecode log?
assert_array_prepended(
self.output.end.contract_class_logs_hashes,
previous_kernel.end.contract_class_logs_hashes,
Expand Down Expand Up @@ -324,7 +323,6 @@ impl PrivateKernelCircuitOutputValidator {
offsets.encrypted_logs_hashes,
contract_address,
);
// TODO(Miranda): remove below as we only have 1 bytecode log?
assert_array_appended_scoped(
self.output.end.contract_class_logs_hashes,
private_call.contract_class_logs_hashes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ impl PrivateKernelCircuitPublicInputsComposer {
// Note hashes, nullifiers, note_encrypted_logs_hashes, and encrypted_logs_hashes are sorted in the reset circuit.
self.public_inputs.end.l2_to_l1_msgs.storage =
sort_by_counter_asc(self.public_inputs.end.l2_to_l1_msgs.storage);
// TODO(Miranda): remove below as we only have 1 bytecode log?
self.public_inputs.end.contract_class_logs_hashes.storage =
sort_by_counter_asc(self.public_inputs.end.contract_class_logs_hashes.storage);
self.public_inputs.end.public_call_requests.storage =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ impl TailOutputValidator {
self.hints.sorted_l2_to_l1_msg_hints,
);

// TODO(Miranda): remove below as we only have 1 bytecode log?
assert_exposed_sorted_transformed_value_array(
self.previous_kernel.end.contract_class_logs_hashes,
self.output.end.contract_class_logs_hashes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ use dep::types::{
pub struct TailOutputHints {
// L2 to l1 msgs.
sorted_l2_to_l1_msg_hints: [OrderHint; MAX_L2_TO_L1_MSGS_PER_TX],
// TODO(Miranda): remove below as we only have 1 bytecode log?
// Bytecode log hashes.
// Contract class log hashes.
sorted_contract_class_log_hashes: [ScopedLogHash; MAX_CONTRACT_CLASS_LOGS_PER_TX],
sorted_contract_class_log_hash_hints: [OrderHint; MAX_CONTRACT_CLASS_LOGS_PER_TX],
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub unconstrained fn split_to_public(
}
}
}
// TODO(Miranda): remove below as we only have 1 bytecode log?

let contract_class_logs_hashes = data.contract_class_logs_hashes;
for i in 0..contract_class_logs_hashes.max_len() {
if i < contract_class_logs_hashes.len() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ impl TailToPublicOutputValidator {
hints.sorted_l2_to_l1_msg_hints,
);

// TODO(Miranda): remove below as we only have 1 bytecode log?
// contract_class_logs_hashes
assert_split_sorted_transformed_value_arrays_asc(
prev_data.contract_class_logs_hashes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ use dep::types::{
pub struct TailToPublicOutputHints {
// L2 to l1 msgs.
sorted_l2_to_l1_msg_hints: SplitOrderHints<MAX_L2_TO_L1_MSGS_PER_TX>,
// TODO(Miranda): remove below as we only have 1 bytecode log?
// Bytecode log hashes.
// Contract class log hashes.
sorted_contract_class_log_hash_hints: SplitOrderHints<MAX_CONTRACT_CLASS_LOGS_PER_TX>,
// Public call requests.
sorted_public_call_request_hints: SplitOrderHints<MAX_ENQUEUED_CALLS_PER_TX>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ pub global MAX_KEY_VALIDATION_REQUESTS_PER_CALL: u32 = 16;
pub global MAX_NOTE_ENCRYPTED_LOGS_PER_CALL: u32 = 16;
pub global MAX_ENCRYPTED_LOGS_PER_CALL: u32 = 4;
pub global MAX_UNENCRYPTED_LOGS_PER_CALL: u32 = 4;
// TODO(Miranda): remove arrays and make single?
pub global MAX_CONTRACT_CLASS_LOGS_PER_CALL: u32 = 1;

// TREES RELATED CONSTANTS
Expand Down Expand Up @@ -96,7 +95,6 @@ pub global MAX_KEY_VALIDATION_REQUESTS_PER_TX: u32 = 64;
pub global MAX_NOTE_ENCRYPTED_LOGS_PER_TX: u32 = 64;
pub global MAX_ENCRYPTED_LOGS_PER_TX: u32 = 8;
pub global MAX_UNENCRYPTED_LOGS_PER_TX: u32 = 8;
// TODO(Miranda): remove arrays and make single?
pub global MAX_CONTRACT_CLASS_LOGS_PER_TX: u32 = 1;
// docs:end:constants

Expand Down
8 changes: 8 additions & 0 deletions yarn-project/circuit-types/src/tx/tx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,14 @@ export class Tx extends Gossipable {
kernelOutput.endNonRevertibleData.noteEncryptedLogsHashes,
EncryptedNoteTxL2Logs.empty(),
);

// See comment in enqueued_calls_processor.ts -> tx.filterRevertedLogs()
if (this.data.forPublic) {
this.contractClassLogs = this.contractClassLogs.filterScoped(
this.data.forPublic?.nonRevertibleAccumulatedData.contractClassLogsHashes,
ContractClassTxL2Logs.empty(),
);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,17 @@ export class PrivateKernelResetPrivateInputsBuilder {

const numLogs = this.previousKernel.end.encryptedLogsHashes.filter(l => !l.logHash.randomness.isZero()).length;
const numToSilo = Math.max(0, numLogs - this.numTransientData);
this.requestedDimensions.ENCRYPTED_LOG_SILOING_AMOUNT = numToSilo;
// The reset circuit checks that capped_size must be greater than or equal to all non-empty logs.
// Since there is no current config with ENCRYPTED_LOG_SILOING_AMOUNT = 0 (only 1+), it defaults to 1,
// so the circuit fails when we have more than 1 log and require no siloing.
const numLogsNoSiloing = this.previousKernel.end.encryptedLogsHashes.filter(
l => !l.logHash.isEmpty() && l.logHash.randomness.isZero(),
).length;
const cappedSize = !numToSilo && numLogsNoSiloing > 1 ? numLogsNoSiloing : numToSilo;
// NB: This is a little flimsy, and only works because we have either ENCRYPTED_LOG_SILOING_AMOUNT=1 or 8.
// e.g. if we have 2 logs that need siloing, and 2 that dont, then numLogs = ENCRYPTED_LOG_SILOING_AMOUNT = 2
// This would fail because the circuit thinks that cappedSize = 2, but we have 4 logs.
this.requestedDimensions.ENCRYPTED_LOG_SILOING_AMOUNT = cappedSize;
Comment on lines +457 to +467
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes something I believe is in master - reset will fail for any number of encrypted logs which don't need siloing above 1.
It works now because we'd never tested more than 1 encrypted log with randomness set to 0. In that case, ENCRYPTED_LOG_SILOING_AMOUNT=0 in the above function, and the reset variant with =1 is chosen. Since there is only 1 log overall, it passes. Otherwise, it fails the capped_size check for the same reasons we modify the NULLIFIER_SILOING_AMOUNT just above this fn.


return numToSilo > 0;
}
Expand Down
Loading