From 990e0c02aa9ecb784185f935e7cdc228abc7fc52 Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Fri, 18 Aug 2023 16:21:42 -0400 Subject: [PATCH 01/14] feat(multisig): public key registration * Add a method to register public keys * Add a query to fetch public keys * Distinguish keys and signatures by type --- ampd/src/handlers/multisig.rs | 34 ++- contracts/multisig-prover/src/encoding/evm.rs | 25 +-- .../multisig-prover/src/test/test_data.rs | 6 +- contracts/multisig-prover/src/types.rs | 4 +- contracts/multisig/src/contract.rs | 198 ++++++++++++++++-- contracts/multisig/src/events.rs | 24 ++- contracts/multisig/src/msg.rs | 14 +- contracts/multisig/src/secp256k1.rs | 66 +++--- contracts/multisig/src/signing.rs | 15 +- contracts/multisig/src/state.rs | 7 +- contracts/multisig/src/test/common.rs | 4 +- contracts/multisig/src/types.rs | 116 ++++++++-- 12 files changed, 406 insertions(+), 107 deletions(-) diff --git a/ampd/src/handlers/multisig.rs b/ampd/src/handlers/multisig.rs index 8eca2c251..eab98acc8 100644 --- a/ampd/src/handlers/multisig.rs +++ b/ampd/src/handlers/multisig.rs @@ -2,7 +2,6 @@ use std::collections::HashMap; use std::convert::TryFrom; use ecdsa::VerifyingKey; -use hex::FromHex; use multisig::types::KeyID; use serde::de::value::MapDeserializer; use serde::de::Error as DeserializeError; @@ -35,21 +34,18 @@ fn deserialize_public_keys<'de, D>( where D: Deserializer<'de>, { - let keys_by_address: HashMap = HashMap::deserialize(deserializer)?; + let keys_by_address: HashMap = + HashMap::deserialize(deserializer)?; keys_by_address .into_iter() - .map(|(address, hex)| { - Ok(( + .map(|(address, pk)| match pk { + multisig::types::PublicKey::ECDSA(hex) => Ok(( address, - VerifyingKey::from_sec1_bytes( - >::from_hex(hex) - .map_err(D::Error::custom)? - .as_slice(), - ) - .map_err(D::Error::custom)? - .into(), - )) + VerifyingKey::from_sec1_bytes((&hex).into()) + .map_err(D::Error::custom)? + .into(), + )), }) .collect() } @@ -81,7 +77,7 @@ mod test { use cosmwasm_std::{Addr, HexBinary, Uint64}; use multisig::events::Event::SigningStarted; - use multisig::types::{MsgToSign, PublicKey}; + use multisig::types::{ECDSAPublicKey, MsgToSign}; use tendermint::abci; use crate::broadcaster::key::ECDSASigningKey; @@ -92,10 +88,10 @@ mod test { ECDSASigningKey::random().address().to_string() } - fn rand_public_key() -> PublicKey { - PublicKey::unchecked(HexBinary::from( + fn rand_public_key() -> multisig::types::PublicKey { + multisig::types::PublicKey::ECDSA(ECDSAPublicKey::unchecked(HexBinary::from( ECDSASigningKey::random().public_key().to_bytes(), - )) + ))) } fn rand_message() -> HexBinary { @@ -106,7 +102,7 @@ mod test { fn signing_started_event() -> event_sub::Event { let pub_keys = (0..10) .map(|_| (rand_account(), rand_public_key())) - .collect::>(); + .collect::>(); let poll_started = SigningStarted { session_id: Uint64::one(), @@ -160,10 +156,10 @@ mod test { let mut event = signing_started_event(); let invalid_pub_key: [u8; 32] = rand::random(); - let mut map: HashMap = HashMap::new(); + let mut map: HashMap = HashMap::new(); map.insert( rand_account(), - PublicKey::unchecked(HexBinary::from(invalid_pub_key.as_slice())), + ECDSAPublicKey::unchecked(HexBinary::from(invalid_pub_key.as_slice())), ); match event { event_sub::Event::Abci { diff --git a/contracts/multisig-prover/src/encoding/evm.rs b/contracts/multisig-prover/src/encoding/evm.rs index 38a7479d4..31a00b210 100644 --- a/contracts/multisig-prover/src/encoding/evm.rs +++ b/contracts/multisig-prover/src/encoding/evm.rs @@ -7,7 +7,7 @@ use k256::{elliptic_curve::sec1::ToEncodedPoint, PublicKey}; use sha3::{Digest, Keccak256}; use connection_router::msg::Message; -use multisig::msg::Signer; +use multisig::{msg::Signer, types::Signature}; use crate::{ error::ContractError, @@ -51,7 +51,7 @@ impl TryFrom for Operator { Ok(Self { address: evm_address((&signer.pub_key).into())?, weight: signer.weight, - signature: signer.signature, + signature: signer.signature.map(|Signature::ECDSA(sig)| sig), }) } } @@ -234,6 +234,7 @@ fn batch_id(message_ids: &[String]) -> HexBinary { #[cfg(test)] mod test { use ethabi::ParamType; + use multisig::types::PublicKey; use crate::test::test_data; @@ -414,8 +415,8 @@ mod test { .map(|op| Signer { address: op.address, weight: op.weight.into(), - pub_key: op.pub_key, - signature: op.signature, + pub_key: PublicKey::ECDSA(op.pub_key), + signature: op.signature.map(Signature::ECDSA), }) .collect::>(); @@ -488,8 +489,8 @@ mod test { .map(|op| Signer { address: op.address, weight: op.weight.into(), - pub_key: op.pub_key, - signature: op.signature, + pub_key: PublicKey::ECDSA(op.pub_key), + signature: op.signature.map(Signature::ECDSA), }) .collect::>(); @@ -558,20 +559,20 @@ mod test { Signer { address: operator2.address, weight: operator2.weight, - pub_key: operator2.pub_key, - signature: operator2.signature, + pub_key: PublicKey::ECDSA(operator2.pub_key), + signature: operator2.signature.map(Signature::ECDSA), }, Signer { address: operator1.address, weight: operator1.weight, - pub_key: operator1.pub_key, - signature: operator1.signature, + pub_key: PublicKey::ECDSA(operator1.pub_key), + signature: operator1.signature.map(Signature::ECDSA), }, Signer { address: operator3.address, weight: operator3.weight, - pub_key: operator3.pub_key, - signature: operator3.signature, + pub_key: PublicKey::ECDSA(operator3.pub_key), + signature: operator3.signature.map(Signature::ECDSA), }, ]; diff --git a/contracts/multisig-prover/src/test/test_data.rs b/contracts/multisig-prover/src/test/test_data.rs index 16fcb8b90..3da62a4f3 100644 --- a/contracts/multisig-prover/src/test/test_data.rs +++ b/contracts/multisig-prover/src/test/test_data.rs @@ -3,7 +3,7 @@ use connection_router::msg::Message; use cosmwasm_std::{Addr, HexBinary, Uint256, Uint64}; -use multisig::types::Signature; +use multisig::types::ECDSASignature; fn legacy_cmd_id_input( source_transaction: HexBinary, @@ -104,10 +104,10 @@ pub fn execute_data() -> HexBinary { #[derive(Debug)] pub struct TestOperator { pub address: Addr, - pub pub_key: multisig::types::PublicKey, + pub pub_key: multisig::types::ECDSAPublicKey, pub operator: HexBinary, pub weight: Uint256, - pub signature: Option, + pub signature: Option, } pub fn operators() -> Vec { diff --git a/contracts/multisig-prover/src/types.rs b/contracts/multisig-prover/src/types.rs index 200c72e97..d6deab99f 100644 --- a/contracts/multisig-prover/src/types.rs +++ b/contracts/multisig-prover/src/types.rs @@ -2,7 +2,7 @@ use std::fmt::Display; use cosmwasm_schema::cw_serde; use cosmwasm_std::{HexBinary, Uint256}; -use multisig::types::Signature; +use multisig::types::ECDSASignature; use crate::encoding::Data; @@ -38,5 +38,5 @@ pub struct CommandBatch { pub struct Operator { pub address: HexBinary, pub weight: Uint256, - pub signature: Option, + pub signature: Option, } diff --git a/contracts/multisig/src/contract.rs b/contracts/multisig/src/contract.rs index 04b1f1e19..393064dc3 100644 --- a/contracts/multisig/src/contract.rs +++ b/contracts/multisig/src/contract.rs @@ -1,7 +1,8 @@ #[cfg(not(feature = "library"))] use cosmwasm_std::entry_point; use cosmwasm_std::{ - to_binary, Binary, Deps, DepsMut, Env, HexBinary, MessageInfo, Response, StdResult, Uint64, + to_binary, Addr, Binary, Deps, DepsMut, Env, HexBinary, MessageInfo, Response, StdResult, + Uint64, }; use axelar_wasm_std::Snapshot; @@ -11,7 +12,7 @@ use crate::{ events::Event, msg::{ExecuteMsg, InstantiateMsg, Multisig, QueryMsg}, state::{get_key, KEYS, SIGNING_SESSIONS, SIGNING_SESSION_COUNTER}, - types::{Key, KeyID, MsgToSign, MultisigState, PublicKey, Signature}, + types::{Key, KeyID, MsgToSign, MultisigState}, ContractError, }; @@ -41,17 +42,25 @@ pub fn execute( ExecuteMsg::SubmitSignature { session_id, signature, - } => execute::submit_signature(deps, info, session_id, signature.try_into()?), + } => execute::submit_signature(deps, info, session_id, signature), ExecuteMsg::KeyGen { key_id, snapshot, pub_keys, } => execute::key_gen(deps, info, key_id, snapshot, pub_keys), + ExecuteMsg::RegisterPublicKey { + public_key, + key_type, + } => execute::register_pub_key(deps, info, public_key, key_type), } } pub mod execute { - use crate::signing::SigningSession; + use crate::{ + signing::SigningSession, + state::PUB_KEYS, + types::{KeyType, PublicKey, Signature}, + }; use super::*; @@ -93,13 +102,26 @@ pub mod execute { deps: DepsMut, info: MessageInfo, session_id: Uint64, - signature: Signature, + signature: HexBinary, ) -> Result { let mut session = SIGNING_SESSIONS .load(deps.storage, session_id.into()) .map_err(|_| ContractError::SigningSessionNotFound { session_id })?; let key = KEYS.load(deps.storage, &session.key_id)?; + let signature: Signature = match key + .pub_keys + .iter() + .find(|pk| pk.0 == &info.sender.to_string()) + { + None => { + return Err(ContractError::NotAParticipant { + session_id, + signer: info.sender.into(), + }) + } + Some((_, pk)) => (pk.clone(), signature).try_into()?, + }; session.add_signature(key, info.sender.clone().into(), signature.clone())?; @@ -125,7 +147,7 @@ pub mod execute { info: MessageInfo, key_id: String, snapshot: Snapshot, - pub_keys: HashMap, + pub_keys: HashMap, ) -> Result { for participant in snapshot.participants.keys() { if !pub_keys.contains_key(participant) { @@ -144,7 +166,12 @@ pub mod execute { snapshot, pub_keys: pub_keys .into_iter() - .map(|(k, v)| (k, PublicKey::try_from(v).unwrap())) + .map(|(k, v)| { + ( + k, + PublicKey::try_from(v).expect("failed to decode public key"), + ) + }) .collect(), }; @@ -157,6 +184,29 @@ pub mod execute { Ok(Response::default()) } + + pub fn register_pub_key( + deps: DepsMut, + info: MessageInfo, + public_key: HexBinary, + key_type: KeyType, + ) -> Result { + let pub_key: PublicKey = (key_type.clone(), public_key).try_into()?; + PUB_KEYS.save( + deps.storage, + (info.sender.clone(), key_type.clone()), + &pub_key.clone(), + )?; + + Ok(Response::new().add_event( + Event::PublicKeyRegistered { + worker: info.sender, + public_key: pub_key, + key_type, + } + .into(), + )) + } } #[cfg_attr(not(feature = "library"), entry_point)] @@ -164,11 +214,22 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { match msg { QueryMsg::GetMultisig { session_id } => to_binary(&query::get_multisig(deps, session_id)?), QueryMsg::GetKey { key_id } => to_binary(&query::get_key(deps, key_id)?), + QueryMsg::GetPublicKeys { + worker_addresses, + key_type, + } => to_binary(&query::get_keys( + deps, + worker_addresses + .iter() + .map(|addr| deps.api.addr_validate(addr)) + .collect::, _>>()?, + key_type, + )?), } } pub mod query { - use crate::msg::Signer; + use crate::{msg::Signer, state::PUB_KEYS, types::KeyType, types::PublicKey}; use super::*; @@ -206,15 +267,28 @@ pub mod query { pub fn get_key(deps: Deps, key_id: KeyID) -> StdResult { KEYS.load(deps.storage, &key_id) } + + pub fn get_keys( + deps: Deps, + workers: Vec, + key_type: KeyType, + ) -> StdResult> { + workers + .into_iter() + .map(|worker| PUB_KEYS.load(deps.storage, (worker, key_type.clone()))) + .collect::, _>>() + } } #[cfg(test)] mod tests { + use std::vec; + use crate::{ msg::Multisig, test::common::test_data, test::common::{build_snapshot, TestSigner}, - types::MultisigState, + types::{ECDSASignature, KeyType, MultisigState, PublicKey, Signature}, }; use super::*; @@ -245,8 +319,13 @@ mod tests { let signers = test_data::signers(); let pub_keys = signers .iter() - .map(|signer| (signer.address.clone().to_string(), signer.pub_key.clone())) - .collect::>(); + .map(|signer| { + ( + signer.address.clone().to_string(), + (KeyType::ECDSA, signer.pub_key.clone()), + ) + }) + .collect::>(); let subkey = "key".to_string(); let snapshot = build_snapshot(&signers); @@ -318,6 +397,35 @@ mod tests { ) } + fn do_register_key( + deps: DepsMut, + worker: Addr, + public_key: HexBinary, + key_type: KeyType, + ) -> Result { + let msg = ExecuteMsg::RegisterPublicKey { + public_key, + key_type, + }; + execute(deps, mock_env(), mock_info(worker.as_str(), &[]), msg) + } + + fn query_registered_keys( + deps: Deps, + workers: Vec, + key_type: KeyType, + ) -> StdResult { + let env = mock_env(); + query( + deps, + env, + QueryMsg::GetPublicKeys { + worker_addresses: workers.into_iter().map(|w| w.to_string()).collect(), + key_type, + }, + ) + } + fn setup() -> OwnedDeps { let mut deps = mock_dependencies(); do_instantiate(deps.as_mut()).unwrap(); @@ -464,7 +572,7 @@ mod tests { .signatures .get(&signer.address.clone().into_string()) .unwrap(), - &Signature::try_from(signer.signature.clone()).unwrap() + &Signature::ECDSA(ECDSASignature::try_from(signer.signature.clone()).unwrap()) ); assert_eq!(session.state, MultisigState::Pending); @@ -511,7 +619,7 @@ mod tests { .signatures .get(&signer.address.into_string()) .unwrap(), - &Signature::try_from(signer.signature).unwrap() + &Signature::ECDSA(ECDSASignature::try_from(signer.signature).unwrap()) ); assert_eq!(session.state, MultisigState::Completed); @@ -578,4 +686,68 @@ mod tests { assert_eq!(signer.signature, session.signatures.get(address).cloned()); }); } + #[test] + fn test_register_key() { + let mut deps = mock_dependencies(); + do_instantiate(deps.as_mut()).unwrap(); + let signers = test_data::signers(); + let pub_keys = signers + .iter() + .map(|signer| (signer.address.clone(), signer.pub_key.clone())) + .collect::>(); + + for (addr, pub_key) in &pub_keys { + let res = do_register_key(deps.as_mut(), addr.clone(), pub_key.clone(), KeyType::ECDSA); + assert!(res.is_ok()); + } + + let res = query_registered_keys( + deps.as_ref(), + pub_keys.clone().into_iter().map(|(addr, _)| addr).collect(), + KeyType::ECDSA, + ); + assert!(res.is_ok()); + assert_eq!( + pub_keys + .into_iter() + .map(|(_, pk)| PublicKey::try_from((KeyType::ECDSA, pk)).unwrap()) + .collect::>(), + from_binary::>(&res.unwrap()).unwrap() + ); + } + + #[test] + fn test_update_key() { + let mut deps = mock_dependencies(); + do_instantiate(deps.as_mut()).unwrap(); + let signers = test_data::signers(); + let pub_keys = signers + .iter() + .map(|signer| (signer.address.clone(), signer.pub_key.clone())) + .collect::>(); + + for (addr, pub_key) in &pub_keys { + let res = do_register_key(deps.as_mut(), addr.clone(), pub_key.clone(), KeyType::ECDSA); + assert!(res.is_ok()); + } + + let new_pub_key = HexBinary::from_hex( + "021a381b3e07347d3a05495347e1fb2fe04764afcea5a74084fa957947b59f9026", + ) + .unwrap(); + let res = do_register_key( + deps.as_mut(), + pub_keys[0].0.clone(), + new_pub_key.clone(), + KeyType::ECDSA, + ); + assert!(res.is_ok()); + + let res = query_registered_keys(deps.as_ref(), vec![pub_keys[0].0.clone()], KeyType::ECDSA); + assert!(res.is_ok()); + assert_eq!( + vec![PublicKey::try_from((KeyType::ECDSA, new_pub_key)).unwrap()], + from_binary::>(&res.unwrap()).unwrap() + ); + } } diff --git a/contracts/multisig/src/events.rs b/contracts/multisig/src/events.rs index 330a9e35e..900412a71 100644 --- a/contracts/multisig/src/events.rs +++ b/contracts/multisig/src/events.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use cosmwasm_std::{Addr, HexBinary, Uint64}; use serde_json::to_string; -use crate::types::{KeyID, MsgToSign, PublicKey, Signature}; +use crate::types::{KeyID, KeyType, MsgToSign, PublicKey, Signature}; pub enum Event { // Emitted when a new signing session is open @@ -23,6 +23,11 @@ pub enum Event { SigningCompleted { session_id: Uint64, }, + PublicKeyRegistered { + worker: Addr, + public_key: PublicKey, + key_type: KeyType, + }, } impl From for cosmwasm_std::Event { @@ -55,6 +60,23 @@ impl From for cosmwasm_std::Event { .add_attribute("signature", HexBinary::from(signature).to_hex()), Event::SigningCompleted { session_id } => cosmwasm_std::Event::new("signing_completed") .add_attribute("session_id", session_id), + Event::PublicKeyRegistered { + worker, + public_key, + key_type, + } => cosmwasm_std::Event::new("public_key_registered") + .add_attribute( + "worker", + to_string(&worker).expect("failed to serialize worker"), + ) + .add_attribute( + "public_key", + to_string(&public_key).expect("failed to serialize public key"), + ) + .add_attribute( + "key_type", + to_string(&key_type).expect("failed to serialize key type"), + ), } } } diff --git a/contracts/multisig/src/msg.rs b/contracts/multisig/src/msg.rs index 063a39179..0e0456910 100644 --- a/contracts/multisig/src/msg.rs +++ b/contracts/multisig/src/msg.rs @@ -4,7 +4,7 @@ use axelar_wasm_std::Snapshot; use cosmwasm_schema::{cw_serde, QueryResponses}; use cosmwasm_std::{Addr, HexBinary, Uint256, Uint64}; -use crate::types::{Key, KeyID, MultisigState, PublicKey, Signature}; +use crate::types::{Key, KeyID, KeyType, MultisigState, PublicKey, Signature}; #[cw_serde] pub struct InstantiateMsg {} @@ -22,7 +22,11 @@ pub enum ExecuteMsg { KeyGen { key_id: String, snapshot: Snapshot, - pub_keys: HashMap, + pub_keys: HashMap, + }, + RegisterPublicKey { + public_key: HexBinary, + key_type: KeyType, }, } @@ -34,6 +38,12 @@ pub enum QueryMsg { #[returns(Key)] GetKey { key_id: KeyID }, + + #[returns(Vec)] + GetPublicKeys { + worker_addresses: Vec, + key_type: KeyType, + }, } #[cw_serde] diff --git a/contracts/multisig/src/secp256k1.rs b/contracts/multisig/src/secp256k1.rs index d3ca37e3c..457026377 100644 --- a/contracts/multisig/src/secp256k1.rs +++ b/contracts/multisig/src/secp256k1.rs @@ -3,7 +3,7 @@ use cosmwasm_std::HexBinary; // TODO: Logic specific to secp256k1 will most likely be handled by core in the future. use crate::{ - types::{MsgToSign, PublicKey, Signature, VerifiableSignature}, + types::{ECDSAPublicKey, ECDSASignature, MsgToSign}, ContractError, }; @@ -13,7 +13,7 @@ const UNCOMPRESSED_PUBKEY_LEN: usize = 65; const EVM_SIGNATURE_LEN: usize = 65; const ECDSA_SIGNATURE_LEN: usize = 64; -impl TryFrom for PublicKey { +impl TryFrom for ECDSAPublicKey { type Error = ContractError; fn try_from(other: HexBinary) -> Result { @@ -23,7 +23,7 @@ impl TryFrom for PublicKey { }); } - Ok(PublicKey::unchecked(other)) + Ok(ECDSAPublicKey::unchecked(other)) } } @@ -41,7 +41,7 @@ impl TryFrom for MsgToSign { } } -impl TryFrom for Signature { +impl TryFrom for ECDSASignature { type Error = ContractError; fn try_from(other: HexBinary) -> Result { @@ -51,22 +51,24 @@ impl TryFrom for Signature { }); } - Ok(Signature::unchecked(other)) + Ok(ECDSASignature::unchecked(other)) } } -impl VerifiableSignature for Signature { - fn verify(&self, msg: &MsgToSign, pub_key: &PublicKey) -> Result { - let signature: &[u8] = self.into(); - secp256k1_verify( - msg.into(), - &signature[0..ECDSA_SIGNATURE_LEN], - pub_key.into(), - ) - .map_err(|e| ContractError::SignatureVerificationFailed { - reason: e.to_string(), - }) - } +pub fn ecdsa_verify( + msg: &MsgToSign, + sig: &ECDSASignature, + pub_key: &ECDSAPublicKey, +) -> Result { + let signature: &[u8] = sig.into(); + secp256k1_verify( + msg.into(), + &signature[0..ECDSA_SIGNATURE_LEN], + pub_key.into(), + ) + .map_err(|e| ContractError::SignatureVerificationFailed { + reason: e.to_string(), + }) } #[cfg(test)] @@ -76,17 +78,17 @@ mod tests { use crate::test::common::test_data; #[test] - fn test_try_from_hexbinary_to_public_key() { + fn test_try_from_hexbinary_to_ecdsa_public_key() { let hex = test_data::pub_key(); - let pub_key = PublicKey::try_from(hex.clone()).unwrap(); + let pub_key = ECDSAPublicKey::try_from(hex.clone()).unwrap(); assert_eq!(HexBinary::from(pub_key), hex); } #[test] - fn test_try_from_hexbinary_to_public_key_fails() { + fn test_try_from_hexbinary_to_eccdsa_public_key_fails() { let hex = HexBinary::from_hex("049b").unwrap(); assert_eq!( - PublicKey::try_from(hex.clone()).unwrap_err(), + ECDSAPublicKey::try_from(hex.clone()).unwrap_err(), ContractError::InvalidPublicKeyFormat { reason: "Invalid input length".into() } @@ -114,7 +116,7 @@ mod tests { #[test] fn test_try_from_hexbinary_to_signature() { let hex = test_data::signature(); - let signature = Signature::try_from(hex.clone()).unwrap(); + let signature = ECDSASignature::try_from(hex.clone()).unwrap(); assert_eq!(HexBinary::from(signature), hex); } @@ -124,7 +126,7 @@ mod tests { HexBinary::from_hex("283786d844a7c4d1d424837074d0c8ec71becdcba4dd42b5307cb543a0e2c8b81c10ad541defd5ce84d2a608fc454827d0b65b4865c8192a2ea1736a5c4b72") .unwrap(); assert_eq!( - Signature::try_from(hex.clone()).unwrap_err(), + ECDSASignature::try_from(hex.clone()).unwrap_err(), ContractError::InvalidSignatureFormat { reason: "Invalid input length".into() } @@ -133,10 +135,10 @@ mod tests { #[test] fn test_verify_signature() { - let signature = Signature::try_from(test_data::signature()).unwrap(); + let signature = ECDSASignature::try_from(test_data::signature()).unwrap(); let message = MsgToSign::try_from(test_data::message()).unwrap(); - let public_key = PublicKey::try_from(test_data::pub_key()).unwrap(); - let result = signature.verify(&message, &public_key).unwrap(); + let public_key = ECDSAPublicKey::try_from(test_data::pub_key()).unwrap(); + let result = ecdsa_verify(&message, &signature, &public_key).unwrap(); assert_eq!(result, true); } @@ -147,10 +149,10 @@ mod tests { ) .unwrap(); - let signature = Signature::try_from(invalid_signature).unwrap(); + let signature = ECDSASignature::try_from(invalid_signature).unwrap(); let message = MsgToSign::try_from(test_data::message()).unwrap(); - let public_key = PublicKey::try_from(test_data::pub_key()).unwrap(); - let result = signature.verify(&message, &public_key).unwrap(); + let public_key = ECDSAPublicKey::try_from(test_data::pub_key()).unwrap(); + let result = ecdsa_verify(&message, &signature, &public_key).unwrap(); assert_eq!(result, false); } @@ -161,10 +163,10 @@ mod tests { ) .unwrap(); - let signature = Signature::try_from(test_data::signature()).unwrap(); + let signature = ECDSASignature::try_from(test_data::signature()).unwrap(); let message = MsgToSign::try_from(test_data::message()).unwrap(); - let public_key = PublicKey::try_from(invalid_pub_key).unwrap(); - let result = signature.verify(&message, &public_key).unwrap(); + let public_key = ECDSAPublicKey::try_from(invalid_pub_key).unwrap(); + let result = ecdsa_verify(&message, &signature, &public_key).unwrap(); assert_eq!(result, false); } } diff --git a/contracts/multisig/src/signing.rs b/contracts/multisig/src/signing.rs index 3e90335c8..0004c1ec1 100644 --- a/contracts/multisig/src/signing.rs +++ b/contracts/multisig/src/signing.rs @@ -104,6 +104,7 @@ mod tests { state::{KEYS, SIGNING_SESSIONS}, test::common::test_data, test::common::{build_key, build_snapshot, TestSigner}, + types::{ECDSASignature, KeyType}, }; use super::*; @@ -150,7 +151,11 @@ mod tests { let res = session.add_signature( key, signer.address.clone().into_string(), - Signature::try_from(signer.signature.clone()).unwrap(), + Signature::try_from(( + (KeyType::ECDSA, signer.pub_key).try_into().unwrap(), + signer.signature.clone(), + )) + .unwrap(), ); SIGNING_SESSIONS.save(&mut config.store, session.id.u64(), &session)?; @@ -220,7 +225,7 @@ mod tests { let mut session = SigningSession::new(Uint64::one(), config.key_id.clone(), config.message.clone()); - let invalid_sig: Signature = HexBinary::from_hex("a58c9543b9df54578ec45838948e19afb1c6e4c86b34d9899b10b44e619ea74e19b457611e41a047030ed233af437d7ecff84de97cb6b3c13d73d22874e035111c") + let invalid_sig: ECDSASignature = HexBinary::from_hex("a58c9543b9df54578ec45838948e19afb1c6e4c86b34d9899b10b44e619ea74e19b457611e41a047030ed233af437d7ecff84de97cb6b3c13d73d22874e035111c") .unwrap().try_into().unwrap(); let key = KEYS.load(&config.store, (&session.key_id).into()).unwrap(); @@ -228,7 +233,7 @@ mod tests { let result = session.add_signature( key, config.signers[0].address.clone().into_string(), - invalid_sig, + Signature::ECDSA(invalid_sig), ); assert_eq!( @@ -254,7 +259,9 @@ mod tests { let result = session.add_signature( key, invalid_participant.clone(), - Signature::try_from(config.signers[0].signature.clone()).unwrap(), + Signature::ECDSA( + ECDSASignature::try_from(config.signers[0].signature.clone()).unwrap(), + ), ); assert_eq!( diff --git a/contracts/multisig/src/state.rs b/contracts/multisig/src/state.rs index af4aad931..47830b55d 100644 --- a/contracts/multisig/src/state.rs +++ b/contracts/multisig/src/state.rs @@ -1,9 +1,10 @@ -use cosmwasm_std::{Storage, Uint64}; +use cosmwasm_std::{Addr, Storage, Uint64}; use cw_storage_plus::{Item, Map}; use crate::{ signing::SigningSession, - types::{Key, KeyID}, + types::KeyType, + types::{Key, KeyID, PublicKey}, ContractError, }; @@ -19,3 +20,5 @@ pub fn get_key(store: &dyn Storage, key_id: &KeyID) -> Result = Map::new("registered_pub_keys"); diff --git a/contracts/multisig/src/test/common.rs b/contracts/multisig/src/test/common.rs index db129efc8..5aa7647c2 100644 --- a/contracts/multisig/src/test/common.rs +++ b/contracts/multisig/src/test/common.rs @@ -4,7 +4,7 @@ use axelar_wasm_std::{Participant, Snapshot, Threshold}; use cosmwasm_std::{Addr, HexBinary, Timestamp, Uint256, Uint64}; use rand::Rng; -use crate::types::{Key, KeyID, PublicKey}; +use crate::types::{Key, KeyID, KeyType, PublicKey}; #[derive(Clone)] pub struct TestSigner { @@ -77,7 +77,7 @@ pub fn build_key(key_id: KeyID, signers: &Vec, snapshot: Snapshot) - .map(|signer| { ( signer.address.clone().to_string(), - PublicKey::try_from(signer.pub_key.clone()).unwrap(), + PublicKey::try_from((KeyType::ECDSA, signer.pub_key.clone())).unwrap(), ) }) .collect::>(); diff --git a/contracts/multisig/src/types.rs b/contracts/multisig/src/types.rs index d0c108a95..c5c509b09 100644 --- a/contracts/multisig/src/types.rs +++ b/contracts/multisig/src/types.rs @@ -2,31 +2,85 @@ use std::{collections::HashMap, fmt}; use axelar_wasm_std::Snapshot; use cosmwasm_schema::cw_serde; -use cosmwasm_std::{from_binary, Addr, HexBinary, StdResult}; +use cosmwasm_std::{from_binary, Addr, HexBinary, StdError, StdResult}; use cw_storage_plus::{KeyDeserialize, PrimaryKey}; -use crate::ContractError; +use crate::{secp256k1::ecdsa_verify, ContractError}; pub trait VerifiableSignature { fn verify(&self, msg: &MsgToSign, pub_key: &PublicKey) -> Result; } #[cw_serde] -pub struct PublicKey(HexBinary); +pub enum KeyType { + ECDSA, +} -impl From for HexBinary { - fn from(original: PublicKey) -> Self { - original.0 +impl<'a> PrimaryKey<'a> for KeyType { + type Prefix = (); + type SubPrefix = (); + type Suffix = Self; + type SuperSuffix = Self; + + fn key(&self) -> Vec { + vec![cw_storage_plus::Key::Val8( + vec![self.clone() as u8] + .try_into() + .expect("failed to serialize key type"), + )] + } +} + +impl KeyDeserialize for KeyType { + type Output = Self; + + fn from_vec(value: Vec) -> StdResult { + serde_json::from_slice(value.as_slice()).map_err(|err| StdError::ParseErr { + target_type: "KeyType".into(), + msg: err.to_string(), + }) + } +} + +#[cw_serde] +pub enum PublicKey { + ECDSA(ECDSAPublicKey), +} + +impl TryFrom<(KeyType, HexBinary)> for PublicKey { + type Error = ContractError; + + fn try_from((key_type, pub_key): (KeyType, HexBinary)) -> Result { + match key_type { + KeyType::ECDSA => ECDSAPublicKey::try_from(pub_key).map(PublicKey::ECDSA), + } } } impl<'a> From<&'a PublicKey> for &'a [u8] { - fn from(original: &'a PublicKey) -> Self { + fn from(value: &'a PublicKey) -> Self { + match value { + PublicKey::ECDSA(pub_key) => pub_key.into(), + } + } +} + +#[cw_serde] +pub struct ECDSAPublicKey(HexBinary); + +impl From for HexBinary { + fn from(original: ECDSAPublicKey) -> Self { + original.0 + } +} + +impl<'a> From<&'a ECDSAPublicKey> for &'a [u8] { + fn from(original: &'a ECDSAPublicKey) -> Self { original.0.as_slice() } } -impl PublicKey { +impl ECDSAPublicKey { pub fn unchecked(hex: HexBinary) -> Self { Self(hex) } @@ -55,32 +109,64 @@ impl MsgToSign { #[cw_serde] #[derive(Ord, PartialOrd, Eq)] -pub struct Signature(HexBinary); +pub enum Signature { + ECDSA(ECDSASignature), +} impl From for HexBinary { - fn from(original: Signature) -> Self { + fn from(value: Signature) -> Self { + match value { + Signature::ECDSA(sig) => sig.into(), + } + } +} + +impl TryFrom<(PublicKey, HexBinary)> for Signature { + type Error = ContractError; + + fn try_from((pk, sig): (PublicKey, HexBinary)) -> Result { + match pk { + PublicKey::ECDSA(_) => ECDSASignature::try_from(sig).map(Signature::ECDSA), + } + } +} + +#[cw_serde] +#[derive(Ord, PartialOrd, Eq)] +pub struct ECDSASignature(HexBinary); + +impl From for HexBinary { + fn from(original: ECDSASignature) -> Self { original.0 } } -impl From<&Signature> for Vec { - fn from(original: &Signature) -> Self { +impl From<&ECDSASignature> for Vec { + fn from(original: &ECDSASignature) -> Self { original.0.to_vec() } } -impl<'a> From<&'a Signature> for &'a [u8] { - fn from(original: &'a Signature) -> Self { +impl<'a> From<&'a ECDSASignature> for &'a [u8] { + fn from(original: &'a ECDSASignature) -> Self { original.0.as_slice() } } -impl Signature { +impl ECDSASignature { pub fn unchecked(hex: HexBinary) -> Self { Self(hex) } } +impl VerifiableSignature for Signature { + fn verify(&self, msg: &MsgToSign, pub_key: &PublicKey) -> Result { + match (self, pub_key) { + (Signature::ECDSA(sig), PublicKey::ECDSA(pub_key)) => ecdsa_verify(msg, sig, pub_key), + } + } +} + #[cw_serde] pub struct KeyID { pub owner: Addr, From 9b9344e49dc73d2e6cc3a3bcb8666bc9566247d3 Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Thu, 31 Aug 2023 12:09:37 -0400 Subject: [PATCH 02/14] fix tests --- ampd/tests/report.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ampd/tests/report.rs b/ampd/tests/report.rs index 5538a4ca7..ff87113c0 100644 --- a/ampd/tests/report.rs +++ b/ampd/tests/report.rs @@ -27,15 +27,15 @@ fn correct_error_log() { let expected_err = LoggableError { msg: "error3".to_string(), attachments: vec!["opaque attachment".to_string()], - location: "ampd/tests/report.rs:16:10".to_string(), + location: "ampd/tests/report.rs:15:10".to_string(), cause: Some(Box::new(LoggableError { msg: "error2".to_string(), attachments: vec!["test1".to_string(), "test2".to_string()], - location: "ampd/tests/report.rs:13:10".to_string(), + location: "ampd/tests/report.rs:12:10".to_string(), cause: Some(Box::new(LoggableError { msg: "error1".to_string(), attachments: vec!["foo1".to_string()], - location: "ampd/tests/report.rs:11:18".to_string(), + location: "ampd/tests/report.rs:10:18".to_string(), cause: None, backtrace: None, })), From 1a4760319b1dfafecf8c35321bb34e92d3a77a50 Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Thu, 31 Aug 2023 12:27:39 -0400 Subject: [PATCH 03/14] refactor --- .../src/test/mocks/multisig.rs | 6 +-- contracts/multisig/src/contract.rs | 50 ++++++++----------- contracts/multisig/src/msg.rs | 6 +-- 3 files changed, 26 insertions(+), 36 deletions(-) diff --git a/contracts/multisig-prover/src/test/mocks/multisig.rs b/contracts/multisig-prover/src/test/mocks/multisig.rs index 63fd5845f..9a1b3a421 100644 --- a/contracts/multisig-prover/src/test/mocks/multisig.rs +++ b/contracts/multisig-prover/src/test/mocks/multisig.rs @@ -45,9 +45,9 @@ pub fn query(_deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { match msg { QueryMsg::GetMultisig { session_id: _ } => to_binary(&query::query_success()), QueryMsg::GetKey { key_id: _ } => unimplemented!(), - QueryMsg::GetPublicKeys { - worker_addresses, - key_type, + QueryMsg::GetPublicKey { + worker_address: _, + key_type: _, } => unimplemented!(), } } diff --git a/contracts/multisig/src/contract.rs b/contracts/multisig/src/contract.rs index 7b9a5213c..156ac7b8f 100644 --- a/contracts/multisig/src/contract.rs +++ b/contracts/multisig/src/contract.rs @@ -220,15 +220,12 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { match msg { QueryMsg::GetMultisig { session_id } => to_binary(&query::get_multisig(deps, session_id)?), QueryMsg::GetKey { key_id } => to_binary(&query::get_key(deps, key_id)?), - QueryMsg::GetPublicKeys { - worker_addresses, + QueryMsg::GetPublicKey { + worker_address, key_type, - } => to_binary(&query::get_keys( + } => to_binary(&query::get_public_key( deps, - worker_addresses - .iter() - .map(|addr| deps.api.addr_validate(addr)) - .collect::, _>>()?, + deps.api.addr_validate(&worker_address)?, key_type, )?), } @@ -274,15 +271,8 @@ pub mod query { KEYS.load(deps.storage, &key_id) } - pub fn get_keys( - deps: Deps, - workers: Vec, - key_type: KeyType, - ) -> StdResult> { - workers - .into_iter() - .map(|worker| PUB_KEYS.load(deps.storage, (worker, key_type.clone()))) - .collect::, _>>() + pub fn get_public_key(deps: Deps, worker: Addr, key_type: KeyType) -> StdResult { + PUB_KEYS.load(deps.storage, (worker, key_type.clone())) } } @@ -416,17 +406,17 @@ mod tests { execute(deps, mock_env(), mock_info(worker.as_str(), &[]), msg) } - fn query_registered_keys( + fn query_registered_public_key( deps: Deps, - workers: Vec, + worker: Addr, key_type: KeyType, ) -> StdResult { let env = mock_env(); query( deps, env, - QueryMsg::GetPublicKeys { - worker_addresses: workers.into_iter().map(|w| w.to_string()).collect(), + QueryMsg::GetPublicKey { + worker_address: worker.to_string(), key_type, }, ) @@ -707,19 +697,19 @@ mod tests { let res = do_register_key(deps.as_mut(), addr.clone(), pub_key.clone(), KeyType::ECDSA); assert!(res.is_ok()); } + let mut ret_pub_keys: Vec = vec![]; - let res = query_registered_keys( - deps.as_ref(), - pub_keys.clone().into_iter().map(|(addr, _)| addr).collect(), - KeyType::ECDSA, - ); - assert!(res.is_ok()); + for (addr, _) in &pub_keys { + let res = query_registered_public_key(deps.as_ref(), addr.clone(), KeyType::ECDSA); + 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::>(), - from_binary::>(&res.unwrap()).unwrap() + ret_pub_keys ); } @@ -750,11 +740,11 @@ mod tests { ); assert!(res.is_ok()); - let res = query_registered_keys(deps.as_ref(), vec![pub_keys[0].0.clone()], KeyType::ECDSA); + let res = query_registered_public_key(deps.as_ref(), pub_keys[0].0.clone(), KeyType::ECDSA); assert!(res.is_ok()); assert_eq!( - vec![PublicKey::try_from((KeyType::ECDSA, new_pub_key)).unwrap()], - from_binary::>(&res.unwrap()).unwrap() + PublicKey::try_from((KeyType::ECDSA, new_pub_key)).unwrap(), + from_binary::(&res.unwrap()).unwrap() ); } } diff --git a/contracts/multisig/src/msg.rs b/contracts/multisig/src/msg.rs index 0e0456910..d160ef69d 100644 --- a/contracts/multisig/src/msg.rs +++ b/contracts/multisig/src/msg.rs @@ -39,9 +39,9 @@ pub enum QueryMsg { #[returns(Key)] GetKey { key_id: KeyID }, - #[returns(Vec)] - GetPublicKeys { - worker_addresses: Vec, + #[returns(PublicKey)] + GetPublicKey { + worker_address: String, key_type: KeyType, }, } From ce76b67ba6be8ed3cd7b62d42c34bb879afe626d Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Wed, 23 Aug 2023 11:49:16 -0400 Subject: [PATCH 04/14] feat(multisig-prover): start update worker set * detect if worker set needs to be updated * ABI encode new worker set and add to batch --- contracts/multisig-prover/src/contract.rs | 47 ++++ contracts/multisig-prover/src/encoding/evm.rs | 242 ++++++++++++++---- contracts/multisig-prover/src/encoding/mod.rs | 2 +- contracts/multisig-prover/src/error.rs | 13 + contracts/multisig-prover/src/execute.rs | 94 ++++++- contracts/multisig-prover/src/msg.rs | 2 + contracts/multisig-prover/src/state.rs | 47 +++- .../src/test/mocks/multisig.rs | 24 +- .../multisig-prover/src/test/multicontract.rs | 1 + .../multisig-prover/src/test/test_data.rs | 74 +++++- contracts/multisig-prover/src/types.rs | 9 +- contracts/multisig/src/contract.rs | 31 ++- contracts/multisig/src/msg.rs | 3 +- 13 files changed, 509 insertions(+), 80 deletions(-) diff --git a/contracts/multisig-prover/src/contract.rs b/contracts/multisig-prover/src/contract.rs index 8b928e118..f0260b7e8 100644 --- a/contracts/multisig-prover/src/contract.rs +++ b/contracts/multisig-prover/src/contract.rs @@ -41,6 +41,7 @@ pub fn instantiate( service_name: msg.service_name, chain_name: ChainName::from_str(&msg.chain_name) .map_err(|_| ContractError::InvalidChainName)?, + worker_set_diff_threshold: msg.worker_set_diff_threshold, }; CONFIG.save(deps.storage, &config)?; @@ -65,6 +66,7 @@ pub fn execute( execute::rotate_snapshot(deps, env, config, pub_keys, key_id) } + ExecuteMsg::UpdateWorkerSet {} => execute::update_worker_set(deps, env), } } @@ -204,6 +206,7 @@ mod test { signing_threshold, service_name: service_name.to_string(), chain_name: "Ethereum".to_string(), + worker_set_diff_threshold: 0, }; let res = instantiate(deps.as_mut(), env, info, msg); @@ -285,3 +288,47 @@ mod test { } } } + +#[cfg(test)] +mod tests { + use crate::{contract::execute::should_update_worker_set, test::test_data}; + + #[test] + fn should_update_worker_set_no_change() { + let worker_set = test_data::new_worker_set(); + assert!(!should_update_worker_set(&worker_set, &worker_set, 0)); + } + + #[test] + fn should_update_worker_set_one_more() { + let worker_set = test_data::new_worker_set(); + let mut new_worker_set = worker_set.clone(); + new_worker_set.signers.pop(); + assert!(should_update_worker_set(&worker_set, &new_worker_set, 0)); + } + + #[test] + fn should_update_worker_set_one_less() { + let worker_set = test_data::new_worker_set(); + let mut new_worker_set = worker_set.clone(); + new_worker_set.signers.pop(); + assert!(should_update_worker_set(&new_worker_set, &worker_set, 0)); + } + + #[test] + fn should_update_worker_set_one_more_higher_threshold() { + let worker_set = test_data::new_worker_set(); + let mut new_worker_set = worker_set.clone(); + new_worker_set.signers.pop(); + assert!(!should_update_worker_set(&worker_set, &new_worker_set, 1)); + } + + #[test] + fn should_update_worker_set_diff_pub_key() { + let worker_set = test_data::new_worker_set(); + let mut new_worker_set = worker_set.clone(); + new_worker_set.signers[0].pub_key = worker_set.signers[1].pub_key.clone(); + new_worker_set.signers[1].pub_key = worker_set.signers[0].pub_key.clone(); + assert!(should_update_worker_set(&worker_set, &new_worker_set, 0)); + } +} diff --git a/contracts/multisig-prover/src/encoding/evm.rs b/contracts/multisig-prover/src/encoding/evm.rs index 91f789d20..9a3efeae9 100644 --- a/contracts/multisig-prover/src/encoding/evm.rs +++ b/contracts/multisig-prover/src/encoding/evm.rs @@ -1,5 +1,6 @@ use std::str::FromStr; +use axelar_wasm_std::{operators::Operators, Snapshot}; use cosmwasm_schema::cw_serde; use cosmwasm_std::{HexBinary, Uint256}; use ethabi::{ethereum_types, short_signature, ParamType, Token}; @@ -11,6 +12,7 @@ use multisig::{msg::Signer, types::Signature}; use crate::{ error::ContractError, + state::WorkerSet, types::{BatchID, Command, CommandBatch, CommandType, Operator}, }; @@ -44,34 +46,69 @@ impl TryFrom for Command { } } -impl TryFrom for Operator { +impl TryFrom for Command { + type Error = ContractError; + fn try_from(worker_set: WorkerSet) -> Result { + let params = transfer_operatorship_params(&worker_set)?; + Ok(Command { + ty: CommandType::TransferOperatorship, + params, + id: worker_set.hash(), + }) + } +} + +impl TryFrom<(Signer, Option)> for Operator { type Error = ContractError; - fn try_from(signer: Signer) -> Result { + fn try_from((signer, sig): (Signer, Option)) -> Result { Ok(Self { address: evm_address((&signer.pub_key).into())?, weight: signer.weight, - signature: signer.signature.map(|Signature::ECDSA(sig)| sig), + signature: sig.map(|Signature::ECDSA(sig)| sig), }) } } +pub fn make_worker_set( + snapshot: Snapshot, + pub_keys: Vec, +) -> Result { + let addresses = pub_keys + .iter() + .map(|pub_key| evm_address(pub_key.into())) + .collect::, _>>()?; + let zipped: Vec<(HexBinary, Uint256)> = addresses + .into_iter() + .zip(snapshot.participants.values().map(|p| p.weight.into())) + .collect(); + Ok(Operators { + weights: zipped, + threshold: snapshot.quorum.into(), + }) +} + impl CommandBatch { pub fn new( messages: Vec, destination_chain_id: Uint256, + new_worker_set: Option, ) -> Result { let message_ids: Vec = messages.iter().map(|msg| msg.id.clone()).collect(); + let mut commands = messages + .into_iter() + .map(TryInto::try_into) + .collect::, ContractError>>()?; + if new_worker_set.is_some() { + commands.push(new_worker_set.clone().unwrap().try_into()?); + } let data = Data { destination_chain_id, - commands: messages - .into_iter() - .map(TryInto::try_into) - .collect::, ContractError>>()?, + commands, }; - let id = BatchID::new(&message_ids); + let id = BatchID::new(&message_ids, new_worker_set); Ok(Self { id, @@ -96,7 +133,7 @@ impl CommandBatch { pub fn encode_execute_data( &self, quorum: Uint256, - signers: Vec, + signers: Vec<(Signer, Option)>, ) -> Result { let param = ethabi::encode(&[ Token::Bytes(self.data.encode().into()), @@ -116,7 +153,7 @@ impl CommandBatch { fn encode_proof( &self, quorum: Uint256, - signers: Vec, + signers: Vec<(Signer, Option)>, ) -> Result { let operators = sorted_operators(signers)?; @@ -150,6 +187,40 @@ impl CommandBatch { } } +fn transfer_operatorship_params(worker_set: &WorkerSet) -> Result { + let mut operators: Vec<(HexBinary, Uint256)> = worker_set + .signers + .iter() + .map(|s| { + ( + evm_address((&s.pub_key).into()).expect("couldn't convert pubkey to evm address"), + s.weight, + ) + }) + .collect(); + operators.sort_by_key(|op| op.0.clone()); + + let (addresses, weights) = operators.iter().fold( + (vec![], vec![]), + |(mut addresses, mut weights), operator| { + addresses.push(Token::Address(ethereum_types::Address::from_slice( + operator.0.as_slice(), + ))); + weights.push(Token::Uint(ethereum_types::U256::from_big_endian( + &operator.1.to_be_bytes(), + ))); + + (addresses, weights) + }, + ); + + let quorum = Token::Uint(ethereum_types::U256::from_big_endian( + &worker_set.threshold.to_be_bytes(), + )); + + Ok(ethabi::encode(&[Token::Array(addresses), Token::Array(weights), quorum]).into()) +} + #[cw_serde] pub struct Data { pub destination_chain_id: Uint256, @@ -192,7 +263,9 @@ fn evm_address(pub_key: &[u8]) -> Result { Ok(Keccak256::digest(&pub_key.as_bytes()[1..]).as_slice()[12..].into()) } -fn sorted_operators(signers: Vec) -> Result, ContractError> { +fn sorted_operators( + signers: Vec<(Signer, Option)>, +) -> Result, ContractError> { let mut operators = signers .into_iter() .map(TryInto::try_into) @@ -245,6 +318,19 @@ mod test { ) .unwrap() } + fn decode_operator_transfer_command_params<'a>( + encoded_params: impl Into>, + ) -> Vec { + ethabi::decode( + &[ + ParamType::Array(Box::new(ParamType::Address)), + ParamType::Array(Box::new(ParamType::Uint(32))), + ParamType::Uint(32), + ], + &encoded_params.into(), + ) + .unwrap() + } pub fn decode_data(encoded_data: &HexBinary) -> Data { let tokens_array = ðabi::decode( @@ -284,7 +370,8 @@ mod test { id: id.to_owned().try_into().unwrap(), ty: match ty.as_str() { "approveContractCall" => CommandType::ApproveContractCall, - _ => panic!("undecodable command type"), + "transferOperatorship" => CommandType::TransferOperatorship, + &_ => panic!("undecodable command type"), }, params: HexBinary::from(params.to_owned()), }; @@ -360,13 +447,49 @@ mod test { ); } + #[test] + fn test_command_operator_transfer() { + let mut new_worker_set = test_data::new_worker_set(); + new_worker_set + .signers + .sort_by_key(|signer| evm_address((&signer.pub_key).into()).unwrap()); + let res = Command::try_from(new_worker_set.clone()); + assert!(res.is_ok()); + + let tokens = decode_operator_transfer_command_params(res.unwrap().params); + + for i in 0..new_worker_set.signers.len() { + assert_eq!( + tokens[0].clone().into_array().unwrap()[i], + Token::Address(ethereum_types::Address::from_slice( + evm_address((&new_worker_set.signers[i].pub_key).into()) + .expect("couldn't convert pubkey to evm address") + .as_slice() + )) + ); + + assert_eq!( + tokens[1].clone().into_array().unwrap()[i], + Token::Uint(ethereum_types::U256::from_big_endian( + &new_worker_set.signers[i].weight.to_be_bytes() + )) + ); + } + assert_eq!( + tokens[2], + Token::Uint(ethereum_types::U256::from_big_endian( + &new_worker_set.threshold.to_be_bytes() + )) + ); + } + #[test] fn test_new_command_batch() { let messages = test_data::messages(); let destination_chain_id = test_data::destination_chain_id(); let test_data = decode_data(&test_data::encoded_data()); - let res = CommandBatch::new(messages, destination_chain_id).unwrap(); + let res = CommandBatch::new(messages, destination_chain_id, None).unwrap(); assert_eq!( res.message_ids, @@ -394,6 +517,19 @@ mod test { }); } + #[test] + fn test_new_command_batch_with_operator_transfer() { + let test_data = decode_data(&test_data::encoded_data_with_operator_transfer()); + + let res = CommandBatch::new( + vec![], + test_data::chain_id_operator_transfer(), + Some(test_data::new_worker_set()), + ); + assert!(res.is_ok()); + assert_eq!(res.unwrap().data, test_data); + } + #[test] fn test_batch_with_proof() { let messages = test_data::messages(); @@ -401,17 +537,21 @@ mod test { let operators = test_data::operators(); let quorum = test_data::quorum(); - let batch = CommandBatch::new(messages, destination_chain_id).unwrap(); + let batch = CommandBatch::new(messages, destination_chain_id, None).unwrap(); let signers = operators .into_iter() - .map(|op| Signer { - address: op.address, - weight: op.weight.into(), - pub_key: PublicKey::ECDSA(op.pub_key), - signature: op.signature.map(Signature::ECDSA), + .map(|op| { + ( + Signer { + address: op.address, + weight: op.weight.into(), + pub_key: PublicKey::ECDSA(op.pub_key), + }, + op.signature.map(Signature::ECDSA), + ) }) - .collect::>(); + .collect::)>>(); let execute_data = &batch.encode_execute_data(quorum, signers).unwrap(); @@ -479,13 +619,17 @@ mod test { let signers = operators .into_iter() - .map(|op| Signer { - address: op.address, - weight: op.weight.into(), - pub_key: PublicKey::ECDSA(op.pub_key), - signature: op.signature.map(Signature::ECDSA), + .map(|op| { + ( + Signer { + address: op.address, + weight: op.weight.into(), + pub_key: PublicKey::ECDSA(op.pub_key), + }, + op.signature.map(Signature::ECDSA), + ) }) - .collect::>(); + .collect::)>>(); let res = batch.encode_execute_data(quorum, signers).unwrap(); assert_eq!(res, test_data::execute_data()); @@ -506,10 +650,10 @@ mod test { let mut message_ids: Vec = messages.iter().map(|msg| msg.id.clone()).collect(); message_ids.sort(); - let res = BatchID::new(&message_ids); + let res = BatchID::new(&message_ids, None); message_ids.reverse(); - let res2 = BatchID::new(&message_ids); + let res2 = BatchID::new(&message_ids, None); assert_eq!(res, res2); } @@ -549,24 +693,30 @@ mod test { ); let signers = vec![ - Signer { - address: operator2.address, - weight: operator2.weight, - pub_key: PublicKey::ECDSA(operator2.pub_key), - signature: operator2.signature.map(Signature::ECDSA), - }, - Signer { - address: operator1.address, - weight: operator1.weight, - pub_key: PublicKey::ECDSA(operator1.pub_key), - signature: operator1.signature.map(Signature::ECDSA), - }, - Signer { - address: operator3.address, - weight: operator3.weight, - pub_key: PublicKey::ECDSA(operator3.pub_key), - signature: operator3.signature.map(Signature::ECDSA), - }, + ( + Signer { + address: operator2.address, + weight: operator2.weight, + pub_key: PublicKey::ECDSA(operator2.pub_key), + }, + operator2.signature.map(Signature::ECDSA), + ), + ( + Signer { + address: operator1.address, + weight: operator1.weight, + pub_key: PublicKey::ECDSA(operator1.pub_key), + }, + operator1.signature.map(Signature::ECDSA), + ), + ( + Signer { + address: operator3.address, + weight: operator3.weight, + pub_key: PublicKey::ECDSA(operator3.pub_key), + }, + operator3.signature.map(Signature::ECDSA), + ), ]; let operators = sorted_operators(signers).unwrap(); diff --git a/contracts/multisig-prover/src/encoding/mod.rs b/contracts/multisig-prover/src/encoding/mod.rs index 7f2c6ad53..972eae919 100644 --- a/contracts/multisig-prover/src/encoding/mod.rs +++ b/contracts/multisig-prover/src/encoding/mod.rs @@ -1,4 +1,4 @@ // Adding more encoding modules will require using feature flags in here to switch between them -mod evm; +pub mod evm; pub use evm::Data; diff --git a/contracts/multisig-prover/src/error.rs b/contracts/multisig-prover/src/error.rs index c53a5961d..d56060dd1 100644 --- a/contracts/multisig-prover/src/error.rs +++ b/contracts/multisig-prover/src/error.rs @@ -1,3 +1,4 @@ +use axelar_wasm_std::nonempty; use cosmwasm_std::StdError; use thiserror::Error; @@ -23,4 +24,16 @@ pub enum ContractError { #[error("invalid contract reply: {reason}")] InvalidContractReply { reason: String }, + + #[error("public key not found for participant {participant}")] + PublicKeyNotFound { participant: String }, + + #[error("{0}")] + ServiceRegistryError(#[from] service_registry::ContractError), + + #[error("{0}")] + NonEmptyError(#[from] nonempty::Error), + + #[error("worker set has not changed sufficiently since last update")] + WorkerSetUnchanged, } diff --git a/contracts/multisig-prover/src/execute.rs b/contracts/multisig-prover/src/execute.rs index ee8316002..fd29536a8 100644 --- a/contracts/multisig-prover/src/execute.rs +++ b/contracts/multisig-prover/src/execute.rs @@ -2,11 +2,11 @@ use cosmwasm_std::{ to_binary, wasm_execute, Addr, BlockInfo, DepsMut, Env, HexBinary, QuerierWrapper, QueryRequest, Response, SubMsg, WasmMsg, WasmQuery, }; -use multisig::types::KeyType; +use multisig::types::{KeyType, PublicKey}; use std::{collections::HashMap, str::FromStr}; -use axelar_wasm_std::{Participant, Snapshot}; +use axelar_wasm_std::{snapshot, Participant, Snapshot}; use connection_router::{msg::Message, types::ChainName}; use service_registry::state::Worker; @@ -14,7 +14,10 @@ use crate::{ contract::START_MULTISIG_REPLY_ID, error::ContractError, events::Event, - state::{Config, COMMANDS_BATCH, CONFIG, KEY_ID, REPLY_BATCH}, + state::{ + Config, WorkerSet, COMMANDS_BATCH, CONFIG, CURRENT_WORKER_SET, KEY_ID, NEXT_WORKER_SET, + REPLY_BATCH, + }, types::{BatchID, CommandBatch}, }; @@ -22,14 +25,14 @@ pub fn construct_proof(deps: DepsMut, message_ids: Vec) -> Result batch, None => { - let batch = CommandBatch::new(messages, config.destination_chain_id)?; + let batch = CommandBatch::new(messages, config.destination_chain_id, None)?; COMMANDS_BATCH.save(deps.storage, &batch.id, &batch)?; @@ -80,6 +83,87 @@ fn get_messages( Ok(messages) } +pub fn update_worker_set(deps: DepsMut, env: Env) -> Result { + let config = CONFIG.load(deps.storage)?; + + let active_workers_query = service_registry::msg::QueryMsg::GetActiveWorkers { + service_name: config.service_name, + chain_name: config.chain_name.into(), + }; + let workers: Vec = deps.querier.query(&QueryRequest::Wasm(WasmQuery::Smart { + contract_addr: config.service_registry.to_string(), + msg: to_binary(&active_workers_query)?, + }))?; + + let participants = workers + .clone() + .into_iter() + .map(service_registry::state::Worker::try_into) + .collect::, _>>()?; + + let snapshot = snapshot::Snapshot::new( + env.block.time.try_into()?, + env.block.height.try_into()?, + config.signing_threshold, + participants.try_into()?, + ); + + let mut pub_keys = vec![]; + for worker in &workers { + let pub_key_query = multisig::msg::QueryMsg::GetPublicKey { + worker_address: worker.address.to_string(), + key_type: multisig::types::KeyType::ECDSA, + }; + let pub_key: PublicKey = deps.querier.query(&QueryRequest::Wasm(WasmQuery::Smart { + contract_addr: config.multisig.to_string(), + msg: to_binary(&pub_key_query)?, + }))?; + pub_keys.push(pub_key); + } + + let new_worker_set = WorkerSet::new(snapshot, pub_keys, env)?; + + let cur_worker_set = CURRENT_WORKER_SET.load(deps.storage)?; + + if !should_update_worker_set( + &new_worker_set, + &cur_worker_set, + config.worker_set_diff_threshold as usize, + ) { + return Err(ContractError::WorkerSetUnchanged); + } + + NEXT_WORKER_SET.save(deps.storage, &new_worker_set)?; + + let batch = CommandBatch::new(vec![], config.destination_chain_id, Some(new_worker_set))?; + + let start_sig_msg = multisig::msg::ExecuteMsg::StartSigningSession { + key_id: "static".to_string(), // TODO remove the key_id + msg: batch.msg_to_sign(), + }; + + // TODO handle the reply + Ok(Response::new().add_message(wasm_execute(config.multisig, &start_sig_msg, vec![])?)) +} + +pub fn should_update_worker_set( + new_workers: &WorkerSet, + cur_workers: &WorkerSet, + max_diff: usize, +) -> bool { + let count_new = |a: &WorkerSet, b: &WorkerSet| { + a.signers + .iter() + .filter(|a_signer| { + !b.signers.iter().any(|b_signer| { + a_signer.address == b_signer.address && a_signer.pub_key == b_signer.pub_key + }) + }) + .count() + }; + count_new(new_workers, cur_workers) + count_new(cur_workers, new_workers) > max_diff +} + pub fn rotate_snapshot( deps: DepsMut, env: Env, diff --git a/contracts/multisig-prover/src/msg.rs b/contracts/multisig-prover/src/msg.rs index a2c30992e..b5615f86d 100644 --- a/contracts/multisig-prover/src/msg.rs +++ b/contracts/multisig-prover/src/msg.rs @@ -16,6 +16,7 @@ pub struct InstantiateMsg { pub signing_threshold: Threshold, pub service_name: String, pub chain_name: String, + pub worker_set_diff_threshold: u32, } #[cw_serde] @@ -29,6 +30,7 @@ pub enum ExecuteMsg { pub_keys: HashMap, key_id: String, }, + UpdateWorkerSet {}, } #[cw_serde] diff --git a/contracts/multisig-prover/src/state.rs b/contracts/multisig-prover/src/state.rs index 0939a63b9..ec2f66214 100644 --- a/contracts/multisig-prover/src/state.rs +++ b/contracts/multisig-prover/src/state.rs @@ -1,9 +1,12 @@ -use axelar_wasm_std::Threshold; +use axelar_wasm_std::{Snapshot, Threshold}; use connection_router::types::ChainName; use cosmwasm_schema::cw_serde; -use cosmwasm_std::{Addr, Uint256}; +use cosmwasm_std::{Addr, Env, HexBinary, Uint256}; use cw_storage_plus::{Item, Map}; +use multisig::msg::Signer; +use sha3::{Digest, Keccak256}; +use crate::error::ContractError; use crate::types::{BatchID, CommandBatch}; #[cw_serde] @@ -16,6 +19,7 @@ pub struct Config { pub signing_threshold: Threshold, pub service_name: String, pub chain_name: ChainName, + pub worker_set_diff_threshold: u32, } pub const CONFIG: Item = Item::new("config"); @@ -24,3 +28,42 @@ pub const COMMANDS_BATCH: Map<&BatchID, CommandBatch> = Map::new("command_batch" pub const MULTISIG_SESSION_BATCH: Map = Map::new("multisig_session_batch"); pub const REPLY_BATCH: Item = Item::new("reply_tracker"); + +#[cw_serde] +pub struct WorkerSet { + pub signers: Vec, + pub threshold: Uint256, + pub id: u64, // for randomness +} + +impl WorkerSet { + pub fn new( + snapshot: Snapshot, + pub_keys: Vec, + env: Env, + ) -> Result { + let signers = pub_keys + .into_iter() + .zip(snapshot.participants.iter()) + .map(|(pub_key, (_, participant))| Signer { + address: participant.address.clone(), + weight: participant.weight.into(), + pub_key, + }) + .collect(); + + Ok(WorkerSet { + signers, + threshold: snapshot.quorum.into(), + id: env.block.height, + }) + } + + pub fn hash(&self) -> HexBinary { + Keccak256::digest(serde_json::to_vec(&self).expect("couldn't serialize worker set")) + .as_slice() + .into() + } +} +pub const CURRENT_WORKER_SET: Item = Item::new("current_worker_set"); +pub const NEXT_WORKER_SET: Item = Item::new("next_worker_set"); diff --git a/contracts/multisig-prover/src/test/mocks/multisig.rs b/contracts/multisig-prover/src/test/mocks/multisig.rs index 9a1b3a421..62c851035 100644 --- a/contracts/multisig-prover/src/test/mocks/multisig.rs +++ b/contracts/multisig-prover/src/test/mocks/multisig.rs @@ -53,7 +53,10 @@ pub fn query(_deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { } mod query { - use multisig::msg::{Multisig, Signer}; + use multisig::{ + msg::{Multisig, Signer}, + types::Signature, + }; use crate::test::test_data; @@ -65,15 +68,18 @@ mod query { let signers = operators .into_iter() - .map(|op| Signer { - address: op.address, - weight: op.weight.into(), - pub_key: multisig::types::PublicKey::ECDSA(op.pub_key), - signature: op - .signature - .map(|sig| multisig::types::Signature::ECDSA(sig)), + .map(|op| { + ( + Signer { + address: op.address, + weight: op.weight.into(), + pub_key: multisig::types::PublicKey::ECDSA(op.pub_key), + }, + op.signature + .map(|sig| multisig::types::Signature::ECDSA(sig)), + ) }) - .collect::>(); + .collect::)>>(); Multisig { state: MultisigState::Completed, diff --git a/contracts/multisig-prover/src/test/multicontract.rs b/contracts/multisig-prover/src/test/multicontract.rs index 06f2b29c4..3c9c5c657 100644 --- a/contracts/multisig-prover/src/test/multicontract.rs +++ b/contracts/multisig-prover/src/test/multicontract.rs @@ -131,6 +131,7 @@ fn instantiate_prover( signing_threshold: test_data::threshold(), service_name: "service-name".to_string(), chain_name: "Ethereum".to_string(), + worker_set_diff_threshold: 0, }; app.instantiate_contract( diff --git a/contracts/multisig-prover/src/test/test_data.rs b/contracts/multisig-prover/src/test/test_data.rs index c90d71492..ee7670b1a 100644 --- a/contracts/multisig-prover/src/test/test_data.rs +++ b/contracts/multisig-prover/src/test/test_data.rs @@ -4,7 +4,12 @@ use axelar_wasm_std::{nonempty, Threshold}; use connection_router::msg::Message; use cosmwasm_std::{Addr, HexBinary, Uint256, Uint64}; -use multisig::types::ECDSASignature; +use multisig::{ + msg::Signer, + types::{ECDSAPublicKey, ECDSASignature, PublicKey}, +}; + +use crate::state::WorkerSet; fn legacy_cmd_id_input( source_transaction: HexBinary, @@ -27,6 +32,65 @@ fn legacy_cmd_id_input( unsafe { String::from_utf8_unchecked(data) } } +pub fn new_worker_set() -> WorkerSet { + WorkerSet { + signers: vec![ + Signer { + address: Addr::unchecked("axelarvaloper1x86a8prx97ekkqej2x636utrdu23y8wupp9gk5"), + weight: Uint256::from(10u128), + pub_key: PublicKey::ECDSA(ECDSAPublicKey::unchecked( + HexBinary::from_hex( + "03d123ce370b163acd576be0e32e436bb7e63262769881d35fa3573943bf6c6f81", + ) + .unwrap(), + )), + }, + Signer { + address: Addr::unchecked("axelarvaloper1ff675m593vve8yh82lzhdnqfpu7m23cxstr6h4"), + weight: Uint256::from(10u128), + pub_key: PublicKey::ECDSA(ECDSAPublicKey::unchecked( + HexBinary::from_hex( + "03c6ddb0fcee7b528da1ef3c9eed8d51eeacd7cc28a8baa25c33037c5562faa6e4", + ) + .unwrap(), + )), + }, + Signer { + address: Addr::unchecked("axelarvaloper12cwre2gdhyytc3p97z9autzg27hmu4gfzz4rxc"), + weight: Uint256::from(10u128), + pub_key: PublicKey::ECDSA(ECDSAPublicKey::unchecked( + HexBinary::from_hex( + "0274b5d2a4c55d7edbbf9cc210c4d25adbb6194d6b444816235c82984bee518255", + ) + .unwrap(), + )), + }, + Signer { + address: Addr::unchecked("axelarvaloper1vs9rdplntrf7ceqdkznjmanrr59qcpjq6le0yw"), + weight: Uint256::from(10u128), + pub_key: PublicKey::ECDSA(ECDSAPublicKey::unchecked( + HexBinary::from_hex( + "02a670f57de55b8b39b4cb051e178ca8fb3fe3a78cdde7f8238baf5e6ce1893185", + ) + .unwrap(), + )), + }, + Signer { + address: Addr::unchecked("axelarvaloper1hz0slkejw96dukw87fztjkvwjdpcu20jewg6mw"), + weight: Uint256::from(10u128), + pub_key: PublicKey::ECDSA(ECDSAPublicKey::unchecked( + HexBinary::from_hex( + "028584592624e742ba154c02df4c0b06e4e8a957ba081083ea9fe5309492aa6c7b", + ) + .unwrap(), + )), + }, + ], + threshold: Uint256::from(30u128), + id: 1, + } +} + pub fn messages() -> Vec { vec![ Message { @@ -85,6 +149,14 @@ pub fn evm_address() -> HexBinary { HexBinary::from_hex("01212E8f3996651D6978147E76aA1f36C34b6556").unwrap() } +pub fn encoded_data_with_operator_transfer() -> HexBinary { + HexBinary::from_hex("0000000000000000000000000000000000000000000000000000000000000539000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000c00000000000000000000000000000000000000000000000000000000000000140000000000000000000000000000000000000000000000000000000000000000134e64bdf8c193ec13ccab8272cb202f77da188598236de9b6a3b6b8793af9b650000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000147472616e736665724f70657261746f72736869700000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000001e000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000001e00000000000000000000000000000000000000000000000000000000000000050000000000000000000000000249c31dd0eacb2d73bbe5a0513416cd3888cb5b0000000000000000000000001ae3758c032ae8ebf6f075bb5b6ff6129b56e632000000000000000000000000adb32b50b13f962d302619111de6a1020fbd55f7000000000000000000000000defab04334a82fdea683bca3617c33bc469d4cc9000000000000000000000000e6857cf86038ba741e64ce0d3c883a26a7d3cb460000000000000000000000000000000000000000000000000000000000000005000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a").unwrap() +} + +pub fn chain_id_operator_transfer() -> Uint256 { + Uint256::from(1337u128) +} + pub fn encoded_data() -> HexBinary { HexBinary::from_hex( "0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000e000000000000000000000000000000000000000000000000000000000000001c00000000000000000000000000000000000000000000000000000000000000002cdf61b5aa2024f5a27383b0785fc393c566eef69569cf5abec945794b097bb734ddf46ac2855e6614da7c654d224a58ad9eb9f567c45432c5120aa83a772a1e50000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000013617070726f7665436f6e747261637443616c6c000000000000000000000000000000000000000000000000000000000000000000000000000000000000000013617070726f7665436f6e747261637443616c6c000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000001c0000000000000000000000000000000000000000000000000000000000000016000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000000000010000000000000000000000000005a8aa0ed1e1bc598c23b415f67cd774b530546cdf0e679e57348329e51e4337b7839882c29f21a3095a718c239f147b143f9ff8c8a0024fa264d538986271bdf8d2901c443321faa33440b9f28e38ea28e6141f00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000007506f6c79676f6e00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002a30783636343233613162343565313445614238423133323636354665624337456338364266634246343400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000016000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000000000010000000000000000000000000005a8aa0ed1e1bc598c23b415f67cd774b530546cd8f619df9786ea29e466d37c846576a49080089909bf228c19458739606341a5e7a7263a63ac449b4c6ce2a93accfae9ae49c1d96e3fa9c19cc417130bcfda2200000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000007506f6c79676f6e00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002a30783636343233613162343565313445614238423133323636354665624337456338364266634246343400000000000000000000000000000000000000000000").unwrap() diff --git a/contracts/multisig-prover/src/types.rs b/contracts/multisig-prover/src/types.rs index 9731ba5ab..2a70ebb3f 100644 --- a/contracts/multisig-prover/src/types.rs +++ b/contracts/multisig-prover/src/types.rs @@ -6,17 +6,19 @@ use cw_storage_plus::{Key, KeyDeserialize, PrimaryKey}; use multisig::types::ECDSASignature; use sha3::{Digest, Keccak256}; -use crate::encoding::Data; +use crate::{encoding::Data, state::WorkerSet}; #[cw_serde] pub enum CommandType { ApproveContractCall, + TransferOperatorship, } impl Display for CommandType { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { CommandType::ApproveContractCall => write!(f, "approveContractCall"), + CommandType::TransferOperatorship => write!(f, "transferOperatorship"), } } } @@ -63,10 +65,13 @@ impl KeyDeserialize for BatchID { } impl BatchID { - pub fn new(message_ids: &[String]) -> BatchID { + pub fn new(message_ids: &[String], new_worker_set: Option) -> BatchID { let mut message_ids = message_ids.to_vec(); message_ids.sort(); + if let Some(new_worker_set) = new_worker_set { + message_ids.push(new_worker_set.hash().to_string()) + } Keccak256::digest(message_ids.join(",")).as_slice().into() } } diff --git a/contracts/multisig/src/contract.rs b/contracts/multisig/src/contract.rs index 156ac7b8f..6324288df 100644 --- a/contracts/multisig/src/contract.rs +++ b/contracts/multisig/src/contract.rs @@ -232,7 +232,12 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { } pub mod query { - use crate::{msg::Signer, state::PUB_KEYS, types::KeyType, types::PublicKey}; + use crate::{ + msg::Signer, + state::PUB_KEYS, + types::PublicKey, + types::{KeyType, Signature}, + }; use super::*; @@ -251,14 +256,16 @@ pub mod query { .remove(&address) .expect("violated invariant: pub_key not found"); - Signer { - address: participant.address, - weight: participant.weight.into(), - pub_key, - signature: session.signatures.get(&address).cloned(), - } + ( + Signer { + address: participant.address, + weight: participant.weight.into(), + pub_key, + }, + session.signatures.get(&address).cloned(), + ) }) - .collect::>(); + .collect::)>>(); Ok(Multisig { state: session.state, @@ -675,12 +682,12 @@ mod tests { let signer = query_res .signers .iter() - .find(|signer| signer.address == participant.address) + .find(|signer| signer.0.address == participant.address) .unwrap(); - assert_eq!(signer.weight, Uint256::from(participant.weight)); - assert_eq!(signer.pub_key, key.pub_keys.get(address).unwrap().clone()); - assert_eq!(signer.signature, session.signatures.get(address).cloned()); + 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] diff --git a/contracts/multisig/src/msg.rs b/contracts/multisig/src/msg.rs index d160ef69d..55adc0325 100644 --- a/contracts/multisig/src/msg.rs +++ b/contracts/multisig/src/msg.rs @@ -51,12 +51,11 @@ pub struct Signer { pub address: Addr, pub weight: Uint256, pub pub_key: PublicKey, - pub signature: Option, } #[cw_serde] pub struct Multisig { pub state: MultisigState, pub quorum: Uint256, - pub signers: Vec, + pub signers: Vec<(Signer, Option)>, } From 0a153f02276204804e885d8442312e26fa3476ab Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Fri, 1 Sep 2023 10:46:08 -0400 Subject: [PATCH 05/14] address review --- ampd/src/handlers/multisig.rs | 21 ++- contracts/multisig-prover/src/encoding/evm.rs | 29 ++- contracts/multisig-prover/src/execute.rs | 4 +- .../src/test/mocks/multisig.rs | 10 +- .../multisig-prover/src/test/test_data.rs | 10 +- contracts/multisig-prover/src/types.rs | 4 +- contracts/multisig/src/contract.rs | 33 ++-- contracts/multisig/src/error.rs | 3 + contracts/multisig/src/events.rs | 5 +- contracts/multisig/src/key.rs | 172 ++++++++++++++++++ contracts/multisig/src/lib.rs | 1 + contracts/multisig/src/msg.rs | 5 +- contracts/multisig/src/secp256k1.rs | 84 +++------ contracts/multisig/src/signing.rs | 21 +-- contracts/multisig/src/state.rs | 4 +- contracts/multisig/src/test/common.rs | 7 +- contracts/multisig/src/types.rs | 139 +------------- 17 files changed, 282 insertions(+), 270 deletions(-) create mode 100644 contracts/multisig/src/key.rs diff --git a/ampd/src/handlers/multisig.rs b/ampd/src/handlers/multisig.rs index 9d9f2dc68..c0dd5384c 100644 --- a/ampd/src/handlers/multisig.rs +++ b/ampd/src/handlers/multisig.rs @@ -32,15 +32,15 @@ fn deserialize_public_keys<'de, D>( where D: Deserializer<'de>, { - let keys_by_address: HashMap = + let keys_by_address: HashMap = HashMap::deserialize(deserializer)?; keys_by_address .into_iter() .map(|(address, pk)| match pk { - multisig::types::PublicKey::ECDSA(hex) => Ok(( + multisig::key::PublicKey::Ecdsa(hex) => Ok(( address, - VerifyingKey::from_sec1_bytes((&hex).into()) + VerifyingKey::from_sec1_bytes(hex.as_ref()) .map_err(D::Error::custom)? .into(), )), @@ -60,7 +60,8 @@ mod test { use tendermint::abci; use multisig::events::Event::SigningStarted; - use multisig::types::{ECDSAPublicKey, MsgToSign}; + use multisig::key::PublicKey; + use multisig::types::MsgToSign; use crate::broadcaster::key::ECDSASigningKey; @@ -70,10 +71,10 @@ mod test { ECDSASigningKey::random().address().to_string() } - fn rand_public_key() -> multisig::types::PublicKey { - multisig::types::PublicKey::ECDSA(ECDSAPublicKey::unchecked(HexBinary::from( + fn rand_public_key() -> multisig::key::PublicKey { + multisig::key::PublicKey::Ecdsa(HexBinary::from( ECDSASigningKey::random().public_key().to_bytes(), - ))) + )) } fn rand_message() -> HexBinary { @@ -84,7 +85,7 @@ mod test { fn signing_started_event() -> events::Event { let pub_keys = (0..10) .map(|_| (rand_account(), rand_public_key())) - .collect::>(); + .collect::>(); let poll_started = SigningStarted { session_id: Uint64::one(), @@ -140,10 +141,10 @@ mod test { let mut event = signing_started_event(); let invalid_pub_key: [u8; 32] = rand::random(); - let mut map: HashMap = HashMap::new(); + let mut map: HashMap = HashMap::new(); map.insert( rand_account(), - ECDSAPublicKey::unchecked(HexBinary::from(invalid_pub_key.as_slice())), + PublicKey::Ecdsa(HexBinary::from(invalid_pub_key.as_slice())), ); match event { events::Event::Abci { diff --git a/contracts/multisig-prover/src/encoding/evm.rs b/contracts/multisig-prover/src/encoding/evm.rs index 91f789d20..d64540fb9 100644 --- a/contracts/multisig-prover/src/encoding/evm.rs +++ b/contracts/multisig-prover/src/encoding/evm.rs @@ -7,7 +7,7 @@ use k256::{elliptic_curve::sec1::ToEncodedPoint, PublicKey}; use sha3::{Digest, Keccak256}; use connection_router::msg::Message; -use multisig::{msg::Signer, types::Signature}; +use multisig::msg::Signer; use crate::{ error::ContractError, @@ -49,9 +49,9 @@ impl TryFrom for Operator { fn try_from(signer: Signer) -> Result { Ok(Self { - address: evm_address((&signer.pub_key).into())?, + address: evm_address(signer.pub_key.as_ref())?, weight: signer.weight, - signature: signer.signature.map(|Signature::ECDSA(sig)| sig), + signature: signer.signature, }) } } @@ -131,7 +131,7 @@ impl CommandBatch { ))); if let Some(signature) = &operator.signature { - signatures.push(Token::Bytes(signature.into())); + signatures.push(Token::Bytes(>::from(signature.clone()))); } (addresses, weights, signatures) @@ -227,7 +227,6 @@ fn command_params( #[cfg(test)] mod test { use ethabi::ParamType; - use multisig::types::PublicKey; use crate::test::test_data; @@ -408,8 +407,8 @@ mod test { .map(|op| Signer { address: op.address, weight: op.weight.into(), - pub_key: PublicKey::ECDSA(op.pub_key), - signature: op.signature.map(Signature::ECDSA), + pub_key: op.pub_key, + signature: op.signature, }) .collect::>(); @@ -482,8 +481,8 @@ mod test { .map(|op| Signer { address: op.address, weight: op.weight.into(), - pub_key: PublicKey::ECDSA(op.pub_key), - signature: op.signature.map(Signature::ECDSA), + pub_key: op.pub_key, + signature: op.signature, }) .collect::>(); @@ -552,20 +551,20 @@ mod test { Signer { address: operator2.address, weight: operator2.weight, - pub_key: PublicKey::ECDSA(operator2.pub_key), - signature: operator2.signature.map(Signature::ECDSA), + pub_key: operator2.pub_key, + signature: operator2.signature, }, Signer { address: operator1.address, weight: operator1.weight, - pub_key: PublicKey::ECDSA(operator1.pub_key), - signature: operator1.signature.map(Signature::ECDSA), + pub_key: operator1.pub_key, + signature: operator1.signature, }, Signer { address: operator3.address, weight: operator3.weight, - pub_key: PublicKey::ECDSA(operator3.pub_key), - signature: operator3.signature.map(Signature::ECDSA), + pub_key: operator3.pub_key, + signature: operator3.signature, }, ]; diff --git a/contracts/multisig-prover/src/execute.rs b/contracts/multisig-prover/src/execute.rs index ee8316002..1943e6e5b 100644 --- a/contracts/multisig-prover/src/execute.rs +++ b/contracts/multisig-prover/src/execute.rs @@ -2,7 +2,7 @@ use cosmwasm_std::{ to_binary, wasm_execute, Addr, BlockInfo, DepsMut, Env, HexBinary, QuerierWrapper, QueryRequest, Response, SubMsg, WasmMsg, WasmQuery, }; -use multisig::types::KeyType; +use multisig::key::KeyType; use std::{collections::HashMap, str::FromStr}; @@ -99,7 +99,7 @@ pub fn rotate_snapshot( pub_keys: pub_keys .clone() .into_iter() - .map(|(k, v)| (k, (KeyType::ECDSA, v))) + .map(|(k, v)| (k, (KeyType::Ecdsa, v))) .collect(), })?, funds: vec![], diff --git a/contracts/multisig-prover/src/test/mocks/multisig.rs b/contracts/multisig-prover/src/test/mocks/multisig.rs index 9a1b3a421..cf99cf96a 100644 --- a/contracts/multisig-prover/src/test/mocks/multisig.rs +++ b/contracts/multisig-prover/src/test/mocks/multisig.rs @@ -53,7 +53,9 @@ pub fn query(_deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { } mod query { - use multisig::msg::{Multisig, Signer}; + use multisig::{ + msg::{Multisig, Signer}, + }; use crate::test::test_data; @@ -68,10 +70,8 @@ mod query { .map(|op| Signer { address: op.address, weight: op.weight.into(), - pub_key: multisig::types::PublicKey::ECDSA(op.pub_key), - signature: op - .signature - .map(|sig| multisig::types::Signature::ECDSA(sig)), + pub_key: op.pub_key, + signature: op.signature, }) .collect::>(); diff --git a/contracts/multisig-prover/src/test/test_data.rs b/contracts/multisig-prover/src/test/test_data.rs index c90d71492..250d0088d 100644 --- a/contracts/multisig-prover/src/test/test_data.rs +++ b/contracts/multisig-prover/src/test/test_data.rs @@ -4,7 +4,7 @@ use axelar_wasm_std::{nonempty, Threshold}; use connection_router::msg::Message; use cosmwasm_std::{Addr, HexBinary, Uint256, Uint64}; -use multisig::types::ECDSASignature; +use multisig::key::{KeyType, Signature}; fn legacy_cmd_id_input( source_transaction: HexBinary, @@ -111,10 +111,10 @@ pub fn threshold() -> Threshold { #[derive(Debug)] pub struct TestOperator { pub address: Addr, - pub pub_key: multisig::types::ECDSAPublicKey, + pub pub_key: multisig::key::PublicKey, pub operator: HexBinary, pub weight: Uint256, - pub signature: Option, + pub signature: Option, } pub fn operators() -> Vec { @@ -194,10 +194,10 @@ pub fn operators() -> Vec { .map(|(address, pub_key, operator, weight, signature)| { TestOperator { address: Addr::unchecked(address), - pub_key: HexBinary::from_hex(pub_key).unwrap().try_into().unwrap(), + pub_key: (KeyType::Ecdsa,HexBinary::from_hex(pub_key).unwrap()).try_into().unwrap(), operator: HexBinary::from_hex(operator).unwrap(), weight: Uint256::from(weight), - signature: signature.map(|s| HexBinary::from_hex(s).unwrap().try_into().unwrap()) } + signature: signature.map(|s| (KeyType::Ecdsa, HexBinary::from_hex(s).unwrap()).try_into().unwrap()) } }) .collect() } diff --git a/contracts/multisig-prover/src/types.rs b/contracts/multisig-prover/src/types.rs index 9731ba5ab..d4c91a892 100644 --- a/contracts/multisig-prover/src/types.rs +++ b/contracts/multisig-prover/src/types.rs @@ -3,7 +3,7 @@ use std::fmt::Display; use cosmwasm_schema::cw_serde; use cosmwasm_std::{from_binary, HexBinary, StdResult, Uint256}; use cw_storage_plus::{Key, KeyDeserialize, PrimaryKey}; -use multisig::types::ECDSASignature; +use multisig::key::Signature; use sha3::{Digest, Keccak256}; use crate::encoding::Data; @@ -83,5 +83,5 @@ pub struct CommandBatch { pub struct Operator { pub address: HexBinary, pub weight: Uint256, - pub signature: Option, + pub signature: Option, } diff --git a/contracts/multisig/src/contract.rs b/contracts/multisig/src/contract.rs index 156ac7b8f..4157e145b 100644 --- a/contracts/multisig/src/contract.rs +++ b/contracts/multisig/src/contract.rs @@ -57,9 +57,9 @@ pub fn execute( pub mod execute { use crate::{ + key::{KeyType, KeyTyped, PublicKey, Signature}, signing::SigningSession, state::PUB_KEYS, - types::{KeyType, PublicKey, Signature}, }; use super::*; @@ -122,7 +122,7 @@ pub mod execute { signer: info.sender.into(), }) } - Some((_, pk)) => (pk.clone(), signature).try_into()?, + Some((_, pk)) => (pk.key_type(), signature).try_into()?, }; session.add_signature(key, info.sender.clone().into(), signature.clone())?; @@ -232,7 +232,11 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { } pub mod query { - use crate::{msg::Signer, state::PUB_KEYS, types::KeyType, types::PublicKey}; + use crate::{ + key::{KeyType, PublicKey}, + msg::Signer, + state::PUB_KEYS, + }; use super::*; @@ -281,10 +285,11 @@ mod tests { use std::vec; use crate::{ + key::{KeyType, PublicKey, Signature}, msg::Multisig, test::common::test_data, test::common::{build_snapshot, TestSigner}, - types::{ECDSASignature, KeyType, MultisigState, PublicKey, Signature}, + types::MultisigState, }; use super::*; @@ -318,7 +323,7 @@ mod tests { .map(|signer| { ( signer.address.clone().to_string(), - (KeyType::ECDSA, signer.pub_key.clone()), + (KeyType::Ecdsa, signer.pub_key.clone()), ) }) .collect::>(); @@ -569,7 +574,7 @@ mod tests { .signatures .get(&signer.address.clone().into_string()) .unwrap(), - &Signature::ECDSA(ECDSASignature::try_from(signer.signature.clone()).unwrap()) + &Signature::try_from((KeyType::Ecdsa, signer.signature.clone())).unwrap() ); assert_eq!(session.state, MultisigState::Pending); @@ -616,7 +621,7 @@ mod tests { .signatures .get(&signer.address.into_string()) .unwrap(), - &Signature::ECDSA(ECDSASignature::try_from(signer.signature).unwrap()) + &Signature::try_from((KeyType::Ecdsa, signer.signature)).unwrap() ); assert_eq!(session.state, MultisigState::Completed); @@ -694,20 +699,20 @@ mod tests { .collect::>(); for (addr, pub_key) in &pub_keys { - let res = do_register_key(deps.as_mut(), addr.clone(), pub_key.clone(), KeyType::ECDSA); + let res = do_register_key(deps.as_mut(), addr.clone(), pub_key.clone(), KeyType::Ecdsa); 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); + let res = query_registered_public_key(deps.as_ref(), addr.clone(), KeyType::Ecdsa); 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()) + .map(|(_, pk)| PublicKey::try_from((KeyType::Ecdsa, pk)).unwrap()) .collect::>(), ret_pub_keys ); @@ -724,7 +729,7 @@ mod tests { .collect::>(); for (addr, pub_key) in &pub_keys { - let res = do_register_key(deps.as_mut(), addr.clone(), pub_key.clone(), KeyType::ECDSA); + let res = do_register_key(deps.as_mut(), addr.clone(), pub_key.clone(), KeyType::Ecdsa); assert!(res.is_ok()); } @@ -736,14 +741,14 @@ mod tests { deps.as_mut(), pub_keys[0].0.clone(), new_pub_key.clone(), - KeyType::ECDSA, + KeyType::Ecdsa, ); assert!(res.is_ok()); - let res = query_registered_public_key(deps.as_ref(), pub_keys[0].0.clone(), KeyType::ECDSA); + 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)).unwrap(), + PublicKey::try_from((KeyType::Ecdsa, new_pub_key)).unwrap(), from_binary::(&res.unwrap()).unwrap() ); } diff --git a/contracts/multisig/src/error.rs b/contracts/multisig/src/error.rs index d3cf35d8b..34d140a14 100644 --- a/contracts/multisig/src/error.rs +++ b/contracts/multisig/src/error.rs @@ -44,4 +44,7 @@ pub enum ContractError { #[error("missing public key for participant {participant}")] MissingPublicKey { participant: String }, + + #[error("key type mismatch")] + KeyTypeMismatch, } diff --git a/contracts/multisig/src/events.rs b/contracts/multisig/src/events.rs index 900412a71..f7b6727d5 100644 --- a/contracts/multisig/src/events.rs +++ b/contracts/multisig/src/events.rs @@ -3,7 +3,10 @@ use std::collections::HashMap; use cosmwasm_std::{Addr, HexBinary, Uint64}; use serde_json::to_string; -use crate::types::{KeyID, KeyType, MsgToSign, PublicKey, Signature}; +use crate::{ + key::{KeyType, PublicKey, Signature}, + types::{KeyID, MsgToSign}, +}; pub enum Event { // Emitted when a new signing session is open diff --git a/contracts/multisig/src/key.rs b/contracts/multisig/src/key.rs new file mode 100644 index 000000000..2788ea568 --- /dev/null +++ b/contracts/multisig/src/key.rs @@ -0,0 +1,172 @@ +use cosmwasm_schema::cw_serde; +use cosmwasm_std::{HexBinary, StdError, StdResult}; +use cw_storage_plus::{KeyDeserialize, PrimaryKey}; + +use crate::{ + secp256k1::ecdsa_verify, + types::{MsgToSign, VerifiableSignature}, + ContractError, +}; + +#[cw_serde] +pub enum KeyType { + Ecdsa, +} + +#[cw_serde] +#[derive(PartialOrd, Ord, Eq)] +pub enum Signature { + Ecdsa(HexBinary), +} + +#[cw_serde] +pub enum PublicKey { + Ecdsa(HexBinary), +} + +pub trait KeyTyped { + fn matches(&self, other: &T) -> bool + where + T: KeyTyped, + { + self.key_type() == other.key_type() + } + + fn key_type(&self) -> KeyType; +} + +impl KeyTyped for PublicKey { + fn key_type(&self) -> KeyType { + match self { + PublicKey::Ecdsa(_) => KeyType::Ecdsa, + } + } +} + +impl KeyTyped for Signature { + fn key_type(&self) -> KeyType { + match self { + Signature::Ecdsa(_) => KeyType::Ecdsa, + } + } +} + +impl VerifiableSignature for Signature { + fn verify(&self, msg: &MsgToSign, pub_key: &PublicKey) -> Result { + if !self.matches(pub_key) { + 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()) + } + } + } +} + +impl<'a> PrimaryKey<'a> for KeyType { + type Prefix = (); + type SubPrefix = (); + type Suffix = Self; + type SuperSuffix = Self; + + fn key(&self) -> Vec { + vec![cw_storage_plus::Key::Val8( + vec![self.clone() as u8] + .try_into() + .expect("failed to serialize key type"), + )] + } +} + +impl KeyDeserialize for KeyType { + type Output = Self; + + fn from_vec(value: Vec) -> StdResult { + serde_json::from_slice(value.as_slice()).map_err(|err| StdError::ParseErr { + target_type: "KeyType".into(), + msg: err.to_string(), + }) + } +} + +const ECDSA_COMPRESSED_PUBKEY_LEN: usize = 33; +const ECDSA_UNCOMPRESSED_PUBKEY_LEN: usize = 65; +const EVM_SIGNATURE_LEN: usize = 65; + +impl TryFrom<(KeyType, HexBinary)> for PublicKey { + type Error = ContractError; + + fn try_from((key_type, pub_key): (KeyType, HexBinary)) -> Result { + match key_type { + KeyType::Ecdsa => { + if pub_key.len() != ECDSA_COMPRESSED_PUBKEY_LEN + && pub_key.len() != ECDSA_UNCOMPRESSED_PUBKEY_LEN + { + return Err(ContractError::InvalidPublicKeyFormat { + reason: "Invalid input length".into(), + }); + } + Ok(PublicKey::Ecdsa(pub_key)) + } + } + } +} + +impl TryFrom<(KeyType, HexBinary)> for Signature { + type Error = ContractError; + + fn try_from((key_type, sig): (KeyType, HexBinary)) -> Result { + match key_type { + KeyType::Ecdsa => { + if sig.len() != EVM_SIGNATURE_LEN { + return Err(ContractError::InvalidSignatureFormat { + reason: "Invalid input length".into(), + }); + } + Ok(Signature::Ecdsa(sig)) + } + } + } +} + +impl AsRef<[u8]> for PublicKey { + fn as_ref(&self) -> &[u8] { + match self { + PublicKey::Ecdsa(pk) => pk.as_ref(), + } + } +} + +impl AsRef<[u8]> for Signature { + fn as_ref(&self) -> &[u8] { + match self { + Signature::Ecdsa(sig) => sig.as_ref(), + } + } +} + +impl From for Vec { + fn from(value: Signature) -> Vec { + match value { + Signature::Ecdsa(sig) => sig.to_vec(), + } + } +} + +impl From for HexBinary { + fn from(original: Signature) -> Self { + match original { + Signature::Ecdsa(sig) => sig, + } + } +} + +impl From for HexBinary { + fn from(original: PublicKey) -> Self { + match original { + PublicKey::Ecdsa(sig) => sig, + } + } +} diff --git a/contracts/multisig/src/lib.rs b/contracts/multisig/src/lib.rs index 82c6c5c5b..298ed9b4f 100644 --- a/contracts/multisig/src/lib.rs +++ b/contracts/multisig/src/lib.rs @@ -1,6 +1,7 @@ pub mod contract; pub mod error; pub mod events; +pub mod key; pub mod msg; pub mod signing; pub mod state; diff --git a/contracts/multisig/src/msg.rs b/contracts/multisig/src/msg.rs index d160ef69d..c76b0ab0c 100644 --- a/contracts/multisig/src/msg.rs +++ b/contracts/multisig/src/msg.rs @@ -4,7 +4,10 @@ use axelar_wasm_std::Snapshot; use cosmwasm_schema::{cw_serde, QueryResponses}; use cosmwasm_std::{Addr, HexBinary, Uint256, Uint64}; -use crate::types::{Key, KeyID, KeyType, MultisigState, PublicKey, Signature}; +use crate::{ + key::{KeyType, PublicKey, Signature}, + types::{Key, KeyID, MultisigState}, +}; #[cw_serde] pub struct InstantiateMsg {} diff --git a/contracts/multisig/src/secp256k1.rs b/contracts/multisig/src/secp256k1.rs index 457026377..4faaabf15 100644 --- a/contracts/multisig/src/secp256k1.rs +++ b/contracts/multisig/src/secp256k1.rs @@ -2,31 +2,11 @@ use cosmwasm_crypto::secp256k1_verify; use cosmwasm_std::HexBinary; // TODO: Logic specific to secp256k1 will most likely be handled by core in the future. -use crate::{ - types::{ECDSAPublicKey, ECDSASignature, MsgToSign}, - ContractError, -}; +use crate::{types::MsgToSign, ContractError}; const MESSAGE_HASH_LEN: usize = 32; -const COMPRESSED_PUBKEY_LEN: usize = 33; -const UNCOMPRESSED_PUBKEY_LEN: usize = 65; -const EVM_SIGNATURE_LEN: usize = 65; const ECDSA_SIGNATURE_LEN: usize = 64; -impl TryFrom for ECDSAPublicKey { - type Error = ContractError; - - fn try_from(other: HexBinary) -> Result { - if other.len() != COMPRESSED_PUBKEY_LEN && other.len() != UNCOMPRESSED_PUBKEY_LEN { - return Err(ContractError::InvalidPublicKeyFormat { - reason: "Invalid input length".into(), - }); - } - - Ok(ECDSAPublicKey::unchecked(other)) - } -} - impl TryFrom for MsgToSign { type Error = ContractError; @@ -41,33 +21,11 @@ impl TryFrom for MsgToSign { } } -impl TryFrom for ECDSASignature { - type Error = ContractError; - - fn try_from(other: HexBinary) -> Result { - if other.len() != EVM_SIGNATURE_LEN { - return Err(ContractError::InvalidSignatureFormat { - reason: "Invalid input length".into(), - }); +pub fn ecdsa_verify(msg_hash: &[u8], sig: &[u8], pub_key: &[u8]) -> Result { + secp256k1_verify(msg_hash, &sig[0..ECDSA_SIGNATURE_LEN], pub_key).map_err(|e| { + ContractError::SignatureVerificationFailed { + reason: e.to_string(), } - - Ok(ECDSASignature::unchecked(other)) - } -} - -pub fn ecdsa_verify( - msg: &MsgToSign, - sig: &ECDSASignature, - pub_key: &ECDSAPublicKey, -) -> Result { - let signature: &[u8] = sig.into(); - secp256k1_verify( - msg.into(), - &signature[0..ECDSA_SIGNATURE_LEN], - pub_key.into(), - ) - .map_err(|e| ContractError::SignatureVerificationFailed { - reason: e.to_string(), }) } @@ -75,12 +33,16 @@ pub fn ecdsa_verify( mod tests { use super::*; - use crate::test::common::test_data; + use crate::{ + key::{KeyType, PublicKey, Signature}, + test::common::test_data, + types::VerifiableSignature, + }; #[test] fn test_try_from_hexbinary_to_ecdsa_public_key() { let hex = test_data::pub_key(); - let pub_key = ECDSAPublicKey::try_from(hex.clone()).unwrap(); + let pub_key = PublicKey::try_from((KeyType::Ecdsa, hex.clone())).unwrap(); assert_eq!(HexBinary::from(pub_key), hex); } @@ -88,7 +50,7 @@ mod tests { fn test_try_from_hexbinary_to_eccdsa_public_key_fails() { let hex = HexBinary::from_hex("049b").unwrap(); assert_eq!( - ECDSAPublicKey::try_from(hex.clone()).unwrap_err(), + PublicKey::try_from((KeyType::Ecdsa, hex.clone())).unwrap_err(), ContractError::InvalidPublicKeyFormat { reason: "Invalid input length".into() } @@ -116,7 +78,7 @@ mod tests { #[test] fn test_try_from_hexbinary_to_signature() { let hex = test_data::signature(); - let signature = ECDSASignature::try_from(hex.clone()).unwrap(); + let signature = Signature::try_from((KeyType::Ecdsa, hex.clone())).unwrap(); assert_eq!(HexBinary::from(signature), hex); } @@ -126,7 +88,7 @@ mod tests { HexBinary::from_hex("283786d844a7c4d1d424837074d0c8ec71becdcba4dd42b5307cb543a0e2c8b81c10ad541defd5ce84d2a608fc454827d0b65b4865c8192a2ea1736a5c4b72") .unwrap(); assert_eq!( - ECDSASignature::try_from(hex.clone()).unwrap_err(), + Signature::try_from((KeyType::Ecdsa, hex.clone())).unwrap_err(), ContractError::InvalidSignatureFormat { reason: "Invalid input length".into() } @@ -135,10 +97,10 @@ mod tests { #[test] fn test_verify_signature() { - let signature = ECDSASignature::try_from(test_data::signature()).unwrap(); + let signature = Signature::try_from((KeyType::Ecdsa, test_data::signature())).unwrap(); let message = MsgToSign::try_from(test_data::message()).unwrap(); - let public_key = ECDSAPublicKey::try_from(test_data::pub_key()).unwrap(); - let result = ecdsa_verify(&message, &signature, &public_key).unwrap(); + let public_key = PublicKey::try_from((KeyType::Ecdsa, test_data::pub_key())).unwrap(); + let result = signature.verify(&message, &public_key).unwrap(); assert_eq!(result, true); } @@ -149,10 +111,10 @@ mod tests { ) .unwrap(); - let signature = ECDSASignature::try_from(invalid_signature).unwrap(); + let signature = Signature::try_from((KeyType::Ecdsa, invalid_signature)).unwrap(); let message = MsgToSign::try_from(test_data::message()).unwrap(); - let public_key = ECDSAPublicKey::try_from(test_data::pub_key()).unwrap(); - let result = ecdsa_verify(&message, &signature, &public_key).unwrap(); + let public_key = PublicKey::try_from((KeyType::Ecdsa, test_data::pub_key())).unwrap(); + let result = signature.verify(&message, &public_key).unwrap(); assert_eq!(result, false); } @@ -163,10 +125,10 @@ mod tests { ) .unwrap(); - let signature = ECDSASignature::try_from(test_data::signature()).unwrap(); + let signature = Signature::try_from((KeyType::Ecdsa, test_data::signature())).unwrap(); let message = MsgToSign::try_from(test_data::message()).unwrap(); - let public_key = ECDSAPublicKey::try_from(invalid_pub_key).unwrap(); - let result = ecdsa_verify(&message, &signature, &public_key).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); } } diff --git a/contracts/multisig/src/signing.rs b/contracts/multisig/src/signing.rs index 0004c1ec1..00407ee0f 100644 --- a/contracts/multisig/src/signing.rs +++ b/contracts/multisig/src/signing.rs @@ -6,7 +6,8 @@ use cosmwasm_std::{Uint256, Uint64}; use axelar_wasm_std::Snapshot; use crate::{ - types::{Key, KeyID, MsgToSign, MultisigState, Signature, VerifiableSignature}, + key::Signature, + types::{Key, KeyID, MsgToSign, MultisigState, VerifiableSignature}, ContractError, }; @@ -101,10 +102,10 @@ mod tests { use cosmwasm_std::{testing::MockStorage, Addr, HexBinary}; use crate::{ + key::KeyType, state::{KEYS, SIGNING_SESSIONS}, test::common::test_data, test::common::{build_key, build_snapshot, TestSigner}, - types::{ECDSASignature, KeyType}, }; use super::*; @@ -151,11 +152,7 @@ mod tests { let res = session.add_signature( key, signer.address.clone().into_string(), - Signature::try_from(( - (KeyType::ECDSA, signer.pub_key).try_into().unwrap(), - signer.signature.clone(), - )) - .unwrap(), + Signature::try_from((KeyType::Ecdsa, signer.signature.clone())).unwrap(), ); SIGNING_SESSIONS.save(&mut config.store, session.id.u64(), &session)?; @@ -225,15 +222,15 @@ mod tests { let mut session = SigningSession::new(Uint64::one(), config.key_id.clone(), config.message.clone()); - let invalid_sig: ECDSASignature = HexBinary::from_hex("a58c9543b9df54578ec45838948e19afb1c6e4c86b34d9899b10b44e619ea74e19b457611e41a047030ed233af437d7ecff84de97cb6b3c13d73d22874e035111c") - .unwrap().try_into().unwrap(); + 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(), - Signature::ECDSA(invalid_sig), + invalid_sig, ); assert_eq!( @@ -259,9 +256,7 @@ mod tests { let result = session.add_signature( key, invalid_participant.clone(), - Signature::ECDSA( - ECDSASignature::try_from(config.signers[0].signature.clone()).unwrap(), - ), + Signature::try_from((KeyType::Ecdsa, config.signers[0].signature.clone())).unwrap(), ); assert_eq!( diff --git a/contracts/multisig/src/state.rs b/contracts/multisig/src/state.rs index 47830b55d..f76ff2296 100644 --- a/contracts/multisig/src/state.rs +++ b/contracts/multisig/src/state.rs @@ -2,9 +2,9 @@ use cosmwasm_std::{Addr, Storage, Uint64}; use cw_storage_plus::{Item, Map}; use crate::{ + key::{KeyType, PublicKey}, signing::SigningSession, - types::KeyType, - types::{Key, KeyID, PublicKey}, + types::{Key, KeyID}, ContractError, }; diff --git a/contracts/multisig/src/test/common.rs b/contracts/multisig/src/test/common.rs index 5aa7647c2..1d46731d0 100644 --- a/contracts/multisig/src/test/common.rs +++ b/contracts/multisig/src/test/common.rs @@ -4,7 +4,10 @@ use axelar_wasm_std::{Participant, Snapshot, Threshold}; use cosmwasm_std::{Addr, HexBinary, Timestamp, Uint256, Uint64}; use rand::Rng; -use crate::types::{Key, KeyID, KeyType, PublicKey}; +use crate::{ + key::{KeyType, PublicKey}, + types::{Key, KeyID}, +}; #[derive(Clone)] pub struct TestSigner { @@ -77,7 +80,7 @@ pub fn build_key(key_id: KeyID, signers: &Vec, snapshot: Snapshot) - .map(|signer| { ( signer.address.clone().to_string(), - PublicKey::try_from((KeyType::ECDSA, signer.pub_key.clone())).unwrap(), + PublicKey::try_from((KeyType::Ecdsa, signer.pub_key.clone())).unwrap(), ) }) .collect::>(); diff --git a/contracts/multisig/src/types.rs b/contracts/multisig/src/types.rs index c5c509b09..00c057dd5 100644 --- a/contracts/multisig/src/types.rs +++ b/contracts/multisig/src/types.rs @@ -2,90 +2,15 @@ use std::{collections::HashMap, fmt}; use axelar_wasm_std::Snapshot; use cosmwasm_schema::cw_serde; -use cosmwasm_std::{from_binary, Addr, HexBinary, StdError, StdResult}; +use cosmwasm_std::{from_binary, Addr, HexBinary, StdResult}; use cw_storage_plus::{KeyDeserialize, PrimaryKey}; -use crate::{secp256k1::ecdsa_verify, ContractError}; +use crate::{key::PublicKey, ContractError}; pub trait VerifiableSignature { fn verify(&self, msg: &MsgToSign, pub_key: &PublicKey) -> Result; } -#[cw_serde] -pub enum KeyType { - ECDSA, -} - -impl<'a> PrimaryKey<'a> for KeyType { - type Prefix = (); - type SubPrefix = (); - type Suffix = Self; - type SuperSuffix = Self; - - fn key(&self) -> Vec { - vec![cw_storage_plus::Key::Val8( - vec![self.clone() as u8] - .try_into() - .expect("failed to serialize key type"), - )] - } -} - -impl KeyDeserialize for KeyType { - type Output = Self; - - fn from_vec(value: Vec) -> StdResult { - serde_json::from_slice(value.as_slice()).map_err(|err| StdError::ParseErr { - target_type: "KeyType".into(), - msg: err.to_string(), - }) - } -} - -#[cw_serde] -pub enum PublicKey { - ECDSA(ECDSAPublicKey), -} - -impl TryFrom<(KeyType, HexBinary)> for PublicKey { - type Error = ContractError; - - fn try_from((key_type, pub_key): (KeyType, HexBinary)) -> Result { - match key_type { - KeyType::ECDSA => ECDSAPublicKey::try_from(pub_key).map(PublicKey::ECDSA), - } - } -} - -impl<'a> From<&'a PublicKey> for &'a [u8] { - fn from(value: &'a PublicKey) -> Self { - match value { - PublicKey::ECDSA(pub_key) => pub_key.into(), - } - } -} - -#[cw_serde] -pub struct ECDSAPublicKey(HexBinary); - -impl From for HexBinary { - fn from(original: ECDSAPublicKey) -> Self { - original.0 - } -} - -impl<'a> From<&'a ECDSAPublicKey> for &'a [u8] { - fn from(original: &'a ECDSAPublicKey) -> Self { - original.0.as_slice() - } -} - -impl ECDSAPublicKey { - pub fn unchecked(hex: HexBinary) -> Self { - Self(hex) - } -} - #[cw_serde] pub struct MsgToSign(HexBinary); @@ -107,66 +32,6 @@ impl MsgToSign { } } -#[cw_serde] -#[derive(Ord, PartialOrd, Eq)] -pub enum Signature { - ECDSA(ECDSASignature), -} - -impl From for HexBinary { - fn from(value: Signature) -> Self { - match value { - Signature::ECDSA(sig) => sig.into(), - } - } -} - -impl TryFrom<(PublicKey, HexBinary)> for Signature { - type Error = ContractError; - - fn try_from((pk, sig): (PublicKey, HexBinary)) -> Result { - match pk { - PublicKey::ECDSA(_) => ECDSASignature::try_from(sig).map(Signature::ECDSA), - } - } -} - -#[cw_serde] -#[derive(Ord, PartialOrd, Eq)] -pub struct ECDSASignature(HexBinary); - -impl From for HexBinary { - fn from(original: ECDSASignature) -> Self { - original.0 - } -} - -impl From<&ECDSASignature> for Vec { - fn from(original: &ECDSASignature) -> Self { - original.0.to_vec() - } -} - -impl<'a> From<&'a ECDSASignature> for &'a [u8] { - fn from(original: &'a ECDSASignature) -> Self { - original.0.as_slice() - } -} - -impl ECDSASignature { - pub fn unchecked(hex: HexBinary) -> Self { - Self(hex) - } -} - -impl VerifiableSignature for Signature { - fn verify(&self, msg: &MsgToSign, pub_key: &PublicKey) -> Result { - match (self, pub_key) { - (Signature::ECDSA(sig), PublicKey::ECDSA(pub_key)) => ecdsa_verify(msg, sig, pub_key), - } - } -} - #[cw_serde] pub struct KeyID { pub owner: Addr, From b46cd9a2c41f86e5c3766bcd7071064eb0a45215 Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Fri, 1 Sep 2023 11:14:13 -0400 Subject: [PATCH 06/14] review --- .../src/test/mocks/multisig.rs | 4 +-- contracts/multisig/src/contract.rs | 12 +++---- contracts/multisig/src/events.rs | 31 +++++++------------ contracts/multisig/src/key.rs | 3 +- contracts/multisig/src/state.rs | 7 +++-- 5 files changed, 25 insertions(+), 32 deletions(-) diff --git a/contracts/multisig-prover/src/test/mocks/multisig.rs b/contracts/multisig-prover/src/test/mocks/multisig.rs index cf99cf96a..ff0932f5c 100644 --- a/contracts/multisig-prover/src/test/mocks/multisig.rs +++ b/contracts/multisig-prover/src/test/mocks/multisig.rs @@ -53,9 +53,7 @@ pub fn query(_deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { } mod query { - use multisig::{ - msg::{Multisig, Signer}, - }; + use multisig::msg::{Multisig, Signer}; use crate::test::test_data; diff --git a/contracts/multisig/src/contract.rs b/contracts/multisig/src/contract.rs index 4157e145b..20ed37669 100644 --- a/contracts/multisig/src/contract.rs +++ b/contracts/multisig/src/contract.rs @@ -114,7 +114,7 @@ pub mod execute { let signature: Signature = match key .pub_keys .iter() - .find(|pk| pk.0 == &info.sender.to_string()) + .find(|&(addr, _)| addr == &info.sender.to_string()) { None => { return Err(ContractError::NotAParticipant { @@ -197,18 +197,17 @@ pub mod execute { public_key: HexBinary, key_type: KeyType, ) -> Result { - let pub_key: PublicKey = (key_type.clone(), public_key).try_into()?; + let pub_key: PublicKey = (key_type, public_key).try_into()?; PUB_KEYS.save( deps.storage, - (info.sender.clone(), key_type.clone()), - &pub_key.clone(), + (info.sender.clone(), key_type), + &pub_key.clone().into(), )?; Ok(Response::new().add_event( Event::PublicKeyRegistered { worker: info.sender, public_key: pub_key, - key_type, } .into(), )) @@ -276,7 +275,8 @@ pub mod query { } pub fn get_public_key(deps: Deps, worker: Addr, key_type: KeyType) -> StdResult { - PUB_KEYS.load(deps.storage, (worker, key_type.clone())) + let raw = PUB_KEYS.load(deps.storage, (worker, key_type))?; + Ok(PublicKey::try_from((key_type, raw)).expect("could not decode pub key")) } } diff --git a/contracts/multisig/src/events.rs b/contracts/multisig/src/events.rs index f7b6727d5..7cde84210 100644 --- a/contracts/multisig/src/events.rs +++ b/contracts/multisig/src/events.rs @@ -4,7 +4,7 @@ use cosmwasm_std::{Addr, HexBinary, Uint64}; use serde_json::to_string; use crate::{ - key::{KeyType, PublicKey, Signature}, + key::{PublicKey, Signature}, types::{KeyID, MsgToSign}, }; @@ -29,7 +29,6 @@ pub enum Event { PublicKeyRegistered { worker: Addr, public_key: PublicKey, - key_type: KeyType, }, } @@ -63,23 +62,17 @@ impl From for cosmwasm_std::Event { .add_attribute("signature", HexBinary::from(signature).to_hex()), Event::SigningCompleted { session_id } => cosmwasm_std::Event::new("signing_completed") .add_attribute("session_id", session_id), - Event::PublicKeyRegistered { - worker, - public_key, - key_type, - } => cosmwasm_std::Event::new("public_key_registered") - .add_attribute( - "worker", - to_string(&worker).expect("failed to serialize worker"), - ) - .add_attribute( - "public_key", - to_string(&public_key).expect("failed to serialize public key"), - ) - .add_attribute( - "key_type", - to_string(&key_type).expect("failed to serialize key type"), - ), + Event::PublicKeyRegistered { worker, public_key } => { + cosmwasm_std::Event::new("public_key_registered") + .add_attribute( + "worker", + to_string(&worker).expect("failed to serialize worker"), + ) + .add_attribute( + "public_key", + to_string(&public_key).expect("failed to serialize public key"), + ) + } } } } diff --git a/contracts/multisig/src/key.rs b/contracts/multisig/src/key.rs index 2788ea568..f8fe48a1f 100644 --- a/contracts/multisig/src/key.rs +++ b/contracts/multisig/src/key.rs @@ -9,6 +9,7 @@ use crate::{ }; #[cw_serde] +#[derive(Copy)] pub enum KeyType { Ecdsa, } @@ -73,7 +74,7 @@ impl<'a> PrimaryKey<'a> for KeyType { fn key(&self) -> Vec { vec![cw_storage_plus::Key::Val8( - vec![self.clone() as u8] + vec![*self as u8] .try_into() .expect("failed to serialize key type"), )] diff --git a/contracts/multisig/src/state.rs b/contracts/multisig/src/state.rs index f76ff2296..d9b77834a 100644 --- a/contracts/multisig/src/state.rs +++ b/contracts/multisig/src/state.rs @@ -1,8 +1,8 @@ -use cosmwasm_std::{Addr, Storage, Uint64}; +use cosmwasm_std::{Addr, HexBinary, Storage, Uint64}; use cw_storage_plus::{Item, Map}; use crate::{ - key::{KeyType, PublicKey}, + key::KeyType, signing::SigningSession, types::{Key, KeyID}, ContractError, @@ -21,4 +21,5 @@ pub fn get_key(store: &dyn Storage, key_id: &KeyID) -> Result = Map::new("registered_pub_keys"); +// key type is part of the key so signers can register multiple keys with different types +pub const PUB_KEYS: Map<(Addr, KeyType), HexBinary> = Map::new("registered_pub_keys"); From 902bf3fd49238ceb8e0a2f97ed2bfcd38263faa3 Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Fri, 1 Sep 2023 14:23:31 -0400 Subject: [PATCH 07/14] address review --- contracts/multisig/src/key.rs | 128 ++++++++++++++++++++++++++++ contracts/multisig/src/lib.rs | 2 +- contracts/multisig/src/secp256k1.rs | 122 +------------------------- contracts/multisig/src/types.rs | 36 ++++++++ 4 files changed, 166 insertions(+), 122 deletions(-) diff --git a/contracts/multisig/src/key.rs b/contracts/multisig/src/key.rs index f8fe48a1f..b31634a18 100644 --- a/contracts/multisig/src/key.rs +++ b/contracts/multisig/src/key.rs @@ -1,6 +1,7 @@ use cosmwasm_schema::cw_serde; use cosmwasm_std::{HexBinary, StdError, StdResult}; use cw_storage_plus::{KeyDeserialize, PrimaryKey}; +use serde::{de::Error, Deserializer}; use crate::{ secp256k1::ecdsa_verify, @@ -22,9 +23,21 @@ pub enum Signature { #[cw_serde] pub enum PublicKey { + #[serde(deserialize_with = "deserialize_ecdsa_key")] Ecdsa(HexBinary), } +use serde::Deserialize; +fn deserialize_ecdsa_key<'de, D>(deserializer: D) -> Result +where + D: Deserializer<'de>, +{ + let pk: HexBinary = Deserialize::deserialize(deserializer)?; + PublicKey::try_from((KeyType::Ecdsa, 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 @@ -171,3 +184,118 @@ impl From for HexBinary { } } } + +#[cfg(test)] +mod test { + use cosmwasm_std::HexBinary; + + use crate::{ + key::Signature, + test::common::test_data, + types::{MsgToSign, VerifiableSignature}, + ContractError, + }; + + use super::{KeyType, PublicKey}; + + #[test] + fn deserialize_ecdsa_key() { + let key = PublicKey::try_from(( + KeyType::Ecdsa, + HexBinary::from_hex( + "03f57d1a813febaccbe6429603f9ec57969511b76cd680452dba91fa01f54e756d", + ) + .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_ecdsa_key_fails() { + let key = PublicKey::Ecdsa(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_ecdsa_public_key() { + let hex = test_data::pub_key(); + let pub_key = PublicKey::try_from((KeyType::Ecdsa, 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::Ecdsa, hex.clone())).unwrap_err(), + ContractError::InvalidPublicKeyFormat { + reason: "Invalid input length".into() + } + ); + } + + #[test] + fn test_try_from_hexbinary_to_signature() { + let hex = test_data::signature(); + let signature = Signature::try_from((KeyType::Ecdsa, hex.clone())).unwrap(); + assert_eq!(HexBinary::from(signature), hex); + } + + #[test] + fn test_try_from_hexbinary_to_signature_fails() { + let hex = + HexBinary::from_hex("283786d844a7c4d1d424837074d0c8ec71becdcba4dd42b5307cb543a0e2c8b81c10ad541defd5ce84d2a608fc454827d0b65b4865c8192a2ea1736a5c4b72") + .unwrap(); + assert_eq!( + Signature::try_from((KeyType::Ecdsa, hex.clone())).unwrap_err(), + ContractError::InvalidSignatureFormat { + reason: "Invalid input length".into() + } + ); + } + + #[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 result = signature.verify(&message, &public_key).unwrap(); + assert_eq!(result, true); + } + + #[test] + fn test_verify_signature_invalid_signature() { + let invalid_signature = HexBinary::from_hex( + "a112231719403227b297139cc6beef82a4e034663bfe48cf732687860b16227a51e4bd6be96fceeecf8e77fe7cdd4f5567d71aed5388484d1f2ba355298c954e1b", + ) + .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 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( + "03cd0b61b25b11c59323602dad24336edb9b9a40fb00fdd32c94908967ec16989e", + ) + .unwrap(); + + 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, 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 298ed9b4f..1f7927c63 100644 --- a/contracts/multisig/src/lib.rs +++ b/contracts/multisig/src/lib.rs @@ -11,6 +11,6 @@ pub mod types; mod secp256k1; #[cfg(test)] -mod test; +pub mod test; pub use crate::error::ContractError; diff --git a/contracts/multisig/src/secp256k1.rs b/contracts/multisig/src/secp256k1.rs index 4faaabf15..6880a60fd 100644 --- a/contracts/multisig/src/secp256k1.rs +++ b/contracts/multisig/src/secp256k1.rs @@ -1,26 +1,10 @@ use cosmwasm_crypto::secp256k1_verify; -use cosmwasm_std::HexBinary; // TODO: Logic specific to secp256k1 will most likely be handled by core in the future. -use crate::{types::MsgToSign, ContractError}; +use crate::ContractError; -const MESSAGE_HASH_LEN: usize = 32; const ECDSA_SIGNATURE_LEN: usize = 64; -impl TryFrom for MsgToSign { - type Error = ContractError; - - fn try_from(other: HexBinary) -> Result { - if other.len() != MESSAGE_HASH_LEN { - return Err(ContractError::InvalidMessageFormat { - reason: "Invalid input length".into(), - }); - } - - Ok(MsgToSign::unchecked(other)) - } -} - pub fn ecdsa_verify(msg_hash: &[u8], sig: &[u8], pub_key: &[u8]) -> Result { secp256k1_verify(msg_hash, &sig[0..ECDSA_SIGNATURE_LEN], pub_key).map_err(|e| { ContractError::SignatureVerificationFailed { @@ -28,107 +12,3 @@ pub fn ecdsa_verify(msg_hash: &[u8], sig: &[u8], pub_key: &[u8]) -> Result for MsgToSign { + type Error = ContractError; + + fn try_from(other: HexBinary) -> Result { + if other.len() != MESSAGE_HASH_LEN { + return Err(ContractError::InvalidMessageFormat { + reason: "Invalid input length".into(), + }); + } + + Ok(MsgToSign::unchecked(other)) + } +} + #[cfg(test)] mod tests { use cosmwasm_std::to_binary; + use crate::test::common::test_data; + use super::*; #[test] @@ -95,4 +113,22 @@ mod tests { assert_eq!(key, KeyID::from_vec(serialized.into()).unwrap()); } + + #[test] + fn test_try_from_hexbinary_to_message() { + let hex = test_data::message(); + let message = MsgToSign::try_from(hex.clone()).unwrap(); + assert_eq!(HexBinary::from(message), hex); + } + + #[test] + fn test_try_from_hexbinary_to_message_fails() { + let hex = HexBinary::from_hex("283786d844a7c4d1d424837074d0c8ec71becdcba4dd42b5307cb543a0e2c8b81c10ad541defd5ce84d2a608fc454827d0b65b4865c8192a2ea1736a5c4b72021b").unwrap(); + assert_eq!( + MsgToSign::try_from(hex.clone()).unwrap_err(), + ContractError::InvalidMessageFormat { + reason: "Invalid input length".into() + } + ); + } } From e34f24ccb02a28ed1fa6835ceeabe8a59386435c Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Fri, 1 Sep 2023 14:29:31 -0400 Subject: [PATCH 08/14] address review --- .../src/test/mocks/multisig.rs | 5 +-- contracts/multisig/src/contract.rs | 40 +++++++++---------- contracts/multisig/src/msg.rs | 3 +- 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/contracts/multisig-prover/src/test/mocks/multisig.rs b/contracts/multisig-prover/src/test/mocks/multisig.rs index ff0932f5c..fb4df7a32 100644 --- a/contracts/multisig-prover/src/test/mocks/multisig.rs +++ b/contracts/multisig-prover/src/test/mocks/multisig.rs @@ -34,10 +34,7 @@ pub fn execute( snapshot: _, pub_keys: _, } => Ok(Response::default()), - ExecuteMsg::RegisterPublicKey { - public_key: _, - key_type: _, - } => unimplemented!(), + ExecuteMsg::RegisterPublicKey { public_key: _ } => unimplemented!(), } } diff --git a/contracts/multisig/src/contract.rs b/contracts/multisig/src/contract.rs index 20ed37669..b7c924439 100644 --- a/contracts/multisig/src/contract.rs +++ b/contracts/multisig/src/contract.rs @@ -48,10 +48,9 @@ pub fn execute( snapshot, pub_keys, } => execute::key_gen(deps, info, key_id, snapshot, pub_keys), - ExecuteMsg::RegisterPublicKey { - public_key, - key_type, - } => execute::register_pub_key(deps, info, public_key, key_type), + ExecuteMsg::RegisterPublicKey { public_key } => { + execute::register_pub_key(deps, info, public_key) + } } } @@ -194,20 +193,18 @@ pub mod execute { pub fn register_pub_key( deps: DepsMut, info: MessageInfo, - public_key: HexBinary, - key_type: KeyType, + public_key: PublicKey, ) -> Result { - let pub_key: PublicKey = (key_type, public_key).try_into()?; PUB_KEYS.save( deps.storage, - (info.sender.clone(), key_type), - &pub_key.clone().into(), + (info.sender.clone(), public_key.key_type()), + &public_key.clone().into(), )?; Ok(Response::new().add_event( Event::PublicKeyRegistered { worker: info.sender, - public_key: pub_key, + public_key, } .into(), )) @@ -401,13 +398,9 @@ mod tests { fn do_register_key( deps: DepsMut, worker: Addr, - public_key: HexBinary, - key_type: KeyType, + public_key: PublicKey, ) -> Result { - let msg = ExecuteMsg::RegisterPublicKey { - public_key, - key_type, - }; + let msg = ExecuteMsg::RegisterPublicKey { public_key }; execute(deps, mock_env(), mock_info(worker.as_str(), &[]), msg) } @@ -699,7 +692,11 @@ mod tests { .collect::>(); for (addr, pub_key) in &pub_keys { - let res = do_register_key(deps.as_mut(), addr.clone(), pub_key.clone(), KeyType::Ecdsa); + let res = do_register_key( + deps.as_mut(), + addr.clone(), + PublicKey::Ecdsa(pub_key.clone()), + ); assert!(res.is_ok()); } let mut ret_pub_keys: Vec = vec![]; @@ -729,7 +726,11 @@ mod tests { .collect::>(); for (addr, pub_key) in &pub_keys { - let res = do_register_key(deps.as_mut(), addr.clone(), pub_key.clone(), KeyType::Ecdsa); + let res = do_register_key( + deps.as_mut(), + addr.clone(), + PublicKey::Ecdsa(pub_key.clone()), + ); assert!(res.is_ok()); } @@ -740,8 +741,7 @@ mod tests { let res = do_register_key( deps.as_mut(), pub_keys[0].0.clone(), - new_pub_key.clone(), - KeyType::Ecdsa, + PublicKey::Ecdsa(new_pub_key.clone()), ); assert!(res.is_ok()); diff --git a/contracts/multisig/src/msg.rs b/contracts/multisig/src/msg.rs index c76b0ab0c..4cf2b5a22 100644 --- a/contracts/multisig/src/msg.rs +++ b/contracts/multisig/src/msg.rs @@ -28,8 +28,7 @@ pub enum ExecuteMsg { pub_keys: HashMap, }, RegisterPublicKey { - public_key: HexBinary, - key_type: KeyType, + public_key: PublicKey, }, } From 789ea506a91763d7490f15d2c65d96cf5ce5fd0a Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Fri, 1 Sep 2023 15:55:43 -0400 Subject: [PATCH 09/14] address review --- contracts/multisig-prover/src/contract.rs | 20 ++- contracts/multisig-prover/src/encoding/evm.rs | 60 ++++----- contracts/multisig-prover/src/error.rs | 4 +- contracts/multisig-prover/src/execute.rs | 14 +-- contracts/multisig-prover/src/state.rs | 4 +- .../multisig-prover/src/test/test_data.rs | 115 ++++++++++-------- contracts/multisig-prover/src/types.rs | 6 + contracts/multisig/src/key.rs | 1 + contracts/multisig/src/msg.rs | 1 + contracts/voting-verifier/tests/tests.rs | 10 +- packages/axelar-wasm-std/src/operators.rs | 2 +- 11 files changed, 131 insertions(+), 106 deletions(-) diff --git a/contracts/multisig-prover/src/contract.rs b/contracts/multisig-prover/src/contract.rs index f0260b7e8..3c6b4da12 100644 --- a/contracts/multisig-prover/src/contract.rs +++ b/contracts/multisig-prover/src/contract.rs @@ -291,6 +291,8 @@ mod test { #[cfg(test)] mod tests { + use multisig::msg::Signer; + use crate::{contract::execute::should_update_worker_set, test::test_data}; #[test] @@ -303,7 +305,7 @@ mod tests { fn should_update_worker_set_one_more() { let worker_set = test_data::new_worker_set(); let mut new_worker_set = worker_set.clone(); - new_worker_set.signers.pop(); + new_worker_set.signers.pop_first(); assert!(should_update_worker_set(&worker_set, &new_worker_set, 0)); } @@ -311,7 +313,7 @@ mod tests { fn should_update_worker_set_one_less() { let worker_set = test_data::new_worker_set(); let mut new_worker_set = worker_set.clone(); - new_worker_set.signers.pop(); + new_worker_set.signers.pop_first(); assert!(should_update_worker_set(&new_worker_set, &worker_set, 0)); } @@ -319,7 +321,7 @@ mod tests { fn should_update_worker_set_one_more_higher_threshold() { let worker_set = test_data::new_worker_set(); let mut new_worker_set = worker_set.clone(); - new_worker_set.signers.pop(); + new_worker_set.signers.pop_first(); assert!(!should_update_worker_set(&worker_set, &new_worker_set, 1)); } @@ -327,8 +329,16 @@ mod tests { fn should_update_worker_set_diff_pub_key() { let worker_set = test_data::new_worker_set(); let mut new_worker_set = worker_set.clone(); - new_worker_set.signers[0].pub_key = worker_set.signers[1].pub_key.clone(); - new_worker_set.signers[1].pub_key = worker_set.signers[0].pub_key.clone(); + let first = new_worker_set.signers.pop_first(); + let last = new_worker_set.signers.pop_last(); + new_worker_set.signers.insert(Signer { + pub_key: first.clone().unwrap().pub_key, + ..last.clone().unwrap() + }); + new_worker_set.signers.insert(Signer { + pub_key: last.unwrap().pub_key, + ..first.unwrap() + }); assert!(should_update_worker_set(&worker_set, &new_worker_set, 0)); } } diff --git a/contracts/multisig-prover/src/encoding/evm.rs b/contracts/multisig-prover/src/encoding/evm.rs index 5370397de..c16ca88b3 100644 --- a/contracts/multisig-prover/src/encoding/evm.rs +++ b/contracts/multisig-prover/src/encoding/evm.rs @@ -57,14 +57,14 @@ impl TryFrom for Command { } } -impl TryFrom<(Signer, Option)> for Operator { +impl TryFrom for Operator { type Error = ContractError; - fn try_from((signer, sig): (Signer, Option)) -> Result { + fn try_from(signer: Signer) -> Result { Ok(Self { address: evm_address(signer.pub_key.as_ref())?, weight: signer.weight, - signature: sig, + signature: None, }) } } @@ -80,8 +80,8 @@ impl CommandBatch { .into_iter() .map(TryInto::try_into) .collect::, ContractError>>()?; - if new_worker_set.is_some() { - commands.push(new_worker_set.clone().unwrap().try_into()?); + if let Some(worker_set) = new_worker_set.clone() { + commands.push(worker_set.clone().try_into()?); } let data = Data { @@ -180,20 +180,17 @@ fn transfer_operatorship_params(worker_set: &WorkerSet) -> Result, Vec) = operators + .iter() + .map(|operator| { + ( + Token::Address(ethereum_types::Address::from_slice(operator.0.as_slice())), + Token::Uint(ethereum_types::U256::from_big_endian( + &operator.1.to_be_bytes(), + )), + ) + }) + .unzip(); let quorum = Token::Uint(ethereum_types::U256::from_big_endian( &worker_set.threshold.to_be_bytes(), @@ -249,7 +246,14 @@ fn sorted_operators( ) -> Result, ContractError> { let mut operators = signers .into_iter() - .map(TryInto::try_into) + .map(|(signer, sig)| { + signer.try_into().map(|mut op: Operator| { + if let Some(sig) = sig { + op.set_signature(sig); + } + op + }) + }) .collect::, ContractError>>()?; operators.sort(); @@ -429,20 +433,19 @@ mod test { #[test] fn test_command_operator_transfer() { - let mut new_worker_set = test_data::new_worker_set(); - new_worker_set - .signers - .sort_by_key(|signer| evm_address(signer.pub_key.as_ref()).unwrap()); + let new_worker_set = test_data::new_worker_set(); let res = Command::try_from(new_worker_set.clone()); assert!(res.is_ok()); let tokens = decode_operator_transfer_command_params(res.unwrap().params); - - for i in 0..new_worker_set.signers.len() { + let mut signers: Vec = new_worker_set.signers.into_iter().collect(); + signers.sort_by_key(|signer| evm_address(signer.pub_key.as_ref()).unwrap()); + let mut i = 0; + for signer in signers { assert_eq!( tokens[0].clone().into_array().unwrap()[i], Token::Address(ethereum_types::Address::from_slice( - evm_address(new_worker_set.signers[i].pub_key.as_ref()) + evm_address(signer.pub_key.as_ref()) .expect("couldn't convert pubkey to evm address") .as_slice() )) @@ -451,9 +454,10 @@ mod test { assert_eq!( tokens[1].clone().into_array().unwrap()[i], Token::Uint(ethereum_types::U256::from_big_endian( - &new_worker_set.signers[i].weight.to_be_bytes() + &signer.weight.to_be_bytes() )) ); + i = i + 1; } assert_eq!( tokens[2], diff --git a/contracts/multisig-prover/src/error.rs b/contracts/multisig-prover/src/error.rs index d56060dd1..0020c4871 100644 --- a/contracts/multisig-prover/src/error.rs +++ b/contracts/multisig-prover/src/error.rs @@ -28,10 +28,10 @@ pub enum ContractError { #[error("public key not found for participant {participant}")] PublicKeyNotFound { participant: String }, - #[error("{0}")] + #[error(transparent)] ServiceRegistryError(#[from] service_registry::ContractError), - #[error("{0}")] + #[error(transparent)] NonEmptyError(#[from] nonempty::Error), #[error("worker set has not changed sufficiently since last update")] diff --git a/contracts/multisig-prover/src/execute.rs b/contracts/multisig-prover/src/execute.rs index af6479609..c6b1d52b9 100644 --- a/contracts/multisig-prover/src/execute.rs +++ b/contracts/multisig-prover/src/execute.rs @@ -151,17 +151,9 @@ pub fn should_update_worker_set( cur_workers: &WorkerSet, max_diff: usize, ) -> bool { - let count_new = |a: &WorkerSet, b: &WorkerSet| { - a.signers - .iter() - .filter(|a_signer| { - !b.signers.iter().any(|b_signer| { - a_signer.address == b_signer.address && a_signer.pub_key == b_signer.pub_key - }) - }) - .count() - }; - count_new(new_workers, cur_workers) + count_new(cur_workers, new_workers) > max_diff + new_workers.signers.difference(&cur_workers.signers).count() + + cur_workers.signers.difference(&new_workers.signers).count() + > max_diff } pub fn rotate_snapshot( diff --git a/contracts/multisig-prover/src/state.rs b/contracts/multisig-prover/src/state.rs index 99b2ea9ca..99ed538c1 100644 --- a/contracts/multisig-prover/src/state.rs +++ b/contracts/multisig-prover/src/state.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeSet; + use axelar_wasm_std::{Snapshot, Threshold}; use connection_router::types::ChainName; use cosmwasm_schema::cw_serde; @@ -32,7 +34,7 @@ pub const REPLY_BATCH: Item = Item::new("reply_tracker"); #[cw_serde] pub struct WorkerSet { - pub signers: Vec, + pub signers: BTreeSet, pub threshold: Uint256, pub id: u64, // for randomness } diff --git a/contracts/multisig-prover/src/test/test_data.rs b/contracts/multisig-prover/src/test/test_data.rs index 505d298d8..126ad3002 100644 --- a/contracts/multisig-prover/src/test/test_data.rs +++ b/contracts/multisig-prover/src/test/test_data.rs @@ -1,6 +1,8 @@ // Test data taken from production axelarscan batch // https://axelarscan.io/batch/ethereum/0304b99223f238f417cd015b724d32081a19cee49a41a839b73cd16ccaa538ab +use std::collections::BTreeSet; + use axelar_wasm_std::{nonempty, Threshold}; use connection_router::msg::Message; use cosmwasm_std::{Addr, HexBinary, Uint256, Uint64}; @@ -33,59 +35,66 @@ fn legacy_cmd_id_input( } pub fn new_worker_set() -> WorkerSet { + let signers = vec![ + Signer { + address: Addr::unchecked("axelarvaloper1x86a8prx97ekkqej2x636utrdu23y8wupp9gk5"), + weight: Uint256::from(10u128), + pub_key: PublicKey::Ecdsa( + HexBinary::from_hex( + "03d123ce370b163acd576be0e32e436bb7e63262769881d35fa3573943bf6c6f81", + ) + .unwrap(), + ), + }, + Signer { + address: Addr::unchecked("axelarvaloper1ff675m593vve8yh82lzhdnqfpu7m23cxstr6h4"), + weight: Uint256::from(10u128), + pub_key: PublicKey::Ecdsa( + HexBinary::from_hex( + "03c6ddb0fcee7b528da1ef3c9eed8d51eeacd7cc28a8baa25c33037c5562faa6e4", + ) + .unwrap(), + ), + }, + Signer { + address: Addr::unchecked("axelarvaloper12cwre2gdhyytc3p97z9autzg27hmu4gfzz4rxc"), + weight: Uint256::from(10u128), + pub_key: PublicKey::Ecdsa( + HexBinary::from_hex( + "0274b5d2a4c55d7edbbf9cc210c4d25adbb6194d6b444816235c82984bee518255", + ) + .unwrap(), + ), + }, + Signer { + address: Addr::unchecked("axelarvaloper1vs9rdplntrf7ceqdkznjmanrr59qcpjq6le0yw"), + weight: Uint256::from(10u128), + pub_key: PublicKey::Ecdsa( + HexBinary::from_hex( + "02a670f57de55b8b39b4cb051e178ca8fb3fe3a78cdde7f8238baf5e6ce1893185", + ) + .unwrap(), + ), + }, + Signer { + address: Addr::unchecked("axelarvaloper1hz0slkejw96dukw87fztjkvwjdpcu20jewg6mw"), + weight: Uint256::from(10u128), + pub_key: PublicKey::Ecdsa( + HexBinary::from_hex( + "028584592624e742ba154c02df4c0b06e4e8a957ba081083ea9fe5309492aa6c7b", + ) + .unwrap(), + ), + }, + ]; + + let mut btree_signers = BTreeSet::new(); + for signer in signers { + btree_signers.insert(signer); + } + WorkerSet { - signers: vec![ - Signer { - address: Addr::unchecked("axelarvaloper1x86a8prx97ekkqej2x636utrdu23y8wupp9gk5"), - weight: Uint256::from(10u128), - pub_key: PublicKey::Ecdsa( - HexBinary::from_hex( - "03d123ce370b163acd576be0e32e436bb7e63262769881d35fa3573943bf6c6f81", - ) - .unwrap(), - ), - }, - Signer { - address: Addr::unchecked("axelarvaloper1ff675m593vve8yh82lzhdnqfpu7m23cxstr6h4"), - weight: Uint256::from(10u128), - pub_key: PublicKey::Ecdsa( - HexBinary::from_hex( - "03c6ddb0fcee7b528da1ef3c9eed8d51eeacd7cc28a8baa25c33037c5562faa6e4", - ) - .unwrap(), - ), - }, - Signer { - address: Addr::unchecked("axelarvaloper12cwre2gdhyytc3p97z9autzg27hmu4gfzz4rxc"), - weight: Uint256::from(10u128), - pub_key: PublicKey::Ecdsa( - HexBinary::from_hex( - "0274b5d2a4c55d7edbbf9cc210c4d25adbb6194d6b444816235c82984bee518255", - ) - .unwrap(), - ), - }, - Signer { - address: Addr::unchecked("axelarvaloper1vs9rdplntrf7ceqdkznjmanrr59qcpjq6le0yw"), - weight: Uint256::from(10u128), - pub_key: PublicKey::Ecdsa( - HexBinary::from_hex( - "02a670f57de55b8b39b4cb051e178ca8fb3fe3a78cdde7f8238baf5e6ce1893185", - ) - .unwrap(), - ), - }, - Signer { - address: Addr::unchecked("axelarvaloper1hz0slkejw96dukw87fztjkvwjdpcu20jewg6mw"), - weight: Uint256::from(10u128), - pub_key: PublicKey::Ecdsa( - HexBinary::from_hex( - "028584592624e742ba154c02df4c0b06e4e8a957ba081083ea9fe5309492aa6c7b", - ) - .unwrap(), - ), - }, - ], + signers: btree_signers, threshold: Uint256::from(30u128), id: 1, } @@ -150,7 +159,7 @@ pub fn evm_address() -> HexBinary { } pub fn encoded_data_with_operator_transfer() -> HexBinary { - HexBinary::from_hex("0000000000000000000000000000000000000000000000000000000000000539000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000000000014000000000000000000000000000000000000000000000000000000000000000012d76bacb9fd348392208f39e275a497580b4403457385c2af07440f1593f06420000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000147472616e736665724f70657261746f72736869700000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000001e000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000001e00000000000000000000000000000000000000000000000000000000000000050000000000000000000000000249c31dd0eacb2d73bbe5a0513416cd3888cb5b0000000000000000000000001ae3758c032ae8ebf6f075bb5b6ff6129b56e632000000000000000000000000adb32b50b13f962d302619111de6a1020fbd55f7000000000000000000000000defab04334a82fdea683bca3617c33bc469d4cc9000000000000000000000000e6857cf86038ba741e64ce0d3c883a26a7d3cb460000000000000000000000000000000000000000000000000000000000000005000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a").unwrap() + HexBinary::from_hex("0000000000000000000000000000000000000000000000000000000000000539000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000000000014000000000000000000000000000000000000000000000000000000000000000012e51dcb741369346f5da6b177f37ffaf784789f26e18d10a6cca5f9a4590aed00000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000147472616e736665724f70657261746f72736869700000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000001e000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000001e00000000000000000000000000000000000000000000000000000000000000050000000000000000000000000249c31dd0eacb2d73bbe5a0513416cd3888cb5b0000000000000000000000001ae3758c032ae8ebf6f075bb5b6ff6129b56e632000000000000000000000000adb32b50b13f962d302619111de6a1020fbd55f7000000000000000000000000defab04334a82fdea683bca3617c33bc469d4cc9000000000000000000000000e6857cf86038ba741e64ce0d3c883a26a7d3cb460000000000000000000000000000000000000000000000000000000000000005000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a").unwrap() } pub fn chain_id_operator_transfer() -> Uint256 { diff --git a/contracts/multisig-prover/src/types.rs b/contracts/multisig-prover/src/types.rs index f62baa43a..e8c87141b 100644 --- a/contracts/multisig-prover/src/types.rs +++ b/contracts/multisig-prover/src/types.rs @@ -90,3 +90,9 @@ pub struct Operator { pub weight: Uint256, pub signature: Option, } + +impl Operator { + pub fn set_signature(&mut self, sig: Signature) { + self.signature = Some(sig); + } +} diff --git a/contracts/multisig/src/key.rs b/contracts/multisig/src/key.rs index b31634a18..e16d8a41d 100644 --- a/contracts/multisig/src/key.rs +++ b/contracts/multisig/src/key.rs @@ -22,6 +22,7 @@ pub enum Signature { } #[cw_serde] +#[derive(Ord, PartialOrd, Eq)] pub enum PublicKey { #[serde(deserialize_with = "deserialize_ecdsa_key")] Ecdsa(HexBinary), diff --git a/contracts/multisig/src/msg.rs b/contracts/multisig/src/msg.rs index 7f1cc649e..af67c1e5e 100644 --- a/contracts/multisig/src/msg.rs +++ b/contracts/multisig/src/msg.rs @@ -49,6 +49,7 @@ pub enum QueryMsg { } #[cw_serde] +#[derive(Eq, Ord, PartialOrd)] pub struct Signer { pub address: Addr, pub weight: Uint256, diff --git a/contracts/voting-verifier/tests/tests.rs b/contracts/voting-verifier/tests/tests.rs index e7de405bb..bdfc73f3a 100644 --- a/contracts/voting-verifier/tests/tests.rs +++ b/contracts/voting-verifier/tests/tests.rs @@ -239,7 +239,7 @@ fn should_start_worker_set_confirmation() { let contract_address = initialize_contract(&mut app, service_registry_address.into_string()); let operators = Operators { - weights: vec![(vec![0, 1, 0, 1].into(), 1u64.into())], + weights_by_addresses: vec![(vec![0, 1, 0, 1].into(), 1u64.into())], threshold: 1u64.into(), }; let msg = msg::ExecuteMsg::ConfirmWorkerSet { @@ -278,7 +278,7 @@ fn should_confirm_worker_set() { .unwrap(); let operators = Operators { - weights: vec![(vec![0, 1, 0, 1].into(), 1u64.into())], + weights_by_addresses: vec![(vec![0, 1, 0, 1].into(), 1u64.into())], threshold: 1u64.into(), }; let msg = msg::ExecuteMsg::ConfirmWorkerSet { @@ -332,7 +332,7 @@ fn should_not_confirm_worker_set() { .unwrap(); let operators = Operators { - weights: vec![(vec![0, 1, 0, 1].into(), 1u64.into())], + weights_by_addresses: vec![(vec![0, 1, 0, 1].into(), 1u64.into())], threshold: 1u64.into(), }; let msg = msg::ExecuteMsg::ConfirmWorkerSet { @@ -386,7 +386,7 @@ fn should_confirm_worker_set_after_failed() { .unwrap(); let operators = Operators { - weights: vec![(vec![0, 1, 0, 1].into(), 1u64.into())], + weights_by_addresses: vec![(vec![0, 1, 0, 1].into(), 1u64.into())], threshold: 1u64.into(), }; let msg = msg::ExecuteMsg::ConfirmWorkerSet { @@ -472,7 +472,7 @@ fn should_not_confirm_twice() { .unwrap(); let operators = Operators { - weights: vec![(vec![0, 1, 0, 1].into(), 1u64.into())], + weights_by_addresses: vec![(vec![0, 1, 0, 1].into(), 1u64.into())], threshold: 1u64.into(), }; let msg = msg::ExecuteMsg::ConfirmWorkerSet { diff --git a/packages/axelar-wasm-std/src/operators.rs b/packages/axelar-wasm-std/src/operators.rs index 273a16107..8aa4b7076 100644 --- a/packages/axelar-wasm-std/src/operators.rs +++ b/packages/axelar-wasm-std/src/operators.rs @@ -5,7 +5,7 @@ use sha3::{Digest, Keccak256}; #[cw_serde] pub struct Operators { - pub weights: Vec<(HexBinary, Uint256)>, + pub weights_by_addresses: Vec<(HexBinary, Uint256)>, pub threshold: Uint256, } From 7faa587586a052634aaf947bc9807ce5d54125e0 Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Fri, 1 Sep 2023 17:25:53 -0400 Subject: [PATCH 10/14] address review --- contracts/multisig-prover/src/encoding/evm.rs | 125 ++++++++++-------- contracts/multisig-prover/src/execute.rs | 15 ++- contracts/multisig-prover/src/state.rs | 8 +- .../multisig-prover/src/test/test_data.rs | 4 +- contracts/multisig/src/contract.rs | 4 +- packages/axelar-wasm-std/src/lib.rs | 1 + 6 files changed, 91 insertions(+), 66 deletions(-) diff --git a/contracts/multisig-prover/src/encoding/evm.rs b/contracts/multisig-prover/src/encoding/evm.rs index c16ca88b3..65324686b 100644 --- a/contracts/multisig-prover/src/encoding/evm.rs +++ b/contracts/multisig-prover/src/encoding/evm.rs @@ -45,16 +45,13 @@ impl TryFrom for Command { } } -impl TryFrom for Command { - type Error = ContractError; - fn try_from(worker_set: WorkerSet) -> Result { - let params = transfer_operatorship_params(&worker_set)?; - Ok(Command { - ty: CommandType::TransferOperatorship, - params, - id: worker_set.hash(), - }) - } +fn make_transfer_operatorship(worker_set: WorkerSet) -> Result { + let params = transfer_operatorship_params(&worker_set)?; + Ok(Command { + ty: CommandType::TransferOperatorship, + params, + id: worker_set.hash(), + }) } impl TryFrom for Operator { @@ -69,35 +66,52 @@ impl TryFrom for Operator { } } -impl CommandBatch { - pub fn new( - messages: Vec, - destination_chain_id: Uint256, - new_worker_set: Option, - ) -> Result { - let message_ids: Vec = messages.iter().map(|msg| msg.id.clone()).collect(); - let mut commands = messages - .into_iter() - .map(TryInto::try_into) - .collect::, ContractError>>()?; - if let Some(worker_set) = new_worker_set.clone() { - commands.push(worker_set.clone().try_into()?); +pub struct CommandBatchBuilder { + message_ids: Vec, + new_worker_set: Option, + commands: Vec, + destination_chain_id: Uint256, +} + +impl CommandBatchBuilder { + pub fn new(destination_chain_id: Uint256) -> Self { + Self { + message_ids: vec![], + new_worker_set: None, + commands: vec![], + destination_chain_id, } + } + + pub fn add_message(&mut self, msg: Message) -> Result<(), ContractError> { + self.message_ids.push(msg.id.clone()); + self.commands.push(msg.try_into()?); + Ok(()) + } + + pub fn add_new_worker_set(&mut self, worker_set: WorkerSet) -> Result<(), ContractError> { + self.new_worker_set = Some(worker_set.clone()); + self.commands.push(make_transfer_operatorship(worker_set)?); + Ok(()) + } + pub fn build(self) -> Result { let data = Data { - destination_chain_id, - commands, + destination_chain_id: self.destination_chain_id, + commands: self.commands, }; - let id = BatchID::new(&message_ids, new_worker_set); + let id = BatchID::new(&self.message_ids, self.new_worker_set); - Ok(Self { + Ok(CommandBatch { id, - message_ids, + message_ids: self.message_ids, data, }) } +} +impl CommandBatch { pub fn msg_to_sign(&self) -> HexBinary { let msg = Keccak256::digest(self.data.encode().as_slice()); @@ -136,7 +150,8 @@ impl CommandBatch { quorum: Uint256, signers: Vec<(Signer, Option)>, ) -> Result { - let operators = sorted_operators(signers)?; + let mut operators = make_operators(signers)?; + operators.sort(); let (addresses, weights, signatures) = operators.iter().fold( (vec![], vec![], vec![]), @@ -241,23 +256,17 @@ fn evm_address(pub_key: &[u8]) -> Result { Ok(Keccak256::digest(&pub_key.as_bytes()[1..]).as_slice()[12..].into()) } -fn sorted_operators( - signers: Vec<(Signer, Option)>, +fn make_operators( + signers_with_sigs: Vec<(Signer, Option)>, ) -> Result, ContractError> { - let mut operators = signers - .into_iter() - .map(|(signer, sig)| { - signer.try_into().map(|mut op: Operator| { - if let Some(sig) = sig { - op.set_signature(sig); - } - op - }) + axelar_wasm_std::utils::try_map(signers_with_sigs, |(signer, sig)| { + signer.try_into().map(|mut op: Operator| { + if let Some(sig) = sig { + op.set_signature(sig); + } + op }) - .collect::, ContractError>>()?; - operators.sort(); - - Ok(operators) + }) } fn command_id(message_id: String) -> HexBinary { @@ -434,7 +443,7 @@ mod test { #[test] fn test_command_operator_transfer() { let new_worker_set = test_data::new_worker_set(); - let res = Command::try_from(new_worker_set.clone()); + let res = make_transfer_operatorship(new_worker_set.clone()); assert!(res.is_ok()); let tokens = decode_operator_transfer_command_params(res.unwrap().params); @@ -472,8 +481,12 @@ mod test { let messages = test_data::messages(); let destination_chain_id = test_data::destination_chain_id(); let test_data = decode_data(&test_data::encoded_data()); + let mut builder = CommandBatchBuilder::new(destination_chain_id); + for msg in messages { + builder.add_message(msg).unwrap(); + } - let res = CommandBatch::new(messages, destination_chain_id, None).unwrap(); + let res = builder.build().unwrap(); assert_eq!( res.message_ids, @@ -504,12 +517,10 @@ mod test { #[test] fn test_new_command_batch_with_operator_transfer() { let test_data = decode_data(&test_data::encoded_data_with_operator_transfer()); - - let res = CommandBatch::new( - vec![], - test_data::chain_id_operator_transfer(), - Some(test_data::new_worker_set()), - ); + let mut builder = CommandBatchBuilder::new(test_data::chain_id_operator_transfer()); + let res = builder.add_new_worker_set(test_data::new_worker_set()); + assert!(res.is_ok()); + let res = builder.build(); assert!(res.is_ok()); assert_eq!(res.unwrap().data, test_data); } @@ -521,7 +532,12 @@ mod test { let operators = test_data::operators(); let quorum = test_data::quorum(); - let batch = CommandBatch::new(messages, destination_chain_id, None).unwrap(); + let mut builder = CommandBatchBuilder::new(destination_chain_id); + for msg in messages { + let res = builder.add_message(msg); + assert!(res.is_ok()); + } + let batch = builder.build().unwrap(); let signers = operators .into_iter() @@ -703,7 +719,8 @@ mod test { ), ]; - let operators = sorted_operators(signers).unwrap(); + let mut operators = make_operators(signers).unwrap(); + operators.sort(); assert_eq!( operators[0].address.cmp(&operators[1].address), diff --git a/contracts/multisig-prover/src/execute.rs b/contracts/multisig-prover/src/execute.rs index c6b1d52b9..cb93efaa9 100644 --- a/contracts/multisig-prover/src/execute.rs +++ b/contracts/multisig-prover/src/execute.rs @@ -12,13 +12,14 @@ use service_registry::state::Worker; use crate::{ contract::START_MULTISIG_REPLY_ID, + encoding::evm::CommandBatchBuilder, error::ContractError, events::Event, state::{ Config, WorkerSet, COMMANDS_BATCH, CONFIG, CURRENT_WORKER_SET, KEY_ID, NEXT_WORKER_SET, REPLY_BATCH, }, - types::{BatchID, CommandBatch}, + types::BatchID, }; pub fn construct_proof(deps: DepsMut, message_ids: Vec) -> Result { @@ -32,7 +33,11 @@ pub fn construct_proof(deps: DepsMut, message_ids: Vec) -> Result batch, None => { - let batch = CommandBatch::new(messages, config.destination_chain_id, None)?; + let mut builder = CommandBatchBuilder::new(config.destination_chain_id); + for msg in messages { + builder.add_message(msg)?; + } + let batch = builder.build()?; COMMANDS_BATCH.save(deps.storage, &batch.id, &batch)?; @@ -121,7 +126,7 @@ pub fn update_worker_set(deps: DepsMut, env: Env) -> Result Result = Item::new("reply_tracker"); pub struct WorkerSet { pub signers: BTreeSet, pub threshold: Uint256, - pub id: u64, // for randomness + pub nonce: u64, // for randomness } impl WorkerSet { pub fn new( snapshot: Snapshot, pub_keys: Vec, - env: Env, + block_height: u64, ) -> Result { let signers = pub_keys .into_iter() @@ -58,7 +58,7 @@ impl WorkerSet { Ok(WorkerSet { signers, threshold: snapshot.quorum.into(), - id: env.block.height, + nonce: block_height, }) } diff --git a/contracts/multisig-prover/src/test/test_data.rs b/contracts/multisig-prover/src/test/test_data.rs index 126ad3002..7d372f389 100644 --- a/contracts/multisig-prover/src/test/test_data.rs +++ b/contracts/multisig-prover/src/test/test_data.rs @@ -96,7 +96,7 @@ pub fn new_worker_set() -> WorkerSet { WorkerSet { signers: btree_signers, threshold: Uint256::from(30u128), - id: 1, + nonce: 1, } } @@ -159,7 +159,7 @@ pub fn evm_address() -> HexBinary { } pub fn encoded_data_with_operator_transfer() -> HexBinary { - HexBinary::from_hex("0000000000000000000000000000000000000000000000000000000000000539000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000000000014000000000000000000000000000000000000000000000000000000000000000012e51dcb741369346f5da6b177f37ffaf784789f26e18d10a6cca5f9a4590aed00000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000147472616e736665724f70657261746f72736869700000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000001e000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000001e00000000000000000000000000000000000000000000000000000000000000050000000000000000000000000249c31dd0eacb2d73bbe5a0513416cd3888cb5b0000000000000000000000001ae3758c032ae8ebf6f075bb5b6ff6129b56e632000000000000000000000000adb32b50b13f962d302619111de6a1020fbd55f7000000000000000000000000defab04334a82fdea683bca3617c33bc469d4cc9000000000000000000000000e6857cf86038ba741e64ce0d3c883a26a7d3cb460000000000000000000000000000000000000000000000000000000000000005000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a").unwrap() + HexBinary::from_hex("0000000000000000000000000000000000000000000000000000000000000539000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000001400000000000000000000000000000000000000000000000000000000000000001358b13e07e0020644d29b5f65ab0f3c4d97b311d5a0a49488b18b21eeb06fcee0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000147472616e736665724f70657261746f72736869700000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000001e000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000001e00000000000000000000000000000000000000000000000000000000000000050000000000000000000000000249c31dd0eacb2d73bbe5a0513416cd3888cb5b0000000000000000000000001ae3758c032ae8ebf6f075bb5b6ff6129b56e632000000000000000000000000adb32b50b13f962d302619111de6a1020fbd55f7000000000000000000000000defab04334a82fdea683bca3617c33bc469d4cc9000000000000000000000000e6857cf86038ba741e64ce0d3c883a26a7d3cb460000000000000000000000000000000000000000000000000000000000000005000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000a").unwrap() } pub fn chain_id_operator_transfer() -> Uint256 { diff --git a/contracts/multisig/src/contract.rs b/contracts/multisig/src/contract.rs index b4654bc8d..96d6eaa52 100644 --- a/contracts/multisig/src/contract.rs +++ b/contracts/multisig/src/contract.rs @@ -241,7 +241,7 @@ pub mod query { let mut key = KEYS.load(deps.storage, &session.key_id)?; - let signers = key + let signers_with_sigs = key .snapshot .participants .into_iter() @@ -265,7 +265,7 @@ pub mod query { Ok(Multisig { state: session.state, quorum: key.snapshot.quorum.into(), - signers, + signers: signers_with_sigs, }) } diff --git a/packages/axelar-wasm-std/src/lib.rs b/packages/axelar-wasm-std/src/lib.rs index bc6c84c1e..c31b263c7 100644 --- a/packages/axelar-wasm-std/src/lib.rs +++ b/packages/axelar-wasm-std/src/lib.rs @@ -5,6 +5,7 @@ pub mod nonempty; pub mod operators; pub mod snapshot; pub mod threshold; +pub mod utils; pub mod voting; pub use crate::{ From 5d1ff1df57c1841440e1c93e3c58ddab7433ebc0 Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Fri, 1 Sep 2023 17:26:41 -0400 Subject: [PATCH 11/14] add missing file --- packages/axelar-wasm-std/src/utils.rs | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 packages/axelar-wasm-std/src/utils.rs diff --git a/packages/axelar-wasm-std/src/utils.rs b/packages/axelar-wasm-std/src/utils.rs new file mode 100644 index 000000000..0f6483868 --- /dev/null +++ b/packages/axelar-wasm-std/src/utils.rs @@ -0,0 +1,6 @@ +pub fn try_map(vec: Vec, f: F) -> Result, E> +where + F: FnMut(T) -> Result, +{ + vec.into_iter().map(f).collect::, E>>() +} From ba32ec53b3b8ec2e86ce3308de8d478a6ab54466 Mon Sep 17 00:00:00 2001 From: Edwin Guajardo Date: Wed, 6 Sep 2023 12:30:14 -0600 Subject: [PATCH 12/14] remove unnecessary trait --- contracts/multisig/src/key.rs | 17 ++++------------- contracts/multisig/src/signing.rs | 7 ++----- contracts/multisig/src/types.rs | 4 ---- 3 files changed, 6 insertions(+), 22 deletions(-) diff --git a/contracts/multisig/src/key.rs b/contracts/multisig/src/key.rs index b31634a18..47ebdb4a8 100644 --- a/contracts/multisig/src/key.rs +++ b/contracts/multisig/src/key.rs @@ -3,11 +3,7 @@ use cosmwasm_std::{HexBinary, StdError, StdResult}; use cw_storage_plus::{KeyDeserialize, PrimaryKey}; use serde::{de::Error, Deserializer}; -use crate::{ - secp256k1::ecdsa_verify, - types::{MsgToSign, VerifiableSignature}, - ContractError, -}; +use crate::{secp256k1::ecdsa_verify, types::MsgToSign, ContractError}; #[cw_serde] #[derive(Copy)] @@ -65,8 +61,8 @@ impl KeyTyped for Signature { } } -impl VerifiableSignature for Signature { - fn verify(&self, msg: &MsgToSign, pub_key: &PublicKey) -> Result { +impl Signature { + pub fn verify(&self, msg: &MsgToSign, pub_key: &PublicKey) -> Result { if !self.matches(pub_key) { return Err(ContractError::KeyTypeMismatch); } @@ -189,12 +185,7 @@ impl From for HexBinary { mod test { use cosmwasm_std::HexBinary; - use crate::{ - key::Signature, - test::common::test_data, - types::{MsgToSign, VerifiableSignature}, - ContractError, - }; + use crate::{key::Signature, test::common::test_data, types::MsgToSign, ContractError}; use super::{KeyType, PublicKey}; diff --git a/contracts/multisig/src/signing.rs b/contracts/multisig/src/signing.rs index 00407ee0f..cdbb129b6 100644 --- a/contracts/multisig/src/signing.rs +++ b/contracts/multisig/src/signing.rs @@ -7,7 +7,7 @@ use axelar_wasm_std::Snapshot; use crate::{ key::Signature, - types::{Key, KeyID, MsgToSign, MultisigState, VerifiableSignature}, + types::{Key, KeyID, MsgToSign, MultisigState}, ContractError, }; @@ -36,10 +36,7 @@ impl SigningSession { key: Key, signer: String, signature: Signature, - ) -> Result<(), ContractError> - where - Signature: VerifiableSignature, - { + ) -> Result<(), ContractError> { assert!(self.key_id == key.id, "violated invariant: key_id mismatch"); // TODO: correct way of handling this? if self.signatures.contains_key(&signer) { diff --git a/contracts/multisig/src/types.rs b/contracts/multisig/src/types.rs index ab1f3aff3..3ef094c10 100644 --- a/contracts/multisig/src/types.rs +++ b/contracts/multisig/src/types.rs @@ -7,10 +7,6 @@ use cw_storage_plus::{KeyDeserialize, PrimaryKey}; use crate::{key::PublicKey, ContractError}; -pub trait VerifiableSignature { - fn verify(&self, msg: &MsgToSign, pub_key: &PublicKey) -> Result; -} - #[cw_serde] pub struct MsgToSign(HexBinary); From b67c7f9f1d66c21b4c2a585af34d1d14a03de964 Mon Sep 17 00:00:00 2001 From: Edwin Guajardo Date: Thu, 7 Sep 2023 10:24:53 -0600 Subject: [PATCH 13/14] rename struct field according to contract renaming --- ampd/src/handlers/evm_verify_worker_set.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ampd/src/handlers/evm_verify_worker_set.rs b/ampd/src/handlers/evm_verify_worker_set.rs index 473b5a3de..55680de62 100644 --- a/ampd/src/handlers/evm_verify_worker_set.rs +++ b/ampd/src/handlers/evm_verify_worker_set.rs @@ -8,7 +8,7 @@ use crate::types::{EVMAddress, Hash, TMAddress, U256}; #[derive(Deserialize, Debug)] pub struct Operators { - pub weights: Vec<(EVMAddress, U256)>, + pub weights_by_addresses: Vec<(EVMAddress, U256)>, pub threshold: U256, } @@ -59,7 +59,7 @@ mod tests { log_index: 100, operators: Operators { threshold: 40u64.into(), - weights: vec![ + weights_by_addresses: vec![ ( HexBinary::from(EVMAddress::random().as_bytes()), 10u64.into(), From 909f9ae9a3d93cb00643217d6d3f1ede55bf18b7 Mon Sep 17 00:00:00 2001 From: Christian Gorenflo Date: Thu, 7 Sep 2023 19:51:23 -0400 Subject: [PATCH 14/14] merge conflicts fixed --- ampd/src/evm/verifier.rs | 6 +- contracts/multisig-prover/src/execute.rs | 2 +- contracts/multisig/src/contract.rs | 71 ------------------------ 3 files changed, 4 insertions(+), 75 deletions(-) diff --git a/ampd/src/evm/verifier.rs b/ampd/src/evm/verifier.rs index 74650876a..d9347a413 100644 --- a/ampd/src/evm/verifier.rs +++ b/ampd/src/evm/verifier.rs @@ -37,7 +37,7 @@ impl PartialEq<&WorkerSetConfirmation> for IAxelarGatewayEventsWithLog<'_> { IAxelarGatewayEvents::OperatorshipTransferredFilter(event) => { let (operators, weights): (Vec<_>, Vec<_>) = worker_set .operators - .weights + .weights_by_addresses .iter() .map(|(operator, weight)| { ( @@ -243,7 +243,7 @@ mod tests { log_index, operators: Operators { threshold: Uint256::from(40u64).into(), - weights: vec![ + weights_by_addresses: vec![ (EVMAddress::random(), Uint256::from(10u64).into()), (EVMAddress::random(), Uint256::from(20u64).into()), (EVMAddress::random(), Uint256::from(30u64).into()), @@ -252,7 +252,7 @@ mod tests { }; let (operators, weights): (Vec<_>, Vec<_>) = worker_set .operators - .weights + .weights_by_addresses .iter() .map(|(operator, weight)| { ( diff --git a/contracts/multisig-prover/src/execute.rs b/contracts/multisig-prover/src/execute.rs index a7b953621..cb93efaa9 100644 --- a/contracts/multisig-prover/src/execute.rs +++ b/contracts/multisig-prover/src/execute.rs @@ -2,7 +2,7 @@ use cosmwasm_std::{ to_binary, wasm_execute, Addr, BlockInfo, DepsMut, Env, HexBinary, QuerierWrapper, QueryRequest, Response, SubMsg, WasmMsg, WasmQuery, }; -use multisig::key::KeyType; +use multisig::key::{KeyType, PublicKey}; use std::{collections::HashMap, str::FromStr}; diff --git a/contracts/multisig/src/contract.rs b/contracts/multisig/src/contract.rs index 39b1ce186..96d6eaa52 100644 --- a/contracts/multisig/src/contract.rs +++ b/contracts/multisig/src/contract.rs @@ -717,77 +717,6 @@ mod tests { ); } - #[test] - fn test_update_key() { - let mut deps = mock_dependencies(); - do_instantiate(deps.as_mut()).unwrap(); - let signers = test_data::signers(); - let pub_keys = signers - .iter() - .map(|signer| (signer.address.clone(), signer.pub_key.clone())) - .collect::>(); - - for (addr, pub_key) in &pub_keys { - let res = do_register_key( - deps.as_mut(), - addr.clone(), - PublicKey::Ecdsa(pub_key.clone()), - ); - assert!(res.is_ok()); - } - - let new_pub_key = HexBinary::from_hex( - "021a381b3e07347d3a05495347e1fb2fe04764afcea5a74084fa957947b59f9026", - ) - .unwrap(); - let res = do_register_key( - deps.as_mut(), - pub_keys[0].0.clone(), - PublicKey::Ecdsa(new_pub_key.clone()), - ); - 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)).unwrap(), - from_binary::(&res.unwrap()).unwrap() - ); - } - #[test] - fn test_register_key() { - let mut deps = mock_dependencies(); - do_instantiate(deps.as_mut()).unwrap(); - let signers = test_data::signers(); - let pub_keys = signers - .iter() - .map(|signer| (signer.address.clone(), signer.pub_key.clone())) - .collect::>(); - - for (addr, pub_key) in &pub_keys { - let res = do_register_key( - deps.as_mut(), - addr.clone(), - PublicKey::Ecdsa(pub_key.clone()), - ); - 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); - 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] fn test_update_key() { let mut deps = mock_dependencies();