From 808e2738324367e2a0a27aeacbc20147cfde7775 Mon Sep 17 00:00:00 2001 From: vgorkavenko Date: Fri, 26 Jun 2026 10:19:23 +0200 Subject: [PATCH 1/7] wip: draft --- src/NodeOperatorStrikes.sol | 213 +++++++++++ src/interfaces/INodeOperatorStrikes.sol | 109 ++++++ test/unit/NodeOperatorStrikes.t.sol | 481 ++++++++++++++++++++++++ 3 files changed, 803 insertions(+) create mode 100644 src/NodeOperatorStrikes.sol create mode 100644 src/interfaces/INodeOperatorStrikes.sol create mode 100644 test/unit/NodeOperatorStrikes.t.sol diff --git a/src/NodeOperatorStrikes.sol b/src/NodeOperatorStrikes.sol new file mode 100644 index 000000000..c1aaa01dd --- /dev/null +++ b/src/NodeOperatorStrikes.sol @@ -0,0 +1,213 @@ +// SPDX-FileCopyrightText: 2026 Lido +// SPDX-License-Identifier: GPL-3.0 + +pragma solidity 0.8.33; + +import { AccessControlEnumerableUpgradeable } from "@openzeppelin/contracts-upgradeable/access/extensions/AccessControlEnumerableUpgradeable.sol"; +import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; + +import { ICuratedModule } from "./interfaces/ICuratedModule.sol"; +import { IMetaRegistry } from "./interfaces/IMetaRegistry.sol"; +import { INodeOperatorStrikes, StrikeInput, Strike, StrikeThreshold } from "./interfaces/INodeOperatorStrikes.sol"; +import { MAX_BP } from "./lib/Constants.sol"; + +/// @notice Committee-issued, operator-level strikes that cumulatively reduce +/// a Node Operator's allocation weight. Strikes persist until removed; +/// removal is permissionless once a strike's lifetime elapses. +contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessControlEnumerableUpgradeable { + struct OperatorStrikes { + uint64 lastStrikeId; + Strike[] active; + /// @dev maps a strike id to its position in `active` (0 = not active). + mapping(uint256 strikeId => uint256 position) index; + } + + /// @custom:storage-location erc7201:NodeOperatorStrikes + struct NodeOperatorStrikesStorage { + StrikeThreshold[] thresholds; + mapping(uint256 nodeOperatorId => OperatorStrikes) operatorStrikes; + } + + bytes32 public constant STRIKES_COMMITTEE_ROLE = keccak256("STRIKES_COMMITTEE_ROLE"); + + uint256 public constant MAX_THRESHOLDS = 16; + + ICuratedModule public immutable MODULE; + IMetaRegistry public immutable META_REGISTRY; + + // keccak256(abi.encode(uint256(keccak256("NodeOperatorStrikes")) - 1)) & ~bytes32(uint256(0xff)) + bytes32 private constant NODE_OPERATOR_STRIKES_STORAGE_LOCATION = + 0x510f8e4bbf34090117edc1d950679ffb8abd223dc216175d997628968b892400; + + /// @param module CuratedModule proxy address. + constructor(address module) { + if (module == address(0)) revert ZeroModuleAddress(); + + MODULE = ICuratedModule(module); + META_REGISTRY = ICuratedModule(module).META_REGISTRY(); + + _disableInitializers(); + } + + /// @inheritdoc INodeOperatorStrikes + function initialize(address admin) external initializer { + if (admin == address(0)) revert ZeroAdminAddress(); + _grantRole(DEFAULT_ADMIN_ROLE, admin); + } + + /// @inheritdoc INodeOperatorStrikes + function issueStrike( + StrikeInput calldata input + ) external onlyRole(STRIKES_COMMITTEE_ROLE) returns (uint256 strikeId) { + return _issueStrike(input); + } + + /// @inheritdoc INodeOperatorStrikes + function removeStrike(uint256 nodeOperatorId, uint256 strikeId) external { + _removeStrike(nodeOperatorId, strikeId); + } + + /// @inheritdoc INodeOperatorStrikes + function setStrikeThresholds(StrikeThreshold[] calldata thresholds) external onlyRole(DEFAULT_ADMIN_ROLE) { + _validateStrikeThresholds(thresholds); + + StrikeThreshold[] storage stored = _storage().thresholds; + if (stored.length > 0) delete _storage().thresholds; + for (uint256 i; i < thresholds.length; ++i) { + stored.push(thresholds[i]); + } + + emit StrikeThresholdsSet(thresholds); + } + + /// @inheritdoc INodeOperatorStrikes + function getActiveStrikesCount(uint256 nodeOperatorId) external view returns (uint256 count) { + return _storage().operatorStrikes[nodeOperatorId].active.length; + } + + /// @inheritdoc INodeOperatorStrikes + function getStrikeWeightMultiplier(uint256 nodeOperatorId) external view returns (uint256 multiplierBP) { + multiplierBP = MAX_BP; + + NodeOperatorStrikesStorage storage $ = _storage(); + uint256 count = $.operatorStrikes[nodeOperatorId].active.length; + + StrikeThreshold[] storage thresholds = $.thresholds; + uint256 len = thresholds.length; + for (uint256 i; i < len; ++i) { + if (count < thresholds[i].minCount) break; // thresholds ascend by minCount + multiplierBP = MAX_BP - thresholds[i].reductionBP; + } + } + + /// @inheritdoc INodeOperatorStrikes + function getStrike(uint256 nodeOperatorId, uint256 strikeId) external view returns (Strike memory strike) { + OperatorStrikes storage rec = _storage().operatorStrikes[nodeOperatorId]; + uint256 position = rec.index[strikeId]; + if (position == 0) return strike; + return rec.active[position - 1]; + } + + /// @inheritdoc INodeOperatorStrikes + function getStrikes(uint256 nodeOperatorId) external view returns (Strike[] memory strikes) { + return _storage().operatorStrikes[nodeOperatorId].active; + } + + /// @inheritdoc INodeOperatorStrikes + function getExpiredStrikes(uint256 nodeOperatorId) external view returns (uint256[] memory strikeIds) { + Strike[] storage active = _storage().operatorStrikes[nodeOperatorId].active; + uint256 len = active.length; + + strikeIds = new uint256[](len); + uint256 j; + for (uint256 i; i < len; ++i) { + if (active[i].expiry <= block.timestamp) { + strikeIds[j] = active[i].id; + ++j; + } + } + + assembly ("memory-safe") { + mstore(strikeIds, j) + } + } + + /// @inheritdoc INodeOperatorStrikes + function getStrikeThresholds() external view returns (StrikeThreshold[] memory thresholds) { + return _storage().thresholds; + } + + function _issueStrike(StrikeInput calldata input) internal returns (uint256 strikeId) { + _onlyExistingOperator(input.nodeOperatorId); + + uint256 lifetime = input.lifetime; + if (lifetime == 0) revert InvalidLifetime(); + uint256 expiry = block.timestamp + lifetime; + if (expiry > type(uint64).max) revert InvalidLifetime(); + + // TODO: consider a MAX_STRIKES_PER_OPERATOR cap; past the deepest threshold weight is already 0. + OperatorStrikes storage rec = _storage().operatorStrikes[input.nodeOperatorId]; + strikeId = ++rec.lastStrikeId; + rec.active.push(Strike({ id: uint64(strikeId), expiry: uint64(expiry), category: input.category })); + rec.index[strikeId] = rec.active.length; + + emit StrikeIssued({ + nodeOperatorId: input.nodeOperatorId, + strikeId: strikeId, + category: input.category, + expiry: expiry, + name: input.name, + description: input.description + }); + + META_REGISTRY.refreshOperatorWeight(input.nodeOperatorId); + } + + function _removeStrike(uint256 nodeOperatorId, uint256 strikeId) internal { + OperatorStrikes storage rec = _storage().operatorStrikes[nodeOperatorId]; + uint256 position = rec.index[strikeId]; + if (position == 0) revert StrikeNotActive(); + + uint256 idx = position - 1; + // Committee may remove anytime; anyone else only after the lifetime elapses. + if (!hasRole(STRIKES_COMMITTEE_ROLE, msg.sender) && rec.active[idx].expiry > block.timestamp) { + revert StrikeNotExpired(); + } + + uint256 lastIdx = rec.active.length - 1; + if (idx != lastIdx) { + Strike memory moved = rec.active[lastIdx]; + rec.active[idx] = moved; + rec.index[moved.id] = position; + } + rec.active.pop(); + delete rec.index[strikeId]; + + emit StrikeRemoved(nodeOperatorId, strikeId, msg.sender); + + META_REGISTRY.refreshOperatorWeight(nodeOperatorId); + } + + function _onlyExistingOperator(uint256 nodeOperatorId) internal view { + if (nodeOperatorId >= MODULE.getNodeOperatorsCount()) revert NodeOperatorDoesNotExist(); + } + + function _storage() internal pure returns (NodeOperatorStrikesStorage storage $) { + assembly ("memory-safe") { + $.slot := NODE_OPERATOR_STRIKES_STORAGE_LOCATION + } + } + + function _validateStrikeThresholds(StrikeThreshold[] calldata thresholds) private pure { + uint256 len = thresholds.length; + if (len == 0 || len > MAX_THRESHOLDS) revert InvalidStrikeThresholds(); + if (thresholds[0].minCount == 0) revert InvalidStrikeThresholds(); + if (thresholds[0].reductionBP > MAX_BP) revert InvalidStrikeThresholds(); + + for (uint256 i = 1; i < len; ++i) { + if (thresholds[i].minCount <= thresholds[i - 1].minCount) revert InvalidStrikeThresholds(); + if (thresholds[i].reductionBP < thresholds[i - 1].reductionBP) revert InvalidStrikeThresholds(); + if (thresholds[i].reductionBP > MAX_BP) revert InvalidStrikeThresholds(); + } + } +} diff --git a/src/interfaces/INodeOperatorStrikes.sol b/src/interfaces/INodeOperatorStrikes.sol new file mode 100644 index 000000000..5c0402499 --- /dev/null +++ b/src/interfaces/INodeOperatorStrikes.sol @@ -0,0 +1,109 @@ +// SPDX-FileCopyrightText: 2026 Lido +// SPDX-License-Identifier: GPL-3.0 + +pragma solidity 0.8.33; + +import { ICuratedModule } from "./ICuratedModule.sol"; +import { IMetaRegistry } from "./IMetaRegistry.sol"; + +/// @dev Payload describing a strike to issue. `name` and `description` are emitted in events only. +struct StrikeInput { + uint256 nodeOperatorId; + bytes32 category; + uint256 lifetime; + string name; + string description; +} + +/// @dev Removed or never-issued strike reads as a zeroed slot (`id == 0`). +struct Strike { + uint64 id; + uint64 expiry; + bytes32 category; +} + +/// @dev Cumulative weight reduction step. At `minCount` active strikes the operator's weight is +/// reduced by `reductionBP` basis points (effective multiplier = MAX_BP - reductionBP). +struct StrikeThreshold { + uint256 minCount; + uint256 reductionBP; +} + +interface INodeOperatorStrikes { + event StrikeIssued( + uint256 indexed nodeOperatorId, + uint256 indexed strikeId, + bytes32 indexed category, + uint256 expiry, + string name, + string description + ); + event StrikeRemoved(uint256 indexed nodeOperatorId, uint256 indexed strikeId, address indexed remover); + event StrikeThresholdsSet(StrikeThreshold[] thresholds); + + error ZeroModuleAddress(); + error ZeroAdminAddress(); + error NodeOperatorDoesNotExist(); + error StrikeNotActive(); + error StrikeNotExpired(); + error InvalidLifetime(); + error InvalidStrikeThresholds(); + + /// @notice Role allowed to issue strikes and remove them before their lifetime elapses. + function STRIKES_COMMITTEE_ROLE() external view returns (bytes32); + + /// @notice Maximum number of weight-reduction thresholds. + function MAX_THRESHOLDS() external view returns (uint256); + + /// @notice Curated module used to check operator existence. + function MODULE() external view returns (ICuratedModule); + + /// @notice MetaRegistry called back via `refreshOperatorWeight` on every strike change. + function META_REGISTRY() external view returns (IMetaRegistry); + + /// @notice Initialize the contract. + /// @param admin Address to receive DEFAULT_ADMIN_ROLE. + function initialize(address admin) external; + + /// @notice Issue a strike against a Node Operator (callable by STRIKES_COMMITTEE_ROLE). + /// @param input Strike payload. + /// @return strikeId ID assigned to the new strike. + function issueStrike(StrikeInput calldata input) external returns (uint256 strikeId); + + /// @notice Remove a strike. Allowed for STRIKES_COMMITTEE_ROLE at any time; permissionless once + /// the strike's lifetime has elapsed. + /// @param nodeOperatorId ID of the Node Operator. + /// @param strikeId ID of the strike to remove. + function removeStrike(uint256 nodeOperatorId, uint256 strikeId) external; + + /// @notice Set the global weight-reduction thresholds (callable by DEFAULT_ADMIN_ROLE). + /// @dev MUST be paired with `MetaRegistry.refreshOperatorWeight` for every operator with active strikes. + /// @param thresholds Step function mapping active strike count to weight reduction. + function setStrikeThresholds(StrikeThreshold[] calldata thresholds) external; + + /// @notice Number of active (non-removed) strikes of a Node Operator. + /// @param nodeOperatorId ID of the Node Operator. + function getActiveStrikesCount(uint256 nodeOperatorId) external view returns (uint256 count); + + /// @notice Weight multiplier (in basis points) implied by the operator's active strike count. + /// @dev Counts strikes regardless of expiry: an expired strike keeps reducing the weight until removed. + /// @param nodeOperatorId ID of the Node Operator. + function getStrikeWeightMultiplier(uint256 nodeOperatorId) external view returns (uint256 multiplierBP); + + /// @notice Return a single strike record (zeroed if removed or never issued). + /// @param nodeOperatorId ID of the Node Operator. + /// @param strikeId ID of the strike. + function getStrike(uint256 nodeOperatorId, uint256 strikeId) external view returns (Strike memory strike); + + /// @notice Return the active (non-removed) strikes of a Node Operator. + /// @param nodeOperatorId ID of the Node Operator. + function getStrikes(uint256 nodeOperatorId) external view returns (Strike[] memory strikes); + + /// @notice Return IDs of active strikes whose lifetime has elapsed (`block.timestamp >= expiry`), + /// i.e. the ones that can be removed permissionlessly. + /// @param nodeOperatorId ID of the Node Operator. + function getExpiredStrikes(uint256 nodeOperatorId) external view returns (uint256[] memory strikeIds); + + /// @notice Return the configured global weight-reduction thresholds. + function getStrikeThresholds() external view returns (StrikeThreshold[] memory thresholds); +} diff --git a/test/unit/NodeOperatorStrikes.t.sol b/test/unit/NodeOperatorStrikes.t.sol new file mode 100644 index 000000000..139fa10ed --- /dev/null +++ b/test/unit/NodeOperatorStrikes.t.sol @@ -0,0 +1,481 @@ +// SPDX-FileCopyrightText: 2026 Lido +// SPDX-License-Identifier: GPL-3.0 + +pragma solidity 0.8.33; + +import { Test } from "forge-std/Test.sol"; + +import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; + +import { NodeOperatorStrikes } from "src/NodeOperatorStrikes.sol"; +import { INodeOperatorStrikes, StrikeInput, Strike, StrikeThreshold } from "src/interfaces/INodeOperatorStrikes.sol"; + +import { CuratedMock } from "../helpers/mocks/CuratedMock.sol"; +import { MetaRegistryMock } from "../helpers/mocks/MetaRegistryMock.sol"; +import { Utilities } from "../helpers/Utilities.sol"; +import { Fixtures } from "../helpers/Fixtures.sol"; + +contract NodeOperatorStrikesBaseTest is Test, Utilities, Fixtures { + CuratedMock public module; + MetaRegistryMock public metaRegistryMock; + NodeOperatorStrikes public strikes; + + address public admin; + address public committee; + address public stranger; + + uint256 internal constant MAX_BP = 10_000; + uint256 internal constant LIFETIME = 30 days; + bytes32 internal constant CATEGORY = keccak256("performance"); + uint256 internal constant NO_ID = 0; + + function setUp() public virtual { + admin = nextAddress("ADMIN"); + committee = nextAddress("COMMITTEE"); + stranger = nextAddress("STRANGER"); + + module = new CuratedMock(); + module.mock_setNodeOperatorsCount(3); + + metaRegistryMock = new MetaRegistryMock(); + module.mock_setMetaRegistry(address(metaRegistryMock)); + + strikes = new NodeOperatorStrikes({ module: address(module) }); + _enableInitializers(address(strikes)); + strikes.initialize(admin); + + bytes32 committeeRole = strikes.STRIKES_COMMITTEE_ROLE(); + vm.prank(admin); + strikes.grantRole(committeeRole, committee); + } + + function _input( + uint256 nodeOperatorId, + bytes32 category, + uint256 lifetime + ) internal pure returns (StrikeInput memory) { + return + StrikeInput({ + nodeOperatorId: nodeOperatorId, + category: category, + lifetime: lifetime, + name: "Late attestations", + description: "Operator missed attestations for two consecutive frames" + }); + } + + function _exampleThresholds() internal pure returns (StrikeThreshold[] memory thresholds) { + thresholds = new StrikeThreshold[](4); + thresholds[0] = StrikeThreshold({ minCount: 2, reductionBP: 2_500 }); + thresholds[1] = StrikeThreshold({ minCount: 3, reductionBP: 5_000 }); + thresholds[2] = StrikeThreshold({ minCount: 4, reductionBP: 7_500 }); + thresholds[3] = StrikeThreshold({ minCount: 5, reductionBP: 10_000 }); + } + + function _setExampleThresholds() internal { + vm.prank(admin); + strikes.setStrikeThresholds(_exampleThresholds()); + } + + function _issue(uint256 nodeOperatorId) internal returns (uint256 strikeId) { + vm.prank(committee); + strikeId = strikes.issueStrike(_input(nodeOperatorId, CATEGORY, LIFETIME)); + } +} + +contract NodeOperatorStrikesConstructorTest is NodeOperatorStrikesBaseTest { + function test_constructor_SetsImmutables() public view { + assertEq(address(strikes.MODULE()), address(module)); + assertEq(address(strikes.META_REGISTRY()), address(metaRegistryMock)); + } + + function test_constructor_RevertWhen_ZeroModule() public { + vm.expectRevert(INodeOperatorStrikes.ZeroModuleAddress.selector); + new NodeOperatorStrikes(address(0)); + } +} + +contract NodeOperatorStrikesInitializeTest is NodeOperatorStrikesBaseTest { + function test_initialize_SetsAdmin() public view { + assertTrue(strikes.hasRole(strikes.DEFAULT_ADMIN_ROLE(), admin)); + } + + function test_initialize_RevertWhen_ZeroAdmin() public { + NodeOperatorStrikes s = new NodeOperatorStrikes(address(module)); + _enableInitializers(address(s)); + vm.expectRevert(INodeOperatorStrikes.ZeroAdminAddress.selector); + s.initialize(address(0)); + } + + function test_initialize_RevertWhen_DoubleCall() public { + vm.expectRevert(Initializable.InvalidInitialization.selector); + strikes.initialize(admin); + } +} + +contract NodeOperatorStrikesIssueTest is NodeOperatorStrikesBaseTest { + function test_issueStrike_StoresAndRefreshes() public { + uint256 expiry = block.timestamp + LIFETIME; + + vm.expectEmit(true, true, true, true, address(strikes)); + emit INodeOperatorStrikes.StrikeIssued( + NO_ID, + 1, + CATEGORY, + expiry, + "Late attestations", + "Operator missed attestations for two consecutive frames" + ); + + vm.prank(committee); + uint256 strikeId = strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME)); + + assertEq(strikeId, 1); + assertEq(strikes.getActiveStrikesCount(NO_ID), 1); + + Strike memory s = strikes.getStrike(NO_ID, 1); + assertEq(s.id, 1); + assertEq(s.expiry, expiry); + assertEq(s.category, CATEGORY); + + assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), 1); + assertEq(metaRegistryMock.lastRefreshedOperatorId(), NO_ID); + } + + function test_issueStrike_AssignsSequentialIds() public { + assertEq(_issue(NO_ID), 1); + assertEq(_issue(NO_ID), 2); + assertEq(_issue(NO_ID), 3); + assertEq(strikes.getActiveStrikesCount(NO_ID), 3); + } + + function test_issueStrike_RevertWhen_NotCommittee() public { + expectRoleRevert(stranger, strikes.STRIKES_COMMITTEE_ROLE()); + vm.prank(stranger); + strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME)); + } + + function test_issueStrike_RevertWhen_OperatorDoesNotExist() public { + vm.expectRevert(INodeOperatorStrikes.NodeOperatorDoesNotExist.selector); + vm.prank(committee); + strikes.issueStrike(_input(3, CATEGORY, LIFETIME)); // count == 3, so id 3 doesn't exist + } + + function test_issueStrike_RevertWhen_ZeroLifetime() public { + vm.expectRevert(INodeOperatorStrikes.InvalidLifetime.selector); + vm.prank(committee); + strikes.issueStrike(_input(NO_ID, CATEGORY, 0)); + } + + function test_issueStrike_RevertWhen_LifetimeOverflowsUint64() public { + uint256 hugeLifetime = type(uint64).max; // block.timestamp + this overflows uint64 + vm.expectRevert(INodeOperatorStrikes.InvalidLifetime.selector); + vm.prank(committee); + strikes.issueStrike(_input(NO_ID, CATEGORY, hugeLifetime)); + } +} + +contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { + function test_removeStrike_CommitteeBeforeExpiry() public { + uint256 id = _issue(NO_ID); + uint256 refreshesBefore = metaRegistryMock.refreshOperatorWeightCallCount(); + + vm.expectEmit(true, true, true, true, address(strikes)); + emit INodeOperatorStrikes.StrikeRemoved(NO_ID, id, committee); + + vm.prank(committee); + strikes.removeStrike(NO_ID, id); + + assertEq(strikes.getActiveStrikesCount(NO_ID), 0); + assertEq(strikes.getStrike(NO_ID, id).id, 0); // removed -> zeroed slot + assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), refreshesBefore + 1); + } + + function test_removeStrike_RevertWhen_StrangerBeforeExpiry() public { + uint256 id = _issue(NO_ID); + vm.expectRevert(INodeOperatorStrikes.StrikeNotExpired.selector); + vm.prank(stranger); + strikes.removeStrike(NO_ID, id); + } + + function test_removeStrike_PermissionlessAtExpiry() public { + uint256 id = _issue(NO_ID); + uint256 expiry = strikes.getStrike(NO_ID, id).expiry; + + // One second before expiry: stranger cannot remove. + vm.warp(expiry - 1); + vm.expectRevert(INodeOperatorStrikes.StrikeNotExpired.selector); + vm.prank(stranger); + strikes.removeStrike(NO_ID, id); + + // Exactly at expiry: removal is permissionless. + vm.warp(expiry); + vm.prank(stranger); + strikes.removeStrike(NO_ID, id); + + assertEq(strikes.getActiveStrikesCount(NO_ID), 0); + } + + function test_removeStrike_PermissionlessAfterExpiry() public { + uint256 id = _issue(NO_ID); + uint256 expiry = strikes.getStrike(NO_ID, id).expiry; + + vm.warp(expiry + 1 days); + vm.prank(stranger); + strikes.removeStrike(NO_ID, id); + + assertEq(strikes.getActiveStrikesCount(NO_ID), 0); + } + + function test_removeStrike_RevertWhen_NonExistent() public { + vm.expectRevert(INodeOperatorStrikes.StrikeNotActive.selector); + vm.prank(committee); + strikes.removeStrike(NO_ID, 1); + } + + function test_removeStrike_RevertWhen_AlreadyRemoved() public { + uint256 id = _issue(NO_ID); + vm.prank(committee); + strikes.removeStrike(NO_ID, id); + + vm.expectRevert(INodeOperatorStrikes.StrikeNotActive.selector); + vm.prank(committee); + strikes.removeStrike(NO_ID, id); + } + + function test_strikeIds_MonotonicWithGapsAndExpiredInActive() public { + _issue(NO_ID); // id 1 + uint256 id2 = _issue(NO_ID); // id 2 + _issue(NO_ID); // id 3 + _issue(NO_ID); // id 4 + + // Remove id 2 early (committee): id 4 swaps into its slot. + vm.prank(committee); + strikes.removeStrike(NO_ID, id2); + + // New strikes keep incrementing past the gap — id 2 is never reused. + uint256 id5 = _issue(NO_ID); + assertEq(id5, 5); + assertEq(strikes.getActiveStrikesCount(NO_ID), 4); // 1, 3, 4, 5 + + // After lifetime elapses expired strikes stay in active until explicitly removed. + vm.warp(block.timestamp + LIFETIME); + assertEq(strikes.getActiveStrikesCount(NO_ID), 4); + assertEq(strikes.getExpiredStrikes(NO_ID).length, 4); + + // Gap at id 2 returns zeroed; all others are individually reachable. + assertEq(strikes.getStrike(NO_ID, 1).id, 1); + assertEq(strikes.getStrike(NO_ID, 2).id, 0); + assertEq(strikes.getStrike(NO_ID, 3).id, 3); + assertEq(strikes.getStrike(NO_ID, 4).id, 4); + assertEq(strikes.getStrike(NO_ID, 5).id, 5); + + // Sum of active ids is a duplicate-free witness. + Strike[] memory active = strikes.getStrikes(NO_ID); + assertEq(active.length, 4); + uint256 idSum; + for (uint256 i; i < active.length; ++i) idSum += active[i].id; + assertEq(idSum, 1 + 3 + 4 + 5); + } + + function test_removeStrike_SwapPopKeepsIndexConsistent() public { + _issue(NO_ID); // id 1 + uint256 id2 = _issue(NO_ID); + uint256 id3 = _issue(NO_ID); + + // Remove the first strike: the last one (id3) is swapped into its slot. + vm.prank(committee); + strikes.removeStrike(NO_ID, 1); + + // The swapped strike must still resolve by its id and stay removable. + assertEq(strikes.getActiveStrikesCount(NO_ID), 2); + assertEq(strikes.getStrike(NO_ID, id3).id, id3); + assertEq(strikes.getStrike(NO_ID, id2).id, id2); + + vm.prank(committee); + strikes.removeStrike(NO_ID, id3); + + assertEq(strikes.getActiveStrikesCount(NO_ID), 1); + assertEq(strikes.getStrike(NO_ID, id2).id, id2); + assertEq(strikes.getStrike(NO_ID, id3).id, 0); // removed + } +} + +contract NodeOperatorStrikesThresholdsTest is NodeOperatorStrikesBaseTest { + function test_setStrikeThresholds_RoundTrip() public { + StrikeThreshold[] memory thresholds = _exampleThresholds(); + + vm.expectEmit(false, false, false, true, address(strikes)); + emit INodeOperatorStrikes.StrikeThresholdsSet(thresholds); + + vm.prank(admin); + strikes.setStrikeThresholds(thresholds); + + StrikeThreshold[] memory stored = strikes.getStrikeThresholds(); + assertEq(stored.length, 4); + assertEq(stored[0].minCount, 2); + assertEq(stored[0].reductionBP, 2_500); + assertEq(stored[3].minCount, 5); + assertEq(stored[3].reductionBP, 10_000); + } + + function test_setStrikeThresholds_Replaces() public { + _setExampleThresholds(); + + StrikeThreshold[] memory next = new StrikeThreshold[](1); + next[0] = StrikeThreshold({ minCount: 1, reductionBP: 1_000 }); + vm.prank(admin); + strikes.setStrikeThresholds(next); + + StrikeThreshold[] memory stored = strikes.getStrikeThresholds(); + assertEq(stored.length, 1); + assertEq(stored[0].minCount, 1); + assertEq(stored[0].reductionBP, 1_000); + } + + function test_setStrikeThresholds_RevertWhen_NotAdmin() public { + bytes32 adminRole = strikes.DEFAULT_ADMIN_ROLE(); + expectRoleRevert(stranger, adminRole); + vm.prank(stranger); + strikes.setStrikeThresholds(_exampleThresholds()); + } + + function test_setStrikeThresholds_RevertWhen_Empty() public { + StrikeThreshold[] memory thresholds = new StrikeThreshold[](0); + vm.expectRevert(INodeOperatorStrikes.InvalidStrikeThresholds.selector); + vm.prank(admin); + strikes.setStrikeThresholds(thresholds); + } + + function test_setStrikeThresholds_RevertWhen_FirstMinCountZero() public { + StrikeThreshold[] memory thresholds = new StrikeThreshold[](1); + thresholds[0] = StrikeThreshold({ minCount: 0, reductionBP: 1_000 }); + vm.expectRevert(INodeOperatorStrikes.InvalidStrikeThresholds.selector); + vm.prank(admin); + strikes.setStrikeThresholds(thresholds); + } + + function test_setStrikeThresholds_RevertWhen_MinCountNotAscending() public { + StrikeThreshold[] memory thresholds = new StrikeThreshold[](2); + thresholds[0] = StrikeThreshold({ minCount: 2, reductionBP: 2_500 }); + thresholds[1] = StrikeThreshold({ minCount: 2, reductionBP: 5_000 }); + vm.expectRevert(INodeOperatorStrikes.InvalidStrikeThresholds.selector); + vm.prank(admin); + strikes.setStrikeThresholds(thresholds); + } + + function test_setStrikeThresholds_RevertWhen_ReductionDecreasing() public { + StrikeThreshold[] memory thresholds = new StrikeThreshold[](2); + thresholds[0] = StrikeThreshold({ minCount: 2, reductionBP: 5_000 }); + thresholds[1] = StrikeThreshold({ minCount: 3, reductionBP: 2_500 }); + vm.expectRevert(INodeOperatorStrikes.InvalidStrikeThresholds.selector); + vm.prank(admin); + strikes.setStrikeThresholds(thresholds); + } + + function test_setStrikeThresholds_RevertWhen_ReductionAboveMaxBp() public { + StrikeThreshold[] memory thresholds = new StrikeThreshold[](1); + thresholds[0] = StrikeThreshold({ minCount: 2, reductionBP: MAX_BP + 1 }); + vm.expectRevert(INodeOperatorStrikes.InvalidStrikeThresholds.selector); + vm.prank(admin); + strikes.setStrikeThresholds(thresholds); + } + + function test_setStrikeThresholds_RevertWhen_TooMany() public { + uint256 n = strikes.MAX_THRESHOLDS() + 1; + StrikeThreshold[] memory thresholds = new StrikeThreshold[](n); + for (uint256 i; i < n; ++i) { + thresholds[i] = StrikeThreshold({ minCount: i + 1, reductionBP: 0 }); + } + vm.expectRevert(INodeOperatorStrikes.InvalidStrikeThresholds.selector); + vm.prank(admin); + strikes.setStrikeThresholds(thresholds); + } +} + +contract NodeOperatorStrikesWeightMultiplierTest is NodeOperatorStrikesBaseTest { + function test_getStrikeWeightMultiplier_EmptyThresholdsIsMaxBP() public { + // No thresholds configured yet. + _issue(NO_ID); + _issue(NO_ID); + _issue(NO_ID); + assertEq(strikes.getStrikeWeightMultiplier(NO_ID), MAX_BP); + } + + function test_getStrikeWeightMultiplier_StepFunction() public { + _setExampleThresholds(); + + // 0 strikes -> full weight. + assertEq(strikes.getStrikeWeightMultiplier(NO_ID), MAX_BP); + + _issue(NO_ID); // 1 -> full weight + assertEq(strikes.getStrikeWeightMultiplier(NO_ID), MAX_BP); + + _issue(NO_ID); // 2 -> 75% + assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 7_500); + + _issue(NO_ID); // 3 -> 50% + assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 5_000); + + _issue(NO_ID); // 4 -> 25% + assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 2_500); + + _issue(NO_ID); // 5 -> 0% + assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 0); + + _issue(NO_ID); // 6 -> still 0% (clamped to last band) + assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 0); + } + + function test_getStrikeWeightMultiplier_IncreasesAfterRemoval() public { + _setExampleThresholds(); + + uint256 id1 = _issue(NO_ID); + _issue(NO_ID); + _issue(NO_ID); // 3 active -> 50% + assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 5_000); + + vm.prank(committee); + strikes.removeStrike(NO_ID, id1); // 2 active -> 75% + assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 7_500); + } + + function test_getStrikes_ReturnsActiveOnlyWithIds() public { + uint256 id1 = _issue(NO_ID); + uint256 id2 = _issue(NO_ID); + uint256 id3 = _issue(NO_ID); + + // Remove the middle strike to check removed slots are skipped, not zero-padded. + vm.prank(committee); + strikes.removeStrike(NO_ID, id2); + + Strike[] memory active = strikes.getStrikes(NO_ID); + assertEq(active.length, 2); + assertEq(active[0].id, id1); + assertEq(active[1].id, id3); + } + + function test_getExpiredStrikes() public { + // Two short-lived strikes and one long-lived, all issued at the same time. + uint256 idA = _issue(NO_ID); // lifetime = LIFETIME + uint256 idB = _issue(NO_ID); // lifetime = LIFETIME + vm.prank(committee); + uint256 idC = strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME * 2)); + + // Nothing expired yet. + assertEq(strikes.getExpiredStrikes(NO_ID).length, 0); + + // Warp to the short lifetime boundary: A and B expired, C not. + vm.warp(block.timestamp + LIFETIME); + + uint256[] memory expired = strikes.getExpiredStrikes(NO_ID); + assertEq(expired.length, 2); + assertEq(expired[0], idA); + assertEq(expired[1], idB); + assertGt(strikes.getStrike(NO_ID, idC).expiry, block.timestamp); + + // Warp past the long lifetime: all three expired. + vm.warp(strikes.getStrike(NO_ID, idC).expiry); + assertEq(strikes.getExpiredStrikes(NO_ID).length, 3); + } +} From e5ba4ddb4e8fcc9d90c0e40bf5ffae018ad81b24 Mon Sep 17 00:00:00 2001 From: vgorkavenko Date: Fri, 26 Jun 2026 12:52:10 +0200 Subject: [PATCH 2/7] fix: comments --- src/AdditionalBondRegistry.sol | 1 + src/NodeOperatorStrikes.sol | 128 ++++++++------ src/interfaces/INodeOperatorStrikes.sol | 33 +++- test/unit/NodeOperatorStrikes.t.sol | 217 ++++++++++++++++++------ 4 files changed, 271 insertions(+), 108 deletions(-) diff --git a/src/AdditionalBondRegistry.sol b/src/AdditionalBondRegistry.sol index 0b2f8fa4c..10a3ed34b 100644 --- a/src/AdditionalBondRegistry.sol +++ b/src/AdditionalBondRegistry.sol @@ -52,6 +52,7 @@ contract AdditionalBondRegistry is IAdditionalBondRegistry, Initializable, Acces function initialize(address admin) external initializer { if (admin == address(0)) revert ZeroAdminAddress(); _grantRole(DEFAULT_ADMIN_ROLE, admin); + // TODO: set initial tiers here } /// @inheritdoc IAdditionalBondRegistry diff --git a/src/NodeOperatorStrikes.sol b/src/NodeOperatorStrikes.sol index c1aaa01dd..4ad8beea6 100644 --- a/src/NodeOperatorStrikes.sol +++ b/src/NodeOperatorStrikes.sol @@ -20,6 +20,8 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr Strike[] active; /// @dev maps a strike id to its position in `active` (0 = not active). mapping(uint256 strikeId => uint256 position) index; + /// @dev on-chain strike description; cleared on removal (empty if removed or never issued). + mapping(uint256 strikeId => string description) descriptions; } /// @custom:storage-location erc7201:NodeOperatorStrikes @@ -31,6 +33,7 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr bytes32 public constant STRIKES_COMMITTEE_ROLE = keccak256("STRIKES_COMMITTEE_ROLE"); uint256 public constant MAX_THRESHOLDS = 16; + uint256 public constant MAX_DESCRIPTION_LENGTH = 1024; ICuratedModule public immutable MODULE; IMetaRegistry public immutable META_REGISTRY; @@ -50,34 +53,73 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr } /// @inheritdoc INodeOperatorStrikes - function initialize(address admin) external initializer { + function initialize(address admin, StrikeThreshold[] calldata thresholds) external initializer { if (admin == address(0)) revert ZeroAdminAddress(); _grantRole(DEFAULT_ADMIN_ROLE, admin); + _setStrikeThresholds(thresholds); } /// @inheritdoc INodeOperatorStrikes function issueStrike( StrikeInput calldata input ) external onlyRole(STRIKES_COMMITTEE_ROLE) returns (uint256 strikeId) { - return _issueStrike(input); + _onlyExistingOperator(input.nodeOperatorId); + + if (bytes(input.description).length > MAX_DESCRIPTION_LENGTH) revert DescriptionTooLong(); + + uint256 lifetime = input.lifetime; + if (lifetime == 0) revert InvalidLifetime(); + uint256 expiry = block.timestamp + lifetime; + if (expiry > type(uint64).max) revert InvalidLifetime(); + + OperatorStrikes storage rec = _storage().operatorStrikes[input.nodeOperatorId]; + strikeId = ++rec.lastStrikeId; + rec.active.push(Strike({ id: uint64(strikeId), expiry: uint64(expiry), category: input.category })); + rec.index[strikeId] = rec.active.length; + rec.descriptions[strikeId] = input.description; + + emit StrikeIssued({ + nodeOperatorId: input.nodeOperatorId, + strikeId: strikeId, + category: input.category, + expiry: expiry, + description: input.description + }); + + META_REGISTRY.refreshOperatorWeight(input.nodeOperatorId); } /// @inheritdoc INodeOperatorStrikes - function removeStrike(uint256 nodeOperatorId, uint256 strikeId) external { - _removeStrike(nodeOperatorId, strikeId); + function removeStrike(uint256 nodeOperatorId, uint256 strikeId) external onlyRole(STRIKES_COMMITTEE_ROLE) { + OperatorStrikes storage rec = _storage().operatorStrikes[nodeOperatorId]; + uint256 position = rec.index[strikeId]; + if (position == 0) revert StrikeNotActive(); + + _popStrike(rec, nodeOperatorId, position - 1, strikeId); + + META_REGISTRY.refreshOperatorWeight(nodeOperatorId); } /// @inheritdoc INodeOperatorStrikes - function setStrikeThresholds(StrikeThreshold[] calldata thresholds) external onlyRole(DEFAULT_ADMIN_ROLE) { - _validateStrikeThresholds(thresholds); + function removeExpiredStrikes(uint256 nodeOperatorId) external { + OperatorStrikes storage rec = _storage().operatorStrikes[nodeOperatorId]; - StrikeThreshold[] storage stored = _storage().thresholds; - if (stored.length > 0) delete _storage().thresholds; - for (uint256 i; i < thresholds.length; ++i) { - stored.push(thresholds[i]); + // Back-to-front: swap-pop only moves an already-kept strike down, so none are skipped. + bool removed; + uint256 i = rec.active.length; + while (i > 0) { + --i; + if (rec.active[i].expiry > block.timestamp) continue; + _popStrike(rec, nodeOperatorId, i, rec.active[i].id); + removed = true; } - emit StrikeThresholdsSet(thresholds); + if (removed) META_REGISTRY.refreshOperatorWeight(nodeOperatorId); + } + + /// @inheritdoc INodeOperatorStrikes + function setStrikeThresholds(StrikeThreshold[] calldata thresholds) external onlyRole(DEFAULT_ADMIN_ROLE) { + _setStrikeThresholds(thresholds); } /// @inheritdoc INodeOperatorStrikes @@ -108,6 +150,14 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr return rec.active[position - 1]; } + /// @inheritdoc INodeOperatorStrikes + function getStrikeDescription( + uint256 nodeOperatorId, + uint256 strikeId + ) external view returns (string memory description) { + return _storage().operatorStrikes[nodeOperatorId].descriptions[strikeId]; + } + /// @inheritdoc INodeOperatorStrikes function getStrikes(uint256 nodeOperatorId) external view returns (Strike[] memory strikes) { return _storage().operatorStrikes[nodeOperatorId].active; @@ -137,55 +187,31 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr return _storage().thresholds; } - function _issueStrike(StrikeInput calldata input) internal returns (uint256 strikeId) { - _onlyExistingOperator(input.nodeOperatorId); - - uint256 lifetime = input.lifetime; - if (lifetime == 0) revert InvalidLifetime(); - uint256 expiry = block.timestamp + lifetime; - if (expiry > type(uint64).max) revert InvalidLifetime(); - - // TODO: consider a MAX_STRIKES_PER_OPERATOR cap; past the deepest threshold weight is already 0. - OperatorStrikes storage rec = _storage().operatorStrikes[input.nodeOperatorId]; - strikeId = ++rec.lastStrikeId; - rec.active.push(Strike({ id: uint64(strikeId), expiry: uint64(expiry), category: input.category })); - rec.index[strikeId] = rec.active.length; - - emit StrikeIssued({ - nodeOperatorId: input.nodeOperatorId, - strikeId: strikeId, - category: input.category, - expiry: expiry, - name: input.name, - description: input.description - }); - - META_REGISTRY.refreshOperatorWeight(input.nodeOperatorId); - } - - function _removeStrike(uint256 nodeOperatorId, uint256 strikeId) internal { - OperatorStrikes storage rec = _storage().operatorStrikes[nodeOperatorId]; - uint256 position = rec.index[strikeId]; - if (position == 0) revert StrikeNotActive(); - - uint256 idx = position - 1; - // Committee may remove anytime; anyone else only after the lifetime elapses. - if (!hasRole(STRIKES_COMMITTEE_ROLE, msg.sender) && rec.active[idx].expiry > block.timestamp) { - revert StrikeNotExpired(); - } - + /// @dev Swap-pops the strike at `idx` and clears its bookkeeping. Caller refreshes the weight. + function _popStrike(OperatorStrikes storage rec, uint256 nodeOperatorId, uint256 idx, uint256 strikeId) private { uint256 lastIdx = rec.active.length - 1; if (idx != lastIdx) { Strike memory moved = rec.active[lastIdx]; rec.active[idx] = moved; - rec.index[moved.id] = position; + rec.index[moved.id] = idx + 1; // 1-based position } rec.active.pop(); delete rec.index[strikeId]; + delete rec.descriptions[strikeId]; emit StrikeRemoved(nodeOperatorId, strikeId, msg.sender); + } - META_REGISTRY.refreshOperatorWeight(nodeOperatorId); + function _setStrikeThresholds(StrikeThreshold[] calldata thresholds) internal { + _validateStrikeThresholds(thresholds); + + delete _storage().thresholds; + StrikeThreshold[] storage stored = _storage().thresholds; + for (uint256 i; i < thresholds.length; ++i) { + stored.push(thresholds[i]); + } + + emit StrikeThresholdsSet(thresholds); } function _onlyExistingOperator(uint256 nodeOperatorId) internal view { @@ -206,7 +232,7 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr for (uint256 i = 1; i < len; ++i) { if (thresholds[i].minCount <= thresholds[i - 1].minCount) revert InvalidStrikeThresholds(); - if (thresholds[i].reductionBP < thresholds[i - 1].reductionBP) revert InvalidStrikeThresholds(); + if (thresholds[i].reductionBP <= thresholds[i - 1].reductionBP) revert InvalidStrikeThresholds(); if (thresholds[i].reductionBP > MAX_BP) revert InvalidStrikeThresholds(); } } diff --git a/src/interfaces/INodeOperatorStrikes.sol b/src/interfaces/INodeOperatorStrikes.sol index 5c0402499..57a3aec17 100644 --- a/src/interfaces/INodeOperatorStrikes.sol +++ b/src/interfaces/INodeOperatorStrikes.sol @@ -6,12 +6,11 @@ pragma solidity 0.8.33; import { ICuratedModule } from "./ICuratedModule.sol"; import { IMetaRegistry } from "./IMetaRegistry.sol"; -/// @dev Payload describing a strike to issue. `name` and `description` are emitted in events only. +/// @dev Payload describing a strike to issue. For the stored `description` see `getStrikeDescription`. struct StrikeInput { uint256 nodeOperatorId; bytes32 category; uint256 lifetime; - string name; string description; } @@ -35,7 +34,6 @@ interface INodeOperatorStrikes { uint256 indexed strikeId, bytes32 indexed category, uint256 expiry, - string name, string description ); event StrikeRemoved(uint256 indexed nodeOperatorId, uint256 indexed strikeId, address indexed remover); @@ -45,16 +43,19 @@ interface INodeOperatorStrikes { error ZeroAdminAddress(); error NodeOperatorDoesNotExist(); error StrikeNotActive(); - error StrikeNotExpired(); error InvalidLifetime(); error InvalidStrikeThresholds(); + error DescriptionTooLong(); - /// @notice Role allowed to issue strikes and remove them before their lifetime elapses. + /// @notice Role allowed to issue and remove strikes. function STRIKES_COMMITTEE_ROLE() external view returns (bytes32); /// @notice Maximum number of weight-reduction thresholds. function MAX_THRESHOLDS() external view returns (uint256); + /// @notice Maximum byte length of a strike description. + function MAX_DESCRIPTION_LENGTH() external view returns (uint256); + /// @notice Curated module used to check operator existence. function MODULE() external view returns (ICuratedModule); @@ -62,20 +63,26 @@ interface INodeOperatorStrikes { function META_REGISTRY() external view returns (IMetaRegistry); /// @notice Initialize the contract. - /// @param admin Address to receive DEFAULT_ADMIN_ROLE. - function initialize(address admin) external; + /// @param admin Address to receive DEFAULT_ADMIN_ROLE. + /// @param thresholds Initial weight-reduction thresholds. + function initialize(address admin, StrikeThreshold[] calldata thresholds) external; /// @notice Issue a strike against a Node Operator (callable by STRIKES_COMMITTEE_ROLE). /// @param input Strike payload. /// @return strikeId ID assigned to the new strike. function issueStrike(StrikeInput calldata input) external returns (uint256 strikeId); - /// @notice Remove a strike. Allowed for STRIKES_COMMITTEE_ROLE at any time; permissionless once - /// the strike's lifetime has elapsed. + /// @notice Remove any strike (callable by STRIKES_COMMITTEE_ROLE). For permissionless cleanup of + /// expired strikes use `removeExpiredStrikes`. /// @param nodeOperatorId ID of the Node Operator. /// @param strikeId ID of the strike to remove. function removeStrike(uint256 nodeOperatorId, uint256 strikeId) external; + /// @notice Permissionlessly remove all of a Node Operator's strikes whose lifetime has elapsed. + /// No-op if none are expired. + /// @param nodeOperatorId ID of the Node Operator. + function removeExpiredStrikes(uint256 nodeOperatorId) external; + /// @notice Set the global weight-reduction thresholds (callable by DEFAULT_ADMIN_ROLE). /// @dev MUST be paired with `MetaRegistry.refreshOperatorWeight` for every operator with active strikes. /// @param thresholds Step function mapping active strike count to weight reduction. @@ -95,6 +102,14 @@ interface INodeOperatorStrikes { /// @param strikeId ID of the strike. function getStrike(uint256 nodeOperatorId, uint256 strikeId) external view returns (Strike memory strike); + /// @notice On-chain description of a strike (empty if removed or never issued). + /// @param nodeOperatorId ID of the Node Operator. + /// @param strikeId ID of the strike. + function getStrikeDescription( + uint256 nodeOperatorId, + uint256 strikeId + ) external view returns (string memory description); + /// @notice Return the active (non-removed) strikes of a Node Operator. /// @param nodeOperatorId ID of the Node Operator. function getStrikes(uint256 nodeOperatorId) external view returns (Strike[] memory strikes); diff --git a/test/unit/NodeOperatorStrikes.t.sol b/test/unit/NodeOperatorStrikes.t.sol index 139fa10ed..653055f93 100644 --- a/test/unit/NodeOperatorStrikes.t.sol +++ b/test/unit/NodeOperatorStrikes.t.sol @@ -28,6 +28,7 @@ contract NodeOperatorStrikesBaseTest is Test, Utilities, Fixtures { uint256 internal constant LIFETIME = 30 days; bytes32 internal constant CATEGORY = keccak256("performance"); uint256 internal constant NO_ID = 0; + string internal constant DESCRIPTION = "Operator missed attestations for two consecutive frames"; function setUp() public virtual { admin = nextAddress("ADMIN"); @@ -42,7 +43,7 @@ contract NodeOperatorStrikesBaseTest is Test, Utilities, Fixtures { strikes = new NodeOperatorStrikes({ module: address(module) }); _enableInitializers(address(strikes)); - strikes.initialize(admin); + strikes.initialize(admin, _exampleThresholds()); bytes32 committeeRole = strikes.STRIKES_COMMITTEE_ROLE(); vm.prank(admin); @@ -59,8 +60,7 @@ contract NodeOperatorStrikesBaseTest is Test, Utilities, Fixtures { nodeOperatorId: nodeOperatorId, category: category, lifetime: lifetime, - name: "Late attestations", - description: "Operator missed attestations for two consecutive frames" + description: DESCRIPTION }); } @@ -100,16 +100,45 @@ contract NodeOperatorStrikesInitializeTest is NodeOperatorStrikesBaseTest { assertTrue(strikes.hasRole(strikes.DEFAULT_ADMIN_ROLE(), admin)); } + function test_initialize_SetsThresholds() public { + NodeOperatorStrikes s = new NodeOperatorStrikes(address(module)); + _enableInitializers(address(s)); + s.initialize(admin, _exampleThresholds()); + + StrikeThreshold[] memory stored = s.getStrikeThresholds(); + assertEq(stored.length, 4); + assertEq(stored[0].minCount, 2); + assertEq(stored[3].reductionBP, 10_000); + } + function test_initialize_RevertWhen_ZeroAdmin() public { NodeOperatorStrikes s = new NodeOperatorStrikes(address(module)); _enableInitializers(address(s)); vm.expectRevert(INodeOperatorStrikes.ZeroAdminAddress.selector); - s.initialize(address(0)); + s.initialize(address(0), new StrikeThreshold[](0)); + } + + function test_initialize_RevertWhen_InvalidThresholds() public { + NodeOperatorStrikes s = new NodeOperatorStrikes(address(module)); + _enableInitializers(address(s)); + + StrikeThreshold[] memory bad = new StrikeThreshold[](1); + bad[0] = StrikeThreshold({ minCount: 0, reductionBP: 1_000 }); // minCount 0 is invalid + vm.expectRevert(INodeOperatorStrikes.InvalidStrikeThresholds.selector); + s.initialize(admin, bad); + } + + function test_initialize_RevertWhen_EmptyThresholds() public { + NodeOperatorStrikes s = new NodeOperatorStrikes(address(module)); + _enableInitializers(address(s)); + + vm.expectRevert(INodeOperatorStrikes.InvalidStrikeThresholds.selector); + s.initialize(admin, new StrikeThreshold[](0)); } function test_initialize_RevertWhen_DoubleCall() public { vm.expectRevert(Initializable.InvalidInitialization.selector); - strikes.initialize(admin); + strikes.initialize(admin, new StrikeThreshold[](0)); } } @@ -118,14 +147,7 @@ contract NodeOperatorStrikesIssueTest is NodeOperatorStrikesBaseTest { uint256 expiry = block.timestamp + LIFETIME; vm.expectEmit(true, true, true, true, address(strikes)); - emit INodeOperatorStrikes.StrikeIssued( - NO_ID, - 1, - CATEGORY, - expiry, - "Late attestations", - "Operator missed attestations for two consecutive frames" - ); + emit INodeOperatorStrikes.StrikeIssued(NO_ID, 1, CATEGORY, expiry, DESCRIPTION); vm.prank(committee); uint256 strikeId = strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME)); @@ -137,6 +159,7 @@ contract NodeOperatorStrikesIssueTest is NodeOperatorStrikesBaseTest { assertEq(s.id, 1); assertEq(s.expiry, expiry); assertEq(s.category, CATEGORY); + assertEq(strikes.getStrikeDescription(NO_ID, 1), DESCRIPTION); assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), 1); assertEq(metaRegistryMock.lastRefreshedOperatorId(), NO_ID); @@ -173,6 +196,25 @@ contract NodeOperatorStrikesIssueTest is NodeOperatorStrikesBaseTest { vm.prank(committee); strikes.issueStrike(_input(NO_ID, CATEGORY, hugeLifetime)); } + + function test_issueStrike_AllowsMaxLengthDescription() public { + uint256 maxLen = strikes.MAX_DESCRIPTION_LENGTH(); + StrikeInput memory input = _input(NO_ID, CATEGORY, LIFETIME); + input.description = string(new bytes(maxLen)); + + vm.prank(committee); + uint256 id = strikes.issueStrike(input); + assertEq(bytes(strikes.getStrikeDescription(NO_ID, id)).length, maxLen); + } + + function test_issueStrike_RevertWhen_DescriptionTooLong() public { + StrikeInput memory input = _input(NO_ID, CATEGORY, LIFETIME); + input.description = string(new bytes(strikes.MAX_DESCRIPTION_LENGTH() + 1)); + + vm.expectRevert(INodeOperatorStrikes.DescriptionTooLong.selector); + vm.prank(committee); + strikes.issueStrike(input); + } } contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { @@ -191,40 +233,28 @@ contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), refreshesBefore + 1); } - function test_removeStrike_RevertWhen_StrangerBeforeExpiry() public { + function test_removeStrike_ClearsDescription() public { uint256 id = _issue(NO_ID); - vm.expectRevert(INodeOperatorStrikes.StrikeNotExpired.selector); - vm.prank(stranger); - strikes.removeStrike(NO_ID, id); - } - - function test_removeStrike_PermissionlessAtExpiry() public { - uint256 id = _issue(NO_ID); - uint256 expiry = strikes.getStrike(NO_ID, id).expiry; - - // One second before expiry: stranger cannot remove. - vm.warp(expiry - 1); - vm.expectRevert(INodeOperatorStrikes.StrikeNotExpired.selector); - vm.prank(stranger); - strikes.removeStrike(NO_ID, id); + assertEq(strikes.getStrikeDescription(NO_ID, id), DESCRIPTION); - // Exactly at expiry: removal is permissionless. - vm.warp(expiry); - vm.prank(stranger); + vm.prank(committee); strikes.removeStrike(NO_ID, id); - assertEq(strikes.getActiveStrikesCount(NO_ID), 0); + assertEq(strikes.getStrikeDescription(NO_ID, id), ""); } - function test_removeStrike_PermissionlessAfterExpiry() public { + function test_removeStrike_RevertWhen_NotCommittee() public { uint256 id = _issue(NO_ID); + bytes32 role = strikes.STRIKES_COMMITTEE_ROLE(); uint256 expiry = strikes.getStrike(NO_ID, id).expiry; - vm.warp(expiry + 1 days); + // Even after the lifetime elapses removeStrike stays committee-only; + // permissionless cleanup goes through removeExpiredStrikes. + vm.warp(expiry); + + expectRoleRevert(stranger, role); vm.prank(stranger); strikes.removeStrike(NO_ID, id); - - assertEq(strikes.getActiveStrikesCount(NO_ID), 0); } function test_removeStrike_RevertWhen_NonExistent() public { @@ -301,6 +331,100 @@ contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { } } +contract NodeOperatorStrikesRemoveExpiredTest is NodeOperatorStrikesBaseTest { + /// @dev Issues four strikes: id1/id3 short-lived (LIFETIME), id2/id4 long-lived (2x LIFETIME). + function _issueMixed() internal returns (uint256 id1, uint256 id2, uint256 id3, uint256 id4) { + vm.startPrank(committee); + id1 = strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME)); + id2 = strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME * 2)); + id3 = strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME)); + id4 = strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME * 2)); + vm.stopPrank(); + } + + function test_removeExpiredStrikes_AlternatingSurvivorsStayResolvable() public { + // Alternate expiring/surviving so back-to-front swap-pop must shuffle survivors repeatedly. + vm.startPrank(committee); + strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME)); // id1 expire + uint256 id2 = strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME * 2)); // keep + strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME)); // id3 expire + uint256 id4 = strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME * 2)); // keep + strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME)); // id5 expire + uint256 id6 = strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME * 2)); // keep + vm.stopPrank(); + + vm.warp(block.timestamp + LIFETIME); + vm.prank(stranger); + strikes.removeExpiredStrikes(NO_ID); + + assertEq(strikes.getActiveStrikesCount(NO_ID), 3); + assertEq(strikes.getExpiredStrikes(NO_ID).length, 0); + // Every survivor still resolves by its id (index stayed consistent through the shuffles). + assertEq(strikes.getStrike(NO_ID, id2).id, id2); + assertEq(strikes.getStrike(NO_ID, id4).id, id4); + assertEq(strikes.getStrike(NO_ID, id6).id, id6); + assertEq(strikes.getStrike(NO_ID, 1).id, 0); + assertEq(strikes.getStrike(NO_ID, 3).id, 0); + assertEq(strikes.getStrike(NO_ID, 5).id, 0); + } + + function test_removeExpiredStrikes_RemovesOnlyExpired() public { + (uint256 id1, uint256 id2, uint256 id3, uint256 id4) = _issueMixed(); + + vm.warp(block.timestamp + LIFETIME); // id1, id3 expired; id2, id4 still active + uint256 refreshesBefore = metaRegistryMock.refreshOperatorWeightCallCount(); + + vm.prank(stranger); // permissionless + strikes.removeExpiredStrikes(NO_ID); + + assertEq(strikes.getActiveStrikesCount(NO_ID), 2); + assertEq(strikes.getStrike(NO_ID, id1).id, 0); + assertEq(strikes.getStrike(NO_ID, id3).id, 0); + assertEq(strikes.getStrike(NO_ID, id2).id, id2); + assertEq(strikes.getStrike(NO_ID, id4).id, id4); + assertEq(strikes.getExpiredStrikes(NO_ID).length, 0); + assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), refreshesBefore + 1); // refreshed once + } + + function test_removeExpiredStrikes_KeepsIndexConsistent() public { + (, uint256 id2, , uint256 id4) = _issueMixed(); + + vm.warp(block.timestamp + LIFETIME); + vm.prank(stranger); + strikes.removeExpiredStrikes(NO_ID); + + // Survivors stay resolvable by id and removable (their slots were swapped during cleanup). + vm.prank(committee); + strikes.removeStrike(NO_ID, id2); + assertEq(strikes.getActiveStrikesCount(NO_ID), 1); + assertEq(strikes.getStrike(NO_ID, id4).id, id4); + } + + function test_removeExpiredStrikes_AllExpired() public { + _issue(NO_ID); + _issue(NO_ID); + _issue(NO_ID); + + vm.warp(block.timestamp + LIFETIME); + vm.prank(stranger); + strikes.removeExpiredStrikes(NO_ID); + + assertEq(strikes.getActiveStrikesCount(NO_ID), 0); + } + + function test_removeExpiredStrikes_NoopWhenNoneExpired() public { + _issue(NO_ID); + _issue(NO_ID); + uint256 refreshesBefore = metaRegistryMock.refreshOperatorWeightCallCount(); + + vm.prank(stranger); + strikes.removeExpiredStrikes(NO_ID); + + assertEq(strikes.getActiveStrikesCount(NO_ID), 2); + assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), refreshesBefore); // no refresh + } +} + contract NodeOperatorStrikesThresholdsTest is NodeOperatorStrikesBaseTest { function test_setStrikeThresholds_RoundTrip() public { StrikeThreshold[] memory thresholds = _exampleThresholds(); @@ -373,6 +497,15 @@ contract NodeOperatorStrikesThresholdsTest is NodeOperatorStrikesBaseTest { strikes.setStrikeThresholds(thresholds); } + function test_setStrikeThresholds_RevertWhen_ReductionNotIncreasing() public { + StrikeThreshold[] memory thresholds = new StrikeThreshold[](2); + thresholds[0] = StrikeThreshold({ minCount: 2, reductionBP: 2_500 }); + thresholds[1] = StrikeThreshold({ minCount: 3, reductionBP: 2_500 }); // equal -> redundant band + vm.expectRevert(INodeOperatorStrikes.InvalidStrikeThresholds.selector); + vm.prank(admin); + strikes.setStrikeThresholds(thresholds); + } + function test_setStrikeThresholds_RevertWhen_ReductionAboveMaxBp() public { StrikeThreshold[] memory thresholds = new StrikeThreshold[](1); thresholds[0] = StrikeThreshold({ minCount: 2, reductionBP: MAX_BP + 1 }); @@ -394,17 +527,7 @@ contract NodeOperatorStrikesThresholdsTest is NodeOperatorStrikesBaseTest { } contract NodeOperatorStrikesWeightMultiplierTest is NodeOperatorStrikesBaseTest { - function test_getStrikeWeightMultiplier_EmptyThresholdsIsMaxBP() public { - // No thresholds configured yet. - _issue(NO_ID); - _issue(NO_ID); - _issue(NO_ID); - assertEq(strikes.getStrikeWeightMultiplier(NO_ID), MAX_BP); - } - function test_getStrikeWeightMultiplier_StepFunction() public { - _setExampleThresholds(); - // 0 strikes -> full weight. assertEq(strikes.getStrikeWeightMultiplier(NO_ID), MAX_BP); @@ -428,8 +551,6 @@ contract NodeOperatorStrikesWeightMultiplierTest is NodeOperatorStrikesBaseTest } function test_getStrikeWeightMultiplier_IncreasesAfterRemoval() public { - _setExampleThresholds(); - uint256 id1 = _issue(NO_ID); _issue(NO_ID); _issue(NO_ID); // 3 active -> 50% From 3fb2cdf94a41fc60d1257f4c38f3df7591203b48 Mon Sep 17 00:00:00 2001 From: vgorkavenko Date: Fri, 26 Jun 2026 18:44:40 +0200 Subject: [PATCH 3/7] fix: remarks --- src/NodeOperatorStrikes.sol | 43 +++++++------------ src/interfaces/INodeOperatorStrikes.sol | 11 ++--- test/unit/NodeOperatorStrikes.t.sol | 55 ++++++++----------------- 3 files changed, 34 insertions(+), 75 deletions(-) diff --git a/src/NodeOperatorStrikes.sol b/src/NodeOperatorStrikes.sol index 4ad8beea6..d6fc02786 100644 --- a/src/NodeOperatorStrikes.sol +++ b/src/NodeOperatorStrikes.sol @@ -92,10 +92,8 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr /// @inheritdoc INodeOperatorStrikes function removeStrike(uint256 nodeOperatorId, uint256 strikeId) external onlyRole(STRIKES_COMMITTEE_ROLE) { OperatorStrikes storage rec = _storage().operatorStrikes[nodeOperatorId]; - uint256 position = rec.index[strikeId]; - if (position == 0) revert StrikeNotActive(); - - _popStrike(rec, nodeOperatorId, position - 1, strikeId); + _ensureStrikeExists(rec, strikeId); + _popStrike(rec, nodeOperatorId, rec.index[strikeId] - 1, strikeId); META_REGISTRY.refreshOperatorWeight(nodeOperatorId); } @@ -145,9 +143,8 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr /// @inheritdoc INodeOperatorStrikes function getStrike(uint256 nodeOperatorId, uint256 strikeId) external view returns (Strike memory strike) { OperatorStrikes storage rec = _storage().operatorStrikes[nodeOperatorId]; - uint256 position = rec.index[strikeId]; - if (position == 0) return strike; - return rec.active[position - 1]; + _ensureStrikeExists(rec, strikeId); + return rec.active[rec.index[strikeId] - 1]; } /// @inheritdoc INodeOperatorStrikes @@ -155,7 +152,9 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr uint256 nodeOperatorId, uint256 strikeId ) external view returns (string memory description) { - return _storage().operatorStrikes[nodeOperatorId].descriptions[strikeId]; + OperatorStrikes storage rec = _storage().operatorStrikes[nodeOperatorId]; + _ensureStrikeExists(rec, strikeId); + return rec.descriptions[strikeId]; } /// @inheritdoc INodeOperatorStrikes @@ -163,30 +162,16 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr return _storage().operatorStrikes[nodeOperatorId].active; } - /// @inheritdoc INodeOperatorStrikes - function getExpiredStrikes(uint256 nodeOperatorId) external view returns (uint256[] memory strikeIds) { - Strike[] storage active = _storage().operatorStrikes[nodeOperatorId].active; - uint256 len = active.length; - - strikeIds = new uint256[](len); - uint256 j; - for (uint256 i; i < len; ++i) { - if (active[i].expiry <= block.timestamp) { - strikeIds[j] = active[i].id; - ++j; - } - } - - assembly ("memory-safe") { - mstore(strikeIds, j) - } - } - /// @inheritdoc INodeOperatorStrikes function getStrikeThresholds() external view returns (StrikeThreshold[] memory thresholds) { return _storage().thresholds; } + /// @dev Reverts `StrikeNotExist` if the strike is removed or was never issued. + function _ensureStrikeExists(OperatorStrikes storage rec, uint256 strikeId) private view { + if (rec.index[strikeId] == 0) revert StrikeNotExist(); + } + /// @dev Swap-pops the strike at `idx` and clears its bookkeeping. Caller refreshes the weight. function _popStrike(OperatorStrikes storage rec, uint256 nodeOperatorId, uint256 idx, uint256 strikeId) private { uint256 lastIdx = rec.active.length - 1; @@ -202,7 +187,7 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr emit StrikeRemoved(nodeOperatorId, strikeId, msg.sender); } - function _setStrikeThresholds(StrikeThreshold[] calldata thresholds) internal { + function _setStrikeThresholds(StrikeThreshold[] calldata thresholds) private { _validateStrikeThresholds(thresholds); delete _storage().thresholds; @@ -214,7 +199,7 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr emit StrikeThresholdsSet(thresholds); } - function _onlyExistingOperator(uint256 nodeOperatorId) internal view { + function _onlyExistingOperator(uint256 nodeOperatorId) private view { if (nodeOperatorId >= MODULE.getNodeOperatorsCount()) revert NodeOperatorDoesNotExist(); } diff --git a/src/interfaces/INodeOperatorStrikes.sol b/src/interfaces/INodeOperatorStrikes.sol index 57a3aec17..4a4e5a932 100644 --- a/src/interfaces/INodeOperatorStrikes.sol +++ b/src/interfaces/INodeOperatorStrikes.sol @@ -42,7 +42,7 @@ interface INodeOperatorStrikes { error ZeroModuleAddress(); error ZeroAdminAddress(); error NodeOperatorDoesNotExist(); - error StrikeNotActive(); + error StrikeNotExist(); error InvalidLifetime(); error InvalidStrikeThresholds(); error DescriptionTooLong(); @@ -97,12 +97,12 @@ interface INodeOperatorStrikes { /// @param nodeOperatorId ID of the Node Operator. function getStrikeWeightMultiplier(uint256 nodeOperatorId) external view returns (uint256 multiplierBP); - /// @notice Return a single strike record (zeroed if removed or never issued). + /// @notice Return a single strike record. Reverts with `StrikeNotExist` if removed or never issued. /// @param nodeOperatorId ID of the Node Operator. /// @param strikeId ID of the strike. function getStrike(uint256 nodeOperatorId, uint256 strikeId) external view returns (Strike memory strike); - /// @notice On-chain description of a strike (empty if removed or never issued). + /// @notice On-chain description of a strike. Reverts with `StrikeNotExist` if removed or never issued. /// @param nodeOperatorId ID of the Node Operator. /// @param strikeId ID of the strike. function getStrikeDescription( @@ -114,11 +114,6 @@ interface INodeOperatorStrikes { /// @param nodeOperatorId ID of the Node Operator. function getStrikes(uint256 nodeOperatorId) external view returns (Strike[] memory strikes); - /// @notice Return IDs of active strikes whose lifetime has elapsed (`block.timestamp >= expiry`), - /// i.e. the ones that can be removed permissionlessly. - /// @param nodeOperatorId ID of the Node Operator. - function getExpiredStrikes(uint256 nodeOperatorId) external view returns (uint256[] memory strikeIds); - /// @notice Return the configured global weight-reduction thresholds. function getStrikeThresholds() external view returns (StrikeThreshold[] memory thresholds); } diff --git a/test/unit/NodeOperatorStrikes.t.sol b/test/unit/NodeOperatorStrikes.t.sol index 653055f93..918669b07 100644 --- a/test/unit/NodeOperatorStrikes.t.sol +++ b/test/unit/NodeOperatorStrikes.t.sol @@ -81,6 +81,11 @@ contract NodeOperatorStrikesBaseTest is Test, Utilities, Fixtures { vm.prank(committee); strikeId = strikes.issueStrike(_input(nodeOperatorId, CATEGORY, LIFETIME)); } + + function _expectNoStrike(uint256 nodeOperatorId, uint256 strikeId) internal { + vm.expectRevert(INodeOperatorStrikes.StrikeNotExist.selector); + strikes.getStrike(nodeOperatorId, strikeId); + } } contract NodeOperatorStrikesConstructorTest is NodeOperatorStrikesBaseTest { @@ -229,7 +234,7 @@ contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { strikes.removeStrike(NO_ID, id); assertEq(strikes.getActiveStrikesCount(NO_ID), 0); - assertEq(strikes.getStrike(NO_ID, id).id, 0); // removed -> zeroed slot + _expectNoStrike(NO_ID, id); // removed assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), refreshesBefore + 1); } @@ -240,7 +245,8 @@ contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { vm.prank(committee); strikes.removeStrike(NO_ID, id); - assertEq(strikes.getStrikeDescription(NO_ID, id), ""); + vm.expectRevert(INodeOperatorStrikes.StrikeNotExist.selector); + strikes.getStrikeDescription(NO_ID, id); } function test_removeStrike_RevertWhen_NotCommittee() public { @@ -258,7 +264,7 @@ contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { } function test_removeStrike_RevertWhen_NonExistent() public { - vm.expectRevert(INodeOperatorStrikes.StrikeNotActive.selector); + vm.expectRevert(INodeOperatorStrikes.StrikeNotExist.selector); vm.prank(committee); strikes.removeStrike(NO_ID, 1); } @@ -268,7 +274,7 @@ contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { vm.prank(committee); strikes.removeStrike(NO_ID, id); - vm.expectRevert(INodeOperatorStrikes.StrikeNotActive.selector); + vm.expectRevert(INodeOperatorStrikes.StrikeNotExist.selector); vm.prank(committee); strikes.removeStrike(NO_ID, id); } @@ -291,11 +297,10 @@ contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { // After lifetime elapses expired strikes stay in active until explicitly removed. vm.warp(block.timestamp + LIFETIME); assertEq(strikes.getActiveStrikesCount(NO_ID), 4); - assertEq(strikes.getExpiredStrikes(NO_ID).length, 4); // Gap at id 2 returns zeroed; all others are individually reachable. assertEq(strikes.getStrike(NO_ID, 1).id, 1); - assertEq(strikes.getStrike(NO_ID, 2).id, 0); + _expectNoStrike(NO_ID, 2); // gap assertEq(strikes.getStrike(NO_ID, 3).id, 3); assertEq(strikes.getStrike(NO_ID, 4).id, 4); assertEq(strikes.getStrike(NO_ID, 5).id, 5); @@ -327,7 +332,7 @@ contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { assertEq(strikes.getActiveStrikesCount(NO_ID), 1); assertEq(strikes.getStrike(NO_ID, id2).id, id2); - assertEq(strikes.getStrike(NO_ID, id3).id, 0); // removed + _expectNoStrike(NO_ID, id3); // removed } } @@ -358,14 +363,13 @@ contract NodeOperatorStrikesRemoveExpiredTest is NodeOperatorStrikesBaseTest { strikes.removeExpiredStrikes(NO_ID); assertEq(strikes.getActiveStrikesCount(NO_ID), 3); - assertEq(strikes.getExpiredStrikes(NO_ID).length, 0); // Every survivor still resolves by its id (index stayed consistent through the shuffles). assertEq(strikes.getStrike(NO_ID, id2).id, id2); assertEq(strikes.getStrike(NO_ID, id4).id, id4); assertEq(strikes.getStrike(NO_ID, id6).id, id6); - assertEq(strikes.getStrike(NO_ID, 1).id, 0); - assertEq(strikes.getStrike(NO_ID, 3).id, 0); - assertEq(strikes.getStrike(NO_ID, 5).id, 0); + _expectNoStrike(NO_ID, 1); + _expectNoStrike(NO_ID, 3); + _expectNoStrike(NO_ID, 5); } function test_removeExpiredStrikes_RemovesOnlyExpired() public { @@ -378,11 +382,10 @@ contract NodeOperatorStrikesRemoveExpiredTest is NodeOperatorStrikesBaseTest { strikes.removeExpiredStrikes(NO_ID); assertEq(strikes.getActiveStrikesCount(NO_ID), 2); - assertEq(strikes.getStrike(NO_ID, id1).id, 0); - assertEq(strikes.getStrike(NO_ID, id3).id, 0); + _expectNoStrike(NO_ID, id1); + _expectNoStrike(NO_ID, id3); assertEq(strikes.getStrike(NO_ID, id2).id, id2); assertEq(strikes.getStrike(NO_ID, id4).id, id4); - assertEq(strikes.getExpiredStrikes(NO_ID).length, 0); assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), refreshesBefore + 1); // refreshed once } @@ -575,28 +578,4 @@ contract NodeOperatorStrikesWeightMultiplierTest is NodeOperatorStrikesBaseTest assertEq(active[0].id, id1); assertEq(active[1].id, id3); } - - function test_getExpiredStrikes() public { - // Two short-lived strikes and one long-lived, all issued at the same time. - uint256 idA = _issue(NO_ID); // lifetime = LIFETIME - uint256 idB = _issue(NO_ID); // lifetime = LIFETIME - vm.prank(committee); - uint256 idC = strikes.issueStrike(_input(NO_ID, CATEGORY, LIFETIME * 2)); - - // Nothing expired yet. - assertEq(strikes.getExpiredStrikes(NO_ID).length, 0); - - // Warp to the short lifetime boundary: A and B expired, C not. - vm.warp(block.timestamp + LIFETIME); - - uint256[] memory expired = strikes.getExpiredStrikes(NO_ID); - assertEq(expired.length, 2); - assertEq(expired[0], idA); - assertEq(expired[1], idB); - assertGt(strikes.getStrike(NO_ID, idC).expiry, block.timestamp); - - // Warp past the long lifetime: all three expired. - vm.warp(strikes.getStrike(NO_ID, idC).expiry); - assertEq(strikes.getExpiredStrikes(NO_ID).length, 3); - } } From bc1724d765c2c6f5e2e85b8b56c8d04611c36912 Mon Sep 17 00:00:00 2001 From: vgorkavenko Date: Tue, 30 Jun 2026 10:32:54 +0200 Subject: [PATCH 4/7] refactor: strikes storage --- src/NodeOperatorStrikes.sol | 97 +++++++++++++------------ src/interfaces/INodeOperatorStrikes.sol | 18 ++--- test/unit/NodeOperatorStrikes.t.sol | 69 ++++++++++++------ 3 files changed, 104 insertions(+), 80 deletions(-) diff --git a/src/NodeOperatorStrikes.sol b/src/NodeOperatorStrikes.sol index d6fc02786..c0da27208 100644 --- a/src/NodeOperatorStrikes.sol +++ b/src/NodeOperatorStrikes.sol @@ -16,12 +16,9 @@ import { MAX_BP } from "./lib/Constants.sol"; /// removal is permissionless once a strike's lifetime elapses. contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessControlEnumerableUpgradeable { struct OperatorStrikes { - uint64 lastStrikeId; - Strike[] active; - /// @dev maps a strike id to its position in `active` (0 = not active). - mapping(uint256 strikeId => uint256 position) index; - /// @dev on-chain strike description; cleared on removal (empty if removed or never issued). - mapping(uint256 strikeId => string description) descriptions; + uint64 lastId; + uint256[] activeIds; + mapping(uint256 strikeId => Strike) strikes; } /// @custom:storage-location erc7201:NodeOperatorStrikes @@ -65,7 +62,8 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr ) external onlyRole(STRIKES_COMMITTEE_ROLE) returns (uint256 strikeId) { _onlyExistingOperator(input.nodeOperatorId); - if (bytes(input.description).length > MAX_DESCRIPTION_LENGTH) revert DescriptionTooLong(); + uint256 descLength = bytes(input.description).length; + if (descLength == 0 || descLength > MAX_DESCRIPTION_LENGTH) revert InvalidDescription(); uint256 lifetime = input.lifetime; if (lifetime == 0) revert InvalidLifetime(); @@ -73,10 +71,14 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr if (expiry > type(uint64).max) revert InvalidLifetime(); OperatorStrikes storage rec = _storage().operatorStrikes[input.nodeOperatorId]; - strikeId = ++rec.lastStrikeId; - rec.active.push(Strike({ id: uint64(strikeId), expiry: uint64(expiry), category: input.category })); - rec.index[strikeId] = rec.active.length; - rec.descriptions[strikeId] = input.description; + strikeId = ++rec.lastId; + rec.activeIds.push(strikeId); + rec.strikes[strikeId] = Strike({ + id: uint64(strikeId), + expiry: uint64(expiry), + category: input.category, + description: input.description + }); emit StrikeIssued({ nodeOperatorId: input.nodeOperatorId, @@ -92,8 +94,7 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr /// @inheritdoc INodeOperatorStrikes function removeStrike(uint256 nodeOperatorId, uint256 strikeId) external onlyRole(STRIKES_COMMITTEE_ROLE) { OperatorStrikes storage rec = _storage().operatorStrikes[nodeOperatorId]; - _ensureStrikeExists(rec, strikeId); - _popStrike(rec, nodeOperatorId, rec.index[strikeId] - 1, strikeId); + _removeStrike(rec, nodeOperatorId, _activeIndex(rec, strikeId), strikeId); META_REGISTRY.refreshOperatorWeight(nodeOperatorId); } @@ -101,14 +102,16 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr /// @inheritdoc INodeOperatorStrikes function removeExpiredStrikes(uint256 nodeOperatorId) external { OperatorStrikes storage rec = _storage().operatorStrikes[nodeOperatorId]; + uint256[] storage activeIds = rec.activeIds; - // Back-to-front: swap-pop only moves an already-kept strike down, so none are skipped. + // Back-to-front so swap-pop never skips an id. bool removed; - uint256 i = rec.active.length; + uint256 i = activeIds.length; while (i > 0) { --i; - if (rec.active[i].expiry > block.timestamp) continue; - _popStrike(rec, nodeOperatorId, i, rec.active[i].id); + uint256 strikeId = activeIds[i]; + if (rec.strikes[strikeId].expiry > block.timestamp) continue; + _removeStrike(rec, nodeOperatorId, i, strikeId); removed = true; } @@ -122,7 +125,7 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr /// @inheritdoc INodeOperatorStrikes function getActiveStrikesCount(uint256 nodeOperatorId) external view returns (uint256 count) { - return _storage().operatorStrikes[nodeOperatorId].active.length; + return _storage().operatorStrikes[nodeOperatorId].activeIds.length; } /// @inheritdoc INodeOperatorStrikes @@ -130,7 +133,7 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr multiplierBP = MAX_BP; NodeOperatorStrikesStorage storage $ = _storage(); - uint256 count = $.operatorStrikes[nodeOperatorId].active.length; + uint256 count = $.operatorStrikes[nodeOperatorId].activeIds.length; StrikeThreshold[] storage thresholds = $.thresholds; uint256 len = thresholds.length; @@ -142,24 +145,21 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr /// @inheritdoc INodeOperatorStrikes function getStrike(uint256 nodeOperatorId, uint256 strikeId) external view returns (Strike memory strike) { - OperatorStrikes storage rec = _storage().operatorStrikes[nodeOperatorId]; - _ensureStrikeExists(rec, strikeId); - return rec.active[rec.index[strikeId] - 1]; + strike = _storage().operatorStrikes[nodeOperatorId].strikes[strikeId]; + // expiry == 0 means removed or never issued. + if (strike.expiry == 0) revert StrikeNotExist(); } /// @inheritdoc INodeOperatorStrikes - function getStrikeDescription( - uint256 nodeOperatorId, - uint256 strikeId - ) external view returns (string memory description) { + function getStrikes(uint256 nodeOperatorId) external view returns (Strike[] memory strikes) { OperatorStrikes storage rec = _storage().operatorStrikes[nodeOperatorId]; - _ensureStrikeExists(rec, strikeId); - return rec.descriptions[strikeId]; - } + uint256[] storage activeIds = rec.activeIds; + uint256 len = activeIds.length; - /// @inheritdoc INodeOperatorStrikes - function getStrikes(uint256 nodeOperatorId) external view returns (Strike[] memory strikes) { - return _storage().operatorStrikes[nodeOperatorId].active; + strikes = new Strike[](len); + for (uint256 i; i < len; ++i) { + strikes[i] = rec.strikes[activeIds[i]]; + } } /// @inheritdoc INodeOperatorStrikes @@ -167,22 +167,25 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr return _storage().thresholds; } - /// @dev Reverts `StrikeNotExist` if the strike is removed or was never issued. - function _ensureStrikeExists(OperatorStrikes storage rec, uint256 strikeId) private view { - if (rec.index[strikeId] == 0) revert StrikeNotExist(); + /// @dev Index of `strikeId` in `activeIds`; reverts `StrikeNotExist` if absent. + function _activeIndex(OperatorStrikes storage rec, uint256 strikeId) private view returns (uint256) { + uint256[] storage activeIds = rec.activeIds; + uint256 len = activeIds.length; + for (uint256 i; i < len; ++i) { + if (activeIds[i] == strikeId) return i; + } + revert StrikeNotExist(); } - /// @dev Swap-pops the strike at `idx` and clears its bookkeeping. Caller refreshes the weight. - function _popStrike(OperatorStrikes storage rec, uint256 nodeOperatorId, uint256 idx, uint256 strikeId) private { - uint256 lastIdx = rec.active.length - 1; + /// @dev Swap-pops the id, deletes the record, emits. Caller refreshes the weight (once per batch). + function _removeStrike(OperatorStrikes storage rec, uint256 nodeOperatorId, uint256 idx, uint256 strikeId) private { + uint256[] storage activeIds = rec.activeIds; + uint256 lastIdx = activeIds.length - 1; if (idx != lastIdx) { - Strike memory moved = rec.active[lastIdx]; - rec.active[idx] = moved; - rec.index[moved.id] = idx + 1; // 1-based position + activeIds[idx] = activeIds[lastIdx]; } - rec.active.pop(); - delete rec.index[strikeId]; - delete rec.descriptions[strikeId]; + activeIds.pop(); + delete rec.strikes[strikeId]; emit StrikeRemoved(nodeOperatorId, strikeId, msg.sender); } @@ -190,10 +193,10 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr function _setStrikeThresholds(StrikeThreshold[] calldata thresholds) private { _validateStrikeThresholds(thresholds); - delete _storage().thresholds; - StrikeThreshold[] storage stored = _storage().thresholds; + NodeOperatorStrikesStorage storage $ = _storage(); + delete $.thresholds; for (uint256 i; i < thresholds.length; ++i) { - stored.push(thresholds[i]); + $.thresholds.push(thresholds[i]); } emit StrikeThresholdsSet(thresholds); diff --git a/src/interfaces/INodeOperatorStrikes.sol b/src/interfaces/INodeOperatorStrikes.sol index 4a4e5a932..86933040c 100644 --- a/src/interfaces/INodeOperatorStrikes.sol +++ b/src/interfaces/INodeOperatorStrikes.sol @@ -6,7 +6,7 @@ pragma solidity 0.8.33; import { ICuratedModule } from "./ICuratedModule.sol"; import { IMetaRegistry } from "./IMetaRegistry.sol"; -/// @dev Payload describing a strike to issue. For the stored `description` see `getStrikeDescription`. +/// @dev Payload describing a strike to issue. struct StrikeInput { uint256 nodeOperatorId; bytes32 category; @@ -14,11 +14,13 @@ struct StrikeInput { string description; } -/// @dev Removed or never-issued strike reads as a zeroed slot (`id == 0`). +/// @dev Stored strike record (kept in a mapping by id, so the struct can grow without migration). +/// A live strike has `expiry != 0`; a removed or never-issued one reads as zeroed. struct Strike { uint64 id; uint64 expiry; bytes32 category; + string description; } /// @dev Cumulative weight reduction step. At `minCount` active strikes the operator's weight is @@ -45,7 +47,7 @@ interface INodeOperatorStrikes { error StrikeNotExist(); error InvalidLifetime(); error InvalidStrikeThresholds(); - error DescriptionTooLong(); + error InvalidDescription(); /// @notice Role allowed to issue and remove strikes. function STRIKES_COMMITTEE_ROLE() external view returns (bytes32); @@ -102,15 +104,7 @@ interface INodeOperatorStrikes { /// @param strikeId ID of the strike. function getStrike(uint256 nodeOperatorId, uint256 strikeId) external view returns (Strike memory strike); - /// @notice On-chain description of a strike. Reverts with `StrikeNotExist` if removed or never issued. - /// @param nodeOperatorId ID of the Node Operator. - /// @param strikeId ID of the strike. - function getStrikeDescription( - uint256 nodeOperatorId, - uint256 strikeId - ) external view returns (string memory description); - - /// @notice Return the active (non-removed) strikes of a Node Operator. + /// @notice Return all of a Node Operator's active (non-removed) strikes. /// @param nodeOperatorId ID of the Node Operator. function getStrikes(uint256 nodeOperatorId) external view returns (Strike[] memory strikes); diff --git a/test/unit/NodeOperatorStrikes.t.sol b/test/unit/NodeOperatorStrikes.t.sol index 918669b07..5be53d3c1 100644 --- a/test/unit/NodeOperatorStrikes.t.sol +++ b/test/unit/NodeOperatorStrikes.t.sol @@ -164,7 +164,7 @@ contract NodeOperatorStrikesIssueTest is NodeOperatorStrikesBaseTest { assertEq(s.id, 1); assertEq(s.expiry, expiry); assertEq(s.category, CATEGORY); - assertEq(strikes.getStrikeDescription(NO_ID, 1), DESCRIPTION); + assertEq(s.description, DESCRIPTION); assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), 1); assertEq(metaRegistryMock.lastRefreshedOperatorId(), NO_ID); @@ -209,21 +209,30 @@ contract NodeOperatorStrikesIssueTest is NodeOperatorStrikesBaseTest { vm.prank(committee); uint256 id = strikes.issueStrike(input); - assertEq(bytes(strikes.getStrikeDescription(NO_ID, id)).length, maxLen); + assertEq(bytes(strikes.getStrike(NO_ID, id).description).length, maxLen); } function test_issueStrike_RevertWhen_DescriptionTooLong() public { StrikeInput memory input = _input(NO_ID, CATEGORY, LIFETIME); input.description = string(new bytes(strikes.MAX_DESCRIPTION_LENGTH() + 1)); - vm.expectRevert(INodeOperatorStrikes.DescriptionTooLong.selector); + vm.expectRevert(INodeOperatorStrikes.InvalidDescription.selector); + vm.prank(committee); + strikes.issueStrike(input); + } + + function test_issueStrike_RevertWhen_EmptyDescription() public { + StrikeInput memory input = _input(NO_ID, CATEGORY, LIFETIME); + input.description = ""; + + vm.expectRevert(INodeOperatorStrikes.InvalidDescription.selector); vm.prank(committee); strikes.issueStrike(input); } } contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { - function test_removeStrike_CommitteeBeforeExpiry() public { + function test_removeStrike_RemovesAndRefreshes() public { uint256 id = _issue(NO_ID); uint256 refreshesBefore = metaRegistryMock.refreshOperatorWeightCallCount(); @@ -238,17 +247,6 @@ contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), refreshesBefore + 1); } - function test_removeStrike_ClearsDescription() public { - uint256 id = _issue(NO_ID); - assertEq(strikes.getStrikeDescription(NO_ID, id), DESCRIPTION); - - vm.prank(committee); - strikes.removeStrike(NO_ID, id); - - vm.expectRevert(INodeOperatorStrikes.StrikeNotExist.selector); - strikes.getStrikeDescription(NO_ID, id); - } - function test_removeStrike_RevertWhen_NotCommittee() public { uint256 id = _issue(NO_ID); bytes32 role = strikes.STRIKES_COMMITTEE_ROLE(); @@ -298,7 +296,7 @@ contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { vm.warp(block.timestamp + LIFETIME); assertEq(strikes.getActiveStrikesCount(NO_ID), 4); - // Gap at id 2 returns zeroed; all others are individually reachable. + // Gap at id 2 reverts; all others are individually reachable. assertEq(strikes.getStrike(NO_ID, 1).id, 1); _expectNoStrike(NO_ID, 2); // gap assertEq(strikes.getStrike(NO_ID, 3).id, 3); @@ -313,7 +311,7 @@ contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { assertEq(idSum, 1 + 3 + 4 + 5); } - function test_removeStrike_SwapPopKeepsIndexConsistent() public { + function test_removeStrike_SwapPopResolvesById() public { _issue(NO_ID); // id 1 uint256 id2 = _issue(NO_ID); uint256 id3 = _issue(NO_ID); @@ -389,14 +387,14 @@ contract NodeOperatorStrikesRemoveExpiredTest is NodeOperatorStrikesBaseTest { assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), refreshesBefore + 1); // refreshed once } - function test_removeExpiredStrikes_KeepsIndexConsistent() public { + function test_removeExpiredStrikes_SurvivorStaysRemovable() public { (, uint256 id2, , uint256 id4) = _issueMixed(); vm.warp(block.timestamp + LIFETIME); vm.prank(stranger); strikes.removeExpiredStrikes(NO_ID); - // Survivors stay resolvable by id and removable (their slots were swapped during cleanup). + // A survivor stays removable by the committee (its slot was swapped during cleanup). vm.prank(committee); strikes.removeStrike(NO_ID, id2); assertEq(strikes.getActiveStrikesCount(NO_ID), 1); @@ -500,7 +498,7 @@ contract NodeOperatorStrikesThresholdsTest is NodeOperatorStrikesBaseTest { strikes.setStrikeThresholds(thresholds); } - function test_setStrikeThresholds_RevertWhen_ReductionNotIncreasing() public { + function test_setStrikeThresholds_RevertWhen_ReductionEqual() public { StrikeThreshold[] memory thresholds = new StrikeThreshold[](2); thresholds[0] = StrikeThreshold({ minCount: 2, reductionBP: 2_500 }); thresholds[1] = StrikeThreshold({ minCount: 3, reductionBP: 2_500 }); // equal -> redundant band @@ -564,7 +562,7 @@ contract NodeOperatorStrikesWeightMultiplierTest is NodeOperatorStrikesBaseTest assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 7_500); } - function test_getStrikes_ReturnsActiveOnlyWithIds() public { + function test_getStrikes_ExcludesRemoved() public { uint256 id1 = _issue(NO_ID); uint256 id2 = _issue(NO_ID); uint256 id3 = _issue(NO_ID); @@ -578,4 +576,33 @@ contract NodeOperatorStrikesWeightMultiplierTest is NodeOperatorStrikesBaseTest assertEq(active[0].id, id1); assertEq(active[1].id, id3); } + + function test_getStrikes() public { + uint256 t = block.timestamp; + bytes32 catA = keccak256("late-attestations"); + bytes32 catB = keccak256("missed-proposal"); + + vm.startPrank(committee); + uint256 idA = strikes.issueStrike( + StrikeInput({ nodeOperatorId: NO_ID, category: catA, lifetime: LIFETIME, description: "first" }) + ); + uint256 idB = strikes.issueStrike( + StrikeInput({ nodeOperatorId: NO_ID, category: catB, lifetime: LIFETIME * 2, description: "second" }) + ); + vm.stopPrank(); + + Strike[] memory active = strikes.getStrikes(NO_ID); + assertEq(active.length, 2); + + // Each record carries its own distinct fields. + assertEq(active[0].id, idA); + assertEq(active[0].category, catA); + assertEq(uint256(active[0].expiry), t + LIFETIME); + assertEq(active[0].description, "first"); + + assertEq(active[1].id, idB); + assertEq(active[1].category, catB); + assertEq(uint256(active[1].expiry), t + LIFETIME * 2); + assertEq(active[1].description, "second"); + } } From 388c1225aa2b6ec1194ed5696e77d52182085c4c Mon Sep 17 00:00:00 2001 From: vgorkavenko Date: Wed, 8 Jul 2026 10:50:21 +0200 Subject: [PATCH 5/7] feat: connect to MR --- script/curated/DeployBase.s.sol | 29 +++++++++++++++++ script/curated/DeployHoodi.s.sol | 9 ++++++ script/curated/DeployLocalDevNet.s.sol | 8 +++++ script/curated/DeployMainnet.s.sol | 9 ++++++ src/NodeOperatorStrikes.sol | 13 +++++--- src/interfaces/INodeOperatorStrikes.sol | 13 +++----- test/helpers/mocks/MetaRegistryMock.sol | 5 +++ test/unit/NodeOperatorStrikes.t.sol | 41 +++++++++++++------------ 8 files changed, 95 insertions(+), 32 deletions(-) diff --git a/script/curated/DeployBase.s.sol b/script/curated/DeployBase.s.sol index 9cefdfcb8..61656e4b8 100644 --- a/script/curated/DeployBase.s.sol +++ b/script/curated/DeployBase.s.sol @@ -18,6 +18,7 @@ import { ParametersRegistry } from "../../src/ParametersRegistry.sol"; import { ExitPenalties } from "../../src/ExitPenalties.sol"; import { MetaRegistry } from "../../src/MetaRegistry.sol"; import { AdditionalBondRegistry } from "../../src/AdditionalBondRegistry.sol"; +import { NodeOperatorStrikes } from "../../src/NodeOperatorStrikes.sol"; import { CuratedGate } from "../../src/CuratedGate.sol"; import { MerkleGateFactory } from "../../src/MerkleGateFactory.sol"; @@ -29,6 +30,7 @@ import { IParametersRegistry } from "../../src/interfaces/IParametersRegistry.so import { IBondCurve } from "../../src/interfaces/IBondCurve.sol"; import { IMetaRegistry } from "../../src/interfaces/IMetaRegistry.sol"; import { IWeightBoostProvider } from "../../src/interfaces/IWeightBoostProvider.sol"; +import { StrikeThreshold } from "../../src/interfaces/INodeOperatorStrikes.sol"; import { JsonObj, Json } from "../utils/Json.sol"; import { Dummy } from "../utils/Dummy.sol"; @@ -130,6 +132,9 @@ struct CuratedDeployParams { address secondAdminAddress; // AdditionalBondRegistry AdditionalBondRegistryConfig additionalBondRegistryConfig; + // NodeOperatorStrikes + address strikesCommittee; + StrikeThreshold[] strikesThresholds; } abstract contract DeployBase is Script { @@ -153,6 +158,7 @@ abstract contract DeployBase is Script { ParametersRegistry public parametersRegistry; MetaRegistry public metaRegistry; AdditionalBondRegistry public additionalBondRegistry; + NodeOperatorStrikes public nodeOperatorStrikes; MerkleGateFactory public curatedGateFactory; address[] public curatedGateInstances; address internal curatedGateImpl; @@ -242,6 +248,7 @@ abstract contract DeployBase is Script { oracle = FeeOracle(_deployProxy(deployer, address(dummyImpl))); metaRegistry = MetaRegistry(_deployProxy(deployer, address(dummyImpl))); additionalBondRegistry = AdditionalBondRegistry(_deployProxy(deployer, address(dummyImpl))); + nodeOperatorStrikes = NodeOperatorStrikes(_deployProxy(deployer, address(dummyImpl))); FeeDistributor feeDistributorImpl = new FeeDistributor({ stETH: locator.lido(), @@ -348,12 +355,28 @@ abstract contract DeployBase is Script { additionalBondRegistryProxy.proxy__changeAdmin(config.proxyAdmin); } + NodeOperatorStrikes nodeOperatorStrikesImpl = new NodeOperatorStrikes({ module: address(curatedModule) }); + + { + OssifiableProxy nodeOperatorStrikesProxy = OssifiableProxy(payable(address(nodeOperatorStrikes))); + nodeOperatorStrikesProxy.proxy__upgradeToAndCall( + address(nodeOperatorStrikesImpl), + abi.encodeCall(NodeOperatorStrikes.initialize, (deployer, config.strikesThresholds)) + ); + nodeOperatorStrikesProxy.proxy__changeAdmin(config.proxyAdmin); + } + accounting.grantRole(accounting.MANAGE_BOND_CURVES_ROLE(), address(deployer)); accounting.grantRole(accounting.SET_BOND_CURVE_MULTIPLIER_ROLE(), address(additionalBondRegistry)); metaRegistry.addWeightBoostProvider( IWeightBoostProvider(address(additionalBondRegistry)), IMetaRegistry.WeightBoostProviderMode.NodeOperator ); + metaRegistry.addWeightBoostProvider( + IWeightBoostProvider(address(nodeOperatorStrikes)), + IMetaRegistry.WeightBoostProviderMode.NodeOperator + ); + nodeOperatorStrikes.grantRole(nodeOperatorStrikes.STRIKES_COMMITTEE_ROLE(), config.strikesCommittee); metaRegistry.grantRole(metaRegistry.SET_BOND_CURVE_WEIGHT_ROLE(), deployer); for (uint256 i = 0; i < gatesCount; i++) { @@ -554,6 +577,9 @@ abstract contract DeployBase is Script { additionalBondRegistry.grantRole(additionalBondRegistry.DEFAULT_ADMIN_ROLE(), config.aragonAgent); additionalBondRegistry.revokeRole(additionalBondRegistry.DEFAULT_ADMIN_ROLE(), deployer); + nodeOperatorStrikes.grantRole(nodeOperatorStrikes.DEFAULT_ADMIN_ROLE(), config.aragonAgent); + nodeOperatorStrikes.revokeRole(nodeOperatorStrikes.DEFAULT_ADMIN_ROLE(), deployer); + verifier.grantRole(verifier.DEFAULT_ADMIN_ROLE(), config.aragonAgent); verifier.revokeRole(verifier.DEFAULT_ADMIN_ROLE(), deployer); @@ -580,6 +606,8 @@ abstract contract DeployBase is Script { deployJson.set("MetaRegistryImpl", address(metaRegistryImpl)); deployJson.set("AdditionalBondRegistry", address(additionalBondRegistry)); deployJson.set("AdditionalBondRegistryImpl", address(additionalBondRegistryImpl)); + deployJson.set("NodeOperatorStrikes", address(nodeOperatorStrikes)); + deployJson.set("NodeOperatorStrikesImpl", address(nodeOperatorStrikesImpl)); deployJson.set("ParametersRegistry", address(parametersRegistry)); deployJson.set("ParametersRegistryImpl", address(parametersRegistryImpl)); deployJson.set("Accounting", address(accounting)); @@ -676,6 +704,7 @@ abstract contract DeployBase is Script { parametersRegistry.grantRole(parametersRegistry.DEFAULT_ADMIN_ROLE(), config.secondAdminAddress); metaRegistry.grantRole(metaRegistry.DEFAULT_ADMIN_ROLE(), config.secondAdminAddress); additionalBondRegistry.grantRole(additionalBondRegistry.DEFAULT_ADMIN_ROLE(), config.secondAdminAddress); + nodeOperatorStrikes.grantRole(nodeOperatorStrikes.DEFAULT_ADMIN_ROLE(), config.secondAdminAddress); for (uint256 i = 0; i < curatedGateInstances.length; i++) { CuratedGate gate = CuratedGate(curatedGateInstances[i]); gate.grantRole(gate.DEFAULT_ADMIN_ROLE(), config.secondAdminAddress); diff --git a/script/curated/DeployHoodi.s.sol b/script/curated/DeployHoodi.s.sol index f2a7f3124..a4894e1a9 100644 --- a/script/curated/DeployHoodi.s.sol +++ b/script/curated/DeployHoodi.s.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.33; import { DeployBase, CuratedGateConfig, AdditionalBondRegistryConfig } from "./DeployBase.s.sol"; +import { StrikeThreshold } from "../../src/interfaces/INodeOperatorStrikes.sol"; import { GIndices } from "../constants/GIndices.sol"; contract DeployHoodi is DeployBase { @@ -202,6 +203,14 @@ contract DeployHoodi is DeployBase { // CurveMultiplier config.additionalBondRegistryConfig.curveMultiplierCooldown = 7 days; + // NodeOperatorStrikes + config.strikesCommittee = 0x84DffcfB232594975C608DE92544Ff239a24c9E9; // CMC on Hoodi + // TODO: finalize strike weight-reduction thresholds + config.strikesThresholds.push(StrikeThreshold({ minCount: 2, reductionBP: 2_500 })); + config.strikesThresholds.push(StrikeThreshold({ minCount: 3, reductionBP: 5_000 })); + config.strikesThresholds.push(StrikeThreshold({ minCount: 4, reductionBP: 7_500 })); + config.strikesThresholds.push(StrikeThreshold({ minCount: 5, reductionBP: 10_000 })); + _setUp(); } } diff --git a/script/curated/DeployLocalDevNet.s.sol b/script/curated/DeployLocalDevNet.s.sol index 2944c46a1..504ba3434 100644 --- a/script/curated/DeployLocalDevNet.s.sol +++ b/script/curated/DeployLocalDevNet.s.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.33; import { DeployBase, CuratedGateConfig, AdditionalBondRegistryConfig } from "./DeployBase.s.sol"; +import { StrikeThreshold } from "../../src/interfaces/INodeOperatorStrikes.sol"; import { GIndices } from "../constants/GIndices.sol"; import { BaseOracle } from "../../src/lib/base-oracle/BaseOracle.sol"; import { HashConsensus } from "../../src/lib/base-oracle/HashConsensus.sol"; @@ -190,6 +191,13 @@ contract DeployLocalDevNet is DeployBase { // CurveMultiplier config.additionalBondRegistryConfig.curveMultiplierCooldown = 1 days; + // NodeOperatorStrikes + config.strikesCommittee = vm.envAddress("CSM_FIRST_ADMIN_ADDRESS"); // Dev team EOA + config.strikesThresholds.push(StrikeThreshold({ minCount: 2, reductionBP: 2_500 })); + config.strikesThresholds.push(StrikeThreshold({ minCount: 3, reductionBP: 5_000 })); + config.strikesThresholds.push(StrikeThreshold({ minCount: 4, reductionBP: 7_500 })); + config.strikesThresholds.push(StrikeThreshold({ minCount: 5, reductionBP: 10_000 })); + _setUp(); } diff --git a/script/curated/DeployMainnet.s.sol b/script/curated/DeployMainnet.s.sol index 930c9d558..71c3fccaf 100644 --- a/script/curated/DeployMainnet.s.sol +++ b/script/curated/DeployMainnet.s.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.33; import { DeployBase, CuratedGateConfig, AdditionalBondRegistryConfig } from "./DeployBase.s.sol"; +import { StrikeThreshold } from "../../src/interfaces/INodeOperatorStrikes.sol"; import { GIndices } from "../constants/GIndices.sol"; contract DeployMainnet is DeployBase { @@ -201,6 +202,14 @@ contract DeployMainnet is DeployBase { // CurveMultiplier config.additionalBondRegistryConfig.curveMultiplierCooldown = 7 days; + // NodeOperatorStrikes + config.strikesCommittee = 0x2570e0b22AD904501dfB0d49575991ACB801dD91; // CMC https://docs.lido.fi/multisigs/committees#220-curated-module-committee-cmc + // TODO: finalize strike weight-reduction thresholds + config.strikesThresholds.push(StrikeThreshold({ minCount: 2, reductionBP: 2_500 })); + config.strikesThresholds.push(StrikeThreshold({ minCount: 3, reductionBP: 5_000 })); + config.strikesThresholds.push(StrikeThreshold({ minCount: 4, reductionBP: 7_500 })); + config.strikesThresholds.push(StrikeThreshold({ minCount: 5, reductionBP: 10_000 })); + _setUp(); } } diff --git a/src/NodeOperatorStrikes.sol b/src/NodeOperatorStrikes.sol index c0da27208..d6993bfbe 100644 --- a/src/NodeOperatorStrikes.sol +++ b/src/NodeOperatorStrikes.sol @@ -9,6 +9,7 @@ import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/I import { ICuratedModule } from "./interfaces/ICuratedModule.sol"; import { IMetaRegistry } from "./interfaces/IMetaRegistry.sol"; import { INodeOperatorStrikes, StrikeInput, Strike, StrikeThreshold } from "./interfaces/INodeOperatorStrikes.sol"; +import { IWeightBoostProvider } from "./interfaces/IWeightBoostProvider.sol"; import { MAX_BP } from "./lib/Constants.sol"; /// @notice Committee-issued, operator-level strikes that cumulatively reduce @@ -88,7 +89,7 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr description: input.description }); - META_REGISTRY.refreshOperatorWeight(input.nodeOperatorId); + META_REGISTRY.notifyWeightBoostChanged(input.nodeOperatorId); } /// @inheritdoc INodeOperatorStrikes @@ -96,7 +97,7 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr OperatorStrikes storage rec = _storage().operatorStrikes[nodeOperatorId]; _removeStrike(rec, nodeOperatorId, _activeIndex(rec, strikeId), strikeId); - META_REGISTRY.refreshOperatorWeight(nodeOperatorId); + META_REGISTRY.notifyWeightBoostChanged(nodeOperatorId); } /// @inheritdoc INodeOperatorStrikes @@ -115,12 +116,13 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr removed = true; } - if (removed) META_REGISTRY.refreshOperatorWeight(nodeOperatorId); + if (removed) META_REGISTRY.notifyWeightBoostChanged(nodeOperatorId); } /// @inheritdoc INodeOperatorStrikes function setStrikeThresholds(StrikeThreshold[] calldata thresholds) external onlyRole(DEFAULT_ADMIN_ROLE) { _setStrikeThresholds(thresholds); + META_REGISTRY.notifyWeightBoostProviderConfigChanged(); } /// @inheritdoc INodeOperatorStrikes @@ -128,8 +130,9 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr return _storage().operatorStrikes[nodeOperatorId].activeIds.length; } - /// @inheritdoc INodeOperatorStrikes - function getStrikeWeightMultiplier(uint256 nodeOperatorId) external view returns (uint256 multiplierBP) { + /// @inheritdoc IWeightBoostProvider + /// @dev Counts strikes regardless of expiry: an expired one keeps reducing the weight until removed. + function getWeightBoostMultiplierBP(uint256 nodeOperatorId) external view returns (uint256 multiplierBP) { multiplierBP = MAX_BP; NodeOperatorStrikesStorage storage $ = _storage(); diff --git a/src/interfaces/INodeOperatorStrikes.sol b/src/interfaces/INodeOperatorStrikes.sol index 86933040c..df25ade7e 100644 --- a/src/interfaces/INodeOperatorStrikes.sol +++ b/src/interfaces/INodeOperatorStrikes.sol @@ -5,6 +5,7 @@ pragma solidity 0.8.33; import { ICuratedModule } from "./ICuratedModule.sol"; import { IMetaRegistry } from "./IMetaRegistry.sol"; +import { IWeightBoostProvider } from "./IWeightBoostProvider.sol"; /// @dev Payload describing a strike to issue. struct StrikeInput { @@ -30,7 +31,8 @@ struct StrikeThreshold { uint256 reductionBP; } -interface INodeOperatorStrikes { +/// @notice Committee-issued strikes act as a weight-reduction provider consumed by MetaRegistry. +interface INodeOperatorStrikes is IWeightBoostProvider { event StrikeIssued( uint256 indexed nodeOperatorId, uint256 indexed strikeId, @@ -61,7 +63,7 @@ interface INodeOperatorStrikes { /// @notice Curated module used to check operator existence. function MODULE() external view returns (ICuratedModule); - /// @notice MetaRegistry called back via `refreshOperatorWeight` on every strike change. + /// @notice MetaRegistry called back via `notifyWeightBoostChanged` on every strike change. function META_REGISTRY() external view returns (IMetaRegistry); /// @notice Initialize the contract. @@ -86,7 +88,7 @@ interface INodeOperatorStrikes { function removeExpiredStrikes(uint256 nodeOperatorId) external; /// @notice Set the global weight-reduction thresholds (callable by DEFAULT_ADMIN_ROLE). - /// @dev MUST be paired with `MetaRegistry.refreshOperatorWeight` for every operator with active strikes. + /// @dev Notifies MetaRegistry of the config change so affected operator weights are refreshed. /// @param thresholds Step function mapping active strike count to weight reduction. function setStrikeThresholds(StrikeThreshold[] calldata thresholds) external; @@ -94,11 +96,6 @@ interface INodeOperatorStrikes { /// @param nodeOperatorId ID of the Node Operator. function getActiveStrikesCount(uint256 nodeOperatorId) external view returns (uint256 count); - /// @notice Weight multiplier (in basis points) implied by the operator's active strike count. - /// @dev Counts strikes regardless of expiry: an expired strike keeps reducing the weight until removed. - /// @param nodeOperatorId ID of the Node Operator. - function getStrikeWeightMultiplier(uint256 nodeOperatorId) external view returns (uint256 multiplierBP); - /// @notice Return a single strike record. Reverts with `StrikeNotExist` if removed or never issued. /// @param nodeOperatorId ID of the Node Operator. /// @param strikeId ID of the strike. diff --git a/test/helpers/mocks/MetaRegistryMock.sol b/test/helpers/mocks/MetaRegistryMock.sol index 8178c1164..5a042d8cb 100644 --- a/test/helpers/mocks/MetaRegistryMock.sol +++ b/test/helpers/mocks/MetaRegistryMock.sol @@ -8,6 +8,7 @@ import { IMetaRegistry, OperatorMetadata } from "src/interfaces/IMetaRegistry.so contract MetaRegistryMock { uint256 public notifyWeightBoostChangedCallCount; uint256 public lastChangedBoostOperatorId; + uint256 public notifyConfigChangedCallCount; function setOperatorMetadataAsAdmin(uint256 nodeOperatorId, OperatorMetadata calldata metadata) external { emit IMetaRegistry.OperatorMetadataSet({ nodeOperatorId: nodeOperatorId, metadata: metadata }); @@ -17,4 +18,8 @@ contract MetaRegistryMock { notifyWeightBoostChangedCallCount++; lastChangedBoostOperatorId = nodeOperatorId; } + + function notifyWeightBoostProviderConfigChanged() external { + notifyConfigChangedCallCount++; + } } diff --git a/test/unit/NodeOperatorStrikes.t.sol b/test/unit/NodeOperatorStrikes.t.sol index 5be53d3c1..b5045b3cf 100644 --- a/test/unit/NodeOperatorStrikes.t.sol +++ b/test/unit/NodeOperatorStrikes.t.sol @@ -166,8 +166,8 @@ contract NodeOperatorStrikesIssueTest is NodeOperatorStrikesBaseTest { assertEq(s.category, CATEGORY); assertEq(s.description, DESCRIPTION); - assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), 1); - assertEq(metaRegistryMock.lastRefreshedOperatorId(), NO_ID); + assertEq(metaRegistryMock.notifyWeightBoostChangedCallCount(), 1); + assertEq(metaRegistryMock.lastChangedBoostOperatorId(), NO_ID); } function test_issueStrike_AssignsSequentialIds() public { @@ -234,7 +234,7 @@ contract NodeOperatorStrikesIssueTest is NodeOperatorStrikesBaseTest { contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { function test_removeStrike_RemovesAndRefreshes() public { uint256 id = _issue(NO_ID); - uint256 refreshesBefore = metaRegistryMock.refreshOperatorWeightCallCount(); + uint256 refreshesBefore = metaRegistryMock.notifyWeightBoostChangedCallCount(); vm.expectEmit(true, true, true, true, address(strikes)); emit INodeOperatorStrikes.StrikeRemoved(NO_ID, id, committee); @@ -244,7 +244,7 @@ contract NodeOperatorStrikesRemoveTest is NodeOperatorStrikesBaseTest { assertEq(strikes.getActiveStrikesCount(NO_ID), 0); _expectNoStrike(NO_ID, id); // removed - assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), refreshesBefore + 1); + assertEq(metaRegistryMock.notifyWeightBoostChangedCallCount(), refreshesBefore + 1); } function test_removeStrike_RevertWhen_NotCommittee() public { @@ -374,7 +374,7 @@ contract NodeOperatorStrikesRemoveExpiredTest is NodeOperatorStrikesBaseTest { (uint256 id1, uint256 id2, uint256 id3, uint256 id4) = _issueMixed(); vm.warp(block.timestamp + LIFETIME); // id1, id3 expired; id2, id4 still active - uint256 refreshesBefore = metaRegistryMock.refreshOperatorWeightCallCount(); + uint256 refreshesBefore = metaRegistryMock.notifyWeightBoostChangedCallCount(); vm.prank(stranger); // permissionless strikes.removeExpiredStrikes(NO_ID); @@ -384,7 +384,7 @@ contract NodeOperatorStrikesRemoveExpiredTest is NodeOperatorStrikesBaseTest { _expectNoStrike(NO_ID, id3); assertEq(strikes.getStrike(NO_ID, id2).id, id2); assertEq(strikes.getStrike(NO_ID, id4).id, id4); - assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), refreshesBefore + 1); // refreshed once + assertEq(metaRegistryMock.notifyWeightBoostChangedCallCount(), refreshesBefore + 1); // refreshed once } function test_removeExpiredStrikes_SurvivorStaysRemovable() public { @@ -416,13 +416,13 @@ contract NodeOperatorStrikesRemoveExpiredTest is NodeOperatorStrikesBaseTest { function test_removeExpiredStrikes_NoopWhenNoneExpired() public { _issue(NO_ID); _issue(NO_ID); - uint256 refreshesBefore = metaRegistryMock.refreshOperatorWeightCallCount(); + uint256 refreshesBefore = metaRegistryMock.notifyWeightBoostChangedCallCount(); vm.prank(stranger); strikes.removeExpiredStrikes(NO_ID); assertEq(strikes.getActiveStrikesCount(NO_ID), 2); - assertEq(metaRegistryMock.refreshOperatorWeightCallCount(), refreshesBefore); // no refresh + assertEq(metaRegistryMock.notifyWeightBoostChangedCallCount(), refreshesBefore); // no refresh } } @@ -442,6 +442,9 @@ contract NodeOperatorStrikesThresholdsTest is NodeOperatorStrikesBaseTest { assertEq(stored[0].reductionBP, 2_500); assertEq(stored[3].minCount, 5); assertEq(stored[3].reductionBP, 10_000); + + // The config change is pushed to MetaRegistry so cached weights get refreshed. + assertEq(metaRegistryMock.notifyConfigChangedCallCount(), 1); } function test_setStrikeThresholds_Replaces() public { @@ -528,38 +531,38 @@ contract NodeOperatorStrikesThresholdsTest is NodeOperatorStrikesBaseTest { } contract NodeOperatorStrikesWeightMultiplierTest is NodeOperatorStrikesBaseTest { - function test_getStrikeWeightMultiplier_StepFunction() public { + function test_getWeightBoostMultiplierBP_StepFunction() public { // 0 strikes -> full weight. - assertEq(strikes.getStrikeWeightMultiplier(NO_ID), MAX_BP); + assertEq(strikes.getWeightBoostMultiplierBP(NO_ID), MAX_BP); _issue(NO_ID); // 1 -> full weight - assertEq(strikes.getStrikeWeightMultiplier(NO_ID), MAX_BP); + assertEq(strikes.getWeightBoostMultiplierBP(NO_ID), MAX_BP); _issue(NO_ID); // 2 -> 75% - assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 7_500); + assertEq(strikes.getWeightBoostMultiplierBP(NO_ID), 7_500); _issue(NO_ID); // 3 -> 50% - assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 5_000); + assertEq(strikes.getWeightBoostMultiplierBP(NO_ID), 5_000); _issue(NO_ID); // 4 -> 25% - assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 2_500); + assertEq(strikes.getWeightBoostMultiplierBP(NO_ID), 2_500); _issue(NO_ID); // 5 -> 0% - assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 0); + assertEq(strikes.getWeightBoostMultiplierBP(NO_ID), 0); _issue(NO_ID); // 6 -> still 0% (clamped to last band) - assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 0); + assertEq(strikes.getWeightBoostMultiplierBP(NO_ID), 0); } - function test_getStrikeWeightMultiplier_IncreasesAfterRemoval() public { + function test_getWeightBoostMultiplierBP_IncreasesAfterRemoval() public { uint256 id1 = _issue(NO_ID); _issue(NO_ID); _issue(NO_ID); // 3 active -> 50% - assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 5_000); + assertEq(strikes.getWeightBoostMultiplierBP(NO_ID), 5_000); vm.prank(committee); strikes.removeStrike(NO_ID, id1); // 2 active -> 75% - assertEq(strikes.getStrikeWeightMultiplier(NO_ID), 7_500); + assertEq(strikes.getWeightBoostMultiplierBP(NO_ID), 7_500); } function test_getStrikes_ExcludesRemoved() public { From 9fad27d38546428a4b1ac32fad2d4f0841916d75 Mon Sep 17 00:00:00 2001 From: vgorkavenko Date: Thu, 16 Jul 2026 17:39:07 +0200 Subject: [PATCH 6/7] fix: review --- src/AdditionalBondRegistry.sol | 1 - src/NodeOperatorStrikes.sol | 53 ++++++++++++++----------- src/interfaces/INodeOperatorStrikes.sol | 3 +- test/unit/NodeOperatorStrikes.t.sol | 4 +- 4 files changed, 33 insertions(+), 28 deletions(-) diff --git a/src/AdditionalBondRegistry.sol b/src/AdditionalBondRegistry.sol index 0ff8dac1e..0d0a8192e 100644 --- a/src/AdditionalBondRegistry.sol +++ b/src/AdditionalBondRegistry.sol @@ -53,7 +53,6 @@ contract AdditionalBondRegistry is IAdditionalBondRegistry, Initializable, Acces function initialize(address admin) external initializer { if (admin == address(0)) revert ZeroAdminAddress(); _grantRole(DEFAULT_ADMIN_ROLE, admin); - // TODO: set initial tiers here } /// @inheritdoc IAdditionalBondRegistry diff --git a/src/NodeOperatorStrikes.sol b/src/NodeOperatorStrikes.sol index d6993bfbe..84aaa3527 100644 --- a/src/NodeOperatorStrikes.sol +++ b/src/NodeOperatorStrikes.sol @@ -67,9 +67,9 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr if (descLength == 0 || descLength > MAX_DESCRIPTION_LENGTH) revert InvalidDescription(); uint256 lifetime = input.lifetime; - if (lifetime == 0) revert InvalidLifetime(); + if (lifetime == 0) revert ZeroLifetime(); uint256 expiry = block.timestamp + lifetime; - if (expiry > type(uint64).max) revert InvalidLifetime(); + if (expiry > type(uint64).max) revert LifetimeTooLong(); OperatorStrikes storage rec = _storage().operatorStrikes[input.nodeOperatorId]; strikeId = ++rec.lastId; @@ -125,11 +125,6 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr META_REGISTRY.notifyWeightBoostProviderConfigChanged(); } - /// @inheritdoc INodeOperatorStrikes - function getActiveStrikesCount(uint256 nodeOperatorId) external view returns (uint256 count) { - return _storage().operatorStrikes[nodeOperatorId].activeIds.length; - } - /// @inheritdoc IWeightBoostProvider /// @dev Counts strikes regardless of expiry: an expired one keeps reducing the weight until removed. function getWeightBoostMultiplierBP(uint256 nodeOperatorId) external view returns (uint256 multiplierBP) { @@ -146,6 +141,11 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr } } + /// @inheritdoc INodeOperatorStrikes + function getActiveStrikesCount(uint256 nodeOperatorId) external view returns (uint256 count) { + return _storage().operatorStrikes[nodeOperatorId].activeIds.length; + } + /// @inheritdoc INodeOperatorStrikes function getStrike(uint256 nodeOperatorId, uint256 strikeId) external view returns (Strike memory strike) { strike = _storage().operatorStrikes[nodeOperatorId].strikes[strikeId]; @@ -170,18 +170,13 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr return _storage().thresholds; } - /// @dev Index of `strikeId` in `activeIds`; reverts `StrikeNotExist` if absent. - function _activeIndex(OperatorStrikes storage rec, uint256 strikeId) private view returns (uint256) { - uint256[] storage activeIds = rec.activeIds; - uint256 len = activeIds.length; - for (uint256 i; i < len; ++i) { - if (activeIds[i] == strikeId) return i; - } - revert StrikeNotExist(); - } - /// @dev Swap-pops the id, deletes the record, emits. Caller refreshes the weight (once per batch). - function _removeStrike(OperatorStrikes storage rec, uint256 nodeOperatorId, uint256 idx, uint256 strikeId) private { + function _removeStrike( + OperatorStrikes storage rec, + uint256 nodeOperatorId, + uint256 idx, + uint256 strikeId + ) internal { uint256[] storage activeIds = rec.activeIds; uint256 lastIdx = activeIds.length - 1; if (idx != lastIdx) { @@ -193,7 +188,7 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr emit StrikeRemoved(nodeOperatorId, strikeId, msg.sender); } - function _setStrikeThresholds(StrikeThreshold[] calldata thresholds) private { + function _setStrikeThresholds(StrikeThreshold[] calldata thresholds) internal { _validateStrikeThresholds(thresholds); NodeOperatorStrikesStorage storage $ = _storage(); @@ -205,17 +200,21 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr emit StrikeThresholdsSet(thresholds); } - function _onlyExistingOperator(uint256 nodeOperatorId) private view { + function _onlyExistingOperator(uint256 nodeOperatorId) internal view { if (nodeOperatorId >= MODULE.getNodeOperatorsCount()) revert NodeOperatorDoesNotExist(); } - function _storage() internal pure returns (NodeOperatorStrikesStorage storage $) { - assembly ("memory-safe") { - $.slot := NODE_OPERATOR_STRIKES_STORAGE_LOCATION + /// @dev Index of `strikeId` in `activeIds`; reverts `StrikeNotExist` if absent. + function _activeIndex(OperatorStrikes storage rec, uint256 strikeId) internal view returns (uint256) { + uint256[] storage activeIds = rec.activeIds; + uint256 len = activeIds.length; + for (uint256 i; i < len; ++i) { + if (activeIds[i] == strikeId) return i; } + revert StrikeNotExist(); } - function _validateStrikeThresholds(StrikeThreshold[] calldata thresholds) private pure { + function _validateStrikeThresholds(StrikeThreshold[] calldata thresholds) internal pure { uint256 len = thresholds.length; if (len == 0 || len > MAX_THRESHOLDS) revert InvalidStrikeThresholds(); if (thresholds[0].minCount == 0) revert InvalidStrikeThresholds(); @@ -227,4 +226,10 @@ contract NodeOperatorStrikes is INodeOperatorStrikes, Initializable, AccessContr if (thresholds[i].reductionBP > MAX_BP) revert InvalidStrikeThresholds(); } } + + function _storage() internal pure returns (NodeOperatorStrikesStorage storage $) { + assembly ("memory-safe") { + $.slot := NODE_OPERATOR_STRIKES_STORAGE_LOCATION + } + } } diff --git a/src/interfaces/INodeOperatorStrikes.sol b/src/interfaces/INodeOperatorStrikes.sol index df25ade7e..9e0b895b7 100644 --- a/src/interfaces/INodeOperatorStrikes.sol +++ b/src/interfaces/INodeOperatorStrikes.sol @@ -47,7 +47,8 @@ interface INodeOperatorStrikes is IWeightBoostProvider { error ZeroAdminAddress(); error NodeOperatorDoesNotExist(); error StrikeNotExist(); - error InvalidLifetime(); + error ZeroLifetime(); + error LifetimeTooLong(); error InvalidStrikeThresholds(); error InvalidDescription(); diff --git a/test/unit/NodeOperatorStrikes.t.sol b/test/unit/NodeOperatorStrikes.t.sol index b5045b3cf..c9fb14966 100644 --- a/test/unit/NodeOperatorStrikes.t.sol +++ b/test/unit/NodeOperatorStrikes.t.sol @@ -190,14 +190,14 @@ contract NodeOperatorStrikesIssueTest is NodeOperatorStrikesBaseTest { } function test_issueStrike_RevertWhen_ZeroLifetime() public { - vm.expectRevert(INodeOperatorStrikes.InvalidLifetime.selector); + vm.expectRevert(INodeOperatorStrikes.ZeroLifetime.selector); vm.prank(committee); strikes.issueStrike(_input(NO_ID, CATEGORY, 0)); } function test_issueStrike_RevertWhen_LifetimeOverflowsUint64() public { uint256 hugeLifetime = type(uint64).max; // block.timestamp + this overflows uint64 - vm.expectRevert(INodeOperatorStrikes.InvalidLifetime.selector); + vm.expectRevert(INodeOperatorStrikes.LifetimeTooLong.selector); vm.prank(committee); strikes.issueStrike(_input(NO_ID, CATEGORY, hugeLifetime)); } From dec020e250d5ff76bf7121efe057f8e71c80f453 Mon Sep 17 00:00:00 2001 From: vgorkavenko Date: Fri, 17 Jul 2026 08:43:22 +0200 Subject: [PATCH 7/7] fix: after merge --- test/unit/NodeOperatorStrikes.t.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/NodeOperatorStrikes.t.sol b/test/unit/NodeOperatorStrikes.t.sol index c9fb14966..5c84c7ce0 100644 --- a/test/unit/NodeOperatorStrikes.t.sol +++ b/test/unit/NodeOperatorStrikes.t.sol @@ -416,13 +416,13 @@ contract NodeOperatorStrikesRemoveExpiredTest is NodeOperatorStrikesBaseTest { function test_removeExpiredStrikes_NoopWhenNoneExpired() public { _issue(NO_ID); _issue(NO_ID); - uint256 refreshesBefore = metaRegistryMock.notifyWeightBoostChangedCallCount(); + uint256 refreshesBefore = metaRegistryMock.notifyWeightBoostProviderConfigChangedCallCount(); vm.prank(stranger); strikes.removeExpiredStrikes(NO_ID); assertEq(strikes.getActiveStrikesCount(NO_ID), 2); - assertEq(metaRegistryMock.notifyWeightBoostChangedCallCount(), refreshesBefore); // no refresh + assertEq(metaRegistryMock.notifyWeightBoostProviderConfigChangedCallCount(), refreshesBefore); // no refresh } } @@ -444,7 +444,7 @@ contract NodeOperatorStrikesThresholdsTest is NodeOperatorStrikesBaseTest { assertEq(stored[3].reductionBP, 10_000); // The config change is pushed to MetaRegistry so cached weights get refreshed. - assertEq(metaRegistryMock.notifyConfigChangedCallCount(), 1); + assertEq(metaRegistryMock.notifyWeightBoostProviderConfigChangedCallCount(), 1); } function test_setStrikeThresholds_Replaces() public {