Skip to content

feat(REP-0024): implement logic and unit-tests#166

Open
TuDo1403 wants to merge 10 commits intorelease/v0.8.2from
feature/rep-0024
Open

feat(REP-0024): implement logic and unit-tests#166
TuDo1403 wants to merge 10 commits intorelease/v0.8.2from
feature/rep-0024

Conversation

@TuDo1403
Copy link
Contributor

This pull request introduces significant updates to the StakingVesting contract and its related interface to enhance flexibility and remove deprecated functionality. Key changes include replacing static bonus configurations with dynamic block rewards, adding new error types and events for better validation and tracking, and deprecating REP-10-related logic.

Spec: https://github.com/ronin-chain/REPs/blob/main/REP-0024/REP-0024.md

Enhancements to Block Rewards and Validation

  • Introduced a Reward struct and replaced static bonus configurations with dynamic block rewards managed through new methods (updateBlockRewards, setBlockRewardRange, etc.) in IStakingVesting and StakingVesting. These methods enforce validation rules such as bounds checking and descending order of start blocks. [1] [2] [3]
  • Added new errors (ErrOutOfBound, ErrOutOfOrder, ErrEmptyArray) and the BlockRewardRangeUpdated event to improve input validation and tracking of reward updates. [1] [2]

Deprecation of Unused Functionality

  • Deprecated REP-10-related logic and fields, including REP10FastFinalityRewardActivated and associated storage variables, to streamline the contract. [1] [2] [3]
  • Removed bridge operator bonuses and associated methods/events (bridgeOperatorBlockBonus, setBridgeOperatorBonusPerBlock, etc.), as they are no longer used. [1] [2] [3]

Interface and Method Updates

  • Updated blockProducerBlockBonus to return a uint64 and calculate the bonus dynamically based on the current block number and configured rewards. [1] [2]
  • Added methods to retrieve the block reward range (getBlockRewardRange) and current block rewards (getBlockRewards) for better transparency. [1] [2]

Internal Contract Refactoring

  • Introduced _setBlockRewardRange and _updateBlockRewards internal methods in StakingVesting to encapsulate reward-related logic and enforce validation.
  • Deprecated unused internal variables (_blockProducerBonusPerBlock, _bridgeOperatorBonusPerBlock, etc.) to clean up the contract.

Test and Script Adjustments

  • Updated tests and scripts to align with the new dynamic reward configuration, replacing calls to deprecated methods with the new updateBlockRewards method. [1] [2]

@github-actions
Copy link

github-actions bot commented Jun 13, 2025

Slither report

THIS CHECKLIST IS NOT COMPLETE. Use --show-ignored-findings to show all the results.
Summary

reentrancy-eth

Impact: High
Confidence: Medium

