Util: remove ecsign method (instead directly use secp256k1.sign from external ethereum-cryptography package (#3948)

* util/monorepo: remove ecsign (use secp256k1.sign directly from ethereum-cryptography external package)

* client: read wasm from util

* tx: update auth sign to read from common crypto

* devp2p: convert deprecated ecsign to secp256k1.sign

* update customCrypto.ecsign return type

* fix wasmCrypto test

* switch v to number

* Revert "switch v to number"

This reverts commit 99859e5.

* remove old comment

* dedupe ecdsaSign

* fix test

* remove compat version of ecdsaRecover

* tx: remove TODO

* block/clique: ensure customCrypto and left padding is adhered

---------

Co-authored-by: acolytec3 <[email protected]>
Co-authored-by: Holger Drewes <[email protected]>

Author: 3 people

packages/block/src/consensus/clique.ts

@@ -6,14 +6,16 @@ import {
   BIGINT_27,
   EthereumJSErrorWithoutCode,
   bigIntToBytes,
+  bigIntToUnpaddedBytes,
   bytesToBigInt,
   concatBytes,
   createAddressFromPublicKey,
   createZeroAddress,
   ecrecover,
-  ecsign,
   equalsBytes,
+  setLengthLeft,
 } from '@ethereumjs/util'
+import { secp256k1 } from 'ethereum-cryptography/secp256k1.js'
 
 import type { CliqueConfig } from '@ethereumjs/common'
 import type { BlockHeader } from '../index.ts'
@@ -151,9 +153,13 @@ export function generateCliqueBlockExtraData(
 
   requireClique(header, 'generateCliqueBlockExtraData')
 
-  const ecSignFunction = header.common.customCrypto?.ecsign ?? ecsign
+  const ecSignFunction = header.common.customCrypto?.ecsign ?? secp256k1.sign
   const signature = ecSignFunction(cliqueSigHash(header), cliqueSigner)
-  const signatureB = concatBytes(signature.r, signature.s, bigIntToBytes(signature.v))
+  const signatureB = concatBytes(
+    setLengthLeft(bigIntToUnpaddedBytes(signature.r), 32),
+    setLengthLeft(bigIntToUnpaddedBytes(signature.s), 32),
+    bigIntToBytes(BigInt(signature.recovery)),
+  )
 
   const extraDataWithoutSeal = header.extraData.subarray(
     0,

packages/client/bin/utils.ts

@@ -16,13 +16,13 @@ import {
 } from '@ethereumjs/common'
 import {
   EthereumJSErrorWithoutCode,
+  bytesToBigInt,
   bytesToHex,
   calculateSigRecovery,
   concatBytes,
   createAddressFromPrivateKey,
   createAddressFromString,
   ecrecover,
-  ecsign,
   hexToBytes,
   parseGethGenesisState,
   randomBytes,
@@ -38,7 +38,7 @@ import {
   sha256 as wasmSha256,
 } from '@polkadot/wasm-crypto'
 import { keccak256 } from 'ethereum-cryptography/keccak.js'
-import { ecdsaRecover, ecdsaSign } from 'ethereum-cryptography/secp256k1-compat.js'
+import { secp256k1 } from 'ethereum-cryptography/secp256k1.js'
 import { sha256 } from 'ethereum-cryptography/sha256.js'
 import { KZG as microEthKZG } from 'micro-eth-signer/kzg'
 import * as verkle from 'micro-eth-signer/verkle'
@@ -626,16 +626,12 @@ function generateAccount(): Account {
   return [address, privKey]
 }
 
-export async function generateClientConfig(args: ClientOpts) {
-  // Give chainId priority over networkId
-  // Give networkId precedence over network name
-  const chainName = args.chainId ?? args.networkId ?? args.network ?? Chain.Mainnet
-  const chain = getPresetChainConfig(chainName)
+export async function getCryptoFunctions(useJsCrypto: boolean): Promise<CustomCrypto> {
   const cryptoFunctions: CustomCrypto = {}
 
   const kzg = new microEthKZG(trustedSetup)
   // Initialize WASM crypto if JS crypto is not specified
-  if (args.useJsCrypto === false) {
+  if (useJsCrypto === false) {
     await waitReadyPolkadotSha256()
     cryptoFunctions.keccak256 = keccak256WASM
     cryptoFunctions.ecrecover = (
@@ -654,23 +650,12 @@ export async function generateClientConfig(args: ClientOpts) {
       ).slice(1)
     cryptoFunctions.sha256 = wasmSha256
     cryptoFunctions.ecsign = (msg: Uint8Array, pk: Uint8Array) => {
-      if (msg.length < 32) {
-        // WASM errors with `unreachable` if we try to pass in less than 32 bytes in the message
-        throw EthereumJSErrorWithoutCode('message length must be 32 bytes or greater')
-      }
       const buf = secp256k1Sign(msg, pk)
-      const r = buf.slice(0, 32)
-      const s = buf.slice(32, 64)
-      const v = BigInt(buf[64])
+      const r = bytesToBigInt(buf.slice(0, 32))
+      const s = bytesToBigInt(buf.slice(32, 64))
+      const recovery = buf[64]
 
-      return { r, s, v }
-    }
-    cryptoFunctions.ecdsaSign = (hash: Uint8Array, pk: Uint8Array) => {
-      const sig = secp256k1Sign(hash, pk)
-      return {
-        signature: sig.slice(0, 64),
-        recid: sig[64],
-      }
+      return { r, s, recovery }
     }
     cryptoFunctions.ecdsaRecover = (sig: Uint8Array, recId: number, hash: Uint8Array) => {
       return secp256k1Recover(hash, sig, recId)
@@ -679,12 +664,29 @@ export async function generateClientConfig(args: ClientOpts) {
     cryptoFunctions.keccak256 = keccak256
     cryptoFunctions.ecrecover = ecrecover
     cryptoFunctions.sha256 = sha256
-    cryptoFunctions.ecsign = ecsign
-    cryptoFunctions.ecdsaSign = ecdsaSign
-    cryptoFunctions.ecdsaRecover = ecdsaRecover
+    cryptoFunctions.ecsign = secp256k1.sign
+    cryptoFunctions.ecdsaRecover = (sig: Uint8Array, recId: number, hash: Uint8Array) => {
+      // Adapted from @noble/curves docs
+      const sign = secp256k1.Signature.fromCompact(sig)
+      const point = sign.addRecoveryBit(recId).recoverPublicKey(hash)
+      const address = point.toRawBytes(true)
+      return address
+    }
   }
   cryptoFunctions.kzg = kzg
   cryptoFunctions.verkle = verkle
+  return cryptoFunctions
+}
+
+export async function generateClientConfig(args: ClientOpts) {
+  // Give chainId priority over networkId
+  // Give networkId precedence over network name
+  const chainName = args.chainId ?? args.networkId ?? args.network ?? Chain.Mainnet
+  const chain = getPresetChainConfig(chainName)
+
+  // `useJsCrypto` defaults to `false` in the CLI defaults
+  const cryptoFunctions = await getCryptoFunctions(args.useJsCrypto ?? false)
+
   // Configure accounts for mining and prefunding in a local devnet
   const accounts: Account[] = []
   if (typeof args.unlock === 'string') {

packages/client/test/util/wasmCrypto.spec.ts

@@ -4,7 +4,6 @@ import {
   bytesToHex,
   calculateSigRecovery,
   concatBytes,
-  ecsign,
   hexToBytes,
   randomBytes,
   setLengthLeft,
@@ -18,9 +17,11 @@ import {
   waitReady,
   sha256 as wasmSha256,
 } from '@polkadot/wasm-crypto'
-import { ecdsaRecover, ecdsaSign } from 'ethereum-cryptography/secp256k1-compat.js'
+import { secp256k1 } from 'ethereum-cryptography/secp256k1.js'
+
 import { sha256 as jsSha256 } from 'ethereum-cryptography/sha256.js'
 import { assert, describe, it } from 'vitest'
+import { getCryptoFunctions } from '../../bin/utils.ts'
 describe('WASM crypto tests', () => {
   it('should compute public key and hash correctly using common.customCrypto functions', async () => {
     const wasmecrecover = (
@@ -65,39 +66,36 @@ describe('WASM crypto tests', () => {
   })
 
   it('should compute the same signature whether js or WASM signature used', async () => {
-    const wasmSign = (msg: Uint8Array, pk: Uint8Array) => {
-      if (msg.length < 32) {
-        // WASM errors with `unreachable` if we try to pass in less than 32 bytes in the message
-        throw new Error('message length must be 32 bytes or greater')
-      }
-      const buf = secp256k1Sign(msg, pk)
-      const r = buf.slice(0, 32)
-      const s = buf.slice(32, 64)
-      const v = BigInt(buf[64])
-
-      return { r, s, v }
-    }
+    const crypto = await getCryptoFunctions(true)
+    const wasmSign = crypto.ecsign!
 
     await waitReady()
     const msg = hexToBytes('0x82ff40c0a986c6a5cfad4ddf4c3aa6996f1a7837f9c398e17e5de5cbd5a12b28')
     const pk = hexToBytes('0x3c9229289a6125f7fdf1885a77bb12c37a8d3b4962d936f7e3084dece32a3ca1')
-    const jsSig = ecsign(msg, pk)
+    const jsSig = secp256k1.sign(msg, pk)
     const wasmSig = wasmSign(msg, pk)
     assert.deepEqual(wasmSig, jsSig, 'wasm signatures produce same result as js signatures')
-    assert.throws(
-      () => wasmSign(randomBytes(31), randomBytes(32)),
-      'message length must be 32 bytes or greater',
-    )
   })
   it('should have the same signature and verification', async () => {
+    const crypto = await getCryptoFunctions(true)
     await waitReady()
     const pk = hexToBytes('0xbd3713a6da2c3624fa10bad8a52848b4291e3c9689ab50e0d2761e014d6e4cd7')
     const hash = hexToBytes('0x8c6d72155f746a9424b0621d82c5f5d3f6cc82e497b15df1b2ae601c8c14f75c')
-    const jsSig = ecdsaSign(hash, pk)
+    const jsSig = secp256k1.sign(hash, pk)
     const wasmSig = secp256k1Sign(hash, pk)
-    assert.equal(bytesToHex(wasmSig.slice(0, 64)), bytesToHex(jsSig.signature))
+    assert.equal(bytesToHex(wasmSig.slice(0, 64)), bytesToHex(jsSig.toCompactRawBytes()))
     const wasmRec = secp256k1Recover(hash, wasmSig.slice(0, 64), wasmSig[64])
-    const jsRec = ecdsaRecover(jsSig.signature, jsSig.recid, hash)
+    const jsRec = crypto.ecdsaRecover!(jsSig.toCompactRawBytes(), jsSig.recovery, hash)
+    assert.equal(bytesToHex(wasmRec), bytesToHex(jsRec))
+  })
+  it('should recover the same address', async () => {
+    const crypto = await getCryptoFunctions(true)
+    await waitReady()
+    const pk = hexToBytes('0xbd3713a6da2c3624fa10bad8a52848b4291e3c9689ab50e0d2761e014d6e4cd7')
+    const hash = hexToBytes('0x8c6d72155f746a9424b0621d82c5f5d3f6cc82e497b15df1b2ae601c8c14f75c')
+    const jsSig = secp256k1.sign(hash, pk)
+    const wasmRec = secp256k1Recover(hash, jsSig.toCompactRawBytes(), jsSig.recovery)
+    const jsRec = crypto.ecdsaRecover!(jsSig.toCompactRawBytes(), jsSig.recovery, hash)
     assert.equal(bytesToHex(wasmRec), bytesToHex(jsRec))
   })
 })

packages/common/src/types.ts

@@ -1,10 +1,5 @@
-import type {
-  BigIntLike,
-  ECDSASignature,
-  KZG,
-  PrefixedHexString,
-  VerkleCrypto,
-} from '@ethereumjs/util'
+import type { BigIntLike, KZG, PrefixedHexString, VerkleCrypto } from '@ethereumjs/util'
+import type { secp256k1 } from 'ethereum-cryptography/secp256k1.js'
 import type { ConsensusAlgorithm, ConsensusType, Hardfork } from './enums.ts'
 
 export interface ChainName {
@@ -95,8 +90,7 @@ export interface CustomCrypto {
     msg: Uint8Array,
     pk: Uint8Array,
     ecSignOpts?: { extraEntropy?: Uint8Array | boolean },
-  ) => ECDSASignature
-  ecdsaSign?: (msg: Uint8Array, pk: Uint8Array) => { signature: Uint8Array; recid: number }
+  ) => Pick<ReturnType<typeof secp256k1.sign>, 'recovery' | 'r' | 's'>
   ecdsaRecover?: (sig: Uint8Array, recId: number, hash: Uint8Array) => Uint8Array
   kzg?: KZG
   verkle?: VerkleCrypto

packages/common/test/customCrypto.spec.ts

@@ -1,10 +1,9 @@
-import { concatBytes, randomBytes } from '@ethereumjs/util'
+import { bytesToBigInt, concatBytes, randomBytes } from '@ethereumjs/util'
+import type { secp256k1 } from 'ethereum-cryptography/secp256k1.js'
 import { assert, describe, it } from 'vitest'
 
 import { Common, Mainnet, createCustomCommon } from '../src/index.ts'
 
-import type { ECDSASignature } from '@ethereumjs/util'
-
 describe('[Common]: Custom Crypto', () => {
   const customKeccak256 = (msg: Uint8Array) => {
     return concatBytes(msg, new Uint8Array([1]))
@@ -24,11 +23,14 @@ describe('[Common]: Custom Crypto', () => {
     return msg
   }
 
-  const customEcSign = (_msg: Uint8Array, _pk: Uint8Array): ECDSASignature => {
+  const customEcSign = (
+    _msg: Uint8Array,
+    _pk: Uint8Array,
+  ): Pick<ReturnType<typeof secp256k1.sign>, 'recovery' | 'r' | 's'> => {
     return {
-      v: 0n,
-      r: Uint8Array.from([0, 1, 2, 3]),
-      s: Uint8Array.from([0, 1, 2, 3]),
+      recovery: 0,
+      r: bytesToBigInt(Uint8Array.from([0, 1, 2, 3])),
+      s: bytesToBigInt(Uint8Array.from([0, 1, 2, 3])),
     }
   }
 
@@ -81,6 +83,6 @@ describe('[Common]: Custom Crypto', () => {
       ecsign: customEcSign,
     }
     const c = new Common({ chain: Mainnet, customCrypto })
-    assert.equal(c.customCrypto.ecsign!(randomBytes(32), randomBytes(32)).v, 0n)
+    assert.equal(c.customCrypto.ecsign!(randomBytes(32), randomBytes(32)).recovery, 0)
   })
 })

packages/devp2p/src/dpt/message.ts

@@ -1,19 +1,22 @@
 import { RLP } from '@ethereumjs/rlp'
 import {
   EthereumJSErrorWithoutCode,
+  bigIntToBytes,
   bytesToHex,
   bytesToInt,
   bytesToUtf8,
   concatBytes,
   intToBytes,
+  setLengthLeft,
 } from '@ethereumjs/util'
 import debugDefault from 'debug'
 import { keccak256 } from 'ethereum-cryptography/keccak.js'
-import { ecdsaRecover, ecdsaSign } from 'ethereum-cryptography/secp256k1-compat.js'
+import { ecdsaRecover } from 'ethereum-cryptography/secp256k1-compat.js'
 
 import { assertEq, ipToBytes, ipToString, isV4Format, isV6Format, unstrictDecode } from '../util.ts'
 
 import type { Common } from '@ethereumjs/common'
+import { secp256k1 } from 'ethereum-cryptography/secp256k1'
 import type { PeerInfo } from '../types.ts'
 
 const debug = debugDefault('devp2p:dpt:server')
@@ -186,8 +189,13 @@ export function encode<T>(typename: string, data: T, privateKey: Uint8Array, com
   const typedata = concatBytes(Uint8Array.from([type]), RLP.encode(encodedMsg))
 
   const sighash = (common?.customCrypto.keccak256 ?? keccak256)(typedata)
-  const sig = (common?.customCrypto.ecdsaSign ?? ecdsaSign)(sighash, privateKey)
-  const hashdata = concatBytes(sig.signature, Uint8Array.from([sig.recid]), typedata)
+  const sig = (common?.customCrypto.ecsign ?? secp256k1.sign)(sighash, privateKey)
+  const hashdata = concatBytes(
+    setLengthLeft(bigIntToBytes(sig.r), 32),
+    setLengthLeft(bigIntToBytes(sig.s), 32),
+    Uint8Array.from([sig.recovery]),
+    typedata,
+  )
   const hash = (common?.customCrypto.keccak256 ?? keccak256)(hashdata)
   return concatBytes(hash, hashdata)
 }

packages/devp2p/src/rlpx/ecies.ts

@@ -2,22 +2,24 @@ import * as crypto from 'crypto'
 import { RLP } from '@ethereumjs/rlp'
 import {
   EthereumJSErrorWithoutCode,
+  bigIntToBytes,
   bytesToInt,
   concatBytes,
   hexToBytes,
   intToBytes,
+  setLengthLeft,
 } from '@ethereumjs/util'
 import debugDefault from 'debug'
 import { keccak256 } from 'ethereum-cryptography/keccak.js'
 import { getRandomBytesSync } from 'ethereum-cryptography/random.js'
-import { ecdh, ecdsaRecover, ecdsaSign } from 'ethereum-cryptography/secp256k1-compat.js'
+import { ecdh, ecdsaRecover } from 'ethereum-cryptography/secp256k1-compat.js'
 import { secp256k1 } from 'ethereum-cryptography/secp256k1.js'
 
 import { assertEq, genPrivateKey, id2pk, pk2id, unstrictDecode, xor, zfill } from '../util.ts'
 
 import { MAC } from './mac.ts'
 
-import type { Common } from '@ethereumjs/common'
+import type { Common, CustomCrypto } from '@ethereumjs/common'
 type Decipher = crypto.Decipher
 
 const debug = debugDefault('devp2p:rlpx:peer')
@@ -77,13 +79,7 @@ export class ECIES {
   protected _bodySize: number | null = null
 
   protected _keccakFunction: (msg: Uint8Array) => Uint8Array
-  protected _ecdsaSign: (
-    msg: Uint8Array,
-    pk: Uint8Array,
-  ) => {
-    signature: Uint8Array
-    recid: number
-  }
+  protected _ecdsaSign: Required<CustomCrypto>['ecsign']
   protected _ecdsaRecover: (
     sig: Uint8Array,
     recId: number,
@@ -101,7 +97,7 @@ export class ECIES {
     this._ephemeralPublicKey = secp256k1.getPublicKey(this._ephemeralPrivateKey, false)
 
     this._keccakFunction = common?.customCrypto.keccak256 ?? keccak256
-    this._ecdsaSign = common?.customCrypto.ecdsaSign ?? ecdsaSign
+    this._ecdsaSign = common?.customCrypto.ecsign ?? secp256k1.sign
     this._ecdsaRecover = common?.customCrypto.ecdsaRecover ?? ecdsaRecover
   }
 
@@ -198,7 +194,11 @@ export class ECIES {
     const x = ecdhX(this._remotePublicKey, this._privateKey)
     const sig = this._ecdsaSign(xor(x, this._nonce), this._ephemeralPrivateKey)
     const data = [
-      concatBytes(sig.signature, Uint8Array.from([sig.recid])),
+      concatBytes(
+        setLengthLeft(bigIntToBytes(sig.r), 32),
+        setLengthLeft(bigIntToBytes(sig.s), 32),
+        Uint8Array.from([sig.recovery]),
+      ),
       // this._keccakFunction(pk2id(this._ephemeralPublicKey)),
       pk2id(this._publicKey),
       this._nonce,
@@ -221,8 +221,9 @@ export class ECIES {
     const x = ecdhX(this._remotePublicKey, this._privateKey)
     const sig = this._ecdsaSign(xor(x, this._nonce), this._ephemeralPrivateKey)
     const data = concatBytes(
-      sig.signature,
-      Uint8Array.from([sig.recid]),
+      bigIntToBytes(sig.r),
+      bigIntToBytes(sig.s),
+      Uint8Array.from([sig.recovery]),
       this._keccakFunction(pk2id(this._ephemeralPublicKey)),
       pk2id(this._publicKey),
       this._nonce,