Audit: MM Snap Edits (#117)

* Sayfer MM Snap Edits

- Throw an error when the account private key isn't found instead of silently failing
- Removed `getWallet` function from snapWallet.ts as well as it's associated tests and mocks
- Re arranged imports in api.ts
- Added documentation for snap
- Renamed jest.config.snap.js to jest.config.js so we don't have to explicitly pass in the config path to jest command

* Adjusted snap manifest

* MM Snap Coin Type Update

- Revert change making coin type 60 instead of 118. MM snaps do not allow you to access coin type 118 as of now.

* Added changeset

* Reverted package.json version bump

* Improved documentation

.changeset/curvy-squids-cough.md

@@ -0,0 +1,5 @@
+---
+'@sei-js/metamask-snap': patch
+---
+
+Fixes a silent return if the account private key is not found, adds documentation, removes unused getWallet function from @sei-js/metamask-snap all as fixes to code audit.

packages/metamask-snap/README.md

@@ -1,3 +1,65 @@
-## Sei Metamask Snap
+# Sei Metamask Snap
 
-This snap is under active development and is not yet ready for use until this packages has been properly audited by a third party.
+This documentation describes the functions and RPC request handler used in a MetaMask Snap to handle cryptocurrency transactions and account management using BIP44 standards.
+
+### `onRpcRequest`
+
+An RPC request handler that processes different request methods including `signDirect`, `signAmino`, and `getPrivateKey`. It handles the signing of transactions both in Amino and Direct mode, and also retrieves the private key.
+
+- **Syntax**: `onRpcRequest({ request }): Promise<TransactionSignature | PrivateKey>`
+- **Parameters**:
+  - `request`: An object representing the RPC request.
+- **Returns**: A Promise that resolves to a `TransactionSignature` or `PrivateKey`.
+
+## RPC Methods
+
+### `signDirect`
+
+Signs a transaction in Direct mode.
+
+- **Parameters**:
+  - `account_index`: The bip44 coin type 118 account index to use for signing.
+  - `signerAddress`: The address of the signer.
+  - `signDoc`: The document to be signed.
+- **Error Handling**: Throws an error if the private key is not available or if the user denies the transaction.
+
+### `signAmino`
+
+Signs a transaction in Amino mode.
+
+- **Parameters**:
+  - `account_index`: The bip44 coin type 118 account index to use for signing.
+  - `signerAddress`: The address of the signer.
+  - `signDoc`: The document to be signed.
+  - `enableExtraEntropy`: Boolean flag for extra entropy.
+  - `isADR36`: Boolean flag for ADR-36 compliance.
+- **Error Handling**: Throws an error if the private key is not available or if the user denies the transaction.
+
+
+### `getPrivateKey`
+
+Retrieves the private key for a specified account index using MetaMask's `snap_getBip44Entropy` method and BIP44 standards and coin type 118.
+
+- **Syntax**: `getPrivateKey(account_index: number = 0): Promise<PrivateKey>`
+- **Parameters**:
+  - `account_index` (number, optional): The index of the account, defaulting to 0.
+- **Returns**: A Promise resolving to a `PrivateKey`.
+
+
+## Local development and testing
+
+### Setup
+Run `yarn` to install dependencies.
+
+### Running the snap locally
+In order to run the snap locally simply run `yarn start` and the snap will be served at `localhost:8080`.
+
+### Testing the snap
+
+#### Test UI
+This package has a helpful UI for testing all the exposed functions.
+
+`cd` into the `./test-ui` directory and run `yarn` to install dependencies. Then run `yarn dev` to start the test UI. The test UI will be served at `localhost:3000`. Please checkout the README.md in that directory to learn how to set the snap ID.
+
+#### Unit tests
+Run `yarn test` to run the unit tests.

packages/metamask-snap/package.json

@@ -22,7 +22,7 @@
 		"serve": "mm-snap serve",
 		"start": "mm-snap watch",
 		"test": "yarn test:snap",
-		"test:snap": "jest --config ./jest.config.snap.js"
+		"test:snap": "jest"
 	},
 	"dependencies": {
 		"@metamask/snaps-types": "^3.1.0",

packages/metamask-snap/snap.manifest.json

@@ -1,13 +1,13 @@
 {
-  "version": "1.0.0",
+  "version": "1.0.2",
   "description": "A Metamask snap for Sei built with TypeScript",
   "proposedName": "Sei",
   "repository": {
     "type": "git",
     "url": "https://github.com/MetaMask/template-snap-monorepo.git"
   },
   "source": {
-    "shasum": "BL8YslCsPGPRZSF95ze0fYvr9D/ybmtGXZn19/PqDLs=",
+    "shasum": "Lg7E4N7U729gA6Ikfk3KgiZpNOfDVOmwR01y7C4fZKg=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

packages/metamask-snap/src/api.ts

@@ -1,12 +1,12 @@
+import { BIP44CoinTypeNode, getBIP44AddressKeyDeriver } from '@metamask/key-tree';
 import { OnRpcRequestHandler } from '@metamask/snaps-types';
-import { SignAminoRequest, SignDirectRequest } from './types';
+import { SignDoc } from 'cosmjs-types/cosmos/tx/v1beta1/tx';
 import Long from 'long';
-import { sanitizedUint8Array } from './utils';
-import { getAminoPanel, getDirectPanel } from './ui';
-import { BIP44CoinTypeNode, getBIP44AddressKeyDeriver } from '@metamask/key-tree';
-import { SnapRequest } from './types';
 import { SnapWallet } from './snapWallet';
-import { SignDoc } from 'cosmjs-types/cosmos/tx/v1beta1/tx';
+
+import { SignAminoRequest, SignDirectRequest, SnapRequest } from './types';
+import { getAminoPanel, getDirectPanel } from './ui';
+import { sanitizedUint8Array } from './utils';
 
 export const getPrivateKey = async (account_index: number = 0) => {
 	const bip44CoinNode = (await snap.request({
@@ -27,7 +27,7 @@ export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
 		case 'signDirect': {
 			const account = await getPrivateKey(account_index);
 
-			if (!account?.privateKey) return;
+			if (!account?.privateKey) throw new Error('Unable to get private key from MetaMask.');
 
 			const wallet = SnapWallet.create(account.privateKey);
 
@@ -62,7 +62,7 @@ export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
 
 			const account = await getPrivateKey(account_index);
 
-			if (!account?.privateKey) return;
+			if (!account?.privateKey) throw new Error('Unable to get private key from MetaMask.');
 
 			const wallet = SnapWallet.create(account.privateKey);
 
@@ -82,6 +82,6 @@ export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
 			return await getPrivateKey(account_index);
 		}
 		default:
-			throw new Error('Method not found.');
-	}
+			throw new Error('Function not found.');
+  }
 };

packages/metamask-snap/src/snapWallet.ts

@@ -1,13 +1,10 @@
 import { sign as signSecp256k1, getPublicKey as getSecp256k1PublicKey } from '@noble/secp256k1';
 import { sha256 } from '@noble/hashes/sha256';
 import { SignDoc } from 'cosmjs-types/cosmos/tx/v1beta1/tx';
-import { BIP44Node } from '@metamask/key-tree';
 import { AccountData, encodeSecp256k1Signature, StdSignDoc } from '@cosmjs/amino';
 import { Buffer } from 'buffer';
 import { compressedPubKeyToAddress, serializeAminoSignDoc, serializeDirectSignDoc } from '@sei-js/core';
 
-import { sendReqToSnap } from './utils';
-
 export class SnapWallet {
 	constructor(private privateKey: Uint8Array, private compressedPubKey: Uint8Array, private address: string) {}
 
@@ -76,12 +73,3 @@ export class SnapWallet {
 		};
 	}
 }
-
-export async function getWallet(account_index = 0, snapId: string): Promise<SnapWallet> {
-	const account: BIP44Node = await sendReqToSnap('getPrivateKey', { account_index }, snapId);
-
-	if (account.privateKey) {
-		return SnapWallet.create(account.privateKey);
-	}
-	throw new Error(`Error creating sei wallet!`);
-}

packages/metamask-snap/src/tests/mocks/mocks.ts

@@ -1,7 +1,3 @@
 export const ACCOUNT_ONE_PRIVATE_KEY = '0x0ca350a5d50a08caf95d0922d3adf4117a1ea5b7c24b7c6021173c8d46eceb37';
-export const ACCOUNT_ONE_PUBLIC_KEY =
-	'0x0405794d2d33a8a8c7e21caa891a9cb5b4539fb4c2fb50e46b326b322474ce719763ca0a0c869a87640289ebe4a2c008822059bbf7564129bafe51676abc53b921';
 export const ACCOUNT_ONE_ADDRESS = 'sei15u8zs9pqdjddgv8pkyyh6zvsg4ujs2y6s6cq6u';
-export const ACCOUNT_ONE_PUBKEY_BYTES = new Uint8Array([
-	3, 5, 121, 77, 45, 51, 168, 168, 199, 226, 28, 170, 137, 26, 156, 181, 180, 83, 159, 180, 194, 251, 80, 228, 107, 50, 107, 50, 36, 116, 206, 113, 151
-]);
+

packages/metamask-snap/src/tests/mocks/output.mock.ts

@@ -303,7 +303,7 @@ const output = {
 		}
 	},
 	failure: {
-		invalidMethod: 'Method not found.'
+		invalidMethod: 'Function not found.'
 	}
 };
 

packages/metamask-snap/src/tests/snapWallet.spec.ts

@@ -1,6 +1,5 @@
-import { getWallet, SnapWallet } from '../snapWallet';
-import * as Utils from '../utils';
-import { ACCOUNT_ONE_ADDRESS, ACCOUNT_ONE_PRIVATE_KEY, ACCOUNT_ONE_PUBKEY_BYTES, ACCOUNT_ONE_PUBLIC_KEY } from './mocks/mocks';
+import { SnapWallet } from '../snapWallet';
+import { ACCOUNT_ONE_ADDRESS, ACCOUNT_ONE_PRIVATE_KEY } from './mocks/mocks';
 
 describe('SnapWallet', () => {
 	it('should create a SnapWallet instance', () => {
@@ -17,35 +16,3 @@ describe('SnapWallet', () => {
 		expect(accounts[0].algo).toBe('secp256k1');
 	});
 });
-
-describe('getWallet', () => {
-	it('should create a SnapWallet instance via getWallet', async () => {
-		const expectedBip44Node = {
-			depth: 5,
-			parentFingerprint: 567825211,
-			index: 0,
-			privateKey: ACCOUNT_ONE_PRIVATE_KEY,
-			publicKey: ACCOUNT_ONE_PUBLIC_KEY,
-			chainCode: '0xf2a762ae70bddda87be9629e0be5857be287724fe6c5cc69c4c8cb1dcdccd089'
-		};
-
-		const mock = jest.spyOn(Utils, 'sendReqToSnap');
-		mock.mockResolvedValue(expectedBip44Node);
-
-		const wallet = await getWallet(0, 'npm:@sei-js/metamask-snap');
-		expect(wallet).toBeInstanceOf(SnapWallet);
-
-		const expectedAccounts = [
-			{
-				address: ACCOUNT_ONE_ADDRESS,
-				algo: 'secp256k1',
-				pubkey: ACCOUNT_ONE_PUBKEY_BYTES
-			}
-		];
-
-		const accountsResponse = wallet.getAccounts();
-		expect(accountsResponse).toStrictEqual(expectedAccounts);
-		expect(wallet.signAmino).toBeDefined();
-		expect(wallet.signDirect).toBeDefined();
-	});
-});

yarn.lock

@@ -6405,7 +6405,7 @@ [email protected]:
 
 babel-jest@^29.7.0:
   version "29.7.0"
-  resolved "https://registry.npmjs.org/babel-jest/-/babel-jest-29.7.0.tgz#f4369919225b684c56085998ac63dbd05be020d5"
+  resolved "https://registry.yarnpkg.com/babel-jest/-/babel-jest-29.7.0.tgz#f4369919225b684c56085998ac63dbd05be020d5"
   integrity sha512-BrvGY3xZSwEcCzKvKsCi2GgHqDqsYkOP4/by5xCgIwGXQxIEh+8ew3gmrE1y7XRR6LHZIj6yLYnUi/mm2KXKBg==
   dependencies:
     "@jest/transform" "^29.7.0"