-
Notifications
You must be signed in to change notification settings - Fork 145
feat: migrate transaction scripts to Library #3173
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 all commits
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 |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ use miden_mast_package::Package; | |
|
|
||
| use super::{Felt, Hasher, Word}; | ||
| use crate::account::auth::{PublicKeyCommitment, Signature}; | ||
| use crate::assembly::Library; | ||
| use crate::errors::TransactionScriptError; | ||
| use crate::note::{NoteId, NoteRecipient}; | ||
| use crate::utils::serde::{ | ||
|
|
@@ -318,6 +319,9 @@ impl Deserializable for TransactionScriptRoot { | |
| // TRANSACTION SCRIPT | ||
| // ================================================================================================ | ||
|
|
||
| /// The attribute name used to mark the entrypoint procedure in a transaction script library. | ||
| const TRANSACTION_SCRIPT_ATTRIBUTE: &str = "transaction_script"; | ||
|
|
||
| /// Transaction script. | ||
| /// | ||
| /// A transaction script is a program that is executed in a transaction after all input notes | ||
|
|
@@ -336,6 +340,8 @@ impl TransactionScript { | |
| // -------------------------------------------------------------------------------------------- | ||
|
|
||
| /// Returns a new [TransactionScript] instantiated with the provided code. | ||
| // TODO: we can remove this `Program` based constructor once the compiler integrates the | ||
| // `@transaction_script` attribute (https://github.com/0xMiden/compiler/issues/1190). | ||
| pub fn new(code: Program) -> Self { | ||
| Self::from_parts(code.mast_forest().clone(), code.entrypoint()) | ||
| } | ||
|
|
@@ -364,6 +370,37 @@ impl TransactionScript { | |
| Ok(TransactionScript::new(program)) | ||
| } | ||
|
|
||
| /// Returns a new [TransactionScript] instantiated from the provided library. | ||
| /// | ||
| /// The library must contain exactly one procedure with the `@transaction_script` attribute, | ||
| /// which will be used as the entrypoint. | ||
| /// | ||
| /// # Errors | ||
| /// Returns an error if: | ||
| /// - The library does not contain a procedure with the `@transaction_script` attribute. | ||
| /// - The library contains multiple procedures with the `@transaction_script` attribute. | ||
| pub fn from_library(library: &Library) -> Result<Self, TransactionScriptError> { | ||
| let mut entrypoint = None; | ||
|
|
||
| for export in library.exports() { | ||
| if let Some(proc_export) = export.as_procedure() | ||
| && proc_export.attributes.has(TRANSACTION_SCRIPT_ATTRIBUTE) | ||
| { | ||
| if entrypoint.is_some() { | ||
| return Err(TransactionScriptError::MultipleProceduresWithAttribute); | ||
|
Comment on lines
+382
to
+390
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. Since we error on multiple procedures with that attribute, I'd also add |
||
| } | ||
| entrypoint = Some(proc_export.node); | ||
| } | ||
| } | ||
|
|
||
| let entrypoint = entrypoint.ok_or(TransactionScriptError::NoProcedureWithAttribute)?; | ||
|
|
||
| Ok(Self { | ||
| mast: library.mast_forest().clone(), | ||
| entrypoint, | ||
| }) | ||
| } | ||
|
|
||
| // PUBLIC ACCESSORS | ||
| // -------------------------------------------------------------------------------------------- | ||
|
|
||
|
|
@@ -461,4 +498,48 @@ mod tests { | |
| let stored = mast.advice_map().get(&key).expect("entry should be present"); | ||
| assert_eq!(stored.as_ref(), value.as_slice()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_transaction_script_from_library() { | ||
| use assert_matches::assert_matches; | ||
|
|
||
| use super::TransactionScript; | ||
| use crate::assembly::Assembler; | ||
| use crate::errors::TransactionScriptError; | ||
| use crate::utils::serde::{Deserializable, Serializable}; | ||
|
|
||
| let source = " | ||
| @transaction_script | ||
| pub proc main | ||
| push.1 drop | ||
| end | ||
| "; | ||
| let library = Assembler::default().assemble_library([source]).unwrap(); | ||
|
|
||
| let script = TransactionScript::from_library(&library).unwrap(); | ||
|
|
||
| // the script must round-trip through serialization unchanged | ||
| let bytes = script.to_bytes(); | ||
| let decoded = TransactionScript::read_from_bytes(&bytes).unwrap(); | ||
| assert_eq!(script, decoded); | ||
|
|
||
| // a library without the attribute is rejected | ||
| let no_attr = Assembler::default() | ||
| .assemble_library(["pub proc main push.1 drop end"]) | ||
| .unwrap(); | ||
| assert_matches!( | ||
| TransactionScript::from_library(&no_attr), | ||
| Err(TransactionScriptError::NoProcedureWithAttribute) | ||
| ); | ||
|
|
||
| // a library with multiple tagged procedures is rejected | ||
| let multiple = Assembler::default() | ||
| .assemble_library(["@transaction_script pub proc main_a push.1 drop end | ||
| @transaction_script pub proc main_b push.2 drop end"]) | ||
| .unwrap(); | ||
| assert_matches!( | ||
| TransactionScript::from_library(&multiple), | ||
| Err(TransactionScriptError::MultipleProceduresWithAttribute) | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,7 +63,7 @@ use crate::standards_lib::StandardsLib; | |
| /// # use miden_protocol::CoreLibrary; | ||
| /// # fn example() -> anyhow::Result<()> { | ||
| /// # let module_code = "pub proc test push.1 add end"; | ||
| /// # let script_code = "begin nop end"; | ||
| /// # let script_code = "@transaction_script pub proc main nop end"; | ||
| /// # // Create sample libraries for the example | ||
| /// # let my_lib: Library = CoreLibrary::default().into(); // Convert CoreLibrary to Library | ||
| /// # let fpi_lib: Library = CoreLibrary::default().into(); | ||
|
|
@@ -293,20 +293,6 @@ impl CodeBuilder { | |
| // PRIVATE HELPERS | ||
| // -------------------------------------------------------------------------------------------- | ||
|
|
||
| /// Applies the advice map to a program if it's non-empty. | ||
| /// | ||
| /// This avoids cloning the MAST forest when there are no advice map entries. | ||
| fn apply_advice_map( | ||
| advice_map: AdviceMap, | ||
| program: miden_protocol::vm::Program, | ||
| ) -> miden_protocol::vm::Program { | ||
| if advice_map.is_empty() { | ||
| program | ||
| } else { | ||
| program.with_advice_map(advice_map) | ||
| } | ||
| } | ||
|
|
||
| /// Applies the advice map to a library if it's non-empty. | ||
| /// | ||
| /// This avoids cloning the MAST forest when there are no advice map entries. | ||
|
|
@@ -363,7 +349,8 @@ impl CodeBuilder { | |
| /// The parsed script will have access to all modules that have been added to this builder. | ||
| /// | ||
| /// # Arguments | ||
| /// * `tx_script` - The transaction script source code | ||
| /// - `tx_script` - the transaction script source code which is expected to have a single public | ||
| /// procedure marked with the @transaction_script attribute. | ||
| /// | ||
| /// # Errors | ||
| /// Returns an error if: | ||
|
|
@@ -374,11 +361,23 @@ impl CodeBuilder { | |
| ) -> Result<TransactionScript, CodeBuilderError> { | ||
| let CodeBuilder { assembler, advice_map, .. } = self; | ||
|
|
||
| let program = assembler.assemble_program(tx_script).map_err(|err| { | ||
| CodeBuilderError::build_error_with_report("failed to parse transaction script", err) | ||
| let tx_script_lib = assembler.assemble_library([tx_script]).map_err(|err| { | ||
| CodeBuilderError::build_error_with_report( | ||
| "failed to parse transaction script library", | ||
| err, | ||
| ) | ||
| })?; | ||
|
|
||
| Ok(TransactionScript::new(Self::apply_advice_map(advice_map, program))) | ||
| TransactionScript::from_library(&Self::apply_advice_map_to_library( | ||
| advice_map, | ||
| Arc::unwrap_or_clone(tx_script_lib), | ||
| )) | ||
| .map_err(|err| { | ||
| CodeBuilderError::build_error_with_source( | ||
| "failed to create transaction script from library", | ||
| err, | ||
| ) | ||
| }) | ||
| } | ||
|
|
||
| /// Compiles the provided MASM code into a [`NoteScript`]. | ||
|
|
@@ -525,7 +524,7 @@ mod tests { | |
| fn test_code_builder_basic_script_compiling() -> anyhow::Result<()> { | ||
| let builder = CodeBuilder::default(); | ||
| builder | ||
| .compile_tx_script("begin nop end") | ||
| .compile_tx_script("@transaction_script pub proc main nop end") | ||
| .context("failed to parse basic tx script")?; | ||
| Ok(()) | ||
| } | ||
|
|
@@ -535,7 +534,8 @@ mod tests { | |
| let script_code = " | ||
| use external_contract::counter_contract | ||
|
|
||
| begin | ||
| @transaction_script | ||
| pub proc main | ||
| call.counter_contract::increment | ||
| end | ||
| "; | ||
|
|
@@ -573,7 +573,8 @@ mod tests { | |
| let script_code = " | ||
| use external_contract::counter_contract | ||
|
|
||
| begin | ||
| @transaction_script | ||
| pub proc main | ||
| call.counter_contract::increment | ||
| end | ||
| "; | ||
|
|
@@ -624,7 +625,8 @@ mod tests { | |
| let script_code = " | ||
| use external_contract::counter_contract | ||
|
|
||
| begin | ||
| @transaction_script | ||
| pub proc main | ||
| call.counter_contract::increment | ||
| end | ||
| "; | ||
|
|
@@ -656,8 +658,7 @@ mod tests { | |
|
|
||
| #[test] | ||
| fn test_multiple_chained_modules() -> anyhow::Result<()> { | ||
| let script_code = | ||
| "use test::lib1 use test::lib2 begin exec.lib1::test1 exec.lib2::test2 end"; | ||
| let script_code = "use test::lib1 use test::lib2 @transaction_script pub proc main exec.lib1::test1 exec.lib2::test2 end"; | ||
|
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. nit: consider making this a multi-line string |
||
|
|
||
| // Test chaining multiple modules | ||
| let builder = CodeBuilder::default() | ||
|
|
@@ -676,7 +677,8 @@ mod tests { | |
| let script_code = " | ||
| use contracts::static_contract | ||
|
|
||
| begin | ||
| @transaction_script | ||
| pub proc main | ||
| call.static_contract::increment_1 | ||
| end | ||
| "; | ||
|
|
@@ -732,7 +734,7 @@ mod tests { | |
|
|
||
| let script = CodeBuilder::default() | ||
| .with_advice_map_entry(key, value.clone()) | ||
| .compile_tx_script("begin nop end") | ||
| .compile_tx_script("@transaction_script pub proc main nop end") | ||
| .context("failed to compile tx script with advice map")?; | ||
|
|
||
| let mast = script.mast(); | ||
|
|
@@ -753,7 +755,7 @@ mod tests { | |
|
|
||
| let script = CodeBuilder::default() | ||
| .with_extended_advice_map(advice_map) | ||
| .compile_tx_script("begin nop end") | ||
| .compile_tx_script("@transaction_script pub proc main nop end") | ||
| .context("failed to compile tx script")?; | ||
|
|
||
| let mast = script.mast(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,8 @@ use miden::protocol::tx | |
| #! - delta is 0 or not a u32 in the range 1..=0xFFFF (ERR_TX_INVALID_EXPIRATION_DELTA). | ||
| #! | ||
| #! Invocation: call | ||
| begin | ||
| @transaction_script | ||
| pub proc main | ||
|
Comment on lines
+25
to
+26
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. Not for this PR, but we should move this out into a package (similar to how we do with notes). |
||
| exec.tx::update_expiration_block_delta | ||
| # => [pad(16)] | ||
| end | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,8 @@ use crate::errors::CodeBuilderError; | |
| /// [`FungibleFaucet`]: | ||
| /// | ||
| /// ```masm | ||
| /// begin | ||
| /// @transaction_script | ||
| /// pub proc main | ||
| /// push.{expiration_delta} exec.::miden::protocol::tx::update_expiration_block_delta | ||
| /// | ||
| /// push.{note information} | ||
|
|
@@ -101,7 +102,8 @@ impl SendNotesTransactionScript { | |
| return Err(SendNotesTransactionScriptError::UnsupportedAccountInterface); | ||
| }; | ||
|
|
||
| let script = format!("begin\n{expiration_prelude}\n{body}\nend"); | ||
| let script = | ||
| format!("@transaction_script\npub proc main\n{expiration_prelude}\n{body}\nend"); | ||
|
Comment on lines
+105
to
+106
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. nit: instead of using |
||
|
|
||
| let mut code_builder = CodeBuilder::new(); | ||
| for note in output_notes { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do this in a follow up right after this PR. And we should do the same for note scripts.
cc @greenhat for visibility.