function wrapUpEpoch() external payable virtual override onlyCoinbase whenEpochEnding oncePerEpoch {
unchecked {
uint256 newPeriod = _computePeriod(block.timestamp);
bool periodEnding = _isPeriodEnding(newPeriod);
uint256 lastPeriod = currentPeriod();
uint256 epoch = epochOf(block.number);
uint256 nextEpoch = epoch + 1;
IRandomBeacon randomBeacon = IRandomBeacon(getContract(ContractType.RANDOM_BEACON));
// This request is actually only invoked at the first epoch of the period.
randomBeacon.execRequestRandomSeedForNextPeriod(lastPeriod, newPeriod);
// Get all candidate ids
address[] memory allCids = _candidateIds;
_syncFastFinalityReward({ epoch: epoch, validatorIds: allCids });
if (periodEnding) {
ISlashIndicator slashIndicatorContract = ISlashIndicator(getContract(ContractType.SLASH_INDICATOR));
// Slash submit random beacon proof unavailability first, then update credit scores.
randomBeacon.execRecordAndSlashUnavailability(lastPeriod, newPeriod, address(slashIndicatorContract), allCids);
slashIndicatorContract.execUpdateCreditScores(allCids, lastPeriod);
(uint256[] memory delegatorBlockMiningRewards, uint256[] memory delegatorFastFinalityRewards) =
_distributeRewardToTreasuriesAndCalculateTotalDelegatorsReward(lastPeriod, allCids);
_settleAndTransferDelegatingRewards(
lastPeriod, allCids, delegatorBlockMiningRewards, delegatorFastFinalityRewards
);
_tryRecycleLockedFundsFromEmergencyExits();
_recycleDeprecatedRewards();
address[] memory revokedCandidateIds = _syncCandidateSet(newPeriod);
if (revokedCandidateIds.length > 0) {
// Re-update `allCids` after unsatisfied candidates get removed.
allCids = _candidateIds;
slashIndicatorContract.execResetCreditScores(revokedCandidateIds);
}
// Wrap up the beacon period includes (1) finalizing the beacon proof, and (2) determining the validator list for the next period by new proof.
// Should wrap up the beacon after unsatisfied candidates get removed.
randomBeacon.execFinalizeBeaconAndPendingCids(lastPeriod, newPeriod, allCids);
_periodEndBlock[lastPeriod] = block.number;
_currentPeriodStartAtBlock = block.number + 1;
}
// Clear the previous validator set and block producer set before sync the new set from beacon.
_clearPreviousValidatorSetAndBlockProducerSet();
// Query the new validator set for upcoming epoch from the random beacon contract.
// Save new set into the contract storage.
address[] memory newValidatorIds = _syncValidatorSet(randomBeacon, newPeriod, nextEpoch);
// Activate applicable validators into the block producer set.
_updateApplicableValidatorToBlockProducerSet(newPeriod, nextEpoch, newValidatorIds);
emit WrappedUpEpoch(lastPeriod, epoch, periodEnding);
_periodOf[nextEpoch] = newPeriod;
_lastUpdatedPeriod = newPeriod;
}
}

function execDeprecatePools(
address[] calldata poolIds,
uint256 newPeriod
) external override onlyContract(ContractType.VALIDATOR) {
if (poolIds.length == 0) {
return;
}
for (uint256 i = 0; i < poolIds.length;) {
address poolId = poolIds[i];
PoolDetail storage _pool = _poolDetail[poolId];
// Deactivate the pool admin in the active mapping.
delete _adminOfActivePoolMapping[_pool.__shadowedPoolAdmin];
// Deduct and transfer the self staking amount to the pool admin.
uint256 deductingAmount = _pool.stakingAmount;
if (deductingAmount > 0) {
_deductStakingAmount(_pool, deductingAmount);
if (!_unsafeSendRONLimitGas(payable(_pool.__shadowedPoolAdmin), deductingAmount, DEFAULT_ADDITION_GAS)) {
emit StakingAmountTransferFailed(_pool.pid, _pool.__shadowedPoolAdmin, deductingAmount, address(this).balance);
}
}
// Settle the unclaimed reward and transfer to the pool admin.
uint256 lastRewardAmount = _claimReward(poolId, _pool.__shadowedPoolAdmin, newPeriod);
if (lastRewardAmount > 0) {
_unsafeSendRONLimitGas(payable(_pool.__shadowedPoolAdmin), lastRewardAmount, DEFAULT_ADDITION_GAS);
}
unchecked {
++i;
}
}
emit PoolsDeprecated(poolIds);
}

