wasm_bindgen support for fuel-tx #645

Dentosal · 2023-12-04T15:34:13Z

Closes #579

I'm not sure if we need to add serialization and deserialization support to more types, but this should at least provide a good starting point.

Also, this is still missing some datastructure manipulation, for instance you cannot modify Policies easily.

Dentosal · 2023-12-05T17:19:00Z

Size of fuel-tx wasm blob is just under 200 KiB at 74a4d2b.

Dentosal · 2023-12-06T03:30:07Z

And after further size reduction, at 9d89fbc it's 187 KiB. If the allocator is also replaced with wee_alloc, then the size is only 181 KiB. We could remove 44 KiB of that is just the signature validation in check_input, so if that's not needed we could go even lower.

The command used to determine the size is:

cargo rustc -p fuel-tx --target wasm32-unknown-unknown --no-default-features --features typescript --crate-type=cdylib --profile web-release && wasm-bindgen --typescript --target web ./target/wasm32-unknown-unknown/web-release/fuel_tx.wasm --out-dir tmp/src && wasm-opt tmp/src/fuel_tx_bg.wasm -o tmp/src/fuel_tx_bg.wasm -Oz && wc -c tmp/src/fuel_tx_bg.wasm

xgreenx

It would be nice to use snapshots in JS unit tests to see that deserialisation/serialisation works=)

xgreenx · 2023-12-11T15:04:16Z

fuel-tx/src/transaction/types/output.rs

