From ea8c3cbd11dc419d7df25f65c32d6bf3a0cc393d Mon Sep 17 00:00:00 2001 From: Pat Fives Date: Thu, 13 Jun 2024 18:45:03 -0400 Subject: [PATCH] Only allow approved operators to register nodes, expose nodes by operator (#158) https://testnets.llama.xyz/orgs/river/base-sepolia/actions/116 --------- Co-authored-by: John Terzis --- .../facets/DeployEntitlementChecker.s.sol | 1 + .../facets/checker/EntitlementChecker.sol | 44 +++++++++++++++++-- .../facets/checker/IEntitlementChecker.sol | 5 +++ .../test/crosschain/EntitlementChecker.t.sol | 38 +++++++++++++++- contracts/test/spaces/BaseSetup.sol | 3 ++ core/xchain/create_multi.sh | 9 ++++ 6 files changed, 96 insertions(+), 4 deletions(-) diff --git a/contracts/scripts/deployments/facets/DeployEntitlementChecker.s.sol b/contracts/scripts/deployments/facets/DeployEntitlementChecker.s.sol index 71b42a026..1564f639e 100644 --- a/contracts/scripts/deployments/facets/DeployEntitlementChecker.s.sol +++ b/contracts/scripts/deployments/facets/DeployEntitlementChecker.s.sol @@ -14,6 +14,7 @@ contract DeployEntitlementChecker is Deployer, FacetHelper { addSelector(EntitlementChecker.getNodeAtIndex.selector); addSelector(EntitlementChecker.getRandomNodes.selector); addSelector(EntitlementChecker.requestEntitlementCheck.selector); + addSelector(EntitlementChecker.getNodesByOperator.selector); } function initializer() public pure override returns (bytes4) { diff --git a/contracts/src/base/registry/facets/checker/EntitlementChecker.sol b/contracts/src/base/registry/facets/checker/EntitlementChecker.sol index 39115b6a3..2627c9d4e 100644 --- a/contracts/src/base/registry/facets/checker/EntitlementChecker.sol +++ b/contracts/src/base/registry/facets/checker/EntitlementChecker.sol @@ -7,7 +7,7 @@ import {IEntitlementChecker} from "./IEntitlementChecker.sol"; // libraries import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {EntitlementCheckerStorage} from "./EntitlementCheckerStorage.sol"; -import {NodeOperatorStorage} from "contracts/src/base/registry/facets/operator/NodeOperatorStorage.sol"; +import {NodeOperatorStorage, NodeOperatorStatus} from "contracts/src/base/registry/facets/operator/NodeOperatorStorage.sol"; // contracts import {Facet} from "contracts/src/diamond/facets/Facet.sol"; @@ -35,13 +35,20 @@ contract EntitlementChecker is IEntitlementChecker, Facet { _; } - modifier onlyRegisteredOperator() { + modifier onlyRegisteredApprovedOperator() { NodeOperatorStorage.Layout storage nodeOperatorLayout = NodeOperatorStorage .layout(); if (!nodeOperatorLayout.operators.contains(msg.sender)) revert EntitlementChecker_InvalidOperator(); _; + + if ( + nodeOperatorLayout.statusByOperator[msg.sender] != + NodeOperatorStatus.Approved + ) { + revert EntitlementChecker_OperatorNotActive(); + } } // ============================================================= @@ -53,7 +60,7 @@ contract EntitlementChecker is IEntitlementChecker, Facet { * @param node The address of the node to register * @dev Only valid operators can register a node */ - function registerNode(address node) external onlyRegisteredOperator { + function registerNode(address node) external onlyRegisteredApprovedOperator { EntitlementCheckerStorage.Layout storage layout = EntitlementCheckerStorage .layout(); @@ -153,6 +160,37 @@ contract EntitlementChecker is IEntitlementChecker, Facet { ); } + /** + * @notice Get the nodes registered by an operator + * @param operator The address of the operator + * @return An array of node addresses + */ + function getNodesByOperator( + address operator + ) external view returns (address[] memory) { + EntitlementCheckerStorage.Layout storage layout = EntitlementCheckerStorage + .layout(); + uint256 totalNodeCount = layout.nodes.length(); + uint256 nodeCount = 0; + for (uint256 i = 0; i < totalNodeCount; i++) { + address node = layout.nodes.at(i); + if (layout.operatorByNode[node] == operator) { + nodeCount++; + } + } + address[] memory nodes = new address[](nodeCount); + uint256 j = 0; + for (uint256 i = 0; i < totalNodeCount; i++) { + address node = layout.nodes.at(i); + if (layout.operatorByNode[node] == operator) { + nodes[j] = node; + j++; + } + } + + return nodes; + } + // ============================================================= // Internal // ============================================================= diff --git a/contracts/src/base/registry/facets/checker/IEntitlementChecker.sol b/contracts/src/base/registry/facets/checker/IEntitlementChecker.sol index 1373246dd..e4a676c12 100644 --- a/contracts/src/base/registry/facets/checker/IEntitlementChecker.sol +++ b/contracts/src/base/registry/facets/checker/IEntitlementChecker.sol @@ -7,6 +7,7 @@ interface IEntitlementCheckerBase { error EntitlementChecker_InsufficientNumberOfNodes(); error EntitlementChecker_InvalidNodeOperator(); error EntitlementChecker_InvalidOperator(); + error EntitlementChecker_OperatorNotActive(); // Events event NodeRegistered(address indexed nodeAddress); @@ -42,4 +43,8 @@ interface IEntitlementChecker is IEntitlementCheckerBase { uint256 roleId, address[] memory nodes ) external; + + function getNodesByOperator( + address operator + ) external view returns (address[] memory); } diff --git a/contracts/test/crosschain/EntitlementChecker.t.sol b/contracts/test/crosschain/EntitlementChecker.t.sol index 4c0712b92..0e6b8bde8 100644 --- a/contracts/test/crosschain/EntitlementChecker.t.sol +++ b/contracts/test/crosschain/EntitlementChecker.t.sol @@ -6,6 +6,7 @@ import {BaseSetup} from "contracts/test/spaces/BaseSetup.sol"; //interfaces import {IEntitlementCheckerBase} from "contracts/src/base/registry/facets/checker/IEntitlementChecker.sol"; +import {NodeOperatorStatus} from "contracts/src/base/registry/facets/operator/NodeOperatorStorage.sol"; //libraries @@ -31,6 +32,12 @@ contract EntitlementCheckerTest is BaseSetup, IEntitlementCheckerBase { _; } + modifier givenOperatorIsApproved(address operator) { + vm.prank(deployer); + nodeOperator.setOperatorStatus(operator, NodeOperatorStatus.Approved); + _; + } + modifier givenNodeIsRegistered(address operator, address node) { vm.assume(node != address(0)); vm.assume(!entitlementChecker.isValidNode(node)); @@ -60,6 +67,7 @@ contract EntitlementCheckerTest is BaseSetup, IEntitlementCheckerBase { ) external givenOperatorIsRegistered(operator) + givenOperatorIsApproved(operator) givenNodeIsRegistered(operator, node) { assertTrue(entitlementChecker.isValidNode(node)); @@ -71,6 +79,7 @@ contract EntitlementCheckerTest is BaseSetup, IEntitlementCheckerBase { ) external givenOperatorIsRegistered(operator) + givenOperatorIsApproved(operator) givenNodeIsRegistered(operator, node) { vm.prank(operator); @@ -87,6 +96,7 @@ contract EntitlementCheckerTest is BaseSetup, IEntitlementCheckerBase { ) external givenOperatorIsRegistered(operator) + givenOperatorIsApproved(operator) givenNodeIsRegistered(operator, node) givenNodeIsNotRegistered(operator, node) { @@ -96,7 +106,11 @@ contract EntitlementCheckerTest is BaseSetup, IEntitlementCheckerBase { function test_unregisterNode_revert_nodeNotRegistered( address operator, address node - ) external givenOperatorIsRegistered(operator) { + ) + external + givenOperatorIsRegistered(operator) + givenOperatorIsApproved(operator) + { vm.prank(operator); vm.expectRevert(EntitlementChecker_InvalidNodeOperator.selector); entitlementChecker.unregisterNode(node); @@ -121,4 +135,26 @@ contract EntitlementCheckerTest is BaseSetup, IEntitlementCheckerBase { vm.expectRevert(EntitlementChecker_InsufficientNumberOfNodes.selector); entitlementChecker.getRandomNodes(26); } + + // ============================================================= + // Nodes by Operator + // ============================================================= + + function test_getNodesByOperator() external { + for (uint256 i = 0; i < operators.length; i++) { + uint256 totalNodes = 0; + + address[] memory nodes = entitlementChecker.getNodesByOperator( + operators[i] + ); + uint256 nodeLen = nodes.length; + + for (uint256 j = 0; j < nodeLen; j++) { + vm.prank(operators[i]); + assertTrue(entitlementChecker.isValidNode(nodes[j])); + totalNodes++; + } + assertEq(totalNodes, nodes.length); + } + } } diff --git a/contracts/test/spaces/BaseSetup.sol b/contracts/test/spaces/BaseSetup.sol index 171b1d228..0bc95de90 100644 --- a/contracts/test/spaces/BaseSetup.sol +++ b/contracts/test/spaces/BaseSetup.sol @@ -10,6 +10,7 @@ import {IWalletLink} from "contracts/src/factory/facets/wallet-link/IWalletLink. import {ISpaceOwner} from "contracts/src/spaces/facets/owner/ISpaceOwner.sol"; import {IMainnetDelegation} from "contracts/src/tokens/river/base/delegation/IMainnetDelegation.sol"; import {INodeOperator} from "contracts/src/base/registry/facets/operator/INodeOperator.sol"; +import {NodeOperatorStatus} from "contracts/src/base/registry/facets/operator/NodeOperatorStorage.sol"; // libraries @@ -149,6 +150,8 @@ contract BaseSetup is TestUtils, SpaceHelper { for (uint256 i = 0; i < operators.length; i++) { vm.prank(operators[i]); nodeOperator.registerOperator(operators[i]); + vm.prank(deployer); + nodeOperator.setOperatorStatus(operators[i], NodeOperatorStatus.Approved); } } diff --git a/core/xchain/create_multi.sh b/core/xchain/create_multi.sh index 11da5069d..611b5e3e5 100755 --- a/core/xchain/create_multi.sh +++ b/core/xchain/create_multi.sh @@ -65,6 +65,15 @@ cast send \ $OPERATOR_ADDRESS \ 2 > /dev/null +# set operator to approved +cast send \ + --rpc-url http://127.0.0.1:8545 \ + --private-key $TESTNET_PRIVATE_KEY \ + $BASE_REGISTRY_ADDRESS \ + "setOperatorStatus(address,uint8)" \ + $OPERATOR_ADDRESS \ + 2 \ + 2 > /dev/null # Number of instances N=5