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

Penalty for reward as alternative to slashing #254

Merged
merged 5 commits into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
160 changes: 148 additions & 12 deletions contracts/contracts/TACoApplication.sol
Original file line number Diff line number Diff line change
Expand Up @@ -167,25 +167,45 @@ contract TACoApplication is
address operator
);

/**
* @notice Signals that the staking provider was penalized
* @param stakingProvider Staking provider address
* @param penaltyPercent Percent of reward that was penalized
* @param endPenalty End of penalty
*/
event Penalized(address indexed stakingProvider, uint256 penaltyPercent, uint64 endPenalty);

/**
* @notice Signals that reward was reset after penalty
* @param stakingProvider Staking provider address
*/
event RewardReset(address indexed stakingProvider);

struct StakingProviderInfo {
address operator;
bool operatorConfirmed;
uint64 operatorStartTimestamp;
uint96 authorized;
uint96 deauthorizing; // TODO real usage only in getActiveStakingProviders, maybe remove?
uint96 deauthorizing;
uint64 endDeauthorization;
uint96 tReward;
uint160 rewardPerTokenPaid;
uint64 endCommitment;
uint256 stub;
manumonti marked this conversation as resolved.
Show resolved Hide resolved
uint192 penaltyPercent;
cygnusv marked this conversation as resolved.
Show resolved Hide resolved
uint64 endPenalty;
}

uint256 public constant REWARD_PER_TOKEN_MULTIPLIER = 10 ** 3;
uint256 internal constant FLOATING_POINT_DIVISOR = REWARD_PER_TOKEN_MULTIPLIER * 10 ** 18;
uint256 public constant PENALTY_BASE = 10000;

uint96 public immutable minimumAuthorization;
uint256 public immutable minOperatorSeconds;
uint256 public immutable rewardDuration;
uint256 public immutable deauthorizationDuration;
uint192 public immutable penaltyDefault;
uint256 public immutable penaltyDuration;

uint64 public immutable commitmentDurationOption1;
uint64 public immutable commitmentDurationOption2;
Expand Down Expand Up @@ -222,6 +242,8 @@ contract TACoApplication is
* @param _deauthorizationDuration Duration of decreasing authorization in seconds
* @param _commitmentDurationOptions Options for commitment duration
* @param _commitmentDeadline Last date to make a commitment
* @param _penaltyDefault Default penalty percentage (as a value out of 10000)
* @param _penaltyDuration Duration of penalty
*/
constructor(
IERC20 _token,
Expand All @@ -231,15 +253,20 @@ contract TACoApplication is
uint256 _rewardDuration,
uint256 _deauthorizationDuration,
uint64[] memory _commitmentDurationOptions,
uint64 _commitmentDeadline
uint64 _commitmentDeadline,
uint192 _penaltyDefault,
uint256 _penaltyDuration
) {
uint256 totalSupply = _token.totalSupply();
require(
_rewardDuration != 0 &&
_tStaking.authorizedStake(address(this), address(this)) == 0 &&
totalSupply > 0 &&
_commitmentDurationOptions.length >= 1 &&
_commitmentDurationOptions.length <= 4,
_commitmentDurationOptions.length <= 4 &&
_penaltyDefault > 0 &&
_penaltyDefault < PENALTY_BASE &&
_penaltyDuration > 0,
"Wrong input parameters"
);
// This require is only to check potential overflow for 10% reward
Expand All @@ -266,6 +293,8 @@ contract TACoApplication is
? _commitmentDurationOptions[3]
: 0;
commitmentDeadline = _commitmentDeadline;
penaltyDefault = _penaltyDefault;
penaltyDuration = _penaltyDuration;
_disableInitializers();
}

Expand Down Expand Up @@ -382,15 +411,36 @@ contract TACoApplication is
* @param _stakingProvider Staking provider address
*/
function updateRewardInternal(address _stakingProvider) internal {
StakingProviderInfo storage info = stakingProviderInfo[_stakingProvider];
if (
_stakingProvider != address(0) &&
info.endPenalty != 0 &&
info.endPenalty <= block.timestamp
) {
resetReward(_stakingProvider, info);
}

rewardPerTokenStored = rewardPerToken();
lastUpdateTime = lastTimeRewardApplicable();
if (_stakingProvider != address(0)) {
StakingProviderInfo storage info = stakingProviderInfo[_stakingProvider];
info.tReward = availableRewards(_stakingProvider);
info.rewardPerTokenPaid = rewardPerTokenStored;
}
}

/**
* @notice Resets reward after penalty
*/
function resetReward(address _stakingProvider, StakingProviderInfo storage _info) internal {
uint96 before = effectiveAuthorized(_info.authorized, _info.penaltyPercent);
_info.endPenalty = 0;
_info.penaltyPercent = 0;
if (_info.operatorConfirmed) {
authorizedOverall += _info.authorized - before;
}
emit RewardReset(_stakingProvider);
}

/**
* @notice Returns last time when reward was applicable
*/
Expand Down Expand Up @@ -420,12 +470,49 @@ contract TACoApplication is
if (!info.operatorConfirmed) {
return info.tReward;
}
uint256 result = (uint256(info.authorized) * (rewardPerToken() - info.rewardPerTokenPaid)) /
uint96 authorized = effectiveAuthorized(info.authorized, info);
uint256 result = (uint256(authorized) * (rewardPerToken() - info.rewardPerTokenPaid)) /
FLOATING_POINT_DIVISOR +
info.tReward;
return result.toUint96();
}

function effectiveAuthorized(
uint96 _authorized,
uint192 _penaltyPercent
) internal pure returns (uint96) {
return uint96((_authorized * (PENALTY_BASE - _penaltyPercent)) / PENALTY_BASE);
}

/// @dev In case that a penalty period already ended, this view method may produce
/// outdated results if the penalty hasn't been reset, either by calling
/// `resetReward` explicitly or any function with the `updateReward` modifier.
function effectiveAuthorized(
uint96 _authorized,
StakingProviderInfo storage _info
) internal view returns (uint96) {
if (_info.endPenalty == 0) {
return _info.authorized;
}
return effectiveAuthorized(_authorized, _info.penaltyPercent);
}

/// @dev In case that a penalty period already ended, this view method may produce
/// outdated results if the penalty hasn't been reset, either by calling
/// `resetReward` explicitly or any function with the `updateReward` modifier.
function effectiveDifference(
uint96 _from,
uint96 _to,
StakingProviderInfo storage _info
) internal view returns (uint96) {
if (_info.endPenalty == 0) {
return _from - _to;
}
uint96 effectiveFrom = effectiveAuthorized(_from, _info.penaltyPercent);
uint96 effectiveTo = effectiveAuthorized(_to, _info.penaltyPercent);
return effectiveFrom - effectiveTo;
Comment on lines +508 to +513
Copy link
Member

Choose a reason for hiding this comment

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

Can you just say return effectiveAuthorized(_from, _to)? When endPenalty == 0, L492 will make sure the result is _from - _to.

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure that I follow you, second value in effectiveAuthorized is penalty percent, can you elaborate?
in general I made this thing to decrease read operations (still costly)

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, this is what I meant:

Suggested change
if (_info.endPenalty == 0) {
return _from - _to;
}
uint96 effectiveFrom = effectiveAuthorized(_from, _info.penaltyPercent);
uint96 effectiveTo = effectiveAuthorized(_to, _info.penaltyPercent);
return effectiveFrom - effectiveTo;
return effectiveAuthorized(_from - _to);

Copy link
Member Author

Choose a reason for hiding this comment

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

now I see your question, but I didn't do that on purpose, reason is it could produce different result because of rounding errors, for all other places it's division first and then addition/subtraction.

Copy link
Member

Choose a reason for hiding this comment

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

Opened #260

}

