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 support #351

Merged
merged 11 commits into from
Jun 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,24 @@ jobs:
with:
key: "${{ matrix.command }} ${{ matrix.args }} ${{ matrix.package }}"

- name: Add WASM target
if: ${{ matrix.command == 'test' }}
run: rustup target add wasm32-unknown-unknown

- name: Install WebAssembly Test harness
if: ${{ matrix.command == 'test' }}
uses: actions-rs/cargo@v1
with:
command: install
args: webassembly-test-runner

- name: Test WASM package
if: ${{ matrix.command == 'test' }}
run: |
cd packages/wasm-tests
cargo test --target wasm32-unknown-unknown --all-targets --all-features
cargo test --target wasm32-unknown-unknown --all-targets --no-default-features

- name: Install rustfmt
if: ${{ matrix.command == 'fmt' }}
run: rustup component add rustfmt
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ members = [
"packages/fuels-signers",
"packages/fuels-test-helpers",
"packages/fuels-types",
"packages/wasm-tests",
"tools/fuels-abi-cli",
"scripts/build-test-projects",
"examples/contracts",
Expand Down
4 changes: 2 additions & 2 deletions packages/fuels-core/src/code_gen/abigen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,17 @@ impl Abigen {
(
quote! {
use alloc::{vec, vec::Vec};
use fuels_core::{Detokenize, EnumSelector, InvalidOutputType, ParamType, Tokenizable, Token};
},
quote! {},
)
} else {
(
quote! {
use fuels::contract::contract::{Contract, ContractCall};
use fuels::prelude::InvalidOutputType;
use fuels::signers::LocalWallet;
use fuels::tx::{ContractId, Address};
use fuels::core::{Detokenize, EnumSelector, InvalidOutputType, ParamType, Tokenizable, Token};
use std::str::FromStr;
},
quote! {
Expand Down Expand Up @@ -149,7 +150,6 @@ impl Abigen {
#![allow(unused_imports)]

#includes
use fuels::core::{Detokenize, EnumSelector, ParamType, Tokenizable, Token};

#code

Expand Down
5 changes: 5 additions & 0 deletions packages/wasm-tests/.cargo/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[build]
target = "wasm32-unknown-unknown"

[target.wasm32-unknown-unknown]
runner = "webassembly-test-runner"
16 changes: 16 additions & 0 deletions packages/wasm-tests/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[package]
name = "wasm-tests"
version = "0.0.0"
edition = "2021"
publish = false

[lib]
crate-type = ['cdylib']

[dependencies]
fuels-abigen-macro = { path = "../fuels-abigen-macro" }
fuels-core = { path = "../fuels-core" }
getrandom = { version = "0.2", features = ["js"] }

[dev-dependencies]
webassembly-test = "0.1"
78 changes: 78 additions & 0 deletions packages/wasm-tests/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
extern crate alloc;
use fuels_abigen_macro::wasm_abigen;

wasm_abigen!(
no_name,
r#"[
{
"type":"contract",
"inputs":[
{
"name":"SomeEvent",
"type":"struct SomeEvent",
"components": [
{
"name": "id",
"type": "u64"
},
{
"name": "account",
"type": "b256"
}
]
},
{
"name":"AnotherEvent",
"type":"struct AnotherEvent",
"components": [
{
"name": "id",
"type": "u64"
},
{
"name": "hash",
"type": "b256"
},
{
"name": "bar",
"type": "bool"
}
]
}
],
"name":"takes_struct",
"outputs":[]
}
]
"#
);

pub fn the_fn() {
use fuels_core::{abi_decoder::ABIDecoder, ParamType};
let data = vec![
0, 0, 0, 0, 0, 0, 3, 252, 175, 175, 175, 175, 175, 175, 175, 175, 175, 175, 175, 175, 175,
175, 175, 175, 175, 175, 175, 175, 175, 175, 175, 175, 175, 175, 175, 175, 175, 175, 175,
175,
];

let mut decoder = ABIDecoder::new();

let obj = decoder
.decode(&[ParamType::U64, ParamType::B256], &data)
.expect("Failed to decode");

let a_struct = SomeEvent::new_from_tokens(&obj);

assert_eq!(1020, a_struct.id);
}

#[cfg(test)]
mod tests {
use super::*;
use webassembly_test::webassembly_test;

#[webassembly_test]
fn test() {
the_fn();
}
}
Copy link
Member

@Voxelot Voxelot Jun 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When running cargo test --target wasm32-unknown-unknown with the following code:

Suggested change
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test() {
the_fn()
}
}

I encountered a strange error:

/home/voxelot/code/fuel/fuels-rs/target/wasm32-unknown-unknown/debug/deps/wasm_tests-b1e4d09cfabc0966.wasm: 1: Syntax error: word unexpected (expecting ")")
error: test failed, to rerun pass '--lib'

this was on ubuntu 20.04. My m1 mac wasn't even able to compile this, so that should be factored into the devux of writing wasm indexer modules since it may require a lot of llvm toolchain config and setup to get this working in most dev environments.

Copy link
Member

@Voxelot Voxelot Jun 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given fuels-core still largely relies on std, I'm skeptical about whether any of the fs methods (e.g. loading the json abi file) will actually be implemented or just panic at runtime. Guess it depends on the runtime we use the wasm in. However, that should be accounted for in a macro billed as wasm_abigen since we don't know where people may attempt to run this publicly exported macro.

It seems like the safer option is to perform this abigen as a build step using build-deps in a std env, rather than a normal dep in a wasm environment.

cc @vlopes11 since he also has experience with no_std and might have something to add.

Copy link
Member

@Voxelot Voxelot Jun 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternately, would using the typescript sdk abigen w/ assemblyscript be a more dev friendly experience for writing wasm indexer modules? @digorithm

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That first error was because cargo test was trying to run WASM as if it were a native executable... so not gonna work. I put in a WASM test harness to run that test.
I think for mac, the only thing needed is a brew install llvm, the one shipped by default does not have a WASM target support.

For loading the json abi file, that only happens in the proc macro. Proc macros are actually compiled prior to the crate that's being built, and only needs to be built for the host architecture, not to WASM. The thing that is important here is that the token stream that is output by the proc macro needs to be supported by WASM. All I need are rust type definitions and a way to deserialize a binary blob into those types, and that's the only thing that the ABI gen should output for WASM.

Copy link
Member

@Voxelot Voxelot Jun 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that makes sense, thanks for completing the test harness example. It works for me on ubuntu, however it appears that it doesn't actually run in CI. Maybe we need a separate job to run cargo test with the appropriate target instead of using a workspace level cargo test? https://github.com/FuelLabs/fuels-rs/runs/6729727596?check_suite_focus=true#step:8:358

As far as my mac issue, I used brew llvm and had the wasm target working, but it got stuck on secp256k:

LLVM ERROR: malformed uleb128, extends past end

As far as the proc macro/no-std issue, that is reassuring. It's surprising to me that cargo loads proc-macros from normal deps instead of build deps, considering they're only run at build time. I wonder if that's because types from the macro deps that get used in the resultant token stream need to be transitively imported for hygiene purposes?

Copy link

@ghost ghost Jun 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the .cargo/config.toml would've handled that? I guess not though. I'll put in a separate command for it.

For mac are you env vars pointed at the right binary? I had to do this:

export PATH="/opt/homebrew/opt/llvm/bin:$PATH"
export CC=/opt/homebrew/opt/llvm/bin/clang
export AR=/opt/homebrew/opt/llvm/bin/llvm-ar

Though that issue looks like it's building secp256k to wasm? I don't think we'll ever have a need for making signatures in wasm, so maybe it should be an optional dep of fuels-core?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nice, these exports fixed the mac issue for me!

Having secp256k be optional in fuels-core would make sense since fuels-rs only uses secp256k for signing. If someone was writing fancy indexer logic involving some cryptography, they could source their own no-std libs for that. Looks like we'd just need to disable default features for fuel-tx when importing it within fuels-core.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I put in a CI step to check WASM. I looked at making fuel-tx optional, but fuels-core would not compile with --no-default-features. Mainly because of the Receipt type, which is something we do need in the indexers, so maybe we should keep it as-is for now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah maybe file an issue to investigate this later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, filed it here: #371