From a68eb5b3c28d9f6c0bd665ba012cbec13970f3a8 Mon Sep 17 00:00:00 2001 From: CJ Cobb <46455409+cjcobb23@users.noreply.github.com> Date: Tue, 26 Mar 2024 17:16:30 -0400 Subject: [PATCH] feat(minor-multisig-prover)!: allow governance to confirm workerset (#311) * feat(minor-multisig-prover)!: allow governance to confirm workerset --- contracts/multisig-prover/src/contract.rs | 97 +++++++++++++++---- contracts/multisig-prover/src/execute.rs | 24 +++-- contracts/multisig-prover/src/msg.rs | 6 ++ contracts/multisig-prover/src/state.rs | 7 ++ .../multisig-prover/src/test/multicontract.rs | 4 + .../src/multisig_prover_contract.rs | 1 + 6 files changed, 114 insertions(+), 25 deletions(-) diff --git a/contracts/multisig-prover/src/contract.rs b/contracts/multisig-prover/src/contract.rs index 94bd47b5f..f142b37ab 100644 --- a/contracts/multisig-prover/src/contract.rs +++ b/contracts/multisig-prover/src/contract.rs @@ -7,8 +7,7 @@ use cosmwasm_std::{ use crate::{ error::ContractError, execute, - msg::ExecuteMsg, - msg::{InstantiateMsg, QueryMsg}, + msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg}, query, reply, state::{Config, CONFIG}, }; @@ -22,14 +21,26 @@ pub fn instantiate( _info: MessageInfo, msg: InstantiateMsg, ) -> Result { + let config = make_config(&deps, msg)?; + CONFIG.save(deps.storage, &config)?; + + Ok(Response::default()) +} + +fn make_config( + deps: &DepsMut, + msg: InstantiateMsg, +) -> Result { let admin = deps.api.addr_validate(&msg.admin_address)?; + let governance = deps.api.addr_validate(&msg.governance_address)?; let gateway = deps.api.addr_validate(&msg.gateway_address)?; let multisig = deps.api.addr_validate(&msg.multisig_address)?; let service_registry = deps.api.addr_validate(&msg.service_registry_address)?; let voting_verifier = deps.api.addr_validate(&msg.voting_verifier_address)?; - let config = Config { + Ok(Config { admin, + governance, gateway, multisig, service_registry, @@ -44,11 +55,7 @@ pub fn instantiate( worker_set_diff_threshold: msg.worker_set_diff_threshold, encoder: msg.encoder, key_type: msg.key_type, - }; - - CONFIG.save(deps.storage, &config)?; - - Ok(Response::default()) + }) } #[cfg_attr(not(feature = "library"), entry_point)] @@ -64,7 +71,7 @@ pub fn execute( execute::require_admin(&deps, info)?; execute::update_worker_set(deps, env) } - ExecuteMsg::ConfirmWorkerSet {} => execute::confirm_worker_set(deps), + ExecuteMsg::ConfirmWorkerSet {} => execute::confirm_worker_set(deps, info.sender), } .map_err(axelar_wasm_std::ContractError::from) } @@ -92,6 +99,23 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { } } +#[cfg_attr(not(feature = "libary"), entry_point)] +pub fn migrate( + deps: DepsMut, + _env: Env, + msg: MigrateMsg, +) -> Result { + let old_config = CONFIG.load(deps.storage)?; + let governance = deps.api.addr_validate(&msg.governance_address)?; + let new_config = Config { + governance, + ..old_config + }; + CONFIG.save(deps.storage, &new_config)?; + + Ok(Response::default()) +} + #[cfg(test)] mod tests { use anyhow::Error; @@ -131,14 +155,14 @@ mod tests { ) } - fn confirm_worker_set(test_case: &mut TestCaseConfig) -> Result { + fn confirm_worker_set( + test_case: &mut TestCaseConfig, + sender: Addr, + ) -> Result { let msg = ExecuteMsg::ConfirmWorkerSet {}; - test_case.app.execute_contract( - test_case.admin.clone(), - test_case.prover_address.clone(), - &msg, - &[], - ) + test_case + .app + .execute_contract(sender, test_case.prover_address.clone(), &msg, &[]) } fn execute_construct_proof( @@ -191,6 +215,7 @@ mod tests { fn test_instantiation() { let instantiator = "instantiator"; let admin = "admin"; + let governance = "governance"; let gateway_address = "gateway_address"; let multisig_address = "multisig_address"; let service_registry_address = "service_registry_address"; @@ -211,6 +236,7 @@ mod tests { let msg = InstantiateMsg { admin_address: admin.to_string(), + governance_address: governance.to_string(), gateway_address: gateway_address.to_string(), multisig_address: multisig_address.to_string(), voting_verifier_address: voting_verifier_address.to_string(), @@ -455,7 +481,7 @@ mod tests { quorum, ); - let res = confirm_worker_set(&mut test_case); + let res = confirm_worker_set(&mut test_case, Addr::unchecked("relayer")); assert!(res.is_ok()); let worker_set = query_get_worker_set(&mut test_case); @@ -487,7 +513,7 @@ mod tests { assert!(res.is_ok()); - let res = confirm_worker_set(&mut test_case); + let res = confirm_worker_set(&mut test_case, Addr::unchecked("relayer")); assert!(res.is_err()); assert_eq!( res.unwrap_err() @@ -498,6 +524,39 @@ mod tests { ); } + #[test] + fn test_governance_should_confirm_worker_set_without_verification() { + let mut test_case = setup_test_case(); + let res = execute_update_worker_set(&mut test_case); + + assert!(res.is_ok()); + + let mut new_worker_set = test_data::operators(); + new_worker_set.pop(); + mocks::service_registry::set_active_workers( + &mut test_case.app, + test_case.service_registry_address.clone(), + new_worker_set.clone(), + ); + let res = execute_update_worker_set(&mut test_case); + + assert!(res.is_ok()); + + let governance = test_case.governance.clone(); + let res = confirm_worker_set(&mut test_case, governance); + assert!(res.is_ok()); + + let worker_set = query_get_worker_set(&mut test_case); + assert!(worker_set.is_ok()); + + let worker_set = worker_set.unwrap(); + + let expected_worker_set = + test_operators_to_worker_set(new_worker_set, test_case.app.block_info().height); + + assert_eq!(worker_set, expected_worker_set); + } + #[test] fn test_confirm_worker_set_wrong_set() { let mut test_case = setup_test_case(); @@ -528,7 +587,7 @@ mod tests { assert!(res.is_ok()); - let res = confirm_worker_set(&mut test_case); + let res = confirm_worker_set(&mut test_case, Addr::unchecked("relayer")); assert!(res.is_err()); assert_eq!( res.unwrap_err() diff --git a/contracts/multisig-prover/src/execute.rs b/contracts/multisig-prover/src/execute.rs index 1d4ac5c9a..f017afb23 100644 --- a/contracts/multisig-prover/src/execute.rs +++ b/contracts/multisig-prover/src/execute.rs @@ -239,11 +239,11 @@ pub fn update_worker_set(deps: DepsMut, env: Env) -> Result Result { - let config = CONFIG.load(deps.storage)?; - - let worker_set = NEXT_WORKER_SET.load(deps.storage)?; - +fn ensure_worker_set_verification( + worker_set: &WorkerSet, + config: &Config, + deps: &DepsMut, +) -> Result<(), ContractError> { let query = voting_verifier::msg::QueryMsg::GetWorkerSetStatus { new_operators: make_operators(worker_set.clone(), config.encoder), }; @@ -254,7 +254,19 @@ pub fn confirm_worker_set(deps: DepsMut) -> Result { }))?; if status != VerificationStatus::SucceededOnChain { - return Err(ContractError::WorkerSetNotConfirmed); + Err(ContractError::WorkerSetNotConfirmed) + } else { + Ok(()) + } +} + +pub fn confirm_worker_set(deps: DepsMut, sender: Addr) -> Result { + let config = CONFIG.load(deps.storage)?; + + let worker_set = NEXT_WORKER_SET.load(deps.storage)?; + + if sender != config.governance { + ensure_worker_set_verification(&worker_set, &config, &deps)?; } CURRENT_WORKER_SET.save(deps.storage, &worker_set)?; diff --git a/contracts/multisig-prover/src/msg.rs b/contracts/multisig-prover/src/msg.rs index 85eea7ee5..18c83be66 100644 --- a/contracts/multisig-prover/src/msg.rs +++ b/contracts/multisig-prover/src/msg.rs @@ -9,6 +9,7 @@ use crate::encoding::{Data, Encoder}; #[cw_serde] pub struct InstantiateMsg { pub admin_address: String, + pub governance_address: String, pub gateway_address: String, pub multisig_address: String, pub service_registry_address: String, @@ -41,6 +42,11 @@ pub enum QueryMsg { GetWorkerSet, } +#[cw_serde] +pub struct MigrateMsg { + pub governance_address: String, +} + #[cw_serde] pub enum ProofStatus { Pending, diff --git a/contracts/multisig-prover/src/state.rs b/contracts/multisig-prover/src/state.rs index c1eef5eb7..06ae4f3d2 100644 --- a/contracts/multisig-prover/src/state.rs +++ b/contracts/multisig-prover/src/state.rs @@ -12,6 +12,8 @@ use crate::types::{BatchId, CommandBatch}; #[cw_serde] pub struct Config { pub admin: Addr, + #[serde(default = "default_governance")] + pub governance: Addr, pub gateway: Addr, pub multisig: Addr, pub service_registry: Addr, @@ -25,6 +27,11 @@ pub struct Config { pub key_type: KeyType, } +// temporary, so we can read the old config from storage (that doesn't have the governance field) +fn default_governance() -> Addr { + Addr::unchecked("axelar10d07y265gmmuvt4z0w9aw880jnsr700j7v9daj") +} + pub const CONFIG: Item = Item::new("config"); pub const COMMANDS_BATCH: Map<&BatchId, CommandBatch> = Map::new("command_batch"); pub const MULTISIG_SESSION_BATCH: Map = Map::new("multisig_session_batch"); diff --git a/contracts/multisig-prover/src/test/multicontract.rs b/contracts/multisig-prover/src/test/multicontract.rs index e37a2a36b..b27d804a6 100644 --- a/contracts/multisig-prover/src/test/multicontract.rs +++ b/contracts/multisig-prover/src/test/multicontract.rs @@ -4,6 +4,7 @@ use cw_multi_test::{next_block, App, AppBuilder, Contract, ContractWrapper, Exec use super::{mocks, test_data}; pub const INSTANTIATOR: &str = "instantiator"; +pub const GOVERNANCE: &str = "governance"; pub const RELAYER: &str = "relayer"; pub const SIGNATURE_BLOCK_EXPIRY: u64 = 100; @@ -11,6 +12,7 @@ pub const SIGNATURE_BLOCK_EXPIRY: u64 = 100; pub struct TestCaseConfig { pub app: App, pub admin: Addr, + pub governance: Addr, pub prover_address: Addr, pub service_registry_address: Addr, pub voting_verifier_address: Addr, @@ -158,6 +160,7 @@ fn instantiate_prover( let code_id = app.store_code(contract_prover()); let msg = crate::msg::InstantiateMsg { admin_address: INSTANTIATOR.to_string(), + governance_address: GOVERNANCE.to_string(), gateway_address, multisig_address, service_registry_address, @@ -209,6 +212,7 @@ pub fn setup_test_case() -> TestCaseConfig { TestCaseConfig { app, admin: Addr::unchecked(INSTANTIATOR), + governance: Addr::unchecked(GOVERNANCE), prover_address, service_registry_address, voting_verifier_address, diff --git a/integration-tests/src/multisig_prover_contract.rs b/integration-tests/src/multisig_prover_contract.rs index ec21e0230..c59bcc1a9 100644 --- a/integration-tests/src/multisig_prover_contract.rs +++ b/integration-tests/src/multisig_prover_contract.rs @@ -34,6 +34,7 @@ impl MultisigProverContract { Addr::unchecked("anyone"), &multisig_prover::msg::InstantiateMsg { admin_address: admin_address.to_string(), + governance_address: protocol.governance_address.to_string(), gateway_address: gateway_address.to_string(), multisig_address: protocol.multisig.contract_addr.to_string(), service_registry_address: protocol.service_registry.contract_addr.to_string(),