Skip to content

feat: implement AccountComponentBuilder#1734

Closed
PhilippGackstatter wants to merge 2 commits intonextfrom
pgackst-account-component-builder
Closed

feat: implement AccountComponentBuilder#1734
PhilippGackstatter wants to merge 2 commits intonextfrom
pgackst-account-component-builder

Conversation

@PhilippGackstatter
Copy link
Copy Markdown
Contributor

I've asked claude to implement an AccountComponentBuilder to remove AccountComponent::compile and as a parallel to ScriptBuilder, as an idea for fixing 0xMiden/miden-client#1146 (comment).

Should we go ahead with something like this? I haven't reviewed the PR, but the approach seems good. If we want to go ahead, I'll clean up the PR and make it ready for a proper review.

@partylikeits1983
Copy link
Copy Markdown
Member

This looks good, I think it would match the overall style of following the builder pattern across miden-base. Also one of the pioneers was facing issues with the AccountComponent::compile method, this would make things a bit more intuitive.

Comment on lines +77 to +82
#[derive(Clone)]
pub struct AccountComponentBuilder {
assembler: Assembler,
storage_slots: Vec<StorageSlot>,
supported_types: BTreeSet<AccountType>,
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thinking about it some more, I think this is fine if the goal is to only replace AccountComponent::compile. I'm not sure if this eliminates the need for users to use the TransactionKernel::assembler, though. Basically, I'm wondering how end users are creating account components.

The naive approach would be something like:

const COMPONENT_SOURCE_CODE: &str = "export.myprocedure nop end"

pub struct MyAccountComponent {
    storage_slot: StorageSlot,
}

impl MyAccountComponent {
    pub fn new(...) -> Result<Self, MyError> {
        // Validate whatever needs to be validated.
        Self { ... }
    }
}

impl From<MyAccountComponent> for AccountComponent {
    fn from(my_component: MyAccountComponent) -> Self {
        AccountComponentBuilder::default()
            .with_storage_slot(my_component.storage_slot)
            .with_supported_type(AccountType::RegularAccountUpdatableCode)
            .build(COMPONENT_SOURCE_CODE)
            .expect("my account component should satisfy the requirements of a valid account component");
    }
}

That is, the source code is passed to the builder directly. But if I was a user, I would probably want to use a similar approach as we do for assembling the library statically in a LazyLock, e.g.:

static COMPONENT_LIBRARY: LazyLock<Library> = LazyLock::new(|| {
    let source = NamedSource::new("my::account", COMPONENT_SOURCE_CODE);
    TransactionKernel::assembler()
        .assemble_library([source])
        .expect("my account code should be valid")
});

impl From<MyAccountComponent> for AccountComponent {
    fn from(my_component: MyAccountComponent) -> Self {
        AccountComponentBuilder::default()
            .with_storage_slot(my_component.storage_slot)
            .with_supported_type(AccountType::RegularAccountUpdatableCode)
            .build(COMPONENT_LIBRARY.clone())
            .expect("my account component should satisfy the requirements of a valid account component");
    }
}

That way, the library is only assembled once and this approach also supports building the components in a build.rs script like we do, which is better to catch assembly errors at build time.

Note how this still uses an Assembler directly. So if we want to support this last approach, I think we may need something that builds account component Librarys. We can have this be a separate type altogether, but its API would be very similar to the ScriptBuilder and so this sounds like an opportunity to deduplicate code. That functionality could be implemented by adding a ScriptBuilder::compile_library. The thing is that this would expand the scope of the ScriptBuilder beyond scripts and make it a bit more like the general-purpose Assembler, so I think we'd need a new name for it. Maybe a generic CodeBuilder or ProtocolAssembler to reflect the fact that it's basically a tailor-made Assembler for the Miden protocol.

With that, it could still make sense to have an AccountComponentBuilder, but instead of assembling code, it would just take a Library. Might make constructing AccountComponents a bit more convenient, but that's then a somewhat separate discussion. Any thoughts @partylikeits1983, @mmagician?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

by adding a ScriptBuilder::compile_library. The thing is that this would expand the scope of the ScriptBuilder beyond scripts and make it a bit more like the general-purpose Assembler, so I think we'd need a new name for it. Maybe a generic CodeBuilder or ProtocolAssembler to reflect the fact that it's basically a tailor-made Assembler for the Miden protocol.

I think this is a good idea since it abstracts away the assembler. The most important part, is that the account component builder should be able to take a library path param, i.e. a NamedSource. Adding compile_library to the ScriptBuilder is a great idea. I wrote similar functionality (not in a builder pattern) in the miden-client-tools repo.

A good name might be CodeBuilder, ProtocolAssembler, or ModuleBuilder

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess the main difference between AccountComponentBuilder and ScriptBuilder is that the latter is just for assembling script programs while the latter is going beyond that. That is ScriptBuilder does try to create a full Note object - but just the NoteScript.

So, it does seem like maybe the current AccountComponentBuilder is mixing two things together:

