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

signature/address: support for high recovery and chain IDs #290

Merged
merged 11 commits into from
Mar 4, 2021
Merged
4 changes: 2 additions & 2 deletions src/bytes.ts
Original file line number Diff line number Diff line change
@@ -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'

/**
Expand Down Expand Up @@ -112,7 +112,7 @@ export const unpadHexString = function(a: string): string {
*/
export const toBuffer = function(
v:
| string
| PrefixedHexString
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm assuming that originally we expected these strings to be 0x prefixed?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah actually PrefixedHexString is simply a string type right now, maybe in the future typescript will give us some way to enforce that the first two characters are 0x.

| number
| BN
| Buffer
Expand Down
18 changes: 11 additions & 7 deletions src/signature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import BN from 'bn.js'
import { toBuffer, setLengthLeft, bufferToHex, bufferToInt } from './bytes'
import { keccak } from './hash'
import { assertIsBuffer } from './helpers'
import { BNLike } from './types'

export interface ECDSASignature {
v: number
Expand Down Expand Up @@ -30,12 +31,15 @@ export const ecsign = function(
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)
}

/**
Expand All @@ -44,17 +48,17 @@ function isValidSigRecovery(recovery: number): boolean {
*/
export const ecrecover = function(
msgHash: Buffer,
v: number,
v: BNLike,
r: Buffer,
s: Buffer,
chainId?: number
chainId?: BNLike
): Buffer {
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)
ryanio marked this conversation as resolved.
Show resolved Hide resolved
return Buffer.from(publicKeyConvert(senderPubKey, false).slice(1))
}

Expand Down
4 changes: 2 additions & 2 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
50 changes: 50 additions & 0 deletions test/signature.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,56 @@ 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)')

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() {
Expand Down