Merge pull request #3 from Sui-Volo/audit/ottersec

Audit/ottersec

Author: ilyavolo

liquid_staking/sources/cert.move

@@ -24,6 +24,7 @@ module liquid_staking::cert {
 
     // Track the current version of the module, iterate each upgrade
     const VERSION: u64 = 1;
+    const DECIMALS: u8 = 9;
 
     /* Constants */
 
@@ -49,7 +50,7 @@ module liquid_staking::cert {
     fun init(witness: CERT, ctx: &mut TxContext) {
         // create coin with metadata
         let (treasury_cap, metadata) = coin::create_currency<CERT>(
-            witness, 9, b"voloSUI", b"Volo Staked SUI",
+            witness, DECIMALS, b"voloSUI", b"Volo Staked SUI",
             b"Volo's SUI staking solution provides the best user experience and highest level of decentralization, security, combined with an attractive reward mechanism and instant staking liquidity through a bond-like synthetic token called voloSUI.",
             option::some<Url>(url::new_unsafe_from_bytes(b"https://volo.fi/voloSUI.png")),
             ctx
@@ -159,4 +160,4 @@ module liquid_staking::cert {
     public fun test_init(ctx: &mut TxContext) {
         init(CERT {}, ctx)
     }
-}
+}

liquid_staking/sources/native_pool.move

@@ -46,6 +46,7 @@ module liquid_staking::native_pool {
     const E_REWARD_NOT_IN_THRESHOLD: u64 = 107;
     const E_BAD_SHARES: u64 = 108;
     const E_TOO_BIG_PERCENT: u64 = 109;
+    const E_NOT_ENOUGH_BALANCE: u64 = 110;
 
     /* Events */
     struct StakedEvent has copy, drop {
@@ -188,16 +189,18 @@ module liquid_staking::native_pool {
         *table::borrow(&self.total_staked, self.staked_update_epoch) + pending
     }
 
-    // returns total staked for active epoch
+    /// returns total staked for active epoch
+    /// active stake can be unstaked
     public fun get_total_active_stake(self: &NativePool, ctx: &mut TxContext): u64 {
         let last_active_epoch = self.staked_update_epoch;
         let current_epoch = tx_context::epoch(ctx);
 
         if (last_active_epoch > current_epoch) {
             last_active_epoch = current_epoch;
         };
-
-        *table::borrow(&self.total_staked, last_active_epoch)
+        
+        let pending = get_pending(self);
+        *table::borrow(&self.total_staked, last_active_epoch) + pending
     }
 
     public fun get_total_rewards(self: &NativePool): u64 {
@@ -235,7 +238,7 @@ module liquid_staking::native_pool {
     // we can allow to stake less than 1 SUI
     public entry fun change_min_stake(self: &mut NativePool, _owner_cap: &OwnerCap, value: u64) {
         assert_version(self);
-        assert!(value > 0, E_LIMIT_TOO_LOW);
+        assert!(value > 1000, E_LIMIT_TOO_LOW);
 
         event::emit(MinStakeChangedEvent {
             prev_value: self.min_stake,
@@ -344,7 +347,11 @@ module liquid_staking::native_pool {
     }
 
     fun sub_rewards_unsafe(self: &mut NativePool, value: u64) {
-        self.total_rewards = self.total_rewards - value;
+        if (value > self.total_rewards) {
+            self.total_rewards = 0;
+        } else {
+            self.total_rewards = self.total_rewards - value;
+        };
         event::emit(RewardsUpdated {
             value: self.total_rewards,
         });
@@ -474,6 +481,11 @@ module liquid_staking::native_pool {
     fun stake_pool(self: &mut NativePool, wrapper: &mut SuiSystemState, ctx: &mut TxContext) {
         let pending_value = coin::value(&self.pending);
 
+        let tickets_supply = unstake_ticket::get_total_supply(&self.ticket_metadata);
+        if (pending_value < tickets_supply) {
+            return
+        };
+        pending_value = pending_value - tickets_supply;
         if (pending_value < ONE_SUI) {
             return
         };
@@ -581,10 +593,12 @@ module liquid_staking::native_pool {
         ctx: &mut TxContext
     ): Coin<SUI> {
 
+        assert!(vector::length(&validators) > 0, E_NOTHING_TO_UNSTAKE);
         let i = vector::length(&validators) - 1;
 
-        let total_removed_balance = balance::zero<SUI>();
-        let total_removed_value = 0;
+        let total_removed_value = coin::value(&self.pending);
+        let total_removed_balance = coin::into_balance(coin::split(&mut self.pending, total_removed_value, ctx));
+
         let collectable_reward = 0;
 
         while (total_removed_value < amount_to_unstake) {
@@ -624,6 +638,7 @@ module liquid_staking::native_pool {
         };
 
         // extract our fees
+        assert!(balance::value(&total_removed_balance) >= fee + collectable_reward, E_NOT_ENOUGH_BALANCE);
         let fee_balance = balance::split(&mut total_removed_balance, fee + collectable_reward);
         coin::join(&mut self.collectable_fee, coin::from_balance(fee_balance, ctx));
 
@@ -651,6 +666,9 @@ module liquid_staking::native_pool {
 
     // unstake validators with zero priority and stake to top validator
     public entry fun rebalance(self: &mut NativePool, wrapper: &mut SuiSystemState, ctx: &mut TxContext) {
+        assert_version(self);
+        when_not_paused(self);
+
         // calculate total stake of validators 
         let validators = validator_set::get_bad_validators(&self.validator_set);
         let unstaked_sui = unstake_amount_from_validators(self, wrapper, MAX_UINT_64, 0, validators, ctx);

liquid_staking/sources/validator_set.move

@@ -54,6 +54,7 @@ module liquid_staking::validator_set {
         vaults: Table<address, Vault>, // validator => Vault
         validators: VecMap<address, u64>,
         sorted_validators: vector<address>,
+        is_sorted: bool,
     }
 
     // called only once while native_pool init
@@ -63,6 +64,7 @@ module liquid_staking::validator_set {
             vaults: table::new<address, Vault>(ctx),
             validators: vec_map::empty<address, u64>(),
             sorted_validators: vector::empty<address>(),
+            is_sorted: false,
         }
     }
 
@@ -142,9 +144,11 @@ module liquid_staking::validator_set {
         event::emit(ValidatorsSorted{
             validators: sorted,
         });
+        self.is_sorted = true;
         self.sorted_validators = sorted;
     }
 
+    // @dev add or update validator and priority
     public(friend) fun update_validators(self: &mut ValidatorSet, validators: vector<address>, priorities: vector<u64>) {
         let length = vector::length(&validators);
         assert!(length < MAX_VLDRS_UPDATE, E_TOO_MANY_VLDRS);
@@ -160,6 +164,10 @@ module liquid_staking::validator_set {
             i = i + 1;
         };
 
+        if (length > 0) {
+            self.is_sorted = false;
+        };
+
         assert!(vec_map::size(&self.validators) < MAX_VLDRS_UPDATE, E_TOO_MANY_VLDRS);
     }
 
@@ -227,7 +235,10 @@ module liquid_staking::validator_set {
 
             let staked_sui_to_withdraw;
             let rest_requested_amount = requested_amount - balance::value(&total_withdrawn);
-            if (rest_requested_amount >= MIST_PER_SUI && principal_value > rest_requested_amount && principal_value - rest_requested_amount >= MIST_PER_SUI) {
+            if (rest_requested_amount < MIST_PER_SUI) {
+                rest_requested_amount = MIST_PER_SUI
+            };
+            if (principal_value > rest_requested_amount && principal_value - rest_requested_amount >= MIST_PER_SUI) {
                 // it's possible to split StakedSui
                 staked_sui_to_withdraw = staking_pool::split(staked_sui_mut_ref, rest_requested_amount, ctx);
                 principal_value = rest_requested_amount;

liquid_staking/tests/native_pool_tests.move

@@ -174,7 +174,7 @@ module liquid_staking::native_pool_tests {
         // Stake SUI, epoch = 1
         next_tx(&mut scenario, SENDER);
         {
-            let sui = coin::mint_for_testing<SUI>(MIST_PER_SUI, ctx(&mut scenario)); // 1 SUI
+            let sui = coin::mint_for_testing<SUI>(MIST_PER_SUI * 2, ctx(&mut scenario)); // 1 SUI
             let pool = test_scenario::take_shared<NativePool>(&scenario);
             let metadata = test_scenario::take_shared<Metadata<CERT>>(&scenario);
             let system_state = test_scenario::take_shared<SuiSystemState>(&scenario);
@@ -193,16 +193,16 @@ module liquid_staking::native_pool_tests {
             let pool = test_scenario::take_shared<NativePool>(&scenario);
 
             let total_staked = native_pool::get_total_staked(&pool);
-            test_utils::assert_eq(total_staked, MIST_PER_SUI * 101);
+            test_utils::assert_eq(total_staked, MIST_PER_SUI * 102);
             let total_active_stake = native_pool::get_total_active_stake(&pool, ctx(&mut scenario));
-            test_utils::assert_eq(total_active_stake, MIST_PER_SUI * 100);
+            test_utils::assert_eq(total_active_stake, MIST_PER_SUI * 101);
 
             advance_epoch(&mut scenario);
 
             let total_staked = native_pool::get_total_staked(&pool);
-            test_utils::assert_eq(total_staked, MIST_PER_SUI * 101);
+            test_utils::assert_eq(total_staked, MIST_PER_SUI * 102);
             let total_active_stake = native_pool::get_total_active_stake(&pool, ctx(&mut scenario));
-            test_utils::assert_eq(total_active_stake, MIST_PER_SUI * 101);
+            test_utils::assert_eq(total_active_stake, MIST_PER_SUI * 102);
 
             test_scenario::return_shared(pool);
         };
@@ -223,9 +223,9 @@ module liquid_staking::native_pool_tests {
             coin::burn_for_testing(sui);
 
             let total_staked = native_pool::get_total_staked(&pool);
-            test_utils::assert_eq(total_staked, MIST_PER_SUI * 100);
+            test_utils::assert_eq(total_staked, MIST_PER_SUI * 101);
             let total_active_stake = native_pool::get_total_active_stake(&pool, ctx);
-            test_utils::assert_eq(total_active_stake, MIST_PER_SUI * 100);
+            test_utils::assert_eq(total_active_stake, MIST_PER_SUI * 101);
 
             let ticket_supply = native_pool::get_ticket_supply(&pool);
             test_utils::assert_eq(ticket_supply, MIST_PER_SUI);
@@ -436,9 +436,9 @@ module liquid_staking::native_pool_tests {
             let owner_cap = test_scenario::take_from_sender<OwnerCap>(&scenario);
             let pool = test_scenario::take_shared<NativePool>(&scenario);
 
-            native_pool::change_min_stake(&mut pool, &owner_cap, 2);
+            native_pool::change_min_stake(&mut pool, &owner_cap, 2002);
             let min_stake = native_pool::get_min_stake(&pool);
-            test_utils::assert_eq(min_stake, 2);
+            test_utils::assert_eq(min_stake, 2002);
 
             test_scenario::return_to_address<OwnerCap>(SENDER, owner_cap);
             test_scenario::return_shared(pool);