/**
* @notice Transfer reward for the next period. Can be called only by distributor
* @param _reward Amount of reward
Expand Down Expand Up @@ -477,7 +564,7 @@ contract TACoApplication is
uint96 _properAmount
) internal {
if (_info.authorized != _properAmount) {
authorizedOverall -= _info.authorized - _properAmount;
authorizedOverall -= effectiveDifference(_info.authorized, _properAmount, _info);
}
}

Expand Down Expand Up @@ -507,7 +594,7 @@ contract TACoApplication is

if (info.operatorConfirmed) {
resynchronizeAuthorizedOverall(info, _fromAmount);
authorizedOverall += _toAmount - _fromAmount;
authorizedOverall += effectiveDifference(_toAmount, _fromAmount, info);
}

info.authorized = _toAmount;
Expand All @@ -529,7 +616,7 @@ contract TACoApplication is
StakingProviderInfo storage info = stakingProviderInfo[_stakingProvider];
if (info.operatorConfirmed) {
resynchronizeAuthorizedOverall(info, _fromAmount);
authorizedOverall -= _fromAmount - _toAmount;
authorizedOverall -= effectiveDifference(_fromAmount, _toAmount, info);
}

info.authorized = _toAmount;
Expand Down Expand Up @@ -593,7 +680,7 @@ contract TACoApplication is
uint96 toAmount = tStaking.approveAuthorizationDecrease(_stakingProvider);

if (info.operatorConfirmed) {
authorizedOverall -= info.authorized - toAmount;
authorizedOverall -= effectiveDifference(info.authorized, toAmount, info);
}

emit AuthorizationDecreaseApproved(_stakingProvider, info.authorized, toAmount);
Expand All @@ -619,7 +706,7 @@ contract TACoApplication is
require(info.authorized > newAuthorized, "Nothing to synchronize");

if (info.operatorConfirmed) {
authorizedOverall -= info.authorized - newAuthorized;
authorizedOverall -= effectiveDifference(info.authorized, newAuthorized, info);
}
emit AuthorizationReSynchronized(_stakingProvider, info.authorized, newAuthorized);

Expand Down Expand Up @@ -726,6 +813,17 @@ contract TACoApplication is
return uint64(endDeauthorization - block.timestamp);
}

/**
* @notice Returns information about reward penalty.
*/
function getPenalty(
address _stakingProvider
) external view returns (uint192 penalty, uint64 endPenalty) {
StakingProviderInfo storage info = stakingProviderInfo[_stakingProvider];
penalty = info.penaltyPercent;
endPenalty = info.endPenalty;
}

/**
* @notice Get the value of authorized tokens for active providers as well as providers and their authorized tokens
* @param _startIndex Start index for looking in providers array
Expand Down Expand Up @@ -860,7 +958,7 @@ contract TACoApplication is
}

if (info.operatorConfirmed) {
authorizedOverall -= info.authorized;
authorizedOverall -= effectiveAuthorized(info.authorized, info);
}

// Bond new operator (or unbond if _operator == address(0))
Expand Down Expand Up @@ -891,7 +989,7 @@ contract TACoApplication is
if (!info.operatorConfirmed) {
updateRewardInternal(stakingProvider);
info.operatorConfirmed = true;
authorizedOverall += info.authorized;
authorizedOverall += effectiveAuthorized(info.authorized, info);
emit OperatorConfirmed(stakingProvider, _operator);
}
}
Expand Down Expand Up @@ -957,4 +1055,42 @@ contract TACoApplication is
stakingProviderWrapper[0] = _stakingProvider;
tStaking.seize(_penalty, 100, _investigator, stakingProviderWrapper);
}

/**
* @notice Penalize the staking provider's future reward
* @param _stakingProvider Staking provider address
*/
function penalize(address _stakingProvider) external updateReward(_stakingProvider) {
require(
msg.sender == address(childApplication),
"Only child application allowed to penalize"
);

if (_stakingProvider == address(0)) {
return;
}

StakingProviderInfo storage info = stakingProviderInfo[_stakingProvider];
uint96 before = effectiveAuthorized(info.authorized, info.penaltyPercent);
info.endPenalty = uint64(block.timestamp + penaltyDuration);
vzotova marked this conversation as resolved.
Show resolved Hide resolved
info.penaltyPercent = penaltyDefault;
cygnusv marked this conversation as resolved.
Show resolved Hide resolved
if (info.operatorConfirmed) {
authorizedOverall -= before - effectiveAuthorized(info.authorized, info.penaltyPercent);
}
emit Penalized(_stakingProvider, info.penaltyPercent, info.endPenalty);
}

