Skip to content

Commit

Permalink
CL Vault optimize range math operations (#657)
Browse files Browse the repository at this point in the history
## 1. Overview

This PR aims to streamline and optimize the math operations related to
`range.rs` in the scenario of `ModifyRange` message.

## 2. Implementation Details

Simplified the math operation on withdrawal of position reply in the
context of re-ranging. Considering that on the reply, the balances are
already incremented with the withdrawn assets from the previous step, it
is unnecessary to subtract that with the withdrawn amount, only to
re-add them with a `saturating_sub` in between.

Deprecated `CURRENT_BALANCE` state which was unnecessary. It was used
before creating a position, and we were loading it to subtract the
created position amounts from it. This has been replaced with a simple
balances query which, in the context of a reply, already takes into
account previous operations such as creating a position that already
removes funds from the contract balance.
  • Loading branch information
magiodev authored Jul 18, 2024
1 parent 8b1f300 commit 11b9ebe
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 375 deletions.
31 changes: 30 additions & 1 deletion smart-contracts/contracts/cl-vault/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@ use crate::query::{
};
use crate::reply::Replies;
#[allow(deprecated)]
use crate::state::CURRENT_BALANCE;
#[allow(deprecated)]
use crate::state::{
MigrationStatus, VaultConfig, MIGRATION_STATUS, OLD_VAULT_CONFIG, STRATEGIST_REWARDS,
VAULT_CONFIG,
};
#[allow(deprecated)]
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};
Expand Down Expand Up @@ -207,7 +210,7 @@ pub fn reply(deps: DepsMut, env: Env, msg: Reply) -> Result<Response, ContractEr
}
Replies::CollectIncentives => handle_collect_incentives_reply(deps, env, msg.result),
Replies::CollectSpreadRewards => handle_collect_spread_rewards_reply(deps, env, msg.result),
Replies::WithdrawPosition => handle_withdraw_position_reply(deps, env, msg.result),
Replies::WithdrawPosition => handle_withdraw_position_reply(deps, env),
Replies::RangeInitialCreatePosition => {
handle_initial_create_position_reply(deps, env, msg.result)
}
Expand All @@ -228,6 +231,7 @@ pub fn reply(deps: DepsMut, env: Env, msg: Reply) -> Result<Response, ContractEr

#[cfg_attr(not(feature = "library"), entry_point)]
pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> Result<Response, ContractError> {
#[allow(deprecated)]
let old_vault_config = OLD_VAULT_CONFIG.load(deps.storage)?;
let new_vault_config = VaultConfig {
performance_fee: old_vault_config.performance_fee,
Expand All @@ -236,6 +240,7 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> Result<Response, Co
dex_router: deps.api.addr_validate(msg.dex_router.as_str())?,
};

#[allow(deprecated)]
OLD_VAULT_CONFIG.remove(deps.storage);
VAULT_CONFIG.save(deps.storage, &new_vault_config)?;

Expand All @@ -259,6 +264,7 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> Result<Response, Co
STRATEGIST_REWARDS.remove(deps.storage);

//POSITION state migration
#[allow(deprecated)]
let old_position = OLD_POSITION.load(deps.storage)?;

let cl_querier = ConcentratedliquidityQuerier::new(&deps.querier);
Expand All @@ -279,14 +285,18 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> Result<Response, Co
};

POSITION.save(deps.storage, &new_position)?;
#[allow(deprecated)]
OLD_POSITION.remove(deps.storage);
#[allow(deprecated)]
CURRENT_BALANCE.remove(deps.storage);

Ok(response)
}

#[cfg(test)]
mod tests {
use super::*;
#[allow(deprecated)]
use crate::{
helpers::coinlist::CoinList,
state::{OldPosition, OldVaultConfig, Position, OLD_POSITION, POSITION},
Expand All @@ -308,6 +318,7 @@ mod tests {
_env: Env,
msg: MigrateMsg,
) -> Result<Response, ContractError> {
#[allow(deprecated)]
let old_vault_config = OLD_VAULT_CONFIG.load(deps.storage)?;
let new_vault_config = VaultConfig {
performance_fee: old_vault_config.performance_fee,
Expand All @@ -316,6 +327,7 @@ mod tests {
dex_router: deps.api.addr_validate(msg.dex_router.as_str())?,
};

#[allow(deprecated)]
OLD_VAULT_CONFIG.remove(deps.storage);
VAULT_CONFIG.save(deps.storage, &new_vault_config)?;

Expand All @@ -337,6 +349,7 @@ mod tests {
// Remove the state
#[allow(deprecated)]
STRATEGIST_REWARDS.remove(deps.storage);
#[allow(deprecated)]
let old_position = OLD_POSITION.load(deps.storage)?;

let new_position: Position = Position {
Expand All @@ -347,7 +360,10 @@ mod tests {

POSITION.save(deps.storage, &new_position)?;

#[allow(deprecated)]
OLD_POSITION.remove(deps.storage);
#[allow(deprecated)]
CURRENT_BALANCE.remove(deps.storage);

Ok(response)
}
Expand All @@ -360,9 +376,11 @@ mod tests {
let new_dex_router = Addr::unchecked("dex_router"); // new field nested in existing VaultConfig state

// Mock a previous state item
#[allow(deprecated)]
OLD_POSITION
.save(deps.as_mut().storage, &OldPosition { position_id: 1 })
.unwrap();
#[allow(deprecated)]
OLD_VAULT_CONFIG
.save(
deps.as_mut().storage,
Expand Down Expand Up @@ -393,8 +411,13 @@ mod tests {
assert_eq!(position.join_time, 0);
assert!(position.claim_after.is_none());

#[allow(deprecated)]
let old_position = OLD_POSITION.may_load(deps.as_mut().storage).unwrap();
assert!(old_position.is_none());

#[allow(deprecated)]
let current_balance = CURRENT_BALANCE.may_load(deps.as_mut().storage).unwrap();
assert!(current_balance.is_none());
}

#[test]
Expand All @@ -406,9 +429,11 @@ mod tests {
let new_dex_router = Addr::unchecked("dex_router"); // new field nested in existing VaultConfig state

// Mock a previous state item
#[allow(deprecated)]
OLD_POSITION
.save(deps.as_mut().storage, &OldPosition { position_id: 1 })
.unwrap();
#[allow(deprecated)]
OLD_VAULT_CONFIG
.save(
deps.as_mut().storage,
Expand All @@ -434,6 +459,7 @@ mod tests {
.unwrap();

// Assert OLD_VAULT_CONFIG have been correctly removed by unwrapping the error
#[allow(deprecated)]
OLD_VAULT_CONFIG.load(deps.as_mut().storage).unwrap_err();

// Assert new VAULT_CONFIG.dex_router field have correct value
Expand Down Expand Up @@ -465,6 +491,7 @@ mod tests {
let new_dex_router = Addr::unchecked("dex_router"); // new field nested in existing VaultConfig state

// Mock a previous state item
#[allow(deprecated)]
OLD_VAULT_CONFIG
.save(
deps.as_mut().storage,
Expand All @@ -476,6 +503,7 @@ mod tests {
)
.unwrap();

#[allow(deprecated)]
OLD_POSITION
.save(deps.as_mut().storage, &OldPosition { position_id: 1 })
.unwrap();
Expand Down Expand Up @@ -530,6 +558,7 @@ mod tests {
STRATEGIST_REWARDS.load(deps.as_mut().storage).unwrap_err();

// Assert OLD_VAULT_CONFIG have been correctly removed by unwrapping the error
#[allow(deprecated)]
OLD_VAULT_CONFIG.load(deps.as_mut().storage).unwrap_err();

// Assert new VAULT_CONFIG.dex_router field have correct value
Expand Down
Loading

0 comments on commit 11b9ebe

Please sign in to comment.