Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(multisig-prover): start update worker set #67

Merged
merged 21 commits into from
Sep 8, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ampd/src/handlers/evm_verify_worker_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down Expand Up @@ -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(),
Expand Down
30 changes: 14 additions & 16 deletions ampd/src/handlers/multisig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use cosmrs::cosmwasm::MsgExecuteContract;
use cosmwasm_std::{HexBinary, Uint64};
use ecdsa::VerifyingKey;
use error_stack::ResultExt;
use hex::{encode, FromHex};
use hex::encode;
use serde::de::Error as DeserializeError;
use serde::{Deserialize, Deserializer};
use tracing::info;
Expand Down Expand Up @@ -42,21 +42,18 @@ fn deserialize_public_keys<'de, D>(
where
D: Deserializer<'de>,
{
let keys_by_address: HashMap<TMAddress, String> = HashMap::deserialize(deserializer)?;
let keys_by_address: HashMap<TMAddress, multisig::key::PublicKey> =
HashMap::deserialize(deserializer)?;

keys_by_address
.into_iter()
.map(|(address, hex)| {
Ok((
.map(|(address, pk)| match pk {
multisig::key::PublicKey::Ecdsa(hex) => Ok((
address,
VerifyingKey::from_sec1_bytes(
<Vec<u8>>::from_hex(hex)
.map_err(D::Error::custom)?
.as_slice(),
)
.map_err(D::Error::custom)?
.into(),
))
VerifyingKey::from_sec1_bytes(hex.as_ref())
.map_err(D::Error::custom)?
.into(),
)),
})
.collect()
}
Expand Down Expand Up @@ -190,7 +187,8 @@ mod test {
use crate::tofnd::grpc::{MockEcdsaClient, SharableEcdsaClient};
use crate::types;
use multisig::events::Event::SigningStarted;
use multisig::types::{KeyID, MsgToSign, PublicKey};
use multisig::key::PublicKey;
use multisig::types::{KeyID, MsgToSign};

const MULTISIG_ADDRESS: &str = "axelarvaloper1zh9wrak6ke4n6fclj5e8yk397czv430ygs5jz7";

Expand All @@ -201,8 +199,8 @@ mod test {
.into()
}

fn rand_public_key() -> PublicKey {
PublicKey::unchecked(HexBinary::from(
fn rand_public_key() -> multisig::key::PublicKey {
multisig::key::PublicKey::Ecdsa(HexBinary::from(
types::PublicKey::from(SigningKey::random(&mut OsRng).verifying_key()).to_bytes(),
))
}
Expand Down Expand Up @@ -287,7 +285,7 @@ mod test {
let mut map: HashMap<String, PublicKey> = HashMap::new();
map.insert(
rand_account().to_string(),
PublicKey::unchecked(HexBinary::from(invalid_pub_key.as_slice())),
PublicKey::Ecdsa(HexBinary::from(invalid_pub_key.as_slice())),
);
match event {
events::Event::Abci {
Expand Down
8 changes: 5 additions & 3 deletions ampd/tests/report.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use error_stack::Report;

use ampd::report::{Error, LoggableError};
use std::env;

// Do not move this test or the location field checks break
#[test]
fn correct_error_log() {
env::set_var("RUST_BACKTRACE", "1");
let report = Report::new(Error::new("error1".to_string()))
.attach_printable("foo1")
.change_context(Error::new("error2".to_string()))
Expand All @@ -25,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:13: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:10: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:8:18".to_string(),
location: "ampd/tests/report.rs:10:18".to_string(),
cause: None,
backtrace: None,
})),
Expand Down
57 changes: 57 additions & 0 deletions contracts/multisig-prover/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the max number of workers that can change without triggering a worker set update.

};

CONFIG.save(deps.storage, &config)?;
Expand All @@ -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),
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -285,3 +288,57 @@ mod test {
}
}
}

#[cfg(test)]
mod tests {
use multisig::msg::Signer;

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_first();
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_first();
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_first();
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();
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));
}
}
Loading
Loading