Skip to content

Commit

Permalink
Implement addNodes and getNode (#13022)
Browse files Browse the repository at this point in the history
* implement add node

* Fix the test

* Use p2pId for node identification.

* Revert if node exists

* supportedCapabilityIds string[] => bytes32[]

* Revert when adding node without capabilities

* Verify that supported capabilities exist in the registry

* Create a variable for a non-existent capability.

* Undo a change

* Add p2pId to the event

* Remove redundant comment

* Regen wrappers

* Switch to using bytes32 instead of bytes for p2pId

* Add a comment about allowing deprecated capabilities

* Add signer address to Node

* Move checks around

* Regen wrappers

---------

Co-authored-by: DeividasK <deividas.karzinauskas@gmail.com>
  • Loading branch information
cds95 and DeividasK committed May 1, 2024
1 parent ea08b5f commit 2805fa6
Show file tree
Hide file tree
Showing 9 changed files with 411 additions and 9 deletions.
5 changes: 5 additions & 0 deletions .changeset/thirty-students-explain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

#internal
5 changes: 5 additions & 0 deletions contracts/.changeset/tidy-stingrays-care.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@chainlink/contracts": patch
---

#internal
67 changes: 66 additions & 1 deletion contracts/src/v0.8/keystone/CapabilityRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,21 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
string name;
}

struct Node {
/// @notice The id of the node operator that manages this node
uint256 nodeOperatorId;
/// @notice This is an Ed25519 public key that is used to identify a node.
/// This key is guaranteed to be unique in the CapabilityRegistry. It is
/// used to identify a node in the the P2P network.
bytes32 p2pId;
/// @notice The signer address for application-layer message verification.
address signer;
/// @notice The list of capability IDs this node supports. This list is
/// never empty and all capabilities are guaranteed to exist in the
/// CapabilityRegistry.
bytes32[] supportedCapabilityIds;
}

// CapabilityResponseType indicates whether remote response requires
// aggregation or is an already aggregated report. There are multiple
// possible ways to aggregate.
Expand Down Expand Up @@ -70,6 +85,21 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
/// admin address to the zero address
error InvalidNodeOperatorAdmin();

/// @notice This error is thrown when trying to add a node with P2P ID that
/// is empty bytes or a duplicate.
/// @param p2pId The provided P2P ID
error InvalidNodeP2PId(bytes32 p2pId);

/// @notice This error is thrown when trying to add a node without
/// capabilities or with capabilities that do not exist.
/// @param capabilityIds The IDs of the capabilities that are being added.
error InvalidNodeCapabilities(bytes32[] capabilityIds);

/// @notice This event is emitted when a new node is added
/// @param p2pId The P2P ID of the node
/// @param nodeOperatorId The ID of the node operator that manages this node
event NodeAdded(bytes32 p2pId, uint256 nodeOperatorId);

/// @notice This error is thrown when trying add a capability that already
/// exists.
error CapabilityAlreadyExists();
Expand Down Expand Up @@ -120,7 +150,10 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
EnumerableSet.Bytes32Set private s_deprecatedCapabilityIds;

/// @notice Mapping of node operators
mapping(uint256 nodeOperatorId => NodeOperator) private s_nodeOperators;
mapping(uint256 nodeOperatorId => NodeOperator nodeOperator) private s_nodeOperators;

/// @notice Mapping of nodes
mapping(bytes32 p2pId => Node node) private s_nodes;

/// @notice The latest node operator ID
/// @dev No getter for this as this is an implementation detail
Expand Down Expand Up @@ -184,6 +217,38 @@ contract CapabilityRegistry is OwnerIsCreator, TypeAndVersionInterface {
return s_nodeOperators[nodeOperatorId];
}

/// @notice Adds nodes. Nodes can be added with deprecated capabilities to
/// avoid breaking changes when deprecating capabilities.
/// @param nodes The nodes to add
function addNodes(Node[] calldata nodes) external {
for (uint256 i; i < nodes.length; ++i) {
Node memory node = nodes[i];

NodeOperator memory nodeOperator = s_nodeOperators[node.nodeOperatorId];
if (msg.sender != nodeOperator.admin) revert AccessForbidden();

bool nodeExists = s_nodes[node.p2pId].supportedCapabilityIds.length > 0;
if (nodeExists || bytes32(node.p2pId) == bytes32("")) revert InvalidNodeP2PId(node.p2pId);

if (node.supportedCapabilityIds.length == 0) revert InvalidNodeCapabilities(node.supportedCapabilityIds);

for (uint256 j; j < node.supportedCapabilityIds.length; ++j) {
if (!s_capabilityIds.contains(node.supportedCapabilityIds[j]))
revert InvalidNodeCapabilities(node.supportedCapabilityIds);
}

s_nodes[node.p2pId] = node;
emit NodeAdded(node.p2pId, node.nodeOperatorId);
}
}

/// @notice Gets a node's data
/// @param p2pId The P2P ID of the node to query for
/// @return Node The node data
function getNode(bytes32 p2pId) external view returns (Node memory) {
return s_nodes[p2pId];
}

