[Task] Wasm Bindings Improvements

[HenriqueNogara]

Description

Remove exposure of Ed25519.sign, remove Storage Send/Sync ties to the wasm feature, and make keyDel in typescript compatible with the Rust code. Refactor the Storage trait to return better errors, and simplify its API.

Motivation

1. Users should be able to sign data without relying on our code.
2. We might need the Storage trait not to be Send/Sync for other bindings (e.g Python) as well, so we should not tie its bounds to the wasm feature.
3. Js implementation of MemStore keyDel slightly differs from the Rust code regarding idempotency.
4. identity_account::types::signature::Signature::pkey is not used in the Rust code.
5. Storage::set_password is no longer needed.
6. Improve error feedback. See: Add Wasm Account Storage interface #597 (comment)

To-do list

• Assess the need to expose Ed25519.sign - @noble/ed25519 might provide the correct signature
• Remove Storage ties to wasm feature
  • Change the way the Account handles Storage
  • Use the cfg_attr on Storage and change the name of the feature that enables it
• Decide whether Storage::key_del should be idempotent or not. Refactor KeyLocation #729
• Refactor Account Signature not to contain PublicKey [Task] Refactor Account Signature #722
• Remove SetPassword method from Storage [Task] Remove Storage::set_password #720
• Investigate simplification of state in the Storage interface, potentially reducing the number of state variables
• Refactor Error for Storage Interface
  • Introduce a reasonably abstract error type that can accommodate many Storage implementations, including any type of file- or network-I/O-related failures
• Prefer to use UInt8Array rather than Base58-BTC encoded strings for keys. [Task] Change keys in Wasm from Base58-BTC strings to UInt8Array #721
• Write macro for napi related types and impl - Deduplicate Stronghold Nodejs wrapper code #872
• Assess NapiResult error messages
• Avoid Serializing and Deserializing keys

Change checklist

Add an x to the boxes that are relevant to your changes, and delete any items that are not.

• The feature or fix is implemented in Rust and across all bindings whereas possible.
• The feature or fix has sufficient testing coverage
• All tests and examples build and run locally as expected
• Every piece of code has been document according to the documentation guidelines.
• If conceptual documentation (mdbook) and examples highlighting the feature exist, they are properly updated.
• If the feature is not currently documented, a documentation task Issue has been opened to address this.

[cycraig]

Users are already able to sign data without relying on our code.

E.g. using TweetNaCl

const key = new KeyPair(KeyType.Ed25519);
const seed = bs58.decode(key.private);
let naclKeyPair = nacl.sign.keyPair.fromSeed(seed);
let challengeBytes = Uint8Array.from([1,2,3,4]);
let signature = nacl.sign(challengeBytes, naclKeyPair.secretKey);
console.log(`Signature: ${bs58.encode(signature)}`);

I do think we should switch to using UInt8Array to represent the private/public keys though, from Base58-BTC encoded strings. This is because it's the type existing libraries like TweetNaCl expect as input and to avoid requiring developers bring in Base58 dependencies to decode the strings we return. Alternative would be to expose the dencode_b58, decode_multibase functions too, which might be useful for Storage implementers regardless.

Advantages of exposing Ed25519::sign:

• It can be compatible with our exposed structs, like KeyPair, WasmPrivateKey or generalised to UInt8Array/String.
• It's shipped in the Wasm bytecode anyway, we may as well expose it along with other functions.
• It can avoid external dependencies for developers.

Disadvantages:

• API bloat: increasing the API surface also increases the maintenance cost.
• Slight code bloat.

We have had requests to include other features in identity.rs such as BIP-39 mnemonics, so I think some developers might appreciate being able to use same the functions we do internally out of convenience.
See also this comment: https://github.com/iotaledger/identity.rs/pull/597/files#r811781156

We might need the Storage trait not to be Send/Sync for other bindings (e.g Python) as well, so we should not tie its bounds to the wasm feature.

I agree. Some conditional compilation required now just for wasm might be beneficial for other use-cases too, so we can use other features (named more specifically like storage-sync for example) on top of target-based compilation.
Edit: as long as enabling --all-features does not break compilation.

Verify the need to return an error on KeyDel

I think all Storage functions need to be async and fallible to account for unknowable remote implementations where every little network request could fail. Not the point the issue raised, it's about idempotency and consistency between Rust and bindings behaviour.

[cycraig]

This comment on #660 may have been missed regarding the Stronghold bindings package namespace: https://github.com/iotaledger/identity.rs/pull/660/files#r816770207

Edit: resolved.

[cycraig]

As per internal discussions, the following tasks were marked as either important or easy to do prior to 0.5.0.

• Prefer to use UInt8Array rather than Base58-BTC encoded strings for keys in the Wasm bindings. [Task] Change keys in Wasm from Base58-BTC strings to UInt8Array #721
• Decide whether Storage::key_del should be idempotent or not. Refactor KeyLocation #729
• Refactor Account Signature not to contain PublicKey. [Task] Refactor Account Signature #722
• Remove SetPassword method from Storage. [Task] Remove Storage::set_password #720

Each needs its own issue or PR now.