feat: add Package support in TransactionScript#2779
Conversation
lima-limon-inc
force-pushed
the
fabrizioorsi/i2111-tx-script-from-package
branch
from
23870ff to
c838b8d
Compare
| if !matches!(package_kind, TargetType::TransactionScript) { | ||
| let err_report = Report::msg(format!( | ||
| "package's kind is {}, expected TransactionScript", | ||
| package_kind | ||
| )); | ||
| return Err(TransactionScriptError::PackageNotTransactionScript(err_report)); | ||
| }; | ||
|
|
||
| let program = | ||
| package.try_into_program().map_err(TransactionScriptError::PackageNotProgram)?; |
There was a problem hiding this comment.
I believe this will currently fail because Package::try_into_program() will return an error if package_kind is not TargetType::Executable.
This brings up a question of how can we identify the program entrypoint for TransactionScript packages. For notes, we have the @note_script attribute. Should we add something similar for transaction scripts? cc @bitwalker and @greenhat
There was a problem hiding this comment.
Should we not use TargetType::TransactionScript here?
(...)
I believe this will currently fail becausePackage::try_into_program()will return an error ifpackage_kindis notTargetType::Executable.
For context, I originally went with try_into_program directly because, while working on 0xMiden/faucet#204, the Package containing the compiled TransactionScript had Executable as its kind and not TransactionScript (the project was compiled with cargo-miden 0.8.0).
If I'm not mistaken, this behavior changed inside the compiler. Until version 0.8.0 (the one we were using), Packages were tagged as either Executable or Library (I believe this was done here). However, on the next branch, this is no longer the case: TargetType is used directly (Done here I believe).
However, after compiling (using the latest changes) the mint-tx project from 0xMiden/faucet#204, the resuling package is still tagged as Executable.
I'm not quite sure where this discrepancy stems from.
There was a problem hiding this comment.
I believe this will currently fail because
Package::try_into_program()will return an error ifpackage_kindis notTargetType::Executable.
We still compile transaction scripts as an executable. Only the note script and authentication component are compiled as a library with exports marked with @note_script and @auth_script.
This brings up a question of how can we identify the program entrypoint for
TransactionScriptpackages. For notes, we have the@note_scriptattribute. Should we add something similar for transaction scripts? cc @bitwalker and @greenhat
I agree. The transaction script is the only rollup target that is not compiled as a library. It makes sense to compile it as a library and mark an entrypoint with an attribute.
There was a problem hiding this comment.
We still compile transaction scripts as an executable. Only the note script and authentication component are compiled as a library with exports marked with
@note_scriptand@auth_script.(...)
I agree. The transaction script is the only rollup target that is not compiled as a library. It makes sense to compile it as a library and mark an entrypoint with an attribute.
Noted. So should we use package.try_into_program() for now and switch it back to library once this change is made?
There was a problem hiding this comment.
Yes - let's do this. For now, tx scripts would be programs but once the compiler transitions to using libraries for everything, we can make these changes here too.
There was a problem hiding this comment.
Noted. I've reverted the change in 1b0a3ae back to the original Program approach.
Suggested-by: Ignacio Amigo <ignacio.amigo@lambdaclass.com>
This reverts commit 8513085.
lima-limon-inc
force-pushed
the
fabrizioorsi/i2111-tx-script-from-package
branch
from
8513085 to
1b0a3ae
Compare
4 participants
Closes #2111
This PR adds a
TransactionScript::from_packagemethod, analogous to theAccountComponent::from_packageandNoteScript::from_package. This came up while we were working on: 0xMiden/faucet#204This is a follow up of #2502 and also #1802