-
Notifications
You must be signed in to change notification settings - Fork 609
feat(aztec-nr): Helper to validate the app-siloed secret and index for constrained tagging #23569
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
Changes from 27 commits
98b9784
27e5a5e
381858d
fff3541
3202ab1
49bfefc
d00136d
76c025e
75554b1
9a451ef
14d22ca
bfb03bf
aa3d1e6
df3f982
36d355d
3bad347
f4b6e86
652d92d
f686408
37c3d0e
0afe779
a45d68a
0ff7c94
f7ef01f
3e89f16
dd3de61
21f5920
9f44ac2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,109 @@ | ||||||||||||||||||||||||||||||||||||||
| //! Sender-side helpers for constrained message delivery. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| use crate::context::PrivateContext; | ||||||||||||||||||||||||||||||||||||||
| use crate::nullifier::utils::compute_nullifier_existence_request; | ||||||||||||||||||||||||||||||||||||||
| use crate::oracle::{call_utility_function::call_utility_function, notes::get_next_constrained_tagging_index}; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| use crate::protocol::{ | ||||||||||||||||||||||||||||||||||||||
| abis::function_selector::FunctionSelector, | ||||||||||||||||||||||||||||||||||||||
| address::AztecAddress, | ||||||||||||||||||||||||||||||||||||||
| constants::DOM_SEP__CONSTRAINED_MSG_NULLIFIER, | ||||||||||||||||||||||||||||||||||||||
| hash::poseidon2_hash_with_separator, | ||||||||||||||||||||||||||||||||||||||
| traits::{Deserialize, ToField}, | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /// Resolves the app-siloed shared secret and next per-secret index for a constrained send. | ||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||
| /// Wraps the registry calls needed by every constrained-delivery app: query the handshake registry for an | ||||||||||||||||||||||||||||||||||||||
| /// existing app-siloed secret, bootstrap a fresh handshake if there isn't one, and constrain the | ||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||
| /// oracle-supplied secret. The returned `(secret, index)` pair is the input for the caller's tag derivation | ||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be an index?
Suggested change
|
||||||||||||||||||||||||||||||||||||||
| /// and nullifier emission. | ||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||
| /// ## Implementation Details | ||||||||||||||||||||||||||||||||||||||
| /// 1. Calls `HandshakeRegistry::get_app_siloed_secret` offchain (untrusted hint). | ||||||||||||||||||||||||||||||||||||||
| /// 2. On `None`, calls `HandshakeRegistry::non_interactive_handshake(sender, recipient)` to bootstrap and | ||||||||||||||||||||||||||||||||||||||
| /// returns `(secret, 0)`. The constrained return value of the private call is the source of truth for the | ||||||||||||||||||||||||||||||||||||||
| /// secret. | ||||||||||||||||||||||||||||||||||||||
| /// 3. On `Some(secret)`, reads the per-secret index hint via [`get_next_constrained_tagging_index`] (also untrusted), | ||||||||||||||||||||||||||||||||||||||
| /// then constrains: | ||||||||||||||||||||||||||||||||||||||
| /// - `index == 0`: calls `HandshakeRegistry::validate_handshake` to bind the secret to the registry's | ||||||||||||||||||||||||||||||||||||||
| /// current note. `PrivateMutable::get_note` inside `validate_handshake` nullifies the current handshake | ||||||||||||||||||||||||||||||||||||||
| /// note and emits a fresh one, so this branch already produces a per-call nullifier on the registry | ||||||||||||||||||||||||||||||||||||||
| /// note. | ||||||||||||||||||||||||||||||||||||||
| /// - `index > 0`: asserts the prior nullifier | ||||||||||||||||||||||||||||||||||||||
| /// `poseidon2_hash_with_separator([sender, recipient, secret, index - 1], DOM_SEP__CONSTRAINED_MSG_NULLIFIER)` | ||||||||||||||||||||||||||||||||||||||
| /// exists via [`compute_nullifier_existence_request`](crate::nullifier::utils::compute_nullifier_existence_request). | ||||||||||||||||||||||||||||||||||||||
| /// The chain transitively constrains back to the index-0 `validate_handshake`. The caller is expected | ||||||||||||||||||||||||||||||||||||||
| /// to emit the matching nullifier alongside each constrained send so subsequent calls can prove the | ||||||||||||||||||||||||||||||||||||||
| /// chain. | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+38
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would skip this. I think that either the code is clear enough or, if not, we should make it clearer.
Suggested change
|
||||||||||||||||||||||||||||||||||||||
| pub fn calculate_secret_and_index( | ||||||||||||||||||||||||||||||||||||||
| context: &mut PrivateContext, | ||||||||||||||||||||||||||||||||||||||
| registry: AztecAddress, | ||||||||||||||||||||||||||||||||||||||
| sender: AztecAddress, | ||||||||||||||||||||||||||||||||||||||
| recipient: AztecAddress, | ||||||||||||||||||||||||||||||||||||||
| ) -> (Field, u32) { | ||||||||||||||||||||||||||||||||||||||
| let caller = context.this_address(); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Safety: the returned `Option<Field>` is untrusted and is constrained below before being returned to the | ||||||||||||||||||||||||||||||||||||||
| // caller, either by performing a new handshake or by constraining the prior handshake's existence. | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+47
to
+48
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about something like this? |
||||||||||||||||||||||||||||||||||||||
| let maybe_secret: Option<Field> = unsafe { | ||||||||||||||||||||||||||||||||||||||
| let selector = comptime { | ||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we somehow avoid the selector and to something like |
||||||||||||||||||||||||||||||||||||||
| FunctionSelector::from_signature("get_app_siloed_secret((Field),(Field),(Field))") | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
| let returns = call_utility_function( | ||||||||||||||||||||||||||||||||||||||
| registry, | ||||||||||||||||||||||||||||||||||||||
| selector, | ||||||||||||||||||||||||||||||||||||||
| // TODO(F-671): Replace explicit caller argument once utility context exposes msg_sender(). | ||||||||||||||||||||||||||||||||||||||
| [sender.to_field(), recipient.to_field(), caller.to_field()], | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
| Deserialize::deserialize(returns) | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if maybe_secret.is_none() { | ||||||||||||||||||||||||||||||||||||||
| // Bootstrap: no handshake exists yet. The registry inserts a fresh note and returns the app-siloed | ||||||||||||||||||||||||||||||||||||||
| // secret to the caller. The constrained return is the source of truth for the secret, so no separate | ||||||||||||||||||||||||||||||||||||||
| // `validate_handshake` is needed. | ||||||||||||||||||||||||||||||||||||||
| // TODO(F-660): dispatch to `perform_handshake(sender, recipient, handshake_type)` once interactive | ||||||||||||||||||||||||||||||||||||||
| // handshakes are supported. | ||||||||||||||||||||||||||||||||||||||
| let selector = comptime { | ||||||||||||||||||||||||||||||||||||||
| FunctionSelector::from_signature("non_interactive_handshake((Field),(Field))") | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
| let secret: Field = context | ||||||||||||||||||||||||||||||||||||||
| .call_private_function(registry, selector, [sender.to_field(), recipient.to_field()]) | ||||||||||||||||||||||||||||||||||||||
| .get_preimage(); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Reserve index 0 for the freshly bootstrapped secret so the PXE seeds its per-secret counter, mirroring | ||||||||||||||||||||||||||||||||||||||
| // the `Some` branch. Without this, a later constrained message under the same secret restarts at index 0 | ||||||||||||||||||||||||||||||||||||||
| // and collides on `(secret, 0)`. | ||||||||||||||||||||||||||||||||||||||
| // Safety: this only advances a PXE-side counter; the returned index is constrained to 0 below. | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+75
to
+78
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feels off. Calling the oracle so that the PXE counter can be started? Shouldn't the oracle fetch for logs when starting and start at the correct index? What if the messages started in another PXE, and the index should be 2 when |
||||||||||||||||||||||||||||||||||||||
| let index = unsafe { get_next_constrained_tagging_index(secret) }; | ||||||||||||||||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without this call the new |
||||||||||||||||||||||||||||||||||||||
| assert(index == 0, "freshly bootstrapped secret must start at index 0"); | ||||||||||||||||||||||||||||||||||||||
| (secret, 0) | ||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||
| let secret = maybe_secret.unwrap_unchecked(); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Safety: the index is untrusted; the anchor branches below ensure the returned `(secret, index)` pair | ||||||||||||||||||||||||||||||||||||||
| // is bound to a real registry handshake before it leaves the helper. | ||||||||||||||||||||||||||||||||||||||
| let index = unsafe { get_next_constrained_tagging_index(secret) }; | ||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel this a little clearer
Suggested change
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if index == 0 { | ||||||||||||||||||||||||||||||||||||||
| let selector = comptime { | ||||||||||||||||||||||||||||||||||||||
| FunctionSelector::from_signature("validate_handshake((Field),(Field),Field)") | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
| let _ = context.call_private_function( | ||||||||||||||||||||||||||||||||||||||
| registry, | ||||||||||||||||||||||||||||||||||||||
| selector, | ||||||||||||||||||||||||||||||||||||||
| [sender.to_field(), recipient.to_field(), secret], | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+90
to
+97
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same with selector |
||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||
| let prev_nullifier = poseidon2_hash_with_separator( | ||||||||||||||||||||||||||||||||||||||
| [sender.to_field(), recipient.to_field(), secret, (index - 1) as Field], | ||||||||||||||||||||||||||||||||||||||
| DOM_SEP__CONSTRAINED_MSG_NULLIFIER, | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+99
to
+102
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could probably have a helper function in this file for this, since we'll need to re-use it when we actually send a message |
||||||||||||||||||||||||||||||||||||||
| // TODO(F-670): exercise the index > 0 path end-to-end once send_constrained_msg emits nullifiers. | ||||||||||||||||||||||||||||||||||||||
| context.assert_nullifier_exists(compute_nullifier_existence_request(prev_nullifier, caller)); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| (secret, index) | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| pub mod constrained_delivery; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't quite follow why you moved the file here in this PR and also dropped all the docs. Did you change something else? I don't quite see it I'm ok with moving the file, but maybe we could have a follow up PR where we move the file and directly avoid the compatibility wrapper |
||
|
|
||
| use crate::{ | ||
| context::PrivateContext, | ||
| messages::{ | ||
| encryption::{aes128::AES128, message_encryption::MessageEncryption}, | ||
| logs::utils::compute_discovery_tag, | ||
| offchain_messages::deliver_offchain_message, | ||
| }, | ||
| utils::remove_constraints::remove_constraints_if, | ||
| }; | ||
| use crate::protocol::{address::AztecAddress, constants::DOM_SEP__UNCONSTRAINED_MSG_LOG_TAG, hash::compute_log_tag}; | ||
|
|
||
| /// Specifies how to deliver a message to a recipient. | ||
| /// | ||
| /// All messages are delivered encrypted to their recipient's public address key, so no other account can read their | ||
| /// contents. This struct configures which delivery guarantees exist. | ||
| /// | ||
| /// Use [`MessageDelivery::offchain`] when the sender can deliver directly to the recipient off-chain, | ||
| /// [`MessageDelivery::onchain_unconstrained`] when the chain should retain the message but the sender is trusted to | ||
| /// tag it correctly, and [`MessageDelivery::onchain_constrained`] when the message contents must be constrained. | ||
| pub struct MessageDelivery { | ||
| variant: u8, | ||
| } | ||
|
|
||
| impl Eq for MessageDelivery { | ||
| fn eq(self, other: Self) -> bool { | ||
| self.variant == other.variant | ||
| } | ||
| } | ||
|
|
||
| impl MessageDelivery { | ||
| /// Delivers the message fully off-chain, with no on-chain data availability. | ||
| pub fn offchain() -> Self { | ||
| Self { variant: 1 } | ||
| } | ||
|
|
||
| /// Delivers the message on-chain, but with no constraints on the message content or tag. | ||
| pub fn onchain_unconstrained() -> Self { | ||
| Self { variant: 2 } | ||
| } | ||
|
|
||
| /// Delivers the message on-chain while constraining the encrypted content. | ||
| pub fn onchain_constrained() -> Self { | ||
| Self { variant: 3 } | ||
| } | ||
| } | ||
|
|
||
| /// Performs private delivery of a message to `recipient` according to `delivery_mode`. | ||
| /// | ||
| /// The message is encoded into plaintext and then encrypted for `recipient`. This function takes a _function_ that | ||
| /// returns the plaintext instead of taking the plaintext directly in order to not waste constraints encoding the | ||
| /// message in scenarios where the plaintext will be encrypted with unconstrained encryption. | ||
| /// | ||
| /// `maybe_note_hash_counter` is only relevant for on-chain delivery modes (i.e. via protocol logs): if a newly created | ||
| /// note hash's side effect counter is passed, then the log will be squashed alongside the note should its nullifier be | ||
| /// emitted in the current transaction. This is typically only used for note messages: since the note will not actually | ||
| /// be created, there is no point in delivering the message. | ||
| /// | ||
| /// `delivery_mode` must be a [`MessageDelivery`] value. | ||
| /// | ||
| /// ## Privacy | ||
| /// | ||
| /// The emitted log always has the same length regardless of `MESSAGE_PLAINTEXT_LEN`, because all message ciphertexts | ||
| /// also have the same length. This prevents accidental privacy leakage via the log length. | ||
| pub fn do_private_message_delivery<Env, let MESSAGE_PLAINTEXT_LEN: u32>( | ||
| context: &mut PrivateContext, | ||
| encode_into_message_plaintext: fn[Env]() -> [Field; MESSAGE_PLAINTEXT_LEN], | ||
| maybe_note_hash_counter: Option<u32>, | ||
| recipient: AztecAddress, | ||
| delivery_mode: MessageDelivery, | ||
| ) { | ||
| // This function relies on `delivery_mode` being a constant in order to reduce circuit constraints when | ||
| // unconstrained usage is requested. If `delivery_mode` were a runtime value the compiler would be unable to | ||
| // perform dead-code elimination. | ||
| assert_constant(delivery_mode); | ||
|
|
||
| // The following maps out the 3 dimensions across which we configure message delivery. | ||
| let constrained_encryption = delivery_mode == MessageDelivery::onchain_constrained(); | ||
| let deliver_as_offchain_message = delivery_mode == MessageDelivery::offchain(); | ||
| // TODO(#14565): Add constrained tagging | ||
| let _constrained_tagging = delivery_mode == MessageDelivery::onchain_constrained(); | ||
|
|
||
| let contract_address = context.this_address(); | ||
|
|
||
| let ciphertext = remove_constraints_if( | ||
| !constrained_encryption, | ||
| || AES128::encrypt(encode_into_message_plaintext(), recipient, contract_address), | ||
| ); | ||
|
|
||
| if deliver_as_offchain_message { | ||
| deliver_offchain_message(ciphertext, recipient); | ||
| } else { | ||
| // TODO(#14565): constrained tagging is not yet implemented. Both modes currently use the unconstrained | ||
| // domain separator because the discovery tag always comes from an oracle. Once constrained tagging lands, | ||
| // this should branch on `constrained_tagging` to select the appropriate separator. | ||
| let discovery_tag = compute_discovery_tag(recipient); | ||
| let log_tag = compute_log_tag(discovery_tag, DOM_SEP__UNCONSTRAINED_MSG_LOG_TAG); | ||
|
|
||
| // We forbid this value not being constant to avoid predicating the context calls below, which might result in | ||
| // the context's arrays having unknown compile time write indices and hence dramatically increasing constraints | ||
| // when accessing them. In practice this restriction is not a problem as we always know at compile time whether | ||
| // we're emitting a note or non-note message. | ||
| assert_constant(maybe_note_hash_counter.is_some()); | ||
|
|
||
| let log = BoundedVec::from_array(ciphertext); | ||
| if maybe_note_hash_counter.is_some() { | ||
| // We associate the log with the note's side effect counter, so that if the note ends up being squashed in | ||
| // the current transaction, the log will be removed as well. | ||
| context.emit_raw_note_log_unsafe(log_tag, log, maybe_note_hash_counter.unwrap()); | ||
| } else { | ||
| context.emit_private_log_unsafe(log_tag, log); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Warning, boss-man likes the first line to be 10 words max