From 86b19f614c69db6148b93cb813b4b36313e7a1c4 Mon Sep 17 00:00:00 2001 From: holgerd77 Date: Wed, 29 Jan 2020 11:30:55 +0100 Subject: [PATCH 01/14] Updated secp2561 ECDSA dependency to v4.0.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 57799b81..1075a404 100644 --- a/package.json +++ b/package.json @@ -92,7 +92,7 @@ "ethjs-util": "0.1.6", "keccak": "^3.0.0", "rlp": "^2.2.4", - "secp256k1": "^3.0.1" + "secp256k1": "^4.0.0" }, "devDependencies": { "@ethereumjs/config-prettier": "^1.1.0", From 705cbb201b7419bf142f158bed76ab3f224657f1 Mon Sep 17 00:00:00 2001 From: holgerd77 Date: Mon, 3 Feb 2020 13:44:21 +0100 Subject: [PATCH 02/14] Updated isValidPrivate() (backwards-compatible) to work with changed secp256k1 v4.0.0 API --- src/account.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/account.ts b/src/account.ts index 4543c1a5..c23bc221 100644 --- a/src/account.ts +++ b/src/account.ts @@ -122,7 +122,18 @@ export const generateAddress2 = function( * Checks if the private key satisfies the rules of the curve secp256k1. */ export const isValidPrivate = function(privateKey: Buffer): boolean { - return secp256k1.privateKeyVerify(privateKey) + let valid + try { + valid = secp256k1.privateKeyVerify(privateKey) + } catch (e) { + // TODO: Change API to also throw on wrong input instead of returning false + if (e.message === 'Expected private key to be an Uint8Array with length 32') { + valid = false + } else { + throw e + } + } + return valid } /** From 5b72d72df29e73c67e246bca1b738734e3932951 Mon Sep 17 00:00:00 2001 From: holgerd77 Date: Mon, 3 Feb 2020 13:44:53 +0100 Subject: [PATCH 03/14] Additional test for isValidPrivate() to throw on unrelated exceptions --- test/account.spec.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/account.spec.ts b/test/account.spec.ts index 24521c07..e3fa7804 100644 --- a/test/account.spec.ts +++ b/test/account.spec.ts @@ -27,6 +27,11 @@ describe('isValidPrivate', function() { '3a443d8381a6798a70c6ff9304bdc8cb0163c23211d11628fae52ef9e0dca11a001cf066d56a8156fc201cd5df8a36ef694eecd258903fca7086c1fae7441e1d' assert.equal(isValidPrivate(Buffer.from(tmp, 'hex')), false) }) + it('should fail on wrong input type', function() { + assert.throws(function() { + isValidPrivate('WRONG_INPUT_TYPE') + }) + }) it('should fail on invalid curve (zero)', function() { const tmp = '0000000000000000000000000000000000000000000000000000000000000000' assert.equal(isValidPrivate(Buffer.from(tmp, 'hex')), false) From ba9074907c7906fffe467460455ac3a65b1cd305 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Wed, 22 Apr 2020 18:43:05 -0700 Subject: [PATCH 04/14] Fix "wrong input type" test TS error --- test/account.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/account.spec.ts b/test/account.spec.ts index e3fa7804..eeb74db1 100644 --- a/test/account.spec.ts +++ b/test/account.spec.ts @@ -29,7 +29,7 @@ describe('isValidPrivate', function() { }) it('should fail on wrong input type', function() { assert.throws(function() { - isValidPrivate('WRONG_INPUT_TYPE') + isValidPrivate(('WRONG_INPUT_TYPE') as Buffer) }) }) it('should fail on invalid curve (zero)', function() { From 7f2de0c6b71b8256714b26ede9216cfeee6f5d1f Mon Sep 17 00:00:00 2001 From: cgewecke Date: Wed, 22 Apr 2020 18:46:09 -0700 Subject: [PATCH 05/14] Upgrade @types/secp256k1 to ^4.0.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1075a404..40556ad3 100644 --- a/package.json +++ b/package.json @@ -100,7 +100,7 @@ "@ethereumjs/config-tslint": "^1.1.0", "@types/mocha": "^5.2.7", "@types/node": "^11.9.0", - "@types/secp256k1": "3.5.0", + "@types/secp256k1": "^4.0.1", "contributor": "^0.1.25", "husky": "^2.1.0", "karma": "^4.0.0", From 326431eda33ea6594bc23af68057b92af8f21730 Mon Sep 17 00:00:00 2001 From: Roman Vanesyan Date: Wed, 22 Apr 2020 20:51:30 -0700 Subject: [PATCH 06/14] Update secp256k1 method calls for 4.0.0 interface --- src/account.ts | 6 +++--- src/bytes.ts | 2 +- src/signature.ts | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/account.ts b/src/account.ts index c23bc221..1efdb128 100644 --- a/src/account.ts +++ b/src/account.ts @@ -164,7 +164,7 @@ export const isValidPublic = function(publicKey: Buffer, sanitize: boolean = fal export const pubToAddress = function(pubKey: Buffer, sanitize: boolean = false): Buffer { pubKey = toBuffer(pubKey) if (sanitize && pubKey.length !== 64) { - pubKey = secp256k1.publicKeyConvert(pubKey, false).slice(1) + pubKey = toBuffer(secp256k1.publicKeyConvert(pubKey, false).slice(1)) } assert(pubKey.length === 64) // Only take the lower 160bits of the hash @@ -187,7 +187,7 @@ export const privateToAddress = function(privateKey: Buffer): Buffer { export const privateToPublic = function(privateKey: Buffer): Buffer { privateKey = toBuffer(privateKey) // skip the type flag and use the X, Y points - return secp256k1.publicKeyCreate(privateKey, false).slice(1) + return toBuffer(secp256k1.publicKeyCreate(privateKey, false).slice(1)) } /** @@ -196,7 +196,7 @@ export const privateToPublic = function(privateKey: Buffer): Buffer { export const importPublic = function(publicKey: Buffer): Buffer { publicKey = toBuffer(publicKey) if (publicKey.length !== 64) { - publicKey = secp256k1.publicKeyConvert(publicKey, false).slice(1) + publicKey = toBuffer(secp256k1.publicKeyConvert(publicKey, false).slice(1)) } return publicKey } diff --git a/src/bytes.ts b/src/bytes.ts index 37e8af1e..33f135f2 100644 --- a/src/bytes.ts +++ b/src/bytes.ts @@ -69,7 +69,7 @@ export const stripZeros = unpad */ export const toBuffer = function(v: any): Buffer { if (!Buffer.isBuffer(v)) { - if (Array.isArray(v)) { + if (Array.isArray(v) || v instanceof Uint8Array) { v = Buffer.from(v) } else if (typeof v === 'string') { if (ethjsUtil.isHexString(v)) { diff --git a/src/signature.ts b/src/signature.ts index 23e09e37..478a31a0 100644 --- a/src/signature.ts +++ b/src/signature.ts @@ -17,12 +17,12 @@ export const ecsign = function( privateKey: Buffer, chainId?: number, ): ECDSASignature { - const sig = secp256k1.sign(msgHash, privateKey) - const recovery: number = sig.recovery + const sig = secp256k1.ecdsaSign(msgHash, privateKey) + const recovery: number = sig.recid const ret = { - r: sig.signature.slice(0, 32), - s: sig.signature.slice(32, 64), + r: toBuffer(sig.signature.slice(0, 32)), + s: toBuffer(sig.signature.slice(32, 64)), v: chainId ? recovery + (chainId * 2 + 35) : recovery + 27, } @@ -45,8 +45,8 @@ export const ecrecover = function( if (!isValidSigRecovery(recovery)) { throw new Error('Invalid signature v value') } - const senderPubKey = secp256k1.recover(msgHash, signature, recovery) - return secp256k1.publicKeyConvert(senderPubKey, false).slice(1) + const senderPubKey = secp256k1.ecdsaRecover(signature, recovery, msgHash) + return toBuffer(secp256k1.publicKeyConvert(senderPubKey, false).slice(1)) } /** From 6a4575235beb273c3b4a005e0d9516802013242c Mon Sep 17 00:00:00 2001 From: cgewecke Date: Wed, 22 Apr 2020 21:02:37 -0700 Subject: [PATCH 07/14] Remove secp256k1 as external export --- docs/modules/_externals_.md | 2 +- src/externals.ts | 6 ------ test/externals.spec.ts | 43 ------------------------------------- 3 files changed, 1 insertion(+), 50 deletions(-) diff --git a/docs/modules/_externals_.md b/docs/modules/_externals_.md index 8fb47642..2d9a6773 100644 --- a/docs/modules/_externals_.md +++ b/docs/modules/_externals_.md @@ -4,4 +4,4 @@ Re-exports commonly used modules: * Adds [`ethjs-util`](https://github.com/ethjs/ethjs-util) methods. -* Exports [`BN`](https://github.com/indutny/bn.js), [`rlp`](https://github.com/ethereumjs/rlp), [`secp256k1`](https://github.com/cryptocoinjs/secp256k1-node/). +* Exports [`BN`](https://github.com/indutny/bn.js), [`rlp`](https://github.com/ethereumjs/rlp). diff --git a/src/externals.ts b/src/externals.ts index 8e5b729f..7340a4b4 100644 --- a/src/externals.ts +++ b/src/externals.ts @@ -6,7 +6,6 @@ */ const ethjsUtil = require('ethjs-util') -import * as secp256k1 from 'secp256k1' import * as BN from 'bn.js' import * as rlp from 'rlp' @@ -24,8 +23,3 @@ export { BN } * [`rlp`](https://github.com/ethereumjs/rlp) */ export { rlp } - -/** - * [`secp256k1`](https://github.com/cryptocoinjs/secp256k1-node/) - */ -export { secp256k1 } diff --git a/test/externals.spec.ts b/test/externals.spec.ts index b310549b..6d8402ba 100644 --- a/test/externals.spec.ts +++ b/test/externals.spec.ts @@ -75,49 +75,6 @@ describe('External rlp export', () => { }) }) -describe('External secp256k1 export', () => { - it('should export `secp256k1`', () => { - assert.equal(src.secp256k1, secp256k1_export) - }) - - it('should use a secp256k1 function correctly', () => { - // generate message to sign - const msg = Buffer.from( - '983232e10f8d440b3bde2c0787084b1228a0a0bd6d18bf78165696bc76f3530e', - 'hex', - ) - // generate privKey - const privKey = Buffer.from( - '59812df42e7bbb8f60a0ae92c660dcb6700927f944c709eaa0b9447d9ebffaf7', - 'hex', - ) - // get the public key in a compressed format - const pubKey = src.secp256k1.publicKeyCreate(privKey) - // sign the message - const sigObj = src.secp256k1.sign(msg, privKey) - // verify the signature - assert.ok(src.secp256k1.verify(msg, sigObj.signature, pubKey)) - }) - - it('should throw on exceptions', () => { - // publicKeyVerify should be a Buffer - assert.throws(() => { - src.secp256k1.publicKeyVerify(null as any) - }) - - // publicKeyCombine public keys should have length greater that zero - assert.throws(() => { - src.secp256k1.publicKeyCombine([]) - }) - - // privateKeyImport invalid format - assert.throws(() => { - const buffer = Buffer.from([0x00]) - src.secp256k1.privateKeyImport(buffer) - }) - }) -}) - describe('External ethjsUtil export', () => { it('should have all ethjsUtil methods', () => { const expected = [ From 1065dadbbff6bd72ab442514fb2b0b94a486a015 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Wed, 22 Apr 2020 21:14:03 -0700 Subject: [PATCH 08/14] Set santize flag to false for invalid SEC1 tests --- test/account.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/account.spec.ts b/test/account.spec.ts index eeb74db1..5dd661d8 100644 --- a/test/account.spec.ts +++ b/test/account.spec.ts @@ -84,7 +84,7 @@ describe('isValidPublic', function() { '023a443d8381a6798a70c6ff9304bdc8cb0163c23211d11628fae52ef9e0dca11a001cf066d56a8156fc201cd5df8a36ef694eecd258903fca7086c1fae7441e1d', 'hex', ) - assert.equal(isValidPublic(pubKey, true), false) + assert.equal(isValidPublic(pubKey, false), false) }) it('should work with compressed keys with sanitize enabled', function() { const pubKey = Buffer.from( @@ -153,7 +153,7 @@ describe('publicToAddress', function() { 'hex', ) assert.throws(function() { - publicToAddress(pubKey, true) + publicToAddress(pubKey, false) }) }) it("shouldn't produce an address given an invalid public key", function() { From a994011657c7293305c3a6abe195b2aabd9995e5 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Wed, 22 Apr 2020 22:00:32 -0700 Subject: [PATCH 09/14] Make isValidPrivate throw on wrong key length --- src/account.ts | 13 +------------ test/account.spec.ts | 8 ++++++-- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/account.ts b/src/account.ts index 1efdb128..e58f809c 100644 --- a/src/account.ts +++ b/src/account.ts @@ -122,18 +122,7 @@ export const generateAddress2 = function( * Checks if the private key satisfies the rules of the curve secp256k1. */ export const isValidPrivate = function(privateKey: Buffer): boolean { - let valid - try { - valid = secp256k1.privateKeyVerify(privateKey) - } catch (e) { - // TODO: Change API to also throw on wrong input instead of returning false - if (e.message === 'Expected private key to be an Uint8Array with length 32') { - valid = false - } else { - throw e - } - } - return valid + return secp256k1.privateKeyVerify(privateKey) } /** diff --git a/test/account.spec.ts b/test/account.spec.ts index 5dd661d8..e70ffb1f 100644 --- a/test/account.spec.ts +++ b/test/account.spec.ts @@ -20,12 +20,16 @@ describe('isValidPrivate', function() { const SECP256K1_N = new BN('fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141', 16) it('should fail on short input', function() { const tmp = '0011223344' - assert.equal(isValidPrivate(Buffer.from(tmp, 'hex')), false) + assert.throws(function() { + isValidPrivate(Buffer.from(tmp, 'hex')) + }) }) it('should fail on too big input', function() { const tmp = '3a443d8381a6798a70c6ff9304bdc8cb0163c23211d11628fae52ef9e0dca11a001cf066d56a8156fc201cd5df8a36ef694eecd258903fca7086c1fae7441e1d' - assert.equal(isValidPrivate(Buffer.from(tmp, 'hex')), false) + assert.throws(function() { + isValidPrivate(Buffer.from(tmp, 'hex')) + }) }) it('should fail on wrong input type', function() { assert.throws(function() { From 98bf0dc8ad298afda9f726da84335366fecdf1ed Mon Sep 17 00:00:00 2001 From: cgewecke Date: Thu, 23 Apr 2020 11:29:01 -0700 Subject: [PATCH 10/14] Remove reference to secp256k1 in externals.ts comment --- src/externals.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/externals.ts b/src/externals.ts index 7340a4b4..2e233bdc 100644 --- a/src/externals.ts +++ b/src/externals.ts @@ -1,7 +1,7 @@ /** * Re-exports commonly used modules: * * Adds [`ethjs-util`](https://github.com/ethjs/ethjs-util) methods. - * * Exports [`BN`](https://github.com/indutny/bn.js), [`rlp`](https://github.com/ethereumjs/rlp), [`secp256k1`](https://github.com/cryptocoinjs/secp256k1-node/). + * * Exports [`BN`](https://github.com/indutny/bn.js), [`rlp`](https://github.com/ethereumjs/rlp). * @packageDocumentation */ From 697b161e5824851c0803161902c361d133bda2e7 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Sun, 26 Apr 2020 22:42:46 -0700 Subject: [PATCH 11/14] Remove Node 8 from CI --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ae2a9532..1d66d745 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node-version: [8.x, 10.x, 12.x, 13.x] + node-version: [10.x, 12.x, 13.x] steps: - name: Use Node.js ${{ matrix.node-version }} From 36fe94c846536d6cd34d2775ee9cc3582646b737 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Sun, 26 Apr 2020 23:01:24 -0700 Subject: [PATCH 12/14] Enforce buffer input for isValidPublic --- src/account.ts | 3 ++- test/account.spec.ts | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/account.ts b/src/account.ts index c94f31b5..aa864819 100644 --- a/src/account.ts +++ b/src/account.ts @@ -2,7 +2,7 @@ const ethjsUtil = require('ethjs-util') import * as assert from 'assert' import * as secp256k1 from 'secp256k1' import * as BN from 'bn.js' -import { zeros, bufferToHex } from './bytes' +import { zeros, bufferToHex, toBuffer } from './bytes' import { keccak, keccak256, rlphash } from './hash' import { assertIsHexString, assertIsBuffer } from './helpers' @@ -129,6 +129,7 @@ export const isValidPrivate = function(privateKey: Buffer): boolean { * @param sanitize Accept public keys in other formats */ export const isValidPublic = function(publicKey: Buffer, sanitize: boolean = false): boolean { + assertIsBuffer(publicKey) if (publicKey.length === 64) { // Convert to SEC1 for secp256k1 return secp256k1.publicKeyVerify(Buffer.concat([Buffer.from([4]), publicKey])) diff --git a/test/account.spec.ts b/test/account.spec.ts index 6c7488ce..4b5b8173 100644 --- a/test/account.spec.ts +++ b/test/account.spec.ts @@ -111,6 +111,15 @@ describe('isValidPublic', function() { ) assert.equal(isValidPublic(pubKey), true) }) + it('should throw if input is not Buffer', function() { + const pubKey = + '3a443d8381a6798a70c6ff9304bdc8cb0163c23211d11628fae52ef9e0dca11a001cf066d56a8156fc201cd5df8a36ef694eecd258903fca7086c1fae7441e1d' + try { + isValidPublic((pubKey) as Buffer) + } catch (err) { + assert(err.message.includes('This method only supports Buffer')) + } + }) }) describe('importPublic', function() { From 550c207be7c59d262062f9f1b787371890f41f1a Mon Sep 17 00:00:00 2001 From: cgewecke Date: Sun, 26 Apr 2020 23:05:11 -0700 Subject: [PATCH 13/14] Revert docs changes --- docs/modules/_externals_.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/_externals_.md b/docs/modules/_externals_.md index 2d9a6773..8fb47642 100644 --- a/docs/modules/_externals_.md +++ b/docs/modules/_externals_.md @@ -4,4 +4,4 @@ Re-exports commonly used modules: * Adds [`ethjs-util`](https://github.com/ethjs/ethjs-util) methods. -* Exports [`BN`](https://github.com/indutny/bn.js), [`rlp`](https://github.com/ethereumjs/rlp). +* Exports [`BN`](https://github.com/indutny/bn.js), [`rlp`](https://github.com/ethereumjs/rlp), [`secp256k1`](https://github.com/cryptocoinjs/secp256k1-node/). From 040cb54c6668c2bd91c6221380e1c55457ffc958 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Mon, 27 Apr 2020 10:15:12 -0700 Subject: [PATCH 14/14] Upgrade secp256k1 to ^4.0.1 and revert SEC1 test changes --- package.json | 2 +- test/account.spec.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 40556ad3..885c7279 100644 --- a/package.json +++ b/package.json @@ -92,7 +92,7 @@ "ethjs-util": "0.1.6", "keccak": "^3.0.0", "rlp": "^2.2.4", - "secp256k1": "^4.0.0" + "secp256k1": "^4.0.1" }, "devDependencies": { "@ethereumjs/config-prettier": "^1.1.0", diff --git a/test/account.spec.ts b/test/account.spec.ts index 4b5b8173..44df2c17 100644 --- a/test/account.spec.ts +++ b/test/account.spec.ts @@ -88,7 +88,7 @@ describe('isValidPublic', function() { '023a443d8381a6798a70c6ff9304bdc8cb0163c23211d11628fae52ef9e0dca11a001cf066d56a8156fc201cd5df8a36ef694eecd258903fca7086c1fae7441e1d', 'hex', ) - assert.equal(isValidPublic(pubKey, false), false) + assert.equal(isValidPublic(pubKey, true), false) }) it('should work with compressed keys with sanitize enabled', function() { const pubKey = Buffer.from( @@ -171,7 +171,7 @@ describe('publicToAddress', function() { 'hex', ) assert.throws(function() { - publicToAddress(pubKey, false) + publicToAddress(pubKey, true) }) }) it("shouldn't produce an address given an invalid public key", function() {