  1. Assembling the library for the component.
  2. Building the actual component.

Combining the two into a single struct maybe a bit difficult because we could build account components without the need to assemble the library (e.g., in cases when the library has been previously assembled and we just need to deserialize it).

We could create something like AccountComponentCode(Library) struct (and the corresponding AccountComponentCodeBuilder) but this may be an overkill.

At the same time, I'm not sure yet about creating a single struct that handles all assembly needs (it may be a good idea, but I haven't thought it through yet). Seems like additional benefit over exposing the original assembler is not big - but maybe that's fine. I guess we could even call this struct just Assembler and the main difference from miden-vm assembler would be that we pre-load kernel into it and have compile_tx_script(), compile_note_script(), and compile_component_code() methods on it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Generally agreed, I also wouldn't mix assembling and building components in the same type. (That's basically what claude code came up with when I told it to replace AccountComponent::compile with a ScriptBuilder-like interface).

Seems like additional benefit over exposing the original assembler is not big - but maybe that's fine. I guess we could even call this struct just Assembler and the main difference from miden-vm assembler would be that we pre-load kernel into it and have compile_tx_script(), compile_note_script(), and compile_component_code() methods on it?

Yeah, agreed. I still think the ScriptBuilder is a good idea as it provides a minimal, but sufficiently powerful API for the assembly needs of the protocol, while the VM Assembler goes a bit beyond that (like having the ability to assemble kernels). So having compile_tx_script, compile_note_script, and compile_component_code does seem like a very nice API that should cover everything.

I think I like the AccountComponentCode wrapper so that compile_component_code returns that instead of the generic Library, which would seem a bit at odds with its name (otherwise it should be called compile_library - but I do like the more specific compile_component_code).

Regarding naming:

  • I think I wouldn't call it Assembler, just to avoid confusion with the VM one. We probably no longer would expose the VM Assembler in miden-objects, so the chance for confusion is minimized, but still not sure it's a good idea.
  • TransactionAssembler in theory fits because all scripts and libraries it assembles are assembled against the tx kernel, but the first association is that it assembles transactions. So TxKernelAssembler is a bit better, but a bit verbose.
  • ProtocolAssembler is not amazing, but the one I like most, since it's a bit more precise than the generic CodeBuilder.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For the name, I probably would prefer TransactionAssembler over others (TxKernelAssembler sounds like it is for assembling transaction kernels, and similarly) - but also, don't really love it.

@PhilippGackstatter
Copy link
Copy Markdown
Contributor Author

Opened #1756 to cover the part for assembling account component code. After that, we can still think about having an AccountComponentBuilder that doesn't do any assembly itself. Closing this PR accordingly.

@PhilippGackstatter PhilippGackstatter deleted the pgackst-account-component-builder branch August 26, 2025 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants