Implement full offline signer capability (#37)

lebascou · web-flow · commit f4380ace3ed1 · 2022-01-03T19:00:17.000+01:00
* Improve abstraction to handle amino offline signer

* Bump version to 0.7.2

* Remove useless double check which might cause reflection issues

* Fix offline signing

Co-authored-by: Fabrice Bascoulergue &lt;lebascou@users.noreply.github.com&gt;
diff --git a/docs/lib/classes/lumwalletfactory.md b/docs/lib/classes/lumwalletfactory.md
@@ -92,7 +92,7 @@ Create a LumWallet instance based on an OfflineDirectSigner instance compatible
 
 | Name | Type | Description |
 | :------ | :------ | :------ |
-| `offlineSigner` | `OfflineDirectSigner` | OfflineDirectSigner instance compatible with Comsjs based implementations |
+| `offlineSigner` | `OfflineSigner` | OfflineDirectSigner instance compatible with Comsjs based implementations |
 
 #### Returns
 
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
     "name": "@lum-network/sdk-javascript",
-    "version": "0.7.1",
+    "version": "0.7.2",
     "license": "Apache-2.0",
     "description": "Javascript SDK library for NodeJS and Web browsers to interact with the Lum Network.",
     "homepage": "https://github.com/lum-network/sdk-javascript#readme",
diff --git a/src/wallet/LumOfflineSignerWallet.ts b/src/wallet/LumOfflineSignerWallet.ts
@@ -1,27 +1,33 @@
-import { OfflineDirectSigner } from '@cosmjs/proto-signing';
+import Long from 'long';
+import { OfflineAminoSigner } from '@cosmjs/amino';
+import { OfflineSigner, OfflineDirectSigner } from '@cosmjs/proto-signing';
 import { SignMode } from '../codec/cosmos/tx/signing/v1beta1/signing';
-import { LumUtils, LumTypes, LumConstants } from '..';
+import { LumUtils, LumTypes, LumConstants, LumAminoRegistry } from '..';
 import { LumWallet } from '.';
-import Long from 'long';
 
 export class LumOfflineSignerWallet extends LumWallet {
-    private readonly offlineSigner: OfflineDirectSigner;
+    private readonly offlineSigner: OfflineSigner;
 
     /**
-     * Create a LumOfflineSignerWallet instance based on an OfflineDirectSigner instance compatible with Comsjs based
+     * Create a LumOfflineSignerWallet instance based on an OfflineSigner instance compatible with Comsjs based
      * implementations.
      * This constructor is not intended to be used directly as it does not initialize the underlying key pair
      * Better use the provided static LumPaperWallet builders
      *
      * @param mnemonicOrPrivateKey mnemonic (string) used to derive the private key or private key (Uint8Array)
      */
-    constructor(offlineSigner: OfflineDirectSigner) {
+    constructor(offlineSigner: OfflineSigner) {
         super();
         this.offlineSigner = offlineSigner;
     }
 
     signingMode = (): SignMode => {
-        return SignMode.SIGN_MODE_DIRECT;
+        if (typeof (this.offlineSigner as OfflineAminoSigner).signAmino === 'function') {
+            return SignMode.SIGN_MODE_LEGACY_AMINO_JSON;
+        } else if (typeof (this.offlineSigner as OfflineDirectSigner).signDirect === 'function') {
+            return SignMode.SIGN_MODE_DIRECT;
+        }
+        throw 'Unknown offline signer mode';
     };
 
     canChangeAccount = (): boolean => {
@@ -53,9 +59,27 @@ export class LumOfflineSignerWallet extends LumWallet {
         if (signerIndex === -1) {
             throw new Error('Signer not found in document');
         }
-        const signDoc = LumUtils.generateSignDoc(doc, signerIndex, this.signingMode());
-        const response = await this.offlineSigner.signDirect(this.address, signDoc);
-        return [response.signed, LumUtils.fromBase64(response.signature.signature)];
+        if (this.signingMode() === SignMode.SIGN_MODE_DIRECT) {
+            const signDoc = LumUtils.generateSignDoc(doc, signerIndex, this.signingMode());
+            const response = await (this.offlineSigner as OfflineDirectSigner).signDirect(this.address, signDoc);
+            return [response.signed, LumUtils.fromBase64(response.signature.signature)];
+        } else if (this.signingMode() === SignMode.SIGN_MODE_LEGACY_AMINO_JSON) {
+            const response = await (this.offlineSigner as OfflineAminoSigner).signAmino(this.address, {
+                'account_number': doc.signers[signerIndex].accountNumber.toString(),
+                'chain_id': doc.chainId,
+                'fee': doc.fee,
+                'memo': doc.memo || '',
+                'msgs': doc.messages.map((msg) => LumAminoRegistry.toAmino(msg)),
+                'sequence': doc.signers[signerIndex].sequence.toString(),
+            });
+            if (response.signed) {
+                // Fees and memo could have been edited by the offline signer
+                doc.fee = response.signed.fee;
+                doc.memo = response.signed.memo;
+            }
+            return [LumUtils.generateSignDoc(doc, signerIndex, this.signingMode()), LumUtils.fromBase64(response.signature.signature)];
+        }
+        throw 'Unknown offline signer mode';
     };
 
     signMessage = async (msg: string): Promise<LumTypes.SignMsg> => {
@@ -68,14 +92,19 @@ export class LumOfflineSignerWallet extends LumWallet {
             chainId: LumConstants.LumSignOnlyChainId,
             accountNumber: Long.fromNumber(0),
         };
-        const response = await this.offlineSigner.signDirect(this.address, signDoc);
-        return {
-            address: this.getAddress(),
-            publicKey: this.getPublicKey(),
-            msg: msg,
-            sig: LumUtils.fromBase64(response.signature.signature),
-            version: LumConstants.LumWalletSigningVersion,
-            signer: LumConstants.LumMessageSigner.OFFLINE,
-        };
+        if (this.signingMode() === SignMode.SIGN_MODE_DIRECT) {
+            const response = await (this.offlineSigner as OfflineDirectSigner).signDirect(this.address, signDoc);
+            return {
+                address: this.getAddress(),
+                publicKey: this.getPublicKey(),
+                msg: msg,
+                sig: LumUtils.fromBase64(response.signature.signature),
+                version: LumConstants.LumWalletSigningVersion,
+                signer: LumConstants.LumMessageSigner.OFFLINE,
+            };
+        } else if (typeof (this.offlineSigner as OfflineAminoSigner).signAmino === 'function') {
+            throw 'Feature not available for amino signers';
+        }
+        throw 'Unknown offline signer mode';
     };
 }
diff --git a/src/wallet/LumWalletFactory.ts b/src/wallet/LumWalletFactory.ts
@@ -1,5 +1,5 @@
 import Transport from '@ledgerhq/hw-transport';
-import { OfflineDirectSigner } from '@cosmjs/proto-signing';
+import { OfflineSigner } from '@cosmjs/proto-signing';
 
 import { LumWallet } from './LumWallet';
 import { LumLedgerWallet } from './LumLedgerWallet';
@@ -52,7 +52,7 @@ export class LumWalletFactory {
      *
      * @param offlineSigner OfflineDirectSigner instance compatible with Comsjs based implementations
      */
-    static fromOfflineSigner = async (offlineSigner: OfflineDirectSigner): Promise<LumWallet> => {
+    static fromOfflineSigner = async (offlineSigner: OfflineSigner): Promise<LumWallet> => {
         const wallet = new LumOfflineSignerWallet(offlineSigner);
         await wallet.useAccount();
         return wallet;
diff --git a/tests/wallet.test.ts b/tests/wallet.test.ts
@@ -1,10 +1,11 @@
 import { AccountData, DirectSignResponse, OfflineDirectSigner } from '@cosmjs/proto-signing';
 
 import { SignDoc } from '../src/codec/cosmos/tx/v1beta1/tx';
-import { LumWallet, LumWalletFactory, LumUtils, LumConstants, LumMessages } from '../src';
-import { encodeSecp256k1Signature } from '@cosmjs/amino';
+import { LumWallet, LumWalletFactory, LumUtils, LumConstants, LumMessages, LumRegistry, LumAminoRegistry } from '../src';
+import { AminoSignResponse, encodeSecp256k1Signature, OfflineAminoSigner, StdSignDoc } from '@cosmjs/amino';
+import { SignMode } from '../src/codec/cosmos/tx/signing/v1beta1/signing';
 
-class FakeOfflineSigner implements OfflineDirectSigner {
+class FakeOfflineDirectSigner implements OfflineDirectSigner {
     private readonly privateKey: Uint8Array;
 
     constructor(privateKey: Uint8Array) {
@@ -35,6 +36,48 @@ class FakeOfflineSigner implements OfflineDirectSigner {
     };
 }
 
+class FakeOfflineAminoSigner implements OfflineAminoSigner {
+    private readonly privateKey: Uint8Array;
+
+    constructor(privateKey: Uint8Array) {
+        this.privateKey = privateKey;
+    }
+
+    getAccounts = async (): Promise<AccountData[]> => {
+        const publicKey = await LumUtils.getPublicKeyFromPrivateKey(this.privateKey);
+        return [
+            {
+                pubkey: publicKey,
+                address: LumUtils.getAddressFromPublicKey(publicKey),
+                algo: 'secp256k1',
+            },
+        ];
+    };
+
+    signAmino = async (signerAddress: string, stdSignDoc: StdSignDoc): Promise<AminoSignResponse> => {
+        const publicKey = await LumUtils.getPublicKeyFromPrivateKey(this.privateKey);
+        const signDoc = LumUtils.generateSignDoc(
+            {
+                chainId: stdSignDoc.chain_id,
+                fee: stdSignDoc.fee,
+                memo: stdSignDoc.memo,
+                messages: stdSignDoc.msgs.map((aminoMsg) => LumAminoRegistry.fromAmino(aminoMsg)),
+                signers: [{ accountNumber: parseInt(stdSignDoc.account_number), sequence: parseInt(stdSignDoc.sequence), publicKey: publicKey }],
+            },
+            0,
+            SignMode.SIGN_MODE_DIRECT, // Simulated to enable signature comparison during tests
+        );
+        const signBytes = LumUtils.generateSignDocBytes(signDoc);
+        const hashedMessage = LumUtils.sha256(signBytes);
+        const signature = await LumUtils.generateSignature(hashedMessage, this.privateKey);
+        const stdSig = encodeSecp256k1Signature(publicKey, signature);
+        return {
+            signed: stdSignDoc,
+            signature: stdSig,
+        };
+    };
+}
+
 describe('LumWallet', () => {
     it('Should be identical from mnemonic, privatekey and keystore recovery', async () => {
         const mnemonic = 'surround miss nominee dream gap cross assault thank captain prosper drop duty group candy wealth weather scale put';
@@ -45,7 +88,8 @@ describe('LumWallet', () => {
         const w1 = await LumWalletFactory.fromMnemonic(mnemonic, `m/44'/837'/0'/0/0`);
         const w2 = await LumWalletFactory.fromPrivateKey(LumUtils.keyFromHex(privateKey));
         const w3 = await LumWalletFactory.fromKeyStore(keystore, 'lumiere');
-        const w4 = await LumWalletFactory.fromOfflineSigner(new FakeOfflineSigner(LumUtils.keyFromHex(privateKey)));
+        const w4 = await LumWalletFactory.fromOfflineSigner(new FakeOfflineDirectSigner(LumUtils.keyFromHex(privateKey)));
+        const w5 = await LumWalletFactory.fromOfflineSigner(new FakeOfflineAminoSigner(LumUtils.keyFromHex(privateKey)));
 
         expect(LumUtils.isAddressValid(w1.getAddress())).toBe(true);
         expect(LumUtils.isAddressValid(w1.getAddress(), LumConstants.LumBech32PrefixAccAddr)).toBe(true);
@@ -71,17 +115,27 @@ describe('LumWallet', () => {
             ],
         };
 
+        const w1Response = await w1.signTransaction(doc);
+        expect(LumUtils.verifySignature(w1Response[1], LumUtils.sha256(LumUtils.generateSignDocBytes(w1Response[0])), w5.getPublicKey()));
+
         expect(w1.getAddress()).toEqual(w2.getAddress());
         expect(w1.getPublicKey()).toEqual(w2.getPublicKey());
-        expect(await w1.signTransaction(doc)).toEqual(await w2.signTransaction(doc));
+        expect(w1Response).toEqual(await w2.signTransaction(doc));
 
         expect(w1.getAddress()).toEqual(w3.getAddress());
         expect(w1.getPublicKey()).toEqual(w3.getPublicKey());
-        expect(await w1.signTransaction(doc)).toEqual(await w3.signTransaction(doc));
+        expect(w1Response).toEqual(await w3.signTransaction(doc));
 
         expect(w1.getAddress()).toEqual(w4.getAddress());
         expect(w1.getPublicKey()).toEqual(w4.getPublicKey());
-        expect(await w1.signTransaction(doc)).toEqual(await w4.signTransaction(doc));
+        expect(w1Response).toEqual(await w4.signTransaction(doc));
+
+        expect(w1.getAddress()).toEqual(w5.getAddress());
+        expect(w1.getPublicKey()).toEqual(w5.getPublicKey());
+        // Signature will differ due to the SignMode use but should still be valid
+        const w5Response = await w5.signTransaction(doc);
+        expect(w1Response).not.toEqual(w5Response);
+        expect(LumUtils.verifySignature(w5Response[1], LumUtils.sha256(LumUtils.generateSignDocBytes(w5Response[0])), w5.getPublicKey()));
 
         const randomPrivateKey = LumUtils.generatePrivateKey();
         expect(randomPrivateKey).toHaveLength(LumConstants.PrivateKeyLength);
@@ -101,7 +155,7 @@ describe('LumWallet', () => {
 
         const w1 = await LumWalletFactory.fromMnemonic(mnemonic);
         const w2 = await LumWalletFactory.fromMnemonic(LumUtils.generateMnemonic());
-        const w3 = await LumWalletFactory.fromOfflineSigner(new FakeOfflineSigner(LumUtils.keyFromHex(privateKey)));
+        const w3 = await LumWalletFactory.fromOfflineSigner(new FakeOfflineDirectSigner(LumUtils.keyFromHex(privateKey)));
 
         const signedW1 = await w1.signMessage(message);
         expect(signedW1.signer).toEqual(LumConstants.LumMessageSigner.PAPER);

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@lum-network/sdk-javascript",`
`3`		`- "version": "0.7.1",`
	`3`	`+ "version": "0.7.2",`
`4`	`4`	`"license": "Apache-2.0",`
`5`	`5`	`"description": "Javascript SDK library for NodeJS and Web browsers to interact with the Lum Network.",`
`6`	`6`	`"homepage": "https://github.com/lum-network/sdk-javascript#readme",`