From 55c629951b20c4960c0ee108d7f1502e26a94d4c 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/13] 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 7950f31e2e2172dbb5869c9f0c3b46e84696ff21 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/13] 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 b3e10764712b3495f3d987a09681c60bb5cb5248 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/13] 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 331689324cd083492d3ce9477bc2233e066b58a7 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/13] 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 ef0866456143a9941673b3dbccb4398c8c195b46 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/13] 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 783400221d885c91d7d132873eeb2a8786372694 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/13] 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 83b26597c0c1e674eae7a755c14c5a2c5e68ed37 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/13] 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 400b226afa828e30de1d89603587104cd82cf927 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/13] 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 659363aa5373336f2ae06c4eb3a15ef9ca1a7d97 Mon Sep 17 00:00:00 2001 From: Viktoriia Zotova Date: Sat, 7 Mar 2026 10:26:51 -0500 Subject: [PATCH 09/13] 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 eca5b37ad6159f03ffdd9018290c95711300da2c Mon Sep 17 00:00:00 2001 From: Viktoriia Zotova Date: Tue, 10 Mar 2026 13:53:33 -0400 Subject: [PATCH 10/13] 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 032e8c2cf4800ea6b9eaf89505b9615f504eb140 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/13] 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 7844650a4c685863c3d5782526316082805f9a18 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/13] 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 75710b80f174f7b8b175e8b4950abb22a9be58d4 Mon Sep 17 00:00:00 2001 From: Viktoriia Zotova Date: Fri, 27 Mar 2026 13:55:01 -0400 Subject: [PATCH 13/13] 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 6144babed..9be4f4c5a 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;