From 44af70631577009ced84e7b1f55eb519a18110b3 Mon Sep 17 00:00:00 2001 From: CJ Cobb <46455409+cjcobb23@users.noreply.github.com> Date: Thu, 14 Mar 2024 13:51:13 -0400 Subject: [PATCH] feat(minor-multisig-prover)!: restrict update_workerset to admin only (#299) * feat(minor-multisig-prover)!: restrict update_workerset to admin only --- contracts/multisig-prover/src/contract.rs | 26 +++++++++++++++++-- contracts/multisig-prover/src/execute.rs | 11 ++++++-- integration-tests/src/lib.rs | 1 + .../src/multisig_prover_contract.rs | 25 ++++++++++-------- integration-tests/src/protocol.rs | 21 +++++++++++++++ integration-tests/tests/test_utils/mod.rs | 24 ++++------------- integration-tests/tests/update_worker_set.rs | 4 +-- 7 files changed, 76 insertions(+), 36 deletions(-) create mode 100644 integration-tests/src/protocol.rs diff --git a/contracts/multisig-prover/src/contract.rs b/contracts/multisig-prover/src/contract.rs index aba432b59..ef0c66ef6 100644 --- a/contracts/multisig-prover/src/contract.rs +++ b/contracts/multisig-prover/src/contract.rs @@ -55,12 +55,15 @@ pub fn instantiate( pub fn execute( deps: DepsMut, env: Env, - _info: MessageInfo, + info: MessageInfo, msg: ExecuteMsg, ) -> Result { match msg { ExecuteMsg::ConstructProof { message_ids } => execute::construct_proof(deps, message_ids), - ExecuteMsg::UpdateWorkerSet {} => execute::update_worker_set(deps, env), + ExecuteMsg::UpdateWorkerSet {} => { + execute::require_admin(&deps, info)?; + execute::update_worker_set(deps, env) + } ExecuteMsg::ConfirmWorkerSet {} => execute::confirm_worker_set(deps), } .map_err(axelar_wasm_std::ContractError::from) @@ -286,6 +289,25 @@ mod tests { assert_eq!(worker_set, expected_worker_set); } + #[test] + fn test_update_worker_set_from_non_admin_should_fail() { + let mut test_case = setup_test_case(); + let res = test_case.app.execute_contract( + Addr::unchecked("some random address"), + test_case.prover_address.clone(), + &ExecuteMsg::UpdateWorkerSet {}, + &[], + ); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err() + .downcast::() + .unwrap() + .to_string(), + axelar_wasm_std::ContractError::from(ContractError::Unauthorized).to_string() + ); + } + #[test] fn test_update_worker_set_remove_one() { let mut test_case = setup_test_case(); diff --git a/contracts/multisig-prover/src/execute.rs b/contracts/multisig-prover/src/execute.rs index 0263eba93..094fe060c 100644 --- a/contracts/multisig-prover/src/execute.rs +++ b/contracts/multisig-prover/src/execute.rs @@ -1,6 +1,6 @@ use cosmwasm_std::{ - to_json_binary, wasm_execute, Addr, DepsMut, Env, QuerierWrapper, QueryRequest, Response, - Storage, SubMsg, WasmQuery, + to_json_binary, wasm_execute, Addr, DepsMut, Env, MessageInfo, QuerierWrapper, QueryRequest, + Response, Storage, SubMsg, WasmQuery, }; use multisig::{key::PublicKey, msg::Signer, worker_set::WorkerSet}; @@ -17,6 +17,13 @@ use crate::{ types::{BatchId, WorkersInfo}, }; +pub fn require_admin(deps: &DepsMut, info: MessageInfo) -> Result<(), ContractError> { + match CONFIG.load(deps.storage)?.admin { + admin if admin == info.sender => Ok(()), + _ => Err(ContractError::Unauthorized), + } +} + pub fn construct_proof( deps: DepsMut, message_ids: Vec, diff --git a/integration-tests/src/lib.rs b/integration-tests/src/lib.rs index 182eed44b..f9192b121 100644 --- a/integration-tests/src/lib.rs +++ b/integration-tests/src/lib.rs @@ -3,6 +3,7 @@ pub mod contract; pub mod gateway_contract; pub mod multisig_contract; pub mod multisig_prover_contract; +pub mod protocol; pub mod rewards_contract; pub mod service_registry_contract; pub mod voting_verifier_contract; diff --git a/integration-tests/src/multisig_prover_contract.rs b/integration-tests/src/multisig_prover_contract.rs index c841119a3..ec21e0230 100644 --- a/integration-tests/src/multisig_prover_contract.rs +++ b/integration-tests/src/multisig_prover_contract.rs @@ -1,23 +1,22 @@ -use crate::contract::Contract; +use crate::{contract::Contract, protocol::Protocol}; use axelar_wasm_std::Threshold; use cosmwasm_std::{Addr, Uint256}; -use cw_multi_test::{App, ContractWrapper, Executor}; +use cw_multi_test::{ContractWrapper, Executor}; use multisig::key::KeyType; use multisig_prover::encoding::Encoder; #[derive(Clone)] pub struct MultisigProverContract { pub contract_addr: Addr, + pub admin_addr: Addr, } impl MultisigProverContract { pub fn instantiate_contract( - app: &mut App, + protocol: &mut Protocol, + admin_address: Addr, gateway_address: Addr, - multisig_address: Addr, - service_registry_address: Addr, voting_verifier_address: Addr, - service_name: String, chain_name: String, ) -> Self { let code = ContractWrapper::new( @@ -26,6 +25,7 @@ impl MultisigProverContract { multisig_prover::contract::query, ) .with_reply(multisig_prover::contract::reply); + let app = &mut protocol.app; let code_id = app.store_code(Box::new(code)); let contract_addr = app @@ -33,14 +33,14 @@ impl MultisigProverContract { code_id, Addr::unchecked("anyone"), &multisig_prover::msg::InstantiateMsg { - admin_address: Addr::unchecked("doesn't matter").to_string(), + admin_address: admin_address.to_string(), gateway_address: gateway_address.to_string(), - multisig_address: multisig_address.to_string(), - service_registry_address: service_registry_address.to_string(), + multisig_address: protocol.multisig.contract_addr.to_string(), + service_registry_address: protocol.service_registry.contract_addr.to_string(), voting_verifier_address: voting_verifier_address.to_string(), destination_chain_id: Uint256::zero(), signing_threshold: Threshold::try_from((2, 3)).unwrap().try_into().unwrap(), - service_name: service_name.to_string(), + service_name: protocol.service_name.to_string(), chain_name: chain_name.to_string(), worker_set_diff_threshold: 0, encoder: Encoder::Abi, @@ -52,7 +52,10 @@ impl MultisigProverContract { ) .unwrap(); - MultisigProverContract { contract_addr } + MultisigProverContract { + contract_addr, + admin_addr: admin_address, + } } } diff --git a/integration-tests/src/protocol.rs b/integration-tests/src/protocol.rs new file mode 100644 index 000000000..c3a79a493 --- /dev/null +++ b/integration-tests/src/protocol.rs @@ -0,0 +1,21 @@ +use axelar_wasm_std::nonempty; +use cosmwasm_std::Addr; +use cw_multi_test::App; + +use crate::{ + connection_router_contract::ConnectionRouterContract, multisig_contract::MultisigContract, + rewards_contract::RewardsContract, service_registry_contract::ServiceRegistryContract, +}; + +pub struct Protocol { + pub genesis_address: Addr, // holds u128::max coins, can use to send coins to other addresses + pub governance_address: Addr, + pub connection_router: ConnectionRouterContract, + pub router_admin_address: Addr, + pub multisig: MultisigContract, + pub service_registry: ServiceRegistryContract, + pub service_name: nonempty::String, + pub rewards: RewardsContract, + pub rewards_params: rewards::msg::Params, + pub app: App, +} diff --git a/integration-tests/tests/test_utils/mod.rs b/integration-tests/tests/test_utils/mod.rs index 2d1222c85..2d4161ede 100644 --- a/integration-tests/tests/test_utils/mod.rs +++ b/integration-tests/tests/test_utils/mod.rs @@ -9,7 +9,6 @@ use cosmwasm_std::{ }; use cw_multi_test::{App, AppResponse, Executor}; -use integration_tests::connection_router_contract::ConnectionRouterContract; use integration_tests::contract::Contract; use integration_tests::gateway_contract::GatewayContract; use integration_tests::multisig_contract::MultisigContract; @@ -17,6 +16,7 @@ use integration_tests::multisig_prover_contract::MultisigProverContract; use integration_tests::rewards_contract::RewardsContract; use integration_tests::service_registry_contract::ServiceRegistryContract; use integration_tests::voting_verifier_contract::VotingVerifierContract; +use integration_tests::{connection_router_contract::ConnectionRouterContract, protocol::Protocol}; use k256::ecdsa; use sha3::{Digest, Keccak256}; @@ -279,19 +279,6 @@ pub fn distribute_rewards(protocol: &mut Protocol, chain_name: &ChainName, contr assert!(response.is_ok()); } -pub struct Protocol { - pub genesis_address: Addr, // holds u128::max coins, can use to send coins to other addresses - pub governance_address: Addr, - pub connection_router: ConnectionRouterContract, - pub router_admin_address: Addr, - pub multisig: MultisigContract, - pub service_registry: ServiceRegistryContract, - pub service_name: nonempty::String, - pub rewards: RewardsContract, - pub rewards_params: rewards::msg::Params, - pub app: App, -} - pub fn setup_protocol(service_name: nonempty::String) -> Protocol { let genesis = Addr::unchecked("genesis"); let mut app = App::new(|router, _, storage| { @@ -623,19 +610,18 @@ pub fn setup_chain(protocol: &mut Protocol, chain_name: ChainName) -> Chain { voting_verifier.contract_addr.clone(), ); + let multisig_prover_admin = Addr::unchecked(chain_name.to_string() + "prover_admin"); let multisig_prover = MultisigProverContract::instantiate_contract( - &mut protocol.app, + protocol, + multisig_prover_admin.clone(), gateway.contract_addr.clone(), - protocol.multisig.contract_addr.clone(), - protocol.service_registry.contract_addr.clone(), voting_verifier.contract_addr.clone(), - protocol.service_name.to_string(), chain_name.to_string(), ); let response = multisig_prover.execute( &mut protocol.app, - Addr::unchecked("doesn't matter"), + multisig_prover_admin, &multisig_prover::msg::ExecuteMsg::UpdateWorkerSet, ); assert!(response.is_ok()); diff --git a/integration-tests/tests/update_worker_set.rs b/integration-tests/tests/update_worker_set.rs index d75cab6d8..4698cfd57 100644 --- a/integration-tests/tests/update_worker_set.rs +++ b/integration-tests/tests/update_worker_set.rs @@ -45,7 +45,7 @@ fn worker_set_can_be_initialized_and_then_manually_updated() { let response = protocol .app .execute_contract( - Addr::unchecked("relayer"), + ethereum.multisig_prover.admin_addr.clone(), ethereum.multisig_prover.contract_addr.clone(), &multisig_prover::msg::ExecuteMsg::UpdateWorkerSet, &[], @@ -122,7 +122,7 @@ fn worker_set_cannot_be_updated_again_while_pending_worker_is_not_yet_confirmed( let response = protocol .app .execute_contract( - Addr::unchecked("relayer"), + ethereum.multisig_prover.admin_addr.clone(), ethereum.multisig_prover.contract_addr.clone(), &multisig_prover::msg::ExecuteMsg::UpdateWorkerSet, &[],