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

Problem: disorganised wasm binding code in bindings/wasm/src/lib.rs (fix #225) #243

Merged
merged 4 commits into from
Mar 18, 2022

Conversation

stevenatcrypto
Copy link
Collaborator

@stevenatcrypto stevenatcrypto commented Mar 17, 2022

Close #225

Summary

. Separate wasm binding code to cosmos_sdk.rs, ethereum.rs and nft.rs (as frist step of refactor).

. Add struct CosmosClient and CosmosClientConfig as below.

pub struct CosmosClient {
    config: CosmosClientConfig,
}

impl CosmosClient {
    #[wasm_bindgen(constructor)]
    pub fn new(config: CosmosClientConfig) -> Self {
        Self { config }
    }

    pub fn query_account_balance(&self, address: String, denom: String, api_version: u8) -> Promise { ... }
    pub fn query_account_details(&self, address: String) -> Promise { ... }
    pub fn broadcast_tx(&self, raw_signed_tx: Vec<u8>) -> Promise { ... }
}

pub struct CosmosClientConfig {
    api_url: String,
    tendermint_rpc_url: String,
}

With js_sys::Promise and feature_to_promise, we could convert Rust async instance functions (&self and in a struct) to return JS promise.
In JS example, we could send a transaction as below.

// Create a transaction.
const tx = new wasm.CosmosTx();

// Add a staking delegate message.
tx.add_msg(wasm.CosmosMsg.build_staking_delegate_msg( ... ));

// Add a bank send message.
tx.add_msg(wasm.CosmosMsg.build_bank_send_msg( ... ));

// Sign the transaction and move out all pending messages.
const txData = tx.sign_into( ... );

const config = new wasm.CosmosClientConfig(
  CHAINMAIN_API_URL,
  TENDERMINT_RPC_URL,
);
const client = new wasm.CosmosClient(config);

// Broadcast signed transaction data.
await client.broadcast_tx(txData);

. I add a new issue #244 to delete C-Style functions in wasm binding and update integration-test. I will fix it in another PR. Thanks.

Comment on lines +39 to +41
future_to_promise(async move {
query_account_balance(api_url, address, denom, api_version).await
})
Copy link
Contributor

Choose a reason for hiding this comment

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

is this necessary?
if e.g. this could be

#[wasm_bindgen]
pub async query_account_balance(...) -> Result<JsValue, JsValue> {
  query_account_balance(self.config.api_url.clone(), address, denom, api_version).await
}

and wasm-bindgen will automatically convert to promises?

Copy link
Collaborator Author

@stevenatcrypto stevenatcrypto Mar 17, 2022

Choose a reason for hiding this comment

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

I am sorry that I didn't mention the issue I met.
A below error occurred if directly use async on instance functions. It seems that self cannot live long in JS environment.

error[E0597]: `me` does not live long enough
  --> bindings/wasm/src/cosmos_sdk.rs:19:1
   |
19 | #[wasm_bindgen]
   | ^^^^^^^^^^^^^^-
   | |             |
   | |             `me` dropped here while still borrowed
   | borrowed value does not live long enough
   | argument requires that `me` is borrowed for `'static`

I found this wasm-bindgen issue and a solution comment. It seems that returning Promise explicitly is better than wrapping self with Rc<RefCell<Self>>. What do you think? Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, that is probably better.
I think @damoncro faced similar issues before with the grpc-web functions

@tomtau tomtau merged commit 6293def into main Mar 18, 2022
@tomtau tomtau deleted the refactor/separate-wasm-binding-to-multiple-files branch March 18, 2022 01:08
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

Successfully merging this pull request may close these issues.

Problem: disorganised wasm binding code in bindings/wasm/src/lib.rs
3 participants