refactor loading of gov proposals for execution

crates/apps/src/lib/node/ledger/mod.rs

@@ -9,14 +9,11 @@ pub mod tendermint_node;
 use std::convert::TryInto;
 use std::net::SocketAddr;
 use std::path::PathBuf;
-use std::str::FromStr;
 use std::thread;
 
 use byte_unit::Byte;
 use futures::future::TryFutureExt;
 use namada::eth_bridge::ethers::providers::{Http, Provider};
-use namada::governance::storage::keys as governance_storage;
-use namada::types::storage::Key;
 use namada::types::time::DateTimeUtc;
 use namada_sdk::tendermint::abci::request::CheckTxKind;
 use once_cell::unsync::Lazy;
@@ -64,35 +61,6 @@ const ENV_VAR_RAYON_THREADS: &str = "NAMADA_RAYON_THREADS";
 //     }
 //```
 impl Shell {
-    fn load_proposals(&mut self) {
-        let proposals_key = governance_storage::get_commiting_proposals_prefix(
-            self.wl_storage.storage.last_epoch.0,
-        );
-
-        let (proposal_iter, _) =
-            self.wl_storage.storage.iter_prefix(&proposals_key);
-        for (key, _, _) in proposal_iter {
-            let key =
-                Key::from_str(key.as_str()).expect("Key should be parsable");
-            if governance_storage::get_commit_proposal_epoch(&key).unwrap()
-                != self.wl_storage.storage.last_epoch.0
-            {
-                // NOTE: `iter_prefix` iterate over the matching prefix. In this
-                // case  a proposal with grace_epoch 110 will be
-                // matched by prefixes  1, 11 and 110. Thus we
-                // have to skip to the next iteration of
-                //  the cycle for all the prefixes that don't actually match
-                //  the desired epoch.
-                continue;
-            }
-
-            let proposal_id = governance_storage::get_commit_proposal_id(&key);
-            if let Some(id) = proposal_id {
-                self.proposal_data.insert(id);
-            }
-        }
-    }
-
     fn call(&mut self, req: Request) -> Result<Response, Error> {
         match req {
             Request::InitChain(init) => {
@@ -128,7 +96,6 @@ impl Shell {
             }
             Request::FinalizeBlock(finalize) => {
                 tracing::debug!("Request FinalizeBlock");
-                self.load_proposals();
                 self.finalize_block(finalize).map(Response::FinalizeBlock)
             }
             Request::Commit => {

crates/apps/src/lib/node/ledger/shell/finalize_block.rs

@@ -24,7 +24,7 @@ use namada::types::storage::{BlockHash, BlockResults, Epoch, Header};
 use namada::vote_ext::ethereum_events::MultiSignedEthEvent;
 use namada::vote_ext::ethereum_tx_data_variants;
 
-use super::governance::execute_governance_proposals;
+use super::governance::load_and_execute_governance_proposals;
 use super::*;
 use crate::facade::tendermint::abci::types::{Misbehavior, VoteInfo};
 use crate::node::ledger::shell::stats::InternalStats;
@@ -102,7 +102,11 @@ where
         if new_epoch {
             update_allowed_conversions(&mut self.wl_storage)?;
 
-            execute_governance_proposals(self, &mut response)?;
+            load_and_execute_governance_proposals(
+                self,
+                &mut response,
+                current_epoch,
+            )?;
 
             // Copy the new_epoch + pipeline_len - 1 validator set into
             // new_epoch + pipeline_len
@@ -1650,7 +1654,6 @@ mod test_finalize_block {
         // Add a proposal to be executed on next epoch change.
         let mut add_proposal = |proposal_id, vote| {
             let validator = shell.mode.get_validator_address().unwrap().clone();
-            shell.proposal_data.insert(proposal_id);
 
             let proposal = InitProposalData {
                 id: proposal_id,

crates/apps/src/lib/node/ledger/shell/governance.rs

@@ -3,10 +3,10 @@ use std::collections::HashMap;
 use namada::governance::pgf::storage::keys as pgf_storage;
 use namada::governance::pgf::storage::steward::StewardDetail;
 use namada::governance::pgf::{storage as pgf, ADDRESS};
-use namada::governance::storage::keys as gov_storage;
 use namada::governance::storage::proposal::{
     AddRemove, PGFAction, PGFTarget, ProposalType, StoragePgfFunding,
 };
+use namada::governance::storage::{keys as gov_storage, load_proposals};
 use namada::governance::utils::{
     compute_proposal_result, ProposalVotes, TallyResult, TallyType, TallyVote,
     VotePower,
@@ -35,17 +35,35 @@ pub struct ProposalsResult {
     rejected: Vec<u64>,
 }
 
-pub fn execute_governance_proposals<D, H>(
+pub fn load_and_execute_governance_proposals<D, H>(
     shell: &mut Shell<D, H>,
     response: &mut shim::response::FinalizeBlock,
+    current_epoch: Epoch,
+) -> Result<ProposalsResult>
+where
+    D: DB + for<'iter> DBIter<'iter> + Sync + 'static,
+    H: StorageHasher + Sync + 'static,
+{
+    let proposal_ids = load_proposals(&shell.wl_storage, current_epoch)?;
+
+    let proposals_result =
+        execute_governance_proposals(shell, response, proposal_ids)?;
+
+    Ok(proposals_result)
+}
+
+fn execute_governance_proposals<D, H>(
+    shell: &mut Shell<D, H>,
+    response: &mut shim::response::FinalizeBlock,
+    proposal_ids: BTreeSet<u64>,
 ) -> Result<ProposalsResult>
 where
     D: DB + for<'iter> DBIter<'iter> + Sync + 'static,
     H: StorageHasher + Sync + 'static,
 {
     let mut proposals_result = ProposalsResult::default();
 
-    for id in std::mem::take(&mut shell.proposal_data) {
+    for id in proposal_ids {
         let proposal_funds_key = gov_storage::get_funds_key(id);
         let proposal_end_epoch_key = gov_storage::get_voting_end_epoch_key(id);
         let proposal_type_key = gov_storage::get_proposal_type_key(id);

crates/apps/src/lib/node/ledger/shell/mod.rs

@@ -356,8 +356,6 @@ where
     /// limit the how many block heights in the past can the storage be
     /// queried for reading values.
     storage_read_past_height_limit: Option<u64>,
-    /// Proposal execution tracking
-    pub proposal_data: BTreeSet<u64>,
     /// Log of events emitted by `FinalizeBlock` ABCI calls.
     event_log: EventLog,
 }
@@ -533,7 +531,6 @@ where
                 tx_wasm_compilation_cache as usize,
             ),
             storage_read_past_height_limit,
-            proposal_data: BTreeSet::new(),
             // TODO: config event log params
             event_log: EventLog::default(),
         };

crates/governance/src/storage/mod.rs

@@ -7,7 +7,7 @@ pub mod proposal;
 /// Vote structures
 pub mod vote;
 
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 
 use namada_core::borsh::BorshDeserialize;
 use namada_core::types::address::Address;
@@ -317,3 +317,28 @@ where
     let proposal_result: Option<ProposalResult> = storage.read(&key)?;
     Ok(proposal_result)
 }
+
+/// Load proposals for execution in the current epoch.
+pub fn load_proposals<S>(
+    storage: &S,
+    current_epoch: Epoch,
+) -> StorageResult<BTreeSet<u64>>
+where
+    S: StorageRead,
+{
+    let mut ids = BTreeSet::<u64>::new();
+    let proposals_key =
+        governance_keys::get_commiting_proposals_prefix(current_epoch.0);
+    for key_val in namada_state::iter_prefix_bytes(storage, &proposals_key)? {
+        let (key, _) = key_val?;
+        let grace_epoch = governance_keys::get_commit_proposal_epoch(&key)
+            .expect("this key segment should correspond to an epoch number");
+        if current_epoch.0 == grace_epoch {
+            let proposal_id = governance_keys::get_commit_proposal_id(&key)
+                .expect("ths key segment should correspond to a proposal id");
+            ids.insert(proposal_id);
+        }
+    }
+
+    Ok(ids)
+}