Skip to content

Commit

Permalink
Improve code reuse with proposal_completed_hooks
Browse files Browse the repository at this point in the history
  • Loading branch information
JakeHartnell committed Nov 7, 2023
1 parent 25031f1 commit e2b624a
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 96 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

124 changes: 33 additions & 91 deletions contracts/proposal/dao-proposal-single/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ use cw2::{get_contract_version, set_contract_version, ContractVersion};
use cw_hooks::Hooks;
use cw_storage_plus::Bound;
use cw_utils::{parse_reply_instantiate_data, Duration};
use dao_hooks::proposal::{new_proposal_hooks, proposal_status_changed_hooks};
use dao_hooks::proposal::{
new_proposal_hooks, proposal_completed_hooks, proposal_status_changed_hooks,
};
use dao_hooks::vote::new_vote_hooks;
use dao_interface::voting::IsActiveResponse;
use dao_voting::pre_propose::{PreProposeInfo, ProposalCreationPolicy};
Expand Down Expand Up @@ -40,10 +42,6 @@ use crate::{
pub(crate) const CONTRACT_NAME: &str = "crates.io:dao-proposal-single";
pub(crate) const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION");

/// Message type used for firing hooks to this module's pre-propose
/// module, if one is installed.
type PreProposeHookMsg = dao_pre_propose_base::msg::ExecuteMsg<Empty, Empty>;

#[cfg_attr(not(feature = "library"), entry_point)]
pub fn instantiate(
deps: DepsMut,
Expand Down Expand Up @@ -268,7 +266,6 @@ pub fn execute_veto(
.may_load(deps.storage, proposal_id)?
.ok_or(ContractError::NoSuchProposal { id: proposal_id })?;

// TODO refactor for brevity and better code reuse
match prop.status {
Status::Open => {
match prop.timelock {
Expand All @@ -285,41 +282,28 @@ pub fn execute_veto(
prop.status = Status::Vetoed;
PROPOSALS.save(deps.storage, proposal_id, &prop)?;

let hooks = proposal_status_changed_hooks(
// Add proposal status change hooks
let proposal_status_changed_hooks = proposal_status_changed_hooks(
PROPOSAL_HOOKS,
deps.storage,
proposal_id,
old_status.to_string(),
prop.status.to_string(),
)?;

// TODO refactor into proposal_creation_policy_hooks?
// Add prepropose / deposit module hook which will handle deposit refunds.
let proposal_creation_policy = CREATION_POLICY.load(deps.storage)?;
let hooks = match proposal_creation_policy {
ProposalCreationPolicy::Anyone {} => hooks,
ProposalCreationPolicy::Module { addr } => {
let msg = to_binary(&PreProposeHookMsg::ProposalCompletedHook {
proposal_id,
new_status: prop.status,
})?;
let mut hooks = hooks;
hooks.push(SubMsg::reply_on_error(
WasmMsg::Execute {
contract_addr: addr.into_string(),
msg,
funds: vec![],
},
failed_pre_propose_module_hook_id(),
));
hooks
}
};
let proposal_completed_hooks = proposal_completed_hooks(
proposal_creation_policy,
proposal_id,
prop.status,
)?;

Ok(Response::new()
.add_attribute("action", "veto")
.add_attribute("proposal_id", proposal_id.to_string())
.add_submessages(hooks))
.add_submessages(proposal_status_changed_hooks)
.add_submessages(proposal_completed_hooks))
}
// If timelock is not configured throw error. This should never happen.
None => Err(ContractError::TimelockError(TimelockError::NoTimelock {})),
Expand All @@ -341,7 +325,8 @@ pub fn execute_veto(
prop.status = Status::Vetoed;
PROPOSALS.save(deps.storage, proposal_id, &prop)?;

let hooks = proposal_status_changed_hooks(
// Add proposal status change hooks
let proposal_status_changed_hooks = proposal_status_changed_hooks(
PROPOSAL_HOOKS,
deps.storage,
proposal_id,
Expand All @@ -351,30 +336,17 @@ pub fn execute_veto(

// Add prepropose / deposit module hook which will handle deposit refunds.
let proposal_creation_policy = CREATION_POLICY.load(deps.storage)?;
let hooks = match proposal_creation_policy {
ProposalCreationPolicy::Anyone {} => hooks,
ProposalCreationPolicy::Module { addr } => {
let msg = to_binary(&PreProposeHookMsg::ProposalCompletedHook {
proposal_id,
new_status: prop.status,
})?;
let mut hooks = hooks;
hooks.push(SubMsg::reply_on_error(
WasmMsg::Execute {
contract_addr: addr.into_string(),
msg,
funds: vec![],
},
failed_pre_propose_module_hook_id(),
));
hooks
}
};
let proposal_completed_hooks = proposal_completed_hooks(
proposal_creation_policy,
proposal_id,
prop.status,
)?;

Ok(Response::new()
.add_attribute("action", "veto")
.add_attribute("proposal_id", proposal_id.to_string())
.add_submessages(hooks))
.add_submessages(proposal_status_changed_hooks)
.add_submessages(proposal_completed_hooks))
}
// If timelock is not configured throw error. This should never happen.
None => Err(ContractError::TimelockError(TimelockError::NoTimelock {})),
Expand Down Expand Up @@ -462,7 +434,8 @@ pub fn execute_execute(
}
};

let hooks = proposal_status_changed_hooks(
// Add proposal status change hooks
let proposal_status_changed_hooks = proposal_status_changed_hooks(
PROPOSAL_HOOKS,
deps.storage,
proposal_id,
Expand All @@ -472,28 +445,12 @@ pub fn execute_execute(

// Add prepropose / deposit module hook which will handle deposit refunds.
let proposal_creation_policy = CREATION_POLICY.load(deps.storage)?;
let hooks = match proposal_creation_policy {
ProposalCreationPolicy::Anyone {} => hooks,
ProposalCreationPolicy::Module { addr } => {
let msg = to_binary(&PreProposeHookMsg::ProposalCompletedHook {
proposal_id,
new_status: prop.status,
})?;
let mut hooks = hooks;
hooks.push(SubMsg::reply_on_error(
WasmMsg::Execute {
contract_addr: addr.into_string(),
msg,
funds: vec![],
},
failed_pre_propose_module_hook_id(),
));
hooks
}
};
let proposal_completed_hooks =
proposal_completed_hooks(proposal_creation_policy, proposal_id, prop.status)?;

Ok(response
.add_submessages(hooks)
.add_submessages(proposal_status_changed_hooks)
.add_submessages(proposal_completed_hooks)
.add_attribute("action", "execute")
.add_attribute("sender", info.sender)
.add_attribute("proposal_id", proposal_id.to_string())
Expand Down Expand Up @@ -651,7 +608,8 @@ pub fn execute_close(
prop.status = Status::Closed;
PROPOSALS.save(deps.storage, proposal_id, &prop)?;

let hooks = proposal_status_changed_hooks(
// Add proposal status change hooks
let proposal_status_changed_hooks = proposal_status_changed_hooks(
PROPOSAL_HOOKS,
deps.storage,
proposal_id,
Expand All @@ -661,28 +619,12 @@ pub fn execute_close(

// Add prepropose / deposit module hook which will handle deposit refunds.
let proposal_creation_policy = CREATION_POLICY.load(deps.storage)?;
let hooks = match proposal_creation_policy {
ProposalCreationPolicy::Anyone {} => hooks,
ProposalCreationPolicy::Module { addr } => {
let msg = to_binary(&PreProposeHookMsg::ProposalCompletedHook {
proposal_id,
new_status: prop.status,
})?;
let mut hooks = hooks;
hooks.push(SubMsg::reply_on_error(
WasmMsg::Execute {
contract_addr: addr.into_string(),
msg,
funds: vec![],
},
failed_pre_propose_module_hook_id(),
));
hooks
}
};
let proposal_completed_hooks =
proposal_completed_hooks(proposal_creation_policy, proposal_id, prop.status)?;

Ok(Response::default()
.add_submessages(hooks)
.add_submessages(proposal_status_changed_hooks)
.add_submessages(proposal_completed_hooks)
.add_attribute("action", "close")
.add_attribute("sender", info.sender)
.add_attribute("proposal_id", proposal_id.to_string()))
Expand Down
1 change: 1 addition & 0 deletions packages/dao-hooks/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ cosmwasm-std = { workspace = true }
cosmwasm-schema = { workspace = true }
cw4 = { workspace = true }
cw-hooks = { workspace = true }
dao-pre-propose-base = { workspace = true }
dao-voting = { workspace = true }
4 changes: 3 additions & 1 deletion packages/dao-hooks/src/all_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use cosmwasm_schema::cw_serde;
use cw4::MemberChangedHookMsg;

use crate::nft_stake::NftStakeChangedHookMsg;
use crate::proposal::ProposalHookMsg;
use crate::proposal::{PreProposeHookMsg, ProposalHookMsg};
use crate::stake::StakeChangedHookMsg;
use crate::vote::VoteHookMsg;

Expand All @@ -14,6 +14,8 @@ pub enum DaoHooks {
MemberChangedHook(MemberChangedHookMsg),
/// Called when NFTs are staked or unstaked.
NftStakeChangeHook(NftStakeChangedHookMsg),
/// Pre-propose hooks
PreProposeHook(PreProposeHookMsg),
/// Called when a proposal status changes.
ProposalHook(ProposalHookMsg),
/// Called when tokens are staked or unstaked.
Expand Down
39 changes: 37 additions & 2 deletions packages/dao-hooks/src/proposal.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
use cosmwasm_schema::cw_serde;
use cosmwasm_std::{to_binary, StdResult, Storage, SubMsg, WasmMsg};
use cosmwasm_std::{to_binary, Empty, StdResult, Storage, SubMsg, WasmMsg};
use cw_hooks::Hooks;
use dao_voting::reply::mask_proposal_hook_index;
use dao_voting::{
pre_propose::ProposalCreationPolicy,
reply::{failed_pre_propose_module_hook_id, mask_proposal_hook_index},
status::Status,
};

/// An enum representing proposal hook messages.
/// Either a new propsoal hook, fired when a new proposal is created,
Expand Down Expand Up @@ -88,6 +92,37 @@ pub fn proposal_status_changed_hooks(
Ok(messages)
}

/// Message type used for firing hooks to a proposal module's pre-propose
/// module, if one is installed.
pub type PreProposeHookMsg = dao_pre_propose_base::msg::ExecuteMsg<Empty, Empty>;

/// Adds prepropose / deposit module hook which will handle deposit refunds.
pub fn proposal_completed_hooks(
proposal_creation_policy: ProposalCreationPolicy,
proposal_id: u64,
new_status: Status,
) -> StdResult<Vec<SubMsg>> {
let mut hooks: Vec<SubMsg> = vec![];
match proposal_creation_policy {
ProposalCreationPolicy::Anyone {} => (),
ProposalCreationPolicy::Module { addr } => {
let msg = to_binary(&PreProposeHookMsg::ProposalCompletedHook {
proposal_id,
new_status,
})?;
hooks.push(SubMsg::reply_on_error(
WasmMsg::Execute {
contract_addr: addr.into_string(),
msg,
funds: vec![],
},
failed_pre_propose_module_hook_id(),
));
}
};
Ok(hooks)
}

#[cw_serde]
pub enum ProposalHookExecuteMsg {
ProposalHook(ProposalHookMsg),
Expand Down
1 change: 0 additions & 1 deletion packages/dao-pre-propose-base/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ cw-denom = { workspace = true }
cw-storage-plus = { workspace = true }
cw-utils = { workspace = true }
cw-hooks = { workspace = true }
dao-hooks = { workspace = true }
dao-interface = { workspace = true }
dao-voting = { workspace = true }
serde = { workspace = true }
Expand Down

0 comments on commit e2b624a

Please sign in to comment.