Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

Update secp2561 ECDSA dependency to v4.0.0 #228

Merged
merged 16 commits into from
Apr 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,15 @@
"ethjs-util": "0.1.6",
"keccak": "^3.0.0",
"rlp": "^2.2.4",
"secp256k1": "^3.0.1"
"secp256k1": "^4.0.1"
},
"devDependencies": {
"@ethereumjs/config-prettier": "^1.1.0",
"@ethereumjs/config-tsc": "^1.1.0",
"@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",
Expand Down
9 changes: 5 additions & 4 deletions src/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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]))
Expand All @@ -150,7 +151,7 @@ export const isValidPublic = function(publicKey: Buffer, sanitize: boolean = fal
export const pubToAddress = function(pubKey: Buffer, sanitize: boolean = false): Buffer {
assertIsBuffer(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
Expand All @@ -173,7 +174,7 @@ export const privateToAddress = function(privateKey: Buffer): Buffer {
export const privateToPublic = function(privateKey: Buffer): Buffer {
assertIsBuffer(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))
}

/**
Expand All @@ -182,7 +183,7 @@ export const privateToPublic = function(privateKey: Buffer): Buffer {
export const importPublic = function(publicKey: Buffer): Buffer {
assertIsBuffer(publicKey)
if (publicKey.length !== 64) {
publicKey = secp256k1.publicKeyConvert(publicKey, false).slice(1)
publicKey = toBuffer(secp256k1.publicKeyConvert(publicKey, false).slice(1))
}
return publicKey
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, have checked for all return value conversions, looks complete.

2 changes: 1 addition & 1 deletion src/bytes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ const stripZeros = function(a: any): Buffer | number[] | string {
*/
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)) {
Expand Down
8 changes: 1 addition & 7 deletions src/externals.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
/**
* 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
*/

const ethjsUtil = require('ethjs-util')
import * as secp256k1 from 'secp256k1'
import * as BN from 'bn.js'
import * as rlp from 'rlp'

Expand All @@ -24,8 +23,3 @@ export { BN }
* [`rlp`](https://github.com/ethereumjs/rlp)
*/
export { rlp }

/**
* [`secp256k1`](https://github.com/cryptocoinjs/secp256k1-node/)
*/
export { secp256k1 }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference: removal of the secp2561 re-export

12 changes: 6 additions & 6 deletions src/signature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand All @@ -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))
}

/**
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, also checked for completeness here.

Expand Down
22 changes: 20 additions & 2 deletions test/account.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,21 @@ 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() {
isValidPrivate((<unknown>'WRONG_INPUT_TYPE') as Buffer)
})
})
it('should fail on invalid curve (zero)', function() {
const tmp = '0000000000000000000000000000000000000000000000000000000000000000'
Expand Down Expand Up @@ -102,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((<unknown>pubKey) as Buffer)
} catch (err) {
assert(err.message.includes('This method only supports Buffer'))
}
})
})

describe('importPublic', function() {
Expand Down
43 changes: 0 additions & 43 deletions test/externals.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down