function wrapUpEpoch() external payable virtual override onlyCoinbase whenEpochEnding oncePerEpoch {
unchecked {
uint256 newPeriod = _computePeriod(block.timestamp);
bool periodEnding = _isPeriodEnding(newPeriod);
uint256 lastPeriod = currentPeriod();
uint256 epoch = epochOf(block.number);
uint256 nextEpoch = epoch + 1;
IRandomBeacon randomBeacon = IRandomBeacon(getContract(ContractType.RANDOM_BEACON));
// This request is actually only invoked at the first epoch of the period.
randomBeacon.execRequestRandomSeedForNextPeriod(lastPeriod, newPeriod);
// Get all candidate ids
address[] memory allCids = _candidateIds;
_syncFastFinalityReward({ epoch: epoch, validatorIds: allCids });
if (periodEnding) {
ISlashIndicator slashIndicatorContract = ISlashIndicator(getContract(ContractType.SLASH_INDICATOR));
// Slash submit random beacon proof unavailability first, then update credit scores.
randomBeacon.execRecordAndSlashUnavailability(lastPeriod, newPeriod, address(slashIndicatorContract), allCids);
slashIndicatorContract.execUpdateCreditScores(allCids, lastPeriod);
(uint256[] memory delegatorBlockMiningRewards, uint256[] memory delegatorFastFinalityRewards) =
_distributeRewardToTreasuriesAndCalculateTotalDelegatorsReward(lastPeriod, allCids);
_settleAndTransferDelegatingRewards(
lastPeriod, allCids, delegatorBlockMiningRewards, delegatorFastFinalityRewards
);
_tryRecycleLockedFundsFromEmergencyExits();
_recycleDeprecatedRewards();
address[] memory revokedCandidateIds = _syncCandidateSet(newPeriod);
if (revokedCandidateIds.length > 0) {
// Re-update `allCids` after unsatisfied candidates get removed.
allCids = _candidateIds;
slashIndicatorContract.execResetCreditScores(revokedCandidateIds);
}
// Wrap up the beacon period includes (1) finalizing the beacon proof, and (2) determining the validator list for the next period by new proof.
// Should wrap up the beacon after unsatisfied candidates get removed.
randomBeacon.execFinalizeBeaconAndPendingCids(lastPeriod, newPeriod, allCids);
_periodEndBlock[lastPeriod] = block.number;
_currentPeriodStartAtBlock = block.number + 1;
}
// Clear the previous validator set and block producer set before sync the new set from beacon.
_clearPreviousValidatorSetAndBlockProducerSet();
// Query the new validator set for upcoming epoch from the random beacon contract.
// Save new set into the contract storage.
address[] memory newValidatorIds = _syncValidatorSet(randomBeacon, newPeriod, nextEpoch);
// Activate applicable validators into the block producer set.
_updateApplicableValidatorToBlockProducerSet(newPeriod, nextEpoch, newValidatorIds);
emit WrappedUpEpoch(lastPeriod, epoch, periodEnding);
_periodOf[nextEpoch] = newPeriod;
_lastUpdatedPeriod = newPeriod;
}
}

function _distributeRewardToTreasuriesAndCalculateTotalDelegatorsReward(
uint256 lastPeriod,
address[] memory cids
) private returns (uint256[] memory delegatorBlockMiningRewards, uint256[] memory delegatorFastFinalityRewards) {
address vId; // validator id
address payable treasury;
uint256 length = cids.length;
delegatorBlockMiningRewards = new uint256[](length);
delegatorFastFinalityRewards = new uint256[](length);
(uint256 minRate, uint256 maxRate) = IStaking(getContract(ContractType.STAKING)).getCommissionRateRange();
for (uint256 i; i < length; ++i) {
vId = cids[i];
treasury = _candidateInfo[vId].__shadowedTreasury;
if (!_isJailedById(vId) && !_miningRewardDeprecatedById(vId, lastPeriod)) {
(uint256 validatorFFReward, uint256 delegatorFFReward) = _calcCommissionReward({
vId: vId,
totalReward: _fastFinalityReward[vId],
maxCommissionRate: maxRate,
minCommissionRate: minRate
});
delegatorBlockMiningRewards[i] = _delegatorMiningReward[vId];
delegatorFastFinalityRewards[i] = delegatorFFReward;
_distributeMiningReward(vId, treasury);
_distributeFastFinalityReward(vId, treasury, validatorFFReward);
} else {
_totalDeprecatedReward += _validatorMiningReward[vId] + _delegatorMiningReward[vId] + _fastFinalityReward[vId];
}
delete _delegatorMiningReward[vId];
delete _validatorMiningReward[vId];
delete _fastFinalityReward[vId];
}
}

function wrapUpEpoch() external payable virtual override onlyCoinbase whenEpochEnding oncePerEpoch {
unchecked {
uint256 newPeriod = _computePeriod(block.timestamp);
bool periodEnding = _isPeriodEnding(newPeriod);
uint256 lastPeriod = currentPeriod();
uint256 epoch = epochOf(block.number);
uint256 nextEpoch = epoch + 1;
IRandomBeacon randomBeacon = IRandomBeacon(getContract(ContractType.RANDOM_BEACON));
// This request is actually only invoked at the first epoch of the period.
randomBeacon.execRequestRandomSeedForNextPeriod(lastPeriod, newPeriod);
// Get all candidate ids
address[] memory allCids = _candidateIds;
_syncFastFinalityReward({ epoch: epoch, validatorIds: allCids });
if (periodEnding) {
ISlashIndicator slashIndicatorContract = ISlashIndicator(getContract(ContractType.SLASH_INDICATOR));
// Slash submit random beacon proof unavailability first, then update credit scores.
randomBeacon.execRecordAndSlashUnavailability(lastPeriod, newPeriod, address(slashIndicatorContract), allCids);
slashIndicatorContract.execUpdateCreditScores(allCids, lastPeriod);
(uint256[] memory delegatorBlockMiningRewards, uint256[] memory delegatorFastFinalityRewards) =
_distributeRewardToTreasuriesAndCalculateTotalDelegatorsReward(lastPeriod, allCids);
_settleAndTransferDelegatingRewards(
lastPeriod, allCids, delegatorBlockMiningRewards, delegatorFastFinalityRewards
);
_tryRecycleLockedFundsFromEmergencyExits();
_recycleDeprecatedRewards();
address[] memory revokedCandidateIds = _syncCandidateSet(newPeriod);
if (revokedCandidateIds.length > 0) {
// Re-update `allCids` after unsatisfied candidates get removed.
allCids = _candidateIds;
slashIndicatorContract.execResetCreditScores(revokedCandidateIds);
}
// Wrap up the beacon period includes (1) finalizing the beacon proof, and (2) determining the validator list for the next period by new proof.
// Should wrap up the beacon after unsatisfied candidates get removed.
randomBeacon.execFinalizeBeaconAndPendingCids(lastPeriod, newPeriod, allCids);
_periodEndBlock[lastPeriod] = block.number;
_currentPeriodStartAtBlock = block.number + 1;
}
// Clear the previous validator set and block producer set before sync the new set from beacon.
_clearPreviousValidatorSetAndBlockProducerSet();
// Query the new validator set for upcoming epoch from the random beacon contract.
// Save new set into the contract storage.
address[] memory newValidatorIds = _syncValidatorSet(randomBeacon, newPeriod, nextEpoch);
// Activate applicable validators into the block producer set.
_updateApplicableValidatorToBlockProducerSet(newPeriod, nextEpoch, newValidatorIds);
emit WrappedUpEpoch(lastPeriod, epoch, periodEnding);
_periodOf[nextEpoch] = newPeriod;
_lastUpdatedPeriod = newPeriod;
}
}

function wrapUpEpoch() external payable virtual override onlyCoinbase whenEpochEnding oncePerEpoch {
unchecked {
uint256 newPeriod = _computePeriod(block.timestamp);
bool periodEnding = _isPeriodEnding(newPeriod);
uint256 lastPeriod = currentPeriod();
uint256 epoch = epochOf(block.number);
uint256 nextEpoch = epoch + 1;
IRandomBeacon randomBeacon = IRandomBeacon(getContract(ContractType.RANDOM_BEACON));
// This request is actually only invoked at the first epoch of the period.
randomBeacon.execRequestRandomSeedForNextPeriod(lastPeriod, newPeriod);
// Get all candidate ids
address[] memory allCids = _candidateIds;
_syncFastFinalityReward({ epoch: epoch, validatorIds: allCids });
if (periodEnding) {
ISlashIndicator slashIndicatorContract = ISlashIndicator(getContract(ContractType.SLASH_INDICATOR));
// Slash submit random beacon proof unavailability first, then update credit scores.
randomBeacon.execRecordAndSlashUnavailability(lastPeriod, newPeriod, address(slashIndicatorContract), allCids);
slashIndicatorContract.execUpdateCreditScores(allCids, lastPeriod);
(uint256[] memory delegatorBlockMiningRewards, uint256[] memory delegatorFastFinalityRewards) =
_distributeRewardToTreasuriesAndCalculateTotalDelegatorsReward(lastPeriod, allCids);
_settleAndTransferDelegatingRewards(
lastPeriod, allCids, delegatorBlockMiningRewards, delegatorFastFinalityRewards
);
_tryRecycleLockedFundsFromEmergencyExits();
_recycleDeprecatedRewards();
address[] memory revokedCandidateIds = _syncCandidateSet(newPeriod);
if (revokedCandidateIds.length > 0) {
// Re-update `allCids` after unsatisfied candidates get removed.
allCids = _candidateIds;
slashIndicatorContract.execResetCreditScores(revokedCandidateIds);
}
// Wrap up the beacon period includes (1) finalizing the beacon proof, and (2) determining the validator list for the next period by new proof.
// Should wrap up the beacon after unsatisfied candidates get removed.
randomBeacon.execFinalizeBeaconAndPendingCids(lastPeriod, newPeriod, allCids);
_periodEndBlock[lastPeriod] = block.number;
_currentPeriodStartAtBlock = block.number + 1;
}
// Clear the previous validator set and block producer set before sync the new set from beacon.
_clearPreviousValidatorSetAndBlockProducerSet();
// Query the new validator set for upcoming epoch from the random beacon contract.
// Save new set into the contract storage.
address[] memory newValidatorIds = _syncValidatorSet(randomBeacon, newPeriod, nextEpoch);
// Activate applicable validators into the block producer set.
_updateApplicableValidatorToBlockProducerSet(newPeriod, nextEpoch, newValidatorIds);
emit WrappedUpEpoch(lastPeriod, epoch, periodEnding);
_periodOf[nextEpoch] = newPeriod;
_lastUpdatedPeriod = newPeriod;
}
}

shadowing-state

Impact: High
Confidence: High

uint256[49] private ______gap;

uint256[50] private ______gap;

uninitialized-state

Impact: High
Confidence: High

uint256 internal _firstTrackedPeriodEnd;

uint256 internal _numberOfBlocksInEpoch;

mapping(uint256 epoch => uint256 period) internal _periodOf;

mapping(address => mapping(uint256 => bool)) internal _miningRewardDeprecatedAtPeriod;

uint256 internal _lastUpdatedPeriod;

mapping(address => uint256) internal _blockProducerJailedBlock;

mapping(address cid => bool isBlockProducer) internal _validatorMap;

mapping(uint256 period => uint256 endedAtBlock) internal _periodEndBlock;

mapping(uint256 idx => address cid) internal _validatorIds;

uint256 internal _currentPeriodStartAtBlock;

@TuDo1403 TuDo1403 changed the base branch from mainnet to release/v0.8.2 June 13, 2025 07:08
@TuDo1403 TuDo1403 changed the title feat(REP-0024): implement logic feat(REP-0024): implement logic and unit-tests Jun 17, 2025
Copy link
Collaborator

@huyhuynh3103 huyhuynh3103 left a comment

Choose a reason for hiding this comment

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

Do you have the storage layout log?

error ErrEmptyArray();

struct Reward {
uint64 startBlock;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using uint64 for block numbers is quite risky, since we have a plan to update the block time to 1 second

/// @dev The boolean flag to check if REP-10 is activated.
bool internal _isREP10Activated;
bool internal __deprecatedIsREP10Activated;
Reward[] internal _blockRewards;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add comments

) internal {
uint256 length = rewards.length;
if (length == 0) revert ErrEmptyArray();
delete _blockRewards;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This just sets the array length to zero and leaves the values at each index untouched, right? Is that expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no that actually delete the block reward array imo

/**
* @inheritdoc IStakingVesting
*/
function receiveRON() external payable { }
Copy link
Collaborator

Choose a reason for hiding this comment

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

This logic is not mentioned in REP

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is untouched.

_lastBlockSendingBonus = block.number;

blockProducerBonus = forBlockProducer ? blockProducerBlockBonus(block.number) : 0;
bridgeOperatorBonus = forBridgeOperator ? bridgeOperatorBlockBonus(block.number) : 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This logic also is not mentioned in REP#24

Copy link
Contributor Author

Choose a reason for hiding this comment

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

deprecated long ago

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants