Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Use parity-crypto updated to use upstream rust-secp256k1 #11406

Merged
merged 11 commits into from
Feb 10, 2020
247 changes: 170 additions & 77 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ num_cpus = "1.2"
number_prefix = "0.2"
panic_hook = { path = "util/panic-hook" }
parity-bytes = "0.1"
parity-crypto = { version = "0.4.2", features = ["publickey"] }
parity-crypto = { version = "0.5.0", features = ["publickey"] }
parity-daemonize = "0.3"
parity-hash-fetch = { path = "updater/hash-fetch" }
parity-ipfs-api = { path = "ipfs" }
Expand Down
2 changes: 1 addition & 1 deletion accounts/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ edition = "2018"
ethkey = { path = "ethkey" }
ethstore = { path = "ethstore" }
log = "0.4"
parity-crypto = { version = "0.4.2", features = ["publickey"] }
parity-crypto = { version = "0.5.0", features = ["publickey"] }
parking_lot = "0.10.0"
serde = "1.0"
serde_derive = "1.0"
Expand Down
2 changes: 1 addition & 1 deletion accounts/ethkey/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ edit-distance = "2.0"
log = "0.4"
serde = "1.0"
serde_derive = "1.0"
parity-crypto = { version = "0.4.2", features = ["publickey"] }
parity-crypto = { version = "0.5.0", features = ["publickey"] }
parity-wordlist = "1.3.1"
2 changes: 1 addition & 1 deletion accounts/ethkey/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ docopt = "1.0"
env_logger = "0.5"
ethkey = { path = "../" }
panic_hook = { path = "../../../util/panic-hook" }
parity-crypto = { version = "0.4.2", features = ["publickey"] }
parity-crypto = { version = "0.5.0", features = ["publickey"] }
parity-wordlist= "1.3.1"
rustc-hex = "1.0"
serde = "1.0"
Expand Down
6 changes: 3 additions & 3 deletions accounts/ethkey/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ fn execute<S, I>(command: I) -> Result<String, Error> where I: IntoIterator<Item
let result = if args.flag_brain {
let phrase = args.arg_secret_or_phrase;
let phrase_info = validate_phrase(&phrase);
let keypair = Brain::new(phrase).generate().expect("Brain wallet generator is infallible; qed");
let keypair = Brain::new(phrase).generate();
(keypair, Some(phrase_info))
} else {
let secret = args.arg_secret_or_phrase.parse().map_err(|_| EthkeyError::InvalidSecretKey)?;
Expand All @@ -215,7 +215,7 @@ fn execute<S, I>(command: I) -> Result<String, Error> where I: IntoIterator<Item
let phrase = format!("recovery phrase: {}", brain.phrase());
(keypair, Some(phrase))
} else {
(Random.generate()?, None)
(Random.generate(), None)
}
} else if args.cmd_prefix {
let prefix = args.arg_prefix.from_hex()?;
Expand Down Expand Up @@ -271,7 +271,7 @@ fn execute<S, I>(command: I) -> Result<String, Error> where I: IntoIterator<Item
while let Some(phrase) = it.next() {
i += 1;

let keypair = Brain::new(phrase.clone()).generate().unwrap();
let keypair = Brain::new(phrase.clone()).generate();
if keypair.address() == address {
return Ok(Some((phrase, keypair)))
}
Expand Down
11 changes: 4 additions & 7 deletions accounts/ethkey/src/brain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
// You should have received a copy of the GNU General Public License
// along with Parity Ethereum. If not, see <http://www.gnu.org/licenses/>.

use std::convert::Infallible;
use parity_crypto::publickey::{KeyPair, Generator, Secret};
use parity_crypto::Keccak256;
use parity_wordlist;
Expand All @@ -33,9 +32,7 @@ impl Brain {
}

impl Generator for Brain {
type Error = Infallible;

fn generate(&mut self) -> Result<KeyPair, Self::Error> {
fn generate(&mut self) -> KeyPair {
let seed = self.0.clone();
let mut secret = seed.into_bytes().keccak256();

Expand All @@ -51,7 +48,7 @@ impl Generator for Brain {
{
if pair.address()[0] == 0 {
trace!("Testing: {}, got: {:?}", self.0, pair.address());
return Ok(pair)
return pair
}
}
},
Expand All @@ -68,8 +65,8 @@ mod tests {
#[test]
fn test_brain() {
let words = "this is sparta!".to_owned();
let first_keypair = Brain::new(words.clone()).generate().unwrap();
let second_keypair = Brain::new(words.clone()).generate().unwrap();
let first_keypair = Brain::new(words.clone()).generate();
let second_keypair = Brain::new(words.clone()).generate();
assert_eq!(first_keypair.secret(), second_keypair.secret());
}
}
8 changes: 2 additions & 6 deletions accounts/ethkey/src/brain_prefix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,11 @@ impl BrainPrefix {
pub fn phrase(&self) -> &str {
&self.last_phrase
}
}

impl Generator for BrainPrefix {
type Error = Error;

fn generate(&mut self) -> Result<KeyPair, Error> {
pub fn generate(&mut self) -> Result<KeyPair, Error> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved this to a free function as it needs to return an error, whereas the Generate trait changed (it can't fail now).

for _ in 0..self.iterations {
let phrase = wordlist::random_phrase(self.no_of_words);
let keypair = Brain::new(phrase.clone()).generate().unwrap();
let keypair = Brain::new(phrase.clone()).generate();
if keypair.address().as_ref().starts_with(&self.prefix) {
self.last_phrase = phrase;
return Ok(keypair)
Expand Down
2 changes: 1 addition & 1 deletion accounts/ethkey/src/brain_recover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub fn brain_recover(
) -> Option<String> {
let it = PhrasesIterator::from_known_phrase(known_phrase, expected_words);
for phrase in it {
let keypair = Brain::new(phrase.clone()).generate().expect("Brain wallets are infallible; qed");
let keypair = Brain::new(phrase.clone()).generate();
trace!("Testing: {}, got: {:?}", phrase, keypair.address());
if &keypair.address() == address {
return Some(phrase);
Expand Down
13 changes: 3 additions & 10 deletions accounts/ethkey/src/prefix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,12 @@ pub struct Prefix {

impl Prefix {
pub fn new(prefix: Vec<u8>, iterations: usize) -> Self {
Prefix {
prefix: prefix,
iterations: iterations,
}
Prefix { prefix, iterations }
}
}

impl Generator for Prefix {
type Error = Error;

fn generate(&mut self) -> Result<KeyPair, Error> {
pub fn generate(&mut self) -> Result<KeyPair, Error> {
for _ in 0..self.iterations {
let keypair = Random.generate()?;
let keypair = Random.generate();
if keypair.address().as_ref().starts_with(&self.prefix) {
return Ok(keypair)
}
Expand Down
2 changes: 1 addition & 1 deletion accounts/ethstore/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ rustc-hex = "1.0"
tiny-keccak = "1.4"
time = "0.1.34"
parking_lot = "0.10.0"
parity-crypto = { version = "0.4.2", features = ["publickey"] }
parity-crypto = { version = "0.5.0", features = ["publickey"] }
ethereum-types = "0.8.0"
dir = { path = "../../util/dir" }
smallvec = "1.2.0"
Expand Down
2 changes: 1 addition & 1 deletion accounts/ethstore/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ serde_derive = "1.0"
parking_lot = "0.10.0"
ethstore = { path = "../" }
ethkey = { path = "../../ethkey" }
parity-crypto = { version = "0.4.2", features = ["publickey"] }
parity-crypto = { version = "0.5.0", features = ["publickey"] }
dir = { path = '../../../util/dir' }
panic_hook = { path = "../../../util/panic-hook" }

Expand Down
4 changes: 2 additions & 2 deletions accounts/ethstore/src/account/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ mod tests {

#[test]
fn crypto_with_secret_create() {
let keypair = Random.generate().unwrap();
let keypair = Random.generate();
let passwd = "this is sparta".into();
let crypto = Crypto::with_secret(keypair.secret(), &passwd, 10240).unwrap();
let secret = crypto.secret(&passwd).unwrap();
Expand All @@ -173,7 +173,7 @@ mod tests {

#[test]
fn crypto_with_secret_invalid_password() {
let keypair = Random.generate().unwrap();
let keypair = Random.generate();
let crypto = Crypto::with_secret(keypair.secret(), &"this is sparta".into(), 10240).unwrap();
assert_matches!(crypto.secret(&"this is sparta!".into()), Err(Error::InvalidPassword))
}
Expand Down
8 changes: 4 additions & 4 deletions accounts/ethstore/src/account/safe_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,21 +205,21 @@ mod tests {

#[test]
fn sign_and_verify_public() {
let keypair = Random.generate().unwrap();
let keypair = Random.generate();
let password = "hello world".into();
let message = Message::default();
let message = [1u8; 32].into();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In upstream rust-secp256k1, a Message cannot be all zeroes anymore. This is one big reason why this PR is so noisy: we have a lot of code that use the Default trait to initialize values.

let account = SafeAccount::create(&keypair, [0u8; 16], &password, 10240, "Test".to_owned(), "{}".to_owned());
let signature = account.unwrap().sign(&password, &message).unwrap();
assert!(verify_public(keypair.public(), &signature, &message).unwrap());
}

#[test]
fn change_password() {
let keypair = Random.generate().unwrap();
let keypair = Random.generate();
let first_password = "hello world".into();
let sec_password = "this is sparta".into();
let i = 10240;
let message = Message::default();
let message = [1u8; 32].into();
let account = SafeAccount::create(&keypair, [0u8; 16], &first_password, i, "Test".to_owned(), "{}".to_owned()).unwrap();
let new_account = account.change_password(&first_password, &sec_password, i).unwrap();
assert!(account.sign(&first_password, &message).is_ok());
Expand Down
6 changes: 3 additions & 3 deletions accounts/ethstore/src/accounts_dir/disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ mod test {
// given
let mut dir = env::temp_dir();
dir.push("ethstore_should_create_new_account");
let keypair = Random.generate().unwrap();
let keypair = Random.generate();
let password = "hello world".into();
let directory = RootDiskDirectory::create(dir.clone()).unwrap();

Expand All @@ -385,7 +385,7 @@ mod test {
// given
let mut dir = env::temp_dir();
dir.push("ethstore_should_handle_duplicate_filenames");
let keypair = Random.generate().unwrap();
let keypair = Random.generate();
let password = "hello world".into();
let directory = RootDiskDirectory::create(dir.clone()).unwrap();

Expand Down Expand Up @@ -472,7 +472,7 @@ mod test {
15130871412783076140
);

let keypair = Random.generate().unwrap();
let keypair = Random.generate();
let password = "test pass".into();
let account = SafeAccount::create(&keypair, [0u8; 16], &password, 1024, "Test".to_owned(), "{}".to_owned());
directory.insert(account.unwrap()).expect("Account should be inserted ok");
Expand Down
8 changes: 5 additions & 3 deletions accounts/ethstore/src/ethstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ mod tests {
use ethereum_types::H256;

fn keypair() -> KeyPair {
Random.generate().unwrap()
Random.generate()
}

fn store() -> EthStore {
Expand Down Expand Up @@ -820,6 +820,7 @@ mod tests {
let passwd2 = "xzy".into();
let multi_store = multi_store();
let keypair = keypair();
let message = [1u8; 32].into();
let address = store.insert_account(SecretVaultRef::Root, keypair.secret().clone(), &passwd1).unwrap();
assert_eq!(multi_store.accounts().unwrap().len(), 0);

Expand All @@ -828,7 +829,7 @@ mod tests {

// then
assert!(store.test_password(&address, &passwd1).unwrap(), "First password should work for store.");
assert!(multi_store.sign(&address, &passwd2, &Default::default()).is_ok(), "Second password should work for second store.");
assert!(multi_store.sign(&address, &passwd2, &message).is_ok(), "Second password should work for second store.");
assert_eq!(multi_store.accounts().unwrap().len(), 1);
}

Expand Down Expand Up @@ -1092,8 +1093,9 @@ mod tests {
let accounts = store.accounts().unwrap();
assert_eq!(accounts.len(), 2);

let message = [1u8; 32].into();
// and we can sign with the derived contract
assert!(store.sign(&derived, &"test".into(), &Default::default()).is_ok(), "Second password should work for second store.");
assert!(store.sign(&derived, &"test".into(), &message).is_ok(), "Second password should work for second store.");
}

#[test]
Expand Down
18 changes: 10 additions & 8 deletions accounts/ethstore/tests/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ fn secret_store_open_not_existing() {
}

fn random_secret() -> Secret {
Random.generate().unwrap().secret().clone()
Random.generate().secret().clone()
}

#[test]
Expand All @@ -62,9 +62,10 @@ fn secret_store_sign() {
let store = EthStore::open(Box::new(dir)).unwrap();
assert!(store.insert_account(SecretVaultRef::Root, random_secret(), &"".into()).is_ok());
let accounts = store.accounts().unwrap();
let message = [1u8; 32].into();
assert_eq!(accounts.len(), 1);
assert!(store.sign(&accounts[0], &"".into(), &Default::default()).is_ok());
assert!(store.sign(&accounts[0], &"1".into(), &Default::default()).is_err());
assert!(store.sign(&accounts[0], &"".into(), &message).is_ok());
assert!(store.sign(&accounts[0], &"1".into(), &message).is_err());
}

#[test]
Expand All @@ -73,11 +74,12 @@ fn secret_store_change_password() {
let store = EthStore::open(Box::new(dir)).unwrap();
assert!(store.insert_account(SecretVaultRef::Root, random_secret(), &"".into()).is_ok());
let accounts = store.accounts().unwrap();
let message = [1u8; 32].into();
assert_eq!(accounts.len(), 1);
assert!(store.sign(&accounts[0], &"".into(), &Default::default()).is_ok());
assert!(store.sign(&accounts[0], &"".into(), &message).is_ok());
assert!(store.change_password(&accounts[0], &"".into(), &"1".into()).is_ok());
assert!(store.sign(&accounts[0], &"".into(), &Default::default()).is_err());
assert!(store.sign(&accounts[0], &"1".into(), &Default::default()).is_ok());
assert!(store.sign(&accounts[0], &"".into(), &message).is_err());
assert!(store.sign(&accounts[0], &"1".into(), &message).is_ok());
}

#[test]
Expand All @@ -95,7 +97,7 @@ fn secret_store_remove_account() {
fn test_path() -> &'static str {
match ::std::fs::metadata("ethstore") {
Ok(_) => "ethstore/tests/res/geth_keystore",
Err(_) => "tests/res/geth_keystore",
Err(_) => "tests/res/geth_keystore",
}
}

Expand Down Expand Up @@ -148,7 +150,7 @@ fn test_decrypting_files_with_short_ciphertext() {
StoreAccountRef::root(Address::from_str("d1e64e5480bfaf733ba7d48712decb8227797a4e").unwrap()),
]);

let message = Default::default();
let message = [1u8; 32].into();

let s1 = store.sign(&accounts[0], &"foo".into(), &message).unwrap();
let s2 = store.sign(&accounts[1], &"foo".into(), &message).unwrap();
Expand Down
Loading