tomusdrw / ethsign Goto Github PK

https://iqlusion.blog/k256-crate-pure-rust-projective-secp256k1-library

I only found Pbkdf2 now

Currently only pbkdf2 is supported, we should support scrypt as well.

Note: try implementing using serde's (internally) tagged-enum representation and #[serde(flatten)] (instead of going for custom deserialization as in ethstore)

Check if creating Secp256k1 is heavy enough to be worth introducing lazy_static. Provide some benchmarks.

rust-web3 has a method recover to do this, how can we do this with ethsign, thank you!

use web3::signing::{keccak256, recover};

let sig = "0x30755ed65396facf86c53e6217c52b4daebe72aa4941d89635409de4c9c7f9466d4e9aaec7977f05e923889b33c0d0dd27d7226b6e6f56ce737465c5cfd04be400";
let address = "0x11f4d0A3c12e86B4b5F39B213F7E19D048276DAe";
let original_message = "Hello world";

let message = keccak256(format!("{}{}{}", "\x19Ethereum Signed Message:\n", original_message.len(), original_message).as_bytes());

let signature = hex::decode(&sig).unwrap();
let pubkey = recover(&message, &signature[..64], 1).unwrap();
let pubkey = format!(0x{}, pubkey);
assert_eq!(pubkey.to_lowercase(), address.to_lowercase());

Hi @tomusdrw !

I used this library for eth signature verification.

The main issue that I have with it is that it is very slow. recover is around 3ms, but using rust bindings to native library (that you use under the hood) recover is around 66us. I found reason for this is that you create secp256k1::Secp256k1::verification_only() for every signature verification, this function by itself costs around 2.7 ms so it sums up.

Consider adding benchmarks, also I would suggest using thread_local to store secp256k1::Secp256k1::verification_only() objects.

Sincerely, Vitaly.

Hello @tomusdrw,

Using this peice of code:

pub fn use_key(keystore_file: &String, password: String) -> Option<SecretKey> {
    let file = File::open(&keystore_file).unwrap();
    let key: KeyFile = serde_json::from_reader(file).unwrap();
    let password: Protected = password.into();
    if let Ok(secret) = key.to_secret_key(&password) {
        return Some(secret);
    }
    None
}

I noticed that it takes a relatively long time to unlock the key and return the secret. Is this something that could be optimized?

error[E0308]: mismatched types
   --> ~/.cargo/registry/src/github.com-1ecc6299db9ec823/ethsign-0.7.1/src/keyfile.rs:130:69
    |
130 |             crypto::derive_key_iterations(password.as_ref(), &salt, iterations);
    |                                                                     ^^^^^^^^^^ expected struct `std::num::NonZeroU32`, found u32
    |
    = note: expected type `std::num::NonZeroU32`
               found type `u32`

Cargo.toml snippet

ethsign = {version = "0.7.1", default-features = false, features = ["pure-rust"]}
ethsign-crypto = "0.1.0"

Am I doing something wrong?

It seems ethsign-crypto 0.1.0 source has a breaking change that was published to crates.io also as 0.1.0. Am I misreading something?

Do you need to bump the ethsign-crypto minor version and publish ethsign-crypto 0.2.0?

Since https://docs.rs/ethsign/latest/ethsign/struct.SecretKey.html#method.sign returns the secp256k1 can we consider exporting the error or just use the ethsign::Error instead?

It would be nice to have SecretKey::sing directly use an array [u8; 32] instead of slice &[u8]. This would improve the api since the message should always be 32 bytes.

Hi!
Do you have in plans to add functionality to actually create password protected JSON keyfiles? Function taking as arguments SecretKey, password and path to the file and creating proper keystore file would be extremely useful.

With the last released version 0.8.0 of ethsign, "cargo audit" complains about outdated vulnerable AES dependencies. If I use the master branch instead, these warnings are gone. I prefer to use released versions from crates.io. So what is holding back a new release?

Split off from #93

It would be nice to have SecretKey::sing directly use an array [u8; 32] instead of slice &[u8]. This would improve the api since the message should always be 32 bytes.

I guess using a slice is a carry-over form secp256k1 library. I agree that using [u8; 32] would be more convenient and could reduce the number of Results returned. However in some cases this might trigger unnecessary copying of data, so perhaps instead we could introduce a Message type:

pub struct Message<'a> {
  msg: InternalMessage<'a>,
}

impl<'a> Message {
  pub fn from_slice(slice: &'a [u8]) -> Result<Self, _> {
    // verify length
  }
}
impl<'a> From<[u8; 32]> for Message<'a> {
  ...
}
impl<'a> From<&'a [u8; 32]> for Message<'a> {
...
}

enum InternalMessage<'a> {
   ValidatedSlice(&'a [u8]),
   Borrowed(&'a [u8; 32]),
   Owned([u8; 32]),
}

And take impl Into<Message<'a>> in the API. Wondering how ergonomic the new API would be, since the errors might sometimes be confusing.

@elpiel wdyt?