function addCapability(Capability calldata capability) external onlyOwner {
bytes32 capabilityId = getCapabilityID(capability.capabilityType, capability.version);

Expand Down
14 changes: 13 additions & 1 deletion contracts/src/v0.8/keystone/test/BaseTest.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ contract BaseTest is Test, Constants {
CapabilityConfigurationContract internal s_capabilityConfigurationContract;
CapabilityRegistry.Capability internal s_basicCapability;
CapabilityRegistry.Capability internal s_capabilityWithConfigurationContract;
bytes32 internal s_basicCapabilityId;
bytes32 internal s_capabilityWithConfigurationContractId;
bytes32 internal s_nonExistentCapabilityId;

function setUp() public virtual {
vm.startPrank(ADMIN);
Expand All @@ -23,13 +26,22 @@ contract BaseTest is Test, Constants {
responseType: CapabilityRegistry.CapabilityResponseType.REPORT,
configurationContract: address(0)
});

s_capabilityWithConfigurationContract = CapabilityRegistry.Capability({
capabilityType: "read-ethereum-mainnet-gas-price",
version: "1.0.2",
responseType: CapabilityRegistry.CapabilityResponseType.OBSERVATION_IDENTICAL,
configurationContract: address(s_capabilityConfigurationContract)
});

s_basicCapabilityId = s_capabilityRegistry.getCapabilityID(
s_basicCapability.capabilityType,
s_basicCapability.version
);
s_capabilityWithConfigurationContractId = s_capabilityRegistry.getCapabilityID(
s_capabilityWithConfigurationContract.capabilityType,
s_capabilityWithConfigurationContract.version
);
s_nonExistentCapabilityId = s_capabilityRegistry.getCapabilityID("non-existent-capability", "1.0.0");
}

function _getNodeOperators() internal view returns (CapabilityRegistry.NodeOperator[] memory) {
Expand Down
136 changes: 136 additions & 0 deletions contracts/src/v0.8/keystone/test/CapabilityRegistry_AddNodesTest.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

import {BaseTest} from "./BaseTest.t.sol";
import {CapabilityRegistry} from "../CapabilityRegistry.sol";

contract CapabilityRegistry_AddNodesTest is BaseTest {
event NodeAdded(bytes32 p2pId, uint256 nodeOperatorId);

uint256 private constant TEST_NODE_OPERATOR_ONE_ID = 0;
uint256 private constant TEST_NODE_OPERATOR_TWO_ID = 1;

function setUp() public override {
BaseTest.setUp();
changePrank(ADMIN);
s_capabilityRegistry.addNodeOperators(_getNodeOperators());
s_capabilityRegistry.addCapability(s_basicCapability);
s_capabilityRegistry.addCapability(s_capabilityWithConfigurationContract);
}

function test_RevertWhen_CalledByNonNodeOperatorAdmin() public {
changePrank(STRANGER);
CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);

bytes32[] memory capabilityIds = new bytes32[](1);
capabilityIds[0] = s_basicCapabilityId;

nodes[0] = CapabilityRegistry.Node({
nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
p2pId: P2P_ID,
signer: NODE_OPERATOR_ONE_SIGNER_ADDRESS,
supportedCapabilityIds: capabilityIds
});

vm.expectRevert(CapabilityRegistry.AccessForbidden.selector);
s_capabilityRegistry.addNodes(nodes);
}

function test_RevertWhen_AddingDuplicateP2PId() public {
changePrank(NODE_OPERATOR_ONE_ADMIN);
CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);

bytes32[] memory capabilityIds = new bytes32[](1);
capabilityIds[0] = s_basicCapabilityId;

nodes[0] = CapabilityRegistry.Node({
nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
p2pId: P2P_ID,
signer: NODE_OPERATOR_ONE_SIGNER_ADDRESS,
supportedCapabilityIds: capabilityIds
});
s_capabilityRegistry.addNodes(nodes);

vm.expectRevert(abi.encodeWithSelector(CapabilityRegistry.InvalidNodeP2PId.selector, P2P_ID));
s_capabilityRegistry.addNodes(nodes);
}

function test_RevertWhen_P2PIDEmpty() public {
changePrank(NODE_OPERATOR_ONE_ADMIN);
CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);

bytes32[] memory capabilityIds = new bytes32[](1);
capabilityIds[0] = s_basicCapabilityId;

nodes[0] = CapabilityRegistry.Node({
nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
p2pId: bytes32(""),
signer: NODE_OPERATOR_ONE_SIGNER_ADDRESS,
supportedCapabilityIds: capabilityIds
});

vm.expectRevert(abi.encodeWithSelector(CapabilityRegistry.InvalidNodeP2PId.selector, bytes32("")));
s_capabilityRegistry.addNodes(nodes);
}

function test_RevertWhen_AddingNodeWithoutCapabilities() public {
changePrank(NODE_OPERATOR_ONE_ADMIN);
CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);

bytes32[] memory capabilityIds = new bytes32[](0);

