From e1d09eb0fb385f3f556430351369907678bd69ec Mon Sep 17 00:00:00 2001 From: Eran Rundstein Date: Tue, 19 Sep 2023 11:05:59 -0700 Subject: [PATCH 1/6] feat(multisig): support ed25519 keys --- ampd/src/handlers/multisig.rs | 8 + contracts/multisig/Cargo.toml | 7 +- contracts/multisig/src/contract.rs | 20 +-- contracts/multisig/src/ed25519.rs | 13 ++ contracts/multisig/src/key.rs | 183 ++++++++++++++++++-- contracts/multisig/src/lib.rs | 3 + contracts/multisig/src/signing.rs | 237 +++++++++++++++----------- contracts/multisig/src/test/common.rs | 50 +++++- contracts/multisig/src/types.rs | 4 +- 9 files changed, 392 insertions(+), 133 deletions(-) create mode 100644 contracts/multisig/src/ed25519.rs diff --git a/ampd/src/handlers/multisig.rs b/ampd/src/handlers/multisig.rs index f3bea9faa..17ee7bdc0 100644 --- a/ampd/src/handlers/multisig.rs +++ b/ampd/src/handlers/multisig.rs @@ -56,6 +56,14 @@ where .map_err(D::Error::custom)? .into(), )), + + multisig::key::PublicKey::Ed25519(hex) => { + let pk: cosmrs::tendermint::PublicKey = + cosmrs::tendermint::crypto::ed25519::VerificationKey::try_from(hex.as_ref()) + .map_err(D::Error::custom)? + .into(); + Ok((address, pk.into())) + } }) .collect() } diff --git a/contracts/multisig/Cargo.toml b/contracts/multisig/Cargo.toml index 2aafdad13..1cd348b59 100644 --- a/contracts/multisig/Cargo.toml +++ b/contracts/multisig/Cargo.toml @@ -15,13 +15,16 @@ exclude = [ crate-type = ["cdylib", "rlib"] [features] -default = ["secp256k1"] +default = ["secp256k1", "ed25519"] # for more explicit tests, cargo test --features=backtraces backtraces = ["cosmwasm-std/backtraces"] # use library feature to disable all instantiate/execute/query exports library = [] -# use this feature to use secp256k1 for signature verification +# use this feature to enable secp256k1 for signature verification secp256k1 = [] +# use this feature to enable ed25519 for signature verification +ed25519 = [] + [package.metadata.scripts] optimize = """docker run --rm -v "$(pwd)":/code \ diff --git a/contracts/multisig/src/contract.rs b/contracts/multisig/src/contract.rs index 5531e723b..e193a1ba8 100644 --- a/contracts/multisig/src/contract.rs +++ b/contracts/multisig/src/contract.rs @@ -291,7 +291,7 @@ mod tests { use crate::{ key::{KeyType, PublicKey, Signature}, msg::Multisig, - test::common::test_data, + test::common::ecdsa_test_data, test::common::{build_snapshot, TestSigner}, types::MultisigState, }; @@ -321,7 +321,7 @@ mod tests { let info = mock_info(PROVER, &[]); let env = mock_env(); - let signers = test_data::signers(); + let signers = ecdsa_test_data::signers(); let pub_keys = signers .iter() .map(|signer| { @@ -380,7 +380,7 @@ mod tests { let info = mock_info(sender, &[]); let env = mock_env(); - let message = test_data::message(); + let message = ecdsa_test_data::message(); let msg = ExecuteMsg::StartSigningSession { key_id: "key".to_string(), msg: message.clone(), @@ -510,7 +510,7 @@ mod tests { subkey: "key".to_string(), }; let key = get_key(deps.as_ref().storage, &key_id).unwrap(); - let message = test_data::message(); + let message = ecdsa_test_data::message(); assert_eq!(session.id, Uint64::one()); assert_eq!(session.key_id, key.id); @@ -563,7 +563,7 @@ mod tests { fn test_submit_signature() { let mut deps = setup_with_session_started(); - let signers = test_data::signers(); + let signers = ecdsa_test_data::signers(); let session_id = Uint64::one(); let signer = signers.get(0).unwrap().to_owned(); @@ -606,7 +606,7 @@ mod tests { fn test_submit_signature_completed() { let mut deps = setup_with_session_started(); - let signers = test_data::signers(); + let signers = ecdsa_test_data::signers(); let session_id = Uint64::one(); let signer = signers.get(0).unwrap().to_owned(); @@ -646,7 +646,7 @@ mod tests { let mut deps = setup_with_session_started(); let invalid_session_id = Uint64::zero(); - let signer = test_data::signers().get(0).unwrap().to_owned(); + let signer = ecdsa_test_data::signers().get(0).unwrap().to_owned(); let res = do_sign(deps.as_mut(), invalid_session_id, &signer); assert_eq!( @@ -663,7 +663,7 @@ mod tests { let mut deps = setup_with_session_started(); let session_id = Uint64::one(); - let signer = test_data::signers().get(0).unwrap().to_owned(); + let signer = ecdsa_test_data::signers().get(0).unwrap().to_owned(); do_sign(deps.as_mut(), session_id, &signer).unwrap(); let msg = QueryMsg::GetMultisig { session_id }; @@ -698,7 +698,7 @@ mod tests { fn test_register_key() { let mut deps = mock_dependencies(); do_instantiate(deps.as_mut()).unwrap(); - let signers = test_data::signers(); + let signers = ecdsa_test_data::signers(); let pub_keys = signers .iter() .map(|signer| (signer.address.clone(), signer.pub_key.clone())) @@ -732,7 +732,7 @@ mod tests { fn test_update_key() { let mut deps = mock_dependencies(); do_instantiate(deps.as_mut()).unwrap(); - let signers = test_data::signers(); + let signers = ecdsa_test_data::signers(); let pub_keys = signers .iter() .map(|signer| (signer.address.clone(), signer.pub_key.clone())) diff --git a/contracts/multisig/src/ed25519.rs b/contracts/multisig/src/ed25519.rs new file mode 100644 index 000000000..fb9b02935 --- /dev/null +++ b/contracts/multisig/src/ed25519.rs @@ -0,0 +1,13 @@ + +// TODO: Logic specific to secp256k1 will most likely be handled by core in the future. +use crate::ContractError; + +const ECDSA_SIGNATURE_LEN: usize = 64; + +pub fn ed25519_verify(msg_hash: &[u8], sig: &[u8], pub_key: &[u8]) -> Result { + cosmwasm_crypto::ed25519_verify(msg_hash, &sig[0..ECDSA_SIGNATURE_LEN], pub_key).map_err(|e| { + ContractError::SignatureVerificationFailed { + reason: e.to_string(), + } + }) +} diff --git a/contracts/multisig/src/key.rs b/contracts/multisig/src/key.rs index 672b2691e..4616962a3 100644 --- a/contracts/multisig/src/key.rs +++ b/contracts/multisig/src/key.rs @@ -1,20 +1,22 @@ use cosmwasm_schema::cw_serde; use cosmwasm_std::{HexBinary, StdError, StdResult}; use cw_storage_plus::{KeyDeserialize, PrimaryKey}; -use serde::{de::Error, Deserializer}; +use serde::{de::Error, Deserialize, Deserializer}; -use crate::{secp256k1::ecdsa_verify, types::MsgToSign, ContractError}; +use crate::{ed25519::ed25519_verify, secp256k1::ecdsa_verify, types::MsgToSign, ContractError}; #[cw_serde] #[derive(Copy)] pub enum KeyType { Ecdsa, + Ed25519, } #[cw_serde] #[derive(PartialOrd, Ord, Eq)] pub enum Signature { Ecdsa(HexBinary), + Ed25519(HexBinary), } #[cw_serde] @@ -22,9 +24,11 @@ pub enum Signature { pub enum PublicKey { #[serde(deserialize_with = "deserialize_ecdsa_key")] Ecdsa(HexBinary), + + #[serde(deserialize_with = "deserialize_ed25519_key")] + Ed25519(HexBinary), } -use serde::Deserialize; fn deserialize_ecdsa_key<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, @@ -35,6 +39,16 @@ where Ok(pk) } +fn deserialize_ed25519_key<'de, D>(deserializer: D) -> Result +where + D: Deserializer<'de>, +{ + let pk: HexBinary = Deserialize::deserialize(deserializer)?; + PublicKey::try_from((KeyType::Ed25519, pk.clone())) + .map_err(|e| D::Error::custom(format!("failed to deserialize public key: {}", e)))?; + Ok(pk) +} + pub trait KeyTyped { fn matches(&self, other: &T) -> bool where @@ -50,6 +64,7 @@ impl KeyTyped for PublicKey { fn key_type(&self) -> KeyType { match self { PublicKey::Ecdsa(_) => KeyType::Ecdsa, + PublicKey::Ed25519(_) => KeyType::Ed25519, } } } @@ -58,6 +73,7 @@ impl KeyTyped for Signature { fn key_type(&self) -> KeyType { match self { Signature::Ecdsa(_) => KeyType::Ecdsa, + Signature::Ed25519(_) => KeyType::Ed25519, } } } @@ -72,6 +88,10 @@ impl Signature { (Signature::Ecdsa(sig), PublicKey::Ecdsa(pub_key)) => { ecdsa_verify(msg.into(), sig.as_ref(), pub_key.as_ref()) } + (Signature::Ed25519(sig), PublicKey::Ed25519(pub_key)) => { + ed25519_verify(msg.into(), sig.as_ref(), pub_key.as_ref()) + } + _ => Err(ContractError::KeyTypeMismatch), } } } @@ -106,6 +126,9 @@ const ECDSA_COMPRESSED_PUBKEY_LEN: usize = 33; const ECDSA_UNCOMPRESSED_PUBKEY_LEN: usize = 65; const EVM_SIGNATURE_LEN: usize = 65; +const ED25519_PUBKEY_LEN: usize = 32; +const ED25519_SIGNATURE_LEN: usize = 64; + impl TryFrom<(KeyType, HexBinary)> for PublicKey { type Error = ContractError; @@ -121,6 +144,16 @@ impl TryFrom<(KeyType, HexBinary)> for PublicKey { } Ok(PublicKey::Ecdsa(pub_key)) } + + KeyType::Ed25519 => { + if pub_key.len() != ED25519_PUBKEY_LEN { + return Err(ContractError::InvalidPublicKeyFormat { + reason: "Invalid input length".into(), + }); + } + + Ok(PublicKey::Ed25519(pub_key)) + } } } } @@ -138,6 +171,16 @@ impl TryFrom<(KeyType, HexBinary)> for Signature { } Ok(Signature::Ecdsa(sig)) } + + KeyType::Ed25519 => { + if sig.len() != ED25519_SIGNATURE_LEN { + return Err(ContractError::InvalidSignatureFormat { + reason: "Invalid input length".into(), + }); + } + + Ok(Signature::Ed25519(sig)) + } } } } @@ -146,6 +189,7 @@ impl AsRef<[u8]> for PublicKey { fn as_ref(&self) -> &[u8] { match self { PublicKey::Ecdsa(pk) => pk.as_ref(), + PublicKey::Ed25519(pk) => pk.as_ref(), } } } @@ -154,6 +198,7 @@ impl AsRef<[u8]> for Signature { fn as_ref(&self) -> &[u8] { match self { Signature::Ecdsa(sig) => sig.as_ref(), + Signature::Ed25519(sig) => sig.as_ref(), } } } @@ -162,6 +207,7 @@ impl From for Vec { fn from(value: Signature) -> Vec { match value { Signature::Ecdsa(sig) => sig.to_vec(), + Signature::Ed25519(sig) => sig.to_vec(), } } } @@ -170,6 +216,7 @@ impl From for HexBinary { fn from(original: Signature) -> Self { match original { Signature::Ecdsa(sig) => sig, + Signature::Ed25519(sig) => sig, } } } @@ -178,15 +225,16 @@ impl From for HexBinary { fn from(original: PublicKey) -> Self { match original { PublicKey::Ecdsa(sig) => sig, + PublicKey::Ed25519(sig) => sig, } } } #[cfg(test)] -mod test { +mod ecdsa_tests { use cosmwasm_std::HexBinary; - use crate::{key::Signature, test::common::test_data, types::MsgToSign, ContractError}; + use crate::{key::Signature, test::common::ecdsa_test_data, types::MsgToSign, ContractError}; use super::{KeyType, PublicKey}; @@ -218,7 +266,7 @@ mod test { #[test] fn test_try_from_hexbinary_to_ecdsa_public_key() { - let hex = test_data::pub_key(); + let hex = ecdsa_test_data::pub_key(); let pub_key = PublicKey::try_from((KeyType::Ecdsa, hex.clone())).unwrap(); assert_eq!(HexBinary::from(pub_key), hex); } @@ -236,7 +284,7 @@ mod test { #[test] fn test_try_from_hexbinary_to_signature() { - let hex = test_data::signature(); + let hex = ecdsa_test_data::signature(); let signature = Signature::try_from((KeyType::Ecdsa, hex.clone())).unwrap(); assert_eq!(HexBinary::from(signature), hex); } @@ -256,9 +304,9 @@ mod test { #[test] fn test_verify_signature() { - let signature = Signature::try_from((KeyType::Ecdsa, test_data::signature())).unwrap(); - let message = MsgToSign::try_from(test_data::message()).unwrap(); - let public_key = PublicKey::try_from((KeyType::Ecdsa, test_data::pub_key())).unwrap(); + let signature = Signature::try_from((KeyType::Ecdsa, ecdsa_test_data::signature())).unwrap(); + let message = MsgToSign::try_from(ecdsa_test_data::message()).unwrap(); + let public_key = PublicKey::try_from((KeyType::Ecdsa, ecdsa_test_data::pub_key())).unwrap(); let result = signature.verify(&message, &public_key).unwrap(); assert_eq!(result, true); } @@ -271,8 +319,8 @@ mod test { .unwrap(); let signature = Signature::try_from((KeyType::Ecdsa, invalid_signature)).unwrap(); - let message = MsgToSign::try_from(test_data::message()).unwrap(); - let public_key = PublicKey::try_from((KeyType::Ecdsa, test_data::pub_key())).unwrap(); + let message = MsgToSign::try_from(ecdsa_test_data::message()).unwrap(); + let public_key = PublicKey::try_from((KeyType::Ecdsa, ecdsa_test_data::pub_key())).unwrap(); let result = signature.verify(&message, &public_key).unwrap(); assert_eq!(result, false); } @@ -284,10 +332,117 @@ mod test { ) .unwrap(); - let signature = Signature::try_from((KeyType::Ecdsa, test_data::signature())).unwrap(); - let message = MsgToSign::try_from(test_data::message()).unwrap(); + let signature = Signature::try_from((KeyType::Ecdsa, ecdsa_test_data::signature())).unwrap(); + let message = MsgToSign::try_from(ecdsa_test_data::message()).unwrap(); let public_key = PublicKey::try_from((KeyType::Ecdsa, invalid_pub_key)).unwrap(); let result = signature.verify(&message, &public_key).unwrap(); assert_eq!(result, false); } } + +#[cfg(test)] +mod ed25519_tests { + use cosmwasm_std::HexBinary; + + use crate::{key::Signature, test::common::ed25519_test_data, types::MsgToSign, ContractError}; + + use super::{KeyType, PublicKey}; + + #[test] + fn deserialize_ed25519_key() { + let key = PublicKey::try_from(( + KeyType::Ed25519, + HexBinary::from_hex("d17abc4475ad96b2d9c36f569b6ed1ac8345c562cac906e5f5882230651af574") + .unwrap(), + )) + .unwrap(); + + let serialized = serde_json::to_string(&key).unwrap(); + let deserialized: Result = serde_json::from_str(&serialized); + assert!(deserialized.is_ok()); + assert_eq!(deserialized.unwrap(), key); + } + + #[test] + fn deserialize_ed25519_key_fails() { + let key = PublicKey::Ed25519(HexBinary::from_hex("deadbeef").unwrap()); + + let serialized = serde_json::to_string(&key).unwrap(); + let deserialized: Result = serde_json::from_str(&serialized); + assert!(deserialized.is_err()); + } + + #[test] + fn test_try_from_hexbinary_to_ed25519_public_key() { + let hex = ed25519_test_data::pub_key(); + let pub_key = PublicKey::try_from((KeyType::Ed25519, hex.clone())).unwrap(); + assert_eq!(HexBinary::from(pub_key), hex); + } + + #[test] + fn test_try_from_hexbinary_to_eccdsa_public_key_fails() { + let hex = HexBinary::from_hex("049b").unwrap(); + assert_eq!( + PublicKey::try_from((KeyType::Ed25519, hex.clone())).unwrap_err(), + ContractError::InvalidPublicKeyFormat { + reason: "Invalid input length".into() + } + ); + } + + #[test] + fn test_try_from_hexbinary_to_signature() { + let hex = ed25519_test_data::signature(); + let signature = Signature::try_from((KeyType::Ed25519, hex.clone())).unwrap(); + assert_eq!(HexBinary::from(signature), hex); + } + + #[test] + fn test_try_from_hexbinary_to_signature_fails() { + let hex = + HexBinary::from_hex("304a300506032b65700341007ac37da74e66005581fa85eaf15a54e0bfcb8c857e3ca4e4bc9e210ed0276bf0792562d474a709c942a488cf53f95823a2d58892981043cd687ccab340cc3907") + .unwrap(); + assert_eq!( + Signature::try_from((KeyType::Ed25519, hex.clone())).unwrap_err(), + ContractError::InvalidSignatureFormat { + reason: "Invalid input length".into() + } + ); + } + + #[test] + fn test_verify_signature() { + let signature = Signature::try_from((KeyType::Ed25519, ed25519_test_data::signature())).unwrap(); + let message = MsgToSign::try_from(ed25519_test_data::message()).unwrap(); + let public_key = PublicKey::try_from((KeyType::Ed25519, ed25519_test_data::pub_key())).unwrap(); + let result = signature.verify(&message, &public_key).unwrap(); + assert_eq!(result, true); + } + + #[test] + fn test_verify_signature_invalid_signature() { + let invalid_signature = HexBinary::from_hex( + "1fe264eb7258d48d8feedea4d237ccb20157fbe5eb412bc971d758d072b036a99b06d20853c1f23cdf82085917e08dda2fcfbb5d4d7ee17d74e4988ae81d0308", + ) + .unwrap(); + + let signature = Signature::try_from((KeyType::Ed25519, invalid_signature)).unwrap(); + let message = MsgToSign::try_from(ed25519_test_data::message()).unwrap(); + let public_key = PublicKey::try_from((KeyType::Ed25519, ed25519_test_data::pub_key())).unwrap(); + let result = signature.verify(&message, &public_key).unwrap(); + assert_eq!(result, false); + } + + #[test] + fn test_verify_signature_invalid_pub_key() { + let invalid_pub_key = + HexBinary::from_hex("ffff8a3c50c8381541b682f4941ef7df351376f60e3fa0296f48f0f767a4f321") + .unwrap(); + + let signature = Signature::try_from((KeyType::Ed25519, ed25519_test_data::signature())).unwrap(); + let message = MsgToSign::try_from(ed25519_test_data::message()).unwrap(); + let public_key = PublicKey::try_from((KeyType::Ed25519, invalid_pub_key)).unwrap(); + let result = signature.verify(&message, &public_key).unwrap(); + assert_eq!(result, false); + } +} diff --git a/contracts/multisig/src/lib.rs b/contracts/multisig/src/lib.rs index 1f7927c63..1ab6710b2 100644 --- a/contracts/multisig/src/lib.rs +++ b/contracts/multisig/src/lib.rs @@ -10,6 +10,9 @@ pub mod types; #[cfg(feature = "secp256k1")] mod secp256k1; +#[cfg(feature = "ed25519")] +mod ed25519; + #[cfg(test)] pub mod test; diff --git a/contracts/multisig/src/signing.rs b/contracts/multisig/src/signing.rs index cdbb129b6..eb7bd85c6 100644 --- a/contracts/multisig/src/signing.rs +++ b/contracts/multisig/src/signing.rs @@ -101,8 +101,8 @@ mod tests { use crate::{ key::KeyType, state::{KEYS, SIGNING_SESSIONS}, - test::common::test_data, test::common::{build_key, build_snapshot, TestSigner}, + test::common::{ecdsa_test_data, ed25519_test_data}, }; use super::*; @@ -112,28 +112,54 @@ mod tests { pub key_id: KeyID, pub message: MsgToSign, pub signers: Vec, + pub key_type: KeyType, } - fn setup() -> TestConfig { + fn ecdsa_setup() -> TestConfig { let mut store = MockStorage::new(); - let signers = test_data::signers(); + let signers = ecdsa_test_data::signers(); let snapshot = build_snapshot(&signers); let key_id = KeyID { owner: Addr::unchecked("owner"), subkey: "subkey".to_string(), }; - let key = build_key(key_id, &signers, snapshot); + let key = build_key(KeyType::Ecdsa, key_id, &signers, snapshot); KEYS.save(&mut store, (&key.id).into(), &key).unwrap(); - let message: MsgToSign = test_data::message().try_into().unwrap(); + let message: MsgToSign = ecdsa_test_data::message().try_into().unwrap(); TestConfig { store, key_id: key.id, message, signers, + key_type: KeyType::Ecdsa, + } + } + + fn ed25519_setup() -> TestConfig { + let mut store = MockStorage::new(); + + let signers = ed25519_test_data::signers(); + let snapshot = build_snapshot(&signers); + + let key_id = KeyID { + owner: Addr::unchecked("owner"), + subkey: "subkey".to_string(), + }; + let key = build_key(KeyType::Ed25519, key_id, &signers, snapshot); + KEYS.save(&mut store, (&key.id).into(), &key).unwrap(); + + let message: MsgToSign = ed25519_test_data::message().try_into().unwrap(); + + TestConfig { + store, + key_id: key.id, + message, + signers, + key_type: KeyType::Ed25519, } } @@ -149,7 +175,7 @@ mod tests { let res = session.add_signature( key, signer.address.clone().into_string(), - Signature::try_from((KeyType::Ecdsa, signer.signature.clone())).unwrap(), + Signature::try_from((config.key_type, signer.signature.clone())).unwrap(), ); SIGNING_SESSIONS.save(&mut config.store, session.id.u64(), &session)?; @@ -159,109 +185,116 @@ mod tests { #[test] fn test_add_signature() { - let mut config = setup(); - - let mut session = - SigningSession::new(Uint64::one(), config.key_id.clone(), config.message.clone()); - - let result = sign(&mut session, 0, &mut config); - let stored = SIGNING_SESSIONS - .load(&config.store, session.id.u64()) - .unwrap(); - assert!(result.is_ok()); - assert_eq!(session.signatures.len(), 1); - assert_eq!(session.state, MultisigState::Pending); - assert_eq!(stored, session); - - let result = sign(&mut session, 0, &mut config); - let stored = SIGNING_SESSIONS - .load(&config.store, session.id.u64()) - .unwrap(); - assert_eq!( - result.unwrap_err(), - ContractError::DuplicateSignature { - session_id: session.id, - signer: config.signers[0].address.clone().into_string() - } - ); - assert_eq!(session.signatures.len(), 1); - assert_eq!(session.state, MultisigState::Pending); - assert_eq!(stored, session); - - let result = sign(&mut session, 1, &mut config); - let stored = SIGNING_SESSIONS - .load(&config.store, session.id.u64()) - .unwrap(); - assert!(result.is_ok()); - assert_eq!(session.signatures.len(), 2); - assert_eq!(session.state, MultisigState::Completed); - assert_eq!(stored, session); - - let result = sign(&mut session, 2, &mut config); - let stored = SIGNING_SESSIONS - .load(&config.store, session.id.u64()) - .unwrap(); - assert_eq!( - result.unwrap_err(), - ContractError::SigningSessionClosed { - session_id: session.id - } - ); - assert_eq!(session.signatures.len(), 2); - assert_eq!(session.state, MultisigState::Completed); - assert_eq!(stored, session); + for mut config in [ecdsa_setup(), ed25519_setup()] { + let mut session = + SigningSession::new(Uint64::one(), config.key_id.clone(), config.message.clone()); + + let result = sign(&mut session, 0, &mut config); + let stored = SIGNING_SESSIONS + .load(&config.store, session.id.u64()) + .unwrap(); + assert!(result.is_ok()); + assert_eq!(session.signatures.len(), 1); + assert_eq!(session.state, MultisigState::Pending); + assert_eq!(stored, session); + + let result = sign(&mut session, 0, &mut config); + let stored = SIGNING_SESSIONS + .load(&config.store, session.id.u64()) + .unwrap(); + assert_eq!( + result.unwrap_err(), + ContractError::DuplicateSignature { + session_id: session.id, + signer: config.signers[0].address.clone().into_string() + } + ); + assert_eq!(session.signatures.len(), 1); + assert_eq!(session.state, MultisigState::Pending); + assert_eq!(stored, session); + + let result = sign(&mut session, 1, &mut config); + let stored = SIGNING_SESSIONS + .load(&config.store, session.id.u64()) + .unwrap(); + assert!(result.is_ok()); + assert_eq!(session.signatures.len(), 2); + assert_eq!(session.state, MultisigState::Completed); + assert_eq!(stored, session); + + let result = sign(&mut session, 2, &mut config); + let stored = SIGNING_SESSIONS + .load(&config.store, session.id.u64()) + .unwrap(); + assert_eq!( + result.unwrap_err(), + ContractError::SigningSessionClosed { + session_id: session.id + } + ); + assert_eq!(session.signatures.len(), 2); + assert_eq!(session.state, MultisigState::Completed); + assert_eq!(stored, session); + } } #[test] fn test_add_invalid_signature() { - let config = setup(); - - let mut session = - SigningSession::new(Uint64::one(), config.key_id.clone(), config.message.clone()); - - let invalid_sig : Signature = (KeyType::Ecdsa, HexBinary::from_hex("a58c9543b9df54578ec45838948e19afb1c6e4c86b34d9899b10b44e619ea74e19b457611e41a047030ed233af437d7ecff84de97cb6b3c13d73d22874e035111c") - .unwrap()).try_into().unwrap(); - - let key = KEYS.load(&config.store, (&session.key_id).into()).unwrap(); - - let result = session.add_signature( - key, - config.signers[0].address.clone().into_string(), - invalid_sig, - ); - - assert_eq!( - result.unwrap_err(), - ContractError::InvalidSignature { - session_id: session.id, - signer: config.signers[0].address.clone().into_string() - } - ); + for config in [ecdsa_setup(), ed25519_setup()] { + let mut session = + SigningSession::new(Uint64::one(), config.key_id.clone(), config.message.clone()); + + let sig_bytes = match config.key_type { + KeyType::Ecdsa => "a58c9543b9df54578ec45838948e19afb1c6e4c86b34d9899b10b44e619ea74e19b457611e41a047030ed233af437d7ecff84de97cb6b3c13d73d22874e035111c", + KeyType::Ed25519 => "1fe264eb7258d48d8feedea4d237ccb20157fbe5eb412bc971d758d072b036a99b06d20853c1f23cdf82085917e08dda2fcfbb5d4d7ee17d74e4988ae81d0308", + }; + + let invalid_sig: Signature = (config.key_type, HexBinary::from_hex(sig_bytes).unwrap()) + .try_into() + .unwrap(); + + let key = KEYS.load(&config.store, (&session.key_id).into()).unwrap(); + + let result = session.add_signature( + key, + config.signers[0].address.clone().into_string(), + invalid_sig, + ); + + assert_eq!( + result.unwrap_err(), + ContractError::InvalidSignature { + session_id: session.id, + signer: config.signers[0].address.clone().into_string() + } + ); + } } #[test] fn test_add_signature_not_participant() { - let config = setup(); - - let mut session = - SigningSession::new(Uint64::one(), config.key_id.clone(), config.message.clone()); - - let invalid_participant = "not_a_participant".to_string(); - - let key = KEYS.load(&config.store, (&session.key_id).into()).unwrap(); - - let result = session.add_signature( - key, - invalid_participant.clone(), - Signature::try_from((KeyType::Ecdsa, config.signers[0].signature.clone())).unwrap(), - ); - - assert_eq!( - result.unwrap_err(), - ContractError::NotAParticipant { - session_id: session.id, - signer: invalid_participant - } - ); + for config in [ecdsa_setup(), ed25519_setup()] { + let mut session = + SigningSession::new(Uint64::one(), config.key_id.clone(), config.message.clone()); + + let invalid_participant = "not_a_participant".to_string(); + + let key = KEYS.load(&config.store, (&session.key_id).into()).unwrap(); + + let result = session.add_signature( + key, + invalid_participant.clone(), + Signature::try_from((config.key_type, config.signers[0].signature.clone())) + .unwrap(), + ); + + assert_eq!( + result.unwrap_err(), + ContractError::NotAParticipant { + session_id: session.id, + signer: invalid_participant + } + ); + } } } diff --git a/contracts/multisig/src/test/common.rs b/contracts/multisig/src/test/common.rs index 1d46731d0..db5e9b146 100644 --- a/contracts/multisig/src/test/common.rs +++ b/contracts/multisig/src/test/common.rs @@ -16,7 +16,7 @@ pub struct TestSigner { pub signature: HexBinary, } -pub mod test_data { +pub mod ecdsa_test_data { use super::*; pub fn pub_key() -> HexBinary { @@ -55,6 +55,45 @@ pub mod test_data { } } +pub mod ed25519_test_data { + use super::*; + + pub fn pub_key() -> HexBinary { + HexBinary::from_hex("bc5b2bab5f08e332f85085388ff5d4c770ff82ecf7e5e8de0a4515318f7ef7e6") + .unwrap() + } + + pub fn signature() -> HexBinary { + HexBinary::from_hex("e0876240536b548e5258b46126c6e0941e9da7c5ca3349d9e08f8cd4387ea919008766257c1eb72cc6c535ca678b8217076a23ac4e2ca4dee105aaf596bedd01") + .unwrap() + } + + pub fn message() -> HexBinary { + HexBinary::from_hex("fa0609efd1dfeedfdcc8ba51520fae2d5176b7621d2560f071e801b0817e1537") + .unwrap() + } + + pub fn signers() -> Vec { + vec![ + TestSigner { + address: Addr::unchecked("signer1"), + pub_key: pub_key(), + signature: signature(), + }, + TestSigner { + address: Addr::unchecked("signer2"), + pub_key: pub_key(), + signature: signature(), + }, + TestSigner { + address: Addr::unchecked("signer3"), + pub_key: pub_key(), + signature: signature(), + }, + ] + } +} + pub fn build_snapshot(signers: &Vec) -> Snapshot { let mut rng = rand::thread_rng(); @@ -74,13 +113,18 @@ pub fn build_snapshot(signers: &Vec) -> Snapshot { ) } -pub fn build_key(key_id: KeyID, signers: &Vec, snapshot: Snapshot) -> Key { +pub fn build_key( + key_type: KeyType, + key_id: KeyID, + signers: &Vec, + snapshot: Snapshot, +) -> Key { let pub_keys = signers .iter() .map(|signer| { ( signer.address.clone().to_string(), - PublicKey::try_from((KeyType::Ecdsa, signer.pub_key.clone())).unwrap(), + PublicKey::try_from((key_type, signer.pub_key.clone())).unwrap(), ) }) .collect::>(); diff --git a/contracts/multisig/src/types.rs b/contracts/multisig/src/types.rs index 5f110c388..86f251113 100644 --- a/contracts/multisig/src/types.rs +++ b/contracts/multisig/src/types.rs @@ -95,7 +95,7 @@ impl TryFrom for MsgToSign { mod tests { use cosmwasm_std::to_binary; - use crate::test::common::test_data; + use crate::test::common::ecdsa_test_data; use super::*; @@ -113,7 +113,7 @@ mod tests { #[test] fn test_try_from_hexbinary_to_message() { - let hex = test_data::message(); + let hex = ecdsa_test_data::message(); let message = MsgToSign::try_from(hex.clone()).unwrap(); assert_eq!(HexBinary::from(message), hex); } From 9c1d0d4311db62d1892a77391b7bff9062d5d1fb Mon Sep 17 00:00:00 2001 From: Eran Rundstein Date: Tue, 19 Sep 2023 11:39:08 -0700 Subject: [PATCH 2/6] Add ed25519 tests --- contracts/multisig/src/contract.rs | 473 ++++++++++++++++++----------- 1 file changed, 296 insertions(+), 177 deletions(-) diff --git a/contracts/multisig/src/contract.rs b/contracts/multisig/src/contract.rs index e193a1ba8..19cf79277 100644 --- a/contracts/multisig/src/contract.rs +++ b/contracts/multisig/src/contract.rs @@ -291,8 +291,8 @@ mod tests { use crate::{ key::{KeyType, PublicKey, Signature}, msg::Multisig, - test::common::ecdsa_test_data, test::common::{build_snapshot, TestSigner}, + test::common::{ecdsa_test_data, ed25519_test_data}, types::MultisigState, }; @@ -308,6 +308,9 @@ mod tests { const INSTANTIATOR: &str = "inst"; const PROVER: &str = "prover"; + const ECDSA_SUBKEY: &str = "key_ecdsa"; + const ED25519_SUBKEY: &str = "key_ed25519"; + fn do_instantiate(deps: DepsMut) -> Result { let info = mock_info(INSTANTIATOR, &[]); let env = mock_env(); @@ -317,22 +320,30 @@ mod tests { instantiate(deps, env, info, msg) } - fn do_key_gen(deps: DepsMut) -> Result<(Response, Key), axelar_wasm_std::ContractError> { + fn do_key_gen( + key_type: KeyType, + subkey: &str, + deps: DepsMut, + ) -> Result<(Response, Key), axelar_wasm_std::ContractError> { let info = mock_info(PROVER, &[]); let env = mock_env(); - let signers = ecdsa_test_data::signers(); + let signers = match key_type { + KeyType::Ecdsa => ecdsa_test_data::signers(), + KeyType::Ed25519 => ed25519_test_data::signers(), + }; + let pub_keys = signers .iter() .map(|signer| { ( signer.address.clone().to_string(), - (KeyType::Ecdsa, signer.pub_key.clone()), + (key_type, signer.pub_key.clone()), ) }) .collect::>(); - let subkey = "key".to_string(); + let subkey = subkey.to_string(); let snapshot = build_snapshot(&signers); let msg = ExecuteMsg::KeyGen { key_id: subkey.clone(), @@ -358,7 +369,7 @@ mod tests { }) } - fn query_key(deps: Deps) -> StdResult { + fn query_key(subkey: &str, deps: Deps) -> StdResult { let info = mock_info(PROVER, &[]); let env = mock_env(); query( @@ -367,7 +378,7 @@ mod tests { QueryMsg::GetKey { key_id: KeyID { owner: info.sender, - subkey: "key".to_string(), + subkey: subkey.to_string(), }, }, ) @@ -376,13 +387,14 @@ mod tests { fn do_start_signing_session( deps: DepsMut, sender: &str, + key_id: &str, ) -> Result { let info = mock_info(sender, &[]); let env = mock_env(); let message = ecdsa_test_data::message(); let msg = ExecuteMsg::StartSigningSession { - key_id: "key".to_string(), + key_id: key_id.to_string(), msg: message.clone(), }; execute(deps, env, info, msg) @@ -433,13 +445,16 @@ mod tests { fn setup() -> OwnedDeps { let mut deps = mock_dependencies(); do_instantiate(deps.as_mut()).unwrap(); - do_key_gen(deps.as_mut()).unwrap(); + do_key_gen(KeyType::Ecdsa, ECDSA_SUBKEY, deps.as_mut()).unwrap(); + do_key_gen(KeyType::Ed25519, ED25519_SUBKEY, deps.as_mut()).unwrap(); deps } - fn setup_with_session_started() -> OwnedDeps { + fn setup_with_session_started( + key_id: &str, + ) -> OwnedDeps { let mut deps = setup(); - do_start_signing_session(deps.as_mut(), PROVER).unwrap(); + do_start_signing_session(deps.as_mut(), PROVER, key_id).unwrap(); deps } @@ -455,6 +470,24 @@ mod tests { .map(|attribute| attribute.value.as_str()) } + // Returns a list of (key_type, subkey, signers, session_id) + fn signature_test_data() -> Vec<(KeyType, &'static str, Vec, Uint64)> { + vec![ + ( + KeyType::Ecdsa, + ECDSA_SUBKEY, + ecdsa_test_data::signers(), + Uint64::from(1u64), + ), + ( + KeyType::Ed25519, + ED25519_SUBKEY, + ed25519_test_data::signers(), + Uint64::from(2u64), + ), + ] + } + #[test] fn test_instantiation() { let mut deps = mock_dependencies(); @@ -473,70 +506,88 @@ mod tests { let mut deps = mock_dependencies(); do_instantiate(deps.as_mut()).unwrap(); - let res = do_key_gen(deps.as_mut()); + let res = do_key_gen(KeyType::Ecdsa, "key1", deps.as_mut()); assert!(res.is_ok()); - let key = res.unwrap().1; + let key1 = res.unwrap().1; - let res = query_key(deps.as_ref()); + let res = do_key_gen(KeyType::Ed25519, "key2", deps.as_mut()); assert!(res.is_ok()); - assert_eq!(key, from_binary(&res.unwrap()).unwrap()); + let key2 = res.unwrap().1; - let res = do_key_gen(deps.as_mut()); - assert_eq!( - res.unwrap_err().to_string(), - axelar_wasm_std::ContractError::from(ContractError::DuplicateKeyID { - key_id: KeyID { - owner: Addr::unchecked(PROVER), - subkey: "key".to_string(), - } + let res = query_key("key1", deps.as_ref()); + assert!(res.is_ok()); + assert_eq!(key1, from_binary(&res.unwrap()).unwrap()); + + let res = query_key("key2", deps.as_ref()); + assert!(res.is_ok()); + assert_eq!(key2, from_binary(&res.unwrap()).unwrap()); + + for key_type in [KeyType::Ecdsa, KeyType::Ed25519] { + let res = do_key_gen(key_type, "key1", deps.as_mut()); + assert_eq!( + res.unwrap_err().to_string(), + axelar_wasm_std::ContractError::from(ContractError::DuplicateKeyID { + key_id: KeyID { + owner: Addr::unchecked(PROVER), + subkey: "key1".to_string(), + } + .to_string() + }) .to_string() - }) - .to_string() - ); + ); + } } #[test] fn test_start_signing_session() { let mut deps = setup(); - let res = do_start_signing_session(deps.as_mut(), PROVER); - - assert!(res.is_ok()); - - let session = SIGNING_SESSIONS.load(deps.as_ref().storage, 1u64).unwrap(); - - let key_id: KeyID = KeyID { - owner: Addr::unchecked(PROVER), - subkey: "key".to_string(), - }; - let key = get_key(deps.as_ref().storage, &key_id).unwrap(); - let message = ecdsa_test_data::message(); - - assert_eq!(session.id, Uint64::one()); - assert_eq!(session.key_id, key.id); - assert_eq!(session.msg, message.clone().try_into().unwrap()); - assert!(session.signatures.is_empty()); - assert_eq!(session.state, MultisigState::Pending); + for (i, subkey) in [ECDSA_SUBKEY, ED25519_SUBKEY].into_iter().enumerate() { + let res = do_start_signing_session(deps.as_mut(), PROVER, subkey); - let res = res.unwrap(); - assert_eq!(res.data, Some(to_binary(&session.id).unwrap())); - assert_eq!(res.events.len(), 1); + assert!(res.is_ok()); - let event = res.events.get(0).unwrap(); - assert_eq!(event.ty, "signing_started".to_string()); - assert_eq!( - get_event_attribute(event, "session_id").unwrap(), - session.id.to_string() - ); - assert_eq!( - get_event_attribute(event, "key_id").unwrap(), - to_string(&session.key_id).unwrap() - ); - assert_eq!( - key.pub_keys, - from_str(get_event_attribute(event, "pub_keys").unwrap()).unwrap() - ); - assert_eq!(get_event_attribute(event, "msg").unwrap(), message.to_hex()); + let session = SIGNING_SESSIONS + .load(deps.as_ref().storage, i as u64 + 1) + .unwrap(); + + let key_id: KeyID = KeyID { + owner: Addr::unchecked(PROVER), + subkey: subkey.to_string(), + }; + let key = get_key(deps.as_ref().storage, &key_id).unwrap(); + let message = match subkey { + ECDSA_SUBKEY => ecdsa_test_data::message(), + ED25519_SUBKEY => ed25519_test_data::message(), + _ => panic!("unexpected subkey"), + }; + + assert_eq!(session.id, Uint64::from(i as u64 + 1)); + assert_eq!(session.key_id, key.id); + assert_eq!(session.msg, message.clone().try_into().unwrap()); + assert!(session.signatures.is_empty()); + assert_eq!(session.state, MultisigState::Pending); + + let res = res.unwrap(); + assert_eq!(res.data, Some(to_binary(&session.id).unwrap())); + assert_eq!(res.events.len(), 1); + + let event = res.events.get(0).unwrap(); + assert_eq!(event.ty, "signing_started".to_string()); + assert_eq!( + get_event_attribute(event, "session_id").unwrap(), + session.id.to_string() + ); + assert_eq!( + get_event_attribute(event, "key_id").unwrap(), + to_string(&session.key_id).unwrap() + ); + assert_eq!( + key.pub_keys, + from_str(get_event_attribute(event, "pub_keys").unwrap()).unwrap() + ); + assert_eq!(get_event_attribute(event, "msg").unwrap(), message.to_hex()); + } } #[test] @@ -544,106 +595,115 @@ mod tests { let mut deps = setup(); let sender = "someone else"; - let res = do_start_signing_session(deps.as_mut(), sender); - assert_eq!( - res.unwrap_err().to_string(), - axelar_wasm_std::ContractError::from(ContractError::NoActiveKeyFound { - key_id: KeyID { - owner: Addr::unchecked(sender), - subkey: "key".to_string(), - } + for key_id in [ECDSA_SUBKEY, ED25519_SUBKEY] { + let res = do_start_signing_session(deps.as_mut(), sender, key_id); + + assert_eq!( + res.unwrap_err().to_string(), + axelar_wasm_std::ContractError::from(ContractError::NoActiveKeyFound { + key_id: KeyID { + owner: Addr::unchecked(sender), + subkey: key_id.to_string(), + } + .to_string() + }) .to_string() - }) - .to_string() - ); + ); + } } #[test] fn test_submit_signature() { - let mut deps = setup_with_session_started(); - - let signers = ecdsa_test_data::signers(); + let mut deps = setup(); - let session_id = Uint64::one(); - let signer = signers.get(0).unwrap().to_owned(); - let res = do_sign(deps.as_mut(), session_id, &signer); + for (key_type, subkey, signers, session_id) in signature_test_data() { + do_start_signing_session(deps.as_mut(), PROVER, subkey).unwrap(); - assert!(res.is_ok()); + let signer = signers.get(0).unwrap().to_owned(); + let res = do_sign(deps.as_mut(), Uint64::from(session_id), &signer); - let session = SIGNING_SESSIONS.load(deps.as_ref().storage, 1u64).unwrap(); + assert!(res.is_ok()); - assert_eq!(session.signatures.len(), 1); - assert_eq!( - session - .signatures - .get(&signer.address.clone().into_string()) - .unwrap(), - &Signature::try_from((KeyType::Ecdsa, signer.signature.clone())).unwrap() - ); - assert_eq!(session.state, MultisigState::Pending); + let session = SIGNING_SESSIONS + .load(deps.as_ref().storage, session_id.into()) + .unwrap(); + + assert_eq!(session.signatures.len(), 1); + assert_eq!( + session + .signatures + .get(&signer.address.clone().into_string()) + .unwrap(), + &Signature::try_from((key_type, signer.signature.clone())).unwrap() + ); + assert_eq!(session.state, MultisigState::Pending); - let res = res.unwrap(); - assert_eq!(res.events.len(), 1); + let res = res.unwrap(); + assert_eq!(res.events.len(), 1); - let event = res.events.get(0).unwrap(); - assert_eq!(event.ty, "signature_submitted".to_string()); - assert_eq!( - get_event_attribute(event, "session_id").unwrap(), - session_id.to_string() - ); - assert_eq!( - get_event_attribute(event, "participant").unwrap(), - signer.address.into_string() - ); - assert_eq!( - get_event_attribute(event, "signature").unwrap(), - signer.signature.to_hex() - ); + let event = res.events.get(0).unwrap(); + assert_eq!(event.ty, "signature_submitted".to_string()); + assert_eq!( + get_event_attribute(event, "session_id").unwrap(), + session_id.to_string() + ); + assert_eq!( + get_event_attribute(event, "participant").unwrap(), + signer.address.into_string() + ); + assert_eq!( + get_event_attribute(event, "signature").unwrap(), + signer.signature.to_hex() + ); + } } #[test] fn test_submit_signature_completed() { - let mut deps = setup_with_session_started(); - - let signers = ecdsa_test_data::signers(); + let mut deps = setup(); - let session_id = Uint64::one(); - let signer = signers.get(0).unwrap().to_owned(); - do_sign(deps.as_mut(), session_id, &signer).unwrap(); + for (key_type, subkey, signers, session_id) in signature_test_data() { + do_start_signing_session(deps.as_mut(), PROVER, subkey).unwrap(); - // second signature - let signer = signers.get(1).unwrap().to_owned(); - let res = do_sign(deps.as_mut(), session_id, &signer); + let signer = signers.get(0).unwrap().to_owned(); + do_sign(deps.as_mut(), session_id, &signer).unwrap(); - assert!(res.is_ok()); + // second signature + let signer = signers.get(1).unwrap().to_owned(); + let res = do_sign(deps.as_mut(), session_id, &signer); - let session = SIGNING_SESSIONS.load(deps.as_ref().storage, 1u64).unwrap(); + assert!(res.is_ok()); - assert_eq!(session.signatures.len(), 2); - assert_eq!( - session - .signatures - .get(&signer.address.into_string()) - .unwrap(), - &Signature::try_from((KeyType::Ecdsa, signer.signature)).unwrap() - ); - assert_eq!(session.state, MultisigState::Completed); + let session = SIGNING_SESSIONS + .load(deps.as_ref().storage, session_id.into()) + .unwrap(); + + assert_eq!(session.signatures.len(), 2); + assert_eq!( + session + .signatures + .get(&signer.address.into_string()) + .unwrap(), + &Signature::try_from((key_type, signer.signature)).unwrap() + ); + assert_eq!(session.state, MultisigState::Completed); - let res = res.unwrap(); - assert_eq!(res.events.len(), 2); + let res = res.unwrap(); + assert_eq!(res.events.len(), 2); - let event = res.events.get(1).unwrap(); - assert_eq!(event.ty, "signing_completed".to_string()); - assert_eq!( - get_event_attribute(event, "session_id").unwrap(), - session_id.to_string() - ); + let event = res.events.get(1).unwrap(); + assert_eq!(event.ty, "signing_completed".to_string()); + assert_eq!( + get_event_attribute(event, "session_id").unwrap(), + session_id.to_string() + ); + } } #[test] fn test_submit_signature_wrong_session_id() { - let mut deps = setup_with_session_started(); + let mut deps = setup_with_session_started(ECDSA_SUBKEY); let invalid_session_id = Uint64::zero(); let signer = ecdsa_test_data::signers().get(0).unwrap().to_owned(); @@ -660,51 +720,59 @@ mod tests { #[test] fn test_query_signing_session() { - let mut deps = setup_with_session_started(); + let mut deps = setup(); - let session_id = Uint64::one(); - let signer = ecdsa_test_data::signers().get(0).unwrap().to_owned(); - do_sign(deps.as_mut(), session_id, &signer).unwrap(); + for (_key_type, subkey, signers, session_id) in signature_test_data() { + do_start_signing_session(deps.as_mut(), PROVER, subkey).unwrap(); - let msg = QueryMsg::GetMultisig { session_id }; + let signer = signers.get(0).unwrap().to_owned(); + do_sign(deps.as_mut(), session_id, &signer).unwrap(); - let res = query(deps.as_ref(), mock_env(), msg); - assert!(res.is_ok()); + let msg = QueryMsg::GetMultisig { session_id }; - let query_res: Multisig = from_binary(&res.unwrap()).unwrap(); - let session = SIGNING_SESSIONS.load(deps.as_ref().storage, 1u64).unwrap(); - let key = KEYS - .load(deps.as_ref().storage, (&session.key_id).into()) - .unwrap(); + let res = query(deps.as_ref(), mock_env(), msg); + assert!(res.is_ok()); - assert_eq!(query_res.state, session.state); - assert_eq!(query_res.signers.len(), key.snapshot.participants.len()); - key.snapshot - .participants - .iter() - .for_each(|(address, participant)| { - let signer = query_res - .signers - .iter() - .find(|signer| signer.0.address == participant.address) - .unwrap(); - - assert_eq!(signer.0.weight, Uint256::from(participant.weight)); - assert_eq!(signer.0.pub_key, key.pub_keys.get(address).unwrap().clone()); - assert_eq!(signer.1, session.signatures.get(address).cloned()); - }); + let query_res: Multisig = from_binary(&res.unwrap()).unwrap(); + let session = SIGNING_SESSIONS + .load(deps.as_ref().storage, session_id.into()) + .unwrap(); + let key = KEYS + .load(deps.as_ref().storage, (&session.key_id).into()) + .unwrap(); + + assert_eq!(query_res.state, session.state); + assert_eq!(query_res.signers.len(), key.snapshot.participants.len()); + key.snapshot + .participants + .iter() + .for_each(|(address, participant)| { + let signer = query_res + .signers + .iter() + .find(|signer| signer.0.address == participant.address) + .unwrap(); + + assert_eq!(signer.0.weight, Uint256::from(participant.weight)); + assert_eq!(signer.0.pub_key, key.pub_keys.get(address).unwrap().clone()); + assert_eq!(signer.1, session.signatures.get(address).cloned()); + }); + } } + #[test] fn test_register_key() { let mut deps = mock_dependencies(); do_instantiate(deps.as_mut()).unwrap(); - let signers = ecdsa_test_data::signers(); - let pub_keys = signers + + // Register an ECDSA key + let ecdsa_signers = ecdsa_test_data::signers(); + let ecdsa_pub_keys = ecdsa_signers .iter() .map(|signer| (signer.address.clone(), signer.pub_key.clone())) .collect::>(); - for (addr, pub_key) in &pub_keys { + for (addr, pub_key) in &ecdsa_pub_keys { let res = do_register_key( deps.as_mut(), addr.clone(), @@ -712,20 +780,43 @@ mod tests { ); assert!(res.is_ok()); } - let mut ret_pub_keys: Vec = vec![]; - for (addr, _) in &pub_keys { - let res = query_registered_public_key(deps.as_ref(), addr.clone(), KeyType::Ecdsa); + // Register an ED25519 key + let ed25519_signers = ed25519_test_data::signers(); + let ed25519_pub_keys = ed25519_signers + .iter() + .map(|signer| (signer.address.clone(), signer.pub_key.clone())) + .collect::>(); + + for (addr, pub_key) in &ed25519_pub_keys { + let res = do_register_key( + deps.as_mut(), + addr.clone(), + PublicKey::Ed25519(pub_key.clone()), + ); assert!(res.is_ok()); - ret_pub_keys.push(from_binary(&res.unwrap()).unwrap()); } - assert_eq!( - pub_keys - .into_iter() - .map(|(_, pk)| PublicKey::try_from((KeyType::Ecdsa, pk)).unwrap()) - .collect::>(), - ret_pub_keys - ); + + // Test that we can query both keys + for (key_type, expected_pub_keys) in [ + (KeyType::Ecdsa, ecdsa_pub_keys), + (KeyType::Ed25519, ed25519_pub_keys), + ] { + let mut ret_pub_keys: Vec = vec![]; + + for (addr, _) in &expected_pub_keys { + let res = query_registered_public_key(deps.as_ref(), addr.clone(), key_type); + assert!(res.is_ok()); + ret_pub_keys.push(from_binary(&res.unwrap()).unwrap()); + } + assert_eq!( + expected_pub_keys + .into_iter() + .map(|(_, pk)| PublicKey::try_from((key_type, pk)).unwrap()) + .collect::>(), + ret_pub_keys + ); + } } #[test] @@ -747,6 +838,7 @@ mod tests { assert!(res.is_ok()); } + // Update ECDSA key let new_pub_key = HexBinary::from_hex( "021a381b3e07347d3a05495347e1fb2fe04764afcea5a74084fa957947b59f9026", ) @@ -758,6 +850,33 @@ mod tests { ); assert!(res.is_ok()); + let res = query_registered_public_key(deps.as_ref(), pub_keys[0].0.clone(), KeyType::Ecdsa); + assert!(res.is_ok()); + assert_eq!( + PublicKey::try_from((KeyType::Ecdsa, new_pub_key.clone())).unwrap(), + from_binary::(&res.unwrap()).unwrap() + ); + + // Register an ED25519 key, it should not affect our ECDSA key + let ed25519_pub_key = + HexBinary::from_hex("13606a37daa030d02a72986dc01e45904678c8001429cd34514e69e2d054636a") + .unwrap(); + + let res = do_register_key( + deps.as_mut(), + pub_keys[0].0.clone(), + PublicKey::Ed25519(ed25519_pub_key.clone()), + ); + assert!(res.is_ok()); + + let res = + query_registered_public_key(deps.as_ref(), pub_keys[0].0.clone(), KeyType::Ed25519); + assert!(res.is_ok()); + assert_eq!( + PublicKey::try_from((KeyType::Ed25519, ed25519_pub_key)).unwrap(), + from_binary::(&res.unwrap()).unwrap() + ); + let res = query_registered_public_key(deps.as_ref(), pub_keys[0].0.clone(), KeyType::Ecdsa); assert!(res.is_ok()); assert_eq!( From 37ea2104b335929f2ee1f39d087e6a7e27b99459 Mon Sep 17 00:00:00 2001 From: Eran Rundstein Date: Tue, 19 Sep 2023 12:17:56 -0700 Subject: [PATCH 3/6] fmt --- ampd/src/main.rs | 4 ++-- contracts/multisig/src/ed25519.rs | 1 - contracts/multisig/src/key.rs | 18 ++++++++++++------ 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/ampd/src/main.rs b/ampd/src/main.rs index e9aa9790d..ee13c5fc3 100644 --- a/ampd/src/main.rs +++ b/ampd/src/main.rs @@ -136,8 +136,8 @@ fn parse_config( fn expand_home_dir(path: impl AsRef) -> PathBuf { let path = path.as_ref(); - let Ok(home_subfolder) = path.strip_prefix("~") else{ - return path.to_path_buf() + let Ok(home_subfolder) = path.strip_prefix("~") else { + return path.to_path_buf(); }; dirs::home_dir().map_or(path.to_path_buf(), |home| home.join(home_subfolder)) diff --git a/contracts/multisig/src/ed25519.rs b/contracts/multisig/src/ed25519.rs index fb9b02935..18bab00af 100644 --- a/contracts/multisig/src/ed25519.rs +++ b/contracts/multisig/src/ed25519.rs @@ -1,4 +1,3 @@ - // TODO: Logic specific to secp256k1 will most likely be handled by core in the future. use crate::ContractError; diff --git a/contracts/multisig/src/key.rs b/contracts/multisig/src/key.rs index 4616962a3..42866b6ff 100644 --- a/contracts/multisig/src/key.rs +++ b/contracts/multisig/src/key.rs @@ -304,7 +304,8 @@ mod ecdsa_tests { #[test] fn test_verify_signature() { - let signature = Signature::try_from((KeyType::Ecdsa, ecdsa_test_data::signature())).unwrap(); + let signature = + Signature::try_from((KeyType::Ecdsa, ecdsa_test_data::signature())).unwrap(); let message = MsgToSign::try_from(ecdsa_test_data::message()).unwrap(); let public_key = PublicKey::try_from((KeyType::Ecdsa, ecdsa_test_data::pub_key())).unwrap(); let result = signature.verify(&message, &public_key).unwrap(); @@ -332,7 +333,8 @@ mod ecdsa_tests { ) .unwrap(); - let signature = Signature::try_from((KeyType::Ecdsa, ecdsa_test_data::signature())).unwrap(); + let signature = + Signature::try_from((KeyType::Ecdsa, ecdsa_test_data::signature())).unwrap(); let message = MsgToSign::try_from(ecdsa_test_data::message()).unwrap(); let public_key = PublicKey::try_from((KeyType::Ecdsa, invalid_pub_key)).unwrap(); let result = signature.verify(&message, &public_key).unwrap(); @@ -412,9 +414,11 @@ mod ed25519_tests { #[test] fn test_verify_signature() { - let signature = Signature::try_from((KeyType::Ed25519, ed25519_test_data::signature())).unwrap(); + let signature = + Signature::try_from((KeyType::Ed25519, ed25519_test_data::signature())).unwrap(); let message = MsgToSign::try_from(ed25519_test_data::message()).unwrap(); - let public_key = PublicKey::try_from((KeyType::Ed25519, ed25519_test_data::pub_key())).unwrap(); + let public_key = + PublicKey::try_from((KeyType::Ed25519, ed25519_test_data::pub_key())).unwrap(); let result = signature.verify(&message, &public_key).unwrap(); assert_eq!(result, true); } @@ -428,7 +432,8 @@ mod ed25519_tests { let signature = Signature::try_from((KeyType::Ed25519, invalid_signature)).unwrap(); let message = MsgToSign::try_from(ed25519_test_data::message()).unwrap(); - let public_key = PublicKey::try_from((KeyType::Ed25519, ed25519_test_data::pub_key())).unwrap(); + let public_key = + PublicKey::try_from((KeyType::Ed25519, ed25519_test_data::pub_key())).unwrap(); let result = signature.verify(&message, &public_key).unwrap(); assert_eq!(result, false); } @@ -439,7 +444,8 @@ mod ed25519_tests { HexBinary::from_hex("ffff8a3c50c8381541b682f4941ef7df351376f60e3fa0296f48f0f767a4f321") .unwrap(); - let signature = Signature::try_from((KeyType::Ed25519, ed25519_test_data::signature())).unwrap(); + let signature = + Signature::try_from((KeyType::Ed25519, ed25519_test_data::signature())).unwrap(); let message = MsgToSign::try_from(ed25519_test_data::message()).unwrap(); let public_key = PublicKey::try_from((KeyType::Ed25519, invalid_pub_key)).unwrap(); let result = signature.verify(&message, &public_key).unwrap(); From 6e65b71c65bf2ff6e24cee0008931ec4c6e62131 Mon Sep 17 00:00:00 2001 From: Eran Rundstein Date: Tue, 19 Sep 2023 12:20:04 -0700 Subject: [PATCH 4/6] cargo sort --- contracts/multisig/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/multisig/Cargo.toml b/contracts/multisig/Cargo.toml index 1cd348b59..e66e3cffc 100644 --- a/contracts/multisig/Cargo.toml +++ b/contracts/multisig/Cargo.toml @@ -25,7 +25,6 @@ secp256k1 = [] # use this feature to enable ed25519 for signature verification ed25519 = [] - [package.metadata.scripts] optimize = """docker run --rm -v "$(pwd)":/code \ --mount type=volume,source="$(basename "$(pwd)")_cache",target=/code/target \ From 1aa7a511de561debac47c047e0b75ae71e7b2607 Mon Sep 17 00:00:00 2001 From: Eran Rundstein Date: Thu, 21 Sep 2023 16:05:10 -0700 Subject: [PATCH 5/6] simplify match statement --- contracts/multisig/src/key.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/contracts/multisig/src/key.rs b/contracts/multisig/src/key.rs index 42866b6ff..8db061078 100644 --- a/contracts/multisig/src/key.rs +++ b/contracts/multisig/src/key.rs @@ -84,14 +84,9 @@ impl Signature { return Err(ContractError::KeyTypeMismatch); } - match (self, pub_key) { - (Signature::Ecdsa(sig), PublicKey::Ecdsa(pub_key)) => { - ecdsa_verify(msg.into(), sig.as_ref(), pub_key.as_ref()) - } - (Signature::Ed25519(sig), PublicKey::Ed25519(pub_key)) => { - ed25519_verify(msg.into(), sig.as_ref(), pub_key.as_ref()) - } - _ => Err(ContractError::KeyTypeMismatch), + match self { + Signature::Ecdsa(sig) => ecdsa_verify(msg.into(), sig.as_ref(), pub_key.as_ref()), + Signature::Ed25519(sig) => ed25519_verify(msg.into(), sig.as_ref(), pub_key.as_ref()), } } } From c0e4975d0802574fdb92c50ac52b39be26e41703 Mon Sep 17 00:00:00 2001 From: Eran Rundstein Date: Fri, 22 Sep 2023 13:31:29 -0700 Subject: [PATCH 6/6] code review fixes --- contracts/multisig/src/ed25519.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/contracts/multisig/src/ed25519.rs b/contracts/multisig/src/ed25519.rs index 18bab00af..a4b668ce5 100644 --- a/contracts/multisig/src/ed25519.rs +++ b/contracts/multisig/src/ed25519.rs @@ -1,12 +1,11 @@ -// TODO: Logic specific to secp256k1 will most likely be handled by core in the future. use crate::ContractError; -const ECDSA_SIGNATURE_LEN: usize = 64; +const ED25519_SIGNATURE_LEN: usize = 64; pub fn ed25519_verify(msg_hash: &[u8], sig: &[u8], pub_key: &[u8]) -> Result { - cosmwasm_crypto::ed25519_verify(msg_hash, &sig[0..ECDSA_SIGNATURE_LEN], pub_key).map_err(|e| { - ContractError::SignatureVerificationFailed { + cosmwasm_crypto::ed25519_verify(msg_hash, &sig[0..ED25519_SIGNATURE_LEN], pub_key).map_err( + |e| ContractError::SignatureVerificationFailed { reason: e.to_string(), - } - }) + }, + ) }