Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

farm position first audit fixes #772

Merged
merged 28 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
4b4e0b7
farm position first audit fixes
psorinionut Sep 13, 2023
c044821
metastaking refactor
psorinionut Sep 18, 2023
16576f1
readded claim progress check
psorinionut Sep 18, 2023
3fc935a
clippy fixes
psorinionut Sep 18, 2023
be142a3
dedicated migration function for old positions
psorinionut Sep 18, 2023
a77b760
remove caller check in migrate old position func
psorinionut Sep 19, 2023
3fe300e
remove exit amount parameter from farms
psorinionut Sep 19, 2023
37c9ebc
clippy fix
psorinionut Sep 19, 2023
9859e47
tests fixes
psorinionut Sep 19, 2023
d9fd841
remove metastaking ProxyMergePosModule
psorinionut Sep 20, 2023
65930ce
send rewards to user in claim_boosted_rewards
psorinionut Sep 20, 2023
ee6644c
claim boosted in merge endpoints
psorinionut Sep 20, 2023
1e9ec92
clippy and test fix
psorinionut Sep 20, 2023
bbff637
audit fixes (2)
psorinionut Sep 20, 2023
c609e42
Merge pull request #774 from multiversx/claim-boosted-in-merge-endpoints
psorinionut Sep 20, 2023
8ac9c19
Merge pull request #773 from multiversx/remove-exit-amount-parameter
psorinionut Sep 20, 2023
4303c0c
default migration nonce fix
psorinionut Sep 20, 2023
767f452
use GetCurrentESDTNFTNonce VM endpoint
psorinionut Sep 21, 2023
8100d57
try_set_farm_position_migration_nonce: code dup
CostinCarabas Sep 21, 2023
aba2e4e
fix farm_position_migration_nonce set function
psorinionut Sep 22, 2023
1d92432
Merge pull request #775 from multiversx/farm-position-audit-fixes-2
psorinionut Sep 26, 2023
494b301
farm position functionality tests
psorinionut Sep 26, 2023
772779a
allow_external_claim_rewards_setting extra check
psorinionut Sep 26, 2023
d790945
total_farm_position_claim_for_other extra check
psorinionut Sep 26, 2023
0ee2b42
farm position tests updates
psorinionut Sep 26, 2023
4262fa1
farm staking full position claim test
psorinionut Sep 28, 2023
a6e3e00
Merge pull request #776 from multiversx/farm-position-tests
sasurobert Sep 29, 2023
36be963
clippy fix
psorinionut Sep 29, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 38 additions & 10 deletions common/modules/farm/config/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,20 @@ pub const DEFAULT_NFT_DEPOSIT_MAX_LEN: usize = 10;
PartialEq,
Debug,
)]
pub struct UserTotalFarmPositionStruct<M: ManagedTypeApi> {
pub struct UserTotalFarmPosition<M: ManagedTypeApi> {
pub total_farm_position: BigUint<M>,
pub allow_external_claim_boosted_rewards: bool,
}

impl<M: ManagedTypeApi> Default for UserTotalFarmPosition<M> {
fn default() -> Self {
Self {
total_farm_position: BigUint::zero(),
allow_external_claim_boosted_rewards: false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
}
}

#[multiversx_sc::module]
pub trait ConfigModule: pausable::PausableModule + permissions_module::PermissionsModule {
#[inline]
Expand All @@ -32,17 +41,36 @@ pub trait ConfigModule: pausable::PausableModule + permissions_module::Permissio
state == State::Active
}

fn get_user_total_farm_position_struct(
fn get_user_total_farm_position(
&self,
user: &ManagedAddress,
) -> UserTotalFarmPositionStruct<Self::Api> {
self.user_total_farm_position(user)
.set_if_empty(UserTotalFarmPositionStruct {
total_farm_position: BigUint::zero(),
allow_external_claim_boosted_rewards: false,
});
) -> UserTotalFarmPosition<Self::Api> {
let user_total_farm_position_mapper = self.user_total_farm_position(user);
if user_total_farm_position_mapper.is_empty() {
UserTotalFarmPosition::default()
} else {
user_total_farm_position_mapper.get()
}
}

self.user_total_farm_position(user).get()
#[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);
if user_total_farm_position_mapper.is_empty() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

require!(
allow_external_claim,
"Can only set to true if there is no farm position"
);
let mut new_user_farm_position: UserTotalFarmPosition<Self::Api> =
UserTotalFarmPosition::default();
new_user_farm_position.allow_external_claim_boosted_rewards = allow_external_claim;
} else {
user_total_farm_position_mapper.update(|user_total_farm_position| {
user_total_farm_position.allow_external_claim_boosted_rewards =
allow_external_claim;
});
}
}

#[view(getFarmingTokenId)]
Expand Down Expand Up @@ -73,5 +101,5 @@ pub trait ConfigModule: pausable::PausableModule + permissions_module::Permissio
fn user_total_farm_position(
&self,
user: &ManagedAddress,
) -> SingleValueMapper<UserTotalFarmPositionStruct<Self::Api>>;
) -> SingleValueMapper<UserTotalFarmPosition<Self::Api>>;
}
41 changes: 25 additions & 16 deletions common/modules/farm/farm_base_impl/src/base_traits_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,10 @@ pub trait FarmContract {
}
}

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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

