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

[Staking] Update reward calculation mechanism #44

Merged
merged 7 commits into from
Nov 4, 2022
Merged

[Staking] Update reward calculation mechanism #44

merged 7 commits into from
Nov 4, 2022

Conversation

ducthotran2010
Copy link
Collaborator

@ducthotran2010 ducthotran2010 commented Nov 1, 2022

Description

  • [RewardCalculation.sol] Update reward calculation mechanism: Deprecated slashing feature for staking contract
  • [RoninValidatorSet.sol] Update staking reward recording: updating rewards at the end of each block -> period
  • ABI changes:
    • Rename variables: staking balance to stakingAmount to avoid misleading
    • E.g: balanceOf -> stakingAmountOf, totalBalance -> stakingTotal
    • Update related events

What's to be noticed?

  • RoninValidatorSet.sol
  • RewardCalculation.sol
  • RewardCalculation.test.ts

Checklist

  • I have clearly commented on all the main functions followed the NatSpec Format
  • The box that allows repo maintainers to update this PR is checked
  • I tested locally to make sure this feature/fix works

@ducthotran2010 ducthotran2010 changed the title [RewardCalculation] Deprecated slashing feature for staking contract [Staking] Update reward calculation mechanism Nov 3, 2022
@ducthotran2010 ducthotran2010 self-assigned this Nov 3, 2022
@ducthotran2010 ducthotran2010 added the enhancement New feature or request label Nov 3, 2022
@ducthotran2010 ducthotran2010 marked this pull request as ready for review November 3, 2022 09:24
Copy link
Contributor

@nxqbao nxqbao left a comment

Choose a reason for hiding this comment

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

LGTM

@ducthotran2010 ducthotran2010 mentioned this pull request Nov 4, 2022
8 tasks
@ducthotran2010 ducthotran2010 merged commit 9ef4972 into axieinfinity:main Nov 4, 2022
@ducthotran2010 ducthotran2010 deleted the feat/staking-contract branch November 4, 2022 07:30
/// @dev Mapping from the pool address => settled pool data
mapping(address => SettledPool) internal _settledPool;
/// @dev Mapping from period number => accumulated rewards per share (one unit staking)
mapping(uint256 => PeriodWrapper) private _accumulatedRps;
Copy link
Contributor

@minh-bq minh-bq Nov 7, 2022

Choose a reason for hiding this comment

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

As far as I understand, this is used to track the rps of pool in period, so this must be per pool and a new pool address dimension is needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Make sense

uint256 _aRps = _accumulatedRps[_reward.lastPeriod].inner;
uint256 _lastPeriodReward = _minAmount * (_aRps - _reward.aRps);
uint256 _newPeriodsReward = _latestStakingAmount * (_pool.aRps - _aRps);
return _reward.debited + (_lastPeriodReward + _newPeriodsReward) / 1e18;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove 1e18 here and in _recordRewards below?

@ducthotran2010 ducthotran2010 mentioned this pull request Nov 7, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants