Add modify_position extrinsic, fix active ticks management (#1613)

Author: ales-otf

pallets/subtensor/src/macros/dispatches.rs

@@ -2149,5 +2149,94 @@ mod dispatches {
 
             Ok(())
         }
+
+        /// Modify a liquidity position.
+        ///
+        /// Parameters:
+        /// - origin: The origin of the transaction
+        /// - netuid: Subnet ID
+        /// - position_id: ID of the position to remove
+        /// - liquidity_delta: Liquidity to add (if positive) or remove (if negative)
+        ///
+        /// Emits `Event::LiquidityRemoved` on success
+        #[pallet::call_index(105)]
+        #[pallet::weight((
+		Weight::from_parts(50_000_000, 0)
+			.saturating_add(T::DbWeight::get().reads(4))
+			.saturating_add(T::DbWeight::get().writes(4)),
+            DispatchClass::Operational,
+            Pays::Yes
+        ))]
+        pub fn modify_position(
+            origin: OriginFor<T>,
+            hotkey: T::AccountId,
+            netuid: u16,
+            position_id: u128,
+            liquidity_delta: i64,
+        ) -> DispatchResult {
+            let coldkey = ensure_signed(origin)?;
+
+            // Ensure that the subnet exists.
+            ensure!(
+                Self::if_subnet_exist(netuid),
+                Error::<T>::SubNetworkDoesNotExist
+            );
+
+            // Ensure the hotkey account exists
+            ensure!(
+                Self::hotkey_account_exists(&hotkey),
+                Error::<T>::HotKeyAccountNotExists
+            );
+
+            // Add or remove liquidity
+            let result = T::SwapInterface::modify_position(netuid, &coldkey, &hotkey, position_id, liquidity_delta)?;
+
+            if liquidity_delta > 0 {
+                // Remove TAO and Alpha balances or fail transaction if they can't be removed exactly
+                let tao_provided = Self::remove_balance_from_coldkey_account(&coldkey, result.tao)?;
+                ensure!(tao_provided == result.tao, Error::<T>::InsufficientBalance);
+
+                let alpha_provided = Self::decrease_stake_for_hotkey_and_coldkey_on_subnet(
+                    &hotkey, &coldkey, netuid, result.alpha,
+                );
+                ensure!(alpha_provided == result.alpha, Error::<T>::InsufficientBalance);
+
+                // Emit an event
+                Self::deposit_event(Event::LiquidityAdded {
+                    coldkey,
+                    hotkey,
+                    netuid,
+                    position_id,
+                    liquidity: liquidity_delta as u64,
+                    tao: result.tao,
+                    alpha: result.alpha,
+                });
+            } else {
+                // Credit the returned tao and alpha to the account
+                Self::add_balance_to_coldkey_account(
+                    &coldkey,
+                    result.tao.saturating_add(result.fee_tao),
+                );
+                Self::increase_stake_for_hotkey_and_coldkey_on_subnet(
+                    &hotkey,
+                    &coldkey,
+                    netuid,
+                    result.alpha.saturating_add(result.fee_alpha),
+                );
+
+                // Emit an event
+                Self::deposit_event(Event::LiquidityRemoved {
+                    coldkey,
+                    netuid: netuid.into(),
+                    position_id,
+                    tao: result.tao,
+                    alpha: result.alpha,
+                    fee_tao: result.fee_tao,
+                    fee_alpha: result.fee_alpha,
+                });
+            }
+
+            Ok(())
+        }
     }
 }

pallets/subtensor/src/staking/stake_utils.rs

@@ -3,7 +3,7 @@ use safe_math::*;
 use share_pool::{SharePool, SharePoolDataOperations};
 use sp_std::ops::Neg;
 use substrate_fixed::types::{I64F64, I96F32, U64F64, U96F32};
-use subtensor_swap_interface::{LiquidityDataProvider, OrderType, SwapHandler, SwapResult};
+use subtensor_swap_interface::{OrderType, SwapHandler, SwapResult};
 
 impl<T: Config> Pallet<T> {
     /// Retrieves the total alpha issuance for a given subnet.

pallets/subtensor/src/tests/mock.rs

@@ -21,8 +21,7 @@ use sp_runtime::{
     traits::{BlakeTwo256, IdentityLookup},
 };
 use sp_std::cmp::Ordering;
-use substrate_fixed::types::U64F64;
-use subtensor_swap_interface::{LiquidityDataProvider, OrderType, SwapHandler};
+use subtensor_swap_interface::{OrderType, SwapHandler};
 
 use crate::utils::rate_limiting::TransactionType;
 use crate::*;

pallets/subtensor/src/tests/staking2.rs

@@ -4,7 +4,6 @@ use frame_support::{
     weights::Weight,
 };
 use sp_core::U256;
-use substrate_fixed::types::{I96F32, U96F32};
 use subtensor_swap_interface::SwapHandler;
 
 use super::mock;

pallets/swap-interface/src/lib.rs

@@ -30,6 +30,13 @@ pub trait SwapHandler<AccountId> {
         coldkey_account_id: &AccountId,
         position_id: u128,
     ) -> Result<UpdateLiquidityResult, DispatchError>;
+    fn modify_position(
+        netuid: u16,
+        coldkey_account_id: &AccountId,
+        hotkey_account_id: &AccountId,
+        position_id: u128,
+        liquidity_delta: i64,
+    ) -> Result<UpdateLiquidityResult, DispatchError>;
     fn approx_fee_amount(netuid: u16, amount: u64) -> u64;
     fn current_alpha_price(netuid: u16) -> U96F32;
     fn max_price() -> u64;

pallets/swap/src/pallet/impls.rs

@@ -412,7 +412,6 @@ impl<T: Config> Pallet<T> {
         let mut refund: u64 = 0;
         let mut iteration_counter: u16 = 0;
         let mut in_acc: u64 = 0;
-        let liquidity_before = CurrentLiquidity::<T>::get(netuid);
 
         // Swap one tick at a time until we reach one of the stop conditions
         while amount_remaining > 0 {
@@ -431,6 +430,11 @@ impl<T: Config> Pallet<T> {
                 amount_remaining = 0;
             }
 
+            // The swap step didn't exchange anything
+            if swap_result.amount_to_take == 0 {
+                amount_remaining = 0;
+            }
+
             iteration_counter = iteration_counter.saturating_add(1);
 
             ensure!(
@@ -882,7 +886,7 @@ impl<T: Config> Pallet<T> {
         })
     }
 
-    fn modify_position(
+    pub fn modify_position(
         netuid: NetUid,
         coldkey_account_id: &T::AccountId,
         hotkey_account_id: &T::AccountId,
@@ -1019,6 +1023,9 @@ impl<T: Config> Pallet<T> {
                 });
             }
         });
+
+        // Update active ticks
+        ActiveTickIndexManager::insert::<T>(netuid, tick_index);
     }
 
     /// Remove liquidity at tick index.
@@ -1043,6 +1050,9 @@ impl<T: Config> Pallet<T> {
                 // If no liquidity is left at the tick, remove it
                 if tick.liquidity_gross == 0 {
                     *maybe_tick = None;
+
+                    // Update active ticks: Final liquidity is zero, remove this tick from active.
+                    ActiveTickIndexManager::remove::<T>(netuid, tick_index);
                 }
             }
         });
@@ -1157,6 +1167,17 @@ impl<T: Config> SwapHandler<T::AccountId> for Pallet<T> {
             .map_err(Into::into)
     }
 
+    fn modify_position(
+        netuid: u16,
+        coldkey_account_id: &T::AccountId,
+        hotkey_account_id: &T::AccountId,
+        position_id: u128,
+        liquidity_delta: i64,
+    ) -> Result<UpdateLiquidityResult, DispatchError> {
+        Self::modify_position(netuid.into(), coldkey_account_id, hotkey_account_id, position_id.into(), liquidity_delta)
+            .map_err(Into::into)
+    }
+
     fn approx_fee_amount(netuid: u16, amount: u64) -> u64 {
         Self::calculate_fee_amount(netuid.into(), amount)
     }
