KSI-165: Implement update nodes (#13080)

* implement update nodes * address PR feedback * regen go wrappers
smartcontractkit · May 6, 2024 · 36cc95f · 36cc95f
1 parent fe2e8fd
commit 36cc95f
Show file tree

Hide file tree

Showing 7 changed files with 444 additions and 5 deletions.
diff --git a/.changeset/modern-trainers-hear.md b/.changeset/modern-trainers-hear.md
@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+#internal Generate gethwrappers for capability registry changes
diff --git a/contracts/.changeset/three-hotels-agree.md b/contracts/.changeset/three-hotels-agree.md
@@ -0,0 +1,5 @@
+---
+"@chainlink/contracts": patch
+---
+
+Add function to update nodes in capability registry
diff --git a/contracts/src/v0.8/keystone/CapabilityRegistry.sol b/contracts/src/v0.8/keystone/CapabilityRegistry.sol
@@ -106,6 +106,16 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
   /// @param nodeOperatorId The ID of the node operator that manages this node
   event NodeAdded(bytes32 p2pId, uint256 nodeOperatorId);
 
+  /// @notice This event is emitted when a node is updated
+  /// @param p2pId The P2P ID of the node
+  /// @param nodeOperatorId The ID of the node operator that manages this node
+  /// @param signer The node's signer address
+  event NodeUpdated(bytes32 p2pId, uint256 nodeOperatorId, address signer);
+
+  /// @notice This error is thrown when trying to set the node's
+  /// signer address to zero
+  error InvalidNodeSigner();
+
   /// @notice This error is thrown when trying add a capability that already
   /// exists.
   error CapabilityAlreadyExists();
@@ -236,12 +246,16 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
     for (uint256 i; i < nodes.length; ++i) {
       Node memory node = nodes[i];
 
+      bool isOwner = msg.sender == owner();
+
       NodeOperator memory nodeOperator = s_nodeOperators[node.nodeOperatorId];
-      if (msg.sender != nodeOperator.admin) revert AccessForbidden();
+      if (!isOwner && msg.sender != nodeOperator.admin) revert AccessForbidden();
 
       bool nodeExists = s_nodes[node.p2pId].supportedHashedCapabilityIds.length > 0;
       if (nodeExists || bytes32(node.p2pId) == bytes32("")) revert InvalidNodeP2PId(node.p2pId);
 
+      if (node.signer == address(0)) revert InvalidNodeSigner();
+
       if (node.supportedHashedCapabilityIds.length == 0)
         revert InvalidNodeCapabilities(node.supportedHashedCapabilityIds);
 
@@ -255,6 +269,36 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
     }
   }
 
+  /// @notice Updates nodes.  The node admin can update the node's signer address
+  /// and reconfigure its supported capabilities
+  /// @param nodes The nodes to update
+  function updateNodes(Node[] calldata nodes) external {
+    for (uint256 i; i < nodes.length; ++i) {
+      Node memory node = nodes[i];
+
+      bool isOwner = msg.sender == owner();
+
+      NodeOperator memory nodeOperator = s_nodeOperators[node.nodeOperatorId];
+      if (!isOwner && msg.sender != nodeOperator.admin) revert AccessForbidden();
+
+      bool nodeExists = s_nodes[node.p2pId].supportedHashedCapabilityIds.length > 0;
+      if (!nodeExists) revert InvalidNodeP2PId(node.p2pId);
+
+      if (node.signer == address(0)) revert InvalidNodeSigner();
+
+      if (node.supportedHashedCapabilityIds.length == 0)
+        revert InvalidNodeCapabilities(node.supportedHashedCapabilityIds);
+
+      for (uint256 j; j < node.supportedHashedCapabilityIds.length; ++j) {
+        if (!s_hashedCapabilityIds.contains(node.supportedHashedCapabilityIds[j]))
+          revert InvalidNodeCapabilities(node.supportedHashedCapabilityIds);
+      }
+
+      s_nodes[node.p2pId] = node;
+      emit NodeUpdated(node.p2pId, node.nodeOperatorId, node.signer);
+    }
+  }
+
   /// @notice Gets a node's data
   /// @param p2pId The P2P ID of the node to query for
   /// @return Node The node data

