ArkEcosystem / core

The ARK Core Blockchain Framework. Check https://learn.ark.dev for more information.
https://ark.io
MIT License
336 stars 285 forks source link

Improve crypto by leveraging secp256k1-node #991

Closed spkjp closed 5 years ago

spkjp commented 5 years ago

Currently we have ECPair, ECPoint and ECSignature, re-implementing a lot of ECC in JavaScript. But secp256k1-node can do all of that as well, natively, removing the need to have our own slow ECC implementation.

In essence, by removing our own ECC from crypto.verify and using secp256k1 directly...

this: https://github.com/ArkEcosystem/core/blob/66ba069a93e93c74a706520f6adf734134b9656f/packages/crypto/lib/crypto/crypto.js#L253-L259

becomes:

const secp256k1 = require('secp256k1')
...
const hash = this.getHash(transaction, true, true)
const signatureBuffer = Buffer.from(transaction.signature, 'hex')
const senderPublicKeyBuffer = Buffer.from(transaction.senderPublicKey, 'hex')
const parsedSignature = secp256k1.signatureImport(signatureBuffer)
return secp256k1.verify(hash, parsedSignature, senderPublicKeyBuffer)

It is approximately more than twice as fast as before. I calculated the average of each verify call when syncing devnet by wrapping the verify call in console.time("verify") ... console.timeEnd("verify")

Average of verify: before: 1.56ms after: 0.67ms

Since most blocks are empty right now, the effect is negligible, but it's still absolutely advised to let go of our ECC in favor of secp256k1-node where possible, since we also use it in more places than crypto.verify and the performance difference isn't small.

@fix I'm preparing a branch where we exclusively use secp256k1, would like your feedback @fix

zillionn commented 5 years ago

This implementation is super experimental, use it at your own risk.

fix commented 5 years ago

well, i have made some tests yesterday and today actually. Verifying one block is 1-2 ms (there is a signature too), so i think it will be noticeably faster on the details:

fix commented 5 years ago

for info, the code was based on https://github.com/bitcoinjs/bitcoinjs-lib

fix commented 5 years ago

mmm which is based on https://github.com/bitcoinjs/tiny-secp256k1 which is a derivation of secp256k1-node ...

vRobM commented 5 years ago

https://github.com/gonetwork-project/RNFastSecp256k1