@@ -1206,7 +1227,7 @@ pub enum SwapStepAction {
 #[cfg(test)]
 mod tests {
     use approx::assert_abs_diff_eq;
-    use frame_support::{assert_err, assert_noop, assert_ok};
+    use frame_support::{assert_err, assert_ok};
     use sp_arithmetic::helpers_128bit;
 
     use super::*;
@@ -1647,7 +1668,6 @@ mod tests {
     #[test]
     fn test_modify_position_basic() {
         new_test_ext().execute_with(|| {
-            let min_price = tick_to_price(TickIndex::MIN);
             let max_price = tick_to_price(TickIndex::MAX);
             let max_tick = price_to_tick(max_price);
             let limit_price = 1000.0_f64;
@@ -1667,20 +1687,18 @@ mod tests {
                 //     4_000_000_000_u64,
                 // ),
                 // // Repeat the protocol liquidity at current to max range: Expect the same alpha
-                (0.25, max_price, 2_000_000_000_u64, 1_000_000_000, 4_000_000_000),
-                // Repeat the protocol liquidity at min to current range: Expect all the same tao
-                // (min_price, 0.24999, 2_000_000_000_u64, 1_000_000_000, 0),
-                // // Half to double price - just some sane wothdraw amounts
-                // (0.125, 0.5, 2_000_000_000_u64, 293_000_000, 1_171_000_000),
+                (0.25, max_price, 2_000_000_000_u64, 4_000_000_000),
+                // Half to double price - just some sane wothdraw amounts
+                // (0.125, 0.5, 2_000_000_000_u64, 1_171_000_000),
                 // // Both below price - tao is non-zero, alpha is zero
                 // (0.12, 0.13, 2_000_000_000_u64, 28_270_000, 0),
                 // // Both above price - tao is zero, alpha is non-zero
                 // (0.3, 0.4, 2_000_000_000_u64, 0, 489_200_000),
             ]
             .into_iter()
             .enumerate()
-            .map(|(n, v)| (NetUid::from(n as u16), v.0, v.1, v.2, v.3, v.4))
-            .for_each(|(netuid, price_low, price_high, liquidity, tao, alpha)| {
+            .map(|(n, v)| (NetUid::from(n as u16), v.0, v.1, v.2, v.3))
+            .for_each(|(netuid, price_low, price_high, liquidity, alpha)| {
                 // Calculate ticks (assuming tick math is tested separately)
                 let tick_low = price_to_tick(price_low);
                 let tick_high = price_to_tick(price_high);
@@ -1748,10 +1766,11 @@ mod tests {
                         &OK_COLDKEY_ACCOUNT_ID,
                         &OK_HOTKEY_ACCOUNT_ID,
                         position_id,
-                        -1_i64 * ((liquidity / 10) as i64),
+                        -1_i64 * ((liquidity / 100) as i64),
                     )
                     .unwrap();
-                assert_abs_diff_eq!(modify_result.alpha, alpha / 10, epsilon = alpha / 1000);
+
+                assert_abs_diff_eq!(modify_result.alpha, alpha / 100, epsilon = alpha / 1000);
                 assert_eq!(modify_result.fee_tao, 0);
                 assert_eq!(modify_result.fee_alpha, 0);
             });
@@ -2015,10 +2034,6 @@ mod tests {
                         // Calculate the expected output amount for the cornercase of one step
                         let order_liquidity = order_liquidity_fraction * position_liquidity as f64;
 
-                        let input_amount = match order_type {
-                            OrderType::Buy => order_liquidity * sqrt_current_price.to_num::<f64>(),
-                            OrderType::Sell => order_liquidity / sqrt_current_price.to_num::<f64>(),
-                        };
                         let output_amount = match order_type {
                             OrderType::Buy => {
                                 let denom = sqrt_current_price.to_num::<f64>()
@@ -2165,7 +2180,6 @@ mod tests {
             let min_price = tick_to_price(TickIndex::MIN);
             let max_price = tick_to_price(TickIndex::MAX);
             let max_tick = price_to_tick(max_price);
-            let current_price = 0.25;
             let netuid = NetUid(1);
             assert_eq!(max_tick, TickIndex::MAX);
 
@@ -2350,22 +2364,12 @@ mod tests {
             let order_type = OrderType::Sell;
             let liquidity = 1_000_000_000_000_000_000;
             let tick_low = TickIndex::MIN;
-            let tick_high = TickIndex::MAX;
 
             let sqrt_limit_price: SqrtPrice = tick_low.try_to_sqrt_price().unwrap();
 
             // Setup swap
             assert_ok!(Pallet::<Test>::maybe_initialize_v3(netuid));
 
-            // Get tick infos before the swap
-            let tick_low_info_before = Ticks::<Test>::get(netuid, tick_low).unwrap_or_default();
-            let tick_high_info_before = Ticks::<Test>::get(netuid, tick_high).unwrap_or_default();
-            let liquidity_before = CurrentLiquidity::<Test>::get(netuid);
-
-            // Get current price
-            let sqrt_current_price = AlphaSqrtPrice::<Test>::get(netuid);
-            let current_price = (sqrt_current_price * sqrt_current_price).to_num::<f64>();
-
             // Swap
             let swap_result =
                 Pallet::<Test>::do_swap(netuid, order_type, liquidity, sqrt_limit_price, true)
@@ -2374,4 +2378,25 @@ mod tests {
             assert!(swap_result.amount_paid_out > 0);
         });
     }
+
+    // cargo test --package pallet-subtensor-swap --lib -- pallet::impls::tests::test_price_tick_price_roundtrip --exact --show-output
+    #[test]
+    fn test_price_tick_price_roundtrip() {
+        new_test_ext().execute_with(|| {
+            let netuid = NetUid::from(1);
+
+            // Setup swap
+            assert_ok!(Pallet::<Test>::maybe_initialize_v3(netuid));
+
+            let current_price = SqrtPrice::from_num(0.50000051219212275465);
+            let tick = TickIndex::try_from_sqrt_price(current_price).unwrap();
+            let round_trip_price = TickIndex::try_to_sqrt_price(&tick).unwrap();
+            assert!(round_trip_price <= current_price);
+
+            let roundtrip_tick = TickIndex::try_from_sqrt_price(round_trip_price).unwrap();
+            assert!(tick == roundtrip_tick);
+        });
+    }
+
+
 }

pallets/swap/src/pallet/mod.rs

@@ -6,7 +6,7 @@ use substrate_fixed::types::U64F64;
 use subtensor_swap_interface::LiquidityDataProvider;
 
 use crate::{
-    NetUid, SqrtPrice,
+    NetUid,
     position::{Position, PositionId},
     tick::{LayerLevel, Tick, TickIndex},
     weights::WeightInfo,

pallets/swap/src/position.rs

@@ -97,8 +97,8 @@ impl Position {
         let fee_tao_agg = self.fees_in_range::<T>(true);
         let fee_alpha_agg = self.fees_in_range::<T>(false);
 
-        let mut fee_tao = fee_tao_agg.saturating_sub(U64F64::saturating_from_num(self.fees_tao));
-        let mut fee_alpha = fee_alpha_agg.saturating_sub(U64F64::saturating_from_num(self.fees_alpha));
+        let mut fee_tao = fee_tao_agg.saturating_sub(self.fees_tao);
+        let mut fee_alpha = fee_alpha_agg.saturating_sub(self.fees_alpha);
 
         self.fees_tao = fee_tao_agg;
         self.fees_alpha = fee_alpha_agg;

pallets/swap/src/tick.rs

@@ -15,7 +15,7 @@ use substrate_fixed::types::U64F64;
 use subtensor_macros::freeze_struct;
 
 use crate::pallet::{
-    AlphaSqrtPrice, Config, CurrentTick, FeeGlobalAlpha, FeeGlobalTao, TickIndexBitmapWords, Ticks,
+    Config, CurrentTick, FeeGlobalAlpha, FeeGlobalTao, TickIndexBitmapWords, Ticks,
 };
 use crate::{NetUid, SqrtPrice};
 
@@ -692,9 +692,9 @@ pub enum LayerLevel {
     Bottom = 2,
 }
 
-#[freeze_struct("183175773f3f92e0")]
+#[freeze_struct("4015a04919eb5e2e")]
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Encode, Decode, TypeInfo, MaxEncodedLen)]
-struct BitmapLayer {
+pub(crate) struct BitmapLayer {
     word: u32,
     bit: u32,
 }