From b8c8473152751577dc2053f32a72f1f5edd2d6a4 Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Wed, 31 Mar 2021 21:39:13 -0700 Subject: [PATCH] Remove other uses of elliptic, remove padHex, some cleanup --- umbra-js/src/classes/KeyPair.ts | 97 ++++++++++++++---------------- umbra-js/src/utils/cns.ts | 10 +-- umbra-js/src/utils/utils.ts | 44 ++++---------- umbra-js/test/RandomNumber.test.ts | 5 +- umbra-js/test/utils.test.ts | 15 ----- 5 files changed, 64 insertions(+), 107 deletions(-) diff --git a/umbra-js/src/classes/KeyPair.ts b/umbra-js/src/classes/KeyPair.ts index df853e335..5c19987e6 100644 --- a/umbra-js/src/classes/KeyPair.ts +++ b/umbra-js/src/classes/KeyPair.ts @@ -2,16 +2,33 @@ * @dev Class for managing secp256k1 keys and performing operations with them */ -import * as secp from 'noble-secp256k1'; +import { getSharedSecret as secpGetSharedSecret, getPublicKey, Point, CURVE } from 'noble-secp256k1'; import { Wallet } from 'ethers'; import { BigNumber } from '@ethersproject/bignumber'; import { hexZeroPad, isHexString } from '@ethersproject/bytes'; import { sha256 } from '@ethersproject/sha2'; import { computeAddress } from '@ethersproject/transactions'; import { RandomNumber } from './RandomNumber'; -import { lengths, padHex, recoverPublicKeyFromTransaction } from '../utils/utils'; +import { lengths, recoverPublicKeyFromTransaction } from '../utils/utils'; import { CompressedPublicKey, EncryptedPayload, EthersProvider } from '../types'; +/** + * @notice Private helper method to return the shared secret for a given private key and public key + * @param privateKey Private key as hex string with 0x prefix + * @param publicKey Uncompressed public key as hex string with 0x04 prefix + * @returns 32-byte shared secret as 66 character hex string + */ +function getSharedSecret(privateKey: string, publicKey: string) { + if (privateKey.length !== lengths.privateKey || !isHexString(privateKey)) throw new Error('Invalid private key'); + if (publicKey.length !== lengths.publicKey || !isHexString(publicKey)) throw new Error('Invalid public key'); + + // We use sharedSecret.slice(2) to ensure the shared secret is not dependent on the prefix, which enables + // us to uncompress ephemeralPublicKey from Umbra.sol logs as explained in comments of getUncompressedFromX. + // Note that a shared secret is really just a point on the curve, so it's an uncompressed public key + const sharedSecret = secpGetSharedSecret(privateKey.slice(2), publicKey.slice(2), true) as string; // has 04 prefix but not 0x + return sha256(`0x${sharedSecret.slice(2)}`); +} + export class KeyPair { readonly publicKeyHex: string; // Public key as hex string with 0x04 prefix readonly privateKeyHex: string | null = null; // Private key as hex string with 0x prefix, or null if not provided @@ -30,7 +47,7 @@ export class KeyPair { if (key.length === lengths.privateKey) { // Private key provided this.privateKeyHex = key; - const publicKey = secp.getPublicKey(this.privateKeyHexSlim as string); // hex without 0x prefix but with 04 prefix + const publicKey = getPublicKey(this.privateKeyHexSlim as string); // hex without 0x prefix but with 04 prefix this.publicKeyHex = `0x${publicKey}`; // Save off version with 0x prefix, other forms computed as getters } else if (key.length === lengths.publicKey) { // Public key provided @@ -43,17 +60,17 @@ export class KeyPair { // ===================================================== GETTERS ===================================================== /** - * @notice Returns the private key as an ethers BigNumber + * @notice Returns the private key as a hex string without the 0x prefix */ - get privateKeyBN() { - return this.privateKeyHex ? BigNumber.from(this.privateKeyHex) : null; + get privateKeyHexSlim() { + return this.privateKeyHex ? this.privateKeyHex.slice(2) : null; } /** - * @notice Returns the private key as a hex string without the 0x prefix + * @notice Returns the uncompressed public key as a hex string without the 0x prefix */ - get privateKeyHexSlim() { - return this.privateKeyHex ? this.privateKeyHex.slice(2) : null; + get publicKeyHexSlim() { + return this.publicKeyHex.slice(2); } /** @@ -66,20 +83,20 @@ export class KeyPair { // ============================================= ENCRYPTION / DECRYPTION ============================================= /** - * @notice Encrypt a random number with the instance's public key - * @param randomNumber Random number as instance of RandomNumber class + * @notice Encrypt a number with the instance's public key + * @param randomNumber Number as instance of RandomNumber class * @returns Hex strings of uncompressed 65 byte public key and 32 byte ciphertext */ - encrypt(randomNumber: RandomNumber): EncryptedPayload { - if (!(randomNumber instanceof RandomNumber)) { + encrypt(number: RandomNumber): EncryptedPayload { + if (!(number instanceof RandomNumber)) { throw new Error('Must provide instance of RandomNumber'); } // Get shared secret to use as encryption key const ephemeralWallet = Wallet.createRandom(); - const sharedSecret = KeyPair.getSharedSecret(ephemeralWallet.privateKey, this.publicKeyHex); + const sharedSecret = getSharedSecret(ephemeralWallet.privateKey, this.publicKeyHex); // XOR random number with shared secret to get encrypted value - const ciphertext = randomNumber.value.xor(sharedSecret); + const ciphertext = number.value.xor(sharedSecret); const result = { ephemeralPublicKey: ephemeralWallet.publicKey, // hex string with 0x04 prefix ciphertext: hexZeroPad(ciphertext.toHexString(), 32), // hex string with 0x prefix @@ -102,7 +119,7 @@ export class KeyPair { } // Get shared secret to use as decryption key, then decrypt with XOR - const sharedSecret = KeyPair.getSharedSecret(this.privateKeyHex, ephemeralPublicKey); + const sharedSecret = getSharedSecret(this.privateKeyHex, ephemeralPublicKey); const plaintext = BigNumber.from(ciphertext).xor(sharedSecret); return hexZeroPad(plaintext.toHexString(), 32); } @@ -120,14 +137,13 @@ export class KeyPair { throw new Error('Strings must be in hex form with 0x prefix'); } + // Parse number based on input type const number = isHexString(value) ? BigInt(value as string) // provided a valid hex string : BigInt((value as RandomNumber).asHex); // provided RandomNumber - // Perform the multiplication - const publicKey = secp.Point.fromHex(this.publicKeyHex.slice(2)).multiply(number); - - // Return new KeyPair instance + // Perform the multiplication and return new KeyPair instance + const publicKey = Point.fromHex(this.publicKeyHexSlim).multiply(number); return new KeyPair(`0x${publicKey.toHex()}`); } @@ -142,25 +158,20 @@ export class KeyPair { if (typeof value === 'string' && !isHexString(value)) { throw new Error('Strings must be in hex form with 0x prefix'); } - if (!this.privateKeyBN) { + if (!this.privateKeyHex) { throw new Error('KeyPair has no associated private key'); } - // Parse the number provided + // Parse number based on input type const number = isHexString(value) ? BigInt(value as string) // provided a valid hex string : BigInt((value as RandomNumber).asHex); // provided RandomNumber // Get new private key. Multiplication gives us an arbitrarily large number that is not necessarily in the domain - // of the secp256k1 curve, so then we use modulus operation to get in the correct range. We save it as a BigNumber - // for converting to hex - const privateKeyMod = BigNumber.from((BigInt(this.privateKeyHex) * number) % secp.CURVE.n); - - // Remove 0x prefix to pad hex value, then add back 0x prefix - const privateKey = `0x${padHex(privateKeyMod.toHexString().slice(2))}`; - - // Return new KeyPair instance - return new KeyPair(privateKey); + // of the secp256k1 curve, so then we use modulus operation to get in the correct range. + const privateKeyBigInt = (BigInt(this.privateKeyHex) * number) % CURVE.n; + const privateKey = hexZeroPad(BigNumber.from(privateKeyBigInt).toHexString(), 32); // convert to 32 byte hex + return new KeyPair(privateKey); // return new KeyPair instance } // ================================================= STATIC METHODS ================================================== @@ -177,22 +188,6 @@ export class KeyPair { return new KeyPair(publicKeyHex); } - /** - * @notice Returns the shared secret for a given private key and public key - * @param privateKey Private key as hex string with 0x prefix - * @param publicKey Uncompressed public key as hex string with 0x04 prefix - * @returns 32-byte shared secret as 66 character hex string - */ - static getSharedSecret(privateKey: string, publicKey: string) { - if (privateKey.length !== lengths.privateKey || !isHexString(privateKey)) throw new Error('Invalid private key'); - if (publicKey.length !== lengths.publicKey || !isHexString(publicKey)) throw new Error('Invalid public key'); - - // We use getSharedSecret(pk,Pk,true).slice() to ensure the shared secret is not dependent on the prefix, which - // enables us to uncompress ephemeralPublicKey from Umbra.sol logs as explained in comments of getUncompressedFromX - const sharedSecretRaw = secp.getSharedSecret(privateKey.slice(2), publicKey.slice(2), true).slice(2); - return sha256(`0x${sharedSecretRaw}`); - } - /** * @notice Takes an uncompressed public key and returns the compressed public key * @param publicKey Uncompressed public key, as hex string starting with 0x @@ -202,7 +197,7 @@ export class KeyPair { if (typeof publicKey !== 'string' || !isHexString(publicKey) || publicKey.length !== lengths.publicKey) { throw new Error('Must provide uncompressed public key as hex string'); } - const compressedPublicKey = secp.Point.fromHex(publicKey.slice(2)).toHex(true); + const compressedPublicKey = Point.fromHex(publicKey.slice(2)).toHex(true); return { prefix: Number(compressedPublicKey[1]), // prefix bit is the 2th character in the string (no 0x prefix) pubKeyXCoordinate: `0x${compressedPublicKey.slice(2)}`, @@ -226,12 +221,12 @@ export class KeyPair { if (!(pkx instanceof BigNumber) && typeof pkx !== 'string') { throw new Error('Compressed public key must be a BigNumber or string'); } - const hexWithoutPrefix = padHex(BigNumber.from(pkx).toHexString().slice(2)); + const hexWithoutPrefix = hexZeroPad(BigNumber.from(pkx).toHexString(), 32).slice(2); // pkx as hex string without 0x prefix if (!prefix) { // Only safe to use this branch when uncompressed key is using for scanning your funds - return `0x${secp.Point.fromHex(`02${hexWithoutPrefix}`).toHex()}`; + return `0x${Point.fromHex(`02${hexWithoutPrefix}`).toHex()}`; } const hexWithPrefix = `0${Number(prefix)}${hexWithoutPrefix}`; - return `0x${secp.Point.fromHex(hexWithPrefix).toHex()}`; + return `0x${Point.fromHex(hexWithPrefix).toHex()}`; } } diff --git a/umbra-js/src/utils/cns.ts b/umbra-js/src/utils/cns.ts index d593e3ff1..cc742cf75 100644 --- a/umbra-js/src/utils/cns.ts +++ b/umbra-js/src/utils/cns.ts @@ -1,7 +1,7 @@ /** * @dev Functions for interacting with the Unstoppable Domains Crypto Name Service (CNS) */ -import { computePublicKey } from '@ethersproject/signing-key'; +import { Point } from 'noble-secp256k1'; import { default as Resolution } from '@unstoppabledomains/resolution'; import type { EthersProvider, TransactionResponse } from '../types'; import * as cnsResolverAbi from '../abi/CnsResolver.json'; @@ -62,8 +62,8 @@ export async function getPublicKeys(name: string, provider: EthersProvider, reso throw new Error(`Public keys not found for ${name}. User must setup their Umbra account`); } // Return uncompressed public keys - const spendingPublicKey = computePublicKey(compressedSpendingPublicKey); - const viewingPublicKey = computePublicKey(compressedViewingPublicKey); + const spendingPublicKey = `0x${Point.fromHex(compressedSpendingPublicKey.slice(2)).toHex()}`; + const viewingPublicKey = `0x${Point.fromHex(compressedViewingPublicKey.slice(2)).toHex()}`; return { spendingPublicKey, viewingPublicKey }; } @@ -84,8 +84,8 @@ export async function setPublicKeys( resolution: Resolution ) { // Compress public keys - const compressedSpendingPublicKey = computePublicKey(spendingPublicKey, true); - const compressedViewingPublicKey = computePublicKey(viewingPublicKey, true); + const compressedSpendingPublicKey = `0x${Point.fromHex(spendingPublicKey.slice(2)).toHex(true)}`; + const compressedViewingPublicKey = `0x${Point.fromHex(viewingPublicKey.slice(2)).toHex(true)}`; // Send transaction to set the keys const domainNamehash = resolution.namehash(name); diff --git a/umbra-js/src/utils/utils.ts b/umbra-js/src/utils/utils.ts index b8edb4f73..907226c67 100644 --- a/umbra-js/src/utils/utils.ts +++ b/umbra-js/src/utils/utils.ts @@ -2,16 +2,16 @@ * @dev Assortment of helper methods */ +import { Signature, recoverPublicKey } from 'noble-secp256k1'; import { Contract, ContractInterface } from 'ethers'; -import { arrayify, Bytes, Hexable, isHexString, joinSignature } from '@ethersproject/bytes'; +import { arrayify, Bytes, Hexable, isHexString, splitSignature } from '@ethersproject/bytes'; import { keccak256 } from '@ethersproject/keccak256'; import { resolveProperties } from '@ethersproject/properties'; import { EtherscanProvider } from '@ethersproject/providers'; -import { recoverPublicKey } from '@ethersproject/signing-key'; import { serialize as serializeTransaction } from '@ethersproject/transactions'; import { ens, cns } from '..'; import { DomainService } from '../classes/DomainService'; -import { EthersProvider, SignatureLike } from '../types'; +import { EthersProvider } from '../types'; // Lengths of various properties when represented as full hex strings export const lengths = { @@ -21,26 +21,6 @@ export const lengths = { publicKey: 132, // 64 bytes + 0x04 prefix }; -/** - * @notice Adds leading zeroes to ensure hex strings are the expected length. - * @dev We always expect a hex value to have the full number of characters for its size, - * so we use this tool to ensure no errors occur due to wrong hex character lengths. - * Specifically, we need to pad hex values during the following cases: - * 1. It seems elliptic strips unnecessary leading zeros when pulling out x and y - * coordinates from public keys. - * 2. When computing a new private key from a random number, the new number (i.e. the new - * private key) may not necessarily require all 32-bytes as ethers.js also seems to - * strip leading zeroes. - * 3. When generating random numbers and returning them as hex strings, the leading - * zero bytes get stripped - * @param hex String to pad, without leading 0x - * @param bytes Number of bytes string should have - */ -export function padHex(hex: string, bytes = 32) { - if (!isHexString(`0x${hex}`)) throw new Error('Input must be a hex string without the 0x prefix'); - return hex.padStart(bytes * 2, '0'); -} - /** * @notice Convert hex string with 0x prefix into Buffer * @param value Hex string to convert @@ -66,11 +46,7 @@ export async function recoverPublicKeyFromTransaction(txHash: string, provider: throw new Error('Transaction not found. Are the provider and transaction hash on the same network?'); } - // Get original signature - const splitSignature: SignatureLike = { r: tx.r as string, s: tx.s, v: tx.v }; - const signature = joinSignature(splitSignature); - - // Reconstruct transaction data that was originally signed + // Reconstruct transaction payload that was originally signed const txData = { chainId: tx.chainId, data: tx.data, @@ -81,15 +57,17 @@ export async function recoverPublicKeyFromTransaction(txHash: string, provider: value: tx.value, }; - // Properly format it to get the correct message + // Properly format transaction payload to get the correct message const resolvedTx = await resolveProperties(txData); const rawTx = serializeTransaction(resolvedTx); const msgHash = keccak256(rawTx); - const msgBytes = arrayify(msgHash); - // Recover sender's public key and address - const publicKey = recoverPublicKey(msgBytes, signature); - return publicKey; + // Recover sender's public key + const signature = new Signature(BigInt(tx.r), BigInt(tx.s)); + const recoveryParam = splitSignature({ r: tx.r as string, s: tx.s, v: tx.v }).recoveryParam; + const publicKey = recoverPublicKey(msgHash.slice(2), signature.toHex(), recoveryParam); // without 0x prefix + if (!publicKey) throw new Error('Could not recover public key'); + return `0x${publicKey}`; } /** diff --git a/umbra-js/test/RandomNumber.test.ts b/umbra-js/test/RandomNumber.test.ts index 575713767..1c4e8eaeb 100644 --- a/umbra-js/test/RandomNumber.test.ts +++ b/umbra-js/test/RandomNumber.test.ts @@ -1,9 +1,8 @@ import { RandomNumber } from '../src/classes/RandomNumber'; import * as chai from 'chai'; import { BigNumber } from '@ethersproject/bignumber'; -import { isHexString } from '@ethersproject/bytes'; +import { isHexString, hexZeroPad } from '@ethersproject/bytes'; import { randomBytes } from '@ethersproject/random'; -import { padHex } from '../src/utils/utils'; const { expect } = chai; const numberOfRuns = 1000; // number of runs for tests that execute in a loop @@ -87,7 +86,7 @@ describe('RandomNumber class', () => { it('lets the user set a payload extension when generating a random number', () => { for (let i = 0; i < numberOfRuns; i += 1) { // Generate random hex string with the correct format - const randomHexString = `0x${padHex(BigNumber.from(randomBytes(16)).toHexString().slice(2), 16)}`; + const randomHexString = hexZeroPad(BigNumber.from(randomBytes(16)).toHexString(), 16); random = new RandomNumber(randomHexString); const hex = random.asHex; diff --git a/umbra-js/test/utils.test.ts b/umbra-js/test/utils.test.ts index 17af82092..ac488d616 100644 --- a/umbra-js/test/utils.test.ts +++ b/umbra-js/test/utils.test.ts @@ -36,15 +36,6 @@ const expectRejection = async (promise: Promise, message: string) => { describe('Utilities', () => { describe('Helpers', () => { - it('properly pads hex values', async () => { - const shortHex = '1234'; - const fullHex16 = '00000000000000000000000000001234'; - const fullHex32 = '0000000000000000000000000000000000000000000000000000000000001234'; - expect(utils.padHex(shortHex)).to.equal(fullHex32); - expect(utils.padHex(shortHex, 32)).to.equal(fullHex32); - expect(utils.padHex(shortHex, 16)).to.equal(fullHex16); - }); - it('recovers public keys from transactions', async () => { const hash = '0x45fa716ee2d484ac67ef787625908072d851bfa369db40567e16ee08a7fdefd2'; expect(await utils.recoverPublicKeyFromTransaction(hash, ethersProvider)).to.equal(publicKey); @@ -105,12 +96,6 @@ describe('Utilities', () => { // ts-expect-error statements needed throughout this section to bypass TypeScript checks that would stop this file // from being compiled/ran - it('throws when padHex is given a bad input', () => { - const errorMsg = 'Input must be a hex string without the 0x prefix'; - expect(() => utils.padHex('q')).to.throw(errorMsg); - expect(() => utils.padHex('0x1')).to.throw(errorMsg); - }); - it('throws when recoverPublicKeyFromTransaction is given a bad transaction hash', async () => { const errorMsg = 'Invalid transaction hash provided'; await expectRejection(utils.recoverPublicKeyFromTransaction('q', ethersProvider), errorMsg);