nodes[0] = CapabilityRegistry.Node({
nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
p2pId: P2P_ID,
signer: NODE_OPERATOR_ONE_SIGNER_ADDRESS,
supportedCapabilityIds: capabilityIds
});
vm.expectRevert(abi.encodeWithSelector(CapabilityRegistry.InvalidNodeCapabilities.selector, capabilityIds));
s_capabilityRegistry.addNodes(nodes);
}

function test_RevertWhen_AddingNodeWithInvalidCapability() public {
changePrank(NODE_OPERATOR_ONE_ADMIN);
CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);

bytes32[] memory capabilityIds = new bytes32[](1);
capabilityIds[0] = s_nonExistentCapabilityId;

nodes[0] = CapabilityRegistry.Node({
nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
p2pId: P2P_ID,
signer: NODE_OPERATOR_ONE_SIGNER_ADDRESS,
supportedCapabilityIds: capabilityIds
});

vm.expectRevert(abi.encodeWithSelector(CapabilityRegistry.InvalidNodeCapabilities.selector, capabilityIds));
s_capabilityRegistry.addNodes(nodes);
}

function test_AddsNode() public {
changePrank(NODE_OPERATOR_ONE_ADMIN);

CapabilityRegistry.Node[] memory nodes = new CapabilityRegistry.Node[](1);
bytes32[] memory capabilityIds = new bytes32[](2);
capabilityIds[0] = s_basicCapabilityId;
capabilityIds[1] = s_capabilityWithConfigurationContractId;

nodes[0] = CapabilityRegistry.Node({
nodeOperatorId: TEST_NODE_OPERATOR_ONE_ID,
p2pId: P2P_ID,
signer: NODE_OPERATOR_ONE_SIGNER_ADDRESS,
supportedCapabilityIds: capabilityIds
});

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.supportedCapabilityIds.length, 2);
assertEq(node.supportedCapabilityIds[0], s_basicCapabilityId);
assertEq(node.supportedCapabilityIds[1], s_capabilityWithConfigurationContractId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ contract CapabilityRegistry_AddCapabilityTest is BaseTest {
}

function test_RevertWhen_CapabilityDoesNotExist() public {
bytes32 capabilityId = s_capabilityRegistry.getCapabilityID("non-existent-capability", "1.0.0");

vm.expectRevert(abi.encodeWithSelector(CapabilityRegistry.CapabilityDoesNotExist.selector, capabilityId));
s_capabilityRegistry.deprecateCapability(capabilityId);
vm.expectRevert(
abi.encodeWithSelector(CapabilityRegistry.CapabilityDoesNotExist.selector, s_nonExistentCapabilityId)
);
s_capabilityRegistry.deprecateCapability(s_nonExistentCapabilityId);
}

function test_RevertWhen_CapabilityAlreadyDeprecated() public {
Expand Down
4 changes: 4 additions & 0 deletions contracts/src/v0.8/keystone/test/Constants.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ contract Constants {
address internal STRANGER = address(2);
address internal NODE_OPERATOR_ONE_ADMIN = address(3);
string internal NODE_OPERATOR_ONE_NAME = "node-operator-one";
address internal NODE_OPERATOR_ONE_SIGNER_ADDRESS = address(3333);
address internal NODE_OPERATOR_TWO_ADMIN = address(4);
string internal NODE_OPERATOR_TWO_NAME = "node-operator-two";
address internal NODE_OPERATOR_TWO_SIGNER_ADDRESS = address(4444);

bytes32 internal P2P_ID = hex"e42415859707d90ed4dc534ad730f187a17b0c368e1beec2e9b995587c4b0a05";
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
GETH_VERSION: 1.13.8
forwarder: ../../../contracts/solc/v0.8.19/KeystoneForwarder/KeystoneForwarder.abi ../../../contracts/solc/v0.8.19/KeystoneForwarder/KeystoneForwarder.bin b4c900aae9e022f01abbac7993d41f93912247613ac6270b0c4da4ef6f2016e3
keystone_capability_registry: ../../../contracts/solc/v0.8.19/CapabilityRegistry/CapabilityRegistry.abi ../../../contracts/solc/v0.8.19/CapabilityRegistry/CapabilityRegistry.bin 28fb0e1c437b97271d999f3b028f7bd44558352ec83f89501aabf501fcc915f1
keystone_capability_registry: ../../../contracts/solc/v0.8.19/CapabilityRegistry/CapabilityRegistry.abi ../../../contracts/solc/v0.8.19/CapabilityRegistry/CapabilityRegistry.bin b7d748b585d7cf1cf91e268b609613f77a8390d119e03d724b49c25fd2ea75e7
ocr3_capability: ../../../contracts/solc/v0.8.19/OCR3Capability/OCR3Capability.abi ../../../contracts/solc/v0.8.19/OCR3Capability/OCR3Capability.bin 9dcbdf55bd5729ba266148da3f17733eb592c871c2108ccca546618628fd9ad2

0 comments on commit 2805fa6

Please sign in to comment.