/**
* @notice Resets future reward back to 100%.
* Either this method or any method with `updateReward` modifier should be called
* to stop penalties. Otherwise, reward will be still subtracted
* even after the end of penalties.
* @param _stakingProvider Staking provider address
*/
function resetReward(address _stakingProvider) external {
Copy link
Member

Choose a reason for hiding this comment

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

Can you clarify what happens in resetReward is not called for a penalized staking provider whose penalty ended?

Copy link
Member Author

Choose a reason for hiding this comment

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

resetReward also included in updareReward, so if it none such methods will be called then reward calculation will be looking like staker still has penalty. I know it's not great but with our model - we need active input to update values

Copy link
Member

Choose a reason for hiding this comment

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

In that case, this needs to be stated in the @notice.

Copy link
Member Author

Choose a reason for hiding this comment

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

✔️

StakingProviderInfo storage info = stakingProviderInfo[_stakingProvider];
require(info.endPenalty != 0, "There is no penalty");
require(info.endPenalty <= block.timestamp, "Penalty is still ongoing");
updateRewardInternal(_stakingProvider);
}
}
2 changes: 2 additions & 0 deletions contracts/contracts/coordination/ITACoChildToRoot.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ interface ITACoChildToRoot {
event OperatorConfirmed(address indexed stakingProvider, address indexed operator);

function confirmOperatorAddress(address operator) external;

function penalize(address stakingProvider) external;
}
27 changes: 24 additions & 3 deletions contracts/contracts/coordination/TACoChildApplication.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ import "./Coordinator.sol";
* @notice TACoChildApplication
*/
contract TACoChildApplication is ITACoRootToChild, ITACoChildApplication, Initializable {
/**
* @notice Signals that the staking provider was penalized
* @param stakingProvider Staking provider address
*/
event Penalized(address indexed stakingProvider);

struct StakingProviderInfo {
address operator;
uint96 authorized;
Expand All @@ -25,6 +31,7 @@ contract TACoChildApplication is ITACoRootToChild, ITACoChildApplication, Initia

ITACoChildToRoot public immutable rootApplication;
address public coordinator;
address public adjudicator;

uint96 public immutable minimumAuthorization;

Expand Down Expand Up @@ -54,14 +61,18 @@ contract TACoChildApplication is ITACoRootToChild, ITACoChildApplication, Initia
/**
* @notice Initialize function for using with OpenZeppelin proxy
*/
function initialize(address _coordinator) external initializer {
require(coordinator == address(0), "Coordinator already set");
require(_coordinator != address(0), "Coordinator must be specified");
function initialize(address _coordinator, address _adjudicator) external initializer {
require(coordinator == address(0) || adjudicator == address(0), "Contracts already set");
require(
_coordinator != address(0) && _adjudicator != address(0),
"Contracts must be specified"
);
require(
address(Coordinator(_coordinator).application()) == address(this),
"Invalid coordinator"
);
coordinator = _coordinator;
adjudicator = _adjudicator;
}

function authorizedStake(address _stakingProvider) external view returns (uint96) {
Expand Down Expand Up @@ -184,6 +195,16 @@ contract TACoChildApplication is ITACoRootToChild, ITACoChildApplication, Initia
rootApplication.confirmOperatorAddress(_operator);
}

/**
* @notice Penalize the staking provider's future reward
* @param _stakingProvider Staking provider address
*/
function penalize(address _stakingProvider) external override {
require(msg.sender == address(adjudicator), "Only adjudicator allowed to penalize");
rootApplication.penalize(_stakingProvider);
emit Penalized(_stakingProvider);
}

/**
* @notice Return the length of the array of staking providers
*/
Expand Down
7 changes: 7 additions & 0 deletions contracts/contracts/testnet/LynxSet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ contract MockPolygonRoot is Ownable, ITACoChildToRoot, ITACoRootToChild {
rootApplication.confirmOperatorAddress(operator);
}

function penalize(address _stakingProvider) external override onlyOwner {
rootApplication.penalize(_stakingProvider);
}

// solhint-disable-next-line no-empty-blocks
function updateOperator(address stakingProvider, address operator) external {}

Expand Down Expand Up @@ -87,6 +91,9 @@ contract MockPolygonChild is Ownable, ITACoChildToRoot, ITACoRootToChild {

// solhint-disable-next-line no-empty-blocks
function confirmOperatorAddress(address _operator) external override {}

// solhint-disable-next-line no-empty-blocks
function penalize(address _stakingProvider) external override {}
}

contract LynxRitualToken is ERC20("LynxRitualToken", "LRT") {
Expand Down
3 changes: 3 additions & 0 deletions contracts/test/CoordinatorTestSet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ contract ChildApplicationForCoordinatorMock is ITACoChildApplication {
function confirmOperatorAddress(address _operator) external {
confirmations[_operator] = true;
}

// solhint-disable-next-line no-empty-blocks
function penalize(address _stakingProvider) external {}
}

// /**
Expand Down
Loading
Loading