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: Validate globals in public kernel #6031

Merged
merged 2 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,9 @@ library Constants {
+ MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL + (SIDE_EFFECT_LENGTH * MAX_NEW_NOTE_HASHES_PER_CALL)
+ (SIDE_EFFECT_LINKED_TO_NOTE_HASH_LENGTH * MAX_NEW_NULLIFIERS_PER_CALL)
+ (L2_TO_L1_MESSAGE_LENGTH * MAX_NEW_L2_TO_L1_MSGS_PER_CALL) + 2
+ (SIDE_EFFECT_LENGTH * MAX_UNENCRYPTED_LOGS_PER_CALL) + 1 + HEADER_LENGTH + AZTEC_ADDRESS_LENGTH /* revert_code */
+ 1 + 2 * GAS_LENGTH /* transaction_fee */ + 1;
+ (SIDE_EFFECT_LENGTH * MAX_UNENCRYPTED_LOGS_PER_CALL) + 1 + HEADER_LENGTH
+ GLOBAL_VARIABLES_LENGTH + AZTEC_ADDRESS_LENGTH /* revert_code */ + 1 + 2 * GAS_LENGTH /* transaction_fee */
+ 1;
uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH =
AZTEC_ADDRESS_LENGTH + FUNCTION_DATA_LENGTH + PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH;
uint256 internal constant ENQUEUE_PUBLIC_FUNCTION_CALL_RETURN_LENGTH =
Expand Down
3 changes: 2 additions & 1 deletion noir-projects/aztec-nr/aztec/src/context/private_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
};
use dep::protocol_types::{
abis::{
gas::Gas, call_context::CallContext, function_data::FunctionData,
global_variables::GlobalVariables, gas::Gas, call_context::CallContext, function_data::FunctionData,
function_selector::FunctionSelector, max_block_number::MaxBlockNumber,
nullifier_key_validation_request::NullifierKeyValidationRequest,
private_call_stack_item::PrivateCallStackItem,
Expand Down Expand Up @@ -500,6 +500,7 @@ impl PrivateContext {
unencrypted_logs_hashes: [SideEffect::empty(); MAX_UNENCRYPTED_LOGS_PER_CALL],
unencrypted_log_preimages_length: 0,
historical_header: Header::empty(),
global_variables: GlobalVariables::empty(),
prover_address: AztecAddress::zero(),
revert_code: 0,
start_gas_left: Gas::empty(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ impl PublicContext {
unencrypted_logs_hashes: self.unencrypted_logs_hashes.storage,
unencrypted_log_preimages_length,
historical_header: self.inputs.historical_header,
global_variables: self.inputs.public_global_variables,
prover_address: self.prover_address,
revert_code: 0,
start_gas_left: self.inputs.gas_left,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ struct PrivateKernelInitCircuitPrivateInputs {

impl PrivateKernelInitCircuitPrivateInputs {
fn initialize_end_values(self, public_inputs: &mut PrivateKernelCircuitPublicInputsBuilder) {
public_inputs.constants = CombinedConstantData {
historical_header: self.private_call.call_stack_item.public_inputs.historical_header,
tx_context: self.tx_request.tx_context,
};
public_inputs.constants = CombinedConstantData::private(
self.private_call.call_stack_item.public_inputs.historical_header,
self.tx_request.tx_context,
);
public_inputs.min_revertible_side_effect_counter = self.private_call.call_stack_item.public_inputs.min_revertible_side_effect_counter;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use dep::types::{
kernel_circuit_public_inputs::PublicKernelCircuitPublicInputsBuilder, kernel_data::PublicKernelData,
public_call_data::PublicCallData, public_data_read::PublicDataRead,
public_data_update_request::PublicDataUpdateRequest, read_request::ReadRequestContext,
side_effect::{SideEffect, SideEffectLinkedToNoteHash}
side_effect::{SideEffect, SideEffectLinkedToNoteHash}, global_variables::GlobalVariables,
combined_constant_data::CombinedConstantData
},
address::AztecAddress,
contrakt::{storage_read::StorageRead, storage_update_request::StorageUpdateRequest},
Expand Down Expand Up @@ -33,6 +34,13 @@ pub fn validate_inputs(public_call: PublicCallData) {
assert(public_call.bytecode_hash != 0, "Bytecode hash cannot be zero");
}

pub fn validate_public_call_constants(public_call: PublicCallData, constants: CombinedConstantData) {
let public_call_globals = public_call.call_stack_item.public_inputs.global_variables;
assert(
public_call_globals == constants.global_variables, "Global variables injected into the public call do not match constants"
);
}

pub fn validate_public_call_non_revert(public_call: PublicCallData) {
assert(public_call.call_stack_item.public_inputs.revert_code == 0, "Public call cannot be reverted");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ impl PublicKernelAppLogicCircuitPrivateInputs {
// validate the inputs common to all invocation circumstances
common::validate_inputs(self.public_call);

// validate constants injected into the public call are correct
common::validate_public_call_constants(
self.public_call,
self.previous_kernel.public_inputs.constants
);

// validate the inputs unique to having a previous public kernel
self.validate_inputs();

Expand Down Expand Up @@ -485,4 +491,13 @@ mod tests {

builder.failed();
}

#[test(should_fail_with="Global variables injected into the public call do not match constants")]
fn validates_global_varialbles() {
let mut builder = PublicKernelAppLogicCircuitPrivateInputsBuilder::new();

builder.public_call.public_inputs.global_variables.block_number = 11;

builder.failed();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ struct PublicKernelSetupCircuitPrivateInputs {
// Note: One might think that our previous_kernel ought to be
// a PrivateKernelTailData. However, we instead supply a PublicKernelData.
// This is because PrivateKernelTailData is a subset of PublicKernelData.
// And we just initialize the missing values to zero in TS before passing it to the circuit.
// And we just initialize the missing values to zero in TS before passing it to the circuit,
// except for the constants.global_variables which we populate with the current block values.
// This is a bit of a hack, but it allows us to reuse the setup circuit until
// the setup phase of the public kernel is complete. Maybe in a perfect world we would
// have a SetupInit, SetupInner, etc, but this will change anyway once the public VM is able to
Expand Down Expand Up @@ -36,6 +37,15 @@ impl PublicKernelSetupCircuitPrivateInputs {
// validate the inputs common to all invocation circumstances
common::validate_inputs(self.public_call);

// validate constants injected into the public call are correct
// note that the previous_kernel.public_inputs.constants as returned from the private kernel tail
// will be empty, we're having the sequencer manually patch them in ts-land as explained on the
// comment on top of this file
common::validate_public_call_constants(
self.public_call,
self.previous_kernel.public_inputs.constants
);

// validate the inputs unique to having a previous private kernel
self.validate_inputs();

Expand Down Expand Up @@ -540,4 +550,13 @@ mod tests {

builder.failed();
}

#[test(should_fail_with="Global variables injected into the public call do not match constants")]
fn validates_global_varialbles() {
let mut builder = PublicKernelSetupCircuitPrivateInputsBuilder::new();

builder.public_call.public_inputs.global_variables.block_number = 11;

builder.failed();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ impl PublicKernelTeardownCircuitPrivateInputs {
let transaction_fee = self.public_call.call_stack_item.public_inputs.transaction_fee;
// Note that teardown_gas is already included in end.gas_used as it was injected by the private kernel
let total_gas_used = self.previous_kernel.public_inputs.end.gas_used.add(self.previous_kernel.public_inputs.end_non_revertible.gas_used);
// TODO(palla/gas): Load gas fees from a PublicConstantData struct that's currently missing
let block_gas_fees = GasFees::default();
let block_gas_fees = self.previous_kernel.public_inputs.constants.global_variables.gas_fees;
let inclusion_fee = self.previous_kernel.public_inputs.constants.tx_context.gas_settings.inclusion_fee;
let computed_transaction_fee = total_gas_used.compute_fee(block_gas_fees) + inclusion_fee;

Expand Down Expand Up @@ -74,6 +73,12 @@ impl PublicKernelTeardownCircuitPrivateInputs {
// validate the inputs common to all invocation circumstances
common::validate_inputs(self.public_call);

// validate constants injected into the public call are correct
common::validate_public_call_constants(
self.public_call,
self.previous_kernel.public_inputs.constants
);

// validate the inputs unique to having a previous private kernel
self.validate_inputs();

Expand Down Expand Up @@ -425,4 +430,13 @@ mod tests {

builder.failed();
}

#[test(should_fail_with="Global variables injected into the public call do not match constants")]
fn validates_global_varialbles() {
let mut builder = PublicKernelTeardownCircuitPrivateInputsBuilder::new();

builder.public_call.public_inputs.global_variables.block_number = 11;

builder.failed();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@ impl BaseRollupInputs {
== self.constants.global_variables.version, "kernel version does not match the rollup version"
);

// Verify the kernel global variables if set, note these can be empty if this is a request coming directly from the private kernel tail.
// TODO(@spalladino) How can we check that this is a request coming from the private kernel tail?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if I can update this when I implement recursion from the kernels to the base rollup. We will know which verification key has been provided.

assert(
self.kernel_data.public_inputs.constants.global_variables.is_empty()
| (self.kernel_data.public_inputs.constants.global_variables
== self.constants.global_variables), "kernel global variables do not match the rollup global variables"
);

self.validate_kernel_start_state();

let rollup_validation_requests = self.kernel_data.public_inputs.rollup_validation_requests;
Expand Down Expand Up @@ -983,6 +991,14 @@ mod tests {
builder.fails();
}

#[test(should_fail_with = "kernel global variables do not match the rollup global variables")]
unconstrained fn constants_global_variables_dont_match_kernels() {
let mut builder = BaseRollupInputsBuilder::new();
builder.kernel_data.global_variables.block_number = 6;
builder.constants.global_variables.block_number = 7;
builder.fails();
}

#[test(should_fail_with = "kernel max_block_number is smaller than block number")]
unconstrained fn constants_dont_satisfy_smaller_max_block_number() {
let mut builder = BaseRollupInputsBuilder::new();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::transaction::tx_context::TxContext;
use crate::header::Header;
use crate::traits::Empty;
use crate::abis::global_variables::GlobalVariables;

struct CombinedConstantData {
historical_header: Header,
Expand All @@ -9,13 +10,22 @@ struct CombinedConstantData {
// a situation we could be using header from a block before the upgrade took place but be using the updated
// protocol to execute and prove the transaction.
tx_context: TxContext,

global_variables: GlobalVariables,
}

impl CombinedConstantData {
pub fn private(historical_header: Header, tx_context: TxContext) -> CombinedConstantData {
CombinedConstantData { historical_header, tx_context, global_variables: GlobalVariables::empty() }
}
}

impl Empty for CombinedConstantData {
fn empty() -> Self {
CombinedConstantData {
historical_header: Header::empty(),
tx_context: TxContext::empty(),
global_variables: GlobalVariables::empty()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ impl GasFees {
pub fn default() -> Self {
GasFees::new(1, 1, 1)
}

pub fn is_empty(self) -> bool {
(self.fee_per_da_gas == 0) & (self.fee_per_l1_gas == 0) & (self.fee_per_l2_gas == 0)
}
}

impl Serialize<GAS_FEES_LENGTH> for GasFees {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@ struct GlobalVariables {
}
// docs:end:global-variables

impl GlobalVariables {
fn is_empty(self) -> bool {
(self.chain_id == 0)
& (self.version == 0)
& (self.block_number == 0)
& (self.timestamp == 0)
& (self.coinbase.is_zero())
& (self.fee_recipient.is_zero())
& (self.gas_fees.is_empty())
}
}

impl Serialize<GLOBAL_VARIABLES_LENGTH> for GlobalVariables {
fn serialize(self) -> [Field; GLOBAL_VARIABLES_LENGTH] {
let mut serialized: BoundedVec<Field, GLOBAL_VARIABLES_LENGTH> = BoundedVec::new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ mod tests {
let call_stack_item = PublicCallStackItem { contract_address, public_inputs, is_execution_request: true, function_data };

// Value from public_call_stack_item.test.ts "Computes a callstack item request hash" test
let test_data_call_stack_item_request_hash = 0x1b06f4a4960455e9f01c20d4cb01afbf8c8f39eb50094c5d1ad6725ced0f7d08;
let test_data_call_stack_item_request_hash = 0x22848497ff97ff3a4517aec32454059030fb5a3ef4f3ca533ee40132d7a63aea;
assert_eq(call_stack_item.hash(), test_data_call_stack_item_request_hash);
}

Expand All @@ -87,7 +87,7 @@ mod tests {
let call_stack_item = PublicCallStackItem { contract_address, public_inputs, is_execution_request: false, function_data };

// Value from public_call_stack_item.test.ts "Computes a callstack item hash" test
let test_data_call_stack_item_hash = 0x1f3f1902ca41ffd6fd7191fa5a52edd677444a9b6ae8f4448336fa71a4b2d5cc;
let test_data_call_stack_item_hash = 0x0e18ddd9aaddae02d45598f0278d925e289913384d6e15057ce5b4a9e8e7488d;
assert_eq(call_stack_item.hash(), test_data_call_stack_item_hash);
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
abis::{
call_context::CallContext, read_request::ReadRequest,
side_effect::{SideEffect, SideEffectLinkedToNoteHash}, gas::Gas
side_effect::{SideEffect, SideEffectLinkedToNoteHash}, gas::Gas, global_variables::GlobalVariables
},
address::AztecAddress,
constants::{
Expand Down Expand Up @@ -46,6 +46,9 @@ struct PublicCircuitPublicInputs {
// previous to the one in which the tx is included.
historical_header: Header,

// Global variables injected into this circuit
global_variables: GlobalVariables,

prover_address: AztecAddress,

revert_code: u8,
Expand Down Expand Up @@ -99,6 +102,7 @@ impl Serialize<PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH> for PublicCircuitPublicInput
}
fields.push(self.unencrypted_log_preimages_length);
fields.extend_from_array(self.historical_header.serialize());
fields.extend_from_array(self.global_variables.serialize());
fields.push(self.prover_address.to_field());
fields.push(self.revert_code as Field);
fields.extend_from_array(self.start_gas_left.serialize());
Expand Down Expand Up @@ -129,6 +133,7 @@ impl Deserialize<PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH> for PublicCircuitPublicInp
unencrypted_logs_hashes: reader.read_struct_array(SideEffect::deserialize, [SideEffect::empty(); MAX_UNENCRYPTED_LOGS_PER_CALL]),
unencrypted_log_preimages_length: reader.read(),
historical_header: reader.read_struct(Header::deserialize),
global_variables: reader.read_struct(GlobalVariables::deserialize),
prover_address: reader.read_struct(AztecAddress::deserialize),
revert_code: reader.read() as u8,
start_gas_left: reader.read_struct(Gas::deserialize),
Expand Down Expand Up @@ -166,6 +171,7 @@ impl Empty for PublicCircuitPublicInputs {
unencrypted_logs_hashes: [SideEffect::empty(); MAX_UNENCRYPTED_LOGS_PER_CALL],
unencrypted_log_preimages_length: 0,
historical_header: Header::empty(),
global_variables: GlobalVariables::empty(),
prover_address: AztecAddress::zero(),
revert_code: 0 as u8,
start_gas_left: Gas::empty(),
Expand All @@ -189,6 +195,6 @@ fn empty_hash() {
let hash = inputs.hash();

// Value from public_circuit_public_inputs.test.ts "computes empty item hash" test
let test_data_empty_hash = 0x237c89f8b29c3fb169b889940a714b3c72017cb2941d0724d4668a030794d2fb;
let test_data_empty_hash = 0x2d91debc43bd6354caef4fd152975e7c6dd44e8623b6b62c21b9f547f2fabd32;
assert_eq(hash, test_data_empty_hash);
}
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ global TX_CONTEXT_LENGTH: u64 = 2 + GAS_SETTINGS_LENGTH;
global TX_REQUEST_LENGTH: u64 = 2 + TX_CONTEXT_LENGTH + FUNCTION_DATA_LENGTH;
global HEADER_LENGTH: u64 = APPEND_ONLY_TREE_SNAPSHOT_LENGTH + CONTENT_COMMITMENT_LENGTH + STATE_REFERENCE_LENGTH + GLOBAL_VARIABLES_LENGTH;
global PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH: u64 = CALL_CONTEXT_LENGTH + 3 + MAX_BLOCK_NUMBER_LENGTH + (SIDE_EFFECT_LENGTH * MAX_NOTE_HASH_READ_REQUESTS_PER_CALL) + (READ_REQUEST_LENGTH * MAX_NULLIFIER_READ_REQUESTS_PER_CALL) + (NULLIFIER_KEY_VALIDATION_REQUEST_LENGTH * MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_CALL) + (SIDE_EFFECT_LENGTH * MAX_NEW_NOTE_HASHES_PER_CALL) + (SIDE_EFFECT_LINKED_TO_NOTE_HASH_LENGTH * MAX_NEW_NULLIFIERS_PER_CALL) + MAX_PRIVATE_CALL_STACK_LENGTH_PER_CALL + MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL + (L2_TO_L1_MESSAGE_LENGTH * MAX_NEW_L2_TO_L1_MSGS_PER_CALL) + 2 + (SIDE_EFFECT_LENGTH * MAX_ENCRYPTED_LOGS_PER_CALL) + (SIDE_EFFECT_LENGTH * MAX_UNENCRYPTED_LOGS_PER_CALL) + 2 + HEADER_LENGTH + TX_CONTEXT_LENGTH;
global PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH: u64 = CALL_CONTEXT_LENGTH + 2 + (READ_REQUEST_LENGTH * MAX_NULLIFIER_READ_REQUESTS_PER_CALL) + (READ_REQUEST_LENGTH * MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL) + (CONTRACT_STORAGE_UPDATE_REQUEST_LENGTH * MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL) + (CONTRACT_STORAGE_READ_LENGTH * MAX_PUBLIC_DATA_READS_PER_CALL) + MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL + (SIDE_EFFECT_LENGTH * MAX_NEW_NOTE_HASHES_PER_CALL) + (SIDE_EFFECT_LINKED_TO_NOTE_HASH_LENGTH * MAX_NEW_NULLIFIERS_PER_CALL) + (L2_TO_L1_MESSAGE_LENGTH * MAX_NEW_L2_TO_L1_MSGS_PER_CALL) + 2 + (SIDE_EFFECT_LENGTH * MAX_UNENCRYPTED_LOGS_PER_CALL) + 1 + HEADER_LENGTH + AZTEC_ADDRESS_LENGTH + /* revert_code */ 1 + 2 * GAS_LENGTH + /* transaction_fee */ 1;
global PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH: u64 = CALL_CONTEXT_LENGTH + 2 + (READ_REQUEST_LENGTH * MAX_NULLIFIER_READ_REQUESTS_PER_CALL) + (READ_REQUEST_LENGTH * MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL) + (CONTRACT_STORAGE_UPDATE_REQUEST_LENGTH * MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL) + (CONTRACT_STORAGE_READ_LENGTH * MAX_PUBLIC_DATA_READS_PER_CALL) + MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL + (SIDE_EFFECT_LENGTH * MAX_NEW_NOTE_HASHES_PER_CALL) + (SIDE_EFFECT_LINKED_TO_NOTE_HASH_LENGTH * MAX_NEW_NULLIFIERS_PER_CALL) + (L2_TO_L1_MESSAGE_LENGTH * MAX_NEW_L2_TO_L1_MSGS_PER_CALL) + 2 + (SIDE_EFFECT_LENGTH * MAX_UNENCRYPTED_LOGS_PER_CALL) + 1 + HEADER_LENGTH + GLOBAL_VARIABLES_LENGTH + AZTEC_ADDRESS_LENGTH + /* revert_code */ 1 + 2 * GAS_LENGTH + /* transaction_fee */ 1;
global PRIVATE_CALL_STACK_ITEM_LENGTH: u64 = AZTEC_ADDRESS_LENGTH + FUNCTION_DATA_LENGTH + PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH;

global ENQUEUE_PUBLIC_FUNCTION_CALL_RETURN_LENGTH: u64 = 2 + FUNCTION_DATA_LENGTH + CALL_CONTEXT_LENGTH;
Expand Down
Loading
Loading