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: implement canonical key registry 5609 and implement shared mutable getter from another contract 5689 #5723

Merged
merged 40 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
c149e2c
initial
sklppy88 Apr 11, 2024
7365b17
test
sklppy88 Apr 12, 2024
4549dd8
Merge branch 'master' into ek/feat/implement-key-registry
sklppy88 Apr 12, 2024
a1907b0
go
sklppy88 Apr 12, 2024
01bbd3b
sadf
sklppy88 Apr 12, 2024
f976ac0
adf
sklppy88 Apr 12, 2024
8545e23
Merge branch 'master' into ek/feat/implement-key-registry
sklppy88 Apr 12, 2024
f742400
test
sklppy88 Apr 12, 2024
6a86b9d
fix
sklppy88 Apr 12, 2024
b58da64
fix
sklppy88 Apr 12, 2024
32a24e8
fix ci
sklppy88 Apr 12, 2024
6032daa
addressing changes
sklppy88 Apr 15, 2024
600ac9a
Merge branch 'master' into ek/feat/implement-key-registry
sklppy88 Apr 15, 2024
cbda439
feat: add shared mutable access from external contract #5689 (#5758)
sklppy88 Apr 15, 2024
ae19d83
Merge branch 'master' into ek/feat/implement-key-registry
sklppy88 Apr 15, 2024
c75dc4d
Merge branch 'master' into ek/feat/implement-key-registry
sklppy88 Apr 15, 2024
8270e04
fix
sklppy88 Apr 15, 2024
a84c9bd
fix
sklppy88 Apr 15, 2024
ed2466d
fix all
sklppy88 Apr 16, 2024
ac9adc3
Update cli_docs_sandbox.test.ts
sklppy88 Apr 16, 2024
9d3c831
Update cli_docs_sandbox.test.ts
sklppy88 Apr 16, 2024
9e184de
expand comments
sklppy88 Apr 16, 2024
192d870
Merge branch 'master' into ek/feat/implement-key-registry
sklppy88 Apr 16, 2024
bf4529a
Merge branch 'master' into ek/feat/implement-key-registry
sklppy88 Apr 16, 2024
d39ac15
address comments
sklppy88 Apr 16, 2024
442d745
Merge branch 'master' into ek/feat/implement-key-registry
sklppy88 Apr 16, 2024
634c15b
yarn format
sklppy88 Apr 17, 2024
d663fa8
test
sklppy88 Apr 17, 2024
9ab61ba
Update yarn-project/end-to-end/src/e2e_key_registry.test.ts
sklppy88 Apr 17, 2024
2f7e492
Update yarn-project/end-to-end/src/e2e_key_registry.test.ts
sklppy88 Apr 17, 2024
d9e61c7
Update yarn-project/end-to-end/src/e2e_key_registry.test.ts
sklppy88 Apr 17, 2024
51180e3
Update yarn-project/end-to-end/src/e2e_key_registry.test.ts
sklppy88 Apr 17, 2024
4918f41
Update yarn-project/end-to-end/src/e2e_key_registry.test.ts
sklppy88 Apr 17, 2024
8867e5e
Update yarn-project/end-to-end/src/e2e_key_registry.test.ts
sklppy88 Apr 17, 2024
78093cd
Update yarn-project/end-to-end/src/e2e_key_registry.test.ts
sklppy88 Apr 17, 2024
2b1b5cb
address comments
sklppy88 Apr 17, 2024
b655e76
yarn format
sklppy88 Apr 17, 2024
bb4b7cb
fix
sklppy88 Apr 17, 2024
8666545
Merge branch 'master' into ek/feat/implement-key-registry
sklppy88 Apr 17, 2024
96ed013
yarn fmt
sklppy88 Apr 17, 2024
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
2 changes: 2 additions & 0 deletions noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable.nr
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
mod shared_mutable;
mod scheduled_value_change;
mod shared_mutable_private_getter;

use shared_mutable::SharedMutable;
use shared_mutable_private_getter::SharedMutablePrivateGetter;
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
use dep::protocol_types::{hash::pedersen_hash, traits::FromField, address::AztecAddress};

use crate::context::{PrivateContext, Context};
use crate::history::public_storage::public_storage_historical_read;
use crate::public_storage;
use crate::state_vars::{storage::Storage, shared_mutable::scheduled_value_change::ScheduledValueChange};

