Merge pull request #775 from multiversx/farm-position-audit-fixes-2

audit fixes (2)
multiversx · Sep 26, 2023 · 1d92432 · 1d92432
2 parents 8ac9c19 + aba2e4e
commit 1d92432
Show file tree

Hide file tree

Showing 11 changed files with 106 additions and 38 deletions.
diff --git a/common/modules/farm/config/src/config.rs b/common/modules/farm/config/src/config.rs
@@ -7,6 +7,7 @@ use common_structs::Nonce;
 use pausable::State;
 
 pub const DEFAULT_NFT_DEPOSIT_MAX_LEN: usize = 10;
+pub const DEFAULT_FARM_POSITION_MIGRATION_NONCE: u64 = 1;
 
 #[derive(
     ManagedVecItem,
@@ -35,6 +36,15 @@ impl<M: ManagedTypeApi> Default for UserTotalFarmPosition<M> {
 
 #[multiversx_sc::module]
 pub trait ConfigModule: pausable::PausableModule + permissions_module::PermissionsModule {
+    #[endpoint(allowExternalClaimBoostedRewards)]
+    fn allow_external_claim_boosted_rewards(&self, allow_external_claim: bool) {
+        let caller = self.blockchain().get_caller();
+        let mut user_total_farm_position = self.get_user_total_farm_position(&caller);
+        user_total_farm_position.allow_external_claim_boosted_rewards = allow_external_claim;
+        self.user_total_farm_position(&caller)
+            .set(user_total_farm_position);
+    }
+
     #[inline]
     fn is_active(&self) -> bool {
         let state = self.state().get();
@@ -54,21 +64,30 @@ pub trait ConfigModule: pausable::PausableModule + permissions_module::Permissio
     }
 
     fn is_old_farm_position(&self, token_nonce: Nonce) -> bool {
-        let farm_position_migration_block_nonce = self.farm_position_migration_block_nonce().get();
-        token_nonce > 0 && token_nonce < farm_position_migration_block_nonce
+        let farm_position_migration_nonce = self.farm_position_migration_nonce().get();
+        token_nonce > 0 && token_nonce < farm_position_migration_nonce
     }
 
-    #[endpoint(allowExternalClaimBoostedRewards)]
-    fn allow_external_claim_boosted_rewards(&self, allow_external_claim: bool) {
-        let caller = self.blockchain().get_caller();
-        let user_total_farm_position_mapper = self.user_total_farm_position(&caller);
-        require!(
-            !user_total_farm_position_mapper.is_empty(),
-            "User must have a farm position"
-        );
-        user_total_farm_position_mapper.update(|user_total_farm_position| {
-            user_total_farm_position.allow_external_claim_boosted_rewards = allow_external_claim;
-        });
+    fn try_set_farm_position_migration_nonce(
+        &self,
+        farm_token_mapper: NonFungibleTokenMapper<Self::Api>,
+    ) {
+        if !self.farm_position_migration_nonce().is_empty() {
+            return;
+        }
+
+        let migration_farm_token_nonce = if farm_token_mapper.get_token_state().is_set() {
+            let token_identifier = farm_token_mapper.get_token_id_ref();
+            let current_nonce = self
+                .blockchain()
+                .get_current_esdt_nft_nonce(&self.blockchain().get_sc_address(), token_identifier);
+            current_nonce + DEFAULT_FARM_POSITION_MIGRATION_NONCE
+        } else {
+            DEFAULT_FARM_POSITION_MIGRATION_NONCE
+        };
+
+        self.farm_position_migration_nonce()
+            .set(migration_farm_token_nonce);
     }
 
     #[view(getFarmingTokenId)]
@@ -101,7 +120,7 @@ pub trait ConfigModule: pausable::PausableModule + permissions_module::Permissio
         user: &ManagedAddress,
     ) -> SingleValueMapper<UserTotalFarmPosition<Self::Api>>;
 
-    #[view(getFarmPositionMigrationBlockNonce)]
-    #[storage_mapper("farm_position_migration_block_nonce")]
-    fn farm_position_migration_block_nonce(&self) -> SingleValueMapper<Nonce>;
+    #[view(getFarmPositionMigrationNonce)]
+    #[storage_mapper("farm_position_migration_nonce")]
+    fn farm_position_migration_nonce(&self) -> SingleValueMapper<Nonce>;
 }
diff --git a/common/modules/farm/farm_base_impl/src/base_traits_impl.rs b/common/modules/farm/farm_base_impl/src/base_traits_impl.rs
@@ -196,12 +196,12 @@ pub trait FarmContract {
     ) {
         let farm_token_mapper = sc.farm_token();
         for farm_position in farm_positions {
+            farm_token_mapper.require_same_token(&farm_position.token_identifier);
+
             if sc.is_old_farm_position(farm_position.token_nonce) {
                 continue;
             }
 
-            farm_token_mapper.require_same_token(&farm_position.token_identifier);
-
             let token_attributes: FarmTokenAttributes<<Self::FarmSc as ContractBase>::Api> =
                 farm_token_mapper.get_token_attributes(farm_position.token_nonce);
 

diff --git a/dex/farm-with-locked-rewards/src/lib.rs b/dex/farm-with-locked-rewards/src/lib.rs
@@ -75,6 +75,10 @@ pub trait Farm:
 
         let current_epoch = self.blockchain().get_block_epoch();
         self.first_week_start_epoch().set_if_empty(current_epoch);
+
+        // Farm position migration code
+        let farm_token_mapper = self.farm_token();
+        self.try_set_farm_position_migration_nonce(farm_token_mapper);
     }
 
     #[payable("*")]
@@ -155,11 +159,13 @@ pub trait Farm:
 
         let payment = self.call_value().single_esdt();
 
-        self.migrate_old_farm_positions(&orig_caller);
+        let migrated_amount = self.migrate_old_farm_positions(&orig_caller);
 
         let exit_farm_result = self.exit_farm::<NoMintWrapper<Self>>(orig_caller.clone(), payment);
-        let rewards = exit_farm_result.rewards;
 
+        self.decrease_old_farm_positions(migrated_amount, &orig_caller);
+
+        let rewards = exit_farm_result.rewards;
         self.send_payment_non_zero(&caller, &exit_farm_result.farming_tokens);
 
         let locked_rewards_payment = self.send_to_lock_contract_non_zero(

diff --git a/dex/farm-with-locked-rewards/wasm/src/lib.rs b/dex/farm-with-locked-rewards/wasm/src/lib.rs
@@ -40,7 +40,7 @@ multiversx_sc_wasm_adapter::endpoints! {
         getLastRewardBlockNonce => last_reward_block_nonce
         getDivisionSafetyConstant => division_safety_constant
         getUserTotalFarmPosition => user_total_farm_position
-        getFarmPositionMigrationBlockNonce => farm_position_migration_block_nonce
+        getFarmPositionMigrationNonce => farm_position_migration_nonce
         setLockingScAddress => set_locking_sc_address
         setLockEpochs => set_lock_epochs
         getLockingScAddress => locking_sc_address

diff --git a/dex/farm/src/base_functions.rs b/dex/farm/src/base_functions.rs
@@ -19,6 +19,8 @@ pub type DoubleMultiPayment<M> = MultiValue2<EsdtTokenPayment<M>, EsdtTokenPayme
 pub type ClaimRewardsResultType<M> = DoubleMultiPayment<M>;
 pub type ExitFarmResultType<M> = DoubleMultiPayment<M>;
 
+pub const DEFAULT_FARM_POSITION_MIGRATION_NONCE: u64 = 1;
+
 pub struct ClaimRewardsResultWrapper<M: ManagedTypeApi> {
     pub new_farm_token: EsdtTokenPayment<M>,
     pub rewards: EsdtTokenPayment<M>,
@@ -204,20 +206,37 @@ pub trait BaseFunctionsModule:
         reward
     }
 
-    fn migrate_old_farm_positions(&self, caller: &ManagedAddress) {
+    fn migrate_old_farm_positions(&self, caller: &ManagedAddress) -> BigUint {
         let payments = self.get_non_empty_payments();
         let farm_token_mapper = self.farm_token();
         let farm_token_id = farm_token_mapper.get_token_id();
+        let mut migrated_amount = BigUint::zero();
         for farm_position in &payments {
             if farm_position.token_identifier == farm_token_id
                 && self.is_old_farm_position(farm_position.token_nonce)
             {
-                let mut user_total_farm_position = self.get_user_total_farm_position(caller);
-                user_total_farm_position.total_farm_position += farm_position.amount;
-                self.user_total_farm_position(caller)
-                    .set(user_total_farm_position);
+                migrated_amount += farm_position.amount;
             }
         }
+
+        if migrated_amount > 0 {
+            let mut user_total_farm_position = self.get_user_total_farm_position(caller);
+            user_total_farm_position.total_farm_position += &migrated_amount;
+            self.user_total_farm_position(caller)
+                .set(user_total_farm_position);
+        }
+
+        migrated_amount
+    }
+
+    fn decrease_old_farm_positions(&self, migrated_amount: BigUint, caller: &ManagedAddress) {
+        if migrated_amount == BigUint::zero() {
+            return;
+        }
+        self.user_total_farm_position(caller)
+            .update(|user_total_farm_position| {
+                user_total_farm_position.total_farm_position -= migrated_amount;
+            });
     }
 
     fn end_produce_rewards<FC: FarmContract<FarmSc = Self>>(&self) {

diff --git a/dex/farm/src/lib.rs b/dex/farm/src/lib.rs
@@ -78,9 +78,8 @@ pub trait Farm:
         self.first_week_start_epoch().set_if_empty(current_epoch);
 
         // Farm position migration code
-        let block_nonce = self.blockchain().get_block_nonce();
-        self.farm_position_migration_block_nonce()
-            .set_if_empty(block_nonce);
+        let farm_token_mapper = self.farm_token();
+        self.try_set_farm_position_migration_nonce(farm_token_mapper);
     }
 
     #[payable("*")]
@@ -159,10 +158,12 @@ pub trait Farm:
 
         let payment = self.call_value().single_esdt();
 
-        self.migrate_old_farm_positions(&orig_caller);
+        let migrated_amount = self.migrate_old_farm_positions(&orig_caller);
 
         let exit_farm_result = self.exit_farm::<Wrapper<Self>>(orig_caller.clone(), payment);
 
+        self.decrease_old_farm_positions(migrated_amount, &orig_caller);
+
         self.send_payment_non_zero(&caller, &exit_farm_result.farming_tokens);
         self.send_payment_non_zero(&caller, &exit_farm_result.rewards);
 

diff --git a/dex/farm/wasm/src/lib.rs b/dex/farm/wasm/src/lib.rs
@@ -41,7 +41,7 @@ multiversx_sc_wasm_adapter::endpoints! {
         getLastRewardBlockNonce => last_reward_block_nonce
         getDivisionSafetyConstant => division_safety_constant
         getUserTotalFarmPosition => user_total_farm_position
-        getFarmPositionMigrationBlockNonce => farm_position_migration_block_nonce
+        getFarmPositionMigrationNonce => farm_position_migration_nonce
         registerFarmToken => register_farm_token
         getFarmTokenId => farm_token
         getFarmTokenSupply => farm_token_supply

diff --git a/farm-staking/farm-staking/src/claim_only_boosted_staking_rewards.rs b/farm-staking/farm-staking/src/claim_only_boosted_staking_rewards.rs
@@ -49,20 +49,37 @@ pub trait ClaimOnlyBoostedStakingRewardsModule:
         boosted_rewards_payment
     }
 
-    fn migrate_old_farm_positions(&self, caller: &ManagedAddress) {
+    fn migrate_old_farm_positions(&self, caller: &ManagedAddress) -> BigUint {
         let payments = self.call_value().all_esdt_transfers().clone_value();
         let farm_token_mapper = self.farm_token();
         let farm_token_id = farm_token_mapper.get_token_id();
+        let mut migrated_amount = BigUint::zero();
         for farm_position in &payments {
             if farm_position.token_identifier == farm_token_id
                 && self.is_old_farm_position(farm_position.token_nonce)
             {
-                let mut user_total_farm_position = self.get_user_total_farm_position(caller);
-                user_total_farm_position.total_farm_position += farm_position.amount;
-                self.user_total_farm_position(caller)
-                    .set(user_total_farm_position);
+                migrated_amount += farm_position.amount;
             }
         }
+
+        if migrated_amount > 0 {
+            let mut user_total_farm_position = self.get_user_total_farm_position(caller);
+            user_total_farm_position.total_farm_position += &migrated_amount;
+            self.user_total_farm_position(caller)
+                .set(user_total_farm_position);
+        }
+
+        migrated_amount
+    }
+
+    fn decrease_old_farm_positions(&self, migrated_amount: BigUint, caller: &ManagedAddress) {
+        if migrated_amount == BigUint::zero() {
+            return;
+        }
+        self.user_total_farm_position(caller)
+            .update(|user_total_farm_position| {
+                user_total_farm_position.total_farm_position -= migrated_amount;
+            });
     }
 
     // Cannot import the one from farm, as the Wrapper struct has different dependencies

diff --git a/farm-staking/farm-staking/src/lib.rs b/farm-staking/farm-staking/src/lib.rs
@@ -88,6 +88,10 @@ pub trait FarmStaking:
             "Invalid min unbond epochs"
         );
         self.min_unbond_epochs().set_if_empty(min_unbond_epochs);
+
+        // Farm position migration code
+        let farm_token_mapper = self.farm_token();
+        self.try_set_farm_position_migration_nonce(farm_token_mapper);
     }
 
     #[payable("*")]

diff --git a/farm-staking/farm-staking/src/unstake_farm.rs b/farm-staking/farm-staking/src/unstake_farm.rs
@@ -72,11 +72,13 @@ pub trait UnstakeFarmModule:
         payment: EsdtTokenPayment,
         opt_unbond_amount: Option<BigUint>,
     ) -> ExitFarmWithPartialPosResultType<Self::Api> {
-        self.migrate_old_farm_positions(&original_caller);
+        let migrated_amount = self.migrate_old_farm_positions(&original_caller);
 
         let exit_result =
             self.exit_farm_base::<FarmStakingWrapper<Self>>(original_caller.clone(), payment);
 
+        self.decrease_old_farm_positions(migrated_amount, &original_caller);
+
         let unbond_token_amount =
             opt_unbond_amount.unwrap_or(exit_result.farming_token_payment.amount);
         let farm_token_id = exit_result.storage_cache.farm_token_id.clone();

diff --git a/farm-staking/farm-staking/wasm/src/lib.rs b/farm-staking/farm-staking/wasm/src/lib.rs
@@ -43,7 +43,7 @@ multiversx_sc_wasm_adapter::endpoints! {
         getLastRewardBlockNonce => last_reward_block_nonce
         getDivisionSafetyConstant => division_safety_constant
         getUserTotalFarmPosition => user_total_farm_position
-        getFarmPositionMigrationBlockNonce => farm_position_migration_block_nonce
+        getFarmPositionMigrationNonce => farm_position_migration_nonce
         registerFarmToken => register_farm_token
         getFarmTokenId => farm_token
         getFarmTokenSupply => farm_token_supply