Skip to content

Commit

Permalink
Use ERC165Checker in CapabilitiesRegistry (#14231)
Browse files Browse the repository at this point in the history
* Use ERC165Checker

* Update snapshot

* Update gethwrappers

---------

Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
  • Loading branch information
1 parent f8e2cb2 commit 7a41ae7
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 39 deletions.
5 changes: 5 additions & 0 deletions contracts/.changeset/unlucky-rocks-marry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@chainlink/contracts': patch
---

#internal use ERC165Checker
52 changes: 26 additions & 26 deletions contracts/gas-snapshots/keystone.gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
CapabilitiesRegistry_AddCapabilitiesTest:test_AddCapability_NoConfigurationContract() (gas: 154809)
CapabilitiesRegistry_AddCapabilitiesTest:test_AddCapability_WithConfiguration() (gas: 178790)
CapabilitiesRegistry_AddCapabilitiesTest:test_AddCapability_WithConfiguration() (gas: 180379)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_CalledByNonAdmin() (gas: 24678)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_CapabilityExists() (gas: 145613)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_ConfigurationContractDoesNotMatchInterface() (gas: 94561)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_ConfigurationContractNotDeployed() (gas: 92916)
CapabilitiesRegistry_AddDONTest:test_AddDON() (gas: 373685)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_CalledByNonAdmin() (gas: 19273)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_CapabilityDoesNotExist() (gas: 169752)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_DeprecatedCapabilityAdded() (gas: 239724)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_DuplicateCapabilityAdded() (gas: 250935)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_DuplicateNodeAdded() (gas: 116890)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_FaultToleranceIsZero() (gas: 43358)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_NodeAlreadyBelongsToWorkflowDON() (gas: 343924)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_NodeDoesNotSupportCapability() (gas: 180150)
CapabilitiesRegistry_AddDONTest_WhenMaliciousCapabilityConfigurationConfigured:test_RevertWhen_MaliciousCapabilitiesConfigContractTriesToRemoveCapabilitiesFromDONNodes() (gas: 340499)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_ConfigurationContractDoesNotMatchInterface() (gas: 94543)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_ConfigurationContractNotDeployed() (gas: 96326)
CapabilitiesRegistry_AddDONTest:test_AddDON() (gas: 373700)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_CalledByNonAdmin() (gas: 19288)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_CapabilityDoesNotExist() (gas: 169767)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_DeprecatedCapabilityAdded() (gas: 239739)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_DuplicateCapabilityAdded() (gas: 250950)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_DuplicateNodeAdded() (gas: 116905)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_FaultToleranceIsZero() (gas: 43373)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_NodeAlreadyBelongsToWorkflowDON() (gas: 343954)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_NodeDoesNotSupportCapability() (gas: 180165)
CapabilitiesRegistry_AddDONTest_WhenMaliciousCapabilityConfigurationConfigured:test_RevertWhen_MaliciousCapabilitiesConfigContractTriesToRemoveCapabilitiesFromDONNodes() (gas: 340514)
CapabilitiesRegistry_AddNodeOperatorsTest:test_AddNodeOperators() (gas: 184157)
CapabilitiesRegistry_AddNodeOperatorsTest:test_RevertWhen_CalledByNonAdmin() (gas: 17624)
CapabilitiesRegistry_AddNodeOperatorsTest:test_RevertWhen_NodeOperatorAdminAddressZero() (gas: 18520)
Expand Down Expand Up @@ -47,23 +47,23 @@ CapabilitiesRegistry_RemoveDONsTest:test_RevertWhen_DONDoesNotExist() (gas: 1655
CapabilitiesRegistry_RemoveNodeOperatorsTest:test_RemovesNodeOperator() (gas: 36122)
CapabilitiesRegistry_RemoveNodeOperatorsTest:test_RevertWhen_CalledByNonOwner() (gas: 15816)
CapabilitiesRegistry_RemoveNodesTest:test_CanAddNodeWithSameSignerAddressAfterRemoving() (gas: 115150)
CapabilitiesRegistry_RemoveNodesTest:test_CanRemoveWhenDONDeleted() (gas: 287648)
CapabilitiesRegistry_RemoveNodesTest:test_CanRemoveWhenNodeNoLongerPartOfDON() (gas: 560993)
CapabilitiesRegistry_RemoveNodesTest:test_CanRemoveWhenDONDeleted() (gas: 287663)
CapabilitiesRegistry_RemoveNodesTest:test_CanRemoveWhenNodeNoLongerPartOfDON() (gas: 561017)
CapabilitiesRegistry_RemoveNodesTest:test_OwnerCanRemoveNodes() (gas: 73358)
CapabilitiesRegistry_RemoveNodesTest:test_RemovesNode() (gas: 75192)
CapabilitiesRegistry_RemoveNodesTest:test_RevertWhen_CalledByNonNodeOperatorAdminAndNonOwner() (gas: 25008)
CapabilitiesRegistry_RemoveNodesTest:test_RevertWhen_NodeDoesNotExist() (gas: 18373)
CapabilitiesRegistry_RemoveNodesTest:test_RevertWhen_NodePartOfCapabilitiesDON() (gas: 385324)
CapabilitiesRegistry_RemoveNodesTest:test_RevertWhen_NodePartOfCapabilitiesDON() (gas: 385339)
CapabilitiesRegistry_RemoveNodesTest:test_RevertWhen_P2PIDEmpty() (gas: 18363)
CapabilitiesRegistry_TypeAndVersionTest:test_TypeAndVersion() (gas: 9796)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_CalledByNonAdmin() (gas: 19314)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_CapabilityDoesNotExist() (gas: 152949)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DONDoesNotExist() (gas: 17740)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DeprecatedCapabilityAdded() (gas: 222966)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DuplicateCapabilityAdded() (gas: 236977)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DuplicateNodeAdded() (gas: 107678)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_NodeDoesNotSupportCapability() (gas: 163392)
CapabilitiesRegistry_UpdateDONTest:test_UpdatesDON() (gas: 373308)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_CalledByNonAdmin() (gas: 19323)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_CapabilityDoesNotExist() (gas: 152958)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DONDoesNotExist() (gas: 17749)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DeprecatedCapabilityAdded() (gas: 222975)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DuplicateCapabilityAdded() (gas: 236986)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DuplicateNodeAdded() (gas: 107687)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_NodeDoesNotSupportCapability() (gas: 163401)
CapabilitiesRegistry_UpdateDONTest:test_UpdatesDON() (gas: 373317)
CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_CalledByNonAdminAndNonOwner() (gas: 20684)
CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_NodeOperatorAdminIsZeroAddress() (gas: 20008)
CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_NodeOperatorDoesNotExist() (gas: 19746)
Expand All @@ -77,8 +77,8 @@ CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_CalledByNonNodeOperatorAdmi
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_NodeDoesNotExist() (gas: 29221)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_NodeSignerAlreadyAssignedToAnotherNode() (gas: 31348)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_P2PIDEmpty() (gas: 29187)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_RemovingCapabilityRequiredByCapabilityDON() (gas: 470932)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_RemovingCapabilityRequiredByWorkflowDON() (gas: 341213)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_RemovingCapabilityRequiredByCapabilityDON() (gas: 470947)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_RemovingCapabilityRequiredByWorkflowDON() (gas: 341228)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_SignerAddressEmpty() (gas: 29080)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_UpdatingNodeWithoutCapabilities() (gas: 27609)
CapabilitiesRegistry_UpdateNodesTest:test_UpdatesNodeParams() (gas: 162264)
Expand Down
5 changes: 2 additions & 3 deletions contracts/src/v0.8/keystone/CapabilitiesRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ pragma solidity 0.8.24;

import {TypeAndVersionInterface} from "../interfaces/TypeAndVersionInterface.sol";
import {OwnerIsCreator} from "../shared/access/OwnerIsCreator.sol";
import {IERC165} from "../vendor/openzeppelin-solidity/v4.8.3/contracts/interfaces/IERC165.sol";
import {EnumerableSet} from "../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/structs/EnumerableSet.sol";
import {ERC165Checker} from "../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/introspection/ERC165Checker.sol";
import {ICapabilityConfiguration} from "./interfaces/ICapabilityConfiguration.sol";

/// @notice CapabilitiesRegistry is used to manage Nodes (including their links to Node
Expand Down Expand Up @@ -1021,8 +1021,7 @@ contract CapabilitiesRegistry is OwnerIsCreator, TypeAndVersionInterface {
/// by implementing both getCapabilityConfiguration and
/// beforeCapabilityConfigSet
if (
capability.configurationContract.code.length == 0 ||
!IERC165(capability.configurationContract).supportsInterface(type(ICapabilityConfiguration).interfaceId)
!ERC165Checker.supportsInterface(capability.configurationContract, type(ICapabilityConfiguration).interfaceId)
) revert InvalidCapabilityConfigurationContractInterface(capability.configurationContract);
}
s_capabilities[hashedCapabilityId] = capability;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
pragma solidity 0.8.24;

import {ICapabilityConfiguration} from "../../interfaces/ICapabilityConfiguration.sol";
import {ERC165} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/introspection/ERC165.sol";
import {IERC165} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/interfaces/IERC165.sol";

contract CapabilityConfigurationContract is ICapabilityConfiguration, ERC165 {
contract CapabilityConfigurationContract is ICapabilityConfiguration, IERC165 {
mapping(uint256 => bytes) private s_donConfiguration;

function getCapabilityConfiguration(uint32 donId) external view returns (bytes memory configuration) {
Expand All @@ -17,7 +17,7 @@ contract CapabilityConfigurationContract is ICapabilityConfiguration, ERC165 {
s_donConfiguration[donId] = config;
}

function supportsInterface(bytes4 interfaceId) public pure override returns (bool) {
return interfaceId == this.getCapabilityConfiguration.selector ^ this.beforeCapabilityConfigSet.selector;
function supportsInterface(bytes4 interfaceId) public pure returns (bool) {
return interfaceId == type(ICapabilityConfiguration).interfaceId || interfaceId == type(IERC165).interfaceId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ pragma solidity 0.8.24;

import {ICapabilityConfiguration} from "../../interfaces/ICapabilityConfiguration.sol";
import {CapabilitiesRegistry} from "../../CapabilitiesRegistry.sol";
import {ERC165} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/introspection/ERC165.sol";
import {IERC165} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/interfaces/IERC165.sol";
import {Constants} from "../Constants.t.sol";

contract MaliciousConfigurationContract is ICapabilityConfiguration, ERC165, Constants {
contract MaliciousConfigurationContract is ICapabilityConfiguration, IERC165, Constants {
bytes32 internal s_capabilityWithConfigurationContractId;

constructor(bytes32 capabilityWithConfigContractId) {
Expand Down Expand Up @@ -41,7 +41,7 @@ contract MaliciousConfigurationContract is ICapabilityConfiguration, ERC165, Con
CapabilitiesRegistry(msg.sender).updateNodes(nodes);
}

function supportsInterface(bytes4 interfaceId) public pure override returns (bool) {
return interfaceId == this.getCapabilityConfiguration.selector ^ this.beforeCapabilityConfigSet.selector;
function supportsInterface(bytes4 interfaceId) public pure returns (bool) {
return interfaceId == type(ICapabilityConfiguration).interfaceId || interfaceId == type(IERC165).interfaceId;
}
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
GETH_VERSION: 1.13.8
capabilities_registry: ../../../contracts/solc/v0.8.24/CapabilitiesRegistry/CapabilitiesRegistry.abi ../../../contracts/solc/v0.8.24/CapabilitiesRegistry/CapabilitiesRegistry.bin 7e95d72f24940f08ada0ee3b85d894d6bfccfd6c8a3e0ceeff65bae52c899d54
capabilities_registry: ../../../contracts/solc/v0.8.24/CapabilitiesRegistry/CapabilitiesRegistry.abi ../../../contracts/solc/v0.8.24/CapabilitiesRegistry/CapabilitiesRegistry.bin 2c8947475e3db9e4feadde2c4325bb093f905e352879518dadac470f33e000ce
feeds_consumer: ../../../contracts/solc/v0.8.24/KeystoneFeedsConsumer/KeystoneFeedsConsumer.abi ../../../contracts/solc/v0.8.24/KeystoneFeedsConsumer/KeystoneFeedsConsumer.bin 8c3a2b18a80be41e7c40d2bc3a4c8d1b5e18d55c1fd20ad5af68cebb66109fc5
forwarder: ../../../contracts/solc/v0.8.24/KeystoneForwarder/KeystoneForwarder.abi ../../../contracts/solc/v0.8.24/KeystoneForwarder/KeystoneForwarder.bin 45d9b866c64b41c1349a90b6764aee42a6d078b454d38f369b5fe02b23b9d16e
ocr3_capability: ../../../contracts/solc/v0.8.24/OCR3Capability/OCR3Capability.abi ../../../contracts/solc/v0.8.24/OCR3Capability/OCR3Capability.bin 8bf0f53f222efce7143dea6134552eb26ea1eef845407b4475a0d79b7d7ba9f8

0 comments on commit 7a41ae7

Please sign in to comment.