struct SharedMutablePrivateGetter<T, DELAY> {
sklppy88 marked this conversation as resolved.
Show resolved Hide resolved
context: PrivateContext,
// The contract address of the contract we want to read from
contract_address: AztecAddress,
sklppy88 marked this conversation as resolved.
Show resolved Hide resolved
// The storage slot where the SharedMutable is stored on the other contract
storage_slot: Field,
}

// We have this as a view-only interface to reading Shared Mutables in other contracts.
// Currently the Shared Mutable does not support this. We can adapt SharedMutable at a later date
impl<T, DELAY> SharedMutablePrivateGetter<T, DELAY> {
pub fn new(
context: PrivateContext,
contract_address: AztecAddress,
storage_slot: Field,
) -> Self {
assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1.");
assert(contract_address.to_field() != 0, "Contract address cannot be 0");
Self { context, contract_address, storage_slot }
}

pub fn get_current_value_in_private(self) -> T where T: FromField {
let mut context = self.context;

let (scheduled_value_change, historical_block_number) = self.historical_read_from_public_storage(context);
let block_horizon = scheduled_value_change.get_block_horizon(historical_block_number);

// We prevent this transaction from being included in any block after the block horizon, ensuring that the
// historical public value matches the current one, since it can only change after the horizon.
context.set_tx_max_block_number(block_horizon);
scheduled_value_change.get_current_at(historical_block_number)
}

fn historical_read_from_public_storage(
self,
context: PrivateContext
) -> (ScheduledValueChange<T, DELAY>, u32) where T: FromField {
let derived_slot = self.get_derived_storage_slot();

// Ideally the following would be simply public_storage::read_historical, but we can't implement that yet.
let mut raw_fields = [0; 3];
for i in 0..3 {
raw_fields[i] = public_storage_historical_read(
context,
derived_slot + i as Field,
self.contract_address
);
}

let scheduled_value: ScheduledValueChange<T, DELAY> = ScheduledValueChange::deserialize(raw_fields);
let historical_block_number = context.historical_header.global_variables.block_number as u32;

(scheduled_value, historical_block_number)
}

fn get_derived_storage_slot(self) -> Field {
// Since we're actually storing three values (a ScheduledValueChange struct), we hash the storage slot to get a
// unique location in which we can safely store as much data as we need. This could be removed if we informed
// the slot allocator of how much space we need so that proper padding could be added.
// See https://github.com/AztecProtocol/aztec-packages/issues/5492
pedersen_hash([self.storage_slot, 0], 0)
}
}
2 changes: 2 additions & 0 deletions noir-projects/noir-contracts/Nargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ members = [
"contracts/escrow_contract",
"contracts/gas_token_contract",
"contracts/import_test_contract",
"contracts/key_registry_contract",
"contracts/inclusion_proofs_contract",
"contracts/lending_contract",
"contracts/parent_contract",
Expand All @@ -33,6 +34,7 @@ members = [
"contracts/schnorr_hardcoded_account_contract",
"contracts/schnorr_single_key_account_contract",
"contracts/slow_tree_contract",
"contracts/state_vars_contract",
"contracts/stateful_test_contract",
"contracts/test_contract",
"contracts/token_contract",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[package]
name = "key_registry_contract"
authors = [""]
compiler_version = ">=0.25.0"
type = "contract"

[dependencies]
aztec = { path = "../../../aztec-nr/aztec" }
authwit = { path = "../../../aztec-nr/authwit" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
contract KeyRegistry {
use dep::std::hash::poseidon2::Poseidon2::hash as poseidon2_hash;
use dep::authwit::auth::assert_current_call_valid_authwit_public;

use dep::aztec::{
context::gas::GasOpts,
state_vars::{
SharedMutable,
Map
},
protocol_types::{
abis::function_selector::FunctionSelector,
contract_class_id::ContractClassId,
address::{
AztecAddress,
EthAddress,
PublicKeysHash,
PartialAddress,
},
constants::{
GENERATOR_INDEX__CONTRACT_ADDRESS_V1,
GENERATOR_INDEX__PUBLIC_KEYS_HASH
},
traits::{
Serialize,
Deserialize,
}
},
};

global KEY_ROTATION_DELAY = 5;

#[aztec(storage)]
struct Storage {
// We are not supporting rotating / changing keys other than the nullifier public in the registry at the moment, but will in the future.
// Uncomment lines below to enable that functionality
nullifier_public_key_registry: Map<AztecAddress, SharedMutable<Field, KEY_ROTATION_DELAY>>,
// incoming_public_key_registry: Map<AztecAddress, SharedMutable<Field, KEY_ROTATION_DELAY>>,
sklppy88 marked this conversation as resolved.
Show resolved Hide resolved
// outgoing_public_key_registry: Map<AztecAddress, SharedMutable<Field, KEY_ROTATION_DELAY>>,
// tagging_public_key_registry: Map<AztecAddress, SharedMutable<Field, KEY_ROTATION_DELAY>>,
}

#[aztec(public)]
fn rotate_keys(
address: AztecAddress,
new_nullifier_public_key: Field,
// We are not supporting rotating / changing keys other than the nullifier public in the registry at the moment, but will in the future.
// Uncomment lines below to enable that functionality
// new_incoming_public_key: FIeld,
// new_outgoing_public_key: FIeld,
// new_tagging_public_key: FIeld,
) {
sklppy88 marked this conversation as resolved.
Show resolved Hide resolved
assert(
(new_nullifier_public_key != 0),
// We are not supporting rotating / changing keys other than the nullifier public in the registry at the moment, but will in the future.
// Uncomment lines below to enable that functionality // (new_incoming_public_key != 0) &
// (new_outgoing_public_key != 0) &
// (new_tagging_public_key != 0),
"New nullifier public key must be non-zero"
);

if (!address.eq(context.msg_sender())) {
assert_current_call_valid_authwit_public(&mut context, address);
}

let nullifier_key_registry = storage.nullifier_public_key_registry.at(address);
// We are not supporting rotating / changing keys other than the nullifier public in the registry at the moment, but will in the future.
// Uncomment lines below to enable that functionality
// let incoming_entry = storage.nullifier_public_key_registry.at(context.msg_sender());
// let outgoing_entry = storage.nullifier_public_key_registry.at(context.msg_sender());
// let tagging_entry = storage.nullifier_public_key_registry.at(context.msg_sender());

nullifier_key_registry.schedule_value_change(new_nullifier_public_key);
// We are not supporting rotating / changing keys other than the nullifier public in the registry at the moment, but will in the future.
// Uncomment lines below to enable that functionality
// incoming_entry.schedule_value_change(incoming_public_key);
// outgoing_entry.schedule_value_change(outgoing_public_key);
// tagging_entry.schedule_value_change(tagging_public_key);
}

#[aztec(public)]
fn register_from_preimage(
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just call this register and the other function rotate_nullifier_public_key. The preimage check is an implementation detail so I don't think it makes sense to have it in func name and I think we'll always want to rotate the keys only 1 by 1 so it doesn't make sense to have generic rotate_keys function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like this, addressed in 2b1b5c, along with all the other feedback. Thank you 🙏

address: AztecAddress,
partial_address: PartialAddress,
nullifier_public_key: Field,
incoming_public_key: Field,
outgoing_public_key: Field,
tagging_public_key: Field,
) {
assert(
(nullifier_public_key != 0) &
(incoming_public_key != 0) &
(outgoing_public_key != 0) &
(tagging_public_key != 0),
"All public keys must be non-zero"
);

// TODO (ek): Do it below after refactoring all public_keys_hash_elemtns
// let public_keys_hash = PublicKeysHash::compute(nullifier_public_key, tagging_public_key, incoming_public_key, outgoing_public_key);
// let address = AztecAddress::compute(public_keys_hash, partial_address);
// We could also pass in original_public_keys_hash instead of computing it here, if all we need the original one is for being able to prove ownership of address
let public_keys_hash = poseidon2_hash([
nullifier_public_key,
incoming_public_key,
outgoing_public_key,
tagging_public_key,
GENERATOR_INDEX__PUBLIC_KEYS_HASH,
],
5
);

let computed_address = AztecAddress::from_field(
poseidon2_hash([
partial_address.to_field(),
public_keys_hash.to_field(),
GENERATOR_INDEX__CONTRACT_ADDRESS_V1 as Field,
],
3
)
);

assert(computed_address.eq(address), "Computed address does not match supplied address");

let nullifier_key_registry = storage.nullifier_public_key_registry.at(address);
// We are not supporting rotating / changing keys other than the nullifier public in the registry at the moment, but will in the future.
// Uncomment lines below to enable that functionality
// let incoming_key_registry = storage.incoming_public_key_registry.at(address);
// let outgoing_key_registry = storage.outgoing_public_key_registry.at(address);
// let tagging_key_registry = storage.taggin_public_key_registry.at(address);

nullifier_key_registry.schedule_value_change(nullifier_public_key);
// We are not supporting rotating / changing keys other than the nullifier public in the registry at the moment, but will in the future.
// Uncomment lines below to enable that functionality // incoming_key_registry.schedule_value_change(new_incoming_public_key);
// outgoing_key_registry.schedule_value_change(new_outgoing_public_key);
// tagging_key_registry.schedule_value_change(new_tagging_public_key);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[package]
name = "state_vars_contract"
authors = [""]
compiler_version = ">=0.25.0"
type = "contract"

[dependencies]
aztec = { path = "../../../aztec-nr/aztec" }
authwit = { path = "../../../aztec-nr/authwit" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
contract StateVars {
sklppy88 marked this conversation as resolved.
Show resolved Hide resolved
use dep::aztec::prelude::{
AztecAddress, FunctionSelector, NoteHeader, NoteGetterOptions, NoteViewerOptions,
PrivateContext, Map, PublicMutable, PublicImmutable, PrivateMutable, PrivateImmutable,
PrivateSet, SharedImmutable
};
use dep::aztec::protocol_types::traits::{Deserialize, Serialize};

use dep::aztec::{
state_vars::shared_mutable::SharedMutablePrivateGetter,
context::{PublicContext, Context},
log::{emit_unencrypted_log_from_private},
};

#[aztec(private)]
fn test_shared_mutable_private_getter_for_registry_contract(
contract_address_to_read: AztecAddress,
storage_slot_of_shared_mutable: Field,
address_to_get_in_registry: AztecAddress,
) {
// We have to derive this slot to get the location of the shared mutable inside the Map
let derived_slot = dep::aztec::hash::pedersen_hash([storage_slot_of_shared_mutable, address_to_get_in_registry.to_field()], 0);
// It's a bit wonky because we need to know the delay for get_current_value_in_private to work correctly
let registry_private_getter: SharedMutablePrivateGetter<AztecAddress, 5> = SharedMutablePrivateGetter::new(context, contract_address_to_read, derived_slot);
let nullifier_public_key = registry_private_getter.get_current_value_in_private();

emit_unencrypted_log_from_private(&mut context, nullifier_public_key);
}

#[aztec(private)]
fn test_shared_mutable_private_getter(
contract_address_to_read: AztecAddress,
storage_slot_of_shared_mutable: Field,
) {
// It's a bit wonky because we need to know the delay for get_current_value_in_private to work correctly
let test: SharedMutablePrivateGetter<AztecAddress, 5> = SharedMutablePrivateGetter::new(context, contract_address_to_read, storage_slot_of_shared_mutable);
let authorized = test.get_current_value_in_private();

emit_unencrypted_log_from_private(&mut context, authorized);
}

#[aztec(public)]
fn delay() {
// We use this as a util function to "mine a block"
dep::aztec::log::emit_unencrypted_log(
&mut context,
"dummy"
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ use crate::{
aztec_address::AztecAddress
},
constants::GENERATOR_INDEX__PARTIAL_ADDRESS, contract_class_id::ContractClassId,
hash::pedersen_hash, traits::ToField
hash::pedersen_hash, traits::{ToField, FromField, Serialize, Deserialize}
};

global PARTIAL_ADDRESS_LENGTH = 1;

// Partial address
struct PartialAddress {
inner : Field
Expand All @@ -18,6 +20,18 @@ impl ToField for PartialAddress {
}
}

impl Serialize<PARTIAL_ADDRESS_LENGTH> for PartialAddress {
fn serialize(self: Self) -> [Field; PARTIAL_ADDRESS_LENGTH] {
[self.to_field()]
}
}

impl Deserialize<PARTIAL_ADDRESS_LENGTH> for PartialAddress {
fn deserialize(fields: [Field; PARTIAL_ADDRESS_LENGTH]) -> Self {
PartialAddress { inner: fields[0] }
}
}

impl PartialAddress {
pub fn from_field(field: Field) -> Self {
Self { inner: field }
Expand Down
2 changes: 2 additions & 0 deletions yarn-project/end-to-end/src/composed/cli_docs_sandbox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ FPCContractArtifact
GasTokenContractArtifact
ImportTestContractArtifact
InclusionProofsContractArtifact
KeyRegistryContractArtifact
LendingContractArtifact
MultiCallEntrypointContractArtifact
ParentContractArtifact
Expand All @@ -127,6 +128,7 @@ SchnorrAccountContractArtifact
SchnorrHardcodedAccountContractArtifact
SchnorrSingleKeyAccountContractArtifact
SlowTreeContractArtifact
StateVarsContractArtifact
StatefulTestContractArtifact
TestContractArtifact
TokenBlacklistContractArtifact
Expand Down
Loading
Loading