farm position first audit fixes #772

psorinionut · 2023-09-13T08:15:31Z

No description provided.

sasurobert · 2023-09-14T10:34:41Z

common/modules/farm/config/src/config.rs

+    fn default() -> Self {
+        Self {
+            total_farm_position: BigUint::zero(),
+            allow_external_claim_boosted_rewards: false,


we need a new bool here: migrated. And to keep that even if total_farm_position gets to 0 again. Or to not delete claimProgressWeek from user.

sasurobert · 2023-09-14T10:36:57Z

common/modules/farm/config/src/config.rs

+    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);
+        if user_total_farm_position_mapper.is_empty() {


there is no need for this if. Actually the user must have a farm position before making this allow call.

sasurobert · 2023-09-14T10:40:28Z

common/modules/farm/farm_base_impl/src/base_traits_impl.rs

-        let user_total_farm_position_struct = sc.get_user_total_farm_position_struct(user);
-        if user_total_farm_position_struct.total_farm_position == BigUint::zero() {
+        let user_total_farm_position = sc.get_user_total_farm_position(user);
+        if user_total_farm_position.total_farm_position == BigUint::zero()


No need for this if. Make farm_position_increase += farm_position.amount if the farm_position is old.

Also Do not decrease_user_farm_position for old_farm_positions.

sasurobert · 2023-09-14T10:48:12Z

dex/farm-with-locked-rewards/src/lib.rs

        let caller = self.blockchain().get_caller();
        let orig_caller = self.get_orig_caller_from_opt(&caller, opt_orig_caller);
-        self.check_claim_progress_for_merge(&orig_caller);
+
+        let boosted_rewards = self.claim_only_boosted_payment(&orig_caller);


if the payments are old tokens - we need to increase the total_position of the user. If they are new tokens we have to check and change total_positions if original_user != caller.

I think we could make a function called = adjust_total_position - which gets the payments and computes the total_position for the user and decreases for original_user.

We do not need to claim_boosted_rewards.

sasurobert · 2023-09-14T10:55:28Z

dex/farm/src/base_functions.rs

-        let user_total_farm_position = user_total_farm_position_struct.total_farm_position;
-        if user_total_farm_position == BigUint::zero() {
-            return BigUint::zero();
+        if user_farm_position > 0 {


this if is not needed.

sasurobert · 2023-09-14T10:58:52Z

dex/farm/src/lib.rs

        let caller = self.blockchain().get_caller();
        let orig_caller = self.get_orig_caller_from_opt(&caller, opt_orig_caller);
-        self.check_claim_progress_for_merge(&orig_caller);
+
+        let boosted_rewards = self.claim_only_boosted_payment(&orig_caller);


no need to claim boosted - only adjust_total_positions as explained in a comment before.

sasurobert · 2023-09-14T11:00:59Z

dex/farm/src/progress_update.rs

-    + energy_query::EnergyQueryModule
-    + utils::UtilsModule
-{
-    fn check_claim_progress_for_merge(&self, caller: &ManagedAddress) {


why deleted ?

sasurobert · 2023-09-14T11:02:17Z

farm-staking/farm-staking/src/unstake_farm.rs

@@ -86,8 +86,10 @@ pub trait UnstakeFarmModule:
            "Exit amount is bigger than the payment amount"
        );

+        let boosted_rewards = self.claim_only_boosted_payment(&original_caller);


adjust position and claim boosted.

sasurobert · 2023-09-14T11:03:12Z

farm-staking/farm-staking/src/stake_farm.rs

-            None => EsdtTokenPayment::new(self.reward_token_id().get(), 0, BigUint::zero()),
+        let caller = self.blockchain().get_caller();
+
+        let boosted_rewards = self.claim_only_boosted_payment(&original_caller);


adjust position and claim boosted. like everywhere. in farms as well. for entering and exiting. For merging we need only adjust_position.

sasurobert · 2023-09-14T11:03:34Z

farm-staking/farm-staking/src/lib.rs

-        self.check_claim_progress_for_merge(&caller);
+        let orig_caller = self.get_orig_caller_from_opt(&caller, opt_orig_caller);
+
+        let boosted_rewards = self.claim_only_boosted_payment(&orig_caller);


Only adjust_positions - no need for claim boosted here.

github-actions · 2023-09-18T11:18:07Z

Contract comparison - from 249c3ee to 36be963

Path	size	has-allocator	has-format
mx-exchange-sc
- pause-all/pause-all.wasm	5438	No	No
mx-exchange-sc/dex
- farm/farm.wasm	39645 ➡️ 38696 🟢	No	No
- farm-with-locked-rewards/farm-with-locked-rewards.wasm	39306 ➡️ 38299 🟢	No	No
- governance/governance.wasm	17283	No	No
- pair
- - pair.wasm	30521 ➡️ 30520 🟢	No	No
- - pair-full.wasm	31968 ➡️ 31970 🔴	No	No
- - safe-price-view.wasm	7862	No	No
- pair-mock/pair-mock.wasm	5474	No	No
- price-discovery/price-discovery.wasm	16254	No	No
- proxy-deployer/proxy-deployer.wasm	4457	No	No
- router/router.wasm	25991	No	No
mx-exchange-sc/energy-integration
- energy-factory-mock/energy-factory-mock.wasm	4233	No	No
- energy-update/energy-update.wasm	1510	No	No
- fees-collector/fees-collector.wasm	18400 ➡️ 18531 🔴	No	No
- governance-v2/governance-v2.wasm	16822	No	No
mx-exchange-sc/farm-staking
- farm-staking/farm-staking.wasm	42902 ➡️ 42863 🟢	No	No
- farm-staking-proxy/farm-staking-proxy.wasm	21021 ➡️ 18629 🟢	No	No
- metabonding-staking/metabonding-staking.wasm	9593	No	No
mx-exchange-sc/locked-asset
- distribution/distribution.wasm	14358	No	No
- energy-factory/energy-factory.wasm	34193	No	No
- factory/factory.wasm	30374 ➡️ 30375 🔴	No	No
- lkmex-transfer/lkmex-transfer.wasm	11214	No	No
- locked-token-wrapper/locked-token-wrapper.wasm	14291	No	No
- proxy_dex/proxy_dex.wasm	34335 ➡️ 34017 🟢	No	No
- simple-lock/simple-lock.wasm	24299 ➡️ 24111 🟢	No	No
- simple-lock-whitelist/simple-lock-whitelist.wasm	25946 ➡️ 25759 🟢	No	No
- token-unstake/token-unstake.wasm	13625	No	No

sasurobert · 2023-09-19T10:20:29Z

dex/farm/src/base_functions.rs

+                let token_attributes: FarmTokenAttributes<Self::Api> =
+                    farm_token_mapper.get_token_attributes(farm_position.token_nonce);
+
+                if &token_attributes.original_owner == caller {


No need for this check.

You can delete it safely.

sasurobert · 2023-09-19T10:24:00Z

dex/farm-with-locked-rewards/src/lib.rs

@@ -163,7 +162,10 @@ pub trait Farm:
            "Exit amount is bigger than the payment amount"
        );

-        let boosted_rewards_full_position = self.claim_only_boosted_payment(&orig_caller, &payment);
+        let boosted_rewards = self.claim_only_boosted_payment(&orig_caller);


Call migrate_old_farm_position here as well.

claim rewards and make a new position for the whole payment.

Do the same on farm-staking.

sasurobert · 2023-09-19T10:32:51Z

dex/farm/src/lib.rs

@@ -162,7 +156,10 @@ pub trait Farm:
            "Exit amount is bigger than the payment amount"
        );

-        let boosted_rewards_full_position = self.claim_only_boosted_payment(&orig_caller, &payment);
+        let boosted_rewards = self.claim_only_boosted_payment(&orig_caller);


call migrate_old_position before claim_boosted

sasurobert · 2023-09-19T10:36:00Z

dex/farm/src/lib.rs

@@ -180,7 +189,7 @@ pub trait Farm:
        self.send_payment_non_zero(&caller, &exit_farm_result.rewards);
        self.send_payment_non_zero(&caller, &remaining_farm_payment);

-        self.clear_user_energy_if_needed(&orig_caller, &remaining_farm_payment.amount);
+        self.clear_user_energy_if_needed(&orig_caller);


Let's not clear.

claim boosted in merge endpoints

remove exit amount parameter from farms

Moved try_set_farm_position_migration_nonce to common modules to remove code duplication.

audit fixes (2)

farm position functionality tests

farm position first audit fixes

4b4e0b7

sasurobert requested changes Sep 14, 2023

View reviewed changes

psorinionut added 3 commits September 18, 2023 13:25

metastaking refactor

c044821

readded claim progress check

16576f1

clippy fixes

3fc935a

dedicated migration function for old positions

be142a3

sasurobert reviewed Sep 19, 2023

View reviewed changes

psorinionut and others added 22 commits September 19, 2023 13:39

remove caller check in migrate old position func

a77b760

remove exit amount parameter from farms

3fe300e

clippy fix

37c9ebc

tests fixes

9859e47

remove metastaking ProxyMergePosModule

d9fd841

send rewards to user in claim_boosted_rewards

65930ce

claim boosted in merge endpoints

ee6644c

clippy and test fix

1e9ec92

audit fixes (2)

bbff637

Merge pull request #774 from multiversx/claim-boosted-in-merge-endpoints

c609e42

claim boosted in merge endpoints

Merge pull request #773 from multiversx/remove-exit-amount-parameter

8ac9c19

remove exit amount parameter from farms

default migration nonce fix

4303c0c

use GetCurrentESDTNFTNonce VM endpoint

767f452

try_set_farm_position_migration_nonce: code dup

8100d57

Moved try_set_farm_position_migration_nonce to common modules to remove code duplication.

fix farm_position_migration_nonce set function

aba2e4e

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

1d92432

audit fixes (2)

farm position functionality tests

494b301

allow_external_claim_rewards_setting extra check

772779a

total_farm_position_claim_for_other extra check

d790945

farm position tests updates

0ee2b42

farm staking full position claim test

4262fa1

Merge pull request #776 from multiversx/farm-position-tests

a6e3e00

farm position functionality tests

sasurobert approved these changes Sep 29, 2023

View reviewed changes

clippy fix

36be963

sasurobert approved these changes Sep 29, 2023

View reviewed changes

psorinionut merged commit ff4f6b1 into farm-position-functionality Sep 29, 2023
4 checks passed

psorinionut deleted the farm-position-fixes branch September 29, 2023 07:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

farm position first audit fixes #772

farm position first audit fixes #772

psorinionut commented Sep 13, 2023

sasurobert Sep 14, 2023

sasurobert Sep 14, 2023

sasurobert Sep 14, 2023

sasurobert Sep 14, 2023

sasurobert Sep 14, 2023

sasurobert Sep 14, 2023

sasurobert Sep 14, 2023

sasurobert Sep 14, 2023

sasurobert Sep 14, 2023

sasurobert Sep 14, 2023

github-actions bot commented Sep 18, 2023 •

edited

Loading

sasurobert Sep 19, 2023

sasurobert Sep 19, 2023

sasurobert Sep 19, 2023

sasurobert Sep 19, 2023

sasurobert Sep 19, 2023

sasurobert Sep 19, 2023

farm position first audit fixes #772

farm position first audit fixes #772

Conversation

psorinionut commented Sep 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 18, 2023 •

edited

Loading