Improve code reuse with proposal_completed_hooks

DA0-DA0 · Nov 7, 2023 · bb69e94 · bb69e94
1 parent f80bc40
commit bb69e94
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 96 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/contracts/proposal/dao-proposal-single/src/contract.rs b/contracts/proposal/dao-proposal-single/src/contract.rs
@@ -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};
@@ -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,
@@ -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 {
@@ -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 {})),
@@ -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,
@@ -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 {})),
@@ -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,
@@ -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())
@@ -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,
@@ -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()))

diff --git a/packages/dao-hooks/Cargo.toml b/packages/dao-hooks/Cargo.toml
@@ -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 }
diff --git a/packages/dao-hooks/src/all_hooks.rs b/packages/dao-hooks/src/all_hooks.rs
@@ -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;
 
@@ -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.

diff --git a/packages/dao-hooks/src/proposal.rs b/packages/dao-hooks/src/proposal.rs
@@ -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,
@@ -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),

diff --git a/packages/dao-pre-propose-base/Cargo.toml b/packages/dao-pre-propose-base/Cargo.toml
@@ -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 }