diff --git a/contracts/src/v0.8/keystone/test/CapabilityRegistry_AddNodesTest.t.sol b/contracts/src/v0.8/keystone/test/CapabilityRegistry_AddNodesTest.t.sol
@@ -18,7 +18,7 @@ contract CapabilityRegistry_AddNodesTest is BaseTest {
     s_capabilityRegistry.addCapability(s_capabilityWithConfigurationContract);
   }
 
-  function test_RevertWhen_CalledByNonNodeOperatorAdmin() public {
+  function test_RevertWhen_CalledByNonNodeOperatorAdminAndNonOwner() public {
     changePrank(STRANGER);
     CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
 
@@ -36,6 +36,24 @@ contract CapabilityRegistry_AddNodesTest is BaseTest {
     s_capabilityRegistry.addNodes(nodes);
   }
 
+  function test_RevertWhen_SignerAddressEmpty() public {
+    changePrank(NODE_OPERATOR_ONE_ADMIN);
+    CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
+
+    bytes32[] memory hashedCapabilityIds = new bytes32[](1);
+    hashedCapabilityIds[0] = s_basicHashedCapabilityId;
+
+    nodes[0] = CapabilityRegistry.Node({
+      nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
+      p2pId: P2P_ID,
+      signer: address(0),
+      supportedHashedCapabilityIds: hashedCapabilityIds
+    });
+
+    vm.expectRevert(abi.encodeWithSelector(CapabilityRegistry.InvalidNodeSigner.selector));
+    s_capabilityRegistry.addNodes(nodes);
+  }
+
   function test_RevertWhen_AddingDuplicateP2PId() public {
     changePrank(NODE_OPERATOR_ONE_ADMIN);
     CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
@@ -133,4 +151,31 @@ contract CapabilityRegistry_AddNodesTest is BaseTest {
     assertEq(node.supportedHashedCapabilityIds[0], s_basicHashedCapabilityId);
     assertEq(node.supportedHashedCapabilityIds[1], s_capabilityWithConfigurationContractId);
   }
+
+  function test_OwnerCanAddNodes() public {
+    changePrank(ADMIN);
+
+    CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
+    bytes32[] memory hashedCapabilityIds = new bytes32[](2);
+    hashedCapabilityIds[0] = s_basicHashedCapabilityId;
+    hashedCapabilityIds[1] = s_capabilityWithConfigurationContractId;
+
+    nodes[0] = CapabilityRegistry.Node({
+      nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
+      p2pId: P2P_ID,
+      signer: NODE_OPERATOR_ONE_SIGNER_ADDRESS,
+      supportedHashedCapabilityIds: hashedCapabilityIds
+    });
+
+    vm.expectEmit(address(s_capabilityRegistry));
+    emit NodeAdded(P2P_ID, TEST_NODE_OPERATOR_ONE_ID);
+    s_capabilityRegistry.addNodes(nodes);
+
+    CapabilityRegistry.Node memory node = s_capabilityRegistry.getNode(P2P_ID);
+    assertEq(node.nodeOperatorId, TEST_NODE_OPERATOR_ONE_ID);
+    assertEq(node.p2pId, P2P_ID);
+    assertEq(node.supportedHashedCapabilityIds.length, 2);
+    assertEq(node.supportedHashedCapabilityIds[0], s_basicHashedCapabilityId);
+    assertEq(node.supportedHashedCapabilityIds[1], s_capabilityWithConfigurationContractId);
+  }
 }
diff --git a/contracts/src/v0.8/keystone/test/CapabilityRegistry_UpdateNodesTest.t.sol b/contracts/src/v0.8/keystone/test/CapabilityRegistry_UpdateNodesTest.t.sol
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.19;
+
+import {BaseTest} from "./BaseTest.t.sol";
+import {CapabilityRegistry} from "../CapabilityRegistry.sol";
+
+contract CapabilityRegistry_UpdateNodesTest is BaseTest {
+  event NodeUpdated(bytes32 p2pId, uint256 nodeOperatorId, address signer);
+
+  uint256 private constant TEST_NODE_OPERATOR_ONE_ID = 0;
+  uint256 private constant TEST_NODE_OPERATOR_TWO_ID = 1;
+  bytes32 private constant INVALID_P2P_ID = bytes32("fake-p2p");
+
+  function setUp() public override {
+    BaseTest.setUp();
+    changePrank(ADMIN);
+    s_capabilityRegistry.addNodeOperators(_getNodeOperators());
+    s_capabilityRegistry.addCapability(s_basicCapability);
+    s_capabilityRegistry.addCapability(s_capabilityWithConfigurationContract);
+
+    CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
+    bytes32[] memory hashedCapabilityIds = new bytes32[](2);
+    hashedCapabilityIds[0] = s_basicHashedCapabilityId;
+    hashedCapabilityIds[1] = s_capabilityWithConfigurationContractId;
+
+    nodes[0] = CapabilityRegistry.Node({
+      nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
+      p2pId: P2P_ID,
+      signer: NODE_OPERATOR_ONE_SIGNER_ADDRESS,
+      supportedHashedCapabilityIds: hashedCapabilityIds
+    });
+
+    changePrank(NODE_OPERATOR_ONE_ADMIN);
+
+    s_capabilityRegistry.addNodes(nodes);
+  }
+
+  function test_RevertWhen_CalledByNonNodeOperatorAdminAndNonOwner() public {
+    changePrank(STRANGER);
+    CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
+
+    bytes32[] memory hashedCapabilityIds = new bytes32[](1);
+    hashedCapabilityIds[0] = s_basicHashedCapabilityId;
+
+    nodes[0] = CapabilityRegistry.Node({
+      nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
+      p2pId: P2P_ID,
+      signer: NODE_OPERATOR_TWO_SIGNER_ADDRESS,
+      supportedHashedCapabilityIds: hashedCapabilityIds
+    });
+
+    vm.expectRevert(CapabilityRegistry.AccessForbidden.selector);
+    s_capabilityRegistry.updateNodes(nodes);
+  }
+
+  function test_RevertWhen_NodeDoesNotExist() public {
+    changePrank(NODE_OPERATOR_ONE_ADMIN);
+    CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
+
+    bytes32[] memory hashedCapabilityIds = new bytes32[](1);
+    hashedCapabilityIds[0] = s_basicHashedCapabilityId;
+
+    nodes[0] = CapabilityRegistry.Node({
+      nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
+      p2pId: INVALID_P2P_ID,
+      signer: NODE_OPERATOR_ONE_SIGNER_ADDRESS,
+      supportedHashedCapabilityIds: hashedCapabilityIds
+    });
+
+    vm.expectRevert(abi.encodeWithSelector(CapabilityRegistry.InvalidNodeP2PId.selector, INVALID_P2P_ID));
+    s_capabilityRegistry.updateNodes(nodes);
+  }
+
+  function test_RevertWhen_P2PIDEmpty() public {
+    changePrank(NODE_OPERATOR_ONE_ADMIN);
+    CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
+
+    bytes32[] memory hashedCapabilityIds = new bytes32[](1);
+    hashedCapabilityIds[0] = s_basicHashedCapabilityId;
+
+    nodes[0] = CapabilityRegistry.Node({
+      nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
+      p2pId: bytes32(""),
+      signer: NODE_OPERATOR_ONE_SIGNER_ADDRESS,
+      supportedHashedCapabilityIds: hashedCapabilityIds
+    });
+
+    vm.expectRevert(abi.encodeWithSelector(CapabilityRegistry.InvalidNodeP2PId.selector, bytes32("")));
+    s_capabilityRegistry.updateNodes(nodes);
+  }
+
+  function test_RevertWhen_SignerAddressEmpty() public {
+    changePrank(NODE_OPERATOR_ONE_ADMIN);
+    CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
+
+    bytes32[] memory hashedCapabilityIds = new bytes32[](1);
+    hashedCapabilityIds[0] = s_basicHashedCapabilityId;
+
+    nodes[0] = CapabilityRegistry.Node({
+      nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
+      p2pId: P2P_ID,
+      signer: address(0),
+      supportedHashedCapabilityIds: hashedCapabilityIds
+    });
+
+    vm.expectRevert(abi.encodeWithSelector(CapabilityRegistry.InvalidNodeSigner.selector));
+    s_capabilityRegistry.updateNodes(nodes);
+  }
+
+  function test_RevertWhen_UpdatingNodeWithoutCapabilities() public {
+    changePrank(NODE_OPERATOR_ONE_ADMIN);
+    CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
+
+    bytes32[] memory hashedCapabilityIds = new bytes32[](0);
+
+    nodes[0] = CapabilityRegistry.Node({
+      nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
+      p2pId: P2P_ID,
+      signer: NODE_OPERATOR_ONE_SIGNER_ADDRESS,
+      supportedHashedCapabilityIds: hashedCapabilityIds
+    });
+    vm.expectRevert(abi.encodeWithSelector(CapabilityRegistry.InvalidNodeCapabilities.selector, hashedCapabilityIds));
+    s_capabilityRegistry.updateNodes(nodes);
+  }
+
+  function test_RevertWhen_AddingNodeWithInvalidCapability() public {
+    changePrank(NODE_OPERATOR_ONE_ADMIN);
+    CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
+
+    bytes32[] memory hashedCapabilityIds = new bytes32[](1);
+    hashedCapabilityIds[0] = s_nonExistentHashedCapabilityId;
+
+    nodes[0] = CapabilityRegistry.Node({
+      nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
+      p2pId: P2P_ID,
+      signer: NODE_OPERATOR_ONE_SIGNER_ADDRESS,
+      supportedHashedCapabilityIds: hashedCapabilityIds
+    });
+
+    vm.expectRevert(abi.encodeWithSelector(CapabilityRegistry.InvalidNodeCapabilities.selector, hashedCapabilityIds));
+    s_capabilityRegistry.updateNodes(nodes);
+  }
+
+  function test_UpdatesNode() public {
+    changePrank(NODE_OPERATOR_ONE_ADMIN);
+
+    CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
+    bytes32[] memory hashedCapabilityIds = new bytes32[](1);
+    hashedCapabilityIds[0] = s_basicHashedCapabilityId;
+
+    nodes[0] = CapabilityRegistry.Node({
+      nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
+      p2pId: P2P_ID,
+      signer: NODE_OPERATOR_TWO_SIGNER_ADDRESS,
+      supportedHashedCapabilityIds: hashedCapabilityIds
+    });
+
+    vm.expectEmit(address(s_capabilityRegistry));
+    emit NodeUpdated(P2P_ID, TEST_NODE_OPERATOR_ONE_ID, NODE_OPERATOR_TWO_SIGNER_ADDRESS);
+    s_capabilityRegistry.updateNodes(nodes);
+
+    CapabilityRegistry.Node memory node = s_capabilityRegistry.getNode(P2P_ID);
+    assertEq(node.nodeOperatorId, TEST_NODE_OPERATOR_ONE_ID);
+    assertEq(node.p2pId, P2P_ID);
+    assertEq(node.signer, NODE_OPERATOR_TWO_SIGNER_ADDRESS);
+    assertEq(node.supportedHashedCapabilityIds.length, 1);
+    assertEq(node.supportedHashedCapabilityIds[0], s_basicHashedCapabilityId);
+  }
+
+  function test_OwnerCanUpdateNodes() public {
+    changePrank(ADMIN);
+
+    CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
+    bytes32[] memory hashedCapabilityIds = new bytes32[](1);
+    hashedCapabilityIds[0] = s_basicHashedCapabilityId;
+
+    nodes[0] = CapabilityRegistry.Node({
+      nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
+      p2pId: P2P_ID,
+      signer: NODE_OPERATOR_TWO_SIGNER_ADDRESS,
+      supportedHashedCapabilityIds: hashedCapabilityIds
+    });
+
+    vm.expectEmit(address(s_capabilityRegistry));
+    emit NodeUpdated(P2P_ID, TEST_NODE_OPERATOR_ONE_ID, NODE_OPERATOR_TWO_SIGNER_ADDRESS);
+    s_capabilityRegistry.updateNodes(nodes);
+
+    CapabilityRegistry.Node memory node = s_capabilityRegistry.getNode(P2P_ID);
+    assertEq(node.nodeOperatorId, TEST_NODE_OPERATOR_ONE_ID);
+    assertEq(node.p2pId, P2P_ID);
+    assertEq(node.signer, NODE_OPERATOR_TWO_SIGNER_ADDRESS);
+    assertEq(node.supportedHashedCapabilityIds.length, 1);
+    assertEq(node.supportedHashedCapabilityIds[0], s_basicHashedCapabilityId);
+  }
+}