From c47b4c6ad187879fa4f36233c3e9c9f6fec04e85 Mon Sep 17 00:00:00 2001 From: Victoria Zotova Date: Fri, 12 Apr 2024 13:35:26 -0400 Subject: [PATCH 1/5] TACoApplication: draft of reward only penalty --- contracts/contracts/TACoApplication.sol | 113 +++++++++++++++++++++--- tests/application/conftest.py | 5 ++ 2 files changed, 107 insertions(+), 11 deletions(-) diff --git a/contracts/contracts/TACoApplication.sol b/contracts/contracts/TACoApplication.sol index ca2be1e8..2fa50c45 100644 --- a/contracts/contracts/TACoApplication.sol +++ b/contracts/contracts/TACoApplication.sol @@ -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, uint256 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; + uint192 penaltyPercent; + 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; @@ -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 + * @param _penaltyDuration Duration of penalty */ constructor( IERC20 _token, @@ -231,7 +253,9 @@ contract TACoApplication is uint256 _rewardDuration, uint256 _deauthorizationDuration, uint64[] memory _commitmentDurationOptions, - uint64 _commitmentDeadline + uint64 _commitmentDeadline, + uint192 _penaltyDefault, + uint256 _penaltyDuration ) { uint256 totalSupply = _token.totalSupply(); require( @@ -239,7 +263,10 @@ contract TACoApplication is _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 @@ -266,6 +293,8 @@ contract TACoApplication is ? _commitmentDurationOptions[3] : 0; commitmentDeadline = _commitmentDeadline; + penaltyDefault = _penaltyDefault; + penaltyDuration = _penaltyDuration; _disableInitializers(); } @@ -420,12 +449,46 @@ 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(); } + // FIXME + function effectiveAuthorized( + uint96 _authorized, + uint192 _penaltyPercent + ) internal view returns (uint96) { + return uint96((_authorized * (PENALTY_BASE - _penaltyPercent)) / PENALTY_BASE); + } + + function effectiveAuthorized( + uint96 _authorized, + StakingProviderInfo storage _info + ) internal view returns (uint96) { + if (_info.endPenalty != 0 && _info.endPenalty > block.timestamp) { + return effectiveAuthorized(_authorized, _info.penaltyPercent); + } else { + return _info.authorized; + } + } + + function effectiveDifference( + uint96 _from, + uint96 _to, + StakingProviderInfo storage _info + ) internal view returns (uint96) { + if (_info.endPenalty != 0 && _info.endPenalty > block.timestamp) { + uint96 effectiveFrom = effectiveAuthorized(_from, _info.penaltyPercent); + uint96 effectiveTo = effectiveAuthorized(_to, _info.penaltyPercent); + return effectiveFrom - effectiveTo; + } else { + return _from - _to; + } + } + /** * @notice Transfer reward for the next period. Can be called only by distributor * @param _reward Amount of reward @@ -477,7 +540,7 @@ contract TACoApplication is uint96 _properAmount ) internal { if (_info.authorized != _properAmount) { - authorizedOverall -= _info.authorized - _properAmount; + authorizedOverall -= effectiveDifference(_info.authorized, _properAmount, _info); } } @@ -507,7 +570,7 @@ contract TACoApplication is if (info.operatorConfirmed) { resynchronizeAuthorizedOverall(info, _fromAmount); - authorizedOverall += _toAmount - _fromAmount; + authorizedOverall += effectiveDifference(_toAmount, _fromAmount, info); } info.authorized = _toAmount; @@ -529,7 +592,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; @@ -593,7 +656,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); @@ -619,7 +682,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); @@ -860,7 +923,7 @@ contract TACoApplication is } if (info.operatorConfirmed) { - authorizedOverall -= info.authorized; + authorizedOverall -= effectiveAuthorized(info.authorized, info); } // Bond new operator (or unbond if _operator == address(0)) @@ -891,7 +954,7 @@ contract TACoApplication is if (!info.operatorConfirmed) { updateRewardInternal(stakingProvider); info.operatorConfirmed = true; - authorizedOverall += info.authorized; + authorizedOverall += effectiveAuthorized(info.authorized, info); emit OperatorConfirmed(stakingProvider, _operator); } } @@ -957,4 +1020,32 @@ 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" + ); + StakingProviderInfo storage info = stakingProviderInfo[_stakingProvider]; + info.penaltyPercent = penaltyDefault; + info.endPenalty = uint64(block.timestamp + penaltyDuration); + emit Penalized(_stakingProvider, info.penaltyPercent, info.endPenalty); + } + + /** + * @notice Resets future reward back to 100% + * @param _stakingProvider Staking provider address + */ + function resetReward(address _stakingProvider) external updateReward(_stakingProvider) { + StakingProviderInfo storage info = stakingProviderInfo[_stakingProvider]; + require(info.endPenalty != 0, "There are no any penalties"); + require(info.endPenalty <= block.timestamp, "Penalty is still ongoing"); + info.endPenalty = 0; + info.penaltyPercent = 0; + emit RewardReset(_stakingProvider); + } } diff --git a/tests/application/conftest.py b/tests/application/conftest.py index bb993ecb..1f4a9453 100644 --- a/tests/application/conftest.py +++ b/tests/application/conftest.py @@ -31,6 +31,9 @@ COMMITMENT_DURATION_3 = 3 * COMMITMENT_DURATION_1 # 365 days in seconds COMMITMENT_DEADLINE = 60 * 60 * 24 * 100 # 100 days after deploymwent +PENALTY_DEFAULT = 1000 # 10% penalty +PENALTY_DURATION = 60 * 60 * 24 # 1 day in seconds + @pytest.fixture() def token(project, accounts): @@ -78,6 +81,8 @@ def taco_application(project, creator, token, threshold_staking, oz_dependency, DEAUTHORIZATION_DURATION, [COMMITMENT_DURATION_1, COMMITMENT_DURATION_2, COMMITMENT_DURATION_3], now + COMMITMENT_DEADLINE, + PENALTY_DEFAULT, + PENALTY_DURATION, ) encoded_initializer_function = encode_function_data() From c50a53a1b9b31a9a0da53d6f7a3d15212ead054e Mon Sep 17 00:00:00 2001 From: Victoria Zotova Date: Sun, 14 Apr 2024 13:31:25 -0400 Subject: [PATCH 2/5] TACoApplication: reset reward after penalty as soon as possible, proper calculation authorizedOverall --- contracts/contracts/TACoApplication.sol | 70 +++++++++--- contracts/test/TACoApplicationTestSet.sol | 4 + tests/application/test_authorization.py | 2 + tests/application/test_operator.py | 123 ++++++++++++++++++++++ tests/application/test_reward.py | 19 ++++ 5 files changed, 202 insertions(+), 16 deletions(-) diff --git a/contracts/contracts/TACoApplication.sol b/contracts/contracts/TACoApplication.sol index 2fa50c45..2fb18187 100644 --- a/contracts/contracts/TACoApplication.sol +++ b/contracts/contracts/TACoApplication.sol @@ -411,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 */ @@ -456,37 +477,36 @@ contract TACoApplication is return result.toUint96(); } - // FIXME function effectiveAuthorized( uint96 _authorized, uint192 _penaltyPercent - ) internal view returns (uint96) { + ) internal pure returns (uint96) { return uint96((_authorized * (PENALTY_BASE - _penaltyPercent)) / PENALTY_BASE); } + /// @dev This view should be called after updateReward modifier function effectiveAuthorized( uint96 _authorized, StakingProviderInfo storage _info ) internal view returns (uint96) { - if (_info.endPenalty != 0 && _info.endPenalty > block.timestamp) { - return effectiveAuthorized(_authorized, _info.penaltyPercent); - } else { + if (_info.endPenalty == 0) { return _info.authorized; } + return effectiveAuthorized(_authorized, _info.penaltyPercent); } + /// @dev This view should be called after updateReward modifier function effectiveDifference( uint96 _from, uint96 _to, StakingProviderInfo storage _info ) internal view returns (uint96) { - if (_info.endPenalty != 0 && _info.endPenalty > block.timestamp) { - uint96 effectiveFrom = effectiveAuthorized(_from, _info.penaltyPercent); - uint96 effectiveTo = effectiveAuthorized(_to, _info.penaltyPercent); - return effectiveFrom - effectiveTo; - } else { + if (_info.endPenalty == 0) { return _from - _to; } + uint96 effectiveFrom = effectiveAuthorized(_from, _info.penaltyPercent); + uint96 effectiveTo = effectiveAuthorized(_to, _info.penaltyPercent); + return effectiveFrom - effectiveTo; } /** @@ -789,6 +809,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 @@ -1030,9 +1061,18 @@ contract TACoApplication is msg.sender == address(childApplication), "Only child application allowed to penalize" ); + + if (_stakingProvider == address(0)) { + return; + } + StakingProviderInfo storage info = stakingProviderInfo[_stakingProvider]; - info.penaltyPercent = penaltyDefault; + uint96 before = effectiveAuthorized(info.authorized, info.penaltyPercent); info.endPenalty = uint64(block.timestamp + penaltyDuration); + info.penaltyPercent = penaltyDefault; + if (info.operatorConfirmed) { + authorizedOverall -= before - effectiveAuthorized(info.authorized, info.penaltyPercent); + } emit Penalized(_stakingProvider, info.penaltyPercent, info.endPenalty); } @@ -1040,12 +1080,10 @@ contract TACoApplication is * @notice Resets future reward back to 100% * @param _stakingProvider Staking provider address */ - function resetReward(address _stakingProvider) external updateReward(_stakingProvider) { + function resetReward(address _stakingProvider) external { StakingProviderInfo storage info = stakingProviderInfo[_stakingProvider]; require(info.endPenalty != 0, "There are no any penalties"); require(info.endPenalty <= block.timestamp, "Penalty is still ongoing"); - info.endPenalty = 0; - info.penaltyPercent = 0; - emit RewardReset(_stakingProvider); + updateRewardInternal(_stakingProvider); } } diff --git a/contracts/test/TACoApplicationTestSet.sol b/contracts/test/TACoApplicationTestSet.sol index 33e6dff5..017d10d3 100644 --- a/contracts/test/TACoApplicationTestSet.sol +++ b/contracts/test/TACoApplicationTestSet.sol @@ -172,4 +172,8 @@ contract ChildApplicationForTACoApplicationMock { function confirmOperatorAddress(address _operator) external { rootApplication.confirmOperatorAddress(_operator); } + + function penalize(address _stakingProvider) external { + rootApplication.penalize(_stakingProvider); + } } diff --git a/tests/application/test_authorization.py b/tests/application/test_authorization.py index 90922970..4f78e907 100644 --- a/tests/application/test_authorization.py +++ b/tests/application/test_authorization.py @@ -25,6 +25,8 @@ END_COMMITMENT_SLOT = 8 MIN_AUTHORIZATION = Web3.to_wei(40_000, "ether") DEAUTHORIZATION_DURATION = 60 * 60 * 24 * 60 # 60 days in seconds +PENALTY_DEFAULT = 1000 # 10% penalty +PENALTY_DURATION = 60 * 60 * 24 # 1 day in seconds def test_authorization_parameters(taco_application): diff --git a/tests/application/test_operator.py b/tests/application/test_operator.py index eb0b15bf..af9aee3a 100644 --- a/tests/application/test_operator.py +++ b/tests/application/test_operator.py @@ -22,6 +22,8 @@ CONFIRMATION_SLOT = 1 MIN_AUTHORIZATION = Web3.to_wei(40_000, "ether") MIN_OPERATOR_SECONDS = 24 * 60 * 60 +PENALTY_DEFAULT = 1000 # 10% penalty +PENALTY_DURATION = 60 * 60 * 24 # 1 day in seconds def test_bond_operator(accounts, threshold_staking, taco_application, child_application, chain): @@ -356,6 +358,10 @@ def test_confirm_address( min_authorization = MIN_AUTHORIZATION min_operator_seconds = MIN_OPERATOR_SECONDS + # Only child app can penalize + with ape.reverts("Only child application allowed to confirm operator"): + taco_application.confirmOperatorAddress(staking_provider, sender=staking_provider) + # Skips confirmation if operator is not associated with staking provider child_application.confirmOperatorAddress(staking_provider, sender=staking_provider) assert not taco_application.isOperatorConfirmed(staking_provider) @@ -398,3 +404,120 @@ def test_slash(accounts, threshold_staking, taco_application): assert threshold_staking.notifier() == investigator assert threshold_staking.stakingProvidersToSeize(0) == staking_provider assert threshold_staking.getLengthOfStakingProvidersToSeize() == 1 + + +def test_penalize(accounts, threshold_staking, taco_application, child_application, chain): + creator, staking_provider, *everyone_else = accounts[0:] + min_authorization = MIN_AUTHORIZATION + + # Only child app can penalize + with ape.reverts("Only child application allowed to penalize"): + taco_application.penalize(staking_provider, sender=creator) + + # Skips penalty if staking provider was not specified + child_application.penalize(ZERO_ADDRESS, sender=staking_provider) + assert taco_application.getPenalty(staking_provider) == [0, 0] + + # Penalize staking provider with 0 authorization + tx = child_application.penalize(staking_provider, sender=staking_provider) + timestamp = tx.timestamp + end_of_penalty = timestamp + PENALTY_DURATION + assert taco_application.getPenalty(staking_provider) == [PENALTY_DEFAULT, end_of_penalty] + assert tx.events == [ + taco_application.Penalized( + stakingProvider=staking_provider, + penaltyPercent=PENALTY_DEFAULT, + endPenalty=end_of_penalty, + ) + ] + assert taco_application.authorizedOverall() == 0 + + # Increase authorization with no confirmation and check penalty + chain.pending_timestamp += PENALTY_DURATION + threshold_staking.authorizationIncreased(staking_provider, 0, min_authorization, sender=creator) + tx = child_application.penalize(staking_provider, sender=staking_provider) + timestamp = tx.timestamp + end_of_penalty = timestamp + PENALTY_DURATION + assert taco_application.getPenalty(staking_provider) == [PENALTY_DEFAULT, end_of_penalty] + assert taco_application.authorizedOverall() == 0 + assert tx.events == [ + taco_application.Penalized( + stakingProvider=staking_provider, + penaltyPercent=PENALTY_DEFAULT, + endPenalty=end_of_penalty, + ) + ] + + # Increase authorization with confirmation and check penalty + chain.pending_timestamp += PENALTY_DURATION + taco_application.bondOperator(staking_provider, staking_provider, sender=staking_provider) + child_application.confirmOperatorAddress(staking_provider, sender=staking_provider) + assert taco_application.authorizedOverall() == min_authorization + tx = child_application.penalize(staking_provider, sender=staking_provider) + timestamp = tx.timestamp + end_of_penalty = timestamp + PENALTY_DURATION + assert taco_application.getPenalty(staking_provider) == [PENALTY_DEFAULT, end_of_penalty] + assert taco_application.authorizedOverall() == min_authorization * 9 / 10 + assert tx.events == [ + taco_application.Penalized( + stakingProvider=staking_provider, + penaltyPercent=PENALTY_DEFAULT, + endPenalty=end_of_penalty, + ) + ] + + # Penalize again + tx = child_application.penalize(staking_provider, sender=staking_provider) + timestamp = tx.timestamp + end_of_penalty = timestamp + PENALTY_DURATION + assert taco_application.getPenalty(staking_provider) == [PENALTY_DEFAULT, end_of_penalty] + assert taco_application.authorizedOverall() == min_authorization * 9 / 10 + assert tx.events == [ + taco_application.Penalized( + stakingProvider=staking_provider, + penaltyPercent=PENALTY_DEFAULT, + endPenalty=end_of_penalty, + ) + ] + + +def test_reset_reward(accounts, threshold_staking, taco_application, child_application, chain): + creator, staking_provider, *everyone_else = accounts[0:] + min_authorization = MIN_AUTHORIZATION + + # This method only for penalized staking providers + with ape.reverts("There are no any penalties"): + taco_application.resetReward(staking_provider, sender=creator) + + # Penalize staking provider + child_application.penalize(staking_provider, sender=staking_provider) + + # Not enough time passed + with ape.reverts("Penalty is still ongoing"): + taco_application.resetReward(staking_provider, sender=creator) + + chain.pending_timestamp += PENALTY_DURATION + tx = taco_application.resetReward(staking_provider, sender=creator) + assert taco_application.getPenalty(staking_provider) == [0, 0] + assert taco_application.authorizedOverall() == 0 + assert tx.events == [taco_application.RewardReset(stakingProvider=staking_provider)] + + # Increase authorization with no confirmation and reset reward + threshold_staking.authorizationIncreased(staking_provider, 0, min_authorization, sender=creator) + child_application.penalize(staking_provider, sender=staking_provider) + chain.pending_timestamp += PENALTY_DURATION + tx = taco_application.resetReward(staking_provider, sender=creator) + assert taco_application.getPenalty(staking_provider) == [0, 0] + assert taco_application.authorizedOverall() == 0 + assert tx.events == [taco_application.RewardReset(stakingProvider=staking_provider)] + + # Increase authorization with confirmation and reset reward + taco_application.bondOperator(staking_provider, staking_provider, sender=staking_provider) + child_application.confirmOperatorAddress(staking_provider, sender=staking_provider) + child_application.penalize(staking_provider, sender=staking_provider) + chain.pending_timestamp += PENALTY_DURATION + assert taco_application.authorizedOverall() == min_authorization * 9 / 10 + tx = taco_application.resetReward(staking_provider, sender=creator) + assert taco_application.getPenalty(staking_provider) == [0, 0] + assert taco_application.authorizedOverall() == min_authorization + assert tx.events == [taco_application.RewardReset(stakingProvider=staking_provider)] diff --git a/tests/application/test_reward.py b/tests/application/test_reward.py index 69c3630a..1dc1b370 100644 --- a/tests/application/test_reward.py +++ b/tests/application/test_reward.py @@ -28,6 +28,7 @@ DEAUTHORIZATION_DURATION = 60 * 60 * 24 * 60 # 60 days in seconds FLOATING_POINT_DIVISOR = 10**21 REWARD_PORTION = MIN_AUTHORIZATION * 10**3 +PENALTY_DURATION = 60 * 60 * 24 # 1 day in seconds def test_push_reward( @@ -248,6 +249,15 @@ def check_reward_with_confirmation(): taco_application.resynchronizeAuthorization(staking_provider_2, sender=creator) check_reward_no_confirmation() + # Penalize staking provider, no confirmed operator + child_application.penalize(staking_provider_2, sender=creator) + check_reward_no_confirmation() + + # Reset reward after penalty, no confirmed operator + chain.pending_timestamp += PENALTY_DURATION + taco_application.resetReward(staking_provider_2, sender=creator) + check_reward_no_confirmation() + # Wait and confirm operator taco_application.pushReward(reward_portion, sender=distributor) chain.pending_timestamp += reward_duration // 2 @@ -289,6 +299,15 @@ def check_reward_with_confirmation(): taco_application.resynchronizeAuthorization(staking_provider_2, sender=creator) check_reward_with_confirmation() + # Penalize staking provider with confirmation + child_application.penalize(staking_provider_2, sender=creator) + check_reward_with_confirmation() + + # Reset reward after penalty, with confirmation + chain.pending_timestamp += PENALTY_DURATION + taco_application.resetReward(staking_provider_2, sender=creator) + check_reward_with_confirmation() + # Bond operator with confirmation (confirmation will be dropped) taco_application.pushReward(reward_portion, sender=distributor) chain.pending_timestamp += min_operator_seconds From a6211ac4ef59067b89a06b270c49aade7a8f90d2 Mon Sep 17 00:00:00 2001 From: Victoria Zotova Date: Thu, 18 Apr 2024 15:08:40 -0400 Subject: [PATCH 3/5] TACoApplication: tests for penalty --- tests/application/conftest.py | 2 +- tests/application/test_authorization.py | 173 +++++++++++++++++++++++- tests/application/test_operator.py | 69 ++++++++-- 3 files changed, 229 insertions(+), 15 deletions(-) diff --git a/tests/application/conftest.py b/tests/application/conftest.py index 1f4a9453..26c70f9e 100644 --- a/tests/application/conftest.py +++ b/tests/application/conftest.py @@ -29,7 +29,7 @@ COMMITMENT_DURATION_1 = 182 * 60 * 24 * 60 # 182 days in seconds COMMITMENT_DURATION_2 = 2 * COMMITMENT_DURATION_1 # 365 days in seconds COMMITMENT_DURATION_3 = 3 * COMMITMENT_DURATION_1 # 365 days in seconds -COMMITMENT_DEADLINE = 60 * 60 * 24 * 100 # 100 days after deploymwent +COMMITMENT_DEADLINE = 60 * 60 * 24 * 200 # 200 days after deploymwent PENALTY_DEFAULT = 1000 # 10% penalty PENALTY_DURATION = 60 * 60 * 24 # 1 day in seconds diff --git a/tests/application/test_authorization.py b/tests/application/test_authorization.py index 4f78e907..7f6d19b5 100644 --- a/tests/application/test_authorization.py +++ b/tests/application/test_authorization.py @@ -36,7 +36,9 @@ def test_authorization_parameters(taco_application): assert parameters[2] == DEAUTHORIZATION_DURATION -def test_authorization_increase(accounts, threshold_staking, taco_application, child_application): +def test_authorization_increase( + accounts, threshold_staking, taco_application, child_application, chain +): """ Tests for authorization method: authorizationIncreased """ @@ -103,6 +105,25 @@ def test_authorization_increase(accounts, threshold_staking, taco_application, c ) ] + # Increase authorization for staker with penalty (no confirmation) + child_application.penalize(staking_provider, sender=staking_provider) + tx = threshold_staking.authorizationIncreased( + staking_provider, value // 4, 2 * value, sender=creator + ) + assert taco_application.stakingProviderInfo(staking_provider)[AUTHORIZATION_SLOT] == 2 * value + assert taco_application.authorizedOverall() == 0 + assert taco_application.authorizedStake(staking_provider) == 2 * value + assert child_application.stakingProviderInfo(staking_provider) == (2 * value, 0, 0) + assert taco_application.isAuthorized(staking_provider) + + events = taco_application.AuthorizationIncreased.from_receipt(tx) + assert events == [ + taco_application.AuthorizationIncreased( + stakingProvider=staking_provider, fromAmount=value // 4, toAmount=2 * value + ) + ] + chain.pending_timestamp += PENALTY_DURATION + # Confirm operator address and try to increase authorization again taco_application.bondOperator(staking_provider, staking_provider, sender=staking_provider) child_application.confirmOperatorAddress(staking_provider, sender=staking_provider) @@ -126,6 +147,28 @@ def test_authorization_increase(accounts, threshold_staking, taco_application, c ) ] + # Increase authorization for staker with penalty + authorization = 3 * value + child_application.penalize(staking_provider, sender=staking_provider) + tx = threshold_staking.authorizationIncreased( + staking_provider, 2 * value + 1, authorization, sender=creator + ) + assert ( + taco_application.stakingProviderInfo(staking_provider)[AUTHORIZATION_SLOT] == authorization + ) + assert taco_application.authorizedOverall() == authorization * 9 // 10 + assert taco_application.authorizedStake(staking_provider) == authorization + assert child_application.stakingProviderInfo(staking_provider) == (authorization, 0, 0) + assert taco_application.isAuthorized(staking_provider) + + events = taco_application.AuthorizationIncreased.from_receipt(tx) + assert events == [ + taco_application.AuthorizationIncreased( + stakingProvider=staking_provider, fromAmount=2 * value + 1, toAmount=authorization + ) + ] + chain.pending_timestamp += PENALTY_DURATION + # Emulate slash and desync by sending smaller fromAmount tx = threshold_staking.authorizationIncreased( staking_provider, value // 2, value, sender=creator @@ -143,6 +186,27 @@ def test_authorization_increase(accounts, threshold_staking, taco_application, c ) ] + # Desync again for staker with penalty + tx = threshold_staking.authorizationIncreased( + staking_provider, value, authorization, sender=creator + ) + child_application.penalize(staking_provider, sender=staking_provider) + tx = threshold_staking.authorizationIncreased( + staking_provider, value // 2, value, sender=creator + ) + assert taco_application.stakingProviderInfo(staking_provider)[AUTHORIZATION_SLOT] == value + assert taco_application.authorizedOverall() == value * 9 // 10 + assert taco_application.authorizedStake(staking_provider) == value + assert child_application.stakingProviderInfo(staking_provider) == (value, 0, 0) + assert taco_application.isAuthorized(staking_provider) + + events = taco_application.AuthorizationIncreased.from_receipt(tx) + assert events == [ + taco_application.AuthorizationIncreased( + stakingProvider=staking_provider, fromAmount=value // 2, toAmount=value + ) + ] + def test_involuntary_authorization_decrease( accounts, threshold_staking, taco_application, child_application, chain @@ -187,6 +251,17 @@ def test_involuntary_authorization_decrease( ) ] + # Decrease again for staker with penalty + child_application.penalize(staking_provider, sender=staking_provider) + threshold_staking.authorizationIncreased(staking_provider, authorization, value, sender=creator) + threshold_staking.involuntaryAuthorizationDecrease( + staking_provider, value, authorization, sender=creator + ) + assert taco_application.authorizedOverall() == 0 + assert taco_application.authorizedStake(staking_provider) == authorization + assert child_application.stakingProviderInfo(staking_provider) == (authorization, 0, 0) + chain.pending_timestamp += PENALTY_DURATION + # Prepare request to decrease before involuntary decrease threshold_staking.authorizationDecreaseRequested( staking_provider, value // 2, 0, sender=creator @@ -254,6 +329,25 @@ def test_involuntary_authorization_decrease( ) ] + # Decrease again for staker with penalty + child_application.penalize(staking_provider, sender=staking_provider) + threshold_staking.authorizationIncreased(staking_provider, authorization, value, sender=creator) + threshold_staking.involuntaryAuthorizationDecrease( + staking_provider, value, authorization, sender=creator + ) + assert ( + taco_application.stakingProviderInfo(staking_provider)[AUTHORIZATION_SLOT] == authorization + ) + assert taco_application.pendingAuthorizationDecrease(staking_provider) == authorization + assert taco_application.authorizedOverall() == authorization * 9 // 10 + assert taco_application.authorizedStake(staking_provider) == authorization + assert child_application.stakingProviderInfo(staking_provider) == ( + authorization, + authorization, + end_deauthorization, + ) + chain.pending_timestamp += PENALTY_DURATION + # Decrease everything tx = threshold_staking.involuntaryAuthorizationDecrease( staking_provider, authorization, 0, sender=creator @@ -302,6 +396,22 @@ def test_involuntary_authorization_decrease( ) ] + # Another desync for staker with penalty + child_application.penalize(staking_provider, sender=staking_provider) + threshold_staking.authorizationIncreased( + staking_provider, authorization, 2 * value, sender=creator + ) + threshold_staking.involuntaryAuthorizationDecrease( + staking_provider, value, value // 2, sender=creator + ) + assert ( + taco_application.stakingProviderInfo(staking_provider)[AUTHORIZATION_SLOT] == authorization + ) + assert taco_application.authorizedOverall() == authorization * 9 // 10 + assert taco_application.authorizedStake(staking_provider) == authorization + assert child_application.stakingProviderInfo(staking_provider) == (authorization, 0, 0) + chain.pending_timestamp += PENALTY_DURATION + # Decrease everything again with previous commitment commitment_duration = taco_application.commitmentDurationOption1() taco_application.makeCommitment(staking_provider, commitment_duration, sender=staking_provider) @@ -439,6 +549,25 @@ def test_authorization_decrease_request( ) ] + # Emulate desync for staker with penalty + chain.pending_timestamp += deauthorization_duration + child_application.penalize(staking_provider, sender=staking_provider) + assert taco_application.authorizedOverall() == value // 2 * 9 // 10 + taco_application.approveAuthorizationDecrease(staking_provider, sender=creator) + assert taco_application.authorizedOverall() == 0 + threshold_staking.authorizationIncreased(staking_provider, 0, value, sender=creator) + assert taco_application.authorizedOverall() == 0 + taco_application.bondOperator(staking_provider, staking_provider, sender=staking_provider) + child_application.confirmOperatorAddress(staking_provider, sender=staking_provider) + assert taco_application.authorizedOverall() == value * 9 // 10 + threshold_staking.authorizationDecreaseRequested( + staking_provider, value // 2, 0, sender=creator + ) + assert taco_application.stakingProviderInfo(staking_provider)[AUTHORIZATION_SLOT] == value // 2 + assert taco_application.pendingAuthorizationDecrease(staking_provider) == value // 2 + assert taco_application.authorizedOverall() == value // 2 * 9 // 10 + chain.pending_timestamp += PENALTY_DURATION + # Try to request decrease before ending of commitment chain.pending_timestamp += deauthorization_duration taco_application.approveAuthorizationDecrease(staking_provider, sender=creator) @@ -491,12 +620,23 @@ def test_finish_authorization_decrease( assert taco_application.stakingProviderInfo(staking_provider)[AUTHORIZATION_SLOT] == new_value assert taco_application.pendingAuthorizationDecrease(staking_provider) == 0 assert taco_application.stakingProviderInfo(staking_provider)[END_DEAUTHORIZATION_SLOT] == 0 + assert taco_application.authorizedOverall() == 0 assert tx.events == [ taco_application.AuthorizationDecreaseApproved( stakingProvider=staking_provider, fromAmount=value, toAmount=new_value ) ] + # Try again with penalty + child_application.penalize(staking_provider, sender=staking_provider) + threshold_staking.authorizationIncreased(staking_provider, new_value, value, sender=creator) + threshold_staking.authorizationDecreaseRequested( + staking_provider, value, new_value, sender=creator + ) + taco_application.approveAuthorizationDecrease(staking_provider, sender=creator) + assert taco_application.authorizedOverall() == 0 + chain.pending_timestamp += PENALTY_DURATION + # Bond operator and request decrease again taco_application.bondOperator(staking_provider, staking_provider, sender=staking_provider) threshold_staking.authorizationIncreased(staking_provider, new_value, value, sender=creator) @@ -572,6 +712,19 @@ def test_finish_authorization_decrease( ) ] + # Decrease again for staker with penalty + threshold_staking.authorizationIncreased(staking_provider, new_value, value, sender=creator) + threshold_staking.authorizationDecreaseRequested( + staking_provider, value, minimum_authorization, sender=creator + ) + threshold_staking.setDecreaseRequest(staking_provider, new_value, sender=creator) + chain.pending_timestamp += deauthorization_duration + child_application.penalize(staking_provider, sender=staking_provider) + taco_application.approveAuthorizationDecrease(staking_provider, sender=creator) + assert taco_application.stakingProviderInfo(staking_provider)[AUTHORIZATION_SLOT] == new_value + assert taco_application.authorizedOverall() == new_value * 9 // 10 + chain.pending_timestamp += PENALTY_DURATION + # Decrease everything value = new_value threshold_staking.authorizationDecreaseRequested(staking_provider, value, 0, sender=creator) @@ -656,6 +809,15 @@ def test_resync(accounts, threshold_staking, taco_application, child_application ) ] + # Resync again for staker with penalty + new_value = 3 * minimum_authorization // 2 + child_application.penalize(staking_provider, sender=staking_provider) + threshold_staking.setAuthorized(staking_provider, new_value, sender=creator) + taco_application.resynchronizeAuthorization(staking_provider, sender=creator) + assert taco_application.authorizedOverall() == 0 + assert taco_application.authorizedStake(staking_provider) == new_value + chain.pending_timestamp += PENALTY_DURATION + # Confirm operator and change authorized amount again value = new_value taco_application.bondOperator(staking_provider, staking_provider, sender=staking_provider) @@ -684,6 +846,15 @@ def test_resync(accounts, threshold_staking, taco_application, child_application ) ] + # Resync again for staker with penalty + new_value = 3 * minimum_authorization // 4 + child_application.penalize(staking_provider, sender=staking_provider) + threshold_staking.setAuthorized(staking_provider, new_value, sender=creator) + taco_application.resynchronizeAuthorization(staking_provider, sender=creator) + assert taco_application.authorizedOverall() == new_value * 9 // 10 + assert taco_application.authorizedStake(staking_provider) == new_value + chain.pending_timestamp += PENALTY_DURATION + # Request decrease and change authorized amount again value = new_value threshold_staking.authorizationDecreaseRequested(staking_provider, value, 0, sender=creator) diff --git a/tests/application/test_operator.py b/tests/application/test_operator.py index af9aee3a..53a4f38c 100644 --- a/tests/application/test_operator.py +++ b/tests/application/test_operator.py @@ -102,6 +102,17 @@ def test_bond_operator(accounts, threshold_staking, taco_application, child_appl assert taco_application.stakingProviders(0) == staking_provider_3 assert child_application.stakingProviderToOperator(staking_provider_3) == operator1 assert child_application.operatorToStakingProvider(operator1) == staking_provider_3 + assert taco_application.authorizedOverall() == 0 + + events = taco_application.OperatorBonded.from_receipt(tx) + assert events == [ + taco_application.OperatorBonded( + stakingProvider=staking_provider_3, + operator=operator1, + previousOperator=ZERO_ADDRESS, + startTimestamp=timestamp, + ) + ] # No active stakingProviders before confirmation all_locked, staking_providers = taco_application.getActiveStakingProviders(0, 0, 0) @@ -114,16 +125,6 @@ def test_bond_operator(accounts, threshold_staking, taco_application, child_appl assert child_application.stakingProviderToOperator(staking_provider_3) == operator1 assert child_application.operatorToStakingProvider(operator1) == staking_provider_3 - events = taco_application.OperatorBonded.from_receipt(tx) - assert events == [ - taco_application.OperatorBonded( - stakingProvider=staking_provider_3, - operator=operator1, - previousOperator=ZERO_ADDRESS, - startTimestamp=timestamp, - ) - ] - # After confirmation operator is becoming active all_locked, staking_providers = taco_application.getActiveStakingProviders(0, 0, 0) assert all_locked == min_authorization @@ -350,10 +351,22 @@ def test_bond_operator(accounts, threshold_staking, taco_application, child_appl assert taco_application.operatorToStakingProvider(staking_provider_3) == ZERO_ADDRESS assert taco_application.operatorToStakingProvider(operator2) == ZERO_ADDRESS + # Rebond operator by staker with penalty + authorized_overall = taco_application.authorizedOverall() + child_application.penalize(staking_provider_3, sender=staking_provider_2) + taco_application.bondOperator(staking_provider_3, operator2, sender=owner3) + assert taco_application.authorizedOverall() == authorized_overall + + # Confirm operator and rebond again + chain.pending_timestamp += min_operator_seconds + child_application.penalize(staking_provider_3, sender=staking_provider_2) + child_application.confirmOperatorAddress(operator2, sender=operator2) + assert taco_application.authorizedOverall() == authorized_overall + min_authorization * 9 // 10 + taco_application.bondOperator(staking_provider_3, staking_provider_3, sender=staking_provider_3) + assert taco_application.authorizedOverall() == authorized_overall -def test_confirm_address( - accounts, threshold_staking, taco_application, child_application, chain, project -): + +def test_confirm_address(accounts, threshold_staking, taco_application, child_application, chain): creator, staking_provider, operator, *everyone_else = accounts[0:] min_authorization = MIN_AUTHORIZATION min_operator_seconds = MIN_OPERATOR_SECONDS @@ -391,6 +404,20 @@ def test_confirm_address( assert taco_application.authorizedOverall() == min_authorization assert taco_application.availableRewards(staking_provider) == earned + # Confirm again for staker with penalty + child_application.penalize(staking_provider, sender=staking_provider) + assert taco_application.authorizedOverall() == min_authorization * 9 // 10 + child_application.confirmOperatorAddress(operator, sender=operator) + assert taco_application.authorizedOverall() == min_authorization * 9 // 10 + + # Rebond and confirm again + chain.pending_timestamp += min_operator_seconds + child_application.penalize(staking_provider, sender=staking_provider) + taco_application.bondOperator(staking_provider, staking_provider, sender=staking_provider) + assert taco_application.authorizedOverall() == 0 + child_application.confirmOperatorAddress(staking_provider, sender=staking_provider) + assert taco_application.authorizedOverall() == min_authorization * 9 // 10 + def test_slash(accounts, threshold_staking, taco_application): creator, staking_provider, investigator, *everyone_else = accounts[0:] @@ -480,6 +507,22 @@ def test_penalize(accounts, threshold_staking, taco_application, child_applicati ) ] + # Penalize again after first penalty is over + chain.pending_timestamp += PENALTY_DURATION + tx = child_application.penalize(staking_provider, sender=staking_provider) + timestamp = tx.timestamp + end_of_penalty = timestamp + PENALTY_DURATION + assert taco_application.getPenalty(staking_provider) == [PENALTY_DEFAULT, end_of_penalty] + assert taco_application.authorizedOverall() == min_authorization * 9 / 10 + assert tx.events == [ + taco_application.RewardReset(stakingProvider=staking_provider), + taco_application.Penalized( + stakingProvider=staking_provider, + penaltyPercent=PENALTY_DEFAULT, + endPenalty=end_of_penalty, + ), + ] + def test_reset_reward(accounts, threshold_staking, taco_application, child_application, chain): creator, staking_provider, *everyone_else = accounts[0:] From 53cb1196d4f418b12efe84eb50ad1b301f2545b7 Mon Sep 17 00:00:00 2001 From: Victoria Zotova Date: Sat, 20 Apr 2024 19:50:13 -0400 Subject: [PATCH 4/5] TACoChildApplication: endpoint for sending penalty to the root app --- .../coordination/ITACoChildToRoot.sol | 2 ++ .../coordination/TACoChildApplication.sol | 27 ++++++++++++++++--- contracts/contracts/testnet/LynxSet.sol | 7 +++++ contracts/test/CoordinatorTestSet.sol | 3 +++ .../test/TACoChildApplicationTestSet.sol | 5 ++++ tests/test_child_application.py | 18 ++++++++++++- 6 files changed, 58 insertions(+), 4 deletions(-) diff --git a/contracts/contracts/coordination/ITACoChildToRoot.sol b/contracts/contracts/coordination/ITACoChildToRoot.sol index 2bb336dd..4bfb5e73 100644 --- a/contracts/contracts/coordination/ITACoChildToRoot.sol +++ b/contracts/contracts/coordination/ITACoChildToRoot.sol @@ -15,4 +15,6 @@ interface ITACoChildToRoot { event OperatorConfirmed(address indexed stakingProvider, address indexed operator); function confirmOperatorAddress(address operator) external; + + function penalize(address stakingProvider) external; } diff --git a/contracts/contracts/coordination/TACoChildApplication.sol b/contracts/contracts/coordination/TACoChildApplication.sol index a946452d..2d4f13c4 100644 --- a/contracts/contracts/coordination/TACoChildApplication.sol +++ b/contracts/contracts/coordination/TACoChildApplication.sol @@ -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; @@ -25,6 +31,7 @@ contract TACoChildApplication is ITACoRootToChild, ITACoChildApplication, Initia ITACoChildToRoot public immutable rootApplication; address public coordinator; + address public adjudicator; uint96 public immutable minimumAuthorization; @@ -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) { @@ -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 */ diff --git a/contracts/contracts/testnet/LynxSet.sol b/contracts/contracts/testnet/LynxSet.sol index 57f13029..c0ac099d 100644 --- a/contracts/contracts/testnet/LynxSet.sol +++ b/contracts/contracts/testnet/LynxSet.sol @@ -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 {} @@ -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") { diff --git a/contracts/test/CoordinatorTestSet.sol b/contracts/test/CoordinatorTestSet.sol index bbc21ff7..4d15b480 100644 --- a/contracts/test/CoordinatorTestSet.sol +++ b/contracts/test/CoordinatorTestSet.sol @@ -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 {} } // /** diff --git a/contracts/test/TACoChildApplicationTestSet.sol b/contracts/test/TACoChildApplicationTestSet.sol index 1d27b80a..05036907 100644 --- a/contracts/test/TACoChildApplicationTestSet.sol +++ b/contracts/test/TACoChildApplicationTestSet.sol @@ -12,6 +12,7 @@ contract RootApplicationForTACoChildApplicationMock { ITACoRootToChild public childApplication; mapping(address => bool) public confirmations; + mapping(address => bool) public penalties; function setChildApplication(ITACoRootToChild _childApplication) external { childApplication = _childApplication; @@ -43,6 +44,10 @@ contract RootApplicationForTACoChildApplicationMock { confirmations[_operator] = true; } + function penalize(address _stakingProvider) external { + penalties[_stakingProvider] = true; + } + function resetConfirmation(address _operator) external { confirmations[_operator] = false; } diff --git a/tests/test_child_application.py b/tests/test_child_application.py index 34c8645c..f5e52b20 100644 --- a/tests/test_child_application.py +++ b/tests/test_child_application.py @@ -56,7 +56,7 @@ def coordinator(project, child_application, creator): contract = project.CoordinatorForTACoChildApplicationMock.deploy( child_application, sender=creator ) - child_application.initialize(contract.address, sender=creator) + child_application.initialize(contract.address, creator, sender=creator) return contract @@ -321,3 +321,19 @@ def test_confirm_address(accounts, root_application, child_application, coordina all_locked, staking_providers = child_application.getActiveStakingProviders(0, 0) assert all_locked == 0 assert len(staking_providers) == 0 + + +def test_penalize(accounts, root_application, child_application, coordinator): + ( + creator, + staking_provider, + *everyone_else, + ) = accounts[0:] + + # Penalize can be done only from adjudicator address + with ape.reverts("Only adjudicator allowed to penalize"): + child_application.penalize(staking_provider, sender=staking_provider) + + tx = child_application.penalize(staking_provider, sender=creator) + assert root_application.penalties(staking_provider) + assert tx.events == [child_application.Penalized(stakingProvider=staking_provider)] From f0d7e243bc3912c4085eb326eee8a3c24d7ad79b Mon Sep 17 00:00:00 2001 From: Victoria Date: Mon, 22 Apr 2024 13:56:12 +0200 Subject: [PATCH 5/5] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Manuel Montenegro Co-authored-by: Derek Pierre Co-authored-by: David Núñez --- contracts/contracts/TACoApplication.sol | 19 +++++++++++++------ .../coordination/TACoChildApplication.sol | 2 +- tests/application/test_operator.py | 2 +- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/contracts/contracts/TACoApplication.sol b/contracts/contracts/TACoApplication.sol index 2fb18187..4959016b 100644 --- a/contracts/contracts/TACoApplication.sol +++ b/contracts/contracts/TACoApplication.sol @@ -173,7 +173,7 @@ contract TACoApplication is * @param penaltyPercent Percent of reward that was penalized * @param endPenalty End of penalty */ - event Penalized(address indexed stakingProvider, uint256 penaltyPercent, uint256 endPenalty); + event Penalized(address indexed stakingProvider, uint256 penaltyPercent, uint64 endPenalty); /** * @notice Signals that reward was reset after penalty @@ -242,7 +242,7 @@ 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 + * @param _penaltyDefault Default penalty percentage (as a value out of 10000) * @param _penaltyDuration Duration of penalty */ constructor( @@ -484,7 +484,9 @@ contract TACoApplication is return uint96((_authorized * (PENALTY_BASE - _penaltyPercent)) / PENALTY_BASE); } - /// @dev This view should be called after updateReward modifier + /// @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 @@ -495,7 +497,9 @@ contract TACoApplication is return effectiveAuthorized(_authorized, _info.penaltyPercent); } - /// @dev This view should be called after updateReward modifier + /// @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, @@ -1077,12 +1081,15 @@ contract TACoApplication is } /** - * @notice Resets future reward back to 100% + * @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 { StakingProviderInfo storage info = stakingProviderInfo[_stakingProvider]; - require(info.endPenalty != 0, "There are no any penalties"); + require(info.endPenalty != 0, "There is no penalty"); require(info.endPenalty <= block.timestamp, "Penalty is still ongoing"); updateRewardInternal(_stakingProvider); } diff --git a/contracts/contracts/coordination/TACoChildApplication.sol b/contracts/contracts/coordination/TACoChildApplication.sol index 2d4f13c4..e829ddd0 100644 --- a/contracts/contracts/coordination/TACoChildApplication.sol +++ b/contracts/contracts/coordination/TACoChildApplication.sol @@ -62,7 +62,7 @@ contract TACoChildApplication is ITACoRootToChild, ITACoChildApplication, Initia * @notice Initialize function for using with OpenZeppelin proxy */ 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 already set"); require( _coordinator != address(0) && _adjudicator != address(0), "Contracts must be specified" diff --git a/tests/application/test_operator.py b/tests/application/test_operator.py index 53a4f38c..d5b9d250 100644 --- a/tests/application/test_operator.py +++ b/tests/application/test_operator.py @@ -529,7 +529,7 @@ def test_reset_reward(accounts, threshold_staking, taco_application, child_appli min_authorization = MIN_AUTHORIZATION # This method only for penalized staking providers - with ape.reverts("There are no any penalties"): + with ape.reverts("There is no penalty"): taco_application.resetReward(staking_provider, sender=creator) # Penalize staking provider