Conversation
etscrivner
force-pushed
the
use-rbsecp256k1
branch
from
eb91602 to
085acb6
Compare
This change replaces the current OpenSSL secp256k1 ECDSA with rbsecp256k1, a binding to libsecp256k1. It replaces all key generation, signing, and key recovery methods with their rbsecp256k1 equivalents.
etscrivner
force-pushed
the
use-rbsecp256k1
branch
from
085acb6 to
60a370f
Compare
Remove the OpenSSL and Secp256k1 modules as they are replaced by rbsecp256k1. We also no longer need to check that signatures are in lower-s form as this is done by libsecp256k1.
Factor the recoverable signature functionality out into two separate classes. `PersonalMessage` which represents a message to be recoverably signed, typically to prove key ownership. `RecoverableSignature` which represents a recoverable signature and provides methods for easily recovering the public key from it.
Remove TODO around libsecp256k1 signing from the README as this is now supported.
etscrivner
force-pushed
the
use-rbsecp256k1
branch
from
5ee5edd to
1bb5376
Compare
etscrivner
changed the title
| def personal_sign(message, chain_id = nil) | ||
| signature = | ||
| if chain_id | ||
| PersonalMessage.new(message).sign(private_key, nil) |
There was a problem hiding this comment.
I thought that personal_sign did not take chain ID into account? As far as I can see in this PR, this chain_id parameter is is always nil for PersonalMessage#sign. Could we cut it? Is there a scenario for it I'm missing?
| # @raise [RuntimeError] if v value derived from signature is invalid. | ||
| def initialize(signature, chain_id = nil) | ||
| # Move the last byte containing the v value to the front. | ||
| rotated_signature = Utils.hex_to_bin(signature).bytes.rotate(-1) |
There was a problem hiding this comment.
Isn't it possible that v is larger than a single byte?
| @@ -22,6 +22,7 @@ Gem::Specification.new do |spec| | |||
| spec.add_dependency 'digest-sha3-patched', '~> 1.1' | |||
| spec.add_dependency 'ffi', '~> 1.0' | |||
There was a problem hiding this comment.
| spec.add_dependency 'ffi', '~> 1.0' |
I think we can get rid of the FFI dependency now that we aren't using OpenSSL.
|
@etscrivner unfortunately, your solution still doesn't work for me. When I'm trying to run Any ideas? |
|
@se3000 do you need someone to take over this PR? I'm getting an issue with OpenSSL on Mac (it pretends to be OpenSSL but it's really LibreSSL) and this PR could make my life a lot easier. |
|
would love to have this working! |
|
If anyone wants to take over this work and rebase it on the latest version of ruby-eth, I would be happy to review and integrate it. |
|
I implemented this in q9f/eth.rb#4 and q9f/eth.rb#24 and plan to release it as 0.5.0 soon. Thanks for your pioneering work @etscrivner 🎉 |
6 participants
Overview
This change replaces the current OpenSSL secp256k1 ECDSA with rbsecp256k1, a native extension gem binding to libsecp256k1. It replaces all key generation, signing, and key recovery methods with their rbsecp256k1 equivalents.
Directly using libsecp256k1 will provide performance and security benefits over the current hand-rolled OpenSSL version.