Skip to content
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

WASM Move Calls #1499

Open
wants to merge 69 commits into
base: feat/identity-rebased-alpha
Choose a base branch
from
Open

Conversation

UMR1352
Copy link
Contributor

@UMR1352 UMR1352 commented Jan 15, 2025

Description of change

Port all calls to move functions to TS using IOTA's TS SDK and bind these move calls through WASM into iota_interaction_ts package.

Links to any relevant issues

Closes #1449

Type of change

  • Bug fix (a non-breaking change which fixes an issue)
  • Enhancement (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Fix

How the change has been tested

Huge trust me bro as the project is not yet compilable.

chrisgitiota and others added 30 commits December 4, 2024 12:00
* MoveType has been moved from identity_iota_core to identity_iota_interaction
* The identity_iota_interaction README has been updated to use the latest draft for folder and crate names
…lpha

# Conflicts:
#	identity_iota_core/Cargo.toml
#	identity_iota_core/src/rebased/iota/well_known_networks.rs
…ta_core for the redesigned crate structure

* Compiling identity_iota_core for wasm32 will result in one compiler error
  due to missing wasm-bindings for ProgrammableTransaction
  (TODO in identity_iota_core/src/rebased/transaction.rs)
* identity_iota_core and examples are compilable for non wasm32 platforms
  * Remaining compiler warnings should be investigated
* identity_iota_core e2e tests are compilable but will fail due to serialization/deserialization issues.
  For example:
    IotaTransactionBlockResponse bcs deserialization failed: NotSupported("deserialize_any")
    thread 'identity::adding_controller_works' panicked at /home/christof/Develop/iotaledger/identity.rs/identity_iota_core/src/rebased/transaction.rs:84:14:
    IotaTransactionBlockResponse bcs deserialization failed: NotSupported("deserialize_any")
    stack backtrace:
      0: rust_begin_unwind
      1: core::panicking::panic_fmt
      2: core::result::unwrap_failed
      3: <identity_iota_core::rebased::transaction::TransactionOutput<T> as core::convert::From<identity_iota_core::rebased::transaction::TransactionOutputInternal<T>>>::from
      4: <T as identity_iota_core::rebased::transaction::Transaction>::execute_with_opt_gas::{{closure}}
      5: identity_iota_core::rebased::transaction::Transaction::execute::{{closure}}
      6: e2e::identity::adding_controller_works::{{closure}}
      7: tokio::runtime::scheduler::current_thread::CurrentThread::block_on
      8: tokio::runtime::runtime::Runtime::block_on
      9: e2e::identity::adding_controller_works
      10: core::ops::function::FnOnce::call_once
    note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
…ialization

BCS deserialization is not available for IotaTransactionBlockResponse because the BCS format is not self describing and several struct fields of IotaTransactionBlockResponse are optionally skipped during serialization. JSON deserialization will probably be available but using a cloned instance to convert a TransactionOutputInternal into a TransactionOutput will have a much better efficiency.
…Credential

All tests running successfully for the first time now
* Fix clippy error writing `&Vec` instead of `&[_]` involves a new object where a slice will do
* allow error clippy::too_many_arguments
…iles and add clippy + format steps for bindings/wasm/iota_interaction_ts
@chrisgitiota chrisgitiota requested review from wulfraem and removed request for wulfraem January 21, 2025 08:23
@chrisgitiota
Copy link

I'll definitely need some more hours to deep dive into this PR especially for my personal favorite check this out 👍 🤩

One request from my side would be to change the target branch to feat/identity-rebased-alpha. The branch feat/wasm32-develop-alpha has already been merged into it and will be deleted probably next week (it only still exists, because I wanted to maintain the purged commit messages for a while).

@UMR1352 UMR1352 changed the base branch from feat/wasm32-develop-alpha to feat/identity-rebased-alpha January 23, 2025 09:42
@UMR1352
Copy link
Contributor Author

UMR1352 commented Jan 23, 2025

Here some more context to better explain the hack used in the implementation of user-driven move-calls.

WASM bindgen allows to pass Rust closures to JS, but it only accepts Fn or FnMut closures. The intent function we receive from the user is neither of these two types and is in fact a FnOnce closure. The reason behind this is that FnOnce better fit the use case we have with "intent function" i.e., a pure function that is called exactly once.
Therefore, in order to make WASM bindgen happy, the intent function we receive has to become either a Fn or FnMut. Since we don't really care about modifying the captured context, Fn is a better fit.
Changing the type of intent function is not straightforward, the function traits cannot be manually implemented in stable Rust and using std::mem::transmute can't really help in this case.
The only way around this is to construct a new closure that implements Fn and that calls our intent_fn for us.
Though, this is not as straight forward as doing:

let f = || intent_fn(...);

since intent_fn is a FnOnce, invoking it with (...) also consumes it (see FnOnce::call_once and therefore the compiler will make f: FnOnce as well.

In order for f to be Fn it has to access all captured variables through shared references only, but FnOnce cannot be called through a reference.

Cell to the rescue! By wrapping intent_fn in Cell we can move it inside closure f through a shared reference:

let wrapped_fn = Cell::new(Some(intent_fn)); // We wrap it in an Option to leverage its Default implementation.
let f = || {
  wrapped_fn
    .take() // only requires shared access to self and moves out its content i.e. Option<impl FnOnce>
    .unwrap() // returns an impl FnOnce
    (...); // invokes and consumes `intent_fn`
};

The compiler will infer impl Fn for f and we can then pass it to WASM bindgen.

UMR1352 and others added 2 commits January 23, 2025 15:19
Remove npm dependency @iota/iota-sdk from iota_interaction_ts package.json and use @iota/iota.js instead
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Added A new feature that requires a minor release. Part of "Added" section in changelog Wasm Related to Wasm bindings. Becomes part of the Wasm changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants