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

feat(BridgeReward, BridgeTracking): improve sync reward in bridge reward #14

Merged
merged 11 commits into from
Mar 25, 2024
16 changes: 5 additions & 11 deletions src/interfaces/bridge/IBridgeReward.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import { IBridgeRewardEvents } from "./events/IBridgeRewardEvents.sol";
interface IBridgeReward is IBridgeRewardEvents {
/**
* @dev This function allows bridge operators to manually synchronize the reward for a given period length.
* @param periodLength The length of the reward period for which synchronization is requested.
* @param periodCount The length of the reward period for which synchronization is requested.
*/
function syncReward(uint256 periodLength) external;
function syncRewardManual(uint256 periodCount) external;

/**
* @dev Receives RON from any address.
Expand All @@ -21,15 +21,9 @@ interface IBridgeReward is IBridgeRewardEvents {
*
* Requirements:
* - This method is only called once each period.
* - The caller must be the bridge tracking contract or a bridge operator.
*/
function execSyncReward(
address[] calldata operators,
uint256[] calldata ballots,
uint256 totalBallot,
uint256 totalVote,
uint256 period
) external;
* - The caller must be the bridge tracking contract
*/
function execSyncRewardAuto(uint256 currentPeriod) external;

/**
* @dev Retrieve the total amount of rewards that have been topped up in the contract.
Expand Down
4 changes: 4 additions & 0 deletions src/interfaces/bridge/events/IBridgeRewardEvents.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,8 @@ interface IBridgeRewardEvents {
event BridgeRewardScatterFailed(uint256 indexed period, address operator, uint256 amount);
/// @dev Event emitted when the requesting period to sync is too far.
event BridgeRewardSyncTooFarPeriod(uint256 requestingPeriod, uint256 latestPeriod);

error ErrPeriodNotHappen(uint256 currentPeriod, uint256 latestRewardedPeriod, uint256 periodCount);
error ErrPeriodAlreadyRewarded(uint256 currentPeriod, uint256 latestRewardedPeriod);
error ErrPeriodCountIsZero();
}
132 changes: 87 additions & 45 deletions src/ronin/gateway/BridgeReward.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ contract BridgeReward is IBridgeReward, BridgeTrackingHelper, HasContracts, RONT
TUint256Slot private constant $_TOTAL_REWARDS_TOPPED_UP = TUint256Slot.wrap(0x9a8c9f129792436c37b7bd2d79c56132fc05bf26cc8070794648517c2a0c6c64);
/// @dev value is equal to keccak256("@ronin.dpos.gateway.BridgeReward.totalRewardScattered.slot") - 1
TUint256Slot private constant $_TOTAL_REWARDS_SCATTERED = TUint256Slot.wrap(0x3663384f6436b31a97d9c9a02f64ab8b73ead575c5b6224fa0800a6bd57f62f4);
/// @dev value is equal to keccak256("@ronin.dpos.gateway.BridgeReward.maxRewardingPeriodCount.slot") - 1
TUint256Slot private constant $_MAX_REWARDING_PERIOD_COUNT = TUint256Slot.wrap(0xaf260ffaff563b9407c1c5fe4aec2be8632142d158c44bb0ce4d471cb4883b8c);

address private immutable _self;

Expand Down Expand Up @@ -54,14 +56,32 @@ contract BridgeReward is IBridgeReward, BridgeTrackingHelper, HasContracts, RONT

/**
* @dev Helper for running upgrade script, required to only revoked once by the DPoS's governance admin.
* The following must be assured after initializing REP2: `_lastSyncPeriod` == `{BridgeReward}.latestRewardedPeriod` == `currentPeriod()`
* The following must be assured after initializing REP2:
* ```
* {BridgeTracking}._lastSyncPeriod
* == {BridgeReward}.latestRewardedPeriod
* == {RoninValidatorSet}.currentPeriod()
* ```
*/
function initializeREP2() external onlyContract(ContractType.GOVERNANCE_ADMIN) {
require(getLatestRewardedPeriod() == type(uint256).max, "already init rep 2");
$_LATEST_REWARDED_PERIOD.store(IRoninValidatorSet(getContract(ContractType.VALIDATOR)).currentPeriod() - 1);
_setContract(ContractType.GOVERNANCE_ADMIN, address(0));
}

/**
@dev The following must be assured after initializing V2:
* ```
* {BridgeTracking}._lastSyncPeriod
* == {RoninValidatorSet}.currentPeriod()
* == {BridgeReward}.latestRewardedPeriod + 1
* ```
*/
function initializeV2() external reinitializer(2) {
$_MAX_REWARDING_PERIOD_COUNT.store(5);
$_LATEST_REWARDED_PERIOD.store(getLatestRewardedPeriod() - 1);
}

/**
* @inheritdoc IBridgeReward
*/
Expand All @@ -72,55 +92,82 @@ contract BridgeReward is IBridgeReward, BridgeTrackingHelper, HasContracts, RONT
/**
* @inheritdoc IBridgeReward
*/
function syncReward(uint256 periodLength) external {
function syncRewardManual(uint256 periodCount) external {
if (!_isBridgeOperator(msg.sender)) revert ErrUnauthorizedCall(msg.sig);
uint256 currPd = IRoninValidatorSet(getContract(ContractType.VALIDATOR)).currentPeriod();

uint256 latestRewardedPeriod = getLatestRewardedPeriod();
uint256 currentPeriod = IRoninValidatorSet(getContract(ContractType.VALIDATOR)).currentPeriod();
_syncRewardBatch({ currPd: currPd, pdCount: periodCount });
}

if (currentPeriod <= latestRewardedPeriod) revert ErrInvalidArguments(msg.sig);
if (latestRewardedPeriod + periodLength > currentPeriod) revert ErrInvalidArguments(msg.sig);
/**
* @inheritdoc IBridgeReward
*/
function execSyncRewardAuto(uint256 currentPeriod) external onlyContract(ContractType.BRIDGE_TRACKING) {
_syncRewardBatch({ currPd: currentPeriod, pdCount: 0 });
}

$_LATEST_REWARDED_PERIOD.addAssign(periodLength);
/**
* @dev Sync bridge reward for multiple periods, always assert `latestRewardedPeriod + periodCount < currentPeriod`.
* @param pdCount Number of periods to settle reward. Leave this as 0 to auto calculate.
*/
function _syncRewardBatch(uint256 currPd, uint256 pdCount) internal {
uint256 lastRewardPd = getLatestRewardedPeriod();
if (pdCount == 0) {
uint toSettlePdCount;
if (currPd > lastRewardPd) {
toSettlePdCount = currPd - lastRewardPd - 1;
}

// Restrict number of period to reward in a transaction, to avoid consume too much gas
pdCount = Math.min(toSettlePdCount, $_MAX_REWARDING_PERIOD_COUNT.load());
}

_assertPeriod({ currPd: currPd, pdCount: pdCount, lastRewardPd: lastRewardPd });

address[] memory operators = IBridgeManager(getContract(ContractType.BRIDGE_MANAGER)).getBridgeOperators();
IBridgeTracking bridgeTrackingContract = IBridgeTracking(getContract(ContractType.BRIDGE_TRACKING));

for (uint256 i = 1; i <= periodLength; i++) {
_syncReward({
for (uint256 i = 0; i < pdCount; i++) {
++lastRewardPd;
_settleReward({
operators: operators,
ballots: bridgeTrackingContract.getManyTotalBallots(latestRewardedPeriod, operators),
totalBallot: bridgeTrackingContract.totalBallot(latestRewardedPeriod),
totalVote: bridgeTrackingContract.totalVote(latestRewardedPeriod),
period: latestRewardedPeriod + i
ballots: bridgeTrackingContract.getManyTotalBallots(lastRewardPd, operators),
totalBallot: bridgeTrackingContract.totalBallot(lastRewardPd),
totalVote: bridgeTrackingContract.totalVote(lastRewardPd),
period: lastRewardPd
});
}
}

/**
* @inheritdoc IBridgeReward
* @dev
* Before the last valid rewarding:
* |----------|------------------|------------------------------|-----------------|
* ^ ^ ^
* Validator.current
* Reward.lastReward
* Tracking.lastSync
* Tracking.ballotInfo
* Slash.slashInfo
*
* @dev The `period` a.k.a. `latestSyncedPeriod` must equal to `latestRewardedPeriod` + 1.
*
* After the last valid rewarding, the lastRewardedPeriod always slower than currentPeriod:
* |----------|------------------|------------------------------|-----------------|
* ^ ^
* Validator.current
* Reward.lastReward
* Tracking.lastSync
* Tracking.ballotInfo
* Slash.slashInfo
*/
function execSyncReward(
address[] calldata operators,
uint256[] calldata ballots,
uint256 totalBallot,
uint256 totalVote,
uint256 period
) external onlyContract(ContractType.BRIDGE_TRACKING) {
if (operators.length != ballots.length) revert ErrLengthMismatch(msg.sig);
if (operators.length == 0) return;

// Only sync the period that is after the latest rewarded period, i.e. `latestSyncedPeriod == latestRewardedPeriod + 1`.
unchecked {
uint256 latestRewardedPeriod = getLatestRewardedPeriod();
if (period < latestRewardedPeriod + 1) revert ErrInvalidArguments(msg.sig);
else if (period > latestRewardedPeriod + 1) revert ErrSyncTooFarPeriod(period, latestRewardedPeriod);
}
$_LATEST_REWARDED_PERIOD.store(period);
function _assertPeriod(uint256 currPd, uint256 pdCount, uint256 lastRewardPd) internal pure {
if (pdCount == 0) revert ErrPeriodCountIsZero();

// Not settle the period that already rewarded.
if (currPd <= lastRewardPd + 1) revert ErrPeriodAlreadyRewarded(currPd, lastRewardPd);

Choose a reason for hiding this comment

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

This line is not necessary as pdCount must be >= 1 (as last condition), combined with the following validation is sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noted, but I leave it unchanged, added in-line comment


_syncReward({ operators: operators, ballots: ballots, totalBallot: totalBallot, totalVote: totalVote, period: period });
// Not settle the periods that not happen yet.
if (currPd <= lastRewardPd + pdCount) revert ErrPeriodNotHappen(currPd, lastRewardPd, pdCount);
}

/**
Expand Down Expand Up @@ -156,8 +203,10 @@ contract BridgeReward is IBridgeReward, BridgeTrackingHelper, HasContracts, RONT
* @param totalVote The total number of votes recorded for the period.
* @param period The period for which the rewards are being synchronized.
*/
function _syncReward(address[] memory operators, uint256[] memory ballots, uint256 totalBallot, uint256 totalVote, uint256 period) internal {
function _settleReward(address[] memory operators, uint256[] memory ballots, uint256 totalBallot, uint256 totalVote, uint256 period) internal {
uint256 numBridgeOperators = operators.length;
if (numBridgeOperators != ballots.length) revert ErrLengthMismatch(msg.sig);

uint256 rewardPerPeriod = getRewardPerPeriod();
uint256[] memory slashedDurationList = _getSlashInfo(operators);
// Validate should share the reward equally
Expand All @@ -167,7 +216,7 @@ contract BridgeReward is IBridgeReward, BridgeTrackingHelper, HasContracts, RONT
bool shouldSlash;
uint256 sumRewards;

for (uint256 i; i < numBridgeOperators; ) {
for (uint256 i; i < numBridgeOperators; i++) {
(reward, shouldSlash) = _calcRewardAndCheckSlashedStatus({
shouldShareEqually: shouldShareEqually,
numBridgeOperators: numBridgeOperators,
Expand All @@ -180,13 +229,10 @@ contract BridgeReward is IBridgeReward, BridgeTrackingHelper, HasContracts, RONT

sumRewards += shouldSlash ? 0 : reward;
_updateRewardAndTransfer({ period: period, operator: operators[i], reward: reward, shouldSlash: shouldSlash });

unchecked {
++i;
}
}

$_TOTAL_REWARDS_SCATTERED.addAssign(sumRewards);
nxqbao marked this conversation as resolved.
Show resolved Hide resolved
$_LATEST_REWARDED_PERIOD.store(period);
}

/**
Expand Down Expand Up @@ -231,13 +277,9 @@ contract BridgeReward is IBridgeReward, BridgeTrackingHelper, HasContracts, RONT

/**
* @dev Internal function to check if a specific period should be considered as slashed based on the slash duration.
* @param period The period to check if it should be slashed.
* @param slashDuration The duration until which periods should be considered as slashed.
* @return shouldSlashed A boolean indicating whether the specified period should be slashed.
* @notice This function is used internally to determine if a particular period should be marked as slashed based on the slash duration.
*/
function _shouldSlashedThisPeriod(uint256 period, uint256 slashDuration) internal pure returns (bool) {
return period <= slashDuration;
function _shouldSlashedThisPeriod(uint256 period, uint256 slashUntil) internal pure returns (bool) {
return period <= slashUntil;
}

/**
Expand Down
Loading