minor refactor governance

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

@@ -74,27 +74,31 @@ where
         )?;
         let proposal_result =
             compute_proposal_result(votes, total_voting_power, tally_type);
-        let proposal_result_key = gov_storage::get_proposal_result_key(id);
-        shell
-            .wl_storage
-            .write(&proposal_result_key, proposal_result)?;
+        gov_api::write_proposal_result(
+            &mut shell.wl_storage,
+            id,
+            proposal_result,
+        )?;
 
         let transfer_address = match proposal_result.result {
             TallyResult::Passed => {
                 let proposal_event = match proposal_type {
                     ProposalType::Default(_) => {
-                        let proposal_code_key =
-                            gov_storage::get_proposal_code_key(id);
                         let proposal_code =
-                            shell.wl_storage.read_bytes(&proposal_code_key)?;
+                            gov_api::get_proposal_code(&shell.wl_storage, id)?;
                         let result = execute_default_proposal(
                             shell,
                             id,
                             proposal_code.clone(),
                         )?;
                         tracing::info!(
-                            "Governance proposal (default) {} has been \
-                             executed ({}) and passed.",
+                            "Governance proposal (default {} wasm) {} has \
+                             been executed ({}) and passed.",
+                            if proposal_code.is_some() {
+                                "with"
+                            } else {
+                                "without"
+                            },
                             id,
                             result
                         );
@@ -156,8 +160,7 @@ where
                 response.events.push(proposal_event);
                 proposals_result.passed.push(id);
 
-                let proposal_author_key = gov_storage::get_author_key(id);
-                shell.wl_storage.read::<Address>(&proposal_author_key)?
+                gov_api::get_proposal_author(&shell.wl_storage, id)?
             }
             TallyResult::Rejected => {
                 if let ProposalType::PGFPayment(_) = proposal_type {
@@ -191,7 +194,7 @@ where
             }
         };
 
-        let native_token = shell.wl_storage.storage.native_token.clone();
+        let native_token = shell.wl_storage.get_native_token()?;
         if let Some(address) = transfer_address {
             token::transfer(
                 &mut shell.wl_storage,

crates/governance/src/storage/mod.rs

@@ -127,6 +127,20 @@ where
     Ok(())
 }
 
+/// Write the proposal result to storage.
+pub fn write_proposal_result<S>(
+    storage: &mut S,
+    proposal_id: u64,
+    proposal_result: ProposalResult,
+) -> StorageResult<()>
+where
+    S: StorageRead + StorageWrite,
+{
+    let proposal_result_key =
+        governance_keys::get_proposal_result_key(proposal_id);
+    storage.write(&proposal_result_key, proposal_result)
+}
+
 /// Read a proposal by id from storage
 pub fn get_proposal_by_id<S>(
     storage: &S,
@@ -209,17 +223,40 @@ pub fn is_proposal_accepted<S>(
 where
     S: StorageRead,
 {
-    let proposal_id = u64::try_from_slice(tx_data).ok();
-    match proposal_id {
-        Some(id) => {
-            let proposal_execution_key =
-                governance_keys::get_proposal_execution_key(id);
-            storage.has_key(&proposal_execution_key)
-        }
-        None => Ok(false),
+    let proposal_id = u64::try_from_slice(tx_data);
+    if let Ok(id) = proposal_id {
+        let proposal_execution_key =
+            governance_keys::get_proposal_execution_key(id);
+        storage.has_key(&proposal_execution_key)
+    } else {
+        Ok(false)
     }
 }
 
+/// Get the code associated with a proposal
+pub fn get_proposal_code<S>(
+    storage: &S,
+    proposal_id: u64,
+) -> StorageResult<Option<Vec<u8>>>
+where
+    S: StorageRead,
+{
+    let proposal_code_key = governance_keys::get_proposal_code_key(proposal_id);
+    storage.read_bytes(&proposal_code_key)
+}
+
+/// Get the code associated with a proposal
+pub fn get_proposal_author<S>(
+    storage: &S,
+    proposal_id: u64,
+) -> StorageResult<Option<Address>>
+where
+    S: StorageRead,
+{
+    let proposal_author_key = governance_keys::get_author_key(proposal_id);
+    storage.read::<Address>(&proposal_author_key)
+}
+
 /// Get governance parameters
 pub fn get_parameters<S>(storage: &S) -> StorageResult<GovernanceParameters>
 where

crates/governance/src/utils.rs

@@ -277,18 +277,15 @@ impl TallyVote {
 
     /// Check if two votes are equal, returns an error if the variants of the
     /// two instances are different
-    pub fn is_same_side(
-        &self,
-        other: &TallyVote,
-    ) -> Result<bool, &'static str> {
+    pub fn is_same_side(&self, other: &TallyVote) -> bool {
         match (self, other) {
             (TallyVote::OnChain(vote), TallyVote::OnChain(other_vote)) => {
-                Ok(vote == other_vote)
+                vote == other_vote
             }
             (TallyVote::Offline(vote), TallyVote::Offline(other_vote)) => {
-                Ok(vote.vote == other_vote.vote)
+                vote.vote == other_vote.vote
             }
-            _ => Err("Cannot compare different variants of governance votes"),
+            _ => false,
         }
     }
 }
@@ -384,27 +381,8 @@ pub fn compute_proposal_result(
             let validator_vote = votes.validators_vote.get(&validator);
             if let Some(validator_vote) = validator_vote {
                 let validator_vote_is_same_side =
-                    match validator_vote.is_same_side(delegator_vote) {
-                        Ok(result) => result,
-                        Err(_) => {
-                            // Unexpected path, all the votes should be
-                            // validated by the VP and only online votes should
-                            // be allowed in storage
-                            tracing::warn!(
-                                "Found unexpected offline vote type: forcing \
-                                 the proposal to fail."
-                            );
-                            // Force failure of the proposal
-                            return ProposalResult {
-                                result: TallyResult::Rejected,
-                                tally_type,
-                                total_voting_power: VotePower::default(),
-                                total_yay_power: VotePower::default(),
-                                total_nay_power: VotePower::default(),
-                                total_abstain_power: VotePower::default(),
-                            };
-                        }
-                    };
+                    validator_vote.is_same_side(delegator_vote);
+
                 if !validator_vote_is_same_side {
                     if delegator_vote.is_yay() {
                         yay_voting_power += voting_power;
@@ -1314,4 +1292,112 @@ mod test {
             "abstain"
         );
     }
+
+    #[test]
+    fn test_proposal_fourteen() {
+        let mut proposal_votes = ProposalVotes::default();
+
+        let validator_address = address::testing::established_address_1();
+        let validator_address_two = address::testing::established_address_2();
+
+        let delegator_address_two = address::testing::established_address_3();
+        let delegator_voting_power_two = token::Amount::from_u64(30);
+        proposal_votes.add_delegator(
+            &delegator_address_two,
+            &validator_address_two,
+            delegator_voting_power_two,
+            ProposalVote::Nay.into(),
+        );
+
+        let delegator_address = address::testing::established_address_4();
+        let delegator_voting_power = token::Amount::from_u64(60);
+        proposal_votes.add_delegator(
+            &delegator_address,
+            &validator_address,
+            delegator_voting_power,
+            ProposalVote::Nay.into(),
+        );
+
+        let proposal_result = compute_proposal_result(
+            proposal_votes.clone(),
+            token::Amount::from(100),
+            TallyType::LessOneHalfOverOneThirdNay,
+        );
+
+        assert!(matches!(proposal_result.result, TallyResult::Rejected));
+        assert_eq!(
+            proposal_result.total_voting_power,
+            token::Amount::from(100),
+            "total"
+        );
+        assert_eq!(
+            proposal_result.total_yay_power,
+            token::Amount::from(0),
+            "yay"
+        );
+        assert_eq!(
+            proposal_result.total_nay_power,
+            token::Amount::from(90),
+            "nay"
+        );
+        assert_eq!(
+            proposal_result.total_abstain_power,
+            token::Amount::zero(),
+            "abstain"
+        );
+    }
+
+    #[test]
+    fn test_proposal_fifthteen() {
+        let mut proposal_votes = ProposalVotes::default();
+
+        let validator_address = address::testing::established_address_1();
+        let validator_address_two = address::testing::established_address_2();
+
+        let delegator_address_two = address::testing::established_address_3();
+        let delegator_voting_power_two = token::Amount::from_u64(30);
+        proposal_votes.add_delegator(
+            &delegator_address_two,
+            &validator_address_two,
+            delegator_voting_power_two,
+            ProposalVote::Nay.into(),
+        );
+
+        let delegator_address = address::testing::established_address_4();
+        let delegator_voting_power = token::Amount::from_u64(60);
+        proposal_votes.add_delegator(
+            &delegator_address,
+            &validator_address,
+            delegator_voting_power,
+            ProposalVote::Nay.into(),
+        );
+
+        let proposal_result = compute_proposal_result(
+            proposal_votes.clone(),
+            token::Amount::from(271),
+            TallyType::LessOneHalfOverOneThirdNay,
+        );
+
+        assert!(matches!(proposal_result.result, TallyResult::Passed));
+        assert_eq!(
+            proposal_result.total_voting_power,
+            token::Amount::from(271),
+            "total"
+        );
+        assert_eq!(
+            proposal_result.total_yay_power,
+            token::Amount::from(0),
+            "yay"
+        );
+        assert_eq!(
+            proposal_result.total_nay_power,
+            token::Amount::from(90),
+            "nay"
+        );
+        assert_eq!(
+            proposal_result.total_abstain_power,
+            token::Amount::zero(),
+            "abstain"
+        );
+    }
 }

crates/sdk/src/queries/vp/token.rs

@@ -9,7 +9,7 @@ use crate::queries::RequestCtx;
 
 router! {TOKEN,
     ( "denomination" / [addr: Address] ) -> Option<token::Denomination> = denomination,
-    ( "total_supply" / [addr: Address] ) -> Option<token::Amount> = total_supply,
+    ( "total_supply" / [addr: Address] ) -> token::Amount = total_supply,
 }
 
 /// Get the number of decimal places (in base 10) for a

crates/sdk/src/rpc.rs

@@ -180,11 +180,11 @@ pub async fn get_token_balance<C: crate::queries::Client + Sync>(
     )
 }
 
-/// Query token total supply;
+/// Query token total supply.
 pub async fn get_token_total_supply<C: crate::queries::Client + Sync>(
     client: &C,
     token: &Address,
-) -> Result<Option<token::Amount>, error::Error> {
+) -> Result<token::Amount, error::Error> {
     convert_response::<C, _>(RPC.vp().token().total_supply(client, token).await)
 }