Skip to content

Commit

Permalink
feat: indexed protocol contracts tree (#11897)
Browse files Browse the repository at this point in the history
Closes #11843

This PR converts the protocol contract address tree to an indexed tree.
This is so:
- If a user is calling a protocol contract address, its calculated
address must be in the tree
- If a user is NOT calling a protocol contract address, its calculated
address must NOT be in the tree

This required more changes than expected because the old code had this
statement:
```rust
    let computed_protocol_contract_tree_root = if (MAX_PROTOCOL_CONTRACTS as Field).lt(
        protocol_contract_index,
    ) {
        0
    } else {
        root_from_sibling_path(
            computed_address.to_field(),
            protocol_contract_index,
            private_call_data.protocol_contract_sibling_path,
        )
    };
    assert(
        computed_address.eq(contract_address)
            | computed_protocol_contract_tree_root.eq(protocol_contract_tree_root),
        "computed contract address does not match expected one",
    );
```
For many tests in and out of nr, we input `protocol_contract_tree_root =
0`. That meant that even if we weren't calling a protocol contract
address, an incorrect `contract_address` would pass the assert
statement. The current code does not allow this, so I had to update some
old fixtures and provide a real root for all calls (not just protocol
contract ones).
  • Loading branch information
MirandaWood authored Feb 13, 2025
1 parent 921d2cd commit 96e84d4
Show file tree
Hide file tree
Showing 32 changed files with 1,306 additions and 578 deletions.
32 changes: 30 additions & 2 deletions docs/docs/migration_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,35 @@ keywords: [sandbox, aztec, notes, migration, updating, upgrading]

Aztec is in full-speed development. Literally every version breaks compatibility with the previous ones. This page attempts to target errors and difficulties you might encounter when upgrading, and how to resolve them.

### TBD
## TBD

### The tree of protocol contract addresses is now an indexed tree

This is to allow for non-membership proofs for non-protocol contract addresses. As before, the canonical protocol contract addresses point to the index of the leaf of the 'real' computed protocol address.

For example, the canonical `DEPLOYER_CONTRACT_ADDRESS` is a constant `= 2`. This is used in the kernels as the `contract_address`. We calculate the `computed_address` (currently `0x1665c5fbc1e58ba19c82f64c0402d29e8bbf94b1fde1a056280d081c15b0dac1`) and check that this value exists in the indexed tree at index `2`. This check already existed and ensures that the call cannot do 'special' protocol contract things unless it is a real protocol contract.

The new check an indexed tree allows is non-membership of addresses of non protocol contracts. This ensures that if a call is from a protocol contract, it must use the canonical address. For example, before this check a call could be from the deployer contract and use `0x1665c5fbc1e58ba19c82f64c0402d29e8bbf94b1fde1a056280d081c15b0dac1` as the `contract_address`, but be incorrectly treated as a 'normal' call.

```diff
- let computed_protocol_contract_tree_root = if is_protocol_contract {
- 0
- } else {
- root_from_sibling_path(
- computed_address.to_field(),
- protocol_contract_index,
- private_call_data.protocol_contract_sibling_path,
- )
- };

+ conditionally_assert_check_membership(
+ computed_address.to_field(),
+ is_protocol_contract,
+ private_call_data.protocol_contract_leaf,
+ private_call_data.protocol_contract_membership_witness,
+ protocol_contract_tree_root,
+ );
```

### [Aztec.nr] Changes to `NoteInterface`
We are in a process of discontinuing `NoteHeader` from notes.
Expand Down Expand Up @@ -61,7 +89,7 @@ impl NullifiableNote for EcdsaPublicKeyNote {
}
```

### 0.75.0
## 0.75.0

### Changes to `TokenBridge` interface

Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use dep::types::{
abis::private_kernel::private_call_data::PrivateCallData, address::AztecAddress,
constants::MAX_PROTOCOL_CONTRACTS, merkle_tree::root::root_from_sibling_path, traits::ToField,
constants::MAX_PROTOCOL_CONTRACTS, merkle_tree::conditionally_assert_check_membership,
traits::ToField,
};

pub fn validate_contract_address(
Expand All @@ -22,23 +23,30 @@ pub fn validate_contract_address(
private_call_data.public_keys,
);

let protocol_contract_index = contract_address.to_field();

let computed_protocol_contract_tree_root = if (MAX_PROTOCOL_CONTRACTS as Field).lt(
protocol_contract_index,
) {
0
} else {
root_from_sibling_path(
computed_address.to_field(),
protocol_contract_index,
private_call_data.protocol_contract_sibling_path,
)
};
let contract_address_field = contract_address.to_field();
let is_protocol_contract = contract_address_field.lt(MAX_PROTOCOL_CONTRACTS as Field);

// We either have a normal contract address, which must match the calculated address, or
// A computed protocol contract address which exists at the index held in call_context.contract_address.
assert(
computed_address.eq(contract_address)
| computed_protocol_contract_tree_root.eq(protocol_contract_tree_root),
(!is_protocol_contract & computed_address.eq(contract_address))
| (
is_protocol_contract
& private_call_data.protocol_contract_membership_witness.leaf_index.eq(
contract_address_field,
)
),
"computed contract address does not match expected one",
);

// A non-protocol computed contract address is checked for non-membership below using protocol_contract_leaf as a low leaf.
// A protocol contract address is checked for membership below where protocol_contract_leaf contains the
// computed_address at the index given by contract_address.
conditionally_assert_check_membership(
computed_address.to_field(),
is_protocol_contract,
private_call_data.protocol_contract_leaf,
private_call_data.protocol_contract_membership_witness,
protocol_contract_tree_root,
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ mod tests {
tx_request: self.tx_request,
private_call,
vk_tree_root: FixtureBuilder::vk_tree_root(),
protocol_contract_tree_root: 0,
protocol_contract_tree_root: self.private_call.protocol_contract_tree_root,
is_private_only: false,
first_nullifier_hint: 0,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,10 @@ fn validate_call_is_static_creating_contract_class_logs_hashes_fails() {

#[test(should_fail_with = "only the class registerer may emit contract class logs")]
fn validate_call_is_from_class_registerer_fails() {
// the default bulder address != REGISTERER_CONTRACT_ADDRESS
let mut builder = PrivateCallDataValidatorBuilder::new();

builder.private_call.add_contract_class_log_hash(1, 2);
// set the contract address to be some msg sender (!= REGISTERER_CONTRACT_ADDRESS)
builder.private_call.contract_address = builder.private_call.msg_sender;

builder.validate();
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,16 @@ fn validate_contract_address_protocol_contract_succeeds() {
builder.validate();
}

#[test(should_fail)]
fn validate_contract_address_protocol_contract_computed_address_fails() {
let mut builder = PrivateCallDataValidatorBuilder::new_with_protocol_contract();
// Swap the special address (0x01) with the computed address
builder.private_call.contract_address =
AztecAddress { inner: builder.private_call.protocol_contract_leaf.address };
// Validate may fail with either one of the low leaf membership errors
builder.validate();
}

#[test(should_fail_with = "computed contract address does not match expected one")]
fn validate_contract_address_protocol_contract_wrong_index_fails() {
let mut builder = PrivateCallDataValidatorBuilder::new_with_protocol_contract();
Expand All @@ -94,7 +104,7 @@ fn validate_contract_address_protocol_contract_wrong_index_fails() {
builder.validate();
}

#[test(should_fail_with = "computed contract address does not match expected one")]
#[test(should_fail_with = "Key does not match the key of the leaf preimage")]
fn validate_contract_address_protocol_contract_wrong_computed_address_fails() {
let mut builder = PrivateCallDataValidatorBuilder::new_with_protocol_contract();

Expand All @@ -103,6 +113,24 @@ fn validate_contract_address_protocol_contract_wrong_computed_address_fails() {
builder.validate();
}

#[test(should_fail_with = "Key does not match the key of the leaf preimage")]
fn validate_contract_address_protocol_address_wrong_low_leaf_key() {
let mut builder = PrivateCallDataValidatorBuilder::new_with_protocol_contract();

builder.private_call.protocol_contract_leaf.address += 1;

builder.validate();
}

#[test(should_fail_with = "membership check failed")]
fn validate_contract_address_protocol_address_wrong_low_leaf_next_key() {
let mut builder = PrivateCallDataValidatorBuilder::new_with_protocol_contract();

builder.private_call.protocol_contract_leaf.next_address += 1;

builder.validate();
}

#[test(should_fail_with = "Invalid VK hash")]
fn validate_contract_address_wrong_vk_hash_fails() {
let mut builder = PrivateCallDataValidatorBuilder::new_with_regular_contract();
Expand All @@ -124,3 +152,12 @@ fn validate_contract_address_wrong_vk_fails() {

builder.validate();
}

#[test(should_fail_with = "membership check failed")]
fn validate_contract_address_regular_address_wrong_low_leaf() {
let mut builder = PrivateCallDataValidatorBuilder::new_with_regular_contract();

builder.private_call.protocol_contract_leaf.address += 1;

builder.validate();
}
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,8 @@ mod tests {
inputs.constants.global_variables.chain_id = fixtures::CHAIN_ID;
inputs.constants.global_variables.version = fixtures::VERSION;
inputs.constants.vk_tree_root = inputs.tube_data.vk_tree_root;
inputs.constants.protocol_contract_tree_root =
inputs.tube_data.protocol_contract_tree_root;

inputs.pre_existing_blocks[0] = inputs.tube_data.historical_header.hash();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ mod tests {
inputs.constants.global_variables.version = fixtures::VERSION;
inputs.avm_data.global_variables = inputs.constants.global_variables;
inputs.constants.vk_tree_root = inputs.tube_data.vk_tree_root;
inputs.constants.protocol_contract_tree_root =
inputs.tube_data.protocol_contract_tree_root;

inputs.pre_existing_blocks[0] = inputs.tube_data.historical_header.hash();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub mod function_data;
pub mod global_variables;
pub mod note_hash_leaf_preimage;
pub mod nullifier_leaf_preimage;
pub mod protocol_contract_leaf_preimage;

pub mod tx_constant_data;
pub mod combined_constant_data;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use crate::{
abis::private_circuit_public_inputs::PrivateCircuitPublicInputs,
abis::{
private_circuit_public_inputs::PrivateCircuitPublicInputs,
protocol_contract_leaf_preimage::ProtocolContractLeafPreimage,
},
address::SaltedInitializationHash,
constants::{
FUNCTION_TREE_HEIGHT, PROOF_TYPE_OINK, PROOF_TYPE_PG, PROTOCOL_CONTRACT_TREE_HEIGHT,
Expand All @@ -19,8 +22,8 @@ pub struct PrivateCallData {
pub contract_class_artifact_hash: Field,
pub contract_class_public_bytecode_commitment: Field,
pub function_leaf_membership_witness: MembershipWitness<FUNCTION_TREE_HEIGHT>,
pub protocol_contract_sibling_path: [Field; PROTOCOL_CONTRACT_TREE_HEIGHT],

pub protocol_contract_membership_witness: MembershipWitness<PROTOCOL_CONTRACT_TREE_HEIGHT>,
pub protocol_contract_leaf: ProtocolContractLeafPreimage,
pub acir_hash: Field,
}

Expand All @@ -44,8 +47,8 @@ pub struct PrivateCallDataWithoutPublicInputs {
pub contract_class_artifact_hash: Field,
pub contract_class_public_bytecode_commitment: Field,
pub function_leaf_membership_witness: MembershipWitness<FUNCTION_TREE_HEIGHT>,
pub protocol_contract_sibling_path: [Field; PROTOCOL_CONTRACT_TREE_HEIGHT],

pub protocol_contract_membership_witness: MembershipWitness<PROTOCOL_CONTRACT_TREE_HEIGHT>,
pub protocol_contract_leaf: ProtocolContractLeafPreimage,
pub acir_hash: Field,
}

Expand All @@ -63,7 +66,8 @@ impl PrivateCallDataWithoutPublicInputs {
contract_class_public_bytecode_commitment: self
.contract_class_public_bytecode_commitment,
function_leaf_membership_witness: self.function_leaf_membership_witness,
protocol_contract_sibling_path: self.protocol_contract_sibling_path,
protocol_contract_membership_witness: self.protocol_contract_membership_witness,
protocol_contract_leaf: self.protocol_contract_leaf,
acir_hash: self.acir_hash,
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
use crate::{
hash::poseidon2_hash,
merkle_tree::leaf_preimage::{IndexedTreeLeafPreimage, LeafPreimage},
traits::Empty,
};

/**
Exists to show:
- Membership of a computed protocol contract address in the tree, or
- Non-membership of a computed non-protocol contract address in the tree
in private_call_data_validator/validate_contract_address.nr.
The tree is created never updated within the kernels, so we don't need the functions which update leaves.
*/

pub struct ProtocolContractLeafPreimage {
pub address: Field,
pub next_address: Field,
}

impl Empty for ProtocolContractLeafPreimage {
fn empty() -> Self {
Self { address: 0, next_address: 0 }
}
}

impl LeafPreimage for ProtocolContractLeafPreimage {
fn get_key(self) -> Field {
self.address
}

fn as_leaf(self) -> Field {
poseidon2_hash([self.address, self.next_address])
}
}

impl IndexedTreeLeafPreimage<Field> for ProtocolContractLeafPreimage {
fn get_next_key(self) -> Field {
self.next_address
}

fn points_to_infinity(self) -> bool {
// Unimplemented, not required
false
}

fn update_pointers(self, _next_key: Field, _next_index: u32) -> Self {
assert(false, "Tried to update a static protocol contract index");
Self::empty()
}

fn update_value(self, _value: Field) -> Self {
assert(false, "Tried to update a static protocol contract address");
Self::empty()
}

fn build_insertion_leaf(_value: Field, _low_leaf: Self) -> Self {
assert(false, "Tried to update a static protocol contract address");
Self::empty()
}
}

impl Eq for ProtocolContractLeafPreimage {
fn eq(self, other: Self) -> bool {
(self.address == other.address) & (self.next_address == other.next_address)
}
}
Loading

0 comments on commit 96e84d4

Please sign in to comment.