-
Notifications
You must be signed in to change notification settings - Fork 164
fix(standards): derive the network account target of MINT and BURN notes #3664
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
base: next
Are you sure you want to change the base?
Changes from 7 commits
2b6050b
4d9f972
9dd8b39
2c8d78d
66cba2a
34773cf
284d62d
5d2fb18
6258b88
d8bdb0a
2e1bb9f
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 |
|---|---|---|
|
|
@@ -22,8 +22,8 @@ use miden_protocol::utils::sync::LazyLock; | |
| use miden_protocol::{Felt, MAX_NOTE_STORAGE_ITEMS, Word}; | ||
|
|
||
| use crate::StandardsLib; | ||
| use crate::note::P2idNote; | ||
| use crate::note::costs::{MINT_CONSUMPTION_CYCLES, NoteConsumptionCost}; | ||
| use crate::note::{NetworkAccountTarget, P2idNote}; | ||
|
|
||
| // NOTE SCRIPT | ||
| // ================================================================================================ | ||
|
|
@@ -51,6 +51,11 @@ static MINT_SCRIPT: LazyLock<NoteScript> = LazyLock::new(|| { | |
| /// the output note minted on consumption can be private or public depending on the | ||
| /// [`MintNoteStorage`] variant. | ||
| /// | ||
| /// A note whose faucet is public is routed to it by a | ||
| /// [`NetworkAccountTarget`](crate::note::NetworkAccountTarget) attachment derived from the asset in | ||
| /// its storage, which is also what the note is tagged for. A private faucet can never be a network | ||
| /// account, so such a note carries no target and is only tagged. | ||
| /// | ||
|
mmagician marked this conversation as resolved.
Outdated
|
||
| /// Construct one with the [builder](MintNote::builder); convert it into a protocol [`Note`] | ||
| /// infallibly via `Note::from`. | ||
| #[derive(Debug, Clone)] | ||
|
|
@@ -69,15 +74,23 @@ impl MintNote { | |
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns an error if the attachments exceed their protocol limit (see | ||
| /// [`NoteAttachments::new`]). | ||
| /// Returns an error if: | ||
| /// - the attachments carry a `NetworkAccountTarget` for an account other than the faucet. | ||
| /// - the attachments exceed their protocol limit (see [`NoteAttachments::new`]). | ||
| #[builder] | ||
| pub fn new( | ||
| #[builder(field)] attachments: Vec<NoteAttachment>, | ||
| #[builder(field)] mut attachments: Vec<NoteAttachment>, | ||
| sender: AccountId, | ||
| #[builder(name = mint_storage)] storage: MintNoteStorage, | ||
| serial_number: Word, | ||
| ) -> Result<Self, NoteError> { | ||
| // The network routes the note on this attachment; the stored ASSET_ID is what binds the | ||
| // script to the same faucet on consumption. | ||
| NetworkAccountTarget::ensure_presence_if_public(&mut attachments, storage.faucet_id()) | ||
| .map_err(|err| { | ||
| NoteError::other_with_source("failed to target the MINT note at its faucet", err) | ||
| })?; | ||
|
Comment on lines
+90
to
+93
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. Why for |
||
|
|
||
| let attachments = NoteAttachments::new(attachments)?; | ||
|
|
||
| Ok(Self { | ||
|
|
@@ -317,34 +330,63 @@ mod tests { | |
| use miden_protocol::crypto::rand::RandomCoin; | ||
|
|
||
| use super::*; | ||
| use crate::note::{NetworkNoteExt, NoteExecutionHint}; | ||
|
|
||
| fn faucet() -> AccountId { | ||
| AccountId::builder().account_type(AccountType::Public).build_with_seed([1; 32]) | ||
| } | ||
|
|
||
| fn private_faucet() -> AccountId { | ||
| AccountId::builder().account_type(AccountType::Private).build_with_seed([1; 32]) | ||
| } | ||
|
|
||
| fn owner() -> AccountId { | ||
| AccountId::builder().account_type(AccountType::Private).build_with_seed([2; 32]) | ||
| } | ||
|
|
||
| /// The builder produces a public, asset-less note tagged for the faucet. | ||
| #[test] | ||
| fn builder_builds_public_mint_note() { | ||
| fn build_mint_note(faucet_id: AccountId) -> MintNote { | ||
| let asset = FungibleAsset::new(faucet_id, 50).unwrap(); | ||
| let mut rng = RandomCoin::new(Word::empty()); | ||
| let asset = FungibleAsset::new(faucet(), 50).unwrap(); | ||
| let mint_storage = MintNoteStorage::new_private(Word::empty(), asset, NoteTag::default()); | ||
| let mint_note = MintNote::builder() | ||
| MintNote::builder() | ||
| .sender(owner()) | ||
| .mint_storage(mint_storage) | ||
| .mint_storage(MintNoteStorage::new_private(Word::empty(), asset, NoteTag::default())) | ||
| .generate_serial_number(&mut rng) | ||
| .build() | ||
| .unwrap(); | ||
| .unwrap() | ||
| } | ||
|
|
||
| /// The builder produces a public, asset-less note tagged for the faucet and routed to it by a | ||
| /// derived network target. How that target treats caller-supplied attachments is covered by the | ||
| /// `network_account_target` tests. | ||
| #[test] | ||
| fn builder_builds_public_mint_note() { | ||
| let mint_note = build_mint_note(faucet()); | ||
|
|
||
| assert_eq!(mint_note.faucet_id(), faucet()); | ||
| assert_eq!(mint_note.sender(), owner()); | ||
| assert_eq!(mint_note.attachments().num_attachments(), 1); | ||
|
|
||
| let note = Note::from(mint_note); | ||
| assert_eq!(note.metadata().note_type(), NoteType::Public); | ||
| assert_eq!(note.metadata().tag(), NoteTag::with_account_target(faucet())); | ||
| assert_eq!(note.assets().num_assets(), 0); | ||
| assert!(note.is_network_note()); | ||
|
|
||
| let target = NetworkAccountTarget::try_from(note.attachments()).unwrap(); | ||
| assert_eq!(target.target_id(), faucet()); | ||
| assert_eq!(target.execution_hint(), NoteExecutionHint::Always); | ||
| } | ||
|
|
||
| /// A private faucet is never a network account, so no target is derived for it. The note is | ||
| /// still tagged for the faucet and remains consumable by it. | ||
| #[test] | ||
| fn builder_omits_network_target_for_private_faucet() { | ||
| let mint_note = build_mint_note(private_faucet()); | ||
|
|
||
| assert_eq!(mint_note.attachments().num_attachments(), 0); | ||
|
|
||
| let note = Note::from(mint_note); | ||
| assert_eq!(note.metadata().tag(), NoteTag::with_account_target(private_faucet())); | ||
| assert!(!note.is_network_note()); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,8 +74,53 @@ impl NetworkAccountTarget { | |
| attachments: &mut Vec<NoteAttachment>, | ||
| target_id: AccountId, | ||
| ) -> Result<(), NetworkAccountTargetError> { | ||
| // Every attachment of the scheme is validated, so no attachment can claim a target other | ||
| // than `target_id`. | ||
| if !Self::contains_target(attachments, target_id)? { | ||
| let target = Self::new(target_id, NoteExecutionHint::Always)?; | ||
| attachments.push(NoteAttachment::from(target)); | ||
| } | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| /// Behaves like [`Self::ensure_presence`], except that a non-public `target_id` is accepted | ||
| /// without appending a target. | ||
| /// | ||
| /// A private account is never a network account, so it has no routing target to derive. This | ||
| /// lets a note whose target may be either kind of account carry the target exactly when it is | ||
| /// meaningful, while a caller-supplied target for another account is rejected either way. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns an error if an attachment with the [`NetworkAccountTarget::ATTACHMENT_SCHEME`] does | ||
| /// not decode as a [`NetworkAccountTarget`] or targets an account other than `target_id`. | ||
| pub(crate) fn ensure_presence_if_public( | ||
| attachments: &mut Vec<NoteAttachment>, | ||
| target_id: AccountId, | ||
| ) -> Result<(), NetworkAccountTargetError> { | ||
| if target_id.is_public() { | ||
| return Self::ensure_presence(attachments, target_id); | ||
| } | ||
|
|
||
| // No target is derived, but a caller-supplied attachment of the scheme must still be | ||
| // validated, so the note cannot claim a network target it does not have. The returned flag | ||
| // is discarded because it is always false here: an attachment naming a non-public account | ||
| // never decodes into a `NetworkAccountTarget`, so a present target can only be an error. | ||
|
mmagician marked this conversation as resolved.
Outdated
|
||
| Self::contains_target(attachments, target_id).map(|_| ()) | ||
|
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 need this line to check if a private account has malformed network target attachment, right? Or is there some other reason? Regardless - we should add a brief comment explaining why this line is needed.
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'm struggling to understand the comment, it's very claude-ish. I think the idea is to make sure there is no attachment that a user added that claims to be a // Ensure that none of the user-provided attachments claims to be a
// `NetworkAccountTarget` with a private target. |
||
| } | ||
|
|
||
| /// Returns whether `attachments` carries a [`NetworkAccountTarget`] for `target_id`. | ||
| /// | ||
| /// Every attachment of the scheme is validated, so no attachment can claim a target other than | ||
| /// `target_id`. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns an error if an attachment with the [`NetworkAccountTarget::ATTACHMENT_SCHEME`] does | ||
| /// not decode as a [`NetworkAccountTarget`] or targets an account other than `target_id`. | ||
| fn contains_target( | ||
| attachments: &[NoteAttachment], | ||
| target_id: AccountId, | ||
| ) -> Result<bool, NetworkAccountTargetError> { | ||
| let mut is_present = false; | ||
| for attachment in attachments | ||
| .iter() | ||
|
|
@@ -92,12 +137,7 @@ impl NetworkAccountTarget { | |
| is_present = true; | ||
| } | ||
|
|
||
| if !is_present { | ||
| let target = Self::new(target_id, NoteExecutionHint::Always)?; | ||
| attachments.push(NoteAttachment::from(target)); | ||
| } | ||
|
|
||
| Ok(()) | ||
| Ok(is_present) | ||
| } | ||
|
|
||
| // ACCESSORS | ||
|
|
@@ -287,6 +327,34 @@ mod tests { | |
| Ok(()) | ||
| } | ||
|
|
||
| /// A non-public target has no network routing target, so none is appended, but a | ||
| /// caller-supplied target for another account is still rejected. | ||
| #[test] | ||
| fn ensure_presence_if_public_skips_private_target() -> anyhow::Result<()> { | ||
| let private_id = AccountIdBuilder::new() | ||
| .account_type(AccountType::Private) | ||
| .build_with_rng(&mut rand::rng()); | ||
| let mut attachments = vec![]; | ||
|
|
||
| NetworkAccountTarget::ensure_presence_if_public(&mut attachments, private_id)?; | ||
| assert!(attachments.is_empty()); | ||
|
|
||
| let other_id = public_account_id(); | ||
| let supplied = NetworkAccountTarget::new(other_id, NoteExecutionHint::Always)?; | ||
| let mut attachments = vec![NoteAttachment::from(supplied)]; | ||
|
|
||
| let err = NetworkAccountTarget::ensure_presence_if_public(&mut attachments, private_id) | ||
| .unwrap_err(); | ||
|
|
||
| assert_matches!( | ||
| err, | ||
| NetworkAccountTargetError::TargetMismatch { expected, actual } | ||
| if expected == private_id && actual == other_id | ||
| ); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| #[test] | ||
| fn network_account_target_fails_on_private_target_account() -> anyhow::Result<()> { | ||
| let id = AccountIdBuilder::new() | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.