Clear empty accounts (#199)

* Clear empty accounts.

* Update schema.

* Add tests. Cleanup.

* Improve msg.

* Update CM with rc. New deployment.

contracts/account-nft/src/contract.rs

@@ -6,14 +6,14 @@ use cosmwasm_std::{
 use cw2::set_contract_version;
 use cw721_base::Cw721Contract;
 use mars_account_nft_types::{
-    msg::{ExecuteMsg, InstantiateMsg, QueryMsg},
+    msg::{ExecuteMsg, InstantiateMsg, MigrateV1ToV2, QueryMsg},
     nft_config::NftConfig,
 };
 
 use crate::{
     error::ContractError,
     execute::{burn, mint, update_config},
-    migrations,
+    migrations::{self},
     query::{query_config, query_next_id},
     state::{CONFIG, NEXT_ID},
 };
@@ -71,6 +71,11 @@ pub fn execute(
         ExecuteMsg::Burn {
             token_id,
         } => burn(deps, env, info, token_id),
+        ExecuteMsg::Migrate(msg) => match msg {
+            MigrateV1ToV2::BurnEmptyAccounts {
+                limit,
+            } => migrations::v2_0_0::burn_empty_accounts(deps, limit),
+        },
         _ => Parent::default().execute(deps, env, info, msg.try_into()?).map_err(Into::into),
     }
 }

contracts/account-nft/src/migrations/v2_0_0.rs

@@ -1,11 +1,16 @@
-use cosmwasm_std::{DepsMut, Empty, Response};
+use cosmwasm_std::{
+    to_binary, DepsMut, Empty, QueryRequest, Response, StdError, Storage, WasmQuery,
+};
 use cw2::set_contract_version;
+use cw721::Cw721Query;
 use mars_account_nft_types::nft_config::NftConfig;
+use mars_red_bank_types::oracle::ActionKind;
+use mars_rover_health_types::{AccountKind, HealthValuesResponse, QueryMsg::HealthValues};
 
 use crate::{
-    contract::{CONTRACT_NAME, CONTRACT_VERSION},
-    error::ContractError,
-    state::CONFIG,
+    contract::{Parent, CONTRACT_NAME, CONTRACT_VERSION},
+    error::ContractError::{self, HealthContractNotSet},
+    state::{BurningMarker, CONFIG, MIGRATION_BURNING_MARKER},
 };
 
 const FROM_VERSION: &str = "1.0.0";
@@ -42,3 +47,90 @@ pub fn migrate(deps: DepsMut) -> Result<Response, ContractError> {
 
     Ok(cw721_base::upgrades::v0_17::migrate::<Empty, Empty, Empty, Empty>(deps)?)
 }
+
+pub fn burn_empty_accounts(
+    mut deps: DepsMut,
+    limit: Option<u32>,
+) -> Result<Response, ContractError> {
+    let config = CONFIG.load(deps.storage)?;
+    let Some(health_contract_addr) = config.health_contract_addr else {
+        return Err(HealthContractNotSet);
+    };
+
+    let burning_marker_opt = MIGRATION_BURNING_MARKER.may_load(deps.storage)?;
+    let start_after = match burning_marker_opt {
+        Some(BurningMarker::Finished) => {
+            return Err(StdError::generic_err(
+                "Migration completed. All empty accounts already burned.",
+            )
+            .into())
+        }
+        Some(BurningMarker::StartAfter(start_after)) => Some(start_after),
+        None => None,
+    };
+
+    let res = Parent::default().all_tokens(deps.as_ref(), start_after, limit)?;
+
+    let status = if let Some(last_token) = res.tokens.last() {
+        // Save last token for next iteration
+        MIGRATION_BURNING_MARKER
+            .save(deps.storage, &BurningMarker::StartAfter(last_token.clone()))?;
+
+        for token in res.tokens.into_iter() {
+            burn_empty_account(deps.branch(), health_contract_addr.to_string(), token)?;
+        }
+
+        "in_progress".to_string()
+    } else {
+        // No more tokens. Migration finished
+        MIGRATION_BURNING_MARKER.save(deps.storage, &BurningMarker::Finished)?;
+
+        "done".to_string()
+    };
+
+    Ok(Response::new()
+        .add_attribute("action", "burn_empty_accounts")
+        .add_attribute("status", status))
+}
+
+/// A few checks to ensure accounts are not accidentally deleted:
+/// - Cannot burn if debt balance
+/// - Cannot burn if collateral balance
+fn burn_empty_account(
+    deps: DepsMut,
+    health_contract: String,
+    token_id: String,
+) -> Result<(), ContractError> {
+    let response: HealthValuesResponse =
+        deps.querier.query(&QueryRequest::Wasm(WasmQuery::Smart {
+            contract_addr: health_contract,
+            msg: to_binary(&HealthValues {
+                account_id: token_id.clone(),
+                kind: AccountKind::Default, // all current accounts are default
+                action: ActionKind::Default,
+            })?,
+        }))?;
+
+    // Burn only empty accounts
+    if response.total_debt_value.is_zero() && response.total_collateral_value.is_zero() {
+        burn_without_owner_check(deps.storage, token_id)?;
+    }
+
+    Ok(())
+}
+
+fn burn_without_owner_check(
+    storage: &mut dyn Storage,
+    token_id: String,
+) -> Result<(), ContractError> {
+    let parent = Parent::default();
+
+    // Original function has additional check for token owner:
+    // let token = parnet.tokens.load(deps.storage, &token_id)?;
+    // parnet.check_can_send(deps.as_ref(), &env, &info, &token)?;
+
+    Parent::default().tokens.remove(storage, &token_id)?;
+    parent.decrement_tokens(storage)?;
+
+    Ok(())
+}

contracts/account-nft/src/state.rs

@@ -1,5 +1,14 @@
+use cosmwasm_schema::cw_serde;
 use cw_storage_plus::Item;
 use mars_account_nft_types::nft_config::NftConfig;
 
 pub const CONFIG: Item<NftConfig> = Item::new("config");
 pub const NEXT_ID: Item<u64> = Item::new("next_id");
+
+/// Helper marker used during burning empty accounts. Used only for v1 -> v2 migration.
+#[cw_serde]
+pub enum BurningMarker {
+    StartAfter(String),
+    Finished,
+}
+pub const MIGRATION_BURNING_MARKER: Item<BurningMarker> = Item::new("burning_marker");

contracts/account-nft/tests/helpers/mock_env.rs

@@ -8,7 +8,7 @@ use cw721_base::{
 };
 use cw_multi_test::{App, AppResponse, BasicApp, Executor};
 use mars_account_nft_types::{
-    msg::{ExecuteMsg, ExecuteMsg::UpdateConfig, QueryMsg},
+    msg::{ExecuteMsg, ExecuteMsg::UpdateConfig, MigrateV1ToV2, QueryMsg},
     nft_config::{NftConfigUpdates, UncheckedNftConfig},
 };
 use mars_mock_credit_manager::msg::ExecuteMsg::SetAccountKindResponse;
@@ -152,6 +152,21 @@ impl MockEnv {
         )
     }
 
+    pub fn burn_empty_accounts(
+        &mut self,
+        sender: &Addr,
+        limit: Option<u32>,
+    ) -> AnyResult<AppResponse> {
+        self.app.execute_contract(
+            sender.clone(),
+            self.nft_contract.clone(),
+            &ExecuteMsg::Migrate(MigrateV1ToV2::BurnEmptyAccounts {
+                limit,
+            }),
+            &[],
+        )
+    }
+
     pub fn propose_new_minter(
         &mut self,
         sender: &Addr,

contracts/account-nft/tests/test_burn_empty_accounts.rs

@@ -0,0 +1,148 @@
+use cosmwasm_std::{Addr, StdError};
+use mars_account_nft::error::{ContractError, ContractError::HealthContractNotSet};
+use mars_account_nft_types::msg::QueryMsg::{AllTokens, NumTokens, Tokens};
+use mars_rover_health_types::AccountKind;
+
+use crate::helpers::{generate_health_response, MockEnv};
+
+pub mod helpers;
+
+#[test]
+fn burning_empty_accounts_not_allowed_if_no_health_contract_set() {
+    let mut mock = MockEnv::new().instantiate_with_health_contract(false).build().unwrap();
+    let user = Addr::unchecked("user");
+    mock.mint(&user).unwrap();
+    let res = mock.burn_empty_accounts(&user, None);
+    let error: ContractError = res.unwrap_err().downcast().unwrap();
+    assert_eq!(error, HealthContractNotSet);
+}
+
+#[test]
+fn burn_empty_accounts() {
+    let mut mock = MockEnv::new().build().unwrap();
+
+    // create few accounts
+    let user_1 = Addr::unchecked("user_1");
+    let user_1_token_id = mock.mint(&user_1).unwrap();
+    mock.set_health_response(
+        &user_1,
+        &user_1_token_id,
+        AccountKind::Default,
+        &generate_health_response(10_000, 0),
+    );
+    let user_2 = Addr::unchecked("user_2");
+    let user_2_token_id_1 = mock.mint(&user_2).unwrap();
+    mock.set_health_response(
+        &user_2,
+        &user_2_token_id_1,
+        AccountKind::Default,
+        &generate_health_response(0, 0),
+    );
+    let user_2_token_id_2 = mock.mint(&user_2).unwrap();
+    mock.set_health_response(
+        &user_2,
+        &user_2_token_id_2,
+        AccountKind::Default,
+        &generate_health_response(0, 1),
+    );
+    let user_3 = Addr::unchecked("user_3");
+    let user_3_token_id = mock.mint(&user_3).unwrap();
+    mock.set_health_response(
+        &user_3,
+        &user_3_token_id,
+        AccountKind::Default,
+        &generate_health_response(1, 1),
+    );
+    let user_4 = Addr::unchecked("user_4");
+    let user_4_token_id = mock.mint(&user_4).unwrap();
+    mock.set_health_response(
+        &user_4,
+        &user_4_token_id,
+        AccountKind::Default,
+        &generate_health_response(0, 0),
+    );
+    let user_5 = Addr::unchecked("user_5");
+    let user_5_token_id = mock.mint(&user_5).unwrap();
+    mock.set_health_response(
+        &user_5,
+        &user_5_token_id,
+        AccountKind::Default,
+        &generate_health_response(0, 0),
+    );
+    let user_6 = Addr::unchecked("user_6");
+    let user_6_token_id = mock.mint(&user_6).unwrap();
+    mock.set_health_response(
+        &user_6,
+        &user_6_token_id,
+        AccountKind::Default,
+        &generate_health_response(0, 100),
+    );
+
+    // check that all accounts are created
+    let res: cw721::NumTokensResponse =
+        mock.app.wrap().query_wasm_smart(mock.nft_contract.clone(), &NumTokens {}).unwrap();
+    assert_eq!(res.count, 7);
+
+    // check that for user 2 there are 2 tokens
+    let res: cw721::TokensResponse = mock
+        .app
+        .wrap()
+        .query_wasm_smart(
+            mock.nft_contract.clone(),
+            &Tokens {
+                owner: user_2.to_string(),
+                start_after: None,
+                limit: None,
+            },
+        )
+        .unwrap();
+    assert_eq!(res.tokens, vec![user_2_token_id_1.clone(), user_2_token_id_2.clone()]);
+
+    // burn empty accounts
+    let user = Addr::unchecked("random_user");
+    mock.burn_empty_accounts(&user, Some(2)).unwrap();
+    mock.burn_empty_accounts(&user, Some(2)).unwrap();
+    mock.burn_empty_accounts(&user, Some(2)).unwrap();
+    mock.burn_empty_accounts(&user, Some(2)).unwrap();
+    mock.burn_empty_accounts(&user, Some(2)).unwrap(); // set flag to Finished
+    let res = mock.burn_empty_accounts(&user, Some(2));
+    let error: ContractError = res.unwrap_err().downcast().unwrap();
+    assert_eq!(
+        error,
+        ContractError::Std(StdError::generic_err(
+            "Migration completed. All empty accounts already burned."
+        ))
+    );
+
+    // check that only empty accounts are burned
+    let res: cw721::TokensResponse = mock
+        .app
+        .wrap()
+        .query_wasm_smart(
+            mock.nft_contract.clone(),
+            &AllTokens {
+                start_after: None,
+                limit: None,
+            },
+        )
+        .unwrap();
+    assert_eq!(
+        res.tokens,
+        vec![user_1_token_id, user_2_token_id_2.clone(), user_3_token_id, user_6_token_id]
+    );
+
+    // check that for user 2 there is only one token, second one should be burned
+    let res: cw721::TokensResponse = mock
+        .app
+        .wrap()
+        .query_wasm_smart(
+            mock.nft_contract.clone(),
+            &Tokens {
+                owner: user_2.to_string(),
+                start_after: None,
+                limit: None,
+            },
+        )
+        .unwrap();
+    assert_eq!(res.tokens, vec![user_2_token_id_2]);
+}

packages/account-nft-types/src/msg/execute.rs

@@ -26,6 +26,11 @@ pub enum ExecuteMsg {
         token_id: String,
     },
 
+    //--------------------------------------------------------------------------------------------------
+    // Migrate message to work in batches
+    //--------------------------------------------------------------------------------------------------
+    Migrate(MigrateV1ToV2),
+
     //--------------------------------------------------------------------------------------------------
     // Base cw721 messages
     //--------------------------------------------------------------------------------------------------
@@ -67,6 +72,15 @@ pub enum ExecuteMsg {
     UpdateOwnership(Action),
 }
 
+/// Migrate from V1 to V2
+#[cw_serde]
+pub enum MigrateV1ToV2 {
+    /// Burns empty accounts in batches
+    BurnEmptyAccounts {
+        limit: Option<u32>,
+    },
+}
+
 impl TryInto<ParentExecuteMsg<Empty, Empty>> for ExecuteMsg {
     type Error = StdError;
 

packages/account-nft-types/src/msg/mod.rs

@@ -2,6 +2,6 @@ mod execute;
 mod instantiate;
 mod query;
 
-pub use execute::ExecuteMsg;
+pub use execute::{ExecuteMsg, MigrateV1ToV2};
 pub use instantiate::InstantiateMsg;
 pub use query::QueryMsg;