Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

### Changes

- [BREAKING] Deconstructed `AccountComponentCode` into `Arc<MastForest>` + `Vec<ProcedureExport>` fields, replacing the `Library` newtype wrapper ([#2597](https://github.com/0xMiden/protocol/pull/2597)).
- [BREAKING] Removed `NoteAssets::add_asset`; `OutputNoteBuilder` now accumulates assets in a `Vec` and computes the commitment only when `build()` is called, avoiding rehashing on every asset addition. ([#2577](https://github.com/0xMiden/protocol/pull/2577))
- [BREAKING] Made `supported_types` a required parameter of `AccountComponentMetadata::new()`; removed `with_supported_type`, `with_supported_types`, `with_supports_all_types`, and `with_supports_regular_types` builder methods; added `AccountType::all()` and `AccountType::regular()` helpers ([#2554](https://github.com/0xMiden/protocol/pull/2554)).
- [BREAKING] Migrated to miden-vm 0.21 and miden-crypto 0.22 ([#2508](https://github.com/0xMiden/miden-base/pull/2508)).
Expand Down
100 changes: 74 additions & 26 deletions crates/miden-protocol/src/account/component/code.rs
Original file line number Diff line number Diff line change
@@ -1,33 +1,64 @@
use miden_assembly::Library;
use miden_processor::mast::MastForest;
use alloc::collections::BTreeMap;
use alloc::sync::Arc;
use alloc::vec::Vec;

use miden_assembly::library::ProcedureExport;
use miden_assembly::{Library, Path};
use miden_core::Word;
use miden_core::mast::{MastForest, MastNodeExt};

use crate::vm::AdviceMap;

// ACCOUNT COMPONENT CODE
// ================================================================================================

/// A [`Library`] that has been assembled for use as component code.
/// The code associated with an account component, consisting of a [`MastForest`] and the set of
/// procedures exported by the component.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct AccountComponentCode(Library);
pub struct AccountComponentCode {
mast: Arc<MastForest>,
exports: Vec<ProcedureExport>,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we actually need to store full ProcedureExport here? I guess we use these to build Library out of account component node - but I wonder if that's actually needed.

The alternative could be to follow the pattern we have in NoteScript - e.g., this could look like:

pub struct AccountComponentCode {
    mast: Arc<MastForest>,
    exports: Vec<MastNodeId>,
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently, we have the following pattern for component code:

static BASIC_WALLET_LIBRARY: LazyLock<Library> = ...

pub fn basic_wallet_library() -> Library {
    BASIC_WALLET_LIBRARY.clone()
}

procedure_digest!(
    BASIC_WALLET_RECEIVE_ASSET,
    BasicWallet::NAME,
    BasicWallet::RECEIVE_ASSET_PROC_NAME,
    basic_wallet_library
);

impl BasicWallet {
    pub const NAME: &'static str = "miden::standards::components::wallets::basic_wallet";

    const RECEIVE_ASSET_PROC_NAME: &str = "receive_asset";

    pub fn receive_asset_digest() -> Word {
        *BASIC_WALLET_RECEIVE_ASSET
    }
}

While thinking about a trait AccountComponentInterface, I thought it would make more sense if we would actually use AccountComponentCode instead of Library and make the code accessible on the component itself, so:

static BASIC_WALLET_CODE: LazyLock<AccountComponentCode> = LazyLock::new(|| {
    let bytes = include_bytes!(concat!(
        env!("OUT_DIR"),
        "/assets/account_components/wallets/basic_wallet.masl"
    ));
    let library =
        Library::read_from_bytes(bytes).expect("Shipped Basic Wallet library is well-formed");
    AccountComponentCode::from(library)
});

impl BasicWallet {
    pub fn code() -> &'static AccountComponentCode {
        &BASIC_WALLET_CODE
    }
}

This would also mean changing procedure_digest to take AccountComponentCode as an input instead of a Library, so it would need to have the ability to call get_procedure_root_by_path.

That would be cleaner and increase type safety - we'd get more out of the AccountComponentCode type. Maybe this is still possible with what you proposed. Just mentioning that this would be nice to be able to do eventually and the changes here would have to be compatible with that.


impl AccountComponentCode {
/// Returns a reference to the underlying [`Library`]
pub fn as_library(&self) -> &Library {
&self.0
// CONSTRUCTORS
// --------------------------------------------------------------------------------------------

/// Creates a new [`AccountComponentCode`] from the provided MAST forest and procedure exports.
pub fn new(mast: Arc<MastForest>, exports: Vec<ProcedureExport>) -> Self {
Self { mast, exports }
}

/// Returns a reference to the code's [`MastForest`]
// PUBLIC ACCESSORS
// --------------------------------------------------------------------------------------------

/// Returns a reference to the code's [`MastForest`].
pub fn mast_forest(&self) -> &MastForest {
self.0.mast_forest().as_ref()
self.mast.as_ref()
}

/// Consumes `self` and returns the underlying [`Library`]
pub fn into_library(self) -> Library {
self.0
/// Returns the [`MastForest`] wrapped in an [`Arc`].
pub fn mast(&self) -> Arc<MastForest> {
self.mast.clone()
}

/// Returns a new [AccountComponentCode] with the provided advice map entries merged into the
/// underlying [Library]'s [MastForest].
/// Returns the procedure exports of this component.
pub fn exports(&self) -> &[ProcedureExport] {
&self.exports
}

/// Returns the digest of the procedure with the specified path, or `None` if it was not found
/// in this component.
pub fn get_procedure_root_by_path(&self, path: impl AsRef<Path>) -> Option<Word> {
let path = path.as_ref().to_absolute();
self.exports
.iter()
.find(|export| export.path.as_ref() == path.as_ref())
.map(|export| self.mast[export.node].digest())
}

/// Returns a new [`AccountComponentCode`] with the provided advice map entries merged into the
/// underlying [`MastForest`].
///
/// This allows adding advice map entries to an already-compiled account component,
/// which is useful when the entries are determined after compilation.
Expand All @@ -36,28 +67,46 @@ impl AccountComponentCode {
return self;
}

Self(self.0.with_advice_map(advice_map))
}
}
let mut mast = Arc::unwrap_or_clone(self.mast);
mast.advice_map_mut().extend(advice_map);

impl AsRef<Library> for AccountComponentCode {
fn as_ref(&self) -> &Library {
self.as_library()
Self {
mast: Arc::new(mast),
exports: self.exports,
}
}
}

// CONVERSIONS
// ================================================================================================

impl From<Library> for AccountComponentCode {
fn from(value: Library) -> Self {
Self(value)
fn from(library: Library) -> Self {
let mast = library.mast_forest().clone();
let exports: Vec<ProcedureExport> =
library.exports().filter_map(|export| export.as_procedure().cloned()).collect();

Self { mast, exports }
}
}

impl From<AccountComponentCode> for Library {
fn from(value: AccountComponentCode) -> Self {
value.into_library()
let exports: BTreeMap<_, _> =
value.exports.into_iter().map(|e| (e.path.clone(), e.into())).collect();

Library::new(value.mast, exports)
.expect("AccountComponentCode should have at least one export")
}
}

impl From<&AccountComponentCode> for Library {
fn from(value: &AccountComponentCode) -> Self {
let exports: BTreeMap<_, _> =
value.exports.iter().cloned().map(|e| (e.path.clone(), e.into())).collect();

Library::new(value.mast.clone(), exports)
.expect("AccountComponentCode should have at least one export")
Copy link
Contributor

Choose a reason for hiding this comment

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

I generally think we should not implement conversion using references when the implementation clones. In such cases, I think it's better to make the clone explicit to the caller instead of hiding it, e.g. Library::from(component_code.clone()).

Implementing both APIs could easily lead to a clone even when one actually has an owned value that could be moved.

Copy link
Collaborator Author

@igamigo igamigo Mar 18, 2026

Choose a reason for hiding this comment

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

Yeah, sorry about this one, you've mentioned this before. I think I was mostly focused in keeping the API close to what it was before (mainly for linking in CodeBuilder)

}
}

Expand All @@ -82,10 +131,9 @@ mod tests {
assert!(component_code.mast_forest().advice_map().is_empty());

// Empty advice map should be a no-op (digest stays the same)
let cloned = component_code.clone();
let original_digest = cloned.as_library().digest();
let original_digest = *Library::from(component_code.clone()).digest();
let component_code = component_code.with_advice_map(AdviceMap::default());
assert_eq!(original_digest, component_code.as_library().digest());
assert_eq!(&original_digest, Library::from(component_code.clone()).digest());

// Non-empty advice map should add entries
let key = Word::from([10u32, 20, 30, 40]);
Expand Down
31 changes: 13 additions & 18 deletions crates/miden-protocol/src/account/component/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,31 +196,26 @@ impl AccountComponent {
self.metadata.supported_types().contains(&account_type)
}

/// Returns a vector of tuples (digest, is_auth) for all procedures in this component.
/// Returns an iterator over (digest, is_auth) for all procedures in this component.
///
/// A procedure is considered an authentication procedure if it has the `@auth_script`
/// attribute.
pub fn get_procedures(&self) -> Vec<(Word, bool)> {
let library = self.code.as_library();
let mut procedures = Vec::new();
for export in library.exports() {
if let Some(proc_export) = export.as_procedure() {
let digest = library
.mast_forest()
.get_node_by_id(proc_export.node)
.expect("export node not in the forest")
.digest();
let is_auth = proc_export.attributes.has(AUTH_SCRIPT_ATTRIBUTE);
procedures.push((digest, is_auth));
}
}
procedures
pub fn get_procedures(&self) -> impl Iterator<Item = (Word, bool)> + '_ {
let mast = self.code.mast_forest();
self.code.exports().iter().map(move |proc_export| {
let digest = mast
.get_node_by_id(proc_export.node)
.expect("export node not in the forest")
.digest();
let is_auth = proc_export.attributes.has(AUTH_SCRIPT_ATTRIBUTE);
(digest, is_auth)
})
}

/// Returns the digest of the procedure with the specified path, or `None` if it was not found
/// in this component's library or its library path is malformed.
/// in this component.
pub fn get_procedure_root_by_path(&self, proc_name: impl AsRef<Path>) -> Option<Word> {
self.code.as_library().get_procedure_root_by_path(proc_name)
self.code.get_procedure_root_by_path(proc_name)
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/miden-protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub use protocol::ProtocolLib;
pub mod assembly {
pub use miden_assembly::ast::{Module, ModuleKind, ProcedureName, QualifiedProcedureName};
pub use miden_assembly::debuginfo::SourceManagerSync;
pub use miden_assembly::library::LibraryExport;
pub use miden_assembly::library::{LibraryExport, ProcedureExport as LibraryProcedureExport};
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason that we need this rename? I think, ideally, we'd keep the original name as having two names for the same thing often leads to confusion and I recall having IDE import problems with the Hasher type (which is also renamed and I wish we would remove that).

pub use miden_assembly::{
Assembler,
DefaultSourceManager,
Expand Down
9 changes: 5 additions & 4 deletions crates/miden-standards/src/code_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ use crate::standards_lib::StandardsLib;
/// let script = CodeBuilder::default()
/// .with_linked_module("my::module", module_code).context("failed to link module")?
/// .with_statically_linked_library(&my_lib).context("failed to link static library")?
/// .with_dynamically_linked_library(&fpi_lib).context("failed to link dynamic library")? // For FPI calls
/// .with_dynamically_linked_library(fpi_lib).context("failed to link dynamic library")? // For FPI calls
/// .compile_tx_script(script_code).context("failed to parse tx script")?;
/// # Ok(())
/// # }
Expand Down Expand Up @@ -223,9 +223,10 @@ impl CodeBuilder {
/// Returns an error if the library cannot be added to the assembler
pub fn with_dynamically_linked_library(
mut self,
library: impl AsRef<Library>,
library: impl Into<Library>,
) -> Result<Self, CodeBuilderError> {
self.link_dynamic_library(library.as_ref())?;
let library = library.into();
self.link_dynamic_library(&library)?;
Ok(self)
}

Expand Down Expand Up @@ -697,7 +698,7 @@ mod tests {
let builder = CodeBuilder::default()
.with_statically_linked_library(&static_lib)
.context("failed to link static library")?
.with_dynamically_linked_library(&dynamic_lib)
.with_dynamically_linked_library(dynamic_lib)
.context("failed to link dynamic library")?;

builder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ static INCR_NONCE_AUTH_LIBRARY: LazyLock<Library> = LazyLock::new(|| {
CodeBuilder::default()
.compile_component_code("incr_nonce", INCR_NONCE_AUTH_CODE)
.expect("incr nonce code should be valid")
.into_library()
.into()
});

/// Creates a mock authentication [`AccountComponent`] for testing purposes under the "incr_nonce"
Expand Down
4 changes: 2 additions & 2 deletions crates/miden-standards/src/testing/mock_account_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,14 @@ static MOCK_FAUCET_LIBRARY: LazyLock<Library> = LazyLock::new(|| {
CodeBuilder::default()
.compile_component_code("mock::faucet", MOCK_FAUCET_CODE)
.expect("mock faucet code should be valid")
.into_library()
.into()
});

static MOCK_ACCOUNT_LIBRARY: LazyLock<Library> = LazyLock::new(|| {
CodeBuilder::default()
.compile_component_code("mock::account", MOCK_ACCOUNT_CODE)
.expect("mock account code should be valid")
.into_library()
.into()
});

// MOCK ACCOUNT CODE EXT
Expand Down
2 changes: 1 addition & 1 deletion crates/miden-testing/src/kernel_tests/tx/test_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,7 @@ async fn transaction_executor_account_code_using_custom_library() -> anyhow::Res
.build_existing()?;

let tx_script = CodeBuilder::default()
.with_dynamically_linked_library(&account_component_lib)?
.with_dynamically_linked_library(account_component_lib.clone())?
.compile_tx_script(tx_script_src)?;

let tx_context = TransactionContextBuilder::new(native_account.clone())
Expand Down
4 changes: 2 additions & 2 deletions crates/miden-testing/src/kernel_tests/tx/test_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ async fn test_array_get_and_set() -> anyhow::Result<()> {

// Compile the transaction script with the wrapper library linked
let tx_script = CodeBuilder::default()
.with_dynamically_linked_library(&wrapper_library)?
.with_dynamically_linked_library(wrapper_library.clone())?
.compile_tx_script(tx_script_code)?;

// Create transaction context and execute
Expand Down Expand Up @@ -243,7 +243,7 @@ async fn test_double_word_array_get_and_set() -> anyhow::Result<()> {
);

let tx_script = CodeBuilder::default()
.with_dynamically_linked_library(&wrapper_library)?
.with_dynamically_linked_library(wrapper_library.clone())?
.compile_tx_script(tx_script_code)?;

let tx_context = TransactionContextBuilder::new(account).tx_script(tx_script).build()?;
Expand Down
2 changes: 1 addition & 1 deletion crates/miden-testing/src/kernel_tests/tx/test_tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,7 @@ async fn inputs_created_correctly() -> anyhow::Result<()> {
"#;

let tx_script = CodeBuilder::default()
.with_dynamically_linked_library(component_code.as_library())?
.with_dynamically_linked_library(component_code)?
.compile_tx_script(script)?;

assert!(tx_script.mast().advice_map().get(&Word::try_from([1u64, 2, 3, 4])?).is_some());
Expand Down
Loading