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 and clvm_tools-js integration #108

Open
ChiaMineJP opened this issue Aug 20, 2021 · 9 comments
Open

wasm and clvm_tools-js integration #108

ChiaMineJP opened this issue Aug 20, 2021 · 9 comments
Assignees

Comments

@ChiaMineJP
Copy link
Contributor

@richardkiss
I've just confirmed that the wasm of clvm_rs can be successfully integrated into clvm_tools-js!
clvm_tools-js#clvm_rs

# Terminal
git clone https://github.com/Chia-Mine/clvm_tools-js
cd clvm_tools-js
git checkout clvm_rs
npm install
npm run build
node .dist/npm/bin/cli.js brun --experiment-backend rust --time "(+ 1 (q . 3))" "2"

However, I'm aware that current wasm API is not fully aligned with Python's clvm_rs API.
At wasm, run_clvm Rust function is exposed to wasm, which accepts only clvm program and arguments, and returns only program result without value of cost.

#[wasm_bindgen]
pub fn run_clvm(program: &[u8], args: &[u8]) -> Vec<u8> {
// ...

On the other hand, Python's API accepts additional parameters and also return cost.

#[allow(clippy::too_many_arguments)]
#[pyfunction]
pub fn deserialize_and_run_program2(
    py: Python,
    program: &[u8],
    args: &[u8],
    quote_kw: u8,
    apply_kw: u8,
    opcode_lookup_by_name: HashMap<String, Vec<u8>>,
    max_cost: Cost,
    flags: u32,
) -> PyResult<(Cost, LazyNode)> {
// ...

At a quick glance, I think arguments quote_kw, apply_kw, opcode_lookup_by_name in deserialize_and_run_program2 are not necessary for wasm's run_clvm because those arguments seem always the same value and no user interference.
Question is whether the above assuption is correct or not.

As for max_cost and flags, I'll find the good way/appropriate type to connect to JavaScript. (JavaScript built-in number cannot represent unsigned int 64bit as is. So u64 of Rust should be represented in uint8array in JavaScript or something)

P.S.

In case you are interested, I'll attach the benchmark result among

  • clvm_tools(js)-clvm(js)
  • clvm_tools(js)-clvm(rust)
  • clvm_tools(python)-clvm(python)
  • clvm_tools(python)-clvm(rust)

benchmark-result-32bit-value-200-ops-clvm_rs_sum_all

benchmark-result-32bit-value-200-ops-clvm_rs_assemble_from_ir

benchmark-result-32bit-value-200-ops-clvm_rs_to_sexp_f

benchmark-result-32bit-value-200-ops-clvm_rs_run_program

@ChiaMineJP
Copy link
Contributor Author

bump (to avoid being labeled as stale)

@github-actions
Copy link

'This issue has been flagged as stale as there has been no activity on it in 14 days. If this issue is still affecting you and in need of review, please update it to keep it open.'

@ChiaMineJP
Copy link
Contributor Author

Resetting stale timer...
We need clvm_rs@0.1.1 for npm.

@github-actions
Copy link

'This issue has been flagged as stale as there has been no activity on it in 14 days. If this issue is still affecting you and in need of review, please update it to keep it open.'

@richardkiss
Copy link
Contributor

There is now an npm package automatically created whenever we do new releases of clvm_rs. It has a very bad and limited API right now, mostly because I am the only one who has worked on this and I don't know very much about how to create wasm APIs that bridge to JS. If you want to create some pull requests that improve the wasm API, I would be very happy to merge them.

@richardkiss
Copy link
Contributor

There is now a Dialect object which may make it easier to provide a useful API to wasm. The main problem is I am not an expert on how to export a new complex rust type like Dialect to JS. Take a look at https://github.com/Chia-Network/clvm_rs/blob/main/src/dialect.rs and https://github.com/Chia-Network/clvm_rs/blob/main/src/chia_dialect.rs to get started.

@ChiaMineJP
Copy link
Contributor Author

It's OK. I'll take a look into it soon.

@github-actions
Copy link

'This issue has been flagged as stale as there has been no activity on it in 14 days. If this issue is still affecting you and in need of review, please update it to keep it open.'

@github-actions
Copy link

'This issue was automatically closed because it has been flagged as stale and subsequently passed 7 days with no further activity.'

@ChiaMineJP ChiaMineJP reopened this Jul 21, 2022
@ChiaMineJP ChiaMineJP self-assigned this Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants