From 73245494998dafc9ae1a2a41df9887146a2df76f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Thu, 26 Feb 2026 09:30:53 +0100 Subject: [PATCH 01/17] Draft tests for PenaltyBoard with compensation --- tests/test_penalty_board_compensation.py | 181 +++++++++++++++++++++++ 1 file changed, 181 insertions(+) create mode 100644 tests/test_penalty_board_compensation.py diff --git a/tests/test_penalty_board_compensation.py b/tests/test_penalty_board_compensation.py new file mode 100644 index 000000000..75325d53c --- /dev/null +++ b/tests/test_penalty_board_compensation.py @@ -0,0 +1,181 @@ +"""TDD tests for PenaltyBoard compensation. Some pass with stubs; others fail until C3 implementation.""" + +import ape +import pytest + +PERIOD_DURATION = 3600 +FIXED_COMPENSATION = 1000 +TOKEN_SUPPLY = 1_000_000 * 10**18 + + +@pytest.fixture(scope="module") +def deployer(accounts): + return accounts[0] + + +@pytest.fixture(scope="module") +def informer(accounts): + return accounts[1] + + +@pytest.fixture(scope="module") +def fund_holder(accounts): + return accounts[2] + + +@pytest.fixture(scope="module") +def staking_provider(accounts): + return accounts[3] + + +@pytest.fixture(scope="module") +def beneficiary(accounts): + return accounts[4] + + +@pytest.fixture(scope="module") +def other_account(accounts): + return accounts[5] + + +@pytest.fixture(scope="module") +def mock_taco(project, deployer): + return project.MockTACoForPenaltyBoard.deploy(sender=deployer) + + +@pytest.fixture(scope="module") +def token(project, deployer): + return project.TestToken.deploy(TOKEN_SUPPLY, sender=deployer) + + +@pytest.fixture +def penalty_board_comp(project, deployer, informer, chain, mock_taco, token, fund_holder): + """PenaltyBoard with compensation enabled (7-arg constructor).""" + genesis_time = chain.pending_timestamp + contract = project.PenaltyBoard.deploy( + genesis_time, + PERIOD_DURATION, + deployer.address, + mock_taco.address, + token.address, + FIXED_COMPENSATION, + fund_holder.address, + sender=deployer, + ) + contract.grantRole(contract.INFORMER_ROLE(), informer.address, sender=deployer) + return contract + + +@pytest.fixture +def registered_staker(mock_taco, staking_provider, deployer, beneficiary): + """Register staking_provider in mock TACo: owner=deployer, beneficiary=beneficiary, not stakeless.""" + mock_taco.setRoles( + staking_provider.address, + deployer.address, + beneficiary.address, + False, + sender=deployer, + ) + + +def test_penalized_periods_by_staker_updated( + penalty_board_comp, informer, staking_provider +): + """setPenalizedProvidersForPeriod(provs, period) causes getPenalizedPeriodsForStaker(prov) to include period.""" + current = penalty_board_comp.getCurrentPeriod() + provs = [staking_provider.address] + penalty_board_comp.setPenalizedProvidersForPeriod(provs, current, sender=informer) + periods = penalty_board_comp.getPenalizedPeriodsForStaker(staking_provider.address) + assert len(periods) == 1 + assert periods[0] == current + + +def test_penalized_periods_monotonic_append( + penalty_board_comp, informer, staking_provider, chain +): + """Calling setPenalizedProvidersForPeriod for different periods appends to each staker's list (monotonic).""" + current = penalty_board_comp.getCurrentPeriod() + provs = [staking_provider.address] + penalty_board_comp.setPenalizedProvidersForPeriod(provs, current, sender=informer) + chain.pending_timestamp += PERIOD_DURATION + assert penalty_board_comp.getCurrentPeriod() == current + 1 + penalty_board_comp.setPenalizedProvidersForPeriod(provs, current + 1, sender=informer) + periods = penalty_board_comp.getPenalizedPeriodsForStaker(staking_provider.address) + assert len(periods) == 2 + assert periods[0] == current + assert periods[1] == current + 1 + + +def test_withdraw_reverts_until_implemented( + penalty_board_comp, registered_staker, beneficiary, staking_provider +): + """Withdraw reverts with 'Not implemented' until C3 (stub).""" + with ape.reverts("Not implemented"): + penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) + + +def test_get_accrued_balance_zero_without_accrual(penalty_board_comp, staking_provider): + """getAccruedBalance returns 0 before accrual is implemented (stub).""" + assert penalty_board_comp.getAccruedBalance(staking_provider.address) == 0 + + +def test_accrual_with_no_penalties_gives_positive_balance( + penalty_board_comp, chain, staking_provider, registered_staker +): + """After advancing periods with no penalties, staker should have positive accrued balance (fails until C3).""" + # Advance 2 periods so there is something to accrue + chain.pending_timestamp += 2 * PERIOD_DURATION + balance = penalty_board_comp.getAccruedBalance(staking_provider.address) + assert balance > 0, "Accrual not implemented: getAccruedBalance should be > 0 after 2 periods with no penalties" + + +def test_only_owner_provider_beneficiary_can_withdraw( + penalty_board_comp, registered_staker, other_account, staking_provider +): + """Only owner, staking provider, or beneficiary may call withdraw; others revert (C3: Unauthorized; stub: Not implemented).""" + with ape.reverts(): + penalty_board_comp.withdraw(staking_provider.address, sender=other_account) + + +def test_withdraw_sends_tokens_to_beneficiary( + penalty_board_comp, + token, + fund_holder, + beneficiary, + staking_provider, + registered_staker, + chain, + deployer, +): + """Withdraw sends tokens to beneficiary, not to msg.sender (fails until C3 implements transfer).""" + # Fund the fundHolder and approve PenaltyBoard (token was minted to deployer) + token.transfer(fund_holder.address, FIXED_COMPENSATION * 10, sender=deployer) + token.approve( + penalty_board_comp.address, + 2**256 - 1, + sender=fund_holder, + ) + # Advance so there is accrual (when implemented) + chain.pending_timestamp += PERIOD_DURATION + before = token.balanceOf(beneficiary.address) + # Beneficiary calls withdraw for staking_provider + penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) + after = token.balanceOf(beneficiary.address) + assert after > before, "Withdraw should send tokens to beneficiary" + + +def test_stakeless_staker_gets_zero_compensation( + penalty_board_comp, mock_taco, chain, deployer, beneficiary, accounts +): + """Stakeless staker accrues 0 (fails until C3 implements stakeless check).""" + stakeless_provider = accounts[6] + mock_taco.setRoles( + stakeless_provider.address, + deployer.address, + beneficiary.address, + True, # stakeless + sender=deployer, + ) + chain.pending_timestamp += 2 * PERIOD_DURATION + balance = penalty_board_comp.getAccruedBalance(stakeless_provider.address) + assert balance == 0, "Stakeless staker should have 0 accrued compensation" From 08e821efd02a613ae5f05081344e6754837d7093 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Thu, 26 Feb 2026 09:31:19 +0100 Subject: [PATCH 02/17] Mock TACoApplication to continue development Latest TACoApplication, with required API changes, lives on a parallel universe. Waiting for rebase. --- .../ITACoApplicationForPenaltyBoard.sol | 29 ++++++++++++ contracts/test/MockTACoForPenaltyBoard.sol | 45 +++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol create mode 100644 contracts/test/MockTACoForPenaltyBoard.sol diff --git a/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol b/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol new file mode 100644 index 000000000..dff103be7 --- /dev/null +++ b/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +pragma solidity ^0.8.0; + +/** + * @title ITACoApplicationForPenaltyBoard + * @notice Minimal view interface that PenaltyBoard (with compensation) needs from TACoApplication. + * Used for withdrawal auth (owner, beneficiary), payout address (beneficiary), and stakeless check. + */ +interface ITACoApplicationForPenaltyBoard { + /** + * @notice Returns beneficiary for a staking provider (tokens are sent here on withdraw). + * Assumed never zero for a registered staking provider. + */ + function getBeneficiary(address stakingProvider) external view returns (address payable beneficiary); + + /** + * @notice Returns owner and beneficiary for a staking provider. + * Withdraw(stakingProvider) may be called by stakingProvider, owner, or beneficiary. + * If TACoApplication does not expose this, PenaltyBoard may depend on IStaking.rolesOf instead. + */ + function getRoles(address stakingProvider) external view returns (address owner, address beneficiary); + + /** + * @notice Returns true if the staking provider is stakeless (compensation = 0). + * If not present on TACoApplication, implementation may return false. + */ + function isStakeless(address stakingProvider) external view returns (bool); +} diff --git a/contracts/test/MockTACoForPenaltyBoard.sol b/contracts/test/MockTACoForPenaltyBoard.sol new file mode 100644 index 000000000..203a1289f --- /dev/null +++ b/contracts/test/MockTACoForPenaltyBoard.sol @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +pragma solidity ^0.8.0; + +import "../contracts/coordination/ITACoApplicationForPenaltyBoard.sol"; + +/** + * @notice Mock TACoApplication for PenaltyBoard compensation tests. + * Allows tests to set owner, beneficiary, and isStakeless per staking provider. + */ +contract MockTACoForPenaltyBoard is ITACoApplicationForPenaltyBoard { + struct StakerRoles { + address owner; + address payable beneficiary; + bool isStakeless; + } + + mapping(address stakingProvider => StakerRoles) private _roles; + + function setRoles( + address stakingProvider, + address owner, + address payable beneficiary, + bool isStakeless + ) external { + _roles[stakingProvider] = StakerRoles({ + owner: owner, + beneficiary: beneficiary, + isStakeless: isStakeless + }); + } + + function getBeneficiary(address stakingProvider) external view returns (address payable) { + return _roles[stakingProvider].beneficiary; + } + + function getRoles(address stakingProvider) external view returns (address owner, address beneficiary) { + StakerRoles storage r = _roles[stakingProvider]; + return (r.owner, r.beneficiary); + } + + function isStakeless(address stakingProvider) external view returns (bool) { + return _roles[stakingProvider].isStakeless; + } +} From e6250a382396d3d9eee1a6934475f74cfa02c910 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Thu, 26 Feb 2026 09:39:37 +0100 Subject: [PATCH 03/17] Tweak existing tests to new PenaltyBoard constructor --- tests/test_infraction.py | 11 +++++++++-- tests/test_penalty_board.py | 17 +++++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/tests/test_infraction.py b/tests/test_infraction.py index 8e5e69835..17f3cee69 100644 --- a/tests/test_infraction.py +++ b/tests/test_infraction.py @@ -135,12 +135,17 @@ def infraction_collector(project, deployer, coordinator): @pytest.fixture def penalty_board(project, deployer, informer, chain): """PenaltyBoard with genesis at current chain time and INFORMER_ROLE granted to informer. - Period duration is one week so ritual timeout still lands in period 0.""" + Period duration is one week so ritual timeout still lands in period 0. No compensation.""" genesis_time = chain.pending_timestamp + zero = "0x0000000000000000000000000000000000000000" contract = project.PenaltyBoard.deploy( genesis_time, PENALTY_BOARD_PERIOD_DURATION, deployer.address, + zero, + zero, + 0, + zero, sender=deployer, ) contract.grantRole(contract.INFORMER_ROLE(), informer.address, sender=deployer) @@ -273,4 +278,6 @@ def test_infraction_collector_and_penalty_board_together( penalty_board.setPenalizedProvidersForPeriod( failing_providers, current_period, sender=informer ) - assert penalty_board.getPenalizedProvidersForPeriod(current_period) == failing_providers + for provider in failing_providers: + periods = penalty_board.getPenalizedPeriodsByStaker(provider) + assert periods == [current_period] diff --git a/tests/test_penalty_board.py b/tests/test_penalty_board.py index 0564c30fa..5085f1379 100644 --- a/tests/test_penalty_board.py +++ b/tests/test_penalty_board.py @@ -21,14 +21,22 @@ def other_account(accounts): return accounts[2] +def _zero(): + return "0x0000000000000000000000000000000000000000" + + @pytest.fixture def penalty_board(project, deployer, informer, chain): - """PenaltyBoard with genesis at current chain time and INFORMER_ROLE granted to informer.""" + """PenaltyBoard with genesis at current chain time and INFORMER_ROLE granted to informer (no compensation).""" genesis_time = chain.pending_timestamp contract = project.PenaltyBoard.deploy( genesis_time, PERIOD_DURATION, deployer.address, + _zero(), + _zero(), + 0, + _zero(), sender=deployer, ) contract.grantRole(contract.INFORMER_ROLE(), informer.address, sender=deployer) @@ -37,11 +45,16 @@ def penalty_board(project, deployer, informer, chain): def test_constructor_admin_required(project, deployer, chain): genesis_time = chain.pending_timestamp + zero = "0x0000000000000000000000000000000000000000" with ape.reverts("Admin required"): project.PenaltyBoard.deploy( genesis_time, PERIOD_DURATION, - "0x0000000000000000000000000000000000000000", + zero, + zero, + zero, + 0, + zero, sender=deployer, ) From 420456751525156d2c790cac99bee9798ccc1606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Thu, 26 Feb 2026 10:24:59 +0100 Subject: [PATCH 04/17] Change penalty storage approach from period- to staker-centric --- .../contracts/coordination/PenaltyBoard.sol | 67 ++++++++++++++++--- tests/test_penalty_board.py | 51 +++++++------- tests/test_penalty_board_compensation.py | 14 ++-- 3 files changed, 89 insertions(+), 43 deletions(-) diff --git a/contracts/contracts/coordination/PenaltyBoard.sol b/contracts/contracts/coordination/PenaltyBoard.sol index ce28e648d..be57479c9 100644 --- a/contracts/contracts/coordination/PenaltyBoard.sol +++ b/contracts/contracts/coordination/PenaltyBoard.sol @@ -3,34 +3,68 @@ pragma solidity ^0.8.0; import "@openzeppelin/contracts/access/AccessControl.sol"; +import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "./Periods.sol"; +import "./ITACoApplicationForPenaltyBoard.sol"; /** * @title PenaltyBoard * @notice Records which staking providers are penalized per period (period-oriented summary of infractions). * Independent of InfractionCollector; may live on a different chain. An informer (trusted) sets * the list of penalized providers for a given period. + * With compensation enabled (7-arg constructor): also maintains staker-centric penalty list, + * accrued compensation balance, and withdraw to beneficiary. */ contract PenaltyBoard is Periods, AccessControl { bytes32 public constant INFORMER_ROLE = keccak256("INFORMER_ROLE"); event PenalizedProvidersSet(uint256 indexed period, address[] providers); - mapping(uint256 period => address[]) private _penalizedProvidersByPeriod; + // Compensation (optional: when tacoApplication != address(0)) + ITACoApplicationForPenaltyBoard public immutable tacoApplication; + IERC20 public immutable compensationToken; + address public immutable fundHolder; + uint256 public immutable fixedCompensationPerPeriod; + /** + * @notice Staker-centric penalty storage: list of period indices this staker was penalized in + * (monotonic append; periods are kept as uint256). + */ + mapping(address staker => uint256[]) public penalizedPeriodsByStaker; + + /** + * @notice Returns the full list of period indices a staker was penalized in. + */ + function getPenalizedPeriodsByStaker( + address staker + ) external view returns (uint256[] memory) { + return penalizedPeriodsByStaker[staker]; + } + + /** + * @param genesisTime Start of period 0. + * @param periodDuration Duration of one period in seconds. + * @param admin Default admin. + * @param _tacoApplication TACo (or mock) for beneficiary/owner/stakeless. Pass address(0) to disable compensation. + * @param _token Compensation token. Pass address(0) when compensation disabled. + * @param _fixedCompensationPerPeriod Fixed amount per period (0 when disabled). + * @param _fundHolder Holder of tokens for payouts. Pass address(0) when compensation disabled. + */ constructor( uint256 genesisTime, uint256 periodDuration, - address admin + address admin, + address _tacoApplication, + address _token, + uint256 _fixedCompensationPerPeriod, + address _fundHolder ) Periods(genesisTime, periodDuration) { require(admin != address(0), "Admin required"); _grantRole(DEFAULT_ADMIN_ROLE, admin); - } - - function getPenalizedProvidersForPeriod( - uint256 period - ) external view returns (address[] memory) { - return _penalizedProvidersByPeriod[period]; + tacoApplication = ITACoApplicationForPenaltyBoard(_tacoApplication); + compensationToken = IERC20(_token); + fundHolder = _fundHolder; + fixedCompensationPerPeriod = _fixedCompensationPerPeriod; } /** @@ -45,11 +79,24 @@ contract PenaltyBoard is Periods, AccessControl { uint256 current = getCurrentPeriod(); require(period == current || (current > 0 && period == current - 1), "Invalid period"); - delete _penalizedProvidersByPeriod[period]; for (uint256 i = 0; i < provs.length; i++) { - _penalizedProvidersByPeriod[period].push(provs[i]); + penalizedPeriodsByStaker[provs[i]].push(period); } emit PenalizedProvidersSet(period, provs); } + + /** + * @notice Accrued compensation balance for staker (stub: 0 until C3). + */ + function getAccruedBalance(address /* staker */) external pure returns (uint256) { + return 0; + } + + /** + * @notice Withdraw accrued compensation for stakingProvider; tokens sent to beneficiary. Stub: reverts until C3. + */ + function withdraw(address /* stakingProvider */) external pure { + revert("Not implemented"); + } } diff --git a/tests/test_penalty_board.py b/tests/test_penalty_board.py index 5085f1379..e52597350 100644 --- a/tests/test_penalty_board.py +++ b/tests/test_penalty_board.py @@ -1,8 +1,10 @@ """Tests for PenaltyBoard in isolation (period-oriented penalized providers).""" -import ape import pytest +import ape +from ape.utils import ZERO_ADDRESS + PERIOD_DURATION = 3600 # 1 hour @@ -21,10 +23,6 @@ def other_account(accounts): return accounts[2] -def _zero(): - return "0x0000000000000000000000000000000000000000" - - @pytest.fixture def penalty_board(project, deployer, informer, chain): """PenaltyBoard with genesis at current chain time and INFORMER_ROLE granted to informer (no compensation).""" @@ -33,10 +31,10 @@ def penalty_board(project, deployer, informer, chain): genesis_time, PERIOD_DURATION, deployer.address, - _zero(), - _zero(), + ZERO_ADDRESS, + ZERO_ADDRESS, 0, - _zero(), + ZERO_ADDRESS, sender=deployer, ) contract.grantRole(contract.INFORMER_ROLE(), informer.address, sender=deployer) @@ -45,16 +43,15 @@ def penalty_board(project, deployer, informer, chain): def test_constructor_admin_required(project, deployer, chain): genesis_time = chain.pending_timestamp - zero = "0x0000000000000000000000000000000000000000" with ape.reverts("Admin required"): project.PenaltyBoard.deploy( genesis_time, PERIOD_DURATION, - zero, - zero, - zero, + ZERO_ADDRESS, + ZERO_ADDRESS, + ZERO_ADDRESS, 0, - zero, + ZERO_ADDRESS, sender=deployer, ) @@ -70,7 +67,7 @@ def test_only_informer_can_set_penalized_providers( penalty_board.setPenalizedProvidersForPeriod(provs, current, sender=deployer) penalty_board.setPenalizedProvidersForPeriod(provs, current, sender=informer) - assert penalty_board.getPenalizedProvidersForPeriod(current) == provs + assert penalty_board.getPenalizedPeriodsByStaker(other_account.address) == [current] def test_period_must_be_current_or_previous( @@ -85,7 +82,7 @@ def test_period_must_be_current_or_previous( penalty_board.setPenalizedProvidersForPeriod(provs, current + 1, sender=informer) penalty_board.setPenalizedProvidersForPeriod(provs, current, sender=informer) - assert penalty_board.getPenalizedProvidersForPeriod(current) == provs + assert penalty_board.getPenalizedPeriodsByStaker(other_account.address) == [current] # Advance into period 1; then both 0 and 1 are valid. chain.pending_timestamp += PERIOD_DURATION @@ -93,38 +90,42 @@ def test_period_must_be_current_or_previous( penalty_board.setPenalizedProvidersForPeriod(provs, 0, sender=informer) penalty_board.setPenalizedProvidersForPeriod(provs, 1, sender=informer) - assert penalty_board.getPenalizedProvidersForPeriod(0) == provs - assert penalty_board.getPenalizedProvidersForPeriod(1) == provs + periods = penalty_board.getPenalizedPeriodsByStaker(other_account.address) + assert periods == [current, 0, 1] # Period 2 is invalid (not current or previous). with ape.reverts("Invalid period"): penalty_board.setPenalizedProvidersForPeriod(provs, 2, sender=informer) -def test_getter_returns_set_list( +def test_staker_penalty_list_updated_for_all_providers( penalty_board, informer, other_account, accounts ): - """getPenalizedProvidersForPeriod returns the list set for that period.""" + """penalizedPeriodsByStaker records the period for each provider in provs.""" current = penalty_board.getCurrentPeriod() provs = [accounts[3].address, accounts[4].address, other_account.address] penalty_board.setPenalizedProvidersForPeriod(provs, current, sender=informer) - assert penalty_board.getPenalizedProvidersForPeriod(current) == provs + for addr in provs: + assert penalty_board.getPenalizedPeriodsByStaker(addr) == [current] -def test_setting_again_replaces_list( +def test_setting_again_appends_for_new_providers( penalty_board, informer, other_account, accounts ): - """Calling setPenalizedProvidersForPeriod again for the same period replaces the list.""" + """Calling setPenalizedProvidersForPeriod again for the same period appends penalties for new providers.""" current = penalty_board.getCurrentPeriod() first = [accounts[5].address, accounts[6].address] penalty_board.setPenalizedProvidersForPeriod(first, current, sender=informer) - assert penalty_board.getPenalizedProvidersForPeriod(current) == first + for addr in first: + assert penalty_board.getPenalizedPeriodsByStaker(addr) == [current] second = [other_account.address] penalty_board.setPenalizedProvidersForPeriod(second, current, sender=informer) - assert penalty_board.getPenalizedProvidersForPeriod(current) == second + for addr in first: + assert penalty_board.getPenalizedPeriodsByStaker(addr) == [current] + assert penalty_board.getPenalizedPeriodsByStaker(other_account.address) == [current] def test_period_zero_only_when_current_is_zero( @@ -137,7 +138,7 @@ def test_period_zero_only_when_current_is_zero( provs = [other_account.address] penalty_board.setPenalizedProvidersForPeriod(provs, 0, sender=informer) - assert penalty_board.getPenalizedProvidersForPeriod(0) == provs + assert penalty_board.getPenalizedPeriodsByStaker(other_account.address) == [0] with ape.reverts("Invalid period"): penalty_board.setPenalizedProvidersForPeriod(provs, 1, sender=informer) diff --git a/tests/test_penalty_board_compensation.py b/tests/test_penalty_board_compensation.py index 75325d53c..ae4fb1697 100644 --- a/tests/test_penalty_board_compensation.py +++ b/tests/test_penalty_board_compensation.py @@ -81,13 +81,11 @@ def registered_staker(mock_taco, staking_provider, deployer, beneficiary): def test_penalized_periods_by_staker_updated( penalty_board_comp, informer, staking_provider ): - """setPenalizedProvidersForPeriod(provs, period) causes getPenalizedPeriodsForStaker(prov) to include period.""" + """setPenalizedProvidersForPeriod(provs, period) causes penalizedPeriodsByStaker[prov] to include period.""" current = penalty_board_comp.getCurrentPeriod() provs = [staking_provider.address] penalty_board_comp.setPenalizedProvidersForPeriod(provs, current, sender=informer) - periods = penalty_board_comp.getPenalizedPeriodsForStaker(staking_provider.address) - assert len(periods) == 1 - assert periods[0] == current + assert penalty_board_comp.getPenalizedPeriodsByStaker(staking_provider.address) == [current] def test_penalized_periods_monotonic_append( @@ -100,10 +98,10 @@ def test_penalized_periods_monotonic_append( chain.pending_timestamp += PERIOD_DURATION assert penalty_board_comp.getCurrentPeriod() == current + 1 penalty_board_comp.setPenalizedProvidersForPeriod(provs, current + 1, sender=informer) - periods = penalty_board_comp.getPenalizedPeriodsForStaker(staking_provider.address) - assert len(periods) == 2 - assert periods[0] == current - assert periods[1] == current + 1 + assert penalty_board_comp.getPenalizedPeriodsByStaker(staking_provider.address) == [ + current, + current + 1, + ] def test_withdraw_reverts_until_implemented( From 1aa4cc243000e73427b4ec4fe015f4522bc01fce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Thu, 26 Feb 2026 13:31:54 +0100 Subject: [PATCH 05/17] Accrual compensation algorithm --- .../contracts/coordination/PenaltyBoard.sol | 180 ++++++++++++++++-- tests/test_penalty_board_compensation.py | 139 ++++++++++++-- 2 files changed, 290 insertions(+), 29 deletions(-) diff --git a/contracts/contracts/coordination/PenaltyBoard.sol b/contracts/contracts/coordination/PenaltyBoard.sol index be57479c9..3a687cc68 100644 --- a/contracts/contracts/coordination/PenaltyBoard.sol +++ b/contracts/contracts/coordination/PenaltyBoard.sol @@ -14,6 +14,8 @@ import "./ITACoApplicationForPenaltyBoard.sol"; * the list of penalized providers for a given period. * With compensation enabled (7-arg constructor): also maintains staker-centric penalty list, * accrued compensation balance, and withdraw to beneficiary. + * @dev Gas: getAccruedBalance/withdraw cost scales with (number of periods in accrual window) and + * (number of penalties in range). _getPenalizedPeriodsInRange can be optimized with binary search (monotonic array). */ contract PenaltyBoard is Periods, AccessControl { bytes32 public constant INFORMER_ROLE = keccak256("INFORMER_ROLE"); @@ -32,14 +34,13 @@ contract PenaltyBoard is Periods, AccessControl { */ mapping(address staker => uint256[]) public penalizedPeriodsByStaker; - /** - * @notice Returns the full list of period indices a staker was penalized in. - */ - function getPenalizedPeriodsByStaker( - address staker - ) external view returns (uint256[] memory) { - return penalizedPeriodsByStaker[staker]; - } + /// Number of periods a penalty affects (penalty in period k affects k..k+PENALTY_WINDOW_PERIODS inclusive). + uint256 private constant PENALTY_WINDOW_PERIODS = 3; + + // Accrued compensation state (lazy accrual via _computeAccruedSinceLast). + // _lastAccruedPeriodPlusOne: 0 = never accrued (start from period 0); else start next accrual at this period. + mapping(address staker => uint256) private _accruedBalance; + mapping(address staker => uint256) private _lastAccruedPeriodPlusOne; /** * @param genesisTime Start of period 0. @@ -67,6 +68,15 @@ contract PenaltyBoard is Periods, AccessControl { fixedCompensationPerPeriod = _fixedCompensationPerPeriod; } + /** + * @notice Returns the full list of period indices a staker was penalized in. + */ + function getPenalizedPeriodsByStaker( + address staker + ) external view returns (uint256[] memory) { + return penalizedPeriodsByStaker[staker]; + } + /** * @notice Set the list of penalized staking providers for a period. * @param provs Staking provider addresses to record as penalized for the period. @@ -87,16 +97,158 @@ contract PenaltyBoard is Periods, AccessControl { } /** - * @notice Accrued compensation balance for staker (stub: 0 until C3). + * @notice Accrued compensation balance for staker up to the current period. + * Does not modify state; accrual is persisted on withdraw. */ - function getAccruedBalance(address /* staker */) external pure returns (uint256) { - return 0; + function getAccruedBalance(address stakingProvider) external view returns (uint256) { + if ( + fixedCompensationPerPeriod == 0 || + address(compensationToken) == address(0) || + address(tacoApplication) == address(0) + ) { + return 0; + } + + uint256 current = getCurrentPeriod(); + uint256 delta = _computeAccruedSinceLast(stakingProvider, current); + return _accruedBalance[stakingProvider] + delta; } /** - * @notice Withdraw accrued compensation for stakingProvider; tokens sent to beneficiary. Stub: reverts until C3. + * @notice Withdraw accrued compensation for stakingProvider; tokens sent to beneficiary. */ - function withdraw(address /* stakingProvider */) external pure { - revert("Not implemented"); + function withdraw(address stakingProvider) external { + require(stakingProvider != address(0), "Staking provider required"); + require( + fixedCompensationPerPeriod > 0 && + address(compensationToken) != address(0) && + address(tacoApplication) != address(0) && + fundHolder != address(0), + "Compensation disabled" + ); + + (address owner, address beneficiaryAddress) = tacoApplication.getRoles(stakingProvider); + require( + msg.sender == stakingProvider || + msg.sender == owner || + msg.sender == beneficiaryAddress, + "Unauthorized" + ); + + uint256 current = getCurrentPeriod(); + + // No accrual for stakeless providers. + if (tacoApplication.isStakeless(stakingProvider)) { + revert("Nothing to withdraw"); + } + + uint256 delta = _computeAccruedSinceLast(stakingProvider, current); + uint256 amount = _accruedBalance[stakingProvider] + delta; + require(amount > 0, "Nothing to withdraw"); + + _accruedBalance[stakingProvider] = 0; + _lastAccruedPeriodPlusOne[stakingProvider] = current + 1; + + address payable beneficiary = payable(beneficiaryAddress); + + bool ok = compensationToken.transferFrom(fundHolder, beneficiary, amount); + require(ok, "Transfer failed"); + } + + function _computeAccruedSinceLast( + address stakingProvider, + uint256 currentPeriod + ) internal view returns (uint256) { + if ( + fixedCompensationPerPeriod == 0 || + address(compensationToken) == address(0) || + address(tacoApplication) == address(0) + ) { + return 0; + } + + if (tacoApplication.isStakeless(stakingProvider)) { + return 0; + } + + uint256 startPeriod = _lastAccruedPeriodPlusOne[stakingProvider]; // 0 = never accrued → start at 0 + + if (startPeriod > currentPeriod) { + return 0; + } + + uint256 fromPenalties = startPeriod > PENALTY_WINDOW_PERIODS ? startPeriod - PENALTY_WINDOW_PERIODS : 0; + + uint256[] memory penaltiesInRange = _getPenalizedPeriodsInRange( + stakingProvider, + fromPenalties, + currentPeriod + ); + + // No penalties affecting this window: full periods accrue. + if (penaltiesInRange.length == 0) { + uint256 numPeriods = currentPeriod - startPeriod + 1; + return numPeriods * fixedCompensationPerPeriod; + } + + // penaltiesFactor: nPenalties == 0 => 1, nPenalties > 0 => 0. + // Implemented by checking if there exists any penalty k with P-PENALTY_WINDOW_PERIODS <= k <= P. + uint256 accrued = 0; + for (uint256 p = startPeriod; p <= currentPeriod; p++) { + bool penalized = false; + for (uint256 i = 0; i < penaltiesInRange.length; i++) { + uint256 k = penaltiesInRange[i]; + if (k > p) { + continue; + } + + if (k + PENALTY_WINDOW_PERIODS < p) { + continue; + } + + penalized = true; + break; + } + + if (!penalized) { + accrued += fixedCompensationPerPeriod; + } + } + + return accrued; + } + + /// @dev Optimize later: array is monotonic; use binary search for fromPeriod/toPeriod to get + /// the inclusive index range [i, j], then copy in one pass instead of count + copy. + function _getPenalizedPeriodsInRange( + address stakingProvider, + uint256 fromPeriod, + uint256 toPeriod + ) internal view returns (uint256[] memory) { + uint256[] storage all = penalizedPeriodsByStaker[stakingProvider]; + uint256 len = all.length; + if (len == 0 || fromPeriod > toPeriod) { + return new uint256[](0); + } + + uint256 count; + for (uint256 i = 0; i < len; i++) { + uint256 p = all[i]; + if (p >= fromPeriod && p <= toPeriod) { + count++; + } + } + + uint256[] memory result = new uint256[](count); + uint256 idx; + for (uint256 i = 0; i < len; i++) { + uint256 p = all[i]; + if (p >= fromPeriod && p <= toPeriod) { + result[idx] = p; + idx++; + } + } + + return result; } } diff --git a/tests/test_penalty_board_compensation.py b/tests/test_penalty_board_compensation.py index ae4fb1697..fcea23afe 100644 --- a/tests/test_penalty_board_compensation.py +++ b/tests/test_penalty_board_compensation.py @@ -39,7 +39,7 @@ def other_account(accounts): @pytest.fixture(scope="module") -def mock_taco(project, deployer): +def mock_taco_app(project, deployer): return project.MockTACoForPenaltyBoard.deploy(sender=deployer) @@ -49,14 +49,14 @@ def token(project, deployer): @pytest.fixture -def penalty_board_comp(project, deployer, informer, chain, mock_taco, token, fund_holder): +def penalty_board_comp(project, deployer, informer, chain, mock_taco_app, token, fund_holder): """PenaltyBoard with compensation enabled (7-arg constructor).""" genesis_time = chain.pending_timestamp contract = project.PenaltyBoard.deploy( genesis_time, PERIOD_DURATION, deployer.address, - mock_taco.address, + mock_taco_app.address, token.address, FIXED_COMPENSATION, fund_holder.address, @@ -67,9 +67,9 @@ def penalty_board_comp(project, deployer, informer, chain, mock_taco, token, fun @pytest.fixture -def registered_staker(mock_taco, staking_provider, deployer, beneficiary): +def registered_staker(mock_taco_app, staking_provider, deployer, beneficiary): """Register staking_provider in mock TACo: owner=deployer, beneficiary=beneficiary, not stakeless.""" - mock_taco.setRoles( + mock_taco_app.setRoles( staking_provider.address, deployer.address, beneficiary.address, @@ -104,17 +104,29 @@ def test_penalized_periods_monotonic_append( ] -def test_withdraw_reverts_until_implemented( - penalty_board_comp, registered_staker, beneficiary, staking_provider +def test_withdraw_reverts_nothing_to_withdraw_for_stakeless( + penalty_board_comp, mock_taco_app, deployer, beneficiary, accounts ): - """Withdraw reverts with 'Not implemented' until C3 (stub).""" - with ape.reverts("Not implemented"): - penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) + """Withdraw reverts with 'Nothing to withdraw' for stakeless staker (no accrual).""" + stakeless_provider = accounts[6] + mock_taco_app.setRoles( + stakeless_provider.address, + deployer.address, + beneficiary.address, + True, # stakeless + sender=deployer, + ) + with ape.reverts("Nothing to withdraw"): + penalty_board_comp.withdraw(stakeless_provider.address, sender=beneficiary) -def test_get_accrued_balance_zero_without_accrual(penalty_board_comp, staking_provider): - """getAccruedBalance returns 0 before accrual is implemented (stub).""" - assert penalty_board_comp.getAccruedBalance(staking_provider.address) == 0 +def test_get_accrued_balance_reflects_periods_without_prior_withdraw( + penalty_board_comp, staking_provider, registered_staker +): + """getAccruedBalance returns accrued amount for periods 0..current when staker has not yet withdrawn.""" + # At deploy we are in period 0; one period accrues (fixed per period). + balance = penalty_board_comp.getAccruedBalance(staking_provider.address) + assert balance == FIXED_COMPENSATION def test_accrual_with_no_penalties_gives_positive_balance( @@ -163,11 +175,11 @@ def test_withdraw_sends_tokens_to_beneficiary( def test_stakeless_staker_gets_zero_compensation( - penalty_board_comp, mock_taco, chain, deployer, beneficiary, accounts + penalty_board_comp, mock_taco_app, chain, deployer, beneficiary, accounts ): """Stakeless staker accrues 0 (fails until C3 implements stakeless check).""" stakeless_provider = accounts[6] - mock_taco.setRoles( + mock_taco_app.setRoles( stakeless_provider.address, deployer.address, beneficiary.address, @@ -177,3 +189,100 @@ def test_stakeless_staker_gets_zero_compensation( chain.pending_timestamp += 2 * PERIOD_DURATION balance = penalty_board_comp.getAccruedBalance(stakeless_provider.address) assert balance == 0, "Stakeless staker should have 0 accrued compensation" + + +# --- C4: Edge cases --- + + +def test_first_withdrawal_accrues_from_period_zero( + penalty_board_comp, + token, + fund_holder, + beneficiary, + staking_provider, + registered_staker, + chain, + deployer, +): + """First withdrawal (sentinel: never accrued) accrues from period 0 to current; full amount to beneficiary.""" + num_periods = 4 + chain.pending_timestamp += num_periods * PERIOD_DURATION + # Periods 0..4 → 5 periods + expected = (num_periods + 1) * FIXED_COMPENSATION + token.transfer(fund_holder.address, expected * 2, sender=deployer) + token.approve(penalty_board_comp.address, 2**256 - 1, sender=fund_holder) + + before = token.balanceOf(beneficiary.address) + penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) + after = token.balanceOf(beneficiary.address) + assert after - before == expected + + +def test_withdraw_reverts_when_nothing_left_after_prior_withdraw( + penalty_board_comp, + token, + fund_holder, + beneficiary, + staking_provider, + registered_staker, + deployer, +): + """After a full withdraw, second withdraw without time advance reverts with Nothing to withdraw.""" + token.transfer(fund_holder.address, FIXED_COMPENSATION * 2, sender=deployer) + token.approve(penalty_board_comp.address, 2**256 - 1, sender=fund_holder) + penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) + with ape.reverts("Nothing to withdraw"): + penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) + + +def test_many_periods_full_accrual( + penalty_board_comp, + token, + fund_holder, + beneficiary, + staking_provider, + registered_staker, + chain, + deployer, +): + """Many periods with no penalties: full accrual (numPeriods * fixed per period).""" + n = 10 + chain.pending_timestamp += n * PERIOD_DURATION + # Periods 0..n → n+1 periods + expected = (n + 1) * FIXED_COMPENSATION + token.transfer(fund_holder.address, expected + 1000, sender=deployer) + token.approve(penalty_board_comp.address, 2**256 - 1, sender=fund_holder) + + before = token.balanceOf(beneficiary.address) + penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) + after = token.balanceOf(beneficiary.address) + assert after - before == expected + + +def test_penalty_in_range_reduces_compensation( + penalty_board_comp, + token, + fund_holder, + beneficiary, + staking_provider, + registered_staker, + chain, + deployer, + informer, +): + """A penalty in period k affects k..k+PENALTY_WINDOW_PERIODS; only unaffected periods get full compensation.""" + # Advance to period 1, set penalty for period 1 (current), then advance to period 5. + chain.pending_timestamp += PERIOD_DURATION + penalty_board_comp.setPenalizedProvidersForPeriod( + [staking_provider.address], 1, sender=informer + ) + chain.pending_timestamp += 4 * PERIOD_DURATION + # Now current period is 5. Accrual: periods 0..5. Penalty at 1 affects 1,2,3,4. So only 0 and 5 get full. + expected = 2 * FIXED_COMPENSATION + token.transfer(fund_holder.address, expected + 1000, sender=deployer) + token.approve(penalty_board_comp.address, 2**256 - 1, sender=fund_holder) + + before = token.balanceOf(beneficiary.address) + penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) + after = token.balanceOf(beneficiary.address) + assert after - before == expected From 0d64c6bcb0427e0781e181d271aae46a8b28a42d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Thu, 26 Feb 2026 23:04:35 +0100 Subject: [PATCH 06/17] Adapt TACoApp mock to match rolesOf() instead of getRoles() --- .../coordination/ITACoApplicationForPenaltyBoard.sol | 4 ++-- contracts/contracts/coordination/PenaltyBoard.sol | 2 +- contracts/test/MockTACoForPenaltyBoard.sol | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol b/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol index dff103be7..9fa828fc5 100644 --- a/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol +++ b/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol @@ -17,9 +17,9 @@ interface ITACoApplicationForPenaltyBoard { /** * @notice Returns owner and beneficiary for a staking provider. * Withdraw(stakingProvider) may be called by stakingProvider, owner, or beneficiary. - * If TACoApplication does not expose this, PenaltyBoard may depend on IStaking.rolesOf instead. + * Matches TACoApplication.rolesOf. */ - function getRoles(address stakingProvider) external view returns (address owner, address beneficiary); + function rolesOf(address stakingProvider) external view returns (address owner, address beneficiary); /** * @notice Returns true if the staking provider is stakeless (compensation = 0). diff --git a/contracts/contracts/coordination/PenaltyBoard.sol b/contracts/contracts/coordination/PenaltyBoard.sol index 3a687cc68..cc3b94168 100644 --- a/contracts/contracts/coordination/PenaltyBoard.sol +++ b/contracts/contracts/coordination/PenaltyBoard.sol @@ -127,7 +127,7 @@ contract PenaltyBoard is Periods, AccessControl { "Compensation disabled" ); - (address owner, address beneficiaryAddress) = tacoApplication.getRoles(stakingProvider); + (address owner, address beneficiaryAddress) = tacoApplication.rolesOf(stakingProvider); require( msg.sender == stakingProvider || msg.sender == owner || diff --git a/contracts/test/MockTACoForPenaltyBoard.sol b/contracts/test/MockTACoForPenaltyBoard.sol index 203a1289f..db9b00dcf 100644 --- a/contracts/test/MockTACoForPenaltyBoard.sol +++ b/contracts/test/MockTACoForPenaltyBoard.sol @@ -34,7 +34,7 @@ contract MockTACoForPenaltyBoard is ITACoApplicationForPenaltyBoard { return _roles[stakingProvider].beneficiary; } - function getRoles(address stakingProvider) external view returns (address owner, address beneficiary) { + function rolesOf(address stakingProvider) external view returns (address owner, address beneficiary) { StakerRoles storage r = _roles[stakingProvider]; return (r.owner, r.beneficiary); } From 7e2d0e2e1e99ac8b12257bad25ec85d0228eac26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Fri, 27 Feb 2026 08:40:08 +0100 Subject: [PATCH 07/17] Appease linter --- .../coordination/ITACoApplicationForPenaltyBoard.sol | 8 ++++++-- contracts/contracts/coordination/PenaltyBoard.sol | 8 ++++---- contracts/test/MockTACoForPenaltyBoard.sol | 4 +++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol b/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol index 9fa828fc5..644c812a2 100644 --- a/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol +++ b/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol @@ -12,14 +12,18 @@ interface ITACoApplicationForPenaltyBoard { * @notice Returns beneficiary for a staking provider (tokens are sent here on withdraw). * Assumed never zero for a registered staking provider. */ - function getBeneficiary(address stakingProvider) external view returns (address payable beneficiary); + function getBeneficiary( + address stakingProvider + ) external view returns (address payable beneficiary); /** * @notice Returns owner and beneficiary for a staking provider. * Withdraw(stakingProvider) may be called by stakingProvider, owner, or beneficiary. * Matches TACoApplication.rolesOf. */ - function rolesOf(address stakingProvider) external view returns (address owner, address beneficiary); + function rolesOf( + address stakingProvider + ) external view returns (address owner, address beneficiary); /** * @notice Returns true if the staking provider is stakeless (compensation = 0). diff --git a/contracts/contracts/coordination/PenaltyBoard.sol b/contracts/contracts/coordination/PenaltyBoard.sol index cc3b94168..9065811fc 100644 --- a/contracts/contracts/coordination/PenaltyBoard.sol +++ b/contracts/contracts/coordination/PenaltyBoard.sol @@ -71,9 +71,7 @@ contract PenaltyBoard is Periods, AccessControl { /** * @notice Returns the full list of period indices a staker was penalized in. */ - function getPenalizedPeriodsByStaker( - address staker - ) external view returns (uint256[] memory) { + function getPenalizedPeriodsByStaker(address staker) external view returns (uint256[] memory) { return penalizedPeriodsByStaker[staker]; } @@ -177,7 +175,9 @@ contract PenaltyBoard is Periods, AccessControl { return 0; } - uint256 fromPenalties = startPeriod > PENALTY_WINDOW_PERIODS ? startPeriod - PENALTY_WINDOW_PERIODS : 0; + uint256 fromPenalties = startPeriod > PENALTY_WINDOW_PERIODS + ? startPeriod - PENALTY_WINDOW_PERIODS + : 0; uint256[] memory penaltiesInRange = _getPenalizedPeriodsInRange( stakingProvider, diff --git a/contracts/test/MockTACoForPenaltyBoard.sol b/contracts/test/MockTACoForPenaltyBoard.sol index db9b00dcf..86c3f5b37 100644 --- a/contracts/test/MockTACoForPenaltyBoard.sol +++ b/contracts/test/MockTACoForPenaltyBoard.sol @@ -34,7 +34,9 @@ contract MockTACoForPenaltyBoard is ITACoApplicationForPenaltyBoard { return _roles[stakingProvider].beneficiary; } - function rolesOf(address stakingProvider) external view returns (address owner, address beneficiary) { + function rolesOf( + address stakingProvider + ) external view returns (address owner, address beneficiary) { StakerRoles storage r = _roles[stakingProvider]; return (r.owner, r.beneficiary); } From 83f140cae056b8b9a4e9e9954f1cab0476e53ea3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Fri, 6 Mar 2026 15:57:05 +0100 Subject: [PATCH 08/17] WIP on penalties --- .../contracts/coordination/PenaltyBoard.sol | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/contracts/contracts/coordination/PenaltyBoard.sol b/contracts/contracts/coordination/PenaltyBoard.sol index 9065811fc..88af2c8e9 100644 --- a/contracts/contracts/coordination/PenaltyBoard.sol +++ b/contracts/contracts/coordination/PenaltyBoard.sol @@ -27,6 +27,8 @@ contract PenaltyBoard is Periods, AccessControl { IERC20 public immutable compensationToken; address public immutable fundHolder; uint256 public immutable fixedCompensationPerPeriod; + uint256 public immutable fixedCompensationPerPeriod2Penalties; + uint256 public immutable fixedCompensationPerPeriod3Penalties; /** * @notice Staker-centric penalty storage: list of period indices this staker was penalized in @@ -58,6 +60,8 @@ contract PenaltyBoard is Periods, AccessControl { address _tacoApplication, address _token, uint256 _fixedCompensationPerPeriod, + uint256 _fixedCompensationPerPeriod2Penalties, + uint256 _fixedCompensationPerPeriod3Penalties, address _fundHolder ) Periods(genesisTime, periodDuration) { require(admin != address(0), "Admin required"); @@ -66,6 +70,8 @@ contract PenaltyBoard is Periods, AccessControl { compensationToken = IERC20(_token); fundHolder = _fundHolder; fixedCompensationPerPeriod = _fixedCompensationPerPeriod; + fixedCompensationPerPeriod2Penalties = _fixedCompensationPerPeriod2Penalties; + fixedCompensationPerPeriod3Penalties = _fixedCompensationPerPeriod3Penalties; } /** @@ -185,33 +191,38 @@ contract PenaltyBoard is Periods, AccessControl { currentPeriod ); - // No penalties affecting this window: full periods accrue. - if (penaltiesInRange.length == 0) { + // No penalties or only one penalty affecting this window: full periods accrue. + if (penaltiesInRange.length <= 1) { uint256 numPeriods = currentPeriod - startPeriod + 1; return numPeriods * fixedCompensationPerPeriod; } - // penaltiesFactor: nPenalties == 0 => 1, nPenalties > 0 => 0. - // Implemented by checking if there exists any penalty k with P-PENALTY_WINDOW_PERIODS <= k <= P. + // TODO: Consider potential optimization for length > 1: + // - Check if all periods with penalties are spaced out by at least PENALTY_WINDOW_PERIODS. If so, we can skip the loop and return the full compensation + + // Naive approach where we check ALL periods in the range. + // - For each period, we check how many penalties are in the window that ends at the current period. + // - This check involves iterating over the penaltiesInRange array, which is sorted. uint256 accrued = 0; for (uint256 p = startPeriod; p <= currentPeriod; p++) { - bool penalized = false; - for (uint256 i = 0; i < penaltiesInRange.length; i++) { - uint256 k = penaltiesInRange[i]; - if (k > p) { - continue; - } + uint256 windowWidth = p + 1 >= PENALTY_WINDOW_PERIODS ? PENALTY_WINDOW_PERIODS : p; + uint256 penaltyHorizon = p - windowWidth; - if (k + PENALTY_WINDOW_PERIODS < p) { - continue; + uint256 numPenaltiesInWindow = 0; + for (uint256 i = 0; i < penaltiesInRange.length; i++) { + if (penaltiesInRange[i] >= penaltyHorizon && penaltiesInRange[i] <= p) { + numPenaltiesInWindow++; + } else { + break; } - - penalized = true; - break; } - if (!penalized) { + if (numPenaltiesInWindow <= 1) { accrued += fixedCompensationPerPeriod; + } else if (numPenaltiesInWindow == 2) { + accrued += fixedCompensationPerPeriod2Penalties; + } else if (numPenaltiesInWindow == 3) { + accrued += fixedCompensationPerPeriod3Penalties; } } From 2b68c4afb09f9b2843fc48f27f7d97f9962ca13b Mon Sep 17 00:00:00 2001 From: Viktoriia Zotova Date: Sat, 7 Mar 2026 10:26:51 -0500 Subject: [PATCH 09/17] Makes tests for PenaltyBoard work --- .../contracts/coordination/PenaltyBoard.sol | 14 ++++--- tests/test_infraction.py | 10 ++--- tests/test_penalty_board.py | 19 ++++----- tests/test_penalty_board_compensation.py | 40 ++++++++++++++----- 4 files changed, 52 insertions(+), 31 deletions(-) diff --git a/contracts/contracts/coordination/PenaltyBoard.sol b/contracts/contracts/coordination/PenaltyBoard.sol index 88af2c8e9..1662e677f 100644 --- a/contracts/contracts/coordination/PenaltyBoard.sol +++ b/contracts/contracts/coordination/PenaltyBoard.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.0; import "@openzeppelin/contracts/access/AccessControl.sol"; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import "./Periods.sol"; import "./ITACoApplicationForPenaltyBoard.sol"; @@ -18,6 +19,8 @@ import "./ITACoApplicationForPenaltyBoard.sol"; * (number of penalties in range). _getPenalizedPeriodsInRange can be optimized with binary search (monotonic array). */ contract PenaltyBoard is Periods, AccessControl { + using SafeERC20 for IERC20; + bytes32 public constant INFORMER_ROLE = keccak256("INFORMER_ROLE"); event PenalizedProvidersSet(uint256 indexed period, address[] providers); @@ -153,10 +156,9 @@ contract PenaltyBoard is Periods, AccessControl { _accruedBalance[stakingProvider] = 0; _lastAccruedPeriodPlusOne[stakingProvider] = current + 1; - address payable beneficiary = payable(beneficiaryAddress); + address beneficiary = beneficiaryAddress; - bool ok = compensationToken.transferFrom(fundHolder, beneficiary, amount); - require(ok, "Transfer failed"); + compensationToken.safeTransferFrom(fundHolder, beneficiary, amount); } function _computeAccruedSinceLast( @@ -205,12 +207,14 @@ contract PenaltyBoard is Periods, AccessControl { // - This check involves iterating over the penaltiesInRange array, which is sorted. uint256 accrued = 0; for (uint256 p = startPeriod; p <= currentPeriod; p++) { - uint256 windowWidth = p + 1 >= PENALTY_WINDOW_PERIODS ? PENALTY_WINDOW_PERIODS : p; + uint256 windowWidth = p >= PENALTY_WINDOW_PERIODS ? PENALTY_WINDOW_PERIODS : p; uint256 penaltyHorizon = p - windowWidth; uint256 numPenaltiesInWindow = 0; for (uint256 i = 0; i < penaltiesInRange.length; i++) { - if (penaltiesInRange[i] >= penaltyHorizon && penaltiesInRange[i] <= p) { + if (penaltiesInRange[i] < penaltyHorizon) { + continue; + } else if (penaltiesInRange[i] >= penaltyHorizon && penaltiesInRange[i] <= p) { numPenaltiesInWindow++; } else { break; diff --git a/tests/test_infraction.py b/tests/test_infraction.py index 17f3cee69..752f21b42 100644 --- a/tests/test_infraction.py +++ b/tests/test_infraction.py @@ -145,6 +145,8 @@ def penalty_board(project, deployer, informer, chain): zero, zero, 0, + 0, + 0, zero, sender=deployer, ) @@ -163,11 +165,11 @@ def test_no_infractions( coordinator.initiateRitual( fee_model, nodes, initiator, DURATION, global_allow_list.address, sender=initiator ) - + size = len(nodes) threshold = coordinator.getThresholdForRitualSize(size) transcript = generate_transcript(size, threshold) - + for node in nodes: coordinator.publishTranscript(0, transcript, sender=node) @@ -275,9 +277,7 @@ def test_infraction_collector_and_penalty_board_together( # Same period still (period duration is 1 week; we advanced ~2000s). Informer records # penalized providers for this period on PenaltyBoard. current_period = penalty_board.getCurrentPeriod() - penalty_board.setPenalizedProvidersForPeriod( - failing_providers, current_period, sender=informer - ) + penalty_board.setPenalizedProvidersForPeriod(failing_providers, current_period, sender=informer) for provider in failing_providers: periods = penalty_board.getPenalizedPeriodsByStaker(provider) assert periods == [current_period] diff --git a/tests/test_penalty_board.py b/tests/test_penalty_board.py index e52597350..ccf5f43a7 100644 --- a/tests/test_penalty_board.py +++ b/tests/test_penalty_board.py @@ -1,8 +1,7 @@ """Tests for PenaltyBoard in isolation (period-oriented penalized providers).""" -import pytest - import ape +import pytest from ape.utils import ZERO_ADDRESS PERIOD_DURATION = 3600 # 1 hour @@ -34,6 +33,8 @@ def penalty_board(project, deployer, informer, chain): ZERO_ADDRESS, ZERO_ADDRESS, 0, + 0, + 0, ZERO_ADDRESS, sender=deployer, ) @@ -51,6 +52,8 @@ def test_constructor_admin_required(project, deployer, chain): ZERO_ADDRESS, ZERO_ADDRESS, 0, + 0, + 0, ZERO_ADDRESS, sender=deployer, ) @@ -70,9 +73,7 @@ def test_only_informer_can_set_penalized_providers( assert penalty_board.getPenalizedPeriodsByStaker(other_account.address) == [current] -def test_period_must_be_current_or_previous( - penalty_board, chain, informer, other_account -): +def test_period_must_be_current_or_previous(penalty_board, chain, informer, other_account): """setPenalizedProvidersForPeriod accepts only current or previous period.""" provs = [other_account.address] @@ -110,9 +111,7 @@ def test_staker_penalty_list_updated_for_all_providers( assert penalty_board.getPenalizedPeriodsByStaker(addr) == [current] -def test_setting_again_appends_for_new_providers( - penalty_board, informer, other_account, accounts -): +def test_setting_again_appends_for_new_providers(penalty_board, informer, other_account, accounts): """Calling setPenalizedProvidersForPeriod again for the same period appends penalties for new providers.""" current = penalty_board.getCurrentPeriod() @@ -128,9 +127,7 @@ def test_setting_again_appends_for_new_providers( assert penalty_board.getPenalizedPeriodsByStaker(other_account.address) == [current] -def test_period_zero_only_when_current_is_zero( - penalty_board, informer, other_account -): +def test_period_zero_only_when_current_is_zero(penalty_board, informer, other_account): """When current period is 0, only period 0 is allowed (no underflow on previous).""" current = penalty_board.getCurrentPeriod() if current != 0: diff --git a/tests/test_penalty_board_compensation.py b/tests/test_penalty_board_compensation.py index fcea23afe..546de7c66 100644 --- a/tests/test_penalty_board_compensation.py +++ b/tests/test_penalty_board_compensation.py @@ -5,6 +5,8 @@ PERIOD_DURATION = 3600 FIXED_COMPENSATION = 1000 +REDUCED_FIXED_COMPENSATION_1 = 600 +REDUCED_FIXED_COMPENSATION_2 = 100 TOKEN_SUPPLY = 1_000_000 * 10**18 @@ -59,6 +61,8 @@ def penalty_board_comp(project, deployer, informer, chain, mock_taco_app, token, mock_taco_app.address, token.address, FIXED_COMPENSATION, + REDUCED_FIXED_COMPENSATION_1, + REDUCED_FIXED_COMPENSATION_2, fund_holder.address, sender=deployer, ) @@ -78,9 +82,7 @@ def registered_staker(mock_taco_app, staking_provider, deployer, beneficiary): ) -def test_penalized_periods_by_staker_updated( - penalty_board_comp, informer, staking_provider -): +def test_penalized_periods_by_staker_updated(penalty_board_comp, informer, staking_provider): """setPenalizedProvidersForPeriod(provs, period) causes penalizedPeriodsByStaker[prov] to include period.""" current = penalty_board_comp.getCurrentPeriod() provs = [staking_provider.address] @@ -88,9 +90,7 @@ def test_penalized_periods_by_staker_updated( assert penalty_board_comp.getPenalizedPeriodsByStaker(staking_provider.address) == [current] -def test_penalized_periods_monotonic_append( - penalty_board_comp, informer, staking_provider, chain -): +def test_penalized_periods_monotonic_append(penalty_board_comp, informer, staking_provider, chain): """Calling setPenalizedProvidersForPeriod for different periods appends to each staker's list (monotonic).""" current = penalty_board_comp.getCurrentPeriod() provs = [staking_provider.address] @@ -136,7 +136,9 @@ def test_accrual_with_no_penalties_gives_positive_balance( # Advance 2 periods so there is something to accrue chain.pending_timestamp += 2 * PERIOD_DURATION balance = penalty_board_comp.getAccruedBalance(staking_provider.address) - assert balance > 0, "Accrual not implemented: getAccruedBalance should be > 0 after 2 periods with no penalties" + assert ( + balance > 0 + ), "Accrual not implemented: getAccruedBalance should be > 0 after 2 periods with no penalties" def test_only_owner_provider_beneficiary_can_withdraw( @@ -276,12 +278,30 @@ def test_penalty_in_range_reduces_compensation( penalty_board_comp.setPenalizedProvidersForPeriod( [staking_provider.address], 1, sender=informer ) - chain.pending_timestamp += 4 * PERIOD_DURATION + chain.pending_timestamp += PERIOD_DURATION + penalty_board_comp.setPenalizedProvidersForPeriod( + [staking_provider.address], 2, sender=informer + ) + chain.pending_timestamp += PERIOD_DURATION + penalty_board_comp.setPenalizedProvidersForPeriod( + [staking_provider.address], 3, sender=informer + ) + chain.pending_timestamp += PERIOD_DURATION + penalty_board_comp.setPenalizedProvidersForPeriod( + [staking_provider.address], 4, sender=informer + ) + chain.pending_timestamp += PERIOD_DURATION + # chain.pending_timestamp += 4 * PERIOD_DURATION # Now current period is 5. Accrual: periods 0..5. Penalty at 1 affects 1,2,3,4. So only 0 and 5 get full. - expected = 2 * FIXED_COMPENSATION - token.transfer(fund_holder.address, expected + 1000, sender=deployer) + # 0 1 2 3 4 3 -> F F R1 R2 0 R2 + expected = ( + 2 * FIXED_COMPENSATION + REDUCED_FIXED_COMPENSATION_1 + 2 * REDUCED_FIXED_COMPENSATION_2 + ) + token.transfer(fund_holder.address, 10 * expected, sender=deployer) token.approve(penalty_board_comp.address, 2**256 - 1, sender=fund_holder) + assert penalty_board_comp.getAccruedBalance(staking_provider.address) == expected + before = token.balanceOf(beneficiary.address) penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) after = token.balanceOf(beneficiary.address) From c3c406148c5468f65c54272b33db078a31099ff3 Mon Sep 17 00:00:00 2001 From: Viktoriia Zotova Date: Tue, 10 Mar 2026 13:53:33 -0400 Subject: [PATCH 10/17] Better tests for PenaltyBoard --- .../contracts/coordination/PenaltyBoard.sol | 16 +- contracts/contracts/coordination/Periods.sol | 4 + tests/test_penalty_board_compensation.py | 145 ++++++++++-------- 3 files changed, 99 insertions(+), 66 deletions(-) diff --git a/contracts/contracts/coordination/PenaltyBoard.sol b/contracts/contracts/coordination/PenaltyBoard.sol index 1662e677f..b8db0b0b1 100644 --- a/contracts/contracts/coordination/PenaltyBoard.sol +++ b/contracts/contracts/coordination/PenaltyBoard.sol @@ -116,8 +116,10 @@ contract PenaltyBoard is Periods, AccessControl { return 0; } - uint256 current = getCurrentPeriod(); - uint256 delta = _computeAccruedSinceLast(stakingProvider, current); + int256 current = getCurrentPaymentPeriod(); + uint256 delta = current >= 0 + ? _computeAccruedSinceLast(stakingProvider, uint256(current)) + : 0; return _accruedBalance[stakingProvider] + delta; } @@ -142,19 +144,21 @@ contract PenaltyBoard is Periods, AccessControl { "Unauthorized" ); - uint256 current = getCurrentPeriod(); - // No accrual for stakeless providers. if (tacoApplication.isStakeless(stakingProvider)) { revert("Nothing to withdraw"); } - uint256 delta = _computeAccruedSinceLast(stakingProvider, current); + int256 current = getCurrentPaymentPeriod(); + + uint256 delta = current >= 0 + ? _computeAccruedSinceLast(stakingProvider, uint256(current)) + : 0; uint256 amount = _accruedBalance[stakingProvider] + delta; require(amount > 0, "Nothing to withdraw"); _accruedBalance[stakingProvider] = 0; - _lastAccruedPeriodPlusOne[stakingProvider] = current + 1; + _lastAccruedPeriodPlusOne[stakingProvider] = uint256(current) + 1; address beneficiary = beneficiaryAddress; diff --git a/contracts/contracts/coordination/Periods.sol b/contracts/contracts/coordination/Periods.sol index 9c002c117..c4c2aad30 100644 --- a/contracts/contracts/coordination/Periods.sol +++ b/contracts/contracts/coordination/Periods.sol @@ -20,4 +20,8 @@ contract Periods { function getCurrentPeriod() public view returns (uint256) { return getPeriodForTimestamp(block.timestamp); } + + function getCurrentPaymentPeriod() public view returns (int256) { + return int256(getCurrentPeriod()) - 2; + } } diff --git a/tests/test_penalty_board_compensation.py b/tests/test_penalty_board_compensation.py index 546de7c66..b4d9c42d1 100644 --- a/tests/test_penalty_board_compensation.py +++ b/tests/test_penalty_board_compensation.py @@ -4,9 +4,9 @@ import pytest PERIOD_DURATION = 3600 -FIXED_COMPENSATION = 1000 -REDUCED_FIXED_COMPENSATION_1 = 600 -REDUCED_FIXED_COMPENSATION_2 = 100 +FULL_COMPENSATION = 1000 +REDUCED_COMPENSATION_1 = 600 +REDUCED_COMPENSATION_2 = 100 TOKEN_SUPPLY = 1_000_000 * 10**18 @@ -60,13 +60,15 @@ def penalty_board_comp(project, deployer, informer, chain, mock_taco_app, token, deployer.address, mock_taco_app.address, token.address, - FIXED_COMPENSATION, - REDUCED_FIXED_COMPENSATION_1, - REDUCED_FIXED_COMPENSATION_2, + FULL_COMPENSATION, + REDUCED_COMPENSATION_1, + REDUCED_COMPENSATION_2, fund_holder.address, sender=deployer, ) contract.grantRole(contract.INFORMER_ROLE(), informer.address, sender=deployer) + token.transfer(fund_holder.address, 100 * FULL_COMPENSATION, sender=deployer) + token.approve(contract.address, 2**256 - 1, sender=fund_holder) return contract @@ -105,7 +107,7 @@ def test_penalized_periods_monotonic_append(penalty_board_comp, informer, stakin def test_withdraw_reverts_nothing_to_withdraw_for_stakeless( - penalty_board_comp, mock_taco_app, deployer, beneficiary, accounts + penalty_board_comp, mock_taco_app, deployer, beneficiary, accounts, chain ): """Withdraw reverts with 'Nothing to withdraw' for stakeless staker (no accrual).""" stakeless_provider = accounts[6] @@ -116,17 +118,19 @@ def test_withdraw_reverts_nothing_to_withdraw_for_stakeless( True, # stakeless sender=deployer, ) + chain.pending_timestamp += 2 * PERIOD_DURATION with ape.reverts("Nothing to withdraw"): penalty_board_comp.withdraw(stakeless_provider.address, sender=beneficiary) def test_get_accrued_balance_reflects_periods_without_prior_withdraw( - penalty_board_comp, staking_provider, registered_staker + penalty_board_comp, staking_provider, registered_staker, chain ): """getAccruedBalance returns accrued amount for periods 0..current when staker has not yet withdrawn.""" # At deploy we are in period 0; one period accrues (fixed per period). + chain.pending_timestamp += 2 * PERIOD_DURATION balance = penalty_board_comp.getAccruedBalance(staking_provider.address) - assert balance == FIXED_COMPENSATION + assert balance == FULL_COMPENSATION def test_accrual_with_no_penalties_gives_positive_balance( @@ -160,15 +164,8 @@ def test_withdraw_sends_tokens_to_beneficiary( deployer, ): """Withdraw sends tokens to beneficiary, not to msg.sender (fails until C3 implements transfer).""" - # Fund the fundHolder and approve PenaltyBoard (token was minted to deployer) - token.transfer(fund_holder.address, FIXED_COMPENSATION * 10, sender=deployer) - token.approve( - penalty_board_comp.address, - 2**256 - 1, - sender=fund_holder, - ) # Advance so there is accrual (when implemented) - chain.pending_timestamp += PERIOD_DURATION + chain.pending_timestamp += 3 * PERIOD_DURATION before = token.balanceOf(beneficiary.address) # Beneficiary calls withdraw for staking_provider penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) @@ -208,11 +205,9 @@ def test_first_withdrawal_accrues_from_period_zero( ): """First withdrawal (sentinel: never accrued) accrues from period 0 to current; full amount to beneficiary.""" num_periods = 4 - chain.pending_timestamp += num_periods * PERIOD_DURATION + chain.pending_timestamp += (num_periods + 2) * PERIOD_DURATION # Periods 0..4 → 5 periods - expected = (num_periods + 1) * FIXED_COMPENSATION - token.transfer(fund_holder.address, expected * 2, sender=deployer) - token.approve(penalty_board_comp.address, 2**256 - 1, sender=fund_holder) + expected = (num_periods + 1) * FULL_COMPENSATION before = token.balanceOf(beneficiary.address) penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) @@ -222,16 +217,13 @@ def test_first_withdrawal_accrues_from_period_zero( def test_withdraw_reverts_when_nothing_left_after_prior_withdraw( penalty_board_comp, - token, - fund_holder, beneficiary, staking_provider, registered_staker, - deployer, + chain, ): """After a full withdraw, second withdraw without time advance reverts with Nothing to withdraw.""" - token.transfer(fund_holder.address, FIXED_COMPENSATION * 2, sender=deployer) - token.approve(penalty_board_comp.address, 2**256 - 1, sender=fund_holder) + chain.pending_timestamp += 2 * PERIOD_DURATION penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) with ape.reverts("Nothing to withdraw"): penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) @@ -249,18 +241,41 @@ def test_many_periods_full_accrual( ): """Many periods with no penalties: full accrual (numPeriods * fixed per period).""" n = 10 - chain.pending_timestamp += n * PERIOD_DURATION + chain.pending_timestamp += (n + 2) * PERIOD_DURATION # Periods 0..n → n+1 periods - expected = (n + 1) * FIXED_COMPENSATION - token.transfer(fund_holder.address, expected + 1000, sender=deployer) - token.approve(penalty_board_comp.address, 2**256 - 1, sender=fund_holder) - + expected = (n + 1) * FULL_COMPENSATION before = token.balanceOf(beneficiary.address) penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) after = token.balanceOf(beneficiary.address) assert after - before == expected +@pytest.mark.parametrize( + "periods", + [ + # T F F F F T -> 0 1 2 3 4 3 -> F F R1 R2 0 R2 + { + "penalized_providers": [1, 2, 3, 4], + "expected": [2, 1, 2], + "num_periods": 5, + "intermediate_withdraws": [], + }, + # F T T T F T -> 1 1 1 1 1 1 -> F F F F F F + { + "penalized_providers": [0, 4], + "expected": [6, 0, 0], + "num_periods": 5, + "intermediate_withdraws": [2, 3, 4], + }, + # T T T F F F T -> 0 0 0 1 2 3 3 -> F F F F R1 R2 R2 + { + "penalized_providers": [3, 4, 5], + "expected": [4, 1, 2], + "num_periods": 6, + "intermediate_withdraws": [2, 3, 4, 5], + }, + ], +) def test_penalty_in_range_reduces_compensation( penalty_board_comp, token, @@ -271,38 +286,48 @@ def test_penalty_in_range_reduces_compensation( chain, deployer, informer, + periods, ): """A penalty in period k affects k..k+PENALTY_WINDOW_PERIODS; only unaffected periods get full compensation.""" - # Advance to period 1, set penalty for period 1 (current), then advance to period 5. - chain.pending_timestamp += PERIOD_DURATION - penalty_board_comp.setPenalizedProvidersForPeriod( - [staking_provider.address], 1, sender=informer - ) - chain.pending_timestamp += PERIOD_DURATION - penalty_board_comp.setPenalizedProvidersForPeriod( - [staking_provider.address], 2, sender=informer - ) - chain.pending_timestamp += PERIOD_DURATION - penalty_board_comp.setPenalizedProvidersForPeriod( - [staking_provider.address], 3, sender=informer - ) - chain.pending_timestamp += PERIOD_DURATION - penalty_board_comp.setPenalizedProvidersForPeriod( - [staking_provider.address], 4, sender=informer - ) - chain.pending_timestamp += PERIOD_DURATION - # chain.pending_timestamp += 4 * PERIOD_DURATION - # Now current period is 5. Accrual: periods 0..5. Penalty at 1 affects 1,2,3,4. So only 0 and 5 get full. - # 0 1 2 3 4 3 -> F F R1 R2 0 R2 - expected = ( - 2 * FIXED_COMPENSATION + REDUCED_FIXED_COMPENSATION_1 + 2 * REDUCED_FIXED_COMPENSATION_2 - ) - token.transfer(fund_holder.address, 10 * expected, sender=deployer) - token.approve(penalty_board_comp.address, 2**256 - 1, sender=fund_holder) - - assert penalty_board_comp.getAccruedBalance(staking_provider.address) == expected + penalized_providers = periods["penalized_providers"] + expected = periods["expected"] + num_periods = periods["num_periods"] + intermediate_withdraws = periods["intermediate_withdraws"] before = token.balanceOf(beneficiary.address) + + penalty_index = 0 + withdraw_index = 0 + accrued_balance = 0 + for current_period in range(num_periods): + if ( + withdraw_index < len(intermediate_withdraws) + and intermediate_withdraws[withdraw_index] == current_period + ): + accrued_balance += penalty_board_comp.getAccruedBalance(staking_provider.address) + penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) + withdraw_index += 1 + if ( + penalty_index < len(penalized_providers) + and penalized_providers[penalty_index] == current_period + ): + penalty_board_comp.setPenalizedProvidersForPeriod( + [staking_provider.address], current_period, sender=informer + ) + penalty_index += 1 + + chain.pending_timestamp += PERIOD_DURATION + + chain.pending_timestamp += 2 * PERIOD_DURATION + + expected_compensation = ( + expected[0] * FULL_COMPENSATION + + expected[1] * REDUCED_COMPENSATION_1 + + expected[2] * REDUCED_COMPENSATION_2 + ) + + accrued_balance += penalty_board_comp.getAccruedBalance(staking_provider.address) + assert accrued_balance == expected_compensation penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) after = token.balanceOf(beneficiary.address) - assert after - before == expected + assert after - before == expected_compensation From 7e49157a5657787f7c57922e2b86f3278fc899bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Wed, 25 Mar 2026 20:20:24 +0100 Subject: [PATCH 11/17] Improve tests for compensation --- tests/test_penalty_board_compensation.py | 147 ++++++++++++++--------- 1 file changed, 90 insertions(+), 57 deletions(-) diff --git a/tests/test_penalty_board_compensation.py b/tests/test_penalty_board_compensation.py index b4d9c42d1..d88a9edf6 100644 --- a/tests/test_penalty_board_compensation.py +++ b/tests/test_penalty_board_compensation.py @@ -1,5 +1,3 @@ -"""TDD tests for PenaltyBoard compensation. Some pass with stubs; others fail until C3 implementation.""" - import ape import pytest @@ -8,6 +6,7 @@ REDUCED_COMPENSATION_1 = 600 REDUCED_COMPENSATION_2 = 100 TOKEN_SUPPLY = 1_000_000 * 10**18 +MAX_UINT256 = 2**256 - 1 @pytest.fixture(scope="module") @@ -36,10 +35,20 @@ def beneficiary(accounts): @pytest.fixture(scope="module") -def other_account(accounts): +def owner(accounts): return accounts[5] +@pytest.fixture(scope="module") +def other_account(accounts): + return accounts[6] + + +@pytest.fixture(scope="module") +def stakeless_provider(accounts): + return accounts[7] + + @pytest.fixture(scope="module") def mock_taco_app(project, deployer): return project.MockTACoForPenaltyBoard.deploy(sender=deployer) @@ -68,29 +77,47 @@ def penalty_board_comp(project, deployer, informer, chain, mock_taco_app, token, ) contract.grantRole(contract.INFORMER_ROLE(), informer.address, sender=deployer) token.transfer(fund_holder.address, 100 * FULL_COMPENSATION, sender=deployer) - token.approve(contract.address, 2**256 - 1, sender=fund_holder) + token.approve(contract.address, MAX_UINT256, sender=fund_holder) return contract @pytest.fixture -def registered_staker(mock_taco_app, staking_provider, deployer, beneficiary): - """Register staking_provider in mock TACo: owner=deployer, beneficiary=beneficiary, not stakeless.""" +def registered_staker(mock_taco_app, staking_provider, owner, beneficiary, deployer): + """Register staking_provider in mock TACo: owner=owner, beneficiary=beneficiary, not stakeless.""" mock_taco_app.setRoles( staking_provider.address, - deployer.address, + owner.address, beneficiary.address, False, sender=deployer, ) + return staking_provider + + +@pytest.fixture +def registered_stakeless(mock_taco_app, stakeless_provider, owner, beneficiary, deployer): + """Register stakeless_provider in mock TACo: owner=owner, beneficiary=beneficiary, stakeless.""" + mock_taco_app.setRoles( + stakeless_provider.address, + owner.address, + beneficiary.address, + True, # stakeless + sender=deployer, + ) + return stakeless_provider -def test_penalized_periods_by_staker_updated(penalty_board_comp, informer, staking_provider): +def test_penalized_periods_by_staker_updated(penalty_board_comp, informer, staking_provider, other_account): """setPenalizedProvidersForPeriod(provs, period) causes penalizedPeriodsByStaker[prov] to include period.""" current = penalty_board_comp.getCurrentPeriod() provs = [staking_provider.address] penalty_board_comp.setPenalizedProvidersForPeriod(provs, current, sender=informer) assert penalty_board_comp.getPenalizedPeriodsByStaker(staking_provider.address) == [current] + provs = [other_account.address] + penalty_board_comp.setPenalizedProvidersForPeriod(provs, current, sender=informer) + assert penalty_board_comp.getPenalizedPeriodsByStaker(staking_provider.address) == [current] + def test_penalized_periods_monotonic_append(penalty_board_comp, informer, staking_provider, chain): """Calling setPenalizedProvidersForPeriod for different periods appends to each staker's list (monotonic).""" @@ -107,52 +134,55 @@ def test_penalized_periods_monotonic_append(penalty_board_comp, informer, stakin def test_withdraw_reverts_nothing_to_withdraw_for_stakeless( - penalty_board_comp, mock_taco_app, deployer, beneficiary, accounts, chain + penalty_board_comp, registered_stakeless, chain, beneficiary ): """Withdraw reverts with 'Nothing to withdraw' for stakeless staker (no accrual).""" - stakeless_provider = accounts[6] - mock_taco_app.setRoles( - stakeless_provider.address, - deployer.address, - beneficiary.address, - True, # stakeless - sender=deployer, - ) chain.pending_timestamp += 2 * PERIOD_DURATION with ape.reverts("Nothing to withdraw"): - penalty_board_comp.withdraw(stakeless_provider.address, sender=beneficiary) + penalty_board_comp.withdraw(registered_stakeless.address, sender=beneficiary) def test_get_accrued_balance_reflects_periods_without_prior_withdraw( - penalty_board_comp, staking_provider, registered_staker, chain + penalty_board_comp, registered_staker, chain ): """getAccruedBalance returns accrued amount for periods 0..current when staker has not yet withdrawn.""" - # At deploy we are in period 0; one period accrues (fixed per period). - chain.pending_timestamp += 2 * PERIOD_DURATION - balance = penalty_board_comp.getAccruedBalance(staking_provider.address) + # At deploy we are in period 0, so there's nothing to accrue yet (accrual lag is 2 periods) + balance = penalty_board_comp.getAccruedBalance(registered_staker.address) + assert penalty_board_comp.getCurrentPeriod() == 0 + assert balance == 0 + + # After advancing 1 period, there's still nothing to accrue (accrual lag is 2 periods) + chain.pending_timestamp += PERIOD_DURATION + assert penalty_board_comp.getCurrentPeriod() == 1 + balance = penalty_board_comp.getAccruedBalance(registered_staker.address) + assert balance == 0 + + # After advancing 1 more period (2 total), we should accrue for period 0 + chain.pending_timestamp += PERIOD_DURATION + assert penalty_board_comp.getCurrentPeriod() == 2 + balance = penalty_board_comp.getAccruedBalance(registered_staker.address) assert balance == FULL_COMPENSATION def test_accrual_with_no_penalties_gives_positive_balance( penalty_board_comp, chain, staking_provider, registered_staker ): - """After advancing periods with no penalties, staker should have positive accrued balance (fails until C3).""" + """After advancing periods with no penalties, staker should have positive accrued balance""" # Advance 2 periods so there is something to accrue chain.pending_timestamp += 2 * PERIOD_DURATION balance = penalty_board_comp.getAccruedBalance(staking_provider.address) - assert ( - balance > 0 - ), "Accrual not implemented: getAccruedBalance should be > 0 after 2 periods with no penalties" + assert balance == FULL_COMPENSATION, "Incorrect accrual: getAccruedBalance should be > 0 after 2 periods with no penalties" -def test_only_owner_provider_beneficiary_can_withdraw( +def test_unauthorized_caller_cant_withdraw( penalty_board_comp, registered_staker, other_account, staking_provider ): - """Only owner, staking provider, or beneficiary may call withdraw; others revert (C3: Unauthorized; stub: Not implemented).""" + """Only owner, staking provider, or beneficiary may call withdraw; others revert""" with ape.reverts(): penalty_board_comp.withdraw(staking_provider.address, sender=other_account) +@pytest.mark.parametrize("caller", ["owner", "staking_provider", "beneficiary"]) def test_withdraw_sends_tokens_to_beneficiary( penalty_board_comp, token, @@ -161,47 +191,42 @@ def test_withdraw_sends_tokens_to_beneficiary( staking_provider, registered_staker, chain, - deployer, + caller, + request ): - """Withdraw sends tokens to beneficiary, not to msg.sender (fails until C3 implements transfer).""" + """Withdraw sends tokens to beneficiary, not to msg.sender""" + # We parametrize over the 3 authorized caller types to confirm that tokens are sent to beneficiary regardless of which authorized caller calls withdraw + caller = request.getfixturevalue(caller) # Advance so there is accrual (when implemented) chain.pending_timestamp += 3 * PERIOD_DURATION before = token.balanceOf(beneficiary.address) - # Beneficiary calls withdraw for staking_provider - penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) + # Authorized caller calls withdraw for staking_provider + penalty_board_comp.withdraw(staking_provider.address, sender=caller) after = token.balanceOf(beneficiary.address) assert after > before, "Withdraw should send tokens to beneficiary" def test_stakeless_staker_gets_zero_compensation( - penalty_board_comp, mock_taco_app, chain, deployer, beneficiary, accounts + penalty_board_comp, registered_stakeless, chain ): - """Stakeless staker accrues 0 (fails until C3 implements stakeless check).""" - stakeless_provider = accounts[6] - mock_taco_app.setRoles( - stakeless_provider.address, - deployer.address, - beneficiary.address, - True, # stakeless - sender=deployer, - ) - chain.pending_timestamp += 2 * PERIOD_DURATION - balance = penalty_board_comp.getAccruedBalance(stakeless_provider.address) + """Stakeless staker accrues 0""" + chain.pending_timestamp += 10 * PERIOD_DURATION + balance = penalty_board_comp.getAccruedBalance(registered_stakeless.address) assert balance == 0, "Stakeless staker should have 0 accrued compensation" -# --- C4: Edge cases --- +#TODO: Test that stakers that are released or unstaked, stop accruing compensation + +# --- Edge cases --- def test_first_withdrawal_accrues_from_period_zero( penalty_board_comp, token, - fund_holder, beneficiary, staking_provider, registered_staker, chain, - deployer, ): """First withdrawal (sentinel: never accrued) accrues from period 0 to current; full amount to beneficiary.""" num_periods = 4 @@ -229,25 +254,35 @@ def test_withdraw_reverts_when_nothing_left_after_prior_withdraw( penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) -def test_many_periods_full_accrual( +@pytest.mark.parametrize("cadence", [(12,), (2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12)]) +def test_many_periods_full_accrual_regardless_of_withdraw_cadence( penalty_board_comp, token, - fund_holder, beneficiary, staking_provider, registered_staker, chain, - deployer, + cadence ): """Many periods with no penalties: full accrual (numPeriods * fixed per period).""" n = 10 - chain.pending_timestamp += (n + 2) * PERIOD_DURATION + last_period = n + 2 + first_period = 2 + + chain.pending_timestamp += 2 * PERIOD_DURATION # Periods 0..n → n+1 periods expected = (n + 1) * FULL_COMPENSATION - before = token.balanceOf(beneficiary.address) - penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) - after = token.balanceOf(beneficiary.address) - assert after - before == expected + initial_balance = token.balanceOf(beneficiary.address) + for withdraw_period in cadence: + current_period = penalty_board_comp.getCurrentPeriod() + offset = withdraw_period - current_period + if offset > 0: + chain.pending_timestamp += offset * PERIOD_DURATION + penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) + + assert penalty_board_comp.getCurrentPeriod() == last_period + final_balance = token.balanceOf(beneficiary.address) + assert final_balance - initial_balance == expected @pytest.mark.parametrize( @@ -279,12 +314,10 @@ def test_many_periods_full_accrual( def test_penalty_in_range_reduces_compensation( penalty_board_comp, token, - fund_holder, beneficiary, staking_provider, registered_staker, chain, - deployer, informer, periods, ): From 69a4d8d1f3deab64fe0a1dd8a8f72365e07136c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Wed, 25 Mar 2026 21:08:27 +0100 Subject: [PATCH 12/17] setPenalizedProvidersForPeriod -> addPenalizedProvidersForPeriod --- .../contracts/coordination/PenaltyBoard.sol | 5 ++-- tests/test_infraction.py | 2 +- tests/test_penalty_board.py | 30 +++++++++---------- tests/test_penalty_board_compensation.py | 14 ++++----- 4 files changed, 26 insertions(+), 25 deletions(-) diff --git a/contracts/contracts/coordination/PenaltyBoard.sol b/contracts/contracts/coordination/PenaltyBoard.sol index b8db0b0b1..10eb995c4 100644 --- a/contracts/contracts/coordination/PenaltyBoard.sol +++ b/contracts/contracts/coordination/PenaltyBoard.sol @@ -85,11 +85,12 @@ contract PenaltyBoard is Periods, AccessControl { } /** - * @notice Set the list of penalized staking providers for a period. + * @notice Add the list of staking providers as penalized for a period. * @param provs Staking provider addresses to record as penalized for the period. * @param period Period index (must be current or previous period). + * @dev Note that passing an empty list doesn't clear penalties for that period; this is only additive. */ - function setPenalizedProvidersForPeriod( + function addPenalizedProvidersForPeriod( address[] calldata provs, uint256 period ) external onlyRole(INFORMER_ROLE) { diff --git a/tests/test_infraction.py b/tests/test_infraction.py index 752f21b42..44d5fb652 100644 --- a/tests/test_infraction.py +++ b/tests/test_infraction.py @@ -277,7 +277,7 @@ def test_infraction_collector_and_penalty_board_together( # Same period still (period duration is 1 week; we advanced ~2000s). Informer records # penalized providers for this period on PenaltyBoard. current_period = penalty_board.getCurrentPeriod() - penalty_board.setPenalizedProvidersForPeriod(failing_providers, current_period, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(failing_providers, current_period, sender=informer) for provider in failing_providers: periods = penalty_board.getPenalizedPeriodsByStaker(provider) assert periods == [current_period] diff --git a/tests/test_penalty_board.py b/tests/test_penalty_board.py index ccf5f43a7..7716f201b 100644 --- a/tests/test_penalty_board.py +++ b/tests/test_penalty_board.py @@ -62,41 +62,41 @@ def test_constructor_admin_required(project, deployer, chain): def test_only_informer_can_set_penalized_providers( penalty_board, deployer, informer, other_account ): - """Only an account with INFORMER_ROLE can call setPenalizedProvidersForPeriod.""" + """Only an account with INFORMER_ROLE can call addPenalizedProvidersForPeriod.""" current = penalty_board.getCurrentPeriod() provs = [other_account.address] with ape.reverts(): - penalty_board.setPenalizedProvidersForPeriod(provs, current, sender=deployer) + penalty_board.addPenalizedProvidersForPeriod(provs, current, sender=deployer) - penalty_board.setPenalizedProvidersForPeriod(provs, current, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(provs, current, sender=informer) assert penalty_board.getPenalizedPeriodsByStaker(other_account.address) == [current] def test_period_must_be_current_or_previous(penalty_board, chain, informer, other_account): - """setPenalizedProvidersForPeriod accepts only current or previous period.""" + """addPenalizedProvidersForPeriod accepts only current or previous period.""" provs = [other_account.address] current = penalty_board.getCurrentPeriod() # At deploy, genesis = chain.pending_timestamp so current is 0. Period 1 is invalid. with ape.reverts("Invalid period"): - penalty_board.setPenalizedProvidersForPeriod(provs, current + 1, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(provs, current + 1, sender=informer) - penalty_board.setPenalizedProvidersForPeriod(provs, current, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(provs, current, sender=informer) assert penalty_board.getPenalizedPeriodsByStaker(other_account.address) == [current] # Advance into period 1; then both 0 and 1 are valid. chain.pending_timestamp += PERIOD_DURATION assert penalty_board.getCurrentPeriod() == 1 - penalty_board.setPenalizedProvidersForPeriod(provs, 0, sender=informer) - penalty_board.setPenalizedProvidersForPeriod(provs, 1, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(provs, 0, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(provs, 1, sender=informer) periods = penalty_board.getPenalizedPeriodsByStaker(other_account.address) assert periods == [current, 0, 1] # Period 2 is invalid (not current or previous). with ape.reverts("Invalid period"): - penalty_board.setPenalizedProvidersForPeriod(provs, 2, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(provs, 2, sender=informer) def test_staker_penalty_list_updated_for_all_providers( @@ -106,22 +106,22 @@ def test_staker_penalty_list_updated_for_all_providers( current = penalty_board.getCurrentPeriod() provs = [accounts[3].address, accounts[4].address, other_account.address] - penalty_board.setPenalizedProvidersForPeriod(provs, current, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(provs, current, sender=informer) for addr in provs: assert penalty_board.getPenalizedPeriodsByStaker(addr) == [current] def test_setting_again_appends_for_new_providers(penalty_board, informer, other_account, accounts): - """Calling setPenalizedProvidersForPeriod again for the same period appends penalties for new providers.""" + """Calling addPenalizedProvidersForPeriod again for the same period appends penalties for new providers.""" current = penalty_board.getCurrentPeriod() first = [accounts[5].address, accounts[6].address] - penalty_board.setPenalizedProvidersForPeriod(first, current, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(first, current, sender=informer) for addr in first: assert penalty_board.getPenalizedPeriodsByStaker(addr) == [current] second = [other_account.address] - penalty_board.setPenalizedProvidersForPeriod(second, current, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(second, current, sender=informer) for addr in first: assert penalty_board.getPenalizedPeriodsByStaker(addr) == [current] assert penalty_board.getPenalizedPeriodsByStaker(other_account.address) == [current] @@ -134,8 +134,8 @@ def test_period_zero_only_when_current_is_zero(penalty_board, informer, other_ac pytest.skip("chain already past period 0") provs = [other_account.address] - penalty_board.setPenalizedProvidersForPeriod(provs, 0, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(provs, 0, sender=informer) assert penalty_board.getPenalizedPeriodsByStaker(other_account.address) == [0] with ape.reverts("Invalid period"): - penalty_board.setPenalizedProvidersForPeriod(provs, 1, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(provs, 1, sender=informer) diff --git a/tests/test_penalty_board_compensation.py b/tests/test_penalty_board_compensation.py index d88a9edf6..c243ac456 100644 --- a/tests/test_penalty_board_compensation.py +++ b/tests/test_penalty_board_compensation.py @@ -108,25 +108,25 @@ def registered_stakeless(mock_taco_app, stakeless_provider, owner, beneficiary, def test_penalized_periods_by_staker_updated(penalty_board_comp, informer, staking_provider, other_account): - """setPenalizedProvidersForPeriod(provs, period) causes penalizedPeriodsByStaker[prov] to include period.""" + """addPenalizedProvidersForPeriod(provs, period) causes penalizedPeriodsByStaker[prov] to include period.""" current = penalty_board_comp.getCurrentPeriod() provs = [staking_provider.address] - penalty_board_comp.setPenalizedProvidersForPeriod(provs, current, sender=informer) + penalty_board_comp.addPenalizedProvidersForPeriod(provs, current, sender=informer) assert penalty_board_comp.getPenalizedPeriodsByStaker(staking_provider.address) == [current] provs = [other_account.address] - penalty_board_comp.setPenalizedProvidersForPeriod(provs, current, sender=informer) + penalty_board_comp.addPenalizedProvidersForPeriod(provs, current, sender=informer) assert penalty_board_comp.getPenalizedPeriodsByStaker(staking_provider.address) == [current] def test_penalized_periods_monotonic_append(penalty_board_comp, informer, staking_provider, chain): - """Calling setPenalizedProvidersForPeriod for different periods appends to each staker's list (monotonic).""" + """Calling addPenalizedProvidersForPeriod for different periods appends to each staker's list (monotonic).""" current = penalty_board_comp.getCurrentPeriod() provs = [staking_provider.address] - penalty_board_comp.setPenalizedProvidersForPeriod(provs, current, sender=informer) + penalty_board_comp.addPenalizedProvidersForPeriod(provs, current, sender=informer) chain.pending_timestamp += PERIOD_DURATION assert penalty_board_comp.getCurrentPeriod() == current + 1 - penalty_board_comp.setPenalizedProvidersForPeriod(provs, current + 1, sender=informer) + penalty_board_comp.addPenalizedProvidersForPeriod(provs, current + 1, sender=informer) assert penalty_board_comp.getPenalizedPeriodsByStaker(staking_provider.address) == [ current, current + 1, @@ -344,7 +344,7 @@ def test_penalty_in_range_reduces_compensation( penalty_index < len(penalized_providers) and penalized_providers[penalty_index] == current_period ): - penalty_board_comp.setPenalizedProvidersForPeriod( + penalty_board_comp.addPenalizedProvidersForPeriod( [staking_provider.address], current_period, sender=informer ) penalty_index += 1 From 1ad2da20c92e7807503ccb60d089d1e4412834a3 Mon Sep 17 00:00:00 2001 From: Viktoriia Zotova Date: Fri, 27 Mar 2026 13:55:01 -0400 Subject: [PATCH 13/17] Extract penalty information to a struct --- contracts/contracts/TACoApplication.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/contracts/TACoApplication.sol b/contracts/contracts/TACoApplication.sol index 837330911..e64b23c01 100644 --- a/contracts/contracts/TACoApplication.sol +++ b/contracts/contracts/TACoApplication.sol @@ -122,6 +122,7 @@ contract TACoApplication is ITACoChildToRoot, OwnableUpgradeable { uint256 public immutable minOperatorSeconds; IERC20 public immutable token; + // PenaltyBoard public immutable penaltyBoard; ITACoRootToChild public childApplication; address private _adjudicator; From 1aa8bfdcb99bcadc6a849a1ba84e6d739364c851 Mon Sep 17 00:00:00 2001 From: Viktoriia Zotova Date: Fri, 27 Mar 2026 14:38:18 -0400 Subject: [PATCH 14/17] Adds communication between TACo app and PenaltyBoard to know when to turn on/off rewards --- contracts/contracts/TACoApplication.sol | 20 ++++++- .../ITACoApplicationForPenaltyBoard.sol | 5 ++ .../contracts/coordination/PenaltyBoard.sol | 59 +++++++++++++------ contracts/test/MockTACoForPenaltyBoard.sol | 11 +++- 4 files changed, 72 insertions(+), 23 deletions(-) diff --git a/contracts/contracts/TACoApplication.sol b/contracts/contracts/TACoApplication.sol index e64b23c01..f688962fc 100644 --- a/contracts/contracts/TACoApplication.sol +++ b/contracts/contracts/TACoApplication.sol @@ -9,6 +9,7 @@ import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "@openzeppelin-upgradeable/contracts/access/OwnableUpgradeable.sol"; import "./coordination/ITACoRootToChild.sol"; import "./coordination/ITACoChildToRoot.sol"; +import "./coordination/PenaltyBoard.sol"; /** * @title TACo Application @@ -122,7 +123,7 @@ contract TACoApplication is ITACoChildToRoot, OwnableUpgradeable { uint256 public immutable minOperatorSeconds; IERC20 public immutable token; - // PenaltyBoard public immutable penaltyBoard; + PenaltyBoard public immutable penaltyBoard; ITACoRootToChild public childApplication; address private _adjudicator; @@ -149,13 +150,20 @@ contract TACoApplication is ITACoChildToRoot, OwnableUpgradeable { * @param _token T token contract * @param _minimumAuthorization Amount of minimum allowable authorization * @param _minOperatorSeconds Min amount of seconds while an operator can't be changed + * @param _penaltyBoard PenaltyBoard contract */ - constructor(IERC20 _token, uint96 _minimumAuthorization, uint256 _minOperatorSeconds) { + constructor( + IERC20 _token, + uint96 _minimumAuthorization, + uint256 _minOperatorSeconds, + PenaltyBoard _penaltyBoard + ) { uint256 totalSupply = _token.totalSupply(); require(totalSupply > 0, "Wrong input parameters"); minimumAuthorization = _minimumAuthorization; token = _token; minOperatorSeconds = _minOperatorSeconds; + penaltyBoard = _penaltyBoard; _disableInitializers(); } @@ -440,6 +448,7 @@ contract TACoApplication is ITACoChildToRoot, OwnableUpgradeable { info.operatorStartTimestamp = uint64(block.timestamp); emit OperatorBonded(_stakingProvider, _operator, previousOperator, block.timestamp); + penaltyBoard.computeRewards(_stakingProvider); info.operatorConfirmed = false; childApplication.updateOperator(_stakingProvider, _operator); } @@ -461,6 +470,7 @@ contract TACoApplication is ITACoChildToRoot, OwnableUpgradeable { StakingProviderInfo storage info = stakingProviderInfo[stakingProvider]; if (!info.operatorConfirmed) { + penaltyBoard.enableRewards(stakingProvider); info.operatorConfirmed = true; emit OperatorConfirmed(stakingProvider, _operator); } @@ -472,6 +482,7 @@ contract TACoApplication is ITACoChildToRoot, OwnableUpgradeable { * @notice Resets operator confirmation */ function _releaseOperator(address _stakingProvider) internal { + penaltyBoard.computeRewards(_stakingProvider); StakingProviderInfo storage info = stakingProviderInfo[_stakingProvider]; _stakingProviderFromOperator[info.operator] = address(0); info.operator = address(0); @@ -556,4 +567,9 @@ contract TACoApplication is ITACoChildToRoot, OwnableUpgradeable { emit StakelessProviderAdded(_stakingProvider); token.safeTransfer(info.owner, authorized); } + + function isEligibleForReward(address _stakingProvider) external view returns (bool) { + StakingProviderInfo storage info = stakingProviderInfo[_stakingProvider]; + return !info.stakeless && info.operatorConfirmed; + } } diff --git a/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol b/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol index 644c812a2..8f1027fc8 100644 --- a/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol +++ b/contracts/contracts/coordination/ITACoApplicationForPenaltyBoard.sol @@ -30,4 +30,9 @@ interface ITACoApplicationForPenaltyBoard { * If not present on TACoApplication, implementation may return false. */ function isStakeless(address stakingProvider) external view returns (bool); + + /** + * @notice Returns true if staking provider eligible for reward. + */ + function isEligibleForReward(address _stakingProvider) external view returns (bool); } diff --git a/contracts/contracts/coordination/PenaltyBoard.sol b/contracts/contracts/coordination/PenaltyBoard.sol index 10eb995c4..409959d65 100644 --- a/contracts/contracts/coordination/PenaltyBoard.sol +++ b/contracts/contracts/coordination/PenaltyBoard.sol @@ -33,19 +33,19 @@ contract PenaltyBoard is Periods, AccessControl { uint256 public immutable fixedCompensationPerPeriod2Penalties; uint256 public immutable fixedCompensationPerPeriod3Penalties; - /** - * @notice Staker-centric penalty storage: list of period indices this staker was penalized in - * (monotonic append; periods are kept as uint256). - */ - mapping(address staker => uint256[]) public penalizedPeriodsByStaker; - /// Number of periods a penalty affects (penalty in period k affects k..k+PENALTY_WINDOW_PERIODS inclusive). uint256 private constant PENALTY_WINDOW_PERIODS = 3; + struct Staker { + uint256 accruedBalance; + // Accrued compensation state (lazy accrual via _computeAccruedSinceLast). + // _lastAccruedPeriodPlusOne: 0 = never accrued (start from period 0); else start next accrual at this period. + uint256 lastAccruedPeriodPlusOne; + // Staker-centric penalty storage: list of period indices this staker was penalized in + // (monotonic append; periods are kept as uint256). + uint256[] penalizedPeriods; + } - // Accrued compensation state (lazy accrual via _computeAccruedSinceLast). - // _lastAccruedPeriodPlusOne: 0 = never accrued (start from period 0); else start next accrual at this period. - mapping(address staker => uint256) private _accruedBalance; - mapping(address staker => uint256) private _lastAccruedPeriodPlusOne; + mapping(address staker => Staker stakerStruct) public stakers; /** * @param genesisTime Start of period 0. @@ -77,11 +77,16 @@ contract PenaltyBoard is Periods, AccessControl { fixedCompensationPerPeriod3Penalties = _fixedCompensationPerPeriod3Penalties; } + modifier onlyTACoApplication() { + require(msg.sender == address(tacoApplication), "Not TACo app"); + _; + } + /** * @notice Returns the full list of period indices a staker was penalized in. */ function getPenalizedPeriodsByStaker(address staker) external view returns (uint256[] memory) { - return penalizedPeriodsByStaker[staker]; + return stakers[staker].penalizedPeriods; } /** @@ -98,7 +103,7 @@ contract PenaltyBoard is Periods, AccessControl { require(period == current || (current > 0 && period == current - 1), "Invalid period"); for (uint256 i = 0; i < provs.length; i++) { - penalizedPeriodsByStaker[provs[i]].push(period); + stakers[provs[i]].penalizedPeriods.push(period); } emit PenalizedProvidersSet(period, provs); @@ -121,7 +126,7 @@ contract PenaltyBoard is Periods, AccessControl { uint256 delta = current >= 0 ? _computeAccruedSinceLast(stakingProvider, uint256(current)) : 0; - return _accruedBalance[stakingProvider] + delta; + return stakers[stakingProvider].accruedBalance + delta; } /** @@ -152,20 +157,36 @@ contract PenaltyBoard is Periods, AccessControl { int256 current = getCurrentPaymentPeriod(); + Staker storage staker = stakers[stakingProvider]; uint256 delta = current >= 0 ? _computeAccruedSinceLast(stakingProvider, uint256(current)) : 0; - uint256 amount = _accruedBalance[stakingProvider] + delta; + uint256 amount = staker.accruedBalance + delta; require(amount > 0, "Nothing to withdraw"); - _accruedBalance[stakingProvider] = 0; - _lastAccruedPeriodPlusOne[stakingProvider] = uint256(current) + 1; + staker.accruedBalance = 0; + staker.lastAccruedPeriodPlusOne = uint256(current) + 1; address beneficiary = beneficiaryAddress; compensationToken.safeTransferFrom(fundHolder, beneficiary, amount); } + function computeRewards(address stakingProvider) external onlyTACoApplication { + int256 current = getCurrentPaymentPeriod(); + + Staker storage staker = stakers[stakingProvider]; + uint256 delta = current >= 0 + ? _computeAccruedSinceLast(stakingProvider, uint256(current)) + : 0; + staker.accruedBalance += delta; + staker.lastAccruedPeriodPlusOne = uint256(current) + 1; + } + + function enableRewards(address stakingProvider) external onlyTACoApplication { + stakers[stakingProvider].lastAccruedPeriodPlusOne = getCurrentPeriod(); + } + function _computeAccruedSinceLast( address stakingProvider, uint256 currentPeriod @@ -178,11 +199,11 @@ contract PenaltyBoard is Periods, AccessControl { return 0; } - if (tacoApplication.isStakeless(stakingProvider)) { + if (!tacoApplication.isEligibleForReward(stakingProvider)) { return 0; } - uint256 startPeriod = _lastAccruedPeriodPlusOne[stakingProvider]; // 0 = never accrued → start at 0 + uint256 startPeriod = stakers[stakingProvider].lastAccruedPeriodPlusOne; // 0 = never accrued → start at 0 if (startPeriod > currentPeriod) { return 0; @@ -245,7 +266,7 @@ contract PenaltyBoard is Periods, AccessControl { uint256 fromPeriod, uint256 toPeriod ) internal view returns (uint256[] memory) { - uint256[] storage all = penalizedPeriodsByStaker[stakingProvider]; + uint256[] storage all = stakers[stakingProvider].penalizedPeriods; uint256 len = all.length; if (len == 0 || fromPeriod > toPeriod) { return new uint256[](0); diff --git a/contracts/test/MockTACoForPenaltyBoard.sol b/contracts/test/MockTACoForPenaltyBoard.sol index 86c3f5b37..2ee61bbed 100644 --- a/contracts/test/MockTACoForPenaltyBoard.sol +++ b/contracts/test/MockTACoForPenaltyBoard.sol @@ -13,6 +13,7 @@ contract MockTACoForPenaltyBoard is ITACoApplicationForPenaltyBoard { address owner; address payable beneficiary; bool isStakeless; + bool eligibleForReward; } mapping(address stakingProvider => StakerRoles) private _roles; @@ -21,12 +22,14 @@ contract MockTACoForPenaltyBoard is ITACoApplicationForPenaltyBoard { address stakingProvider, address owner, address payable beneficiary, - bool isStakeless + bool isStakeless, + bool eligibleForReward ) external { _roles[stakingProvider] = StakerRoles({ owner: owner, beneficiary: beneficiary, - isStakeless: isStakeless + isStakeless: isStakeless, + eligibleForReward: eligibleForReward }); } @@ -44,4 +47,8 @@ contract MockTACoForPenaltyBoard is ITACoApplicationForPenaltyBoard { function isStakeless(address stakingProvider) external view returns (bool) { return _roles[stakingProvider].isStakeless; } + + function isEligibleForReward(address stakingProvider) external view returns (bool) { + return _roles[stakingProvider].eligibleForReward; + } } From 70f02a72326b17c483ea214061b651b5db563a3f Mon Sep 17 00:00:00 2001 From: Viktoriia Zotova Date: Wed, 1 Apr 2026 14:07:00 -0400 Subject: [PATCH 15/17] Adds start period for reward, tests, fixes --- contracts/contracts/TACoApplication.sol | 19 ++- .../contracts/coordination/PenaltyBoard.sol | 40 +++-- contracts/contracts/coordination/Periods.sol | 7 +- contracts/test/MockTACoForPenaltyBoard.sol | 15 ++ contracts/test/TACoApplicationTestSet.sol | 12 ++ tests/application/conftest.py | 7 + tests/application/test_authorization.py | 16 +- tests/application/test_operator.py | 30 +++- tests/test_infraction.py | 3 +- tests/test_penalty_board.py | 20 ++- tests/test_penalty_board_compensation.py | 155 +++++++++++++++--- tests/test_periods.py | 8 +- tests/test_signing_coordinator.py | 3 + 13 files changed, 264 insertions(+), 71 deletions(-) diff --git a/contracts/contracts/TACoApplication.sol b/contracts/contracts/TACoApplication.sol index f688962fc..32c1c42f9 100644 --- a/contracts/contracts/TACoApplication.sol +++ b/contracts/contracts/TACoApplication.sol @@ -123,7 +123,6 @@ contract TACoApplication is ITACoChildToRoot, OwnableUpgradeable { uint256 public immutable minOperatorSeconds; IERC20 public immutable token; - PenaltyBoard public immutable penaltyBoard; ITACoRootToChild public childApplication; address private _adjudicator; @@ -144,26 +143,20 @@ contract TACoApplication is ITACoChildToRoot, OwnableUpgradeable { // mapping(address => bool) public stakingProviderReleased; // mapping(address => bool) public allowList; + PenaltyBoard public penaltyBoard; /** * @notice Constructor sets address of token contract and parameters for staking * @param _token T token contract * @param _minimumAuthorization Amount of minimum allowable authorization * @param _minOperatorSeconds Min amount of seconds while an operator can't be changed - * @param _penaltyBoard PenaltyBoard contract */ - constructor( - IERC20 _token, - uint96 _minimumAuthorization, - uint256 _minOperatorSeconds, - PenaltyBoard _penaltyBoard - ) { + constructor(IERC20 _token, uint96 _minimumAuthorization, uint256 _minOperatorSeconds) { uint256 totalSupply = _token.totalSupply(); require(totalSupply > 0, "Wrong input parameters"); minimumAuthorization = _minimumAuthorization; token = _token; minOperatorSeconds = _minOperatorSeconds; - penaltyBoard = _penaltyBoard; _disableInitializers(); } @@ -194,6 +187,14 @@ contract TACoApplication is ITACoChildToRoot, OwnableUpgradeable { childApplication = _childApplication; } + /** + * @notice Set contract for compensation + */ + function setPenaltyBoard(PenaltyBoard _penaltyBoard) external onlyOwner { + require(address(_penaltyBoard).code.length > 0, "PenaltyBoard must be contract"); + penaltyBoard = _penaltyBoard; + } + //------------------------Staking------------------------------ function initializeStake( diff --git a/contracts/contracts/coordination/PenaltyBoard.sol b/contracts/contracts/coordination/PenaltyBoard.sol index 409959d65..2f10f2be8 100644 --- a/contracts/contracts/coordination/PenaltyBoard.sol +++ b/contracts/contracts/coordination/PenaltyBoard.sol @@ -32,6 +32,7 @@ contract PenaltyBoard is Periods, AccessControl { uint256 public immutable fixedCompensationPerPeriod; uint256 public immutable fixedCompensationPerPeriod2Penalties; uint256 public immutable fixedCompensationPerPeriod3Penalties; + uint256 public immutable startPeriodForRewards; /// Number of periods a penalty affects (penalty in period k affects k..k+PENALTY_WINDOW_PERIODS inclusive). uint256 private constant PENALTY_WINDOW_PERIODS = 3; @@ -55,6 +56,7 @@ contract PenaltyBoard is Periods, AccessControl { * @param _token Compensation token. Pass address(0) when compensation disabled. * @param _fixedCompensationPerPeriod Fixed amount per period (0 when disabled). * @param _fundHolder Holder of tokens for payouts. Pass address(0) when compensation disabled. + * @param _startPeriodForRewards First period when rewards will be calculated. */ constructor( uint256 genesisTime, @@ -65,7 +67,8 @@ contract PenaltyBoard is Periods, AccessControl { uint256 _fixedCompensationPerPeriod, uint256 _fixedCompensationPerPeriod2Penalties, uint256 _fixedCompensationPerPeriod3Penalties, - address _fundHolder + address _fundHolder, + uint256 _startPeriodForRewards ) Periods(genesisTime, periodDuration) { require(admin != address(0), "Admin required"); _grantRole(DEFAULT_ADMIN_ROLE, admin); @@ -75,6 +78,7 @@ contract PenaltyBoard is Periods, AccessControl { fixedCompensationPerPeriod = _fixedCompensationPerPeriod; fixedCompensationPerPeriod2Penalties = _fixedCompensationPerPeriod2Penalties; fixedCompensationPerPeriod3Penalties = _fixedCompensationPerPeriod3Penalties; + startPeriodForRewards = _startPeriodForRewards; } modifier onlyTACoApplication() { @@ -100,7 +104,7 @@ contract PenaltyBoard is Periods, AccessControl { uint256 period ) external onlyRole(INFORMER_ROLE) { uint256 current = getCurrentPeriod(); - require(period == current || (current > 0 && period == current - 1), "Invalid period"); + require(period == current || period == current - 1, "Invalid period"); for (uint256 i = 0; i < provs.length; i++) { stakers[provs[i]].penalizedPeriods.push(period); @@ -122,10 +126,8 @@ contract PenaltyBoard is Periods, AccessControl { return 0; } - int256 current = getCurrentPaymentPeriod(); - uint256 delta = current >= 0 - ? _computeAccruedSinceLast(stakingProvider, uint256(current)) - : 0; + uint256 current = getCurrentPaymentPeriod(); + uint256 delta = _computeAccruedSinceLast(stakingProvider, current); return stakers[stakingProvider].accruedBalance + delta; } @@ -155,34 +157,37 @@ contract PenaltyBoard is Periods, AccessControl { revert("Nothing to withdraw"); } - int256 current = getCurrentPaymentPeriod(); + uint256 current = getCurrentPaymentPeriod(); Staker storage staker = stakers[stakingProvider]; - uint256 delta = current >= 0 - ? _computeAccruedSinceLast(stakingProvider, uint256(current)) - : 0; + uint256 delta = _computeAccruedSinceLast(stakingProvider, current); uint256 amount = staker.accruedBalance + delta; require(amount > 0, "Nothing to withdraw"); staker.accruedBalance = 0; - staker.lastAccruedPeriodPlusOne = uint256(current) + 1; + staker.lastAccruedPeriodPlusOne = current + 1; address beneficiary = beneficiaryAddress; compensationToken.safeTransferFrom(fundHolder, beneficiary, amount); } + /** + * @notice Compute and save rewards for the specified staker + * @dev Can be used anytime. It has to be called before turning off reward + */ function computeRewards(address stakingProvider) external onlyTACoApplication { - int256 current = getCurrentPaymentPeriod(); + uint256 current = getCurrentPaymentPeriod(); Staker storage staker = stakers[stakingProvider]; - uint256 delta = current >= 0 - ? _computeAccruedSinceLast(stakingProvider, uint256(current)) - : 0; + uint256 delta = _computeAccruedSinceLast(stakingProvider, current); staker.accruedBalance += delta; - staker.lastAccruedPeriodPlusOne = uint256(current) + 1; + staker.lastAccruedPeriodPlusOne = current + 1; } + /** + * @notice Enable rewards before turning on + */ function enableRewards(address stakingProvider) external onlyTACoApplication { stakers[stakingProvider].lastAccruedPeriodPlusOne = getCurrentPeriod(); } @@ -204,6 +209,9 @@ contract PenaltyBoard is Periods, AccessControl { } uint256 startPeriod = stakers[stakingProvider].lastAccruedPeriodPlusOne; // 0 = never accrued → start at 0 + if (startPeriod < startPeriodForRewards) { + startPeriod = startPeriodForRewards; + } if (startPeriod > currentPeriod) { return 0; diff --git a/contracts/contracts/coordination/Periods.sol b/contracts/contracts/coordination/Periods.sol index c4c2aad30..86a1ac03d 100644 --- a/contracts/contracts/coordination/Periods.sol +++ b/contracts/contracts/coordination/Periods.sol @@ -3,6 +3,8 @@ pragma solidity ^0.8.0; contract Periods { + uint256 public constant REWARD_DELAY_PERIODS = 2; + uint256 public immutable genesisTime; uint256 public immutable periodDuration; @@ -10,6 +12,7 @@ contract Periods { require(_periodDuration > 0, "Invalid period duration"); genesisTime = _genesisTime; periodDuration = _periodDuration; + require(getCurrentPeriod() >= REWARD_DELAY_PERIODS, "genesisTime must be in the past"); } function getPeriodForTimestamp(uint256 timestamp) public view returns (uint256) { @@ -21,7 +24,7 @@ contract Periods { return getPeriodForTimestamp(block.timestamp); } - function getCurrentPaymentPeriod() public view returns (int256) { - return int256(getCurrentPeriod()) - 2; + function getCurrentPaymentPeriod() public view returns (uint256) { + return getCurrentPeriod() - REWARD_DELAY_PERIODS; } } diff --git a/contracts/test/MockTACoForPenaltyBoard.sol b/contracts/test/MockTACoForPenaltyBoard.sol index 2ee61bbed..597578da5 100644 --- a/contracts/test/MockTACoForPenaltyBoard.sol +++ b/contracts/test/MockTACoForPenaltyBoard.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.0; import "../contracts/coordination/ITACoApplicationForPenaltyBoard.sol"; +import "../contracts/coordination/PenaltyBoard.sol"; /** * @notice Mock TACoApplication for PenaltyBoard compensation tests. @@ -18,6 +19,12 @@ contract MockTACoForPenaltyBoard is ITACoApplicationForPenaltyBoard { mapping(address stakingProvider => StakerRoles) private _roles; + PenaltyBoard public penaltyBoard; + + function setPenaltyBoard(PenaltyBoard _penaltyBoard) external { + penaltyBoard = _penaltyBoard; + } + function setRoles( address stakingProvider, address owner, @@ -51,4 +58,12 @@ contract MockTACoForPenaltyBoard is ITACoApplicationForPenaltyBoard { function isEligibleForReward(address stakingProvider) external view returns (bool) { return _roles[stakingProvider].eligibleForReward; } + + function computeRewards(address stakingProvider) external { + penaltyBoard.computeRewards(stakingProvider); + } + + function enableRewards(address stakingProvider) external { + penaltyBoard.enableRewards(stakingProvider); + } } diff --git a/contracts/test/TACoApplicationTestSet.sol b/contracts/test/TACoApplicationTestSet.sol index 532280314..97423a9a2 100644 --- a/contracts/test/TACoApplicationTestSet.sol +++ b/contracts/test/TACoApplicationTestSet.sol @@ -60,3 +60,15 @@ contract ChildApplicationForTACoApplicationMock { } } } + +contract PenaltyBoardForTACoApplicationMock { + mapping(address stakingProvider => bool isRewardEnabled) public isRewardEnabled; + + function computeRewards(address stakingProvider) external { + isRewardEnabled[stakingProvider] = false; + } + + function enableRewards(address stakingProvider) external { + isRewardEnabled[stakingProvider] = true; + } +} diff --git a/tests/application/conftest.py b/tests/application/conftest.py index b69165eb3..07ccde8be 100644 --- a/tests/application/conftest.py +++ b/tests/application/conftest.py @@ -81,3 +81,10 @@ def child_application(project, creator, taco_application): ) taco_application.setChildApplication(contract.address, sender=creator) return contract + + +@pytest.fixture() +def penalty_board(project, creator, taco_application): + contract = project.PenaltyBoardForTACoApplicationMock.deploy(sender=creator) + taco_application.setPenaltyBoard(contract.address, sender=creator) + return contract diff --git a/tests/application/test_authorization.py b/tests/application/test_authorization.py index 9713a8e71..6179c3d7d 100644 --- a/tests/application/test_authorization.py +++ b/tests/application/test_authorization.py @@ -25,7 +25,7 @@ MIN_AUTHORIZATION = Web3.to_wei(40_000, "ether") -def test_initialize_stake(accounts, token, taco_application, child_application): +def test_initialize_stake(accounts, token, taco_application, child_application, penalty_board): """ Tests for authorization method: initializeStake """ @@ -58,6 +58,8 @@ def test_initialize_stake(accounts, token, taco_application, child_application): assert taco_application.isAuthorized(staking_provider) assert token.balanceOf(taco_application.address) == value assert token.balanceOf(owner) == 0 + assert not taco_application.isEligibleForReward(staking_provider) + assert not penalty_board.isRewardEnabled(staking_provider) # Check that all events are emitted events = [event for event in tx.events if event.event_name == "Staked"] @@ -122,7 +124,7 @@ def test_request_unstake_callers(accounts, taco_application, token, child_applic assert taco_application.eligibleStake(staking_provider_3) == 0 -def test_request_unstake(accounts, taco_application, child_application, token): +def test_request_unstake(accounts, taco_application, child_application, token, penalty_board): """ Tests for authorization method: requestUnstake """ @@ -183,7 +185,7 @@ def test_request_unstake(accounts, taco_application, child_application, token): assert child_application.stakingProviderReleased(staking_provider_2) -def test_release(accounts, token, taco_application, child_application): +def test_release(accounts, token, taco_application, child_application, penalty_board): """ Tests for authorization method: release+approveUnstake """ @@ -271,7 +273,7 @@ def test_release(accounts, token, taco_application, child_application): assert token.balanceOf(staking_provider_3) == 0 -def test_child_sync(accounts, taco_application, child_application, token, chain): +def test_child_sync(accounts, taco_application, child_application, token, penalty_board): """ Tests for x-chain method: manualChildSynchronization """ @@ -324,7 +326,7 @@ def test_child_sync(accounts, taco_application, child_application, token, chain) ] -def test_add_stakeless_provider(accounts, taco_application, child_application, chain): +def test_add_stakeless_provider(accounts, taco_application, child_application, penalty_board): """ Tests for authorization method: addStakelessProvider """ @@ -349,6 +351,8 @@ def test_add_stakeless_provider(accounts, taco_application, child_application, c assert taco_application.authorizedStake(staking_provider) == value assert child_application.stakingProviderInfo(staking_provider) == (value, 0) assert taco_application.isAuthorized(staking_provider) + assert not taco_application.isEligibleForReward(staking_provider) + assert not penalty_board.isRewardEnabled(staking_provider) # Check that all events are emitted events = [event for event in tx.events if event.event_name == "StakelessProviderAdded"] @@ -359,6 +363,8 @@ def test_add_stakeless_provider(accounts, taco_application, child_application, c taco_application.bondOperator(staking_provider, owner, sender=staking_provider) child_application.confirmOperatorAddress(staking_provider, sender=owner) + assert not taco_application.isEligibleForReward(staking_provider) + assert not penalty_board.isRewardEnabled(staking_provider) with ape.reverts("A provider can't be an operator for another provider"): taco_application.addStakelessProvider(owner, owner, sender=creator) diff --git a/tests/application/test_operator.py b/tests/application/test_operator.py index d141c1591..1292d9ead 100644 --- a/tests/application/test_operator.py +++ b/tests/application/test_operator.py @@ -24,7 +24,7 @@ MIN_OPERATOR_SECONDS = 24 * 60 * 60 -def test_bond_operator(accounts, token, taco_application, child_application, chain): +def test_bond_operator(accounts, token, taco_application, child_application, penalty_board, chain): ( creator, staking_provider_1, @@ -97,6 +97,8 @@ def test_bond_operator(accounts, token, taco_application, child_application, cha 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 not taco_application.isEligibleForReward(staking_provider_3) + assert not penalty_board.isRewardEnabled(staking_provider_3) events = [event for event in tx.events if event.event_name == "OperatorBonded"] assert events == [ @@ -118,6 +120,8 @@ def test_bond_operator(accounts, token, taco_application, child_application, cha assert taco_application.isOperatorConfirmed(operator1) assert child_application.stakingProviderToOperator(staking_provider_3) == operator1 assert child_application.operatorToStakingProvider(operator1) == staking_provider_3 + assert taco_application.isEligibleForReward(staking_provider_3) + assert penalty_board.isRewardEnabled(staking_provider_3) # After confirmation operator is becoming active all_locked, staking_providers = taco_application.getActiveStakingProviders(0, 0) @@ -157,6 +161,8 @@ def test_bond_operator(accounts, token, taco_application, child_application, cha assert taco_application.stakingProviders(0) == staking_provider_3 assert child_application.stakingProviderToOperator(staking_provider_3) == ZERO_ADDRESS assert child_application.operatorToStakingProvider(operator1) == ZERO_ADDRESS + assert not taco_application.isEligibleForReward(staking_provider_3) + assert not penalty_board.isRewardEnabled(staking_provider_3) # Resetting operator removes from active list before next confirmation all_locked, staking_providers = taco_application.getActiveStakingProviders(0, 0) @@ -184,6 +190,8 @@ def test_bond_operator(accounts, token, taco_application, child_application, cha assert taco_application.stakingProviders(0) == staking_provider_3 assert child_application.stakingProviderToOperator(staking_provider_3) == operator2 assert child_application.operatorToStakingProvider(operator2) == staking_provider_3 + assert not taco_application.isEligibleForReward(staking_provider_3) + assert not penalty_board.isRewardEnabled(staking_provider_3) events = [event for event in tx.events if event.event_name == "OperatorBonded"] assert events == [ @@ -205,6 +213,8 @@ def test_bond_operator(accounts, token, taco_application, child_application, cha assert taco_application.stakingProviderInfo(staking_provider_3)[CONFIRMATION_SLOT] assert child_application.stakingProviderToOperator(staking_provider_3) == operator2 assert child_application.operatorToStakingProvider(operator2) == staking_provider_3 + assert taco_application.isEligibleForReward(staking_provider_3) + assert penalty_board.isRewardEnabled(staking_provider_3) # Another staking provider can bond a free operator tx = taco_application.bondOperator(staking_provider_4, operator1, sender=staking_provider_4) @@ -217,6 +227,8 @@ def test_bond_operator(accounts, token, taco_application, child_application, cha assert taco_application.stakingProviders(1) == staking_provider_4 assert child_application.stakingProviderToOperator(staking_provider_4) == operator1 assert child_application.operatorToStakingProvider(operator1) == staking_provider_4 + assert not taco_application.isEligibleForReward(staking_provider_4) + assert not penalty_board.isRewardEnabled(staking_provider_4) events = [event for event in tx.events if event.event_name == "OperatorBonded"] assert events == [ @@ -238,6 +250,8 @@ def test_bond_operator(accounts, token, taco_application, child_application, cha assert taco_application.stakingProviderInfo(staking_provider_4)[CONFIRMATION_SLOT] assert child_application.stakingProviderToOperator(staking_provider_4) == operator1 assert child_application.operatorToStakingProvider(operator1) == staking_provider_4 + assert taco_application.isEligibleForReward(staking_provider_4) + assert penalty_board.isRewardEnabled(staking_provider_4) chain.pending_timestamp += min_operator_seconds tx = taco_application.bondOperator(staking_provider_4, operator3, sender=staking_provider_4) @@ -253,6 +267,8 @@ def test_bond_operator(accounts, token, taco_application, child_application, cha assert child_application.stakingProviderToOperator(staking_provider_4) == operator3 assert child_application.operatorToStakingProvider(operator1) == ZERO_ADDRESS assert child_application.operatorToStakingProvider(operator3) == staking_provider_4 + assert not taco_application.isEligibleForReward(staking_provider_4) + assert not penalty_board.isRewardEnabled(staking_provider_4) # Resetting operator removes from active list before next confirmation all_locked, staking_providers = taco_application.getActiveStakingProviders(1, 0) @@ -313,7 +329,9 @@ def test_bond_operator(accounts, token, taco_application, child_application, cha assert taco_application.operatorToStakingProvider(operator2) == ZERO_ADDRESS -def test_confirm_address(accounts, token, taco_application, child_application, chain): +def test_confirm_address( + accounts, token, taco_application, child_application, penalty_board, chain +): creator, staking_provider, operator, *everyone_else = accounts[0:] min_operator_seconds = MIN_OPERATOR_SECONDS @@ -323,6 +341,8 @@ def test_confirm_address(accounts, token, taco_application, child_application, c # 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) + assert not taco_application.isEligibleForReward(staking_provider) + assert not penalty_board.isRewardEnabled(staking_provider) token.transfer(staking_provider, MIN_AUTHORIZATION, sender=creator) token.approve(taco_application.address, MIN_AUTHORIZATION, sender=staking_provider) @@ -333,9 +353,13 @@ def test_confirm_address(accounts, token, taco_application, child_application, c # Bond operator and make confirmation chain.pending_timestamp += min_operator_seconds taco_application.bondOperator(staking_provider, operator, sender=staking_provider) + assert not taco_application.isEligibleForReward(staking_provider) + assert not penalty_board.isRewardEnabled(staking_provider) tx = child_application.confirmOperatorAddress(operator, sender=operator) assert taco_application.isOperatorConfirmed(operator) assert taco_application.stakingProviderInfo(staking_provider)[CONFIRMATION_SLOT] + assert taco_application.isEligibleForReward(staking_provider) + assert penalty_board.isRewardEnabled(staking_provider) events = [event for event in tx.events if event.event_name == "OperatorConfirmed"] assert events == [ @@ -346,3 +370,5 @@ def test_confirm_address(accounts, token, taco_application, child_application, c child_application.confirmOperatorAddress(operator, sender=operator) assert taco_application.isOperatorConfirmed(operator) assert taco_application.stakingProviderInfo(staking_provider)[CONFIRMATION_SLOT] + assert taco_application.isEligibleForReward(staking_provider) + assert penalty_board.isRewardEnabled(staking_provider) diff --git a/tests/test_infraction.py b/tests/test_infraction.py index 44d5fb652..c44321cec 100644 --- a/tests/test_infraction.py +++ b/tests/test_infraction.py @@ -136,7 +136,7 @@ def infraction_collector(project, deployer, coordinator): def penalty_board(project, deployer, informer, chain): """PenaltyBoard with genesis at current chain time and INFORMER_ROLE granted to informer. Period duration is one week so ritual timeout still lands in period 0. No compensation.""" - genesis_time = chain.pending_timestamp + genesis_time = chain.pending_timestamp - 2 * PENALTY_BOARD_PERIOD_DURATION zero = "0x0000000000000000000000000000000000000000" contract = project.PenaltyBoard.deploy( genesis_time, @@ -148,6 +148,7 @@ def penalty_board(project, deployer, informer, chain): 0, 0, zero, + 0, sender=deployer, ) contract.grantRole(contract.INFORMER_ROLE(), informer.address, sender=deployer) diff --git a/tests/test_penalty_board.py b/tests/test_penalty_board.py index 7716f201b..51cbdc1de 100644 --- a/tests/test_penalty_board.py +++ b/tests/test_penalty_board.py @@ -25,7 +25,7 @@ def other_account(accounts): @pytest.fixture def penalty_board(project, deployer, informer, chain): """PenaltyBoard with genesis at current chain time and INFORMER_ROLE granted to informer (no compensation).""" - genesis_time = chain.pending_timestamp + genesis_time = chain.pending_timestamp - 2 * PERIOD_DURATION contract = project.PenaltyBoard.deploy( genesis_time, PERIOD_DURATION, @@ -36,6 +36,7 @@ def penalty_board(project, deployer, informer, chain): 0, 0, ZERO_ADDRESS, + 0, sender=deployer, ) contract.grantRole(contract.INFORMER_ROLE(), informer.address, sender=deployer) @@ -43,7 +44,7 @@ def penalty_board(project, deployer, informer, chain): def test_constructor_admin_required(project, deployer, chain): - genesis_time = chain.pending_timestamp + genesis_time = chain.pending_timestamp - 2 * PERIOD_DURATION with ape.reverts("Admin required"): project.PenaltyBoard.deploy( genesis_time, @@ -55,6 +56,7 @@ def test_constructor_admin_required(project, deployer, chain): 0, 0, ZERO_ADDRESS, + 0, sender=deployer, ) @@ -85,18 +87,18 @@ def test_period_must_be_current_or_previous(penalty_board, chain, informer, othe penalty_board.addPenalizedProvidersForPeriod(provs, current, sender=informer) assert penalty_board.getPenalizedPeriodsByStaker(other_account.address) == [current] - # Advance into period 1; then both 0 and 1 are valid. + # Advance into period 1; then both 2 and 3 are valid. chain.pending_timestamp += PERIOD_DURATION - assert penalty_board.getCurrentPeriod() == 1 + assert penalty_board.getCurrentPeriod() == 3 - penalty_board.addPenalizedProvidersForPeriod(provs, 0, sender=informer) - penalty_board.addPenalizedProvidersForPeriod(provs, 1, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(provs, 2, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(provs, 3, sender=informer) periods = penalty_board.getPenalizedPeriodsByStaker(other_account.address) - assert periods == [current, 0, 1] + assert periods == [current, 2, 3] - # Period 2 is invalid (not current or previous). + # Period 4 is invalid (not current or previous). with ape.reverts("Invalid period"): - penalty_board.addPenalizedProvidersForPeriod(provs, 2, sender=informer) + penalty_board.addPenalizedProvidersForPeriod(provs, 4, sender=informer) def test_staker_penalty_list_updated_for_all_providers( diff --git a/tests/test_penalty_board_compensation.py b/tests/test_penalty_board_compensation.py index c243ac456..ead941a37 100644 --- a/tests/test_penalty_board_compensation.py +++ b/tests/test_penalty_board_compensation.py @@ -7,6 +7,7 @@ REDUCED_COMPENSATION_2 = 100 TOKEN_SUPPLY = 1_000_000 * 10**18 MAX_UINT256 = 2**256 - 1 +REWARD_DELAY_PERIODS = 2 @pytest.fixture(scope="module") @@ -62,7 +63,7 @@ def token(project, deployer): @pytest.fixture def penalty_board_comp(project, deployer, informer, chain, mock_taco_app, token, fund_holder): """PenaltyBoard with compensation enabled (7-arg constructor).""" - genesis_time = chain.pending_timestamp + genesis_time = chain.pending_timestamp - 2 * PERIOD_DURATION contract = project.PenaltyBoard.deploy( genesis_time, PERIOD_DURATION, @@ -73,11 +74,13 @@ def penalty_board_comp(project, deployer, informer, chain, mock_taco_app, token, REDUCED_COMPENSATION_1, REDUCED_COMPENSATION_2, fund_holder.address, + REWARD_DELAY_PERIODS, sender=deployer, ) contract.grantRole(contract.INFORMER_ROLE(), informer.address, sender=deployer) token.transfer(fund_holder.address, 100 * FULL_COMPENSATION, sender=deployer) token.approve(contract.address, MAX_UINT256, sender=fund_holder) + mock_taco_app.setPenaltyBoard(contract.address, sender=deployer) return contract @@ -89,6 +92,7 @@ def registered_staker(mock_taco_app, staking_provider, owner, beneficiary, deplo owner.address, beneficiary.address, False, + True, sender=deployer, ) return staking_provider @@ -102,12 +106,29 @@ def registered_stakeless(mock_taco_app, stakeless_provider, owner, beneficiary, owner.address, beneficiary.address, True, # stakeless + False, sender=deployer, ) return stakeless_provider -def test_penalized_periods_by_staker_updated(penalty_board_comp, informer, staking_provider, other_account): +@pytest.fixture +def registered_no_reward(mock_taco_app, stakeless_provider, owner, beneficiary, deployer): + """Register stakeless_provider in mock TACo: owner=owner, beneficiary=beneficiary, stakeless.""" + mock_taco_app.setRoles( + stakeless_provider.address, + owner.address, + beneficiary.address, + False, + False, + sender=deployer, + ) + return stakeless_provider + + +def test_penalized_periods_by_staker_updated( + penalty_board_comp, informer, staking_provider, other_account +): """addPenalizedProvidersForPeriod(provs, period) causes penalizedPeriodsByStaker[prov] to include period.""" current = penalty_board_comp.getCurrentPeriod() provs = [staking_provider.address] @@ -137,29 +158,36 @@ def test_withdraw_reverts_nothing_to_withdraw_for_stakeless( penalty_board_comp, registered_stakeless, chain, beneficiary ): """Withdraw reverts with 'Nothing to withdraw' for stakeless staker (no accrual).""" - chain.pending_timestamp += 2 * PERIOD_DURATION with ape.reverts("Nothing to withdraw"): penalty_board_comp.withdraw(registered_stakeless.address, sender=beneficiary) +def test_withdraw_reverts_nothing_to_withdraw_for_no_reward( + penalty_board_comp, registered_no_reward, chain, beneficiary +): + """Withdraw reverts with 'Nothing to withdraw' for stakeless staker (no accrual).""" + with ape.reverts("Nothing to withdraw"): + penalty_board_comp.withdraw(registered_no_reward.address, sender=beneficiary) + + def test_get_accrued_balance_reflects_periods_without_prior_withdraw( penalty_board_comp, registered_staker, chain ): """getAccruedBalance returns accrued amount for periods 0..current when staker has not yet withdrawn.""" # At deploy we are in period 0, so there's nothing to accrue yet (accrual lag is 2 periods) balance = penalty_board_comp.getAccruedBalance(registered_staker.address) - assert penalty_board_comp.getCurrentPeriod() == 0 + assert penalty_board_comp.getCurrentPeriod() == 2 assert balance == 0 # After advancing 1 period, there's still nothing to accrue (accrual lag is 2 periods) chain.pending_timestamp += PERIOD_DURATION - assert penalty_board_comp.getCurrentPeriod() == 1 + assert penalty_board_comp.getCurrentPeriod() == 3 balance = penalty_board_comp.getAccruedBalance(registered_staker.address) assert balance == 0 # After advancing 1 more period (2 total), we should accrue for period 0 chain.pending_timestamp += PERIOD_DURATION - assert penalty_board_comp.getCurrentPeriod() == 2 + assert penalty_board_comp.getCurrentPeriod() == 4 balance = penalty_board_comp.getAccruedBalance(registered_staker.address) assert balance == FULL_COMPENSATION @@ -171,7 +199,9 @@ def test_accrual_with_no_penalties_gives_positive_balance( # Advance 2 periods so there is something to accrue chain.pending_timestamp += 2 * PERIOD_DURATION balance = penalty_board_comp.getAccruedBalance(staking_provider.address) - assert balance == FULL_COMPENSATION, "Incorrect accrual: getAccruedBalance should be > 0 after 2 periods with no penalties" + assert ( + balance == FULL_COMPENSATION + ), "Incorrect accrual: getAccruedBalance should be > 0 after 2 periods with no penalties" def test_unauthorized_caller_cant_withdraw( @@ -192,7 +222,7 @@ def test_withdraw_sends_tokens_to_beneficiary( registered_staker, chain, caller, - request + request, ): """Withdraw sends tokens to beneficiary, not to msg.sender""" # We parametrize over the 3 authorized caller types to confirm that tokens are sent to beneficiary regardless of which authorized caller calls withdraw @@ -206,16 +236,19 @@ def test_withdraw_sends_tokens_to_beneficiary( assert after > before, "Withdraw should send tokens to beneficiary" -def test_stakeless_staker_gets_zero_compensation( - penalty_board_comp, registered_stakeless, chain -): +def test_stakeless_staker_gets_zero_compensation(penalty_board_comp, registered_stakeless, chain): """Stakeless staker accrues 0""" chain.pending_timestamp += 10 * PERIOD_DURATION balance = penalty_board_comp.getAccruedBalance(registered_stakeless.address) assert balance == 0, "Stakeless staker should have 0 accrued compensation" -#TODO: Test that stakers that are released or unstaked, stop accruing compensation +def test_no_reward_staker_gets_zero_compensation(penalty_board_comp, registered_no_reward, chain): + """Stakeless staker accrues 0""" + chain.pending_timestamp += 10 * PERIOD_DURATION + balance = penalty_board_comp.getAccruedBalance(registered_no_reward.address) + assert balance == 0, "Stakeless staker should have 0 accrued compensation" + # --- Edge cases --- @@ -254,24 +287,18 @@ def test_withdraw_reverts_when_nothing_left_after_prior_withdraw( penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) -@pytest.mark.parametrize("cadence", [(12,), (2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12)]) +@pytest.mark.parametrize("cadence", [(12,), (4, 5, 6, 7, 8, 9, 10, 11, 12)]) def test_many_periods_full_accrual_regardless_of_withdraw_cadence( - penalty_board_comp, - token, - beneficiary, - staking_provider, - registered_staker, - chain, - cadence + penalty_board_comp, token, beneficiary, staking_provider, registered_staker, chain, cadence ): """Many periods with no penalties: full accrual (numPeriods * fixed per period).""" n = 10 last_period = n + 2 first_period = 2 - - chain.pending_timestamp += 2 * PERIOD_DURATION + + # chain.pending_timestamp += 2 * PERIOD_DURATION # Periods 0..n → n+1 periods - expected = (n + 1) * FULL_COMPENSATION + expected = (n - 1) * FULL_COMPENSATION initial_balance = token.balanceOf(beneficiary.address) for withdraw_period in cadence: current_period = penalty_board_comp.getCurrentPeriod() @@ -332,6 +359,7 @@ def test_penalty_in_range_reduces_compensation( penalty_index = 0 withdraw_index = 0 accrued_balance = 0 + first_period = 2 for current_period in range(num_periods): if ( withdraw_index < len(intermediate_withdraws) @@ -345,7 +373,7 @@ def test_penalty_in_range_reduces_compensation( and penalized_providers[penalty_index] == current_period ): penalty_board_comp.addPenalizedProvidersForPeriod( - [staking_provider.address], current_period, sender=informer + [staking_provider.address], current_period + first_period, sender=informer ) penalty_index += 1 @@ -364,3 +392,82 @@ def test_penalty_in_range_reduces_compensation( penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) after = token.balanceOf(beneficiary.address) assert after - before == expected_compensation + + +def test_turning_on_and_of_reward( + penalty_board_comp, + staking_provider, + owner, + beneficiary, + deployer, + token, + registered_no_reward, + mock_taco_app, + chain, +): + balance = penalty_board_comp.getAccruedBalance(staking_provider.address) + assert balance == 0 + + # 2 period, off -> 3 period, on + chain.pending_timestamp += PERIOD_DURATION + mock_taco_app.enableRewards(staking_provider.address, sender=deployer) + mock_taco_app.setRoles( + staking_provider.address, + owner.address, + beneficiary.address, + False, + True, + sender=deployer, + ) + + balance = penalty_board_comp.getAccruedBalance(staking_provider.address) + assert balance == 0 + + # 3 period, on -> 5 period, off + chain.pending_timestamp += 2 * PERIOD_DURATION + mock_taco_app.computeRewards(staking_provider.address, sender=deployer) + mock_taco_app.setRoles( + staking_provider.address, + owner.address, + beneficiary.address, + False, + False, + sender=deployer, + ) + + balance = penalty_board_comp.getAccruedBalance(staking_provider.address) + assert balance == FULL_COMPENSATION + + # 5 period, off -> 6 period, on + chain.pending_timestamp += PERIOD_DURATION + mock_taco_app.enableRewards(staking_provider.address, sender=deployer) + mock_taco_app.setRoles( + staking_provider.address, + owner.address, + beneficiary.address, + False, + True, + sender=deployer, + ) + + # 6 period, on -> 10 period, off + chain.pending_timestamp += 4 * PERIOD_DURATION + mock_taco_app.computeRewards(staking_provider.address, sender=deployer) + mock_taco_app.setRoles( + staking_provider.address, + owner.address, + beneficiary.address, + False, + False, + sender=deployer, + ) + + expected = 4 * FULL_COMPENSATION + balance = penalty_board_comp.getAccruedBalance(staking_provider.address) + assert balance == expected + + chain.pending_timestamp += 4 * PERIOD_DURATION + before = token.balanceOf(beneficiary.address) + penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) + after = token.balanceOf(beneficiary.address) + assert after - before == expected diff --git a/tests/test_periods.py b/tests/test_periods.py index a4c7465f1..b90208e30 100644 --- a/tests/test_periods.py +++ b/tests/test_periods.py @@ -8,12 +8,14 @@ def deployer(accounts): return accounts[0] + @pytest.fixture(scope="module") def periods_deployment(chain, deployer, project): - genesis_time = chain.pending_timestamp + genesis_time = chain.pending_timestamp - 2 * PERIOD_DURATION contract = project.Periods.deploy(genesis_time, PERIOD_DURATION, sender=deployer) return genesis_time, contract + @pytest.fixture(scope="module") def genesis(periods_deployment): genesis_time, _ = periods_deployment @@ -65,6 +67,6 @@ def test_get_current_period(periods, chain, genesis): # assert periods.getCurrentPeriod() == 0 chain.pending_timestamp += duration - assert periods.getCurrentPeriod() == 1 - chain.pending_timestamp += 2 * duration assert periods.getCurrentPeriod() == 3 + chain.pending_timestamp += 2 * duration + assert periods.getCurrentPeriod() == 5 diff --git a/tests/test_signing_coordinator.py b/tests/test_signing_coordinator.py index e23bdef1e..32edc87ed 100644 --- a/tests/test_signing_coordinator.py +++ b/tests/test_signing_coordinator.py @@ -72,6 +72,9 @@ def application(project, oz_dependency, deployer, token, nodes): ) taco_application.setChildApplication(child_application.address, sender=deployer) + penalty_board = project.PenaltyBoardForTACoApplicationMock.deploy(sender=deployer) + taco_application.setPenaltyBoard(penalty_board.address, sender=deployer) + # setup stakes / nodes for n in nodes: token.transfer(n, min_auth, sender=deployer) From 6ec01fc0a169514ffaa44c0ec4914870ef9cecec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Fri, 1 May 2026 17:40:35 +0200 Subject: [PATCH 16/17] Enforce monotonic append to staker's penalty list --- contracts/contracts/coordination/PenaltyBoard.sol | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contracts/contracts/coordination/PenaltyBoard.sol b/contracts/contracts/coordination/PenaltyBoard.sol index 2f10f2be8..27f991b03 100644 --- a/contracts/contracts/coordination/PenaltyBoard.sol +++ b/contracts/contracts/coordination/PenaltyBoard.sol @@ -93,6 +93,7 @@ contract PenaltyBoard is Periods, AccessControl { return stakers[staker].penalizedPeriods; } +// TODO: addPenalizedProvidersForThisPeriod /** * @notice Add the list of staking providers as penalized for a period. * @param provs Staking provider addresses to record as penalized for the period. @@ -107,6 +108,11 @@ contract PenaltyBoard is Periods, AccessControl { require(period == current || period == current - 1, "Invalid period"); for (uint256 i = 0; i < provs.length; i++) { + uint256 len = stakers[provs[i]].penalizedPeriods.length; + // Enforce monotonic append to staker's penalty list (no reordering or duplicates allowed). + if (len > 0) { + require(stakers[provs[i]].penalizedPeriods[len - 1] < period, "Periods must be added in order"); + } stakers[provs[i]].penalizedPeriods.push(period); } From 6e23993c467f111d8c7e23f8a4bd4a86b200ab41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=BA=C3=B1ez?= Date: Mon, 4 May 2026 13:41:02 +0200 Subject: [PATCH 17/17] Add more tests and clarifications --- tests/test_penalty_board_compensation.py | 188 +++++++++++++++++------ 1 file changed, 141 insertions(+), 47 deletions(-) diff --git a/tests/test_penalty_board_compensation.py b/tests/test_penalty_board_compensation.py index ead941a37..4ceec4e4d 100644 --- a/tests/test_penalty_board_compensation.py +++ b/tests/test_penalty_board_compensation.py @@ -1,6 +1,9 @@ import ape import pytest +from ape.utils import ZERO_ADDRESS + + PERIOD_DURATION = 3600 FULL_COMPENSATION = 1000 REDUCED_COMPENSATION_1 = 600 @@ -91,22 +94,22 @@ def registered_staker(mock_taco_app, staking_provider, owner, beneficiary, deplo staking_provider.address, owner.address, beneficiary.address, - False, - True, + False, # not stakeless + True, # rewards enabled sender=deployer, ) return staking_provider @pytest.fixture -def registered_stakeless(mock_taco_app, stakeless_provider, owner, beneficiary, deployer): - """Register stakeless_provider in mock TACo: owner=owner, beneficiary=beneficiary, stakeless.""" +def registered_stakeless(mock_taco_app, stakeless_provider, owner, deployer): + """Register stakeless_provider in mock TACo: owner=owner, stakeless.""" mock_taco_app.setRoles( stakeless_provider.address, owner.address, - beneficiary.address, + ZERO_ADDRESS, # beneficiary is zero address since stakeless staker should not be able to accrue rewards True, # stakeless - False, + False, # rewards disabled since stakeless staker should not be able to accrue rewards sender=deployer, ) return stakeless_provider @@ -114,13 +117,13 @@ def registered_stakeless(mock_taco_app, stakeless_provider, owner, beneficiary, @pytest.fixture def registered_no_reward(mock_taco_app, stakeless_provider, owner, beneficiary, deployer): - """Register stakeless_provider in mock TACo: owner=owner, beneficiary=beneficiary, stakeless.""" + """Register provider in mock TACo: owner=owner, beneficiary=beneficiary, stakeless.""" mock_taco_app.setRoles( stakeless_provider.address, owner.address, beneficiary.address, - False, - False, + False, # not stakeless + False, # rewards disabled since this staker should not be able to accrue rewards sender=deployer, ) return stakeless_provider @@ -154,18 +157,45 @@ def test_penalized_periods_monotonic_append(penalty_board_comp, informer, stakin ] +def test_penalized_periods_disallow_unordered_append(penalty_board_comp, informer, staking_provider, chain): + """Calling addPenalizedProvidersForPeriod for different periods must be in order; reverts if period is not > last period for that staker.""" + first_period = penalty_board_comp.getCurrentPeriod() + provs = [staking_provider.address] + + chain.pending_timestamp += PERIOD_DURATION + assert penalty_board_comp.getCurrentPeriod() == first_period + 1 + + penalty_board_comp.addPenalizedProvidersForPeriod(provs, first_period + 1, sender=informer) + with ape.reverts("Periods must be added in order"): + penalty_board_comp.addPenalizedProvidersForPeriod(provs, first_period, sender=informer) + + +def test_withdraw_reverts_unauthorized_for_stakeless( + penalty_board_comp, registered_stakeless, chain, beneficiary, deployer +): + """Withdraw reverts with 'Unauthorized' for stakeless staker even if beneficiary or owner tries to call withdraw.""" + with ape.reverts("Unauthorized"): + penalty_board_comp.withdraw(registered_stakeless.address, sender=beneficiary) + with ape.reverts("Unauthorized"): + penalty_board_comp.withdraw(registered_stakeless.address, sender=deployer) + + def test_withdraw_reverts_nothing_to_withdraw_for_stakeless( - penalty_board_comp, registered_stakeless, chain, beneficiary + penalty_board_comp, registered_stakeless, chain, beneficiary, deployer, owner ): - """Withdraw reverts with 'Nothing to withdraw' for stakeless staker (no accrual).""" + """Withdraw reverts with 'Nothing to withdraw' for stakeless staker even if beneficiary or owner tries to call withdraw.""" with ape.reverts("Nothing to withdraw"): - penalty_board_comp.withdraw(registered_stakeless.address, sender=beneficiary) + penalty_board_comp.withdraw(registered_stakeless.address, sender=registered_stakeless) + with ape.reverts("Nothing to withdraw"): + penalty_board_comp.withdraw(registered_stakeless.address, sender=owner) + with ape.reverts("Nothing to withdraw"): + penalty_board_comp.withdraw(registered_stakeless.address, sender=owner) def test_withdraw_reverts_nothing_to_withdraw_for_no_reward( penalty_board_comp, registered_no_reward, chain, beneficiary ): - """Withdraw reverts with 'Nothing to withdraw' for stakeless staker (no accrual).""" + """Withdraw reverts with 'Nothing to withdraw' for staker with no rewards.""" with ape.reverts("Nothing to withdraw"): penalty_board_comp.withdraw(registered_no_reward.address, sender=beneficiary) @@ -174,7 +204,7 @@ def test_get_accrued_balance_reflects_periods_without_prior_withdraw( penalty_board_comp, registered_staker, chain ): """getAccruedBalance returns accrued amount for periods 0..current when staker has not yet withdrawn.""" - # At deploy we are in period 0, so there's nothing to accrue yet (accrual lag is 2 periods) + # At deploy we are in period 2, so there's nothing to accrue yet (accrual lag is 2 periods) balance = penalty_board_comp.getAccruedBalance(registered_staker.address) assert penalty_board_comp.getCurrentPeriod() == 2 assert balance == 0 @@ -185,14 +215,14 @@ def test_get_accrued_balance_reflects_periods_without_prior_withdraw( balance = penalty_board_comp.getAccruedBalance(registered_staker.address) assert balance == 0 - # After advancing 1 more period (2 total), we should accrue for period 0 + # After advancing 1 more period (2 total), we should accrue for period 2 chain.pending_timestamp += PERIOD_DURATION assert penalty_board_comp.getCurrentPeriod() == 4 balance = penalty_board_comp.getAccruedBalance(registered_staker.address) assert balance == FULL_COMPENSATION -def test_accrual_with_no_penalties_gives_positive_balance( +def test_accrual_with_no_penalties_gives_full_compensation( penalty_board_comp, chain, staking_provider, registered_staker ): """After advancing periods with no penalties, staker should have positive accrued balance""" @@ -204,11 +234,51 @@ def test_accrual_with_no_penalties_gives_positive_balance( ), "Incorrect accrual: getAccruedBalance should be > 0 after 2 periods with no penalties" +def test_accrual_with_one_penalty_gives_full_compensation( + penalty_board_comp, chain, staking_provider, registered_staker, informer +): + """After advancing periods with 1 penalty, staker should still have full accrued balance since penalty only reduces for more than 1 penalty in a period""" + penalized_providers = [staking_provider.address] + current = penalty_board_comp.getCurrentPeriod() + penalty_board_comp.addPenalizedProvidersForPeriod(penalized_providers, current, sender=informer) + # Advance 2 periods so there is something to accrue + chain.pending_timestamp += 2 * PERIOD_DURATION + balance = penalty_board_comp.getAccruedBalance(staking_provider.address) + assert ( + balance == FULL_COMPENSATION + ), "Incorrect accrual: getAccruedBalance should be > 0 after 2 periods with only 1 penalties" + + +def test_accrual_with_two_penalties_gives_reduced_compensation( + penalty_board_comp, chain, staking_provider, registered_staker, informer +): + """After advancing periods with 2 penalties, staker should have reduced accrued balance since penalty reduces for more than 1 penalty in a period""" + penalized_providers = [staking_provider.address] + current = penalty_board_comp.getCurrentPeriod() + penalty_board_comp.addPenalizedProvidersForPeriod(penalized_providers, current, sender=informer) + # Advance 1 period and add another penalty in the same period + chain.pending_timestamp += PERIOD_DURATION + penalty_board_comp.addPenalizedProvidersForPeriod(penalized_providers, current + 1, sender=informer) + # Advance 1 more period so there is something to accrue + chain.pending_timestamp += PERIOD_DURATION + balance = penalty_board_comp.getAccruedBalance(staking_provider.address) + assert ( + balance == FULL_COMPENSATION + ), "Incorrect accrual: getAccruedBalance should be FULL_COMPENSATION after 2 periods with only 1 penalties" + + chain.pending_timestamp += PERIOD_DURATION + balance = penalty_board_comp.getAccruedBalance(staking_provider.address) + assert ( + balance == FULL_COMPENSATION + REDUCED_COMPENSATION_1 + ), "Incorrect accrual: getAccruedBalance should be > 0 after 2 periods with only 1 penalties" + + + def test_unauthorized_caller_cant_withdraw( penalty_board_comp, registered_staker, other_account, staking_provider ): """Only owner, staking provider, or beneficiary may call withdraw; others revert""" - with ape.reverts(): + with ape.reverts("Unauthorized"): penalty_board_comp.withdraw(staking_provider.address, sender=other_account) @@ -227,7 +297,7 @@ def test_withdraw_sends_tokens_to_beneficiary( """Withdraw sends tokens to beneficiary, not to msg.sender""" # We parametrize over the 3 authorized caller types to confirm that tokens are sent to beneficiary regardless of which authorized caller calls withdraw caller = request.getfixturevalue(caller) - # Advance so there is accrual (when implemented) + # Advance so there is accrual chain.pending_timestamp += 3 * PERIOD_DURATION before = token.balanceOf(beneficiary.address) # Authorized caller calls withdraw for staking_provider @@ -244,10 +314,10 @@ def test_stakeless_staker_gets_zero_compensation(penalty_board_comp, registered_ def test_no_reward_staker_gets_zero_compensation(penalty_board_comp, registered_no_reward, chain): - """Stakeless staker accrues 0""" + """No-reward staker accrues 0""" chain.pending_timestamp += 10 * PERIOD_DURATION balance = penalty_board_comp.getAccruedBalance(registered_no_reward.address) - assert balance == 0, "Stakeless staker should have 0 accrued compensation" + assert balance == 0, "No-reward staker should have 0 accrued compensation" # --- Edge cases --- @@ -261,7 +331,7 @@ def test_first_withdrawal_accrues_from_period_zero( registered_staker, chain, ): - """First withdrawal (sentinel: never accrued) accrues from period 0 to current; full amount to beneficiary.""" + """First withdrawal accrues from period 0 to current; full amount to beneficiary.""" num_periods = 4 chain.pending_timestamp += (num_periods + 2) * PERIOD_DURATION # Periods 0..4 → 5 periods @@ -287,14 +357,13 @@ def test_withdraw_reverts_when_nothing_left_after_prior_withdraw( penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) -@pytest.mark.parametrize("cadence", [(12,), (4, 5, 6, 7, 8, 9, 10, 11, 12)]) +@pytest.mark.parametrize("cadence", [(12,), (4, 5, 6, 7, 8, 9, 10, 11, 12), (1, 7, 12)]) def test_many_periods_full_accrual_regardless_of_withdraw_cadence( penalty_board_comp, token, beneficiary, staking_provider, registered_staker, chain, cadence ): """Many periods with no penalties: full accrual (numPeriods * fixed per period).""" n = 10 last_period = n + 2 - first_period = 2 # chain.pending_timestamp += 2 * PERIOD_DURATION # Periods 0..n → n+1 periods @@ -317,22 +386,28 @@ def test_many_periods_full_accrual_regardless_of_withdraw_cadence( [ # T F F F F T -> 0 1 2 3 4 3 -> F F R1 R2 0 R2 { - "penalized_providers": [1, 2, 3, 4], - "expected": [2, 1, 2], + "penalized_periods": [1, 2, 3, 4], + "expected_full": 2, + "expected_reduced_1": 1, + "expected_reduced_2": 2, "num_periods": 5, "intermediate_withdraws": [], }, # F T T T F T -> 1 1 1 1 1 1 -> F F F F F F { - "penalized_providers": [0, 4], - "expected": [6, 0, 0], + "penalized_periods": [0, 4], + "expected_full": 6, + "expected_reduced_1": 0, + "expected_reduced_2": 0, "num_periods": 5, "intermediate_withdraws": [2, 3, 4], }, # T T T F F F T -> 0 0 0 1 2 3 3 -> F F F F R1 R2 R2 { - "penalized_providers": [3, 4, 5], - "expected": [4, 1, 2], + "penalized_periods": [3, 4, 5], + "expected_full": 4, + "expected_reduced_1": 1, + "expected_reduced_2": 2, "num_periods": 6, "intermediate_withdraws": [2, 3, 4, 5], }, @@ -348,9 +423,17 @@ def test_penalty_in_range_reduces_compensation( informer, periods, ): - """A penalty in period k affects k..k+PENALTY_WINDOW_PERIODS; only unaffected periods get full compensation.""" - penalized_providers = periods["penalized_providers"] - expected = periods["expected"] + """ + This test showcases several features of the compensation contract: + - Penalties in a period reduce compensation for the window between affected period and the next PENALTY_WINDOW_PERIODS periods, but not for other periods + - On each period, accrued amount depends on the penalty composition of the window + - Only periods with 0 or 1 penalties in the window get full compensation; 2 penalties get a reduced compensation, 3 penalties a further reduced compensation, and 4 penalties no compensation. + - Intermediate withdraws don't affect the overal accrued and witdrawn amount since accrued amount is always based on the full history of penalties and withdraw just takes the current accrued balance at the time of withdraw + """ + penalized_periods = periods["penalized_periods"] + expected_full = periods["expected_full"] + expected_reduced_1 = periods["expected_reduced_1"] + expected_reduced_2 = periods["expected_reduced_2"] num_periods = periods["num_periods"] intermediate_withdraws = periods["intermediate_withdraws"] @@ -369,8 +452,8 @@ def test_penalty_in_range_reduces_compensation( penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) withdraw_index += 1 if ( - penalty_index < len(penalized_providers) - and penalized_providers[penalty_index] == current_period + penalty_index < len(penalized_periods) + and penalized_periods[penalty_index] == current_period ): penalty_board_comp.addPenalizedProvidersForPeriod( [staking_provider.address], current_period + first_period, sender=informer @@ -382,9 +465,9 @@ def test_penalty_in_range_reduces_compensation( chain.pending_timestamp += 2 * PERIOD_DURATION expected_compensation = ( - expected[0] * FULL_COMPENSATION - + expected[1] * REDUCED_COMPENSATION_1 - + expected[2] * REDUCED_COMPENSATION_2 + expected_full * FULL_COMPENSATION + + expected_reduced_1 * REDUCED_COMPENSATION_1 + + expected_reduced_2 * REDUCED_COMPENSATION_2 ) accrued_balance += penalty_board_comp.getAccruedBalance(staking_provider.address) @@ -394,7 +477,7 @@ def test_penalty_in_range_reduces_compensation( assert after - before == expected_compensation -def test_turning_on_and_of_reward( +def test_turning_on_and_off_reward( penalty_board_comp, staking_provider, owner, @@ -405,25 +488,28 @@ def test_turning_on_and_of_reward( mock_taco_app, chain, ): + """Turning on and off reward accrual should affect compensation: staker should accrue when reward is on, and not accrue when reward is off.""" balance = penalty_board_comp.getAccruedBalance(staking_provider.address) assert balance == 0 - # 2 period, off -> 3 period, on + assert penalty_board_comp.getCurrentPeriod() == 2 + + # Period 2, off -> period 3, on chain.pending_timestamp += PERIOD_DURATION mock_taco_app.enableRewards(staking_provider.address, sender=deployer) mock_taco_app.setRoles( staking_provider.address, owner.address, beneficiary.address, - False, - True, + False, # Not stakeless + True, # rewards enabled since this staker should accrue rewards for now sender=deployer, ) balance = penalty_board_comp.getAccruedBalance(staking_provider.address) assert balance == 0 - # 3 period, on -> 5 period, off + # Period 3, on -> Period 5, off chain.pending_timestamp += 2 * PERIOD_DURATION mock_taco_app.computeRewards(staking_provider.address, sender=deployer) mock_taco_app.setRoles( @@ -431,14 +517,15 @@ def test_turning_on_and_of_reward( owner.address, beneficiary.address, False, - False, + False, # rewards disabled since this staker should not accrue rewards now sender=deployer, ) + assert penalty_board_comp.getCurrentPeriod() == 5 balance = penalty_board_comp.getAccruedBalance(staking_provider.address) assert balance == FULL_COMPENSATION - # 5 period, off -> 6 period, on + # Period 5, off -> Period 6, on chain.pending_timestamp += PERIOD_DURATION mock_taco_app.enableRewards(staking_provider.address, sender=deployer) mock_taco_app.setRoles( @@ -446,11 +533,15 @@ def test_turning_on_and_of_reward( owner.address, beneficiary.address, False, - True, + True, # rewards enabled since this staker should accrue rewards now sender=deployer, ) - # 6 period, on -> 10 period, off + assert penalty_board_comp.getCurrentPeriod() == 6 + balance = penalty_board_comp.getAccruedBalance(staking_provider.address) + assert balance == FULL_COMPENSATION + + # Period 6, on -> Period 10, off chain.pending_timestamp += 4 * PERIOD_DURATION mock_taco_app.computeRewards(staking_provider.address, sender=deployer) mock_taco_app.setRoles( @@ -458,15 +549,18 @@ def test_turning_on_and_of_reward( owner.address, beneficiary.address, False, - False, + False, # rewards disabled again sender=deployer, ) expected = 4 * FULL_COMPENSATION + assert penalty_board_comp.getCurrentPeriod() == 10 balance = penalty_board_comp.getAccruedBalance(staking_provider.address) assert balance == expected + # Period 10, off -> off going forward chain.pending_timestamp += 4 * PERIOD_DURATION + assert penalty_board_comp.getCurrentPeriod() == 14 before = token.balanceOf(beneficiary.address) penalty_board_comp.withdraw(staking_provider.address, sender=beneficiary) after = token.balanceOf(beneficiary.address)