feat: allow building account code with `ScriptBuilder` by igamigo · Pull Request #2142 · 0xMiden/protocol

igamigo · 2025-12-05T19:05:41Z

Adds AccountComponentCode, wrapper of Library
Adds capability of building AccountComponentCode with ScriptBuilder
Attempts to remove all occurrences of TransactionKernel::assembler(). Some of them were not doable for different reasons, though mainly in tests (assembling Program, tests located in miden-objects, etc.)
Does minor refactors (move code that was out of place)
Renames ScriptBuilder to CodeBuilder

~~I think ProtocolAssembler is the most correct of all the alternatives that were discussed throughout the issues. However, I realize this might not be ideal so I'm more than open to changing this.~~

igamigo · 2025-12-05T19:25:55Z

crates/miden-objects/src/account/component/template/mod.rs

+// CONVERSIONS
+// ================================================================================================
+
+impl TryFrom<&Package> for AccountComponentMetadata {


Moved this since it seems better placed here

igamigo · 2025-12-07T20:47:03Z

crates/miden-objects/src/account/code/mod.rs

+        let library = Assembler::default().assemble_library([CODE]).unwrap();
+        let component = AccountComponent::new(library, vec![]).unwrap().with_supports_all_types();


I removed AccountComponent::compile, but because these tests live in miden-objects we can't get around using Assembler

PhilippGackstatter

Looks good to me!

I think the main comments I have are around:

Naming ProtocolAssembler
Whether inserting (mock) libraries in TxContext is necessary
Renaming compile_* methods to parse_*
Removing TransactionKernel::with* testing APIs.

crates/miden-lib/src/account/auth/ecdsa_k256_keccak.rs

crates/miden-lib/src/testing/mock_account_code.rs

crates/miden-lib/src/utils/protocol_assembler.rs

PhilippGackstatter · 2025-12-11T02:09:03Z

crates/miden-lib/src/account/interface/test.rs

-    .unwrap()
-    .with_supports_all_types();
+    let account_code =
+        ProtocolAssembler::with_kernel_library(Arc::new(DefaultSourceManager::default()))


I know you just kept the previous functionality, but this should work with just ProtocolAssembler::default since the kernel library is never accessed in this module.

The same should largely be true for test_fpi.rs where we typically construct actual transactions that cannot access the $kernel library, and so we should be able to just use default. Does not apply whenever $kernel is used.

PhilippGackstatter · 2025-12-11T02:12:43Z

crates/miden-lib/src/utils/code_builder.rs

+    #[cfg(any(feature = "testing", test))]
+    pub fn with_kernel_library(source_manager: Arc<dyn SourceManagerSync>) -> Self {


Nit: I would basically add docs which are mostly a copy of TransactionKernel::with_kernel_library (or link to it?)

crates/miden-testing/src/kernel_tests/tx/test_tx.rs

PhilippGackstatter · 2025-12-11T02:36:03Z

crates/miden-testing/src/tx_context/context.rs

        self.mast_store.insert(TransactionKernel::library().mast_forest().clone());
+        self.mast_store.insert(miden_lib::StdLibrary::default().mast_forest().clone());
+        self.mast_store.insert(miden_lib::MidenLib::default().mast_forest().clone());
+        self.mast_store
+            .insert(AccountCode::mock_account_library().mast_forest().clone());
+        self.mast_store.insert(AccountCode::mock_faucet_library().mast_forest().clone());
        self.mast_store.insert(program.mast_forest().clone());


Why did this work without the stdlib and miden lib before?

Do we really have to insert the mock account and faucet libraries? These should only be relevant when executed against a mock account, in which case the account's code itself will bring these libraries and so no dynamic lookup should be required:

https://github.com/0xMiden/miden-base/blob/a5b4b5bf65ee5153a7412873914e6d8b4f04288b/crates/miden-lib/src/testing/account_component/mock_account_component.rs#L55-L61

I think this was just a change I made while the refactors were still in progress and which I did not revisit later. Removed

I think these are still present in the latest version.

PhilippGackstatter · 2025-12-11T02:47:16Z

crates/miden-lib/src/utils/code_builder.rs

 use crate::transaction::TransactionKernel;

-// SCRIPT BUILDER


I wonder if we should also move TransactionKernel::mock_libraries here? I think we only had it there because it was the best place at the time, but they are not very related to the tx kernel. I think ProtocolAssembler::mock_libraries would make a bit more sense.

Related to this, it would be really nice if we could get rid of TransactionKernel::{with_kernel_library,with_mock_libraries} and replace all of those with ProtocolAssembler. Basically to not have to maintain a "transaction kernel assembler" even for testing purposes and force ourselves to use the ProtocolAssembler.

TransactionKernel::with_kernel_library is needed in CodeExecutor::run to assemble a Program.

TransactionKernel::with_mock_libraries is needed in TransactionContext::execute_code to assemble a Program.

I think we can either do this by constructing an Assembler and add the libraries a bit more manually, or by adding a From<ProtocolAssembler> for Assembler and then do ProtocolAssembler::with*, convert into Assembler and assemble the Programs for the above use cases. I would try the latter.

bobbinth

Looks good! Thank you! Not a super deep review, but I left a few small comments inline.

Also, I do actually think that compile_ was better than parse_ as we are taking textual representation of code and transforming it into a binary representation. But not a strong preference, and I'm fine keeping it as is.

crates/miden-objects/src/account/component/code.rs

crates/miden-lib/src/utils/code_builder.rs

crates/miden-lib/src/account/interface/test.rs

PhilippGackstatter

Looks great!

As for using compile or parse, my preference is slightly on the latter, but happy to go with the majority.

PhilippGackstatter · 2025-12-12T04:28:02Z

crates/miden-testing/src/tx_context/context.rs

        self.mast_store.insert(TransactionKernel::library().mast_forest().clone());
+        self.mast_store.insert(miden_lib::StdLibrary::default().mast_forest().clone());
+        self.mast_store.insert(miden_lib::MidenLib::default().mast_forest().clone());
+        self.mast_store
+            .insert(AccountCode::mock_account_library().mast_forest().clone());
+        self.mast_store.insert(AccountCode::mock_faucet_library().mast_forest().clone());
        self.mast_store.insert(program.mast_forest().clone());


I think these are still present in the latest version.

PhilippGackstatter · 2025-12-12T04:31:45Z

crates/miden-testing/src/tx_context/context.rs

+        let assembler = assembler.with_debug_mode(true);
+
        let program = assembler
            .with_debug_mode(true)


Nit: We seem to enable debug mode twice.

bobbinth · 2025-12-12T23:00:03Z

I tried to resolve the merge conflict - but seems like it broke things (I think there are some new tests that use the old ScriptBuilder). I can fix it later tonight unless @igamigo you'll get to it first.

igamigo · 2025-12-12T23:11:31Z

I tried to resolve the merge conflict - but seems like it broke things (I think there are some new tests that use the old ScriptBuilder). I can fix it later tonight unless @igamigo you'll get to it first.

Fixed!

bobbinth · 2025-12-13T17:28:35Z

I've merged this PR, but I'm not 100% all of @PhilippGackstatter comments have been addressed. @igamigo - could you double check? And if anything slipped through the cracks, we can address that in a follow-up PR.

igamigo · 2025-12-13T21:39:39Z

I've merged this PR, but I'm not 100% all of @PhilippGackstatter comments have been addressed. @igamigo - could you double check? And if anything slipped through the cracks, we can address that in a follow-up PR.

Opened #2175. I believe everything else was addressed.

igamigo commented Dec 5, 2025

View reviewed changes

igamigo force-pushed the igamigo-acc-code-builder branch 2 times, most recently from 681d2ef to 91fc33f Compare December 7, 2025 19:32

igamigo commented Dec 7, 2025

View reviewed changes

igamigo force-pushed the igamigo-acc-code-builder branch 2 times, most recently from 9074789 to ed37996 Compare December 9, 2025 17:39

feat: allow building account code with scriptbuilder

d6601f6

igamigo force-pushed the igamigo-acc-code-builder branch from e5fb980 to d6601f6 Compare December 10, 2025 18:51

igamigo marked this pull request as ready for review December 10, 2025 18:53

igamigo requested a review from PhilippGackstatter December 10, 2025 18:53

igamigo added 2 commits December 10, 2025 16:07

chore: fix test

379ddeb

chore: lints

5fafeb3

PhilippGackstatter reviewed Dec 11, 2025

View reviewed changes

igamigo added 4 commits December 11, 2025 11:12

reviews: rename to parse_*, inline doc comment link

38b422c

reviews: move mock libraries helper functions; remove assembler usage

69638ef

reviews: rename to CodeBuilder

3e10b43

Merge branch 'next' into igamigo-acc-code-builder

200a7b2

igamigo mentioned this pull request Dec 11, 2025

Implement CodeBuilder bindings 0xMiden/miden-client#1615

Closed

igamigo added 2 commits December 11, 2025 16:43

reviews: missing rename, use impl Parse

bbc657a

chore: merge

de28b8d

bobbinth approved these changes Dec 11, 2025

View reviewed changes

crates/miden-objects/src/account/component/code.rs Show resolved Hide resolved

crates/miden-lib/src/utils/code_builder.rs Show resolved Hide resolved

crates/miden-lib/src/utils/code_builder.rs Outdated Show resolved Hide resolved

partylikeits1983 reviewed Dec 12, 2025

View reviewed changes

crates/miden-lib/src/account/interface/test.rs Outdated Show resolved Hide resolved

PhilippGackstatter approved these changes Dec 12, 2025

View reviewed changes

igamigo mentioned this pull request Dec 12, 2025

Deconstruct AccountComponentCode's library into components #2174

Open

igamigo and others added 2 commits December 12, 2025 18:16

reviews: back to compile

ca3ee35

Merge branch 'next' into igamigo-acc-code-builder

5501a9f

test: fix test after merge

16490b0

bobbinth merged commit 7ffb33a into next Dec 13, 2025
17 checks passed

bobbinth deleted the igamigo-acc-code-builder branch December 13, 2025 17:25

igamigo mentioned this pull request Dec 13, 2025

chore: CodeBuilder followups #2175

Merged

		let library = Assembler::default().assemble_library([CODE]).unwrap();
		let component = AccountComponent::new(library, vec![]).unwrap().with_supports_all_types();

		#[cfg(any(feature = "testing", test))]
		pub fn with_kernel_library(source_manager: Arc<dyn SourceManagerSync>) -> Self {

Conversation

igamigo commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PhilippGackstatter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PhilippGackstatter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobbinth commented Dec 12, 2025

Uh oh!

igamigo commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

bobbinth commented Dec 13, 2025

Uh oh!

igamigo commented Dec 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

igamigo commented Dec 5, 2025 •

edited

Loading

igamigo commented Dec 12, 2025 •

edited

Loading