diff --git a/umbra-js/package.json b/umbra-js/package.json index 0c279a686..5b059a086 100644 --- a/umbra-js/package.json +++ b/umbra-js/package.json @@ -25,16 +25,14 @@ "bn.js": "^5.1.3", "buffer": "^6.0.2", "dotenv": "^8.2.0", - "elliptic": "^6.5.4", "eth-ens-namehash": "^2.0.8", "ethers": "^5.0.25", - "js-sha3": "^0.8.0" + "noble-secp256k1": "^1.1.2" }, "devDependencies": { "@openzeppelin/test-environment": "^0.1.6", "@types/bn.js": "^5.1.0", "@types/chai": "^4.2.14", - "@types/elliptic": "^6.4.12", "@types/mocha": "^8.0.4", "@umbra/contracts": "^0.0.1", "chai": "^4.2.0", diff --git a/umbra-js/src/classes/KeyPair.ts b/umbra-js/src/classes/KeyPair.ts index aecf0afdb..5c19987e6 100644 --- a/umbra-js/src/classes/KeyPair.ts +++ b/umbra-js/src/classes/KeyPair.ts @@ -2,18 +2,32 @@ * @dev Class for managing secp256k1 keys and performing operations with them */ -import * as BN from 'bn.js'; -import { ec as EC } from 'elliptic'; +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 { computePublicKey, SigningKey } from '@ethersproject/signing-key'; +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'; -const ec = new EC('secp256k1'); +/** + * @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 @@ -33,8 +47,8 @@ export class KeyPair { if (key.length === lengths.privateKey) { // Private key provided this.privateKeyHex = key; - const publicKey = ec.g.mul(this.privateKeyHexSlim); // Multiply secp256k1 generator point by private key to get public key - this.publicKeyHex = `0x${publicKey.encode('hex') as string}`; // Save off public key, other forms computed as getters + 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 this.publicKeyHex = key; // Save off public key, other forms computed as getters @@ -45,13 +59,6 @@ export class KeyPair { // ===================================================== GETTERS ===================================================== - /** - * @notice Returns the private key as an ethers BigNumber - */ - get privateKeyBN() { - return this.privateKeyHex ? BigNumber.from(this.privateKeyHex) : null; - } - /** * @notice Returns the private key as a hex string without the 0x prefix */ @@ -60,10 +67,10 @@ export class KeyPair { } /** - * @notice Returns an elliptic instance generated from the public key + * @notice Returns the uncompressed public key as a hex string without the 0x prefix */ - get publicKeyEC() { - return ec.keyFromPublic(this.publicKeyHex.slice(2), 'hex'); + get publicKeyHexSlim() { + return this.publicKeyHex.slice(2); } /** @@ -74,22 +81,22 @@ 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 privateKey = new SigningKey(ephemeralWallet.privateKey); - const sharedSecret = privateKey.computeSharedSecret(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 @@ -103,19 +110,16 @@ export class KeyPair { * @returns Decrypted ciphertext as hex string */ decrypt(output: EncryptedPayload) { - if (!output.ephemeralPublicKey || !output.ciphertext) { + const { ephemeralPublicKey, ciphertext } = output; + if (!ephemeralPublicKey || !ciphertext) { throw new Error('Input must be of type EncryptedPayload to decrypt'); } if (!this.privateKeyHex) { throw new Error('KeyPair has no associated private key to decrypt with'); } - // Get shared secret to use as decryption key - const { ephemeralPublicKey, ciphertext } = output; - const privateKey = new SigningKey(this.privateKeyHex); - const sharedSecret = privateKey.computeSharedSecret(ephemeralPublicKey); - - // Decrypt + // Get shared secret to use as decryption key, then decrypt with XOR + const sharedSecret = getSharedSecret(this.privateKeyHex, ephemeralPublicKey); const plaintext = BigNumber.from(ciphertext).xor(sharedSecret); return hexZeroPad(plaintext.toHexString(), 32); } @@ -133,19 +137,14 @@ 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) - ? (value as string).slice(2) // provided a valid hex string - : (value as RandomNumber).asHexSlim; // provided RandomNumber - - // Perform the multiplication - const publicKey = this.publicKeyEC.getPublic().mul(new BN(number, 16)); + ? BigInt(value as string) // provided a valid hex string + : BigInt((value as RandomNumber).asHex); // provided RandomNumber - // Get x,y hex strings and pad each to 32 bytes - const x = padHex(publicKey.getX().toString('hex')); - const y = padHex(publicKey.getY().toString('hex')); - - // Instantiate and return new instance - return new KeyPair(`0x04${x}${y}`); + // Perform the multiplication and return new KeyPair instance + const publicKey = Point.fromHex(this.publicKeyHexSlim).multiply(number); + return new KeyPair(`0x${publicKey.toHex()}`); } /** @@ -159,28 +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) - ? (value as string) // provided a valid hex string - : (value as RandomNumber).asHex; // provided RandomNumber - - // Get new private key. This gives us an arbitrarily large number that is not - // necessarily in the domain of the secp256k1 elliptic curve - const privateKeyFull = this.privateKeyBN.mul(number); - - // Modulo operation to get private key to be in correct range, where ec.n gives the - // order of our curve. We add the 0x prefix as it's required by ethers.js - const privateKeyMod = privateKeyFull.mod(`0x${(ec.n as BN).toString('hex')}`); - - // Remove 0x prefix to pad hex value, then add back 0x prefix - const privateKey = `0x${padHex(privateKeyMod.toHexString().slice(2))}`; - - // Instantiate and return new instance - return new KeyPair(privateKey); + ? 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. + 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 ================================================== @@ -206,22 +197,23 @@ 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 = computePublicKey(publicKey, true); + const compressedPublicKey = Point.fromHex(publicKey.slice(2)).toHex(true); return { - prefix: Number(compressedPublicKey[3]), // prefix bit is the 4th character in the string (e.g. 0x03) - pubKeyXCoordinate: `0x${compressedPublicKey.slice(4)}`, + prefix: Number(compressedPublicKey[1]), // prefix bit is the 2th character in the string (no 0x prefix) + pubKeyXCoordinate: `0x${compressedPublicKey.slice(2)}`, }; } /** * @notice Given the x-coordinate of a public key, without the identifying prefix bit, returns * the uncompressed public key assuming the identifying bit is 02 - * @dev We don't know if the identifying bit is 02 or 03 when uncompressing for the scanning use - * case, but it doesn't actually matter since we are not deriving an address from the public key. - * We use the public key to compute the shared secret to decrypt the random number, and since that - * involves multiplying this public key by a private key, the result is the same shared secret - * regardless of whether we assume the 02 or 03 prefix. Therefore if no prefix is provided, we - * can assume 02, and it's up to the user to make sure they are using this method safely.I + * @dev We don't know if the identifying bit is 02 or 03 when uncompressing for the scanning use case, but it + * doesn't actually matter since we are not deriving an address from the public key. We use the public key to + * compute the shared secret to decrypt the random number, and since that involves multiplying this public key + * by a private key, we can ensure the result is the same shared secret regardless of whether we assume the 02 or + * 03 prefix by using the compressed form of the hex shared secret and ignoring the prefix. Therefore if no prefix + * is provided, we can assume 02, and it's up to the user to make sure they are using this method safely. This is + * done because it saves gas in the Umbra contract * @param pkx x-coordinate of compressed public key, as BigNumber or hex string * @param prefix Prefix bit, must be 2 or 3 */ @@ -229,13 +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 = 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 - const hexWithoutPrefix = BigNumber.from(pkx).toHexString().slice(2); - return computePublicKey(BigNumber.from(`0x02${hexWithoutPrefix}`).toHexString()); + return `0x${Point.fromHex(`02${hexWithoutPrefix}`).toHex()}`; } - const hexWithoutPrefix = padHex(BigNumber.from(pkx).toHexString().slice(2)); - const hexWithPrefix = `0x0${Number(prefix)}${hexWithoutPrefix}`; - return computePublicKey(BigNumber.from(hexWithPrefix).toHexString()); + const hexWithPrefix = `0${Number(prefix)}${hexWithoutPrefix}`; + 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 f8b46e025..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}`; } /** @@ -138,7 +116,7 @@ export async function lookupRecipient(id: string, provider: EthersProvider) { // Get last transaction hash sent by that address const txHash = await getSentTransaction(id, provider); if (!txHash) { - throw new Error('The provider address has not sent any transactions'); + throw new Error('Could not get public key because the provided address has not sent any transactions'); } // Get public key from that transaction diff --git a/umbra-js/test/KeyPair.test.ts b/umbra-js/test/KeyPair.test.ts index 4cdefbd1f..9682fb7bc 100644 --- a/umbra-js/test/KeyPair.test.ts +++ b/umbra-js/test/KeyPair.test.ts @@ -176,7 +176,7 @@ describe('KeyPair class', () => { const keyPair2 = new KeyPair(wallet.publicKey); expect(keyPair1.publicKeyHex).to.equal(keyPair2.publicKeyHex); - expect(JSON.stringify(keyPair1.publicKeyEC)).to.equal(JSON.stringify(keyPair2.publicKeyEC)); + // expect(JSON.stringify(keyPair1.publicKeyEC)).to.equal(JSON.stringify(keyPair2.publicKeyEC)); }); it('supports encryption and decryption of the random number', async () => { 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); diff --git a/yarn.lock b/yarn.lock index 0fd4ec6fc..7086a3288 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3351,13 +3351,6 @@ electron-notarize "^0.1.1" electron-osx-sign "^0.4.11" -"@types/elliptic@^6.4.12": - version "6.4.12" - resolved "https://registry.yarnpkg.com/@types/elliptic/-/elliptic-6.4.12.tgz#e8add831f9cc9a88d9d84b3733ff669b68eaa124" - integrity sha512-gP1KsqoouLJGH6IJa28x7PXb3cRqh83X8HCLezd2dF+XcAIMKYv53KV+9Zn6QA561E120uOqZBQ+Jy/cl+fviw== - dependencies: - "@types/bn.js" "*" - "@types/eslint-visitor-keys@^1.0.0": version "1.0.0" resolved "https://registry.yarnpkg.com/@types/eslint-visitor-keys/-/eslint-visitor-keys-1.0.0.tgz#1ee30d79544ca84d68d4b3cdb0af4f205663dd2d" @@ -14005,6 +13998,11 @@ no-case@^3.0.4: lower-case "^2.0.2" tslib "^2.0.3" +noble-secp256k1@^1.1.2: + version "1.1.2" + resolved "https://registry.yarnpkg.com/noble-secp256k1/-/noble-secp256k1-1.1.2.tgz#43f4cb08933264cb84bd1aabb0dae4adfd186acc" + integrity sha512-+fW9Vt7ev0aT+esL8tVsH79GiDB0b8Nzk9cgwPKXoG1rKJjaA7Phg2VzxV541qdu/V1bG/y8xA0nJBu1QvBmTg== + node-addon-api@^2.0.0: version "2.0.2" resolved "https://registry.yarnpkg.com/node-addon-api/-/node-addon-api-2.0.2.tgz#432cfa82962ce494b132e9d72a15b29f71ff5d32"