fix: unnecessary execute msg duplication (#55) (oak-glow-v2 finding 19)

## What's This

Fix unnecessary duplication of `MigrateOldDepositors` and `UpdateConfig` messages.

Also update `migrate_old_depositors` function to throw an error if there are no lotteries left to migrate.

* fix: unnecessary duplication of execute msgs

Unnecessary duplication of MigrateOldDepositors and UpdateConfig messages

contracts/lotto/src/contract.rs

@@ -320,35 +320,7 @@ pub fn execute(
         ExecuteMsg::ExecuteLottery {} => execute_lottery(deps, env, info),
         ExecuteMsg::ExecutePrize { limit } => execute_prize(deps, env, info, limit),
         ExecuteMsg::ExecuteEpochOps {} => execute_epoch_ops(deps, env),
-        ExecuteMsg::UpdateConfig {
-            owner,
-            oracle_addr,
-            reserve_factor,
-            instant_withdrawal_fee,
-            unbonding_period,
-            epoch_interval,
-            max_holders,
-            max_tickets_per_depositor,
-            paused,
-            lotto_winner_boost_config,
-            operator_glow_emission_rate,
-            sponsor_glow_emission_rate,
-        } => execute_update_config(
-            deps,
-            info,
-            owner,
-            oracle_addr,
-            reserve_factor,
-            instant_withdrawal_fee,
-            unbonding_period,
-            epoch_interval,
-            max_holders,
-            max_tickets_per_depositor,
-            paused,
-            lotto_winner_boost_config,
-            operator_glow_emission_rate,
-            sponsor_glow_emission_rate,
-        ),
+        ExecuteMsg::UpdateConfig { .. } => unreachable!(),
         ExecuteMsg::UpdateLotteryConfig {
             lottery_interval,
             block_time,
@@ -364,9 +336,7 @@ pub fn execute(
             prize_distribution,
             round_delta,
         ),
-        ExecuteMsg::MigrateOldDepositors { .. } => Err(ContractError::Std(StdError::generic_err(
-            "Cannot call MigrateLoop when unpaused.",
-        ))),
+        ExecuteMsg::MigrateOldDepositors { .. } => unreachable!(),
     }
 }
 
@@ -2140,24 +2110,30 @@ pub fn migrate_old_depositors(
         // Don't need to include state.current_lottery
         // because nothing has been saved with id state.current_lottery yet
         for i in 0..state.current_lottery {
-            let old_lottery_info = old_read_lottery_info(deps.storage, i);
-
-            let new_lottery_info = LotteryInfo {
-                rand_round: old_lottery_info.rand_round,
-                sequence: old_lottery_info.sequence,
-                awarded: old_lottery_info.awarded,
-                timestamp: Timestamp::from_seconds(0),
-                prize_buckets: old_lottery_info.prize_buckets,
-                number_winners: old_lottery_info.number_winners,
-                page: old_lottery_info.page,
-                glow_prize_buckets: [Uint256::zero(); NUM_PRIZE_BUCKETS],
-                block_height: old_lottery_info.timestamp,
-                total_user_shares: pool.total_user_shares,
-            };
-
-            store_lottery_info(deps.storage, i, &new_lottery_info)?;
-
-            old_remove_lottery_info(deps.storage, i);
+            let old_lottery_info = old_read_lottery_info(deps.storage, i)?;
+
+            if let Some(old_lottery_info) = old_lottery_info {
+                let new_lottery_info = LotteryInfo {
+                    rand_round: old_lottery_info.rand_round,
+                    sequence: old_lottery_info.sequence,
+                    awarded: old_lottery_info.awarded,
+                    timestamp: Timestamp::from_seconds(0),
+                    prize_buckets: old_lottery_info.prize_buckets,
+                    number_winners: old_lottery_info.number_winners,
+                    page: old_lottery_info.page,
+                    glow_prize_buckets: [Uint256::zero(); NUM_PRIZE_BUCKETS],
+                    block_height: old_lottery_info.timestamp,
+                    total_user_shares: pool.total_user_shares,
+                };
+
+                store_lottery_info(deps.storage, i, &new_lottery_info)?;
+
+                old_remove_lottery_info(deps.storage, i);
+            } else {
+                return Err(ContractError::Std(StdError::generic_err(
+                    "Already migrated depositors and lotteries",
+                )));
+            }
         }
 
         // Set paused to false and save

contracts/lotto/src/state.rs

@@ -272,7 +272,7 @@ pub struct LotteryInfo {
     pub total_user_shares: Uint256,
 }
 
-#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
+#[derive(Default, Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
 pub struct OldLotteryInfo {
     pub rand_round: u64,
     pub sequence: String,
@@ -315,19 +315,11 @@ pub fn read_lottery_info(storage: &dyn Storage, lottery_id: u64) -> LotteryInfo
     }
 }
 
-pub fn old_read_lottery_info(storage: &dyn Storage, lottery_id: u64) -> OldLotteryInfo {
-    match bucket_read(storage, OLD_PREFIX_LOTTERY).load(&lottery_id.to_be_bytes()) {
-        Ok(v) => v,
-        _ => OldLotteryInfo {
-            rand_round: 0,
-            sequence: "".to_string(),
-            awarded: false,
-            timestamp: 0,
-            prize_buckets: [Uint256::zero(); NUM_PRIZE_BUCKETS],
-            number_winners: [0; NUM_PRIZE_BUCKETS],
-            page: "".to_string(),
-        },
-    }
+pub fn old_read_lottery_info(
+    storage: &dyn Storage,
+    lottery_id: u64,
+) -> StdResult<Option<OldLotteryInfo>> {
+    bucket_read(storage, OLD_PREFIX_LOTTERY).may_load(&lottery_id.to_be_bytes())
 }
 
 pub fn old_remove_lottery_info(storage: &mut dyn Storage, lottery_id: u64) {

contracts/lotto/src/tests.rs

@@ -15,8 +15,9 @@ use crate::state::{
     old_read_depositor_info, old_read_lottery_info, old_remove_depositor_info, read_depositor_info,
     read_depositor_stats_at_height, read_lottery_info, read_lottery_prizes, read_prize,
     read_sponsor_info, store_depositor_info, store_depositor_stats, Config, DepositorInfo,
-    DepositorStatsInfo, LotteryInfo, OldConfig, OldDepositorInfo, OldPool, OldState, Pool,
-    PrizeInfo, State, CONFIG, OLDCONFIG, OLDPOOL, OLDSTATE, OLD_PRIZES, POOL, PRIZES, STATE,
+    DepositorStatsInfo, LotteryInfo, OldConfig, OldDepositorInfo, OldLotteryInfo, OldPool,
+    OldState, Pool, PrizeInfo, State, CONFIG, OLDCONFIG, OLDPOOL, OLDSTATE, OLD_PRIZES, POOL,
+    PRIZES, STATE,
 };
 use crate::test_helpers::{
     calculate_lottery_prize_buckets, calculate_prize_buckets,
@@ -6366,9 +6367,10 @@ pub fn test_migrate() {
     // Store some old lotteries
 
     for i in 0..old_state.current_lottery {
-        let mut old_lottery = old_read_lottery_info(deps.as_ref().storage, 100);
-
-        old_lottery.sequence = format!("00000{}", i);
+        let old_lottery = OldLotteryInfo {
+            sequence: format!("00000{}", i),
+            ..Default::default()
+        };
 
         old_store_lottery_info(deps.as_mut().storage, i, &old_lottery).unwrap();
     }
@@ -6538,7 +6540,9 @@ pub fn test_migrate() {
     // New Lottery Info
 
     for i in 0..state.current_lottery {
-        let mut old_lottery = old_read_lottery_info(deps.as_ref().storage, 100);
+        let mut old_lottery = old_read_lottery_info(deps.as_ref().storage, 100)
+            .unwrap()
+            .unwrap();
 
         old_lottery.sequence = format!("00000{}", i);