fix: consolidated migrate (#614)

## 1. Overview

<!-- What are you changing, removing, or adding in this review? -->

## 2. Implementation details

<!-- Describe the implementation (highlights only) as well as design
rationale. -->

## 3. How to test/use

<!-- How can people test/use this? -->

## 4. Checklist

<!-- Checklist for PR author(s). -->

- [ ] Does the Readme need to be updated?

## 5. Limitations (optional)

<!-- Describe any limitation of the capabilities listed in the Overview
section. -->

## 6. Future Work (optional)

<!-- Describe follow up work, if any. -->

---------

Co-authored-by: magiodev <31893902+magiodev@users.noreply.github.com>

smart-contracts/contracts/cl-vault/src/contract.rs

@@ -4,6 +4,7 @@ use cosmwasm_std::{
     to_json_binary, BankMsg, Binary, Deps, DepsMut, Env, MessageInfo, Reply, Response,
 };
 use cw2::set_contract_version;
+use osmosis_std::types::osmosis::concentratedliquidity::v1beta1::ConcentratedliquidityQuerier;
 
 use crate::error::ContractError;
 use crate::helpers::sort_tokens;
@@ -26,6 +27,7 @@ use crate::state::{
     MigrationStatus, VaultConfig, MIGRATION_STATUS, OLD_VAULT_CONFIG, STRATEGIST_REWARDS,
     VAULT_CONFIG,
 };
+use crate::state::{Position, OLD_POSITION, POSITION};
 use crate::vault::admin::execute_admin;
 use crate::vault::any_deposit::{execute_any_deposit, handle_any_deposit_swap_reply};
 use crate::vault::autocompound::{
@@ -258,31 +260,146 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> Result<Response, Co
     #[allow(deprecated)]
     STRATEGIST_REWARDS.remove(deps.storage);
 
+    //POSITION state migration
+    let old_position = OLD_POSITION.load(deps.storage)?;
+
+    let cl_querier = ConcentratedliquidityQuerier::new(&deps.querier);
+    let pos_response = cl_querier.position_by_id(old_position.position_id)?;
+
+    let new_position: Position = Position {
+        position_id: old_position.position_id,
+        join_time: pos_response
+            .position
+            .unwrap()
+            .position
+            .unwrap()
+            .join_time
+            .unwrap()
+            .seconds
+            .unsigned_abs(),
+        claim_after: None,
+    };
+
+    POSITION.save(deps.storage, &new_position)?;
+    OLD_POSITION.remove(deps.storage);
+
     Ok(response)
 }
 
 #[cfg(test)]
 mod tests {
-    use cosmwasm_std::{
-        coin,
-        testing::{mock_dependencies, mock_env},
-        Addr, Coin, CosmosMsg, Decimal, SubMsg, Uint128,
-    };
-    use prost::Message;
-    use std::str::FromStr;
-
+    use super::*;
     #[allow(deprecated)]
     use crate::{
         rewards::CoinList, state::USER_REWARDS,
         test_tube::initialize::initialize::MAX_SLIPPAGE_HIGH,
     };
     use crate::{
-        state::OldVaultConfig,
+        state::{OldPosition, OldVaultConfig, Position, OLD_POSITION, POSITION},
         test_tube::initialize::initialize::{DENOM_BASE, DENOM_QUOTE, DENOM_REWARD},
     };
+    use cosmwasm_std::{
+        coin,
+        testing::{mock_dependencies, mock_env},
+        Addr, Coin, CosmosMsg, Decimal, SubMsg, Uint128,
+    };
     use osmosis_std::{cosmwasm_to_proto_coins, types::cosmos::bank::v1beta1::MsgMultiSend};
+    use prost::Message;
+    use std::str::FromStr;
 
-    use super::*;
+    pub fn mock_migrate(
+        deps: DepsMut,
+        _env: Env,
+        msg: MigrateMsg,
+    ) -> Result<Response, ContractError> {
+        let old_vault_config = OLD_VAULT_CONFIG.load(deps.storage)?;
+        let new_vault_config = VaultConfig {
+            performance_fee: old_vault_config.performance_fee,
+            treasury: old_vault_config.treasury,
+            swap_max_slippage: old_vault_config.swap_max_slippage,
+            dex_router: deps.api.addr_validate(msg.dex_router.as_str())?,
+        };
+
+        OLD_VAULT_CONFIG.remove(deps.storage);
+        VAULT_CONFIG.save(deps.storage, &new_vault_config)?;
+
+        MIGRATION_STATUS.save(deps.storage, &MigrationStatus::Open)?;
+
+        // Declare response object as mut
+        let mut response = Response::new().add_attribute("migrate", "successful");
+
+        // Conditionally add a bank send message if the strategist rewards state is not empty
+        #[allow(deprecated)]
+        let strategist_rewards = STRATEGIST_REWARDS.load(deps.storage)?;
+        if !strategist_rewards.is_empty() {
+            let bank_send_msg = BankMsg::Send {
+                to_address: new_vault_config.treasury.to_string(),
+                amount: sort_tokens(strategist_rewards.coins()),
+            };
+            response = response.add_message(bank_send_msg);
+        }
+        // Remove the state
+        #[allow(deprecated)]
+        STRATEGIST_REWARDS.remove(deps.storage);
+        let old_position = OLD_POSITION.load(deps.storage)?;
+
+        let new_position: Position = Position {
+            position_id: old_position.position_id,
+            join_time: 0,
+            claim_after: None,
+        };
+
+        POSITION.save(deps.storage, &new_position)?;
+
+        OLD_POSITION.remove(deps.storage);
+
+        Ok(response)
+    }
+
+    #[test]
+    fn test_migrate_position_state() {
+        let mut deps = mock_dependencies();
+        let env = mock_env();
+
+        let new_dex_router = Addr::unchecked("dex_router"); // new field nested in existing VaultConfig state
+
+        // Mock a previous state item
+        OLD_POSITION
+            .save(deps.as_mut().storage, &OldPosition { position_id: 1 })
+            .unwrap();
+        OLD_VAULT_CONFIG
+            .save(
+                deps.as_mut().storage,
+                &OldVaultConfig {
+                    performance_fee: Decimal::from_str("0.2").unwrap(),
+                    treasury: Addr::unchecked("treasury"),
+                    swap_max_slippage: Decimal::bps(MAX_SLIPPAGE_HIGH),
+                },
+            )
+            .unwrap();
+        #[allow(deprecated)]
+        STRATEGIST_REWARDS
+            .save(&mut deps.storage, &CoinList::new())
+            .unwrap();
+
+        mock_migrate(
+            deps.as_mut(),
+            env,
+            MigrateMsg {
+                dex_router: new_dex_router,
+            },
+        )
+        .unwrap();
+
+        let position = POSITION.load(deps.as_mut().storage).unwrap();
+
+        assert_eq!(position.position_id, 1);
+        assert_eq!(position.join_time, 0);
+        assert!(position.claim_after.is_none());
+
+        let old_position = OLD_POSITION.may_load(deps.as_mut().storage).unwrap();
+        assert!(old_position.is_none());
+    }
 
     #[test]
     fn test_migrate_no_rewards() {
@@ -293,6 +410,9 @@ mod tests {
         let new_dex_router = Addr::unchecked("dex_router"); // new field nested in existing VaultConfig state
 
         // Mock a previous state item
+        OLD_POSITION
+            .save(deps.as_mut().storage, &OldPosition { position_id: 1 })
+            .unwrap();
         OLD_VAULT_CONFIG
             .save(
                 deps.as_mut().storage,
@@ -303,14 +423,19 @@ mod tests {
                 },
             )
             .unwrap();
+        #[allow(deprecated)]
+        STRATEGIST_REWARDS
+            .save(&mut deps.storage, &CoinList::new())
+            .unwrap();
 
-        let _ = migrate(
+        mock_migrate(
             deps.as_mut(),
             env.clone(),
             MigrateMsg {
                 dex_router: new_dex_router.clone(),
             },
-        );
+        )
+        .unwrap();
 
         // Assert OLD_VAULT_CONFIG have been correctly removed by unwrapping the error
         OLD_VAULT_CONFIG.load(deps.as_mut().storage).unwrap_err();
@@ -355,6 +480,10 @@ mod tests {
             )
             .unwrap();
 
+        OLD_POSITION
+            .save(deps.as_mut().storage, &OldPosition { position_id: 1 })
+            .unwrap();
+
         // Mock USER_REWARDS in order to have something to iterate over
         let rewards_coins = vec![
             coin(1000u128, DENOM_BASE),
@@ -380,14 +509,15 @@ mod tests {
             )
             .unwrap();
 
-        let migrate_resp = migrate(
+        let migrate_resp = mock_migrate(
             deps.as_mut(),
             env.clone(),
             MigrateMsg {
                 dex_router: new_dex_router.clone(),
             },
         )
         .unwrap();
+
         if let Some(SubMsg {
             msg: CosmosMsg::Bank(BankMsg::Send { to_address, amount }),
             ..

smart-contracts/contracts/cl-vault/src/state.rs

@@ -74,14 +74,20 @@ impl PoolConfig {
 pub const POOL_CONFIG: Item<PoolConfig> = Item::new("pool_config");
 
 /// POSITION
+#[cw_serde]
+pub struct OldPosition {
+    pub position_id: u64,
+}
+
 #[cw_serde]
 pub struct Position {
     pub position_id: u64,
     pub join_time: u64, // env block time at time of creation, or taken by osmosis protocol response
     pub claim_after: Option<u64>, // this should be off chain computed and set in order to avoid forfeiting incentives
 }
 
-pub const POSITION: Item<Position> = Item::new("position");
+pub const OLD_POSITION: Item<OldPosition> = Item::new("position");
+pub const POSITION: Item<Position> = Item::new("position_v2");
 
 pub const SHARES: Map<Addr, Uint128> = Map::new("shares");
 

smart-contracts/contracts/cl-vault/src/vault/autocompound.rs

@@ -141,7 +141,13 @@ pub fn execute_migration_step(
     {
         let (address, rewards) = item?;
 
+        // We always push the address in order to remove it later
         addresses.push(address.clone());
+        // If there are no rewards, we skip the address or we will get invalid_coins error
+        // This is because USER_REWARDS is holding 0 amount coins. rewards.coins() only returns a list of coins with non-zero amounts, which it could be empty
+        if rewards.coins().is_empty() {
+            continue;
+        }
         outputs.push(Output {
             address: address.to_string(),
             coins: cosmwasm_to_proto_coins(rewards.coins().iter().cloned()),