hyperledger / iroha-javascript

JavaScript library for Iroha, a Distributed Ledger Technology (blockchain) platform.
https://wiki.hyperledger.org/display/iroha
Apache License 2.0
94 stars 64 forks source link

Refactored crypto & client; publishment #69

Closed 0x009922 closed 3 years ago

0x009922 commented 3 years ago

Unresolved problems

@iroha/crypto is only ESM-compatible

Usage of @iroha/crypto is still not very clear and requires a native ES modules environment. In web it works fine with vite, and also in node with esbuild-register. Other cases are not very well researched.

iroha_crypto_core crate

Located at packages/iroha-crypto/rust/iroha_crypto_core

It is a copy of iroha_crypto crate from hyperledger/iroha (iroha2-dev) repo, refactored to be no-std compatible. The better way to make it I see here is to refactor the main crate to be no-std compatible and use it directly in iroha_crypto_wasm crate in this repo.

Don't forget to free() crypto structures!

They will never be garbage collected manually, so if you don't want memory leaks, you have to free() every created structures manually when they have to be dropped.

Inefficiency of exposed API by wasm_bindgen

There is a lot of data cloning that looks like unnecessary, but I can't determine and don't know how to design it in better way. Maybe we will see it after some real-world usage.

Also, wasm_bindgen has some undocumented behavior (or I haven't found any documentation) - Rust's ownership rules works on incoming exposed structures! For example:

#[wasm_bindgen]
pub struct PublicKey {
    payload: Vec<u8>
}

#[wasm_bindgen]
impl PublicKey {
    #[wasm_bindgen(constructor)]
    pub fn new() -> PublicKey {
        PublicKey { payload: vec![] }
    }
}

#[wasm_bindgen]
pub fn get_pub_key_payload(val: PublicKey) -> Vec<u8> {
    val.payload
}

This will be compilted to JS-wrappers like this:

declare export class PublicKey {
    free(): void;
    constructor();
}

declare export function get_pub_key_payload(val: PublicKey): UInt8Array;

And when you use it like this:

// new struct, memory allocated
const pk = new PublicKey();

const bytes = get_pub_key_payload(pk);
//                                ^^ here `pk` is moved, and we can't use it anymore in JS

// trying to use `pk` again...
get_pub_key_payload(pk); // error! null pointer passed to rust

So, OK, ownership rules affect generated bindings with wasm_bindgen, but it is completely unclear from the JS side! There is no any borrow-checker in JS that will fail compilation in case when ownership rules have been violated. So I removed all cases when incoming values are being moved and I always borrow them and copy when it is necessary.

How to solve it? Idk, maybe we have to completely redesign the WASM crate to exclude any possibility to use it improperly. We need more usage examples.

0x009922 commented 3 years ago

Also this PR closes https://github.com/hyperledger/iroha-javascript/issues/67

0x009922 commented 3 years ago

Publishing

This PR now includes preparations to publishment of its packages. All CI/CD pipeline should look like this:

pnpm i
pnpm type-check
pnpm test
pnpm build
pnpm publish-all
0x009922 commented 3 years ago

I've added an installation notes to the READMEs of @iroha2/client, @iroha2/crypto and @iroha2/data-model.

In short, the end user should just configure package manager to fetch @iroha2-scoped packages from nexus:

# .npmrc
@iroha2:registry=https://nexus.iroha.tech/repository/npm-group/