Improve robustness of the Wasm bindings (#1129)

Author: Oliver E. Anderson

.github/workflows/shared-build-wasm.yml

@@ -67,10 +67,6 @@ jobs:
       - name: Build WASM bindings
         run: npm run build
 
-      - name: Run unit tests
-        if: ${{ inputs.run-unit-tests }}
-        run: npm run test:unit
-
       - name: Run Node unit tests
         if: ${{ inputs.run-unit-tests }}
         run: npm run test:unit:node

bindings/wasm/Cargo.toml

@@ -38,15 +38,11 @@ features = ["client", "revocation-bitmap", "resolver"]
 
 [dev-dependencies]
 rand = "0.8.5"
-wasm-bindgen-test = { version = "0.3" }
 
 [target.'cfg(all(target_arch = "wasm32", not(target_os = "wasi")))'.dependencies]
 getrandom = { version = "0.2", default-features = false, features = ["js"] }
 instant = { version = "0.1", default-features = false, features = ["wasm-bindgen"] }
 
-[package.metadata.wasm-pack.profile.release]
-wasm-opt = true
-
 [profile.release]
 opt-level = 's'
 lto = true

bindings/wasm/docs/api-reference.md

@@ -109,10 +109,6 @@ See <code>IVerifierOptions</code>.</p>
 ## Members
 
 <dl>
-<dt><a href="#StateMetadataEncoding">StateMetadataEncoding</a></dt>
-<dd></dd>
-<dt><a href="#KeyType">KeyType</a></dt>
-<dd></dd>
 <dt><a href="#MethodRelationship">MethodRelationship</a></dt>
 <dd></dd>
 <dt><a href="#StatusCheck">StatusCheck</a></dt>
@@ -157,6 +153,10 @@ This variant is the default used if no other variant is specified when construct
 <dt><a href="#FirstError">FirstError</a></dt>
 <dd><p>Return after the first error occurs.</p>
 </dd>
+<dt><a href="#StateMetadataEncoding">StateMetadataEncoding</a></dt>
+<dd></dd>
+<dt><a href="#KeyType">KeyType</a></dt>
+<dd></dd>
 </dl>
 
 ## Functions
@@ -391,6 +391,7 @@ A method-agnostic DID Document.
         * [.signData(data, privateKey, methodQuery, options)](#CoreDocument+signData) ⇒ <code>any</code>
         * [.clone()](#CoreDocument+clone) ⇒ [<code>CoreDocument</code>](#CoreDocument)
         * [._shallowCloneInternal()](#CoreDocument+_shallowCloneInternal) ⇒ [<code>CoreDocument</code>](#CoreDocument)
+        * [._strongCountInternal()](#CoreDocument+_strongCountInternal) ⇒ <code>number</code>
         * [.toJSON()](#CoreDocument+toJSON) ⇒ <code>any</code>
     * _static_
         * [.fromJSON(json)](#CoreDocument.fromJSON) ⇒ [<code>CoreDocument</code>](#CoreDocument)
@@ -719,6 +720,13 @@ Deep clones the `CoreDocument`.
 ### Warning
 This is for internal use only. Do not rely on or call this method.
 
+**Kind**: instance method of [<code>CoreDocument</code>](#CoreDocument)  
+<a name="CoreDocument+_strongCountInternal"></a>
+
+### coreDocument.\_strongCountInternal() ⇒ <code>number</code>
+### Warning
+This is for internal use only. Do not rely on or call this method.
+
 **Kind**: instance method of [<code>CoreDocument</code>](#CoreDocument)  
 <a name="CoreDocument+toJSON"></a>
 
@@ -1801,6 +1809,7 @@ Deserializes an instance from a JSON object.
         * [.unrevokeCredentials(serviceQuery, indices)](#IotaDocument+unrevokeCredentials)
         * [.clone()](#IotaDocument+clone) ⇒ [<code>IotaDocument</code>](#IotaDocument)
         * [._shallowCloneInternal()](#IotaDocument+_shallowCloneInternal) ⇒ [<code>IotaDocument</code>](#IotaDocument)
+        * [._strongCountInternal()](#IotaDocument+_strongCountInternal) ⇒ <code>number</code>
         * [.toJSON()](#IotaDocument+toJSON) ⇒ <code>any</code>
         * [.toCoreDocument()](#IotaDocument+toCoreDocument) ⇒ [<code>CoreDocument</code>](#CoreDocument)
     * _static_
@@ -2197,6 +2206,13 @@ Returns a deep clone of the `IotaDocument`.
 ### Warning
 This is for internal use only. Do not rely on or call this method.
 
+**Kind**: instance method of [<code>IotaDocument</code>](#IotaDocument)  
+<a name="IotaDocument+_strongCountInternal"></a>
+
+### iotaDocument.\_strongCountInternal() ⇒ <code>number</code>
+### Warning
+This is for internal use only. Do not rely on or call this method.
+
 **Kind**: instance method of [<code>IotaDocument</code>](#IotaDocument)  
 <a name="IotaDocument+toJSON"></a>
 
@@ -3933,14 +3949,6 @@ This is possible because Ed25519 is birationally equivalent to Curve25519 used b
 | --- | --- |
 | publicKey | <code>Uint8Array</code> | 
 
-<a name="StateMetadataEncoding"></a>
-
-## StateMetadataEncoding
-**Kind**: global variable  
-<a name="KeyType"></a>
-
-## KeyType
-**Kind**: global variable  
 <a name="MethodRelationship"></a>
 
 ## MethodRelationship
@@ -4022,6 +4030,14 @@ Return all errors that occur during validation.
 ## FirstError
 Return after the first error occurs.
 
+**Kind**: global variable  
+<a name="StateMetadataEncoding"></a>
+
+## StateMetadataEncoding
+**Kind**: global variable  
+<a name="KeyType"></a>
+
+## KeyType
 **Kind**: global variable  
 <a name="start"></a>
 

bindings/wasm/lib/append_functions.ts

@@ -1,10 +1,12 @@
-import { CoreDocument, IotaDocument, IToCoreDocument } from "~identity_wasm";
+import { CoreDID, CoreDocument, IotaDID, IotaDocument, IToCoreDID, IToCoreDocument } from "~identity_wasm";
 type GetCoreDocument = (arg: IToCoreDocument) => CoreDocument;
 type MaybeGetIotaDocument = (arg: IToCoreDocument) => IotaDocument | void;
+type GetCoreDidClone = (arg: IToCoreDID) => CoreDID;
 
 declare global {
     var _getCoreDocumentInternal: GetCoreDocument;
     var _maybeGetIotaDocumentInternal: MaybeGetIotaDocument;
+    var _getCoreDidCloneInternal: GetCoreDidClone;
 }
 function _getCoreDocumentInternal(arg: IToCoreDocument): CoreDocument {
     if (arg instanceof CoreDocument) {
@@ -22,6 +24,17 @@ function _maybeGetIotaDocumentInternal(arg: IToCoreDocument): IotaDocument | voi
     }
 }
 
+function _getCoreDidCloneInternal(arg: IToCoreDID): CoreDID {
+    if (arg instanceof IotaDID || arg instanceof CoreDID) {
+        return arg.toCoreDid();
+    } else {
+        // Pass deep clone to avoid nulling out pointer.
+        return arg.toCoreDid().clone();
+    }
+}
+
 globalThis._getCoreDocumentInternal = _getCoreDocumentInternal;
 
 globalThis._maybeGetIotaDocumentInternal = _maybeGetIotaDocumentInternal;
+
+globalThis._getCoreDidCloneInternal = _getCoreDidCloneInternal;

bindings/wasm/package.json

@@ -18,13 +18,12 @@
     "build:docs": "node ./build/docs",
     "build": "npm run build:web && npm run build:nodejs && npm run build:docs",
     "example:node": "ts-node --project tsconfig.node.json -r tsconfig-paths/register ./examples/src/main.ts",
-    "test": "npm run test:unit && npm run test:unit:node && npm run test:examples",
+    "test": "npm run test:unit:node && npm run test:examples",
     "test:examples": "npm run test:node && npm run test:readme",
     "test:node": "ts-mocha -r tsconfig-paths/register -p tsconfig.node.json ./examples/src/tests/*.ts --parallel --jobs 4 --retries 3 --timeout 180000 --exit",
     "test:browser:parallel": "cypress-parallel -s test:browser -t 4 -d cypress/e2e -a '\"--quiet\"'",
     "test:browser": "cypress run --headless",
     "test:readme": "mocha ./tests/txm_readme.js --retries 3 --timeout 180000 --exit",
-    "test:unit": "wasm-pack test --release --node",
     "test:unit:node": "ts-mocha -p tsconfig.node.json ./tests/*.ts --parallel --exit",
     "cypress": "cypress open",
     "fmt": "dprint fmt"
@@ -70,8 +69,7 @@
     "tsconfig-paths": "^4.1.0",
     "txm": "^8.0.1",
     "typescript": "^4.7.2",
-    "wasm-opt": "^1.3.0",
-    "wasm-pack": "0.10.1"
+    "wasm-opt": "^1.3.0"
   },
   "dependencies": {
     "@iota/types": "^1.0.0-beta.11",

bindings/wasm/src/did/wasm_core_did.rs

@@ -134,13 +134,23 @@ impl From<CoreDID> for WasmCoreDID {
   }
 }
 
+impl From<&IToCoreDID> for CoreDID {
+  fn from(value: &IToCoreDID) -> Self {
+    get_core_did_clone(value).0
+  }
+}
+
 #[wasm_bindgen]
 extern "C" {
   #[wasm_bindgen(typescript_type = "CoreDID | IToCoreDID")]
   pub type IToCoreDID;
 
-  #[wasm_bindgen(method, js_name= toCoreDid)]
-  pub fn to_core_did(this: &IToCoreDID) -> WasmCoreDID;
+  // Specially crafted JS function called internally that ensures
+  // Custom DID implementations built on `CoreDID` don't get nulled
+  // out by Rust. Also avoids double clones when passing an instance of `CoreDID`
+  // or `IotaDID`.
+  #[wasm_bindgen(js_name = _getCoreDidCloneInternal, skip_typescript)]
+  pub fn get_core_did_clone(input: &IToCoreDID) -> WasmCoreDID;
 
 }
 

bindings/wasm/src/did/wasm_core_document.rs

@@ -533,6 +533,13 @@ impl WasmCoreDocument {
     WasmCoreDocument(self.0.clone())
   }
 
+  /// ### Warning
+  /// This is for internal use only. Do not rely on or call this method.
+  #[wasm_bindgen(js_name = _strongCountInternal)]
+  pub fn strong_count(&self) -> usize {
+    Rc::strong_count(&self.0)
+  }
+
   // ===========================================================================
   // Serialization
   // ===========================================================================

bindings/wasm/src/did/wasm_verification_method.rs

@@ -10,6 +10,7 @@ use crate::did::WasmMethodType;
 use crate::error::Result;
 use crate::error::WasmResult;
 use identity_iota::crypto::PublicKey;
+use identity_iota::did::CoreDID;
 use identity_iota::verification::VerificationMethod;
 use wasm_bindgen::prelude::*;
 
@@ -31,7 +32,7 @@ impl WasmVerificationMethod {
     fragment: String,
   ) -> Result<WasmVerificationMethod> {
     let public_key: PublicKey = PublicKey::from(publicKey);
-    VerificationMethod::new(did.to_core_did().0, keyType.into(), &public_key, &fragment)
+    VerificationMethod::new(CoreDID::from(did), keyType.into(), &public_key, &fragment)
       .map(Self)
       .wasm_result()
   }

bindings/wasm/src/iota/iota_document.rs

@@ -637,6 +637,13 @@ impl WasmIotaDocument {
     WasmIotaDocument(self.0.clone())
   }
 
+  /// ### Warning
+  /// This is for internal use only. Do not rely on or call this method.
+  #[wasm_bindgen(js_name = _strongCountInternal)]
+  pub fn strong_count(&self) -> usize {
+    Rc::strong_count(&self.0)
+  }
+
   // ===========================================================================
   // Serialization
   // ===========================================================================

bindings/wasm/tests/core.ts

@@ -17,6 +17,16 @@ const VALID_DID_EXAMPLE = "did:example:123";
 const KEY_BYTES = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
     25, 26, 27, 28, 29, 30, 31, 32]);
 
+class MockDID {
+    inner: CoreDID;
+    constructor(inner: CoreDID) {
+        this.inner = inner;
+    }
+    toCoreDid(): CoreDID {
+        return this.inner;
+    }
+}
+
 describe("CoreDID", function() {
     describe("#parse", function() {
         it("iota", () => {
@@ -94,6 +104,19 @@ describe("CoreDID", function() {
             assert.deepStrictEqual(CoreDID.validMethodId("method[brackets]"), false);
         });
     });
+    describe("#callingToCoreDid from Rust does not null out DIDs", function() {
+        it("should work", () => {
+            let didStr = "did:example:network:123";
+            let did = CoreDID.parse(didStr);
+            let mockDid = new MockDID(did);
+            const method = new VerificationMethod(mockDid, KeyType.Ed25519, KEY_BYTES, "key-0");
+            // Check that the VerificationMethod constructor does not null out mockDid.inner.
+            assert.deepStrictEqual(mockDid.inner.toString() as string, didStr as string);
+            // Also check for `CoreDID`
+            let method1 = new VerificationMethod(did, KeyType.Ed25519, KEY_BYTES, "key-1");
+            assert.deepStrictEqual(did.toString() as string, didStr as string);
+        });
+    });
 });
 
 describe("CoreDocument", function() {

bindings/wasm/tests/credentials.ts

@@ -289,6 +289,9 @@ describe("CredentialValidator, PresentationValidator", function() {
                     VerifierOptions.default(),
                 )
             );
+            // Check that we are not leaking memory in Rust.
+            assert.deepStrictEqual(issuerDoc._strongCountInternal() as number, 1);
+            assert.deepStrictEqual(subjectDoc._strongCountInternal() as number, 1);
 
             assert.doesNotThrow(() =>
                 CredentialValidator.validate(
@@ -322,6 +325,8 @@ describe("CredentialValidator, PresentationValidator", function() {
                 mockInheritedDocument.toCoreDocumentCalled,
                 false,
             );
+            // Also check that we don't leak memory.
+            assert.deepStrictEqual(mockInheritedDocument._strongCountInternal() as number, 1);
 
             // Characterisation test: Check that toCoreDocument DOES get called
             // when passing a mere implementer of IToCoreDocument (without inheriting from CoreDocument)
@@ -340,6 +345,9 @@ describe("CredentialValidator, PresentationValidator", function() {
                 true,
             );
 
+            // Also check that we don't leak memory.
+            assert.deepStrictEqual(mockIToCoreDocument.inner._strongCountInternal() as number, 1);
+
             // Characterisation test: Check that toCoreDocument does not get called
             // when passing `IotaDocument` (we use inheritance as a way of mocking a normal IotaDocument).
             let mockIotaDoc = new MockInheritedIotaDocument("iota");
@@ -386,6 +394,9 @@ describe("CredentialValidator, PresentationValidator", function() {
                     VerifierOptions.default(),
                 )
             );
+            // Check that we don't leak memory.
+            assert.deepStrictEqual(subjectDoc._strongCountInternal() as number, 1);
+
             assert.doesNotThrow(() =>
                 PresentationValidator.validate(
                     signedPresentation,
@@ -395,6 +406,10 @@ describe("CredentialValidator, PresentationValidator", function() {
                     FailFast.FirstError,
                 )
             );
+            // Check that we don't leak memory.
+            assert.deepStrictEqual(subjectDoc._strongCountInternal() as number, 1);
+            assert.deepStrictEqual(issuerDoc._strongCountInternal() as number, 1);
+
             assert.deepStrictEqual(
                 PresentationValidator.extractHolder(signedPresentation).toString(),
                 subjectDID.toString(),

bindings/wasm/tests/iota.ts

@@ -88,7 +88,6 @@ describe("IotaDocument", function() {
             const fragment = "new-method-1";
             const scope = MethodScope.AssertionMethod();
             const method = new VerificationMethod(doc.id(), KeyType.Ed25519, aliasIdBytes, fragment);
-
             // Add.
             doc.insertMethod(method, scope);
             // Resolve.
@@ -227,4 +226,11 @@ describe("IotaDocument", function() {
             assert.deepStrictEqual(doc.properties(), properties);
         });
     });
+    describe("#callingToCoreDid from Rust does not null out IotaDID", function() {
+        it("should work", () => {
+            const did = new IotaDID(aliasIdBytes, networkName);
+            const method = new VerificationMethod(did, KeyType.Ed25519, aliasIdBytes, "key-0");
+            assert.deepStrictEqual(did.toString(), "did:" + IotaDID.METHOD + ":" + networkName + ":" + aliasIdHex);
+        });
+    });
 });