diff --git a/contracts/dao/neutron-chain-manager/src/contract.rs b/contracts/dao/neutron-chain-manager/src/contract.rs index 7b02f50c..5a0be678 100644 --- a/contracts/dao/neutron-chain-manager/src/contract.rs +++ b/contracts/dao/neutron-chain-manager/src/contract.rs @@ -1,22 +1,21 @@ use crate::cron_module_param_types::{ - MsgUpdateParamsCron, ParamsRequestCron, ParamsResponseCron, MSG_TYPE_UPDATE_PARAMS_CRON, - PARAMS_QUERY_PATH_CRON, + ParamsRequestCron, ParamsResponseCron, PARAMS_QUERY_PATH_CRON, }; +use crate::permission::{match_permission_type, Permission, PermissionType, Validator}; #[cfg(not(feature = "library"))] use cosmwasm_std::entry_point; use cosmwasm_std::{ - to_json_binary, Addr, Binary, CosmosMsg, Deps, DepsMut, Env, MessageInfo, Response, StdResult, + to_json_binary, Addr, Binary, CosmosMsg, Deps, DepsMut, Env, MessageInfo, Response, StdError, + StdResult, }; use cw2::set_contract_version; -use neutron_sdk::bindings::msg::{AdminProposal, NeutronMsg, ProposalExecuteMessage}; +use neutron_sdk::bindings::msg::NeutronMsg; use neutron_sdk::proto_types::neutron::cron::QueryParamsRequest; use neutron_sdk::stargate::aux::make_stargate_query; use crate::error::ContractError; -use crate::msg::{ - ExecuteMsg, InstantiateMsg, MigrateMsg, ProposalExecuteMessageJSON, QueryMsg, Strategy, -}; -use crate::state::STRATEGIES; +use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg}; +use crate::state::PERMISSIONS; pub(crate) const CONTRACT_NAME: &str = "crates.io:neutron-chain-manager"; pub(crate) const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -30,19 +29,15 @@ pub fn instantiate( ) -> Result { set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; - if let Strategy::AllowOnly(_) = msg.initial_strategy { - return Err(ContractError::InvalidInitialStrategy {}); - } - - STRATEGIES.save( + PERMISSIONS.save( deps.storage, - msg.initial_strategy_address.clone(), - &msg.initial_strategy, + (msg.initial_address.clone(), PermissionType::AllowAll), + &Permission::AllowAll, )?; Ok(Response::new() .add_attribute("action", "instantiate") - .add_attribute("init_allow_all_address", msg.initial_strategy_address)) + .add_attribute("init_allow_all_address", msg.initial_address)) } #[cfg_attr(not(feature = "library"), entry_point)] @@ -53,29 +48,27 @@ pub fn execute( msg: ExecuteMsg, ) -> Result, ContractError> { match msg { - ExecuteMsg::AddStrategy { address, strategy } => { - execute_add_strategy(deps, info, address, strategy) - } - ExecuteMsg::RemoveStrategy { address } => execute_remove_strategy(deps, info, address), + ExecuteMsg::AddPermissions { + address, + permissions, + } => execute_add_permissions(deps, info, address, permissions), + ExecuteMsg::RemovePermissions { address } => execute_remove_strategy(deps, info, address), ExecuteMsg::ExecuteMessages { messages } => execute_execute_messages(deps, info, messages), } } -pub fn execute_add_strategy( +pub fn execute_add_permissions( deps: DepsMut, info: MessageInfo, address: Addr, - strategy: Strategy, + permissions: Vec, ) -> Result, ContractError> { is_authorized(deps.as_ref(), info.sender.clone())?; // We add the new strategy, and then we check that it did not replace // the only existing ALLOW_ALL strategy. - STRATEGIES.save(deps.storage, address.clone(), &strategy)?; - if let Strategy::AllowOnly(_) = strategy { - if no_admins_left(deps.as_ref())? { - return Err(ContractError::InvalidDemotion {}); - } + for p in &permissions { + PERMISSIONS.save(deps.storage, (address.clone(), p.to_owned().into()), p)?; } Ok(Response::new() @@ -90,9 +83,16 @@ pub fn execute_remove_strategy( ) -> Result, ContractError> { is_authorized(deps.as_ref(), info.sender.clone())?; - // First we remove the strategy, then we check that it was not the only - // ALLOW_ALL strategy we had. - STRATEGIES.remove(deps.storage, address.clone()); + // First we remove all the permissions, then we check that it was not the only + // ALLOW_ALL permission we had. + let permission_types = PERMISSIONS + .prefix(address.clone()) + .keys(deps.storage, None, None, cosmwasm_std::Order::Ascending) + .collect::, StdError>>()?; + for permission in permission_types { + PERMISSIONS.remove(deps.storage, (address.clone(), permission)) + } + if no_admins_left(deps.as_ref())? { return Err(ContractError::InvalidDemotion {}); } @@ -107,165 +107,54 @@ pub fn execute_execute_messages( info: MessageInfo, messages: Vec>, ) -> Result, ContractError> { - // If the sender doesn't have a strategy associated with them, abort immediately. - if !STRATEGIES.has(deps.storage, info.sender.clone()) { - return Err(ContractError::Unauthorized {}); - } - let response = Response::new() .add_attribute("action", "execute_execute_messages") .add_attribute("address", info.sender.clone()); - let strategy = STRATEGIES.load(deps.storage, info.sender)?; - match strategy { - Strategy::AllowAll => Ok(response + // check the sender is AllowAll account + if PERMISSIONS.has( + deps.storage, + (info.sender.clone(), PermissionType::AllowAll), + ) { + return Ok(response .add_attribute("strategy", "allow_all") - .add_messages(messages)), - Strategy::AllowOnly(_) => { - check_allow_only_permissions(deps.as_ref(), strategy.clone(), messages.clone())?; - Ok(response - .add_attribute("strategy", "allow_only") - .add_messages(messages)) - } + .add_messages(messages)); + }; + + // сопоставляем космос сообщения с типами требуемых разрешений + let permissions_types: Vec<(PermissionType, Validator)> = messages + .iter() + .map(|msg| match_permission_type(msg)) + .collect::, ContractError>>()?; + + for (p, v) in permissions_types { + let permission = PERMISSIONS.load(deps.storage, (info.sender.clone(), p))?; + v.validate(deps.as_ref(), permission)? } + + Ok(response + .add_attribute("strategy", "allow_only") + .add_messages(messages)) } fn is_authorized(deps: Deps, address: Addr) -> Result<(), ContractError> { - match STRATEGIES.load(deps.storage, address) { - Ok(Strategy::AllowAll) => Ok(()), - _ => Err(ContractError::Unauthorized {}), + if PERMISSIONS.has(deps.storage, (address, PermissionType::AllowAll)) { + Ok(()) + } else { + Err(ContractError::Unauthorized {}) } } /// This function returns true if there is no more allow_all strategies left. fn no_admins_left(deps: Deps) -> Result { - let not_found: bool = !STRATEGIES - .range(deps.storage, None, None, cosmwasm_std::Order::Ascending) - .collect::, _>>()? + let not_found: bool = !PERMISSIONS + .keys(deps.storage, None, None, cosmwasm_std::Order::Ascending) + .collect::, _>>()? .into_iter() - .any(|(_, strategy)| matches!(strategy, Strategy::AllowAll)); - + .any(|(_, perm_type)| perm_type == PermissionType::AllowAll); Ok(not_found) } -/// For every message, check whether we have the permission to execute it. -/// Any missing permission aborts the execution. Trying to execute any -/// unknown message aborts the execution. -fn check_allow_only_permissions( - deps: Deps, - strategy: Strategy, - messages: Vec>, -) -> Result<(), ContractError> { - for msg in messages.clone() { - if let CosmosMsg::Custom(neutron_msg) = msg { - check_neutron_msg(deps, strategy.clone(), neutron_msg)? - } else { - return Err(ContractError::Unauthorized {}); - } - } - - Ok(()) -} - -fn check_neutron_msg( - deps: Deps, - strategy: Strategy, - neutron_msg: NeutronMsg, -) -> Result<(), ContractError> { - match neutron_msg { - NeutronMsg::AddSchedule { .. } => { - if !strategy.has_cron_add_schedule_permission() { - return Err(ContractError::Unauthorized {}); - } - } - NeutronMsg::RemoveSchedule { name: _ } => { - if !strategy.has_cron_remove_schedule_permission() { - return Err(ContractError::Unauthorized {}); - } - } - NeutronMsg::SubmitAdminProposal { admin_proposal } => { - check_submit_admin_proposal_message(deps, strategy, admin_proposal)?; - } - _ => { - return Err(ContractError::Unauthorized {}); - } - } - - Ok(()) -} - -fn check_submit_admin_proposal_message( - deps: Deps, - strategy: Strategy, - proposal: AdminProposal, -) -> Result<(), ContractError> { - match proposal { - AdminProposal::ParamChangeProposal(proposal) => { - for param_change in proposal.param_changes { - if !strategy.has_param_change_permission(param_change) { - return Err(ContractError::Unauthorized {}); - } - } - } - AdminProposal::ProposalExecuteMessage(proposal) => { - check_proposal_execute_message(deps, strategy.clone(), proposal)?; - } - _ => { - return Err(ContractError::Unauthorized {}); - } - } - - Ok(()) -} - -/// Processes ProposalExecuteMessage messages. Message type has to be checked -/// as a string; after that, you can parse the JSON payload into a specific -/// message. -fn check_proposal_execute_message( - deps: Deps, - strategy: Strategy, - proposal: ProposalExecuteMessage, -) -> Result<(), ContractError> { - let typed_proposal: ProposalExecuteMessageJSON = - serde_json_wasm::from_str(proposal.message.as_str())?; - - if typed_proposal.type_field.as_str() == MSG_TYPE_UPDATE_PARAMS_CRON { - check_cron_update_msg_params(deps, strategy, proposal)?; - } - - Ok(()) -} -/// Checks that the strategy owner is authorised to change the parameters of the -/// cron module. We query the current values for each parameter & compare them to -/// the values in the proposal; all modifications must be allowed by the strategy. -fn check_cron_update_msg_params( - deps: Deps, - strategy: Strategy, - proposal: ProposalExecuteMessage, -) -> Result<(), ContractError> { - let msg_update_params: MsgUpdateParamsCron = - serde_json_wasm::from_str(proposal.message.as_str())?; - - let cron_update_param_permission = strategy - .get_cron_update_param_permission() - .ok_or(ContractError::Unauthorized {})?; - - let cron_params = get_cron_params(deps, ParamsRequestCron {})?; - if cron_params.params.limit != msg_update_params.params.limit - && !cron_update_param_permission.limit - { - return Err(ContractError::Unauthorized {}); - } - - if cron_params.params.security_address != msg_update_params.params.security_address - && !cron_update_param_permission.security_address - { - return Err(ContractError::Unauthorized {}); - } - - Ok(()) -} - /// Queries the parameters of the cron module. pub fn get_cron_params(deps: Deps, req: ParamsRequestCron) -> StdResult { make_stargate_query(deps, PARAMS_QUERY_PATH_CRON, QueryParamsRequest::from(req)) @@ -274,17 +163,21 @@ pub fn get_cron_params(deps: Deps, req: ParamsRequestCron) -> StdResult StdResult { match msg { - QueryMsg::Strategies {} => to_json_binary(&query_strategies(deps)?), + QueryMsg::Permissions {} => to_json_binary(&query_strategies(deps)?), } } /// No pagination is added because it's unlikely that there is going /// to be more than 10 strategies. -pub fn query_strategies(deps: Deps) -> StdResult> { - let all_strategies: Vec<(Addr, Strategy)> = STRATEGIES +pub fn query_strategies(deps: Deps) -> StdResult> { + // TODO group by addresses + let all_permissions: Vec<(Addr, Permission)> = PERMISSIONS .range(deps.storage, None, None, cosmwasm_std::Order::Ascending) - .collect::, _>>()?; - Ok(all_strategies) + .collect::, _>>()? + .into_iter() + .map(|((a, _), p)| return (a, p)) + .collect(); + Ok(all_permissions) } #[cfg_attr(not(feature = "library"), entry_point)] diff --git a/contracts/dao/neutron-chain-manager/src/error.rs b/contracts/dao/neutron-chain-manager/src/error.rs index 8844a724..20d2f82c 100644 --- a/contracts/dao/neutron-chain-manager/src/error.rs +++ b/contracts/dao/neutron-chain-manager/src/error.rs @@ -6,12 +6,12 @@ pub enum ContractError { #[error("{0}")] Std(#[from] StdError), - #[error("Initial strategy of a wrong type was submitted")] - InvalidInitialStrategy {}, - #[error("Unauthorized")] Unauthorized {}, + #[error("PermissionType not found: {0}")] + PermissionTypeNotFound(String), + // This error is returned when you try to remove the only existing // ALLOW_ALL strategy. #[error("An invalid demotion was attempted")] diff --git a/contracts/dao/neutron-chain-manager/src/lib.rs b/contracts/dao/neutron-chain-manager/src/lib.rs index bbf9755f..b9250086 100644 --- a/contracts/dao/neutron-chain-manager/src/lib.rs +++ b/contracts/dao/neutron-chain-manager/src/lib.rs @@ -6,3 +6,4 @@ pub mod state; mod cron_module_param_types; #[cfg(test)] mod testing; +mod permission; diff --git a/contracts/dao/neutron-chain-manager/src/msg.rs b/contracts/dao/neutron-chain-manager/src/msg.rs index b88d1033..f4228cf1 100644 --- a/contracts/dao/neutron-chain-manager/src/msg.rs +++ b/contracts/dao/neutron-chain-manager/src/msg.rs @@ -1,25 +1,25 @@ use cosmwasm_schema::{cw_serde, QueryResponses}; use cosmwasm_std::{Addr, CosmosMsg}; -use neutron_sdk::bindings::msg::{NeutronMsg, ParamChange}; +use neutron_sdk::bindings::msg::NeutronMsg; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use crate::permission::Permission; + #[derive(Serialize, Deserialize, JsonSchema, Debug, Clone)] pub struct InstantiateMsg { - /// Defines the address for the initial strategy. - pub initial_strategy_address: Addr, - /// Defines the initial strategy. Must be an ALLOW_ALL strategy. - pub initial_strategy: Strategy, + /// Defines the address for the initial setup. + pub initial_address: Addr, } #[derive(Serialize, Deserialize, JsonSchema, Debug, Clone)] #[serde(rename_all = "snake_case")] pub enum ExecuteMsg { - AddStrategy { + AddPermissions { address: Addr, - strategy: Strategy, + permissions: Vec, }, - RemoveStrategy { + RemovePermissions { address: Addr, }, ExecuteMessages { @@ -30,111 +30,19 @@ pub enum ExecuteMsg { #[cw_serde] #[derive(QueryResponses)] pub enum QueryMsg { - #[returns(Vec < Strategy >)] - Strategies {}, + #[returns(Vec < Permission >)] + Permissions {}, } #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] pub struct MigrateMsg {} -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] -pub enum Strategy { - AllowAll, - AllowOnly(Vec), -} - -impl Strategy { - pub fn has_cron_add_schedule_permission(&self) -> bool { - match self { - Strategy::AllowAll => true, - Strategy::AllowOnly(permissions) => { - let mut has_permission = false; - for permission in permissions { - if let Permission::CronPermission(cron_permission) = permission { - has_permission = cron_permission.add_schedule - } - } - - has_permission - } - } - } - pub fn has_cron_remove_schedule_permission(&self) -> bool { - match self { - Strategy::AllowAll => true, - Strategy::AllowOnly(permissions) => { - let mut has_permission = false; - for permission in permissions { - if let Permission::CronPermission(cron_permission) = permission { - has_permission = cron_permission.remove_schedule - } - } - - has_permission - } - } - } - pub fn has_param_change_permission(&self, param_change: ParamChange) -> bool { - match self { - Strategy::AllowAll => true, - Strategy::AllowOnly(permissions) => { - for permission in permissions { - if let Permission::ParamChangePermission(param_change_permissions) = permission - { - for param_change_permission in param_change_permissions.params.clone() { - if param_change.subspace == param_change_permission.subspace - && param_change.key == param_change_permission.key - { - return true; - } - } - } - } - - false - } - } - } - - pub fn get_cron_update_param_permission(&self) -> Option { - match self { - Strategy::AllowAll => Some(CronUpdateParamsPermission { - security_address: true, - limit: true, - }), - Strategy::AllowOnly(permissions) => { - for permission in permissions { - if let Permission::UpdateParamsPermission(update_params_permission) = permission - { - return match update_params_permission { - UpdateParamsPermission::CronUpdateParamsPermission( - cron_update_params, - ) => Some(cron_update_params.clone()), - }; - } - } - - None - } - } - } -} - -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] -pub enum Permission { - // Deprecated, for legacy parameter updates using `params` module. - ParamChangePermission(ParamChangePermission), - // For new-style parameter updates. - UpdateParamsPermission(UpdateParamsPermission), - CronPermission(CronPermission), -} - #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] pub struct ParamChangePermission { pub params: Vec, } -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema, Hash)] pub struct ParamPermission { pub subspace: String, pub key: String, diff --git a/contracts/dao/neutron-chain-manager/src/permission.rs b/contracts/dao/neutron-chain-manager/src/permission.rs new file mode 100644 index 00000000..ee3b9f3f --- /dev/null +++ b/contracts/dao/neutron-chain-manager/src/permission.rs @@ -0,0 +1,210 @@ +use cosmwasm_schema::cw_serde; +use cosmwasm_std::{CosmosMsg, Deps, StdError, StdResult}; +use cw_storage_plus::{Key, KeyDeserialize, Prefixer, PrimaryKey}; +use neutron_sdk::bindings::msg::{ + AdminProposal, NeutronMsg, ParamChangeProposal, ProposalExecuteMessage, +}; + +use std::collections::HashSet; +use std::iter::FromIterator; + +use crate::contract::get_cron_params; +use crate::cron_module_param_types::{MsgUpdateParamsCron, ParamsRequestCron}; +use crate::msg::{CronUpdateParamsPermission, ParamChangePermission, ParamPermission}; +use crate::{ + cron_module_param_types::MSG_TYPE_UPDATE_PARAMS_CRON, error::ContractError, + msg::ProposalExecuteMessageJSON, +}; + +#[cw_serde] +pub enum Permission { + AllowAll, + AddCronPermission, + RemoveCronPermission, + CronUpdateParamsPermission(CronUpdateParamsPermission), + ParamChangePermission(ParamChangePermission), +} + +fn remove_length_prefix(value: Vec) -> Vec { + // That is required due to `Prefixer` and `PrimaryKey` implementations for PermissionType + // ``` + // serde_json_wasm::to_vec(self).unwrap().iter().map(|&v| Key::Val8([v])).collect() + // ``` + // The thing is, during deriving storage key, every element of the Vec + // except the last one becomes prefixed by a length of a key, for example "aaa" (bytes 65 65 65) + // converts into 0 1 65 + 0 1 65 + 65, where 0 1 is 2bytes length + // we converting back prefixed vec to non prefixed + let len_prefix_length = 2; + let mut v = value + .iter() + .skip(len_prefix_length) + .step_by(len_prefix_length + 1) + .collect::>(); + v.push(&value[value.len() - 1]); + v.into_iter().map(|&v| v).collect::>() +} + +impl<'a> KeyDeserialize for PermissionType { + type Output = PermissionType; + + fn from_vec(value: Vec) -> StdResult { + serde_json_wasm::from_slice(remove_length_prefix(value).as_ref()) + .map_err(|e| StdError::generic_err(format!("Invalid PermissionType: {}", e))) + } +} + +impl<'a> PrimaryKey<'a> for PermissionType { + type Prefix = (); + type SubPrefix = (); + type Suffix = Self; + type SuperSuffix = Self; + + fn key(&self) -> Vec { + // TODO: find a way use Key::Ref insated of Key::Val8 + // to simplify KeyDeserialize and remove `remove_length_prefix` + serde_json_wasm::to_vec(self).unwrap().iter().map(|&v| Key::Val8([v])).collect() + } +} + +impl<'a> Prefixer<'a> for PermissionType { + fn prefix(&self) -> Vec { + serde_json_wasm::to_vec(self).unwrap().iter().map(|&v| Key::Val8([v])).collect() + } +} + + +#[cw_serde] +pub enum PermissionType { + AllowAll, + AddCronPermission, + RemoveCronPermission, + CronUpdateParamsPermission, + ParamChangePermission, +} + +pub enum Validator { + Empty, + CronUpdateParams(MsgUpdateParamsCron), + ParamChange(ParamChangeProposal), +} + +impl Validator { + pub fn validate(self, deps: Deps, permission: Permission) -> Result<(), ContractError> { + // match only possible variants + match (self, permission) { + (Validator::Empty, Permission::AllowAll) => Ok(()), + (Validator::Empty, Permission::AddCronPermission) => Ok(()), + (Validator::Empty, Permission::RemoveCronPermission) => Ok(()), + ( + Validator::CronUpdateParams(msg), + Permission::CronUpdateParamsPermission(permissions), + ) => check_cron_update_msg_params(deps, permissions, msg), + ( + Validator::ParamChange(params_change_proposal), + Permission::ParamChangePermission(permissions), + ) => check_param_change_permission(params_change_proposal, permissions), + _ => unreachable!(), + } + } +} + +// match_permission_type returns PermissionType and Validator, for futher processing +// Validator contains message payload if necessary +pub fn match_permission_type( + msg: &CosmosMsg, +) -> Result<(PermissionType, Validator), ContractError> { + match msg { + CosmosMsg::Custom(NeutronMsg::AddSchedule { .. }) => { + Ok((PermissionType::AddCronPermission, Validator::Empty)) + } + CosmosMsg::Custom(NeutronMsg::RemoveSchedule { .. }) => { + Ok((PermissionType::RemoveCronPermission, Validator::Empty)) + } + CosmosMsg::Custom(NeutronMsg::SubmitAdminProposal { admin_proposal }) => { + match admin_proposal { + AdminProposal::ProposalExecuteMessage(ProposalExecuteMessage { message }) => { + let typed_proposal: ProposalExecuteMessageJSON = + serde_json_wasm::from_str(message.as_str())?; + + if typed_proposal.type_field.as_str() == MSG_TYPE_UPDATE_PARAMS_CRON { + let msg_update_params: MsgUpdateParamsCron = + serde_json_wasm::from_str(message.as_str())?; + return Ok(( + PermissionType::CronUpdateParamsPermission, + Validator::CronUpdateParams(msg_update_params), + )); + }; + Err(ContractError::PermissionTypeNotFound( + "no registered admin proposal persimmisions for the message".to_string(), + )) + } + AdminProposal::ParamChangeProposal(param_change_proposal) => Ok(( + PermissionType::ParamChangePermission, + Validator::ParamChange(param_change_proposal.to_owned()), + )), + _ => Err(ContractError::PermissionTypeNotFound( + "no registered admin proposal persimmisions for the message".to_string(), + )), + } + } + _ => Err(ContractError::PermissionTypeNotFound( + "no registered persimmisions for the message".to_string(), + )), + } +} + +impl Into for Permission { + fn into(self) -> PermissionType { + match self { + Permission::AddCronPermission => PermissionType::AddCronPermission, + Permission::RemoveCronPermission => PermissionType::RemoveCronPermission, + Permission::CronUpdateParamsPermission { .. } => { + PermissionType::CronUpdateParamsPermission + } + Permission::AllowAll => PermissionType::AllowAll, + Permission::ParamChangePermission(_) => PermissionType::ParamChangePermission, + } + } +} + +/// Checks that the strategy owner is authorised to change the parameters of the +/// cron module. We query the current values for each parameter & compare them to +/// the values in the proposal; all modifications must be allowed by the strategy. +fn check_cron_update_msg_params( + deps: Deps, + permission: CronUpdateParamsPermission, + msg: MsgUpdateParamsCron, +) -> Result<(), ContractError> { + let cron_params = get_cron_params(deps, ParamsRequestCron {})?; + if cron_params.params.limit != msg.params.limit && !permission.limit { + return Err(ContractError::Unauthorized {}); + } + + if cron_params.params.security_address != msg.params.security_address + && !permission.security_address + { + return Err(ContractError::Unauthorized {}); + } + + Ok(()) +} + +fn check_param_change_permission( + proposal: ParamChangeProposal, + permissions: ParamChangePermission, +) -> Result<(), ContractError> { + // TODO: keep perms as hashset in storage + let perm: HashSet = HashSet::from_iter(permissions.params.into_iter()); + for param_change in proposal.param_changes { + if perm + .get(&ParamPermission { + subspace: param_change.subspace, + key: param_change.key, + }) + .is_none() + { + return Err(ContractError::Unauthorized {}); + } + } + Ok(()) +} diff --git a/contracts/dao/neutron-chain-manager/src/state.rs b/contracts/dao/neutron-chain-manager/src/state.rs index 370e53ca..28b484f9 100644 --- a/contracts/dao/neutron-chain-manager/src/state.rs +++ b/contracts/dao/neutron-chain-manager/src/state.rs @@ -1,6 +1,6 @@ -use crate::msg::Strategy; +use crate::permission::{Permission, PermissionType}; use cosmwasm_std::Addr; use cw_storage_plus::Map; -/// Defines a mapping from an address to a strategy associated with the address. -pub const STRATEGIES: Map = Map::new("chain-manager-strategies"); +/// Defines a mapping from an address to a permission associated with the address. +pub const PERMISSIONS: Map<(Addr, PermissionType), Permission> = Map::new("permissions"); diff --git a/contracts/dao/neutron-chain-manager/src/testing/tests.rs b/contracts/dao/neutron-chain-manager/src/testing/tests.rs index 75fec437..4fe28b06 100644 --- a/contracts/dao/neutron-chain-manager/src/testing/tests.rs +++ b/contracts/dao/neutron-chain-manager/src/testing/tests.rs @@ -1,12 +1,10 @@ use crate::contract::{ - execute_add_strategy, execute_execute_messages, execute_remove_strategy, instantiate, + execute_add_permissions, execute_execute_messages, execute_remove_strategy, instantiate, }; -use crate::error::ContractError::{InvalidDemotion, InvalidInitialStrategy, Unauthorized}; -use crate::msg::Permission::{CronPermission, ParamChangePermission, UpdateParamsPermission}; -use crate::msg::UpdateParamsPermission::CronUpdateParamsPermission as CronUpdateParamsPermissionEnumField; -use crate::msg::{CronPermission as CronPermissionType, CronUpdateParamsPermission}; -use crate::msg::{InstantiateMsg, Strategy}; -use crate::msg::{ParamChangePermission as ParamChangePermissionType, ParamPermission}; +use crate::error::ContractError::{InvalidDemotion, PermissionTypeNotFound, Unauthorized}; +use crate::msg::{CronUpdateParamsPermission, InstantiateMsg}; +use crate::msg::{ParamChangePermission, ParamPermission}; +use crate::permission::Permission; use crate::testing::mock_querier::mock_dependencies; use cosmwasm_std::testing::{mock_env, mock_info}; use cosmwasm_std::{Addr, BankMsg, Coin, CosmosMsg, Uint128}; @@ -21,28 +19,12 @@ fn test_instantiate() { let env = mock_env(); let info = mock_info("addr1", &[]); - let err = instantiate( - deps.as_mut(), - env.clone(), - info.clone(), - InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowOnly(vec![CronPermission(CronPermissionType { - add_schedule: true, - remove_schedule: true, - })]), - }, - ) - .unwrap_err(); - assert_eq!(err, InvalidInitialStrategy {}); - instantiate( deps.as_mut(), env.clone(), info.clone(), InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, + initial_address: Addr::unchecked("neutron_dao_address".to_string()), }, ) .unwrap(); @@ -59,32 +41,31 @@ fn test_add_strategy() { env.clone(), info.clone(), InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, + initial_address: Addr::unchecked("neutron_dao_address".to_string()), }, ) .unwrap(); // Scenario 1: An ALLOW_ALL strategy is added for a new address (passes). let info = mock_info("neutron_dao_address", &[]); - execute_add_strategy( + execute_add_permissions( deps.as_mut(), info.clone(), Addr::unchecked("addr1".to_string()), - Strategy::AllowAll, + vec![Permission::AllowAll], ) .unwrap(); // Scenario 2: An ALLOW_ONLY strategy is added for a new address (passes). let info = mock_info("neutron_dao_address", &[]); - execute_add_strategy( + execute_add_permissions( deps.as_mut(), info.clone(), Addr::unchecked("addr2".to_string()), - Strategy::AllowOnly(vec![CronPermission(CronPermissionType { - add_schedule: true, - remove_schedule: true, - })]), + vec![ + Permission::AddCronPermission, + Permission::RemoveCronPermission, + ], ) .unwrap(); } @@ -102,122 +83,41 @@ fn test_add_strategy_promotion() { env.clone(), info.clone(), InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, + initial_address: Addr::unchecked("neutron_dao_address".to_string()), }, ) .unwrap(); let info = mock_info("neutron_dao_address", &[]); - execute_add_strategy( + execute_add_permissions( deps.as_mut(), info.clone(), Addr::unchecked("addr2".to_string()), - Strategy::AllowOnly(vec![CronPermission(CronPermissionType { - add_schedule: true, - remove_schedule: true, - })]), + vec![ + Permission::AddCronPermission, + Permission::RemoveCronPermission, + ], ) .unwrap(); - execute_add_strategy( + execute_add_permissions( deps.as_mut(), info.clone(), Addr::unchecked("addr2".to_string()), - Strategy::AllowAll, + vec![Permission::AllowAll], ) .unwrap(); + let info = mock_info("addr2", &[]); - execute_add_strategy( + execute_add_permissions( deps.as_mut(), info.clone(), Addr::unchecked("addr3".to_string()), - Strategy::AllowAll, + vec![Permission::AllowAll], ) .unwrap(); } -/// An ALLOW_ONLY strategy is added for one of the existing ALLOW_ALL address -/// (passes, the demoted address can not make privileged actions). -#[test] -fn test_add_strategy_demotion() { - let mut deps = mock_dependencies(); - let env = mock_env(); - let info = mock_info("addr1", &[]); - - instantiate( - deps.as_mut(), - env.clone(), - info.clone(), - InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, - }, - ) - .unwrap(); - - let info = mock_info("neutron_dao_address", &[]); - execute_add_strategy( - deps.as_mut(), - info.clone(), - Addr::unchecked("addr1".to_string()), - Strategy::AllowAll, - ) - .unwrap(); - execute_add_strategy( - deps.as_mut(), - info.clone(), - Addr::unchecked("addr1".to_string()), - Strategy::AllowOnly(vec![CronPermission(CronPermissionType { - add_schedule: true, - remove_schedule: true, - })]), - ) - .unwrap(); - let info = mock_info("addr1", &[]); - let err = execute_add_strategy( - deps.as_mut(), - info, - Addr::unchecked("addr2".to_string()), - Strategy::AllowAll, - ) - .unwrap_err(); - assert_eq!(err, Unauthorized {}) -} - -/// An ALLOW_ONLY strategy is added for the only existing ALLOW_ALL address -/// (fails). -#[test] -fn test_add_strategy_invalid_demotion() { - let mut deps = mock_dependencies(); - let env = mock_env(); - let info = mock_info("addr1", &[]); - - instantiate( - deps.as_mut(), - env.clone(), - info.clone(), - InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, - }, - ) - .unwrap(); - - let info = mock_info("neutron_dao_address", &[]); - let err = execute_add_strategy( - deps.as_mut(), - info.clone(), - Addr::unchecked("neutron_dao_address".to_string()), - Strategy::AllowOnly(vec![CronPermission(CronPermissionType { - add_schedule: true, - remove_schedule: true, - })]), - ) - .unwrap_err(); - assert_eq!(err, InvalidDemotion {}); -} - #[test] fn test_remove_strategy() { let mut deps = mock_dependencies(); @@ -229,18 +129,17 @@ fn test_remove_strategy() { env.clone(), info.clone(), InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, + initial_address: Addr::unchecked("neutron_dao_address".to_string()), }, ) .unwrap(); let info = mock_info("neutron_dao_address", &[]); - execute_add_strategy( + execute_add_permissions( deps.as_mut(), info.clone(), Addr::unchecked("addr1".to_string()), - Strategy::AllowAll, + vec![Permission::AllowAll], ) .unwrap(); execute_remove_strategy( @@ -251,11 +150,11 @@ fn test_remove_strategy() { .unwrap(); let info = mock_info("addr1", &[]); - let err = execute_add_strategy( + let err = execute_add_permissions( deps.as_mut(), info, Addr::unchecked("addr1".to_string()), - Strategy::AllowAll, + vec![Permission::AllowAll], ) .unwrap_err(); assert_eq!(err, Unauthorized {}) @@ -272,8 +171,7 @@ fn test_remove_strategy_invalid_demotion() { env.clone(), info.clone(), InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, + initial_address: Addr::unchecked("neutron_dao_address".to_string()), }, ) .unwrap(); @@ -310,23 +208,22 @@ pub fn test_execute_execute_message_update_params_cron_authorized() { env.clone(), info.clone(), InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, + initial_address: Addr::unchecked("neutron_dao_address".to_string()), }, ) .unwrap(); let info = mock_info("neutron_dao_address", &[]); - execute_add_strategy( + execute_add_permissions( deps.as_mut(), info.clone(), Addr::unchecked("addr1".to_string()), - Strategy::AllowOnly(vec![UpdateParamsPermission( - CronUpdateParamsPermissionEnumField(CronUpdateParamsPermission { + vec![Permission::CronUpdateParamsPermission( + CronUpdateParamsPermission { security_address: true, limit: true, - }), - )]), + }, + )], ) .unwrap(); @@ -356,23 +253,22 @@ pub fn test_execute_execute_message_update_params_cron_unauthorized_limit() { env.clone(), info.clone(), InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, + initial_address: Addr::unchecked("neutron_dao_address".to_string()), }, ) .unwrap(); let info = mock_info("neutron_dao_address", &[]); - execute_add_strategy( + execute_add_permissions( deps.as_mut(), info.clone(), Addr::unchecked("addr1".to_string()), - Strategy::AllowOnly(vec![UpdateParamsPermission( - CronUpdateParamsPermissionEnumField(CronUpdateParamsPermission { + vec![Permission::CronUpdateParamsPermission( + CronUpdateParamsPermission { security_address: true, limit: false, - }), - )]), + }, + )], ) .unwrap(); @@ -403,23 +299,22 @@ pub fn test_execute_execute_message_update_params_cron_unauthorized_security_add env.clone(), info.clone(), InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, + initial_address: Addr::unchecked("neutron_dao_address".to_string()), }, ) .unwrap(); let info = mock_info("neutron_dao_address", &[]); - execute_add_strategy( + execute_add_permissions( deps.as_mut(), info.clone(), Addr::unchecked("addr1".to_string()), - Strategy::AllowOnly(vec![UpdateParamsPermission( - CronUpdateParamsPermissionEnumField(CronUpdateParamsPermission { + vec![Permission::CronUpdateParamsPermission( + CronUpdateParamsPermission { security_address: false, limit: true, - }), - )]), + }, + )], ) .unwrap(); @@ -452,23 +347,22 @@ pub fn test_execute_execute_message_param_change_success() { env.clone(), info.clone(), InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, + initial_address: Addr::unchecked("neutron_dao_address".to_string()), }, ) .unwrap(); let info = mock_info("neutron_dao_address", &[]); - execute_add_strategy( + execute_add_permissions( deps.as_mut(), info.clone(), Addr::unchecked("addr1".to_string()), - Strategy::AllowOnly(vec![ParamChangePermission(ParamChangePermissionType { + vec![Permission::ParamChangePermission(ParamChangePermission { params: vec![ParamPermission { subspace: "globalfee".to_string(), key: "MinimumGasPricesParam".to_string(), }], - })]), + })], ) .unwrap(); @@ -501,23 +395,22 @@ pub fn test_execute_execute_message_param_change_unauthorized_key() { env.clone(), info.clone(), InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, + initial_address: Addr::unchecked("neutron_dao_address".to_string()), }, ) .unwrap(); let info = mock_info("neutron_dao_address", &[]); - execute_add_strategy( + execute_add_permissions( deps.as_mut(), info.clone(), Addr::unchecked("addr1".to_string()), - Strategy::AllowOnly(vec![ParamChangePermission(ParamChangePermissionType { + vec![Permission::ParamChangePermission(ParamChangePermission { params: vec![ParamPermission { subspace: "globalfee".to_string(), key: "0xdeadbeef".to_string(), }], - })]), + })], ) .unwrap(); @@ -551,23 +444,22 @@ pub fn test_execute_execute_message_param_change_unauthorized_subspace() { env.clone(), info.clone(), InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, + initial_address: Addr::unchecked("neutron_dao_address".to_string()), }, ) .unwrap(); let info = mock_info("neutron_dao_address", &[]); - execute_add_strategy( + execute_add_permissions( deps.as_mut(), info.clone(), Addr::unchecked("addr1".to_string()), - Strategy::AllowOnly(vec![ParamChangePermission(ParamChangePermissionType { + vec![Permission::ParamChangePermission(ParamChangePermission { params: vec![ParamPermission { subspace: "0xdeadbeef".to_string(), key: "MinimumGasPricesParam".to_string(), }], - })]), + })], ) .unwrap(); @@ -589,23 +481,22 @@ pub fn test_execute_execute_unknown_message() { env.clone(), info.clone(), InstantiateMsg { - initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), - initial_strategy: Strategy::AllowAll, + initial_address: Addr::unchecked("neutron_dao_address".to_string()), }, ) .unwrap(); let info = mock_info("neutron_dao_address", &[]); - execute_add_strategy( + execute_add_permissions( deps.as_mut(), info.clone(), Addr::unchecked("addr1".to_string()), - Strategy::AllowOnly(vec![ParamChangePermission(ParamChangePermissionType { + vec![Permission::ParamChangePermission(ParamChangePermission { params: vec![ParamPermission { - subspace: "0xdeadbeef".to_string(), - key: "0xdeadbeef".to_string(), + subspace: "0xdedbeef".to_string(), + key: "0xdedbeef".to_string(), }], - })]), + })], ) .unwrap(); @@ -615,7 +506,10 @@ pub fn test_execute_execute_unknown_message() { let info = mock_info("addr1", &[]); let err = execute_execute_messages(deps.as_mut(), info.clone(), vec![msg]).unwrap_err(); - assert_eq!(err, Unauthorized {}); + assert_eq!( + err, + PermissionTypeNotFound("no registered persimmisions for the message".to_string()) + ); let msg = CosmosMsg::Custom(NeutronMsg::BurnTokens { denom: "0xdeadbeef".to_string(), @@ -625,7 +519,10 @@ pub fn test_execute_execute_unknown_message() { let info = mock_info("addr1", &[]); let err = execute_execute_messages(deps.as_mut(), info.clone(), vec![msg]).unwrap_err(); - assert_eq!(err, Unauthorized {}); + assert_eq!( + err, + PermissionTypeNotFound("no registered persimmisions for the message".to_string()) + ); let msg = CosmosMsg::Custom(NeutronMsg::SubmitAdminProposal { admin_proposal: AdminProposal::ClientUpdateProposal(ClientUpdateProposal { @@ -638,5 +535,10 @@ pub fn test_execute_execute_unknown_message() { let info = mock_info("addr1", &[]); let err = execute_execute_messages(deps.as_mut(), info.clone(), vec![msg]).unwrap_err(); - assert_eq!(err, Unauthorized {}); + assert_eq!( + err, + PermissionTypeNotFound( + "no registered admin proposal persimmisions for the message".to_string() + ) + ); }