-
Notifications
You must be signed in to change notification settings - Fork 24
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
PoC of the Miden SDK and rollup account implementation #75
Conversation
Switch cargo based integration tests to target wasm32-wasi instead of wasm32-unknown-unknown to have https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md in compiled Wasm.
(import "env" "miden::sat::account::add_asset" (func $miden::sat::account::add_asset (;0;) (type 0))) | ||
(import "env" "miden::sat::account::remove_asset" (func $miden::sat::account::remove_asset (;1;) (type 0))) | ||
(import "env" "miden::sat::tx::create_note" (func $miden::sat::tx::create_note (;2;) (type 1))) | ||
(func $receive_asset (;3;) (type 0) (param i32 i32) |
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.
@bobbinth @bitwalker This Wasm is compiled from https://github.com/0xPolygonMiden/miden-ir/blob/b27a75e13810fc55a115d9cdee9f0cab7e6f16e5/tests/rust-apps-wasm/sdk-account/src/lib.rs#L1-L29 which uses the SDK skeleton I drafted (see https://github.com/0xPolygonMiden/compiler/tree/b27a75e13810fc55a115d9cdee9f0cab7e6f16e5/sdk/src )
The calling convention is defined at https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md
Side note: To get the above calling convention, I switched to wasm32-wasi
target because wasm32-unknown-unknown
actually produces a different calling convention - the one inspired by wasm-bindgen
. See https://users.rust-lang.org/t/how-are-structs-represented-in-web-assembly/57317/6 for more details.
The main challenge I see and focus on is that structs are passed by reference (address on the linear stack) while callers and callees are always expecting function parameters on the stack.
In the case of imported miden::sat::account::add_asset
and others, I can naively imagine we could rewrite them in miden-base
to expect a pointer to the struct instead of the struct itself and then load it on the stack. But in case of exported functions like receive_asset
and send_asset
it's not even possible since they are supposed to be called via call
(see miden-base) meaning we can only pass values on stack and/or via advice provider.
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.
Since the account methods can be externally called only from notes, I'm also bringing the note code into this PoC.
I plan to use macros to "wrap" note to account calls in serialization/deserialization code to pass arguments and return values via stack and advice provider. Initially, though I'll write this glue code manually and implement its generation via macros.
Let's see what it might look like for the MyWallet::receive_asset
method:
The original user's MyWallet::receive_asset
was renamed to receive_asset_original
. The note code calls via call
the created MyWallet::receive_asset
which serializes the arguments and calls MyWallet::receive_asset_extern
which is an extern "C"
to the account's Miden code. The corresponding Miden code for MyWallet::receive_asset_extern
is provided by the account's MyWallet::receive_asset_impl
which deserializes the arguments and calls MyWallet::receive_asset_original
which is the user-written original method. The return value is serialized by MyWallet::receive_asset_impl
and returned to the note code deserialized in MyWallet::receive_asset
.
Or in a diagram:
Note
->
call
->
MyWallet::recieve_asset
do args serialization
->
extern "C" MyWallet::receive_asset_extern
->
exec MyWallet::receive_asset_impl
exported from the account's code does args deserialization.
->
exec MyWallet::receive_asset_original
->
MyWallet::receive_asset_impl
does return value serialization.
->
MyWallet::receive_asset
does return value deserialization.
->
Note
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.
I took a stab at MyWallet::receive_assets
at the scheme above and got quite far - https://github.com/0xPolygonMiden/miden-ir/blob/57cb25537964986a50e115291658b6c4a20a961b/tests/rust-apps-wasm/sdk-basic-wallet/src/lib.rs#L24-L82
There are a lot of todos in the code that macros will generate, but the idea seems to be viable.
Note that receive_assets_arg_ser
and receive_assets_arg_deser
should have the same name since it's imported func and its implementation, I just have not figured out how to do it yet.
MyWallet::receive_asset
is called from the note's code. It packs the arguments and passes it either via stack or advice provider and calls receive_assets_arg_ser
, which should end up as call.my_wallet::receive_assets_arg_deser
in the note's MASM code. Which in turn deserializes args (stack/advice) and calls the user's MyWallet::receive_assets
.
compilation, clean up imports and exports in account and recv note Wasm code. Restore user's method names in account code for account <-> note DX.
… building the notes
…r function argument passing method
Closed in favor #79 (using Wasm component model) |
Close #72
Changes to make in other components:
miden::sat::account::add_asset
) need to adopt the C ABI for Wasm calling convention.midenc
should "propagate" Wasm imports (e.g.miden::sat::account::add_asset
) to MASM viause
directive.Questions:
call
op? Should we encode it in the function name in Wasm imports?TODO:
Vec<Felt>
<->Vec<u8>
conversionstodo!
leftcall
op