Stake: Try to optimize the withdraw rewards function #395
Conversation
totallynotjakub
force-pushed
the
jakub/stake-rewards-optimization
branch
from
cb1c3af to
7b016d5
Compare
totallynotjakub
force-pushed
the
jakub/stake-rewards-optimization
branch
from
4a625b4 to
1b62564
Compare
| @@ -70,6 +72,13 @@ pub trait StakingTrait { | |||
|
|
|||
| fn query_withdrawable_rewards(env: Env, address: Address) -> WithdrawableRewardsResponse; | |||
|
|
|||
| fn query_withdrawable_rewards_ch( | |||
There was a problem hiding this comment.
why are you using shortened query_withdrawable_rewards_ch instead of query_withdrawable_rewards_chunks
There was a problem hiding this comment.
Character limit per function name.
contracts/stake/src/contract.rs
Outdated
| token_contract::Client::new(&env, &reward_token).transfer( | ||
| &env.current_contract_address(), | ||
| &sender, | ||
| &pending_rewards, |
There was a problem hiding this comment.
wouldn't it better if we use env.contract_invoke - should be better on the storage and (I'm not 100%) but it will avoid migration issues when we upgrade to next soroban-sdk
| let (pending_reward, _) = | ||
| calculate_pending_rewards_chunked(&env, &asset, &stakes, chunk_size, start_day); |
There was a problem hiding this comment.
why don't we return just pending_reward since we're dropping the 2nd part of the tuple with _ from memory
There was a problem hiding this comment.
It's used in another call of that function.
| let stake_age_days = | ||
| ((reward_day - stake.stake_timestamp) / SECONDS_PER_DAY).min(60); |
There was a problem hiding this comment.
what would happen if the user stakes their tokens after the reward_day in question. Their stake.stake_timestamp is set to the time they staked, which is more recent than reward_day.
I'm not 100% sure that .skip_while(|&day| day < start_day.unwrap_or(last_claim_time)) would prevent us from getting to thath state, but last_claim_time may still be before stake.stake_timestamp. what if the user hasn't claimed rewards for a long time, last_claim_time could be an old timestamp
There was a problem hiding this comment.
what would happen if the user stakes their tokens after the reward_day in question
Then it's getting filtered through skip_while(|&day: u64| day < last_claim_time))
what if the user hasn't claimed rewards for a long time, last_claim_time could be an old timestamp
last_claim_time can be even 0 if user never claimed his rewards; That means he needs to start from the beginning of history of the rewards.
| } | ||
|
|
||
| stakes.last_reward_time = last_reward_day; | ||
| save_stakes(&env, &sender, &stakes); |
There was a problem hiding this comment.
btw, I know logs are useless, but we should have some
No description provided.