+    impl Output {
+        #[cfg(feature = "serde")]
+        #[wasm_bindgen(js_name = toJSON)]
+        pub fn to_json(&self) -> String {


Do we want to add fromJSON method? (the same for Input)

Possibly, but I'd leave that as a follow up that can be done if anyone requests it. This is mostly for debugging anyways, and I don't expect the production version to enable serde feature anyway.

xgreenx · 2023-12-11T15:08:22Z

fuel-tx/src/transaction/types/script.rs

@@ -53,6 +53,7 @@ pub(crate) struct ScriptMetadata {

 #[derive(Clone, Derivative)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[cfg_attr(feature = "typescript", wasm_bindgen::prelude::wasm_bindgen)]


How does it work if Input and Output are enums?

The fields are private and not directly accessible from outside, so there's no interface to generate.

fuel-tx/Cargo.toml

xgreenx · 2023-12-11T15:09:52Z

fuel-tx/Cargo.toml

 hashbrown = { version = "0.14", optional = true }
 itertools = { version = "0.10", default-features = false, optional = true }
+js-sys = { version = "0.3", optional = true }


The same here, why do we need it?

js-sys is used to return proper js error types, for instance here:

fuel-vm/fuel-tx/src/tx_pointer.rs

Lines 117 to 121 in 8124114

#[wasm_bindgen(constructor)]

pub fn typescript_new(value: &str) -> Result<TxPointer, js_sys::Error> {

use core::str::FromStr;

TxPointer::from_str(value).map_err(js_sys::Error::new)

}

Why do we return JsValue in one cases and js_sys::Error in another?

I had missed those since I searched for JsValue> to find use in Result types, but I had left inline comments so that it looked like this Result<..., JsValue /* Comment */> instead.

Fixed in 10ac139.

fuel-tx/Cargo.toml

Dentosal · 2023-12-12T20:47:10Z

It would be nice to use snapshots in JS unit tests to see that deserialisation/serialisation works=)

It don't think adding snapshot tests for the frontend side is necessary, since we already do that on the native side, and this uses the exact same serialization code. Maybe we should rather compare that the wasm and native versions produce the same output for an input? Such a test is quite annoying to write, but I can do that if it seems useful to you.

xgreenx · 2023-12-13T00:59:35Z

It would be nice to use snapshots in JS unit tests to see that deserialisation/serialisation works=)

It don't think adding snapshot tests for the frontend side is necessary, since we already do that on the native side, and this uses the exact same serialization code. Maybe we should rather compare that the wasm and native versions produce the same output for an input? Such a test is quite annoying to write, but I can do that if it seems useful to you.

I definitely don't want us to create the same tx on JS side and compare serialized bytes=) I meant to deserialize tx on the JS side using serialized bytes from snapshot files. But if it is hard to extract serialized bytes, then we can skip it. This move proves that deserialization and serialization are the same on both sides.

Dentosal · 2023-12-14T07:49:59Z

I definitely don't want us to create the same tx on JS side and compare serialized bytes=) I meant to deserialize tx on the JS side using serialized bytes from snapshot files. But if it is hard to extract serialized bytes, then we can skip it. This move proves that deserialization and serialization are the same on both sides.

Wasn't too hard. Added the snapshot comparison cases in ff9329b.

xgreenx · 2023-12-15T11:35:32Z

fuel-tx/src/transaction.rs

+#[cfg(feature = "typescript")]
+use self::{
+    input::typescript as input_ts,
+    output::typescript as output_ts,
+};
+


It seems we can move it into typescript module

Dentosal added 9 commits November 28, 2023 19:03

Add Input::check wasm export

2a66427

Expose canonical ser/deser

2017df6

No not export anything else than ser/deser

5ee08d1

Export input and output constructors to wasm

6a0e1e2

Add export tests

8ea8785

npm setup

a4ca514

Add UtxoId methods, make them nice

d9d4718

Make input and output nice and tested

66c84d0

Add input validation

ba67c57

Dentosal self-assigned this Dec 4, 2023

Dentosal added 2 commits December 4, 2023 17:34

Merge branch 'master' into dento/wasm-bindgen-more

0d07a73

Cleanup

74a4d2b

Dentosal added 8 commits December 5, 2023 19:19

toml-fmt

eb607a8

Add changelog

f34f3f9

Clippy

782b3a3

Add output validation test and clean up

90eae1c

Copy tests over to cjs

3caad5f

Make a separate wasm release profile, and optimize further

18723b0

Use web-release profile on .npm

a23e1dc

Remove wee_alloc before merge

9d89fbc

Dentosal added the fuel-tx Related to the `fuel-tx` crate. label Dec 6, 2023

Dentosal requested a review from a team December 6, 2023 03:32

Dentosal marked this pull request as ready for review December 6, 2023 03:32

Fix unused_crate_dependencies

e4aebc2

Dentosal marked this pull request as draft December 6, 2023 20:44

Dentosal added 3 commits December 6, 2023 22:44

Use correct wasm blob

90c9645

Fix incorrect profile name in last commit

ebb2c6a

Export tx variants directly, add tests

c08d290

Dentosal added 2 commits December 7, 2023 00:00

Add witness methods

f1392ff

Fix imports and unused_crate_dependencies

8124114

Dentosal marked this pull request as ready for review December 6, 2023 22:13

xgreenx reviewed Dec 11, 2023

View reviewed changes

Dentosal added 3 commits December 12, 2023 22:21

Remove direct getrandom dependency

347a813

Replace duplicate crates with macro_rules!

3c68879

Remove dead warning-suppression code for duplicate crate

81ad50c

Dentosal added 3 commits December 14, 2023 08:56

Use js_sys::Error instead of JsValue for validation errors

10ac139

Change to_bytes to throw exceptions correctly

958c178

Add snapshot tests to nodejs side

ff9329b

Dentosal requested a review from xgreenx December 14, 2023 18:01

xgreenx approved these changes Dec 15, 2023

View reviewed changes

Dentosal added this pull request to the merge queue Dec 17, 2023

Merged via the queue into master with commit ba08344 Dec 17, 2023
37 checks passed

Dentosal deleted the dento/wasm-bindgen-more branch December 17, 2023 23:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wasm_bindgen support for fuel-tx #645

wasm_bindgen support for fuel-tx #645

Dentosal commented Dec 4, 2023 •

edited

Loading

Dentosal commented Dec 5, 2023

Dentosal commented Dec 6, 2023

xgreenx left a comment

xgreenx Dec 11, 2023

Dentosal Dec 12, 2023

xgreenx Dec 11, 2023

Dentosal Dec 12, 2023

xgreenx Dec 11, 2023

Dentosal Dec 12, 2023

xgreenx Dec 13, 2023

Dentosal Dec 14, 2023

Dentosal commented Dec 12, 2023

xgreenx commented Dec 13, 2023

Dentosal commented Dec 14, 2023

xgreenx Dec 15, 2023

	#[wasm_bindgen(constructor)]
	pub fn typescript_new(value: &str) -> Result<TxPointer, js_sys::Error> {
	use core::str::FromStr;
	TxPointer::from_str(value).map_err(js_sys::Error::new)
	}

wasm_bindgen support for fuel-tx #645

wasm_bindgen support for fuel-tx #645

Conversation

Dentosal commented Dec 4, 2023 • edited Loading

Dentosal commented Dec 5, 2023

Dentosal commented Dec 6, 2023

xgreenx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dentosal commented Dec 12, 2023

xgreenx commented Dec 13, 2023

Dentosal commented Dec 14, 2023

Choose a reason for hiding this comment

Dentosal commented Dec 4, 2023 •

edited

Loading