P2ID note builder (inicio review)

CHANGELOG.md

@@ -3,7 +3,7 @@
 ## v0.16.0 (TBD)
 
 ### Changes
-
+- [BREAKING] Replaced the `P2idNote` marker type and its `P2idNote::create` factory with a `P2idNote` struct built via a `bon` typestate builder (`P2idNote::builder()`). P2ID notes must now carry at least one asset; a `P2idNote` converts into a `Note` via `Note::from`, and the builder offers `.asset()`/`.assets()`, `.attachment()`/`.attachments()`, and `.generate_serial_number()` ([#2283](https://github.com/0xMiden/protocol/issues/2283)).
 - Added a skeleton batch kernel ([#1122](https://github.com/0xMiden/protocol/issues/1122)) wired through `LocalBatchProver::prove` and attached to `ProvenBatch` as an `ExecutionProof`. It does not yet perform any verification.
 - [BREAKING] Renamed `AccountStorageDelta` to `AccountStoragePatch` ([#3002](https://github.com/0xMiden/protocol/pull/3002)).
 - [BREAKING] Replaced the per-tree account and nullifier backend traits with shared `SmtBackend` and `SmtBackendReader` traits, split into read-only and read-write capabilities, enabling read-only `LargeSmt`-backed tree views via `reader()` ([#2755](https://github.com/0xMiden/protocol/pull/2755), [#3009](https://github.com/0xMiden/protocol/pull/3009)).

crates/miden-standards/src/note/p2id.rs

@@ -8,6 +8,7 @@ use miden_protocol::errors::NoteError;
 use miden_protocol::note::{
     Note,
     NoteAssets,
+    NoteAttachment,
     NoteAttachments,
     NoteRecipient,
     NoteScript,
@@ -38,8 +39,60 @@ static P2ID_SCRIPT: LazyLock<NoteScript> = LazyLock::new(|| {
 // P2ID NOTE
 // ================================================================================================
 
-/// TODO: add docs
-pub struct P2idNote;
+/// A Pay-to-ID (P2ID) note: transfers `assets` from `sender` to the `target` account.
+///
+/// Only the `target` account can consume the note and claim its assets.
+///
+/// Construct one with the [builder](P2idNote::builder), which sets sensible defaults for the
+/// optional parameters (private note type, no attachments) and requires at least one asset.
+/// Convert a `P2idNote` into a protocol [`Note`] infallibly via `Note::from`.
+#[derive(Debug, Clone)]
+pub struct P2idNote {
+    sender: AccountId,
+    storage: P2idNoteStorage,
+    serial_number: Word,
+    note_type: NoteType,
+    assets: NoteAssets,
+    attachments: NoteAttachments,
+}
+
+#[bon::bon]
+impl P2idNote {
+    /// Builds a new [`P2idNote`].
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if:
+    /// - No assets were provided.
+    /// - The assets or attachments exceed their protocol limits (see [`NoteAssets::new`] and
+    ///   [`NoteAttachments::new`]).
+    #[builder]
+    pub fn new(
+        #[builder(field)] assets: Vec<Asset>,
+        #[builder(field)] attachments: Vec<NoteAttachment>,
+        sender: AccountId,
+        #[builder(name = target, with = |target: AccountId| P2idNoteStorage::new(target))]
+        storage: P2idNoteStorage,
+        serial_number: Word,
+        #[builder(default)] note_type: NoteType,
+    ) -> Result<Self, NoteError> {
+        if assets.is_empty() {
+            return Err(NoteError::other("a P2ID note must contain at least one asset"));
+        }
+
+        let assets = NoteAssets::new(assets)?;
+        let attachments = NoteAttachments::new(attachments)?;
+
+        Ok(Self {
+            sender,
+            storage,
+            serial_number,
+            note_type,
+            assets,
+            attachments,
+        })
+    }
+}
 
 impl P2idNote {
     // CONSTANTS
@@ -61,39 +114,103 @@ impl P2idNote {
         P2ID_SCRIPT.root()
     }
 
-    // BUILDERS
-    // --------------------------------------------------------------------------------------------
+    /// Returns the account ID of the note's sender.
+    pub fn sender(&self) -> AccountId {
+        self.sender
+    }
 
-    /// Generates a P2ID note - Pay-to-ID note.
-    ///
-    /// This script enables the transfer of assets from the `sender` account to the `target` account
-    /// by specifying the target's account ID.
-    ///
-    /// The passed-in `rng` is used to generate a serial number for the note. The returned note's
-    /// tag is set to the target's account ID.
-    ///
-    /// # Errors
-    /// Returns an error if deserialization or compilation of the `P2ID` script fails.
-    pub fn create<R: FeltRng>(
-        sender: AccountId,
-        target: AccountId,
-        assets: Vec<Asset>,
-        note_type: NoteType,
-        attachments: NoteAttachments,
-        rng: &mut R,
-    ) -> Result<Note, NoteError> {
-        let serial_num = rng.draw_word();
-        let recipient = P2idNoteStorage::new(target).into_recipient(serial_num);
+    /// Returns the note's storage.
+    pub fn storage(&self) -> P2idNoteStorage {
+        self.storage
+    }
 
-        let tag = NoteTag::with_account_target(target);
+    /// Returns the account ID of the note's target (the only account that can consume it).
+    pub fn target(&self) -> AccountId {
+        self.storage.target()
+    }
+
+    /// Returns the note's serial number.
+    pub fn serial_number(&self) -> Word {
+        self.serial_number
+    }
+
+    /// Returns the note's type.
+    pub fn note_type(&self) -> NoteType {
+        self.note_type
+    }
 
-        let metadata = PartialNoteMetadata::new(sender, note_type).with_tag(tag);
-        let vault = NoteAssets::new(assets)?;
+    /// Returns the assets carried by the note.
+    pub fn assets(&self) -> &NoteAssets {
+        &self.assets
+    }
 
-        Ok(Note::with_attachments(vault, metadata, recipient, attachments))
+    /// Returns the attachments carried by the note.
+    pub fn attachments(&self) -> &NoteAttachments {
+        &self.attachments
     }
 }
 
+// BUILDER EXTENSIONS
+// ================================================================================================
+
+impl<S: p2id_note_builder::State> P2idNoteBuilder<S> {
+    /// Adds a single asset to the note. At least one asset is required for `.build()` to succeed.
+    pub fn asset(mut self, asset: impl Into<Asset>) -> Self {
+        self.assets.push(asset.into());
+        self
+    }
+
+    /// Adds multiple assets to the note.
+    pub fn assets(mut self, assets: impl IntoIterator<Item = impl Into<Asset>>) -> Self {
+        self.assets.extend(assets.into_iter().map(Into::into));
+        self
+    }
+
+    /// Adds a single attachment to the note.
+    pub fn attachment(mut self, attachment: impl Into<NoteAttachment>) -> Self {
+        self.attachments.push(attachment.into());
+        self
+    }
+
+    /// Adds multiple attachments to the note.
+    pub fn attachments(
+        mut self,
+        attachments: impl IntoIterator<Item = impl Into<NoteAttachment>>,
+    ) -> Self {
+        self.attachments.extend(attachments.into_iter().map(Into::into));
+        self
+    }
+}
+
+impl<S: p2id_note_builder::State> P2idNoteBuilder<S>
+where
+    S::SerialNumber: p2id_note_builder::IsUnset,
+{
+    /// Draws a serial number from `rng` and sets it on the builder.
+    pub fn generate_serial_number(
+        self,
+        rng: &mut impl FeltRng,
+    ) -> P2idNoteBuilder<p2id_note_builder::SetSerialNumber<S>> {
+        self.serial_number(rng.draw_word())
+    }
+}
+
+// CONVERSIONS
+// ================================================================================================
+
+impl From<P2idNote> for Note {
+    fn from(note: P2idNote) -> Self {
+        let recipient = note.storage.into_recipient(note.serial_number);
+        let tag = NoteTag::with_account_target(note.storage.target());
+        let metadata = PartialNoteMetadata::new(note.sender, note.note_type).with_tag(tag);
+
+        Note::with_attachments(note.assets, metadata, recipient, note.attachments)
+    }
+}
+
+// P2ID NOTE STORAGE
+// ================================================================================================
+
 /// Canonical storage representation for a P2ID note.
 ///
 /// Contains the identifier of the target account that is authorized
@@ -162,12 +279,17 @@ impl TryFrom<&[Felt]> for P2idNoteStorage {
 
 #[cfg(test)]
 mod tests {
-    use miden_protocol::Felt;
     use miden_protocol::account::{AccountId, AccountIdVersion, AccountType};
+    use miden_protocol::asset::FungibleAsset;
+    use miden_protocol::crypto::rand::RandomCoin;
     use miden_protocol::errors::NoteError;
+    use miden_protocol::{Felt, Word};
 
     use super::*;
 
+    // STORAGE TESTS
+    // --------------------------------------------------------------------------------------------
+
     #[test]
     fn try_from_valid_storage_succeeds() {
         let target = AccountId::dummy([1u8; 15], AccountIdVersion::Version1, AccountType::Private);
@@ -205,4 +327,71 @@ mod tests {
 
         assert!(matches!(err, NoteError::Other { source: Some(_), .. }));
     }
+
+    // BUILDER TESTS
+    // --------------------------------------------------------------------------------------------
+
+    fn sender() -> AccountId {
+        AccountId::dummy([1u8; 15], AccountIdVersion::Version1, AccountType::Private)
+    }
+
+    fn target() -> AccountId {
+        AccountId::dummy([2u8; 15], AccountIdVersion::Version1, AccountType::Private)
+    }
+
+    fn faucet_a() -> AccountId {
+        AccountId::dummy([3u8; 15], AccountIdVersion::Version1, AccountType::Public)
+    }
+
+    fn faucet_b() -> AccountId {
+        AccountId::dummy([4u8; 15], AccountIdVersion::Version1, AccountType::Public)
+    }
+
+    /// The minimal builder uses defaults for everything but the required fields.
+    #[test]
+    fn builder_minimal_uses_defaults() {
+        let note = P2idNote::builder()
+            .sender(sender())
+            .target(target())
+            .serial_number(Word::empty())
+            .asset(FungibleAsset::new(faucet_a(), 1).unwrap())
+            .build()
+            .unwrap();
+
+        assert_eq!(note.sender(), sender());
+        assert_eq!(note.target(), target());
+        assert_eq!(note.note_type(), NoteType::default());
+        assert_eq!(note.assets().num_assets(), 1);
+        assert_eq!(note.attachments().num_attachments(), 0);
+    }
+
+    /// `.asset()` and `.assets()` both append, so they can be combined and called repeatedly.
+    #[test]
+    fn builder_accumulates_assets() {
+        let mut rng = RandomCoin::new(Word::empty());
+        let note = P2idNote::builder()
+            .sender(sender())
+            .target(target())
+            .asset(FungibleAsset::new(faucet_a(), 100).unwrap())
+            .assets([Asset::from(FungibleAsset::new(faucet_b(), 200).unwrap())])
+            .generate_serial_number(&mut rng)
+            .build()
+            .unwrap();
+
+        assert_eq!(note.assets().num_assets(), 2);
+        assert_ne!(note.serial_number(), Word::empty());
+    }
+
+    /// A P2ID note must carry at least one asset.
+    #[test]
+    fn builder_rejects_empty_assets() {
+        let err = P2idNote::builder()
+            .sender(sender())
+            .target(target())
+            .serial_number(Word::empty())
+            .build()
+            .expect_err("a note without assets must be rejected");
+
+        assert!(matches!(err, NoteError::Other { .. }));
+    }
 }

crates/miden-testing/src/kernel_tests/batch/proposed_batch.rs

@@ -68,8 +68,8 @@ pub fn setup_chain() -> TestSetup {
     let account1 = generate_account(&mut builder);
     let account2 = generate_account(&mut builder);
     let note1 = builder
-        .add_p2id_note(account1.id(), account2.id(), &[], NoteType::Public)
-        .expect("adding p2id note1 should work");
+        .add_p2any_note(account1.id(), NoteType::Public, [])
+        .expect("adding p2any note1 should work");
     let mut chain = builder.build().expect("genesis should be valid");
     chain.prove_next_block().expect("valid setup");
 

crates/miden-testing/src/kernel_tests/block/proposed_block_errors.rs

@@ -9,8 +9,7 @@ use miden_protocol::asset::FungibleAsset;
 use miden_protocol::block::{BlockInputs, BlockNumber, ProposedBlock};
 use miden_protocol::crypto::merkle::SparseMerklePath;
 use miden_protocol::errors::ProposedBlockError;
-use miden_protocol::note::{NoteAttachments, NoteInclusionProof, NoteType};
-use miden_standards::note::P2idNote;
+use miden_protocol::note::{NoteInclusionProof, NoteType};
 use miden_tx::LocalTransactionProver;
 
 use crate::kernel_tests::batch::proposed_batch::setup_circular_note_dependency_test;
@@ -352,14 +351,7 @@ async fn proposed_block_fails_on_invalid_proof_or_missing_note_inclusion_referen
     let mut builder = MockChain::builder();
     let account0 = builder.add_existing_mock_account(Auth::IncrNonce)?;
     let account1 = builder.add_existing_mock_account(Auth::IncrNonce)?;
-    let p2id_note = P2idNote::create(
-        account0.id(),
-        account1.id(),
-        vec![],
-        NoteType::Private,
-        NoteAttachments::default(),
-        builder.rng_mut(),
-    )?;
+    let p2id_note = create_p2any_note(account0.id(), NoteType::Private, [], builder.rng_mut());
     let spawn_note = builder.add_spawn_note([&p2id_note])?;
     let mut chain = builder.build()?;
 
@@ -373,7 +365,7 @@ async fn proposed_block_fails_on_invalid_proof_or_missing_note_inclusion_referen
     // inclusion of the unauthenticated note.
     let batch0 = chain.create_batch(vec![tx0])?;
 
-    // Add the P2ID note to the chain by consuming the SPAWN note. The note will hence be created as
+    // Add the note to the chain by consuming the SPAWN note. The note will hence be created as
     // part of block 2 and the note inclusion proof references that block.
     let tx = chain
         .build_tx_context(account0.id(), &[spawn_note.id()], &[])?

crates/miden-testing/src/kernel_tests/block/proposed_block_success.rs

@@ -189,8 +189,8 @@ async fn proposed_block_authenticating_unauthenticated_notes() -> anyhow::Result
     let mut builder = MockChain::builder();
     let account0 = builder.add_existing_mock_account(Auth::IncrNonce)?;
     let account1 = builder.add_existing_mock_account(Auth::IncrNonce)?;
-    let note0 = builder.add_p2id_note(sender_id, account0.id(), &[], NoteType::Private)?;
-    let note1 = builder.add_p2id_note(sender_id, account1.id(), &[], NoteType::Public)?;
+    let note0 = builder.add_p2any_note(sender_id, NoteType::Private, [])?;
+    let note1 = builder.add_p2any_note(sender_id, NoteType::Public, [])?;
     let chain = builder.build()?;
 
     // These txs will use block1 as the reference block.