signature/address: support for high recovery and chain IDs

src/bytes.ts

@@ -1,6 +1,6 @@
 import BN from 'bn.js'
 import { intToBuffer, stripHexPrefix, padToEven, isHexString, isHexPrefixed } from 'ethjs-util'
-import { TransformableToArray, TransformableToBuffer } from './types'
+import { PrefixedHexString, TransformableToArray, TransformableToBuffer } from './types'
 import { assertIsBuffer, assertIsArray, assertIsHexString } from './helpers'
 
 /**
@@ -112,7 +112,7 @@ export const unpadHexString = function(a: string): string {
  */
 export const toBuffer = function(
   v:
-    | string
+    | PrefixedHexString
     | number
     | BN
     | Buffer

src/signature.ts

@@ -1,41 +1,79 @@
-const { ecdsaSign, ecdsaRecover, publicKeyConvert } = require('ethereum-cryptography/secp256k1')
+import { ecdsaSign, ecdsaRecover, publicKeyConvert } from 'ethereum-cryptography/secp256k1'
 import BN from 'bn.js'
 import { toBuffer, setLengthLeft, bufferToHex, bufferToInt } from './bytes'
 import { keccak } from './hash'
 import { assertIsBuffer } from './helpers'
+import { BNLike } from './types'
+import { isHexString } from '.'
 
 export interface ECDSASignature {
   v: number
   r: Buffer
   s: Buffer
 }
 
+export interface ECDSASignatureBuffer {
+  v: Buffer
+  r: Buffer
+  s: Buffer
+}
+
 /**
  * Returns the ECDSA signature of a message hash.
  */
-export const ecsign = function(
+export function ecsign(msgHash: Buffer, privateKey: Buffer, chainId?: number): ECDSASignature
+export function ecsign(
   msgHash: Buffer,
   privateKey: Buffer,
-  chainId?: number
-): ECDSASignature {
+  chainId: BN | string | Buffer
+): ECDSASignatureBuffer
+export function ecsign(msgHash: Buffer, privateKey: Buffer, chainId: any): any {
   const sig = ecdsaSign(msgHash, privateKey)
   const recovery: number = sig.recid
 
-  const ret = {
-    r: Buffer.from(sig.signature.slice(0, 32)),
-    s: Buffer.from(sig.signature.slice(32, 64)),
-    v: chainId ? recovery + (chainId * 2 + 35) : recovery + 27
+  let ret
+  const r = Buffer.from(sig.signature.slice(0, 32))
+  const s = Buffer.from(sig.signature.slice(32, 64))
+  if (!chainId || typeof chainId === 'number') {
+    if (chainId && !Number.isSafeInteger(chainId)) {
+      throw new Error(
+        'The provided chainId is greater than MAX_SAFE_INTEGER (please use an alternative input type)'
+      )
+    }
+    return {
+      r,
+      s,
+      v: chainId ? recovery + (chainId * 2 + 35) : recovery + 27
+    }
+  } else {
+    // BN, string, Buffer
+    if (typeof chainId === 'string' && !isHexString(chainId)) {
+      throw new Error(`A chainId string must be provided with a 0x-prefix, given: ${chainId}`)
+    }
+    ret = {
+      r,
+      s,
+      v: toBuffer(
+        new BN(toBuffer(chainId))
+          .muln(2)
+          .addn(35)
+          .addn(recovery)
+      )
+    }
   }
 
   return ret
 }
 
-function calculateSigRecovery(v: number, chainId?: number): number {
-  return chainId ? v - (2 * chainId + 35) : v - 27
+function calculateSigRecovery(v: BNLike, chainId?: BNLike): BN {
+  const vBN = new BN(toBuffer(v))
+  const chainIdBN = chainId ? new BN(toBuffer(chainId)) : undefined
+  return chainIdBN ? vBN.sub(chainIdBN.muln(2).addn(35)) : vBN.subn(27)
 }
 
-function isValidSigRecovery(recovery: number): boolean {
-  return recovery === 0 || recovery === 1
+function isValidSigRecovery(recovery: number | BN): boolean {
+  const rec = new BN(recovery)
+  return rec.eqn(0) || rec.eqn(1)
 }
 
 /**
@@ -44,17 +82,34 @@ function isValidSigRecovery(recovery: number): boolean {
  */
 export const ecrecover = function(
   msgHash: Buffer,
-  v: number,
+  v: BNLike,
   r: Buffer,
   s: Buffer,
-  chainId?: number
+  chainId?: BNLike
 ): Buffer {
+  if (typeof v === 'string' && !isHexString(v)) {
+    throw new Error(`A v value string must be provided with a 0x-prefix, given: ${v}`)
+  }
+  if (typeof chainId === 'string' && !isHexString(chainId)) {
+    throw new Error(`A chainId string must be provided with a 0x-prefix, given: ${chainId}`)
+  }
+  if (typeof v === 'number' && !Number.isSafeInteger(v)) {
+    throw new Error(
+      'The provided v is greater than MAX_SAFE_INTEGER (please use an alternative input type)'
+    )
+  }
+  if (typeof chainId === 'number' && !Number.isSafeInteger(chainId)) {
+    throw new Error(
+      'The provided chainId is greater than MAX_SAFE_INTEGER (please use an alternative input type)'
+    )
+  }
+
   const signature = Buffer.concat([setLengthLeft(r, 32), setLengthLeft(s, 32)], 64)
   const recovery = calculateSigRecovery(v, chainId)
   if (!isValidSigRecovery(recovery)) {
     throw new Error('Invalid signature v value')
   }
-  const senderPubKey = ecdsaRecover(signature, recovery, msgHash)
+  const senderPubKey = ecdsaRecover(signature, recovery.toNumber(), msgHash)
   return Buffer.from(publicKeyConvert(senderPubKey, false).slice(1))
 }
 

src/types.ts

@@ -5,7 +5,7 @@ import { unpadBuffer } from './bytes'
 /*
  * A type that represents a BNLike input that can be converted to a BN.
  */
-export type BNLike = BN | string | number
+export type BNLike = BN | PrefixedHexString | number | Buffer
 
 /*
  * A type that represents a BufferLike input that can be converted to a Buffer.
@@ -28,7 +28,7 @@ export type PrefixedHexString = string
  * A type that represents an Address-like value.
  * To convert to address, use `new Address(toBuffer(value))`
  */
-export type AddressLike = Address | Buffer | string
+export type AddressLike = Address | Buffer | PrefixedHexString
 
 /*
  * A type that represents an object that has a `toArray()` method.

test/signature.spec.ts

@@ -48,17 +48,66 @@ describe('ecsign', function() {
   })
 
   it('should produce a signature for chainId=150', function() {
-    const chainId = 150
-    const sig = ecsign(echash, ecprivkey, chainId)
-    assert.deepEqual(
-      sig.r,
-      Buffer.from('99e71a99cb2270b8cac5254f9e99b6210c6c10224a1579cf389ef88b20a1abe9', 'hex')
+    const expectedSigR = Buffer.from(
+      '99e71a99cb2270b8cac5254f9e99b6210c6c10224a1579cf389ef88b20a1abe9',
+      'hex'
     )
-    assert.deepEqual(
-      sig.s,
-      Buffer.from('129ff05af364204442bdb53ab6f18a99ab48acc9326fa689f228040429e3ca66', 'hex')
+    const expectedSigS = Buffer.from(
+      '129ff05af364204442bdb53ab6f18a99ab48acc9326fa689f228040429e3ca66',
+      'hex'
     )
-    assert.equal(sig.v, chainId * 2 + 35)
+    const expectedSigV = Buffer.from('014f', 'hex')
+
+    const sig = ecsign(echash, ecprivkey, 150)
+    assert.deepEqual(sig.r, expectedSigR)
+    assert.deepEqual(sig.s, expectedSigS)
+    assert.equal(sig.v, 150 * 2 + 35)
+
+    let sigBuffer = ecsign(echash, ecprivkey, new BN(150))
+    assert.deepEqual(sigBuffer.r, expectedSigR)
+    assert.deepEqual(sigBuffer.s, expectedSigS)
+    assert.deepEqual(sigBuffer.v, expectedSigV)
+
+    sigBuffer = ecsign(echash, ecprivkey, Buffer.from([150]))
+    assert.deepEqual(sigBuffer.v, expectedSigV)
+
+    sigBuffer = ecsign(echash, ecprivkey, '0x96')
+    assert.deepEqual(sigBuffer.v, expectedSigV)
+
+    assert.throws(function() {
+      ecsign(echash, ecprivkey, '96')
+    })
+  })
+})
+
+it('should produce a signature for a high number chainId greater than MAX_SAFE_INTEGER', function() {
+  const chainIDBuffer = Buffer.from('796f6c6f763378', 'hex')
+  const expectedSigR = Buffer.from(
+    '99e71a99cb2270b8cac5254f9e99b6210c6c10224a1579cf389ef88b20a1abe9',
+    'hex'
+  )
+  const expectedSigS = Buffer.from(
+    '129ff05af364204442bdb53ab6f18a99ab48acc9326fa689f228040429e3ca66',
+    'hex'
+  )
+  const expectedSigV = Buffer.from('f2ded8deec6713', 'hex')
+
+  let sigBuffer = ecsign(echash, ecprivkey, new BN(chainIDBuffer))
+  assert.deepEqual(sigBuffer.r, expectedSigR)
+  assert.deepEqual(sigBuffer.s, expectedSigS)
+  assert.deepEqual(sigBuffer.v, expectedSigV)
+
+  sigBuffer = ecsign(echash, ecprivkey, chainIDBuffer)
+  assert.deepEqual(sigBuffer.v, expectedSigV)
+
+  sigBuffer = ecsign(echash, ecprivkey, '0x' + chainIDBuffer.toString('hex'))
+  assert.deepEqual(sigBuffer.v, expectedSigV)
+
+  const chainIDNumber = parseInt(chainIDBuffer.toString('hex'), 16)
+  assert.throws(() => {
+    // If we would use a number for the `chainId` parameter then it should throw.
+    // (The numbers are too high to perform arithmetic on)
+    ecsign(echash, ecprivkey, chainIDNumber)
   })
 })
 
@@ -106,6 +155,63 @@ describe('ecrecover', function() {
       ecrecover(echash, 27, s, r)
     })
   })
+  it('should return the right sender when using very high chain id / v values', function() {
+    // This data is from a transaction of the YoloV3 network, block 77, txhash c6121a23ca17b8ff70d4706c7d134920c1da43c8329444c96b4c63a55af1c760
+    /*
+      {
+        nonce: '0x8',
+        gasPrice: '0x3b9aca00',
+        gasLimit: '0x1a965',
+        to: undefined,
+        value: '0x0',
+        data: '0x608060405234801561001057600080fd5b50610101806100206000396000f3fe608060405260043610601f5760003560e01c8063776d1a0114603b576020565b5b6000543660008037600080366000845af43d6000803e3d6000f35b348015604657600080fd5b50608660048036036020811015605b57600080fd5b81019080803573ffffffffffffffffffffffffffffffffffffffff1690602001909291905050506088565b005b806000806101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff1602179055505056fea26469706673582212206d3160e3f009c6ebac579877e529c0a1ca8313678f08fe311659d440067d26ea64736f6c63430007040033',
+        v: '0xf2ded8deec6714',
+        r: '0xec212841e0b7aaffc3b3e33a08adf32fa07159e856ef23db85175a4f6d71dc0f',
+        s: '0x4b8e02b96b94064a5aa2f8d72bd0040616ba8e482a5dd96422e38c9a4611f8d5'
+      }
+    */
+    const senderPubKey = Buffer.from(
+      '78988201fbceed086cfca7b64e382d08d0bd776898731443d2907c097745b7324c54f522087f5964412cddba019f192de0fd57a0ffa63f098c2b200e53594b15',
+      'hex'
+    )
+    const msgHash = Buffer.from(
+      '8ae8cb685a7a9f29494b07b287c3f6a103b73fa178419d10d1184861a40f6afe',
+      'hex'
+    )
+
+    const r = Buffer.from('ec212841e0b7aaffc3b3e33a08adf32fa07159e856ef23db85175a4f6d71dc0f', 'hex')
+    const s = Buffer.from('4b8e02b96b94064a5aa2f8d72bd0040616ba8e482a5dd96422e38c9a4611f8d5', 'hex')
+
+    const vBuffer = Buffer.from('f2ded8deec6714', 'hex')
+    const chainIDBuffer = Buffer.from('796f6c6f763378', 'hex')
+    let sender = ecrecover(msgHash, vBuffer, r, s, chainIDBuffer)
+    assert.ok(sender.equals(senderPubKey), 'sender pubkey correct (Buffer)')
+
+    const vBN = new BN(vBuffer)
+    const chainIDBN = new BN(chainIDBuffer)
+    sender = ecrecover(msgHash, vBN, r, s, chainIDBN)
+    assert.ok(sender.equals(senderPubKey), 'sender pubkey correct (BN)')
+
+    const vHexString = '0xf2ded8deec6714'
+    const chainIDHexString = '0x796f6c6f763378'
+    sender = ecrecover(msgHash, vHexString, r, s, chainIDHexString)
+    assert.ok(sender.equals(senderPubKey), 'sender pubkey correct (HexString)')
+
+    assert.throws(function() {
+      ecrecover(msgHash, 'f2ded8deec6714', r, s, chainIDHexString)
+    })
+    assert.throws(function() {
+      ecrecover(msgHash, vHexString, r, s, '796f6c6f763378')
+    })
+
+    const chainIDNumber = parseInt(chainIDBuffer.toString('hex'), 16)
+    const vNumber = parseInt(vBuffer.toString('hex'), 16)
+    assert.throws(() => {
+      // If we would use numbers for the `v` and `chainId` parameters, then it should throw.
+      // (The numbers are too high to perform arithmetic on)
+      ecrecover(msgHash, vNumber, r, s, chainIDNumber)
+    })
+  })
 })
 
 describe('hashPersonalMessage', function() {