Skip to content

Commit

Permalink
[KS-344] Capability Registry: Polish (#13569)
Browse files Browse the repository at this point in the history
* Validate that capabilities required by a Workflow DON are still supported

* Validate that capabilities required by a capabilities DONs are still supported

* --wip-- [skip CI]

* Create and return CapabilityInfo from getters

* Fix reader and syncer tests

* Changesets
  • Loading branch information
DeividasK authored Jun 18, 2024
1 parent a899925 commit f5a70eb
Show file tree
Hide file tree
Showing 13 changed files with 270 additions and 146 deletions.
5 changes: 5 additions & 0 deletions .changeset/metal-glasses-count.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/two-doors-dance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@chainlink/contracts': patch
---

#internal
59 changes: 30 additions & 29 deletions contracts/gas-snapshots/keystone.gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
CapabilitiesRegistry_AddCapabilitiesTest:test_AddCapability_NoConfigurationContract() (gas: 152156)
CapabilitiesRegistry_AddCapabilitiesTest:test_AddCapability_WithConfiguration() (gas: 176137)
CapabilitiesRegistry_AddCapabilitiesTest:test_AddCapability_NoConfigurationContract() (gas: 154898)
CapabilitiesRegistry_AddCapabilitiesTest:test_AddCapability_WithConfiguration() (gas: 178879)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_CalledByNonAdmin() (gas: 24723)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_CapabilityExists() (gas: 145769)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_ConfigurationContractDoesNotMatchInterface() (gas: 94606)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_ConfigurationContractNotDeployed() (gas: 92961)
CapabilitiesRegistry_AddDONTest:test_AddDON() (gas: 372909)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_CalledByNonAdmin() (gas: 19269)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_CapabilityDoesNotExist() (gas: 169874)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_DeprecatedCapabilityAdded() (gas: 243042)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_DeprecatedCapabilityAdded() (gas: 239974)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_DuplicateCapabilityAdded() (gas: 249901)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_DuplicateNodeAdded() (gas: 116949)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_FaultToleranceIsZero() (gas: 43354)
Expand All @@ -16,41 +16,40 @@ CapabilitiesRegistry_AddDONTest:test_RevertWhen_NodeDoesNotSupportCapability() (
CapabilitiesRegistry_AddNodeOperatorsTest:test_AddNodeOperators() (gas: 184333)
CapabilitiesRegistry_AddNodeOperatorsTest:test_RevertWhen_CalledByNonAdmin() (gas: 17602)
CapabilitiesRegistry_AddNodeOperatorsTest:test_RevertWhen_NodeOperatorAdminAddressZero() (gas: 18498)
CapabilitiesRegistry_AddNodesTest:test_AddsNodeParams() (gas: 358667)
CapabilitiesRegistry_AddNodesTest:test_OwnerCanAddNodes() (gas: 358633)
CapabilitiesRegistry_AddNodesTest:test_AddsNodeParams() (gas: 358630)
CapabilitiesRegistry_AddNodesTest:test_OwnerCanAddNodes() (gas: 358596)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_AddingDuplicateP2PId() (gas: 301350)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_AddingNodeWithInvalidCapability() (gas: 55207)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_AddingNodeWithInvalidCapability() (gas: 55174)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_AddingNodeWithInvalidNodeOperator() (gas: 24895)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_AddingNodeWithoutCapabilities() (gas: 27702)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_AddingNodeWithoutCapabilities() (gas: 27669)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_CalledByNonNodeOperatorAdminAndNonOwner() (gas: 25108)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_P2PIDEmpty() (gas: 27408)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_SignerAddressEmpty() (gas: 27047)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_SignerAddressNotUnique() (gas: 309800)
CapabilitiesRegistry_DeprecateCapabilitiesTest:test_DeprecatesCapability() (gas: 92938)
CapabilitiesRegistry_DeprecateCapabilitiesTest:test_EmitsEvent() (gas: 93066)
CapabilitiesRegistry_DeprecateCapabilitiesTest:test_DeprecatesCapability() (gas: 89870)
CapabilitiesRegistry_DeprecateCapabilitiesTest:test_EmitsEvent() (gas: 89998)
CapabilitiesRegistry_DeprecateCapabilitiesTest:test_RevertWhen_CalledByNonAdmin() (gas: 22944)
CapabilitiesRegistry_DeprecateCapabilitiesTest:test_RevertWhen_CapabilityDoesNotExist() (gas: 16231)
CapabilitiesRegistry_DeprecateCapabilitiesTest:test_RevertWhen_CapabilityIsDeprecated() (gas: 94395)
CapabilitiesRegistry_GetCapabilitiesTest:test_ExcludesDeprecatedCapabilities() (gas: 122114)
CapabilitiesRegistry_GetCapabilitiesTest:test_ReturnsCapabilities() (gas: 58311)
CapabilitiesRegistry_DeprecateCapabilitiesTest:test_RevertWhen_CapabilityIsDeprecated() (gas: 91327)
CapabilitiesRegistry_GetCapabilitiesTest:test_ReturnsCapabilities() (gas: 135740)
CapabilitiesRegistry_GetDONsTest:test_CorrectlyFetchesDONs() (gas: 65746)
CapabilitiesRegistry_GetDONsTest:test_DoesNotIncludeRemovedDONs() (gas: 65329)
CapabilitiesRegistry_GetHashedCapabilityTest:test_CorrectlyGeneratesHashedCapabilityId() (gas: 11428)
CapabilitiesRegistry_GetHashedCapabilityTest:test_DoesNotCauseIncorrectClashes() (gas: 13087)
CapabilitiesRegistry_GetNodeOperatorsTest:test_CorrectlyFetchesNodeOperators() (gas: 36653)
CapabilitiesRegistry_GetNodeOperatorsTest:test_DoesNotIncludeRemovedNodeOperators() (gas: 38938)
CapabilitiesRegistry_GetNodesTest:test_CorrectlyFetchesNodes() (gas: 65482)
CapabilitiesRegistry_GetNodesTest:test_DoesNotIncludeRemovedNodes() (gas: 73664)
CapabilitiesRegistry_GetNodesTest:test_CorrectlyFetchesNodes() (gas: 65408)
CapabilitiesRegistry_GetNodesTest:test_DoesNotIncludeRemovedNodes() (gas: 73634)
CapabilitiesRegistry_RemoveDONsTest:test_RemovesDON() (gas: 54946)
CapabilitiesRegistry_RemoveDONsTest:test_RevertWhen_CalledByNonAdmin() (gas: 15647)
CapabilitiesRegistry_RemoveDONsTest:test_RevertWhen_DONDoesNotExist() (gas: 16550)
CapabilitiesRegistry_RemoveNodeOperatorsTest:test_RemovesNodeOperator() (gas: 36122)
CapabilitiesRegistry_RemoveNodeOperatorsTest:test_RevertWhen_CalledByNonOwner() (gas: 15816)
CapabilitiesRegistry_RemoveNodesTest:test_CanAddNodeWithSameSignerAddressAfterRemoving() (gas: 115436)
CapabilitiesRegistry_RemoveNodesTest:test_CanRemoveWhenDONDeleted() (gas: 288312)
CapabilitiesRegistry_RemoveNodesTest:test_CanRemoveWhenNodeNoLongerPartOfDON() (gas: 562061)
CapabilitiesRegistry_RemoveNodesTest:test_OwnerCanRemoveNodes() (gas: 73457)
CapabilitiesRegistry_RemoveNodesTest:test_RemovesNode() (gas: 75292)
CapabilitiesRegistry_RemoveNodesTest:test_CanAddNodeWithSameSignerAddressAfterRemoving() (gas: 115399)
CapabilitiesRegistry_RemoveNodesTest:test_CanRemoveWhenDONDeleted() (gas: 288275)
CapabilitiesRegistry_RemoveNodesTest:test_CanRemoveWhenNodeNoLongerPartOfDON() (gas: 562024)
CapabilitiesRegistry_RemoveNodesTest:test_OwnerCanRemoveNodes() (gas: 73428)
CapabilitiesRegistry_RemoveNodesTest:test_RemovesNode() (gas: 75262)
CapabilitiesRegistry_RemoveNodesTest:test_RevertWhen_CalledByNonNodeOperatorAdminAndNonOwner() (gas: 25053)
CapabilitiesRegistry_RemoveNodesTest:test_RevertWhen_NodeDoesNotExist() (gas: 18418)
CapabilitiesRegistry_RemoveNodesTest:test_RevertWhen_NodePartOfCapabilitiesDON() (gas: 385680)
Expand All @@ -59,7 +58,7 @@ CapabilitiesRegistry_TypeAndVersionTest:test_TypeAndVersion() (gas: 9796)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_CalledByNonAdmin() (gas: 19411)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_CapabilityDoesNotExist() (gas: 153162)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DONDoesNotExist() (gas: 17831)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DeprecatedCapabilityAdded() (gas: 226375)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DeprecatedCapabilityAdded() (gas: 223307)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DuplicateCapabilityAdded() (gas: 233235)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DuplicateNodeAdded() (gas: 107828)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_NodeDoesNotSupportCapability() (gas: 163668)
Expand All @@ -69,16 +68,18 @@ CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_NodeOperatorAdminIsZ
CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_NodeOperatorDoesNotExist() (gas: 19786)
CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_NodeOperatorIdAndParamLengthsMismatch() (gas: 15426)
CapabilitiesRegistry_UpdateNodeOperatorTest:test_UpdatesNodeOperator() (gas: 36991)
CapabilitiesRegistry_UpdateNodesTest:test_CanUpdateParamsIfNodeSignerAddressNoLongerUsed() (gas: 253325)
CapabilitiesRegistry_UpdateNodesTest:test_OwnerCanUpdateNodes() (gas: 161746)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_AddingNodeWithInvalidCapability() (gas: 35794)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_CalledByNonNodeOperatorAdminAndNonOwner() (gas: 25064)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_NodeDoesNotExist() (gas: 27303)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_NodeSignerAlreadyAssignedToAnotherNode() (gas: 29214)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_P2PIDEmpty() (gas: 27291)
CapabilitiesRegistry_UpdateNodesTest:test_CanUpdateParamsIfNodeSignerAddressNoLongerUsed() (gas: 256393)
CapabilitiesRegistry_UpdateNodesTest:test_OwnerCanUpdateNodes() (gas: 162174)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_AddingNodeWithInvalidCapability() (gas: 35766)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_CalledByNonNodeOperatorAdminAndNonOwner() (gas: 25069)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_NodeDoesNotExist() (gas: 27308)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_NodeSignerAlreadyAssignedToAnotherNode() (gas: 29219)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_P2PIDEmpty() (gas: 27296)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_RemovingCapabilityRequiredByCapabilityDON() (gas: 471181)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_RemovingCapabilityRequiredByWorkflowDON() (gas: 341462)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_SignerAddressEmpty() (gas: 26951)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_UpdatingNodeWithoutCapabilities() (gas: 25508)
CapabilitiesRegistry_UpdateNodesTest:test_UpdatesNodeParams() (gas: 161800)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_UpdatingNodeWithoutCapabilities() (gas: 25480)
CapabilitiesRegistry_UpdateNodesTest:test_UpdatesNodeParams() (gas: 162228)
KeystoneForwarder_ReportTest:test_Report_ConfigVersion() (gas: 1802140)
KeystoneForwarder_ReportTest:test_Report_FailedDeliveryWhenReceiverInterfaceNotSupported() (gas: 126020)
KeystoneForwarder_ReportTest:test_Report_FailedDeliveryWhenReceiverNotContract() (gas: 127513)
Expand Down
122 changes: 86 additions & 36 deletions contracts/src/v0.8/keystone/CapabilitiesRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,41 @@ contract CapabilitiesRegistry is OwnerIsCreator, TypeAndVersionInterface {
///
/// Ex. id = "data-streams-reports:chain:[email protected]"
/// labelledName = "data-streams-reports:chain:ethereum"
string labelledName;
/// @notice Semver, e.g., "1.2.3"
/// @dev must be valid Semver + max 32 characters.
string version;
/// @notice CapabilityType indicates the type of capability which determines
/// where the capability can be used in a Workflow Spec.
CapabilityType capabilityType;
/// @notice CapabilityResponseType indicates whether remote response requires
// aggregation or is an already aggregated report. There are multiple
// possible ways to aggregate.
CapabilityResponseType responseType;
/// @notice An address to the capability configuration contract. Having this defined
// on a capability enforces consistent configuration across DON instances
// serving the same capability. Configuration contract MUST implement
// CapabilityConfigurationContractInterface.
//
/// @dev The main use cases are:
// 1) Sharing capability configuration across DON instances
// 2) Inspect and modify on-chain configuration without off-chain
// capability code.
//
// It is not recommended to store configuration which requires knowledge of
// the DON membership.
address configurationContract;
}

struct CapabilityInfo {
/// @notice A hashed ID created by the `getHashedCapabilityId` function.
bytes32 hashedId;
/// @notice The partially qualified ID for the capability.
/// @dev Given the following capability ID: {name}:{label1_key}_{label1_value}:{label2_key}_{label2_value}@{version}
// Then we denote the `labelledName` as the `{name}:{label1_key}_{label1_value}:{label2_key}_{label2_value}` portion of the ID.
///
/// validation regex: ^[a-z0-9_\-:]{1,32}$
/// Ex. id = "data-streams-reports:chain:[email protected]"
/// labelledName = "data-streams-reports:chain:ethereum"
string labelledName;
/// @notice Semver, e.g., "1.2.3"
/// @dev must be valid Semver + max 32 characters.
Expand All @@ -154,6 +187,8 @@ contract CapabilitiesRegistry is OwnerIsCreator, TypeAndVersionInterface {
// It is not recommended to store configuration which requires knowledge of
// the DON membership.
address configurationContract;
/// @notice True if the capability is deprecated
bool isDeprecated;
}

/// @notice CapabilityConfiguration is a struct that holds the capability configuration
Expand Down Expand Up @@ -328,6 +363,13 @@ contract CapabilitiesRegistry is OwnerIsCreator, TypeAndVersionInterface {
/// @param nodeP2PId The P2P Id of the node
error NodePartOfWorkflowDON(uint32 donId, bytes32 nodeP2PId);

/// @notice This error is thrown when removing a capability from the node
/// when that capability is still required by one of the DONs the node
/// belongs to.
/// @param hashedCapabilityId The hashed ID of the capability
/// @param donId The ID of the DON that requires the capability
error CapabilityRequiredByDON(bytes32 hashedCapabilityId, uint32 donId);

/// @notice This error is thrown when trying to add a capability with a
/// configuration contract that does not implement the required interface.
/// @param proposedConfigurationContract The address of the proposed
Expand Down Expand Up @@ -390,8 +432,6 @@ contract CapabilitiesRegistry is OwnerIsCreator, TypeAndVersionInterface {

/// @notice Set of deprecated hashed capability IDs,
/// A hashed ID is created by the function `getHashedCapabilityId`.
///
/// Deprecated capabilities are skipped by the `getCapabilities` function.
EnumerableSet.Bytes32Set private s_deprecatedHashedCapabilityIds;

/// @notice Encoded node signer addresses
Expand Down Expand Up @@ -571,13 +611,11 @@ contract CapabilitiesRegistry is OwnerIsCreator, TypeAndVersionInterface {
bool isOwner = msg.sender == owner();
for (uint256 i; i < nodes.length; ++i) {
NodeParams memory node = nodes[i];

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

Node storage storedNode = s_nodes[node.p2pId];
if (storedNode.signer == bytes32("")) revert NodeDoesNotExist(node.p2pId);

if (node.signer == bytes32("")) revert InvalidNodeSigner();

bytes32 previousSigner = storedNode.signer;
Expand All @@ -598,6 +636,31 @@ contract CapabilitiesRegistry is OwnerIsCreator, TypeAndVersionInterface {
storedNode.supportedHashedCapabilityIds[capabilityConfigCount].add(supportedHashedCapabilityIds[j]);
}

// Validate that capabilities required by a Workflow DON are still supported
uint32 nodeWorkflowDONId = storedNode.workflowDONId;
if (nodeWorkflowDONId != 0) {
bytes32[] memory workflowDonCapabilityIds = s_dons[nodeWorkflowDONId]
.config[s_dons[nodeWorkflowDONId].configCount]
.capabilityIds;

for (uint256 j; j < workflowDonCapabilityIds.length; ++j) {
if (!storedNode.supportedHashedCapabilityIds[capabilityConfigCount].contains(workflowDonCapabilityIds[j]))
revert CapabilityRequiredByDON(workflowDonCapabilityIds[j], nodeWorkflowDONId);
}
}

// Validate that capabilities required by capabilities DONs are still supported
uint256[] memory capabilitiesDONIds = storedNode.capabilitiesDONIds.values();
for (uint32 j; j < capabilitiesDONIds.length; ++j) {
uint32 donId = uint32(capabilitiesDONIds[j]);
bytes32[] memory donCapabilityIds = s_dons[donId].config[s_dons[donId].configCount].capabilityIds;

for (uint256 k; k < donCapabilityIds.length; ++k) {
if (!storedNode.supportedHashedCapabilityIds[capabilityConfigCount].contains(donCapabilityIds[k]))
revert CapabilityRequiredByDON(donCapabilityIds[k], donId);
}
}

storedNode.nodeOperatorId = node.nodeOperatorId;
storedNode.p2pId = node.p2pId;

Expand Down Expand Up @@ -656,51 +719,38 @@ contract CapabilitiesRegistry is OwnerIsCreator, TypeAndVersionInterface {
if (!s_hashedCapabilityIds.contains(hashedCapabilityId)) revert CapabilityDoesNotExist(hashedCapabilityId);
if (!s_deprecatedHashedCapabilityIds.add(hashedCapabilityId)) revert CapabilityIsDeprecated(hashedCapabilityId);

delete s_capabilities[hashedCapabilityId];
emit CapabilityDeprecated(hashedCapabilityId);
}
}

/// @notice Returns a Capability by its hashed ID.
/// @dev Use `getHashedCapabilityId` to get the hashed ID.
function getCapability(bytes32 hashedId) external view returns (Capability memory) {
return s_capabilities[hashedId];
function getCapability(bytes32 hashedId) public view returns (CapabilityInfo memory) {
return (
CapabilityInfo({
hashedId: hashedId,
labelledName: s_capabilities[hashedId].labelledName,
version: s_capabilities[hashedId].version,
capabilityType: s_capabilities[hashedId].capabilityType,
responseType: s_capabilities[hashedId].responseType,
configurationContract: s_capabilities[hashedId].configurationContract,
isDeprecated: s_deprecatedHashedCapabilityIds.contains(hashedId)
})
);
}

/// @notice Returns all capabilities. This operation will copy capabilities
/// to memory, which can be quite expensive. This is designed to mostly be
/// used by view accessors that are queried without any gas fees.
/// @return hashedCapabilityIds bytes32[] List of hashed capability Ids
/// @return capabilities Capability[] List of capabilities
function getCapabilities()
external
view
returns (bytes32[] memory hashedCapabilityIds, Capability[] memory capabilities)
{
hashedCapabilityIds = s_hashedCapabilityIds.values();

uint256 numSupportedCapabilities = hashedCapabilityIds.length - s_deprecatedHashedCapabilityIds.length();

// Solidity does not support dynamic arrays in memory, so we create a
// fixed-size array and copy the capabilities into it.
capabilities = new Capability[](numSupportedCapabilities);
bytes32[] memory supportedHashedCapabilityIds = new bytes32[](numSupportedCapabilities);

// We need to keep track of the new index because we are skipping
// deprecated capabilities.
uint256 newIndex;
/// @return CapabilityInfo[] List of capabilities
function getCapabilities() external view returns (CapabilityInfo[] memory) {
bytes32[] memory hashedCapabilityIds = s_hashedCapabilityIds.values();
CapabilityInfo[] memory capabilitiesInfo = new CapabilityInfo[](hashedCapabilityIds.length);

for (uint256 i; i < hashedCapabilityIds.length; ++i) {
bytes32 hashedCapabilityId = hashedCapabilityIds[i];

if (!s_deprecatedHashedCapabilityIds.contains(hashedCapabilityId)) {
capabilities[newIndex] = s_capabilities[hashedCapabilityId];
supportedHashedCapabilityIds[newIndex] = hashedCapabilityId;
++newIndex;
}
capabilitiesInfo[i] = getCapability(hashedCapabilityIds[i]);
}

return (supportedHashedCapabilityIds, capabilities);
return capabilitiesInfo;
}

/// @notice This functions returns a capability id that has been hashed to fit into a bytes32 for cheaper access
Expand Down
Loading

0 comments on commit f5a70eb

Please sign in to comment.