Skip to content

Commit

Permalink
chore: sort public call requests in the circuits (#6650)
Browse files Browse the repository at this point in the history
This removes some patching we were doing in TS around the public call
stack items.

Now, we follow the existing pattern to sort public call stack items in
TS, then assert the sort in the circuit.

The only difference is that call *stack* items need a reverse sort, so
the one with the lowest side effect counter is last (and so is run
first).

Also, started making circuit types in TS readonly.
  • Loading branch information
just-mitch authored May 27, 2024
1 parent 1945ed9 commit f67d6f3
Show file tree
Hide file tree
Showing 16 changed files with 306 additions and 150 deletions.
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -166,5 +166,6 @@
"**/l1-contracts/lib/**": true,
"**/barretenberg/cpp/build*/**": true
},
"cmake.sourceDirectory": "${workspaceFolder}/barretenberg/cpp"
"cmake.sourceDirectory": "${workspaceFolder}/barretenberg/cpp",
"noir.nargoPath": "./noir/noir-repo/target/release/nargo"
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ use dep::types::{
private_kernel_data::PrivateKernelData,
kernel_circuit_public_inputs::{KernelCircuitPublicInputs, PrivateKernelCircuitPublicInputsBuilder, PublicKernelCircuitPublicInputs},
note_hash::ScopedNoteHash, nullifier::ScopedNullifier, side_effect::Ordered,
log_hash::{NoteLogHash, ScopedLogHash, ScopedEncryptedLogHash}, gas::Gas
log_hash::{NoteLogHash, ScopedLogHash, ScopedEncryptedLogHash}, gas::Gas, call_request::CallRequest
},
constants::{
MAX_NEW_NOTE_HASHES_PER_TX, MAX_NEW_NULLIFIERS_PER_TX, MAX_ENCRYPTED_LOGS_PER_TX,
MAX_UNENCRYPTED_LOGS_PER_TX, MAX_NOTE_ENCRYPTED_LOGS_PER_TX
MAX_UNENCRYPTED_LOGS_PER_TX, MAX_NOTE_ENCRYPTED_LOGS_PER_TX, MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX
},
hash::{
compute_l2_to_l1_hash, compute_note_hash_nonce, compute_unique_note_hash, silo_note_hash,
Expand All @@ -20,6 +20,10 @@ fn asc_sort_by_counters<T>(a: T, b: T) -> bool where T: Ordered {
a.counter() < b.counter()
}

fn desc_sort_by_counters<T>(a: T, b: T) -> bool where T: Ordered {
a.counter() > b.counter()
}

// Builds:
// .finish -> KernelCircuitPublicInputs (from PrivateKernelTailCircuitPrivateInputs)
// .finish_to_public -> PublicKernelCircuitPublicInputs (from PrivateKernelTailToPublicCircuitPrivateInputs)
Expand Down Expand Up @@ -92,10 +96,14 @@ impl KernelCircuitPublicInputsComposer {
*self
}

pub fn compose_public(&mut self) -> Self {
pub fn compose_public(
&mut self,
sorted_call_requests: [CallRequest; MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX],
sorted_call_requests_indexes: [u64; MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX]
) -> Self {
let _ = self.compose();

self.propagate_sorted_public_call_requests();
self.propagate_sorted_public_call_requests(sorted_call_requests, sorted_call_requests_indexes);
self.propagate_public_teardown_call_request();

*self
Expand Down Expand Up @@ -249,9 +257,19 @@ impl KernelCircuitPublicInputsComposer {
self.public_inputs.end.new_l2_to_l1_msgs = array_to_bounded_vec(accumulated_data.new_l2_to_l1_msgs);
}

fn propagate_sorted_public_call_requests(&mut self) {
fn propagate_sorted_public_call_requests(
&mut self,
sorted_call_requests: [CallRequest; MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX],
sorted_call_requests_indexes: [u64; MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX]
) {
let accumulated_data = self.previous_kernel.public_inputs.end;
self.public_inputs.end.public_call_stack = array_to_bounded_vec(accumulated_data.public_call_stack);
assert_sorted_array(
accumulated_data.public_call_stack,
sorted_call_requests,
sorted_call_requests_indexes,
desc_sort_by_counters
);
self.public_inputs.end.public_call_stack = array_to_bounded_vec(sorted_call_requests);
}

fn propagate_public_teardown_call_request(&mut self) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ use dep::types::{
abis::{
private_kernel_data::PrivateKernelData,
kernel_circuit_public_inputs::PublicKernelCircuitPublicInputs, note_hash::ScopedNoteHash,
nullifier::ScopedNullifier, log_hash::{ScopedEncryptedLogHash, NoteLogHash, ScopedLogHash}
nullifier::ScopedNullifier, log_hash::{ScopedEncryptedLogHash, NoteLogHash, ScopedLogHash},
call_request::CallRequest
},
constants::{
MAX_NEW_NOTE_HASHES_PER_TX, MAX_NEW_NULLIFIERS_PER_TX, MAX_NOTE_HASH_READ_REQUESTS_PER_TX,
MAX_KEY_VALIDATION_REQUESTS_PER_TX, MAX_ENCRYPTED_LOGS_PER_TX, MAX_UNENCRYPTED_LOGS_PER_TX,
MAX_NOTE_ENCRYPTED_LOGS_PER_TX
MAX_NOTE_ENCRYPTED_LOGS_PER_TX, MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX
},
grumpkin_private_key::GrumpkinPrivateKey, utils::arrays::array_length, traits::is_empty
};
Expand All @@ -24,6 +25,8 @@ struct PrivateKernelTailToPublicHints {
sorted_encrypted_log_hashes_indexes: [u64; MAX_ENCRYPTED_LOGS_PER_TX],
sorted_unencrypted_log_hashes: [ScopedLogHash; MAX_UNENCRYPTED_LOGS_PER_TX],
sorted_unencrypted_log_hashes_indexes: [u64; MAX_UNENCRYPTED_LOGS_PER_TX],
sorted_call_requests: [CallRequest; MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX],
sorted_call_requests_indexes: [u64; MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX],
}

struct PrivateKernelTailToPublicCircuitPrivateInputs {
Expand Down Expand Up @@ -55,7 +58,10 @@ impl PrivateKernelTailToPublicCircuitPrivateInputs {
self.hints.sorted_encrypted_log_hashes_indexes,
self.hints.sorted_unencrypted_log_hashes,
self.hints.sorted_unencrypted_log_hashes_indexes
).compose_public().finish_to_public()
).compose_public(
self.hints.sorted_call_requests,
self.hints.sorted_call_requests_indexes
).finish_to_public()
}
}

Expand All @@ -76,6 +82,7 @@ mod tests {
};
use dep::types::{
abis::{
call_request::CallRequest, side_effect::Ordered,
kernel_circuit_public_inputs::PublicKernelCircuitPublicInputs, gas::Gas,
note_hash::{NoteHash, ScopedNoteHash}, nullifier::{Nullifier, ScopedNullifier},
log_hash::{LogHash, ScopedEncryptedLogHash, NoteLogHash, ScopedLogHash}
Expand Down Expand Up @@ -178,6 +185,13 @@ mod tests {
let sorted_unencrypted_log_hashes = sorted.sorted_array;
let sorted_unencrypted_log_hashes_indexes = sorted.sorted_index_hints;

let sorted = sort_get_sorted_hints(
self.previous_kernel.public_call_stack.storage,
|a: CallRequest, b: CallRequest| a.counter() > b.counter()
);
let sorted_call_requests = sorted.sorted_array;
let sorted_call_requests_indexes = sorted.sorted_index_hints;

let hints = PrivateKernelTailToPublicHints {
sorted_new_note_hashes,
sorted_new_note_hashes_indexes,
Expand All @@ -188,7 +202,9 @@ mod tests {
sorted_encrypted_log_hashes,
sorted_encrypted_log_hashes_indexes,
sorted_unencrypted_log_hashes,
sorted_unencrypted_log_hashes_indexes
sorted_unencrypted_log_hashes_indexes,
sorted_call_requests,
sorted_call_requests_indexes
};

let kernel = PrivateKernelTailToPublicCircuitPrivateInputs { previous_kernel: self.previous_kernel.to_private_kernel_data(), hints };
Expand Down
36 changes: 26 additions & 10 deletions yarn-project/circuit-types/src/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ import {
CallRequest,
GasSettings,
LogHash,
MAX_NEW_NULLIFIERS_PER_TX,
MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX,
Nullifier,
PartialPrivateTailPublicInputsForPublic,
PrivateKernelTailCircuitPublicInputs,
PublicAccumulatedDataBuilder,
PublicCallRequest,
computeContractClassId,
getContractClassFromArtifact,
Expand Down Expand Up @@ -69,20 +71,34 @@ export const mockTx = (
data.forRollup = undefined;
data.forPublic = PartialPrivateTailPublicInputsForPublic.empty();

data.forPublic.endNonRevertibleData.newNullifiers[0] = firstNullifier;

publicCallRequests = publicCallRequests.length
? publicCallRequests.slice().sort((a, b) => b.callContext.sideEffectCounter - a.callContext.sideEffectCounter)
: times(totalPublicCallRequests, i => makePublicCallRequest(seed + 0x100 + i));

data.forPublic.end.publicCallStack = makeTuple(MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX, i =>
i < numberOfRevertiblePublicCallRequests ? publicCallRequests[i].toCallRequest() : CallRequest.empty(),
);
data.forPublic.endNonRevertibleData.publicCallStack = makeTuple(MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX, i =>
i < numberOfNonRevertiblePublicCallRequests
? publicCallRequests[numberOfRevertiblePublicCallRequests + i].toCallRequest()
: CallRequest.empty(),
);
const revertibleBuilder = new PublicAccumulatedDataBuilder();
const nonRevertibleBuilder = new PublicAccumulatedDataBuilder();

const nonRevertibleNullifiers = makeTuple(MAX_NEW_NULLIFIERS_PER_TX, Nullifier.empty);
nonRevertibleNullifiers[0] = firstNullifier;

data.forPublic.endNonRevertibleData = nonRevertibleBuilder
.withNewNullifiers(nonRevertibleNullifiers)
.withPublicCallStack(
makeTuple(MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX, i =>
i < numberOfNonRevertiblePublicCallRequests
? publicCallRequests[numberOfRevertiblePublicCallRequests + i].toCallRequest()
: CallRequest.empty(),
),
)
.build();

data.forPublic.end = revertibleBuilder
.withPublicCallStack(
makeTuple(MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX, i =>
i < numberOfRevertiblePublicCallRequests ? publicCallRequests[i].toCallRequest() : CallRequest.empty(),
),
)
.build();

data.forPublic.publicTeardownCallStack = makeTuple(MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX, () => CallRequest.empty());
data.forPublic.publicTeardownCallStack[0] = publicTeardownCallRequest.isEmpty()
Expand Down
4 changes: 4 additions & 0 deletions yarn-project/circuits.js/src/structs/call_request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ export class CallRequest {
);
}

get counter() {
return this.startSideEffectCounter.toNumber();
}

/**
* Deserializes from a buffer or reader.
* @param buffer - Buffer or reader to read from.
Expand Down
19 changes: 10 additions & 9 deletions yarn-project/circuits.js/src/structs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,41 +9,41 @@ export * from './context/public_context_inputs.js';
export * from './contract_storage_read.js';
export * from './contract_storage_update_request.js';
export * from './function_data.js';
export * from './gas.js';
export * from './gas_fees.js';
export * from './gas_settings.js';
export * from './gas.js';
export * from './global_variables.js';
export * from './header.js';
export * from './kernel/combined_accumulated_data.js';
export * from './kernel/private_accumulated_data.js';
export * from './kernel/public_accumulated_data.js';
export * from './kernel/combined_constant_data.js';
export * from './kernel/kernel_circuit_public_inputs.js';
export * from './kernel/kernel_data.js';
export * from './kernel/private_accumulated_data.js';
export * from './kernel/private_call_data.js';
export * from './kernel/private_kernel_init_circuit_private_inputs.js';
export * from './kernel/private_kernel_circuit_public_inputs.js';
export * from './kernel/private_kernel_data.js';
export * from './kernel/private_kernel_init_circuit_private_inputs.js';
export * from './kernel/private_kernel_inner_circuit_private_inputs.js';
export * from './kernel/private_kernel_reset_circuit_private_inputs.js';
export * from './kernel/private_kernel_reset_circuit_private_inputs_variants.js';
export * from './kernel/private_kernel_tail_circuit_private_inputs.js';
export * from './kernel/private_kernel_tail_circuit_public_inputs.js';
export * from './kernel/public_accumulated_data.js';
export * from './kernel/public_accumulated_data_builder.js';
export * from './kernel/public_call_data.js';
export * from './kernel/public_kernel_circuit_private_inputs.js';
export * from './kernel/public_kernel_circuit_public_inputs.js';
export * from './kernel/public_kernel_data.js';
export * from './kernel/public_kernel_tail_circuit_private_inputs.js';
export * from './kernel/kernel_circuit_public_inputs.js';
export * from './kernel/kernel_data.js';
export * from './key_validation_request.js';
export * from './key_validation_request_and_generator.js';
export * from './l2_to_l1_message.js';
export * from './log_hash.js';
export * from './max_block_number.js';
export * from './membership_witness.js';
export * from './non_existent_read_request_hints.js';
export * from './note_hash.js';
export * from './nullifier.js';
export * from './key_validation_request.js';
export * from './key_validation_request_and_generator.js';
export * from './scoped_key_validation_request_and_generator.js';
export * from './parity/base_parity_inputs.js';
export * from './parity/parity_public_inputs.js';
export * from './parity/root_parity_input.js';
Expand Down Expand Up @@ -72,6 +72,7 @@ export * from './rollup/previous_rollup_data.js';
export * from './rollup/root_rollup.js';
export * from './rollup/state_diff_hints.js';
export * from './rollup_validation_requests.js';
export * from './scoped_key_validation_request_and_generator.js';
export * from './shared.js';
export * from './side_effects.js';
export * from './state_reference.js';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import {
MAX_NEW_NOTE_HASHES_PER_TX,
MAX_NEW_NULLIFIERS_PER_TX,
MAX_NOTE_ENCRYPTED_LOGS_PER_TX,
MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX,
MAX_UNENCRYPTED_LOGS_PER_TX,
} from '../../constants.gen.js';
import { countAccumulatedItems } from '../../utils/index.js';
import { CallRequest } from '../call_request.js';
import { NoteLogHash, ScopedEncryptedLogHash, ScopedLogHash } from '../log_hash.js';
import { ScopedNoteHash } from '../note_hash.js';
import { ScopedNullifier } from '../nullifier.js';
Expand Down Expand Up @@ -55,6 +57,14 @@ export class PrivateKernelTailHints {
* The sorted encrypted log hashes indexes. Maps original to sorted.
*/
public sortedUnencryptedLogHashesIndexes: Tuple<number, typeof MAX_UNENCRYPTED_LOGS_PER_TX>,
/**
* The sorted public call requests.
*/
public sortedCallRequests: Tuple<CallRequest, typeof MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX>,
/**
* The sorted public call requests indexes. Maps original to sorted.
*/
public sortedCallRequestsIndexes: Tuple<number, typeof MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX>,
) {}

toBuffer() {
Expand All @@ -69,6 +79,8 @@ export class PrivateKernelTailHints {
this.sortedEncryptedLogHashesIndexes,
this.sortedUnencryptedLogHashes,
this.sortedUnencryptedLogHashesIndexes,
this.sortedCallRequests,
this.sortedCallRequestsIndexes,
);
}

Expand All @@ -90,6 +102,8 @@ export class PrivateKernelTailHints {
reader.readNumbers(MAX_ENCRYPTED_LOGS_PER_TX),
reader.readArray(MAX_UNENCRYPTED_LOGS_PER_TX, ScopedLogHash),
reader.readNumbers(MAX_UNENCRYPTED_LOGS_PER_TX),
reader.readArray(MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX, CallRequest),
reader.readNumbers(MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX),
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export class PublicAccumulatedData {
/**
* Current public call stack.
*/
public publicCallStack: Tuple<CallRequest, typeof MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX>,
public readonly publicCallStack: Tuple<CallRequest, typeof MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX>,

/** Gas used so far by the transaction. */
public gasUsed: Gas,
Expand Down
Loading

0 comments on commit f67d6f3

Please sign in to comment.