feat(minor-multisig-prover)!: allow governance to confirm workerset (a…

…xelarnetwork#311)

* feat(minor-multisig-prover)!: allow governance to confirm workerset

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<Response, axelar_wasm_std::ContractError> {
+    let config = make_config(&deps, msg)?;
+    CONFIG.save(deps.storage, &config)?;
+
+    Ok(Response::default())
+}
+
+fn make_config(
+    deps: &DepsMut,
+    msg: InstantiateMsg,
+) -> Result<Config, axelar_wasm_std::ContractError> {
     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<Binary> {
     }
 }
 
+#[cfg_attr(not(feature = "libary"), entry_point)]
+pub fn migrate(
+    deps: DepsMut,
+    _env: Env,
+    msg: MigrateMsg,
+) -> Result<Response, axelar_wasm_std::ContractError> {
+    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<AppResponse, Error> {
+    fn confirm_worker_set(
+        test_case: &mut TestCaseConfig,
+        sender: Addr,
+    ) -> Result<AppResponse, Error> {
         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()

contracts/multisig-prover/src/execute.rs

@@ -239,11 +239,11 @@ pub fn update_worker_set(deps: DepsMut, env: Env) -> Result<Response, ContractEr
     }
 }
 
-pub fn confirm_worker_set(deps: DepsMut) -> Result<Response, ContractError> {
-    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<Response, ContractError> {
     }))?;
 
     if status != VerificationStatus::SucceededOnChain {
-        return Err(ContractError::WorkerSetNotConfirmed);
+        Err(ContractError::WorkerSetNotConfirmed)
+    } else {
+        Ok(())
+    }
+}
+
+pub fn confirm_worker_set(deps: DepsMut, sender: Addr) -> Result<Response, ContractError> {
+    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)?;

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,

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<Config> = Item::new("config");
 pub const COMMANDS_BATCH: Map<&BatchId, CommandBatch> = Map::new("command_batch");
 pub const MULTISIG_SESSION_BATCH: Map<u64, BatchId> = Map::new("multisig_session_batch");

contracts/multisig-prover/src/test/multicontract.rs

@@ -4,13 +4,15 @@ 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;
 
 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,

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(),