&& total_farm_position > 0
{
Self::increase_user_farm_position(sc, user, &total_farm_position);
} else if farm_position_increase > 0 {
Self::increase_user_farm_position(sc, user, &farm_position_increase);
Expand All @@ -224,10 +226,10 @@ pub trait FarmContract {
user: &ManagedAddress<<Self::FarmSc as ContractBase>::Api>,
increase_farm_position_amount: &BigUint<<Self::FarmSc as ContractBase>::Api>,
) {
let mut user_total_farm_position = sc.get_user_total_farm_position(user);
user_total_farm_position.total_farm_position += increase_farm_position_amount;
sc.user_total_farm_position(user)
.update(|user_farm_position_struct| {
user_farm_position_struct.total_farm_position += increase_farm_position_amount
});
.set(user_total_farm_position);
}

fn decrease_user_farm_position(
Expand All @@ -238,17 +240,24 @@ pub trait FarmContract {
let token_attributes: FarmTokenAttributes<<Self::FarmSc as ContractBase>::Api> =
farm_token_mapper.get_token_attributes(farm_position.token_nonce);

sc.user_total_farm_position(&token_attributes.original_owner)
.update(|user_farm_position_struct| {
let mut user_total_farm_position =
user_farm_position_struct.total_farm_position.clone();
if user_total_farm_position > farm_position.amount {
user_total_farm_position -= &farm_position.amount;
} else {
user_total_farm_position = BigUint::zero();
}
user_farm_position_struct.total_farm_position = user_total_farm_position;
});
let mut user_total_farm_position =
sc.get_user_total_farm_position(&token_attributes.original_owner);

if user_total_farm_position.total_farm_position > farm_position.amount {
user_total_farm_position.total_farm_position -= &farm_position.amount;
} else {
user_total_farm_position.total_farm_position = BigUint::zero();
}

if user_total_farm_position.total_farm_position == 0
&& user_total_farm_position.allow_external_claim_boosted_rewards == false
{
sc.user_total_farm_position(&token_attributes.original_owner)
.clear();
} else {
sc.user_total_farm_position(&token_attributes.original_owner)
.set(user_total_farm_position);
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions common/modules/farm/farm_base_impl/src/claim_rewards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ pub trait BaseClaimRewardsModule:
self.blockchain(),
);

FC::check_and_update_user_farm_position(self, &caller, &payments);

FC::generate_aggregated_rewards(self, &mut storage_cache);

let farm_token_amount = &claim_rewards_context.first_farm_token.payment.amount;
Expand All @@ -83,6 +81,8 @@ pub trait BaseClaimRewardsModule:
);
storage_cache.reward_reserve -= &reward;

FC::check_and_update_user_farm_position(self, &caller, &payments);

let farm_token_mapper = self.farm_token();
let base_attributes = FC::create_claim_rewards_initial_attributes(
self,
Expand Down
3 changes: 2 additions & 1 deletion common/modules/farm/farm_base_impl/src/compound_rewards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ pub trait BaseCompoundRewardsModule:
self.blockchain(),
);

FC::check_and_update_user_farm_position(self, &caller, &payments);
FC::generate_aggregated_rewards(self, &mut storage_cache);

let farm_token_amount = &compound_rewards_context.first_farm_token.payment.amount;
Expand All @@ -71,6 +70,8 @@ pub trait BaseCompoundRewardsModule:
storage_cache.reward_reserve -= &reward;
storage_cache.farm_token_supply += &reward;

FC::check_and_update_user_farm_position(self, &caller, &payments);

let farm_token_mapper = self.farm_token();
let base_attributes = FC::create_compound_rewards_initial_attributes(
self,
Expand Down
121 changes: 78 additions & 43 deletions dex/farm-with-locked-rewards/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use core::marker::PhantomData;
use mergeable::Mergeable;

use farm::{
base_functions::{BaseFunctionsModule, ClaimRewardsResultType, Wrapper},
base_functions::{BaseFunctionsModule, ClaimRewardsResultType, DoubleMultiPayment, Wrapper},
exit_penalty::{
DEFAULT_BURN_GAS_LIMIT, DEFAULT_MINUMUM_FARMING_EPOCHS, DEFAULT_PENALTY_PERCENT,
},
Expand All @@ -34,8 +34,6 @@ pub trait Farm:
+ multiversx_sc_modules::default_issue_callbacks::DefaultIssueCallbacksModule
+ farm::base_functions::BaseFunctionsModule
+ farm::exit_penalty::ExitPenaltyModule
+ farm::progress_update::ProgressUpdateModule
+ farm::claim_boost_only::ClaimBoostOnlyModule
+ farm_base_impl::base_farm_init::BaseFarmInitModule
+ farm_base_impl::base_farm_validation::BaseFarmValidationModule
+ farm_base_impl::enter_farm::BaseEnterFarmModule
Expand Down Expand Up @@ -89,27 +87,24 @@ pub trait Farm:
let caller = self.blockchain().get_caller();
let orig_caller = self.get_orig_caller_from_opt(&caller, opt_orig_caller);

let payments = self.get_non_empty_payments();
let first_additional_payment_index = 1;
let boosted_rewards = match payments.try_get(first_additional_payment_index) {
Some(p) => {
let unlocked_rewards = self.claim_only_boosted_payment(&orig_caller, &p);
self.send_to_lock_contract_non_zero(
unlocked_rewards.token_identifier,
unlocked_rewards.amount,
caller.clone(),
orig_caller.clone(),
)
}
None => EsdtTokenPayment::new(self.reward_token_id().get(), 0, BigUint::zero()),
let boosted_rewards = self.claim_only_boosted_payment(&orig_caller);
let boosted_rewards_payment = if boosted_rewards > 0 {
self.send_to_lock_contract_non_zero(
self.reward_token_id().get(),
boosted_rewards,
caller.clone(),
orig_caller.clone(),
)
} else {
EsdtTokenPayment::new(self.reward_token_id().get(), 0, BigUint::zero())
};

let new_farm_token = self.enter_farm::<NoMintWrapper<Self>>(orig_caller.clone());
self.send_payment_non_zero(&caller, &new_farm_token);

self.update_energy_and_progress(&orig_caller);

(new_farm_token, boosted_rewards).into()
(new_farm_token, boosted_rewards_payment).into()
}

#[payable("*")]
Expand Down Expand Up @@ -163,7 +158,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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call migrate_old_farm_position here as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Do the same on farm-staking.

let boosted_rewards_full_position =
EsdtTokenPayment::new(self.reward_token_id().get(), 0, boosted_rewards);

let remaining_farm_payment = EsdtTokenPayment::new(
payment.token_identifier.clone(),
payment.token_nonce,
Expand All @@ -186,7 +184,7 @@ pub trait Farm:
orig_caller.clone(),
);

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

(
exit_farm_result.farming_tokens,
Expand All @@ -196,41 +194,57 @@ pub trait Farm:
.into()
}

#[view(calculateRewardsForGivenPosition)]
fn calculate_rewards_for_given_position(
&self,
user: ManagedAddress,
farm_token_amount: BigUint,
attributes: FarmTokenAttributes<Self::Api>,
) -> BigUint {
self.require_queried();

let mut storage_cache = StorageCache::new(self);
NoMintWrapper::<Self>::generate_aggregated_rewards(self, &mut storage_cache);

NoMintWrapper::<Self>::calculate_rewards(
self,
&user,
&farm_token_amount,
&attributes,
&storage_cache,
)
}

#[payable("*")]
#[endpoint(mergeFarmTokens)]
fn merge_farm_tokens_endpoint(
&self,
opt_orig_caller: OptionalValue<ManagedAddress>,
) -> EsdtTokenPayment<Self::Api> {
) -> DoubleMultiPayment<Self::Api> {
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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


let merged_farm_token = self.merge_farm_tokens::<NoMintWrapper<Self>>();
self.send_payment_non_zero(&caller, &merged_farm_token);

merged_farm_token
let locked_rewards_payment = self.send_to_lock_contract_non_zero(
self.reward_token_id().get(),
boosted_rewards,
caller,
orig_caller.clone(),
);

(merged_farm_token, locked_rewards_payment).into()
}

#[endpoint(claimBoostedRewards)]
fn claim_boosted_rewards(
&self,
opt_user: OptionalValue<ManagedAddress>,
) -> EsdtTokenPayment<Self::Api> {
let caller = self.blockchain().get_caller();
let user = match &opt_user {
OptionalValue::Some(user) => user,
OptionalValue::None => &caller,
};
let user_total_farm_position = self.get_user_total_farm_position(user);
if user != &caller {
require!(
user_total_farm_position.allow_external_claim_boosted_rewards,
"Cannot claim rewards for this address"
);
}

let boosted_rewards = self.claim_only_boosted_payment(&user);
let locked_rewards_payment = self.send_to_lock_contract_non_zero(
self.reward_token_id().get(),
boosted_rewards,
caller.clone(),
user.clone(),
);

locked_rewards_payment
}

#[endpoint(startProduceRewards)]
Expand All @@ -251,6 +265,27 @@ pub trait Farm:
self.set_per_block_rewards::<NoMintWrapper<Self>>(per_block_amount);
}

#[view(calculateRewardsForGivenPosition)]
fn calculate_rewards_for_given_position(
&self,
user: ManagedAddress,
farm_token_amount: BigUint,
attributes: FarmTokenAttributes<Self::Api>,
) -> BigUint {
self.require_queried();

let mut storage_cache = StorageCache::new(self);
NoMintWrapper::<Self>::generate_aggregated_rewards(self, &mut storage_cache);

NoMintWrapper::<Self>::calculate_rewards(
self,
&user,
&farm_token_amount,
&attributes,
&storage_cache,
)
}

fn send_to_lock_contract_non_zero(
&self,
token_id: TokenIdentifier,
Expand Down
Loading