Disallow zero address signers + pin Solidity version (#13993)

* Disallow zero address signer * pragma ^0.8.19 => 0.8.24 * Changesets * Update gethwrappers --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
smartcontractkit · Aug 2, 2024 · f5e0bd6 · f5e0bd6
1 parent c1bd103
commit f5e0bd6
Show file tree

Hide file tree

Showing 26 changed files with 63 additions and 42 deletions.
diff --git a/.changeset/slimy-forks-wait.md b/.changeset/slimy-forks-wait.md
@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+#internal
diff --git a/contracts/.changeset/silver-pots-cover.md b/contracts/.changeset/silver-pots-cover.md
@@ -0,0 +1,5 @@
+---
+'@chainlink/contracts': patch
+---
+
+#internal
diff --git a/contracts/gas-snapshots/keystone.gas-snapshot b/contracts/gas-snapshots/keystone.gas-snapshot
@@ -63,24 +63,25 @@ CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DuplicateCapabilityAdded() (g
 CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DuplicateNodeAdded() (gas: 107643)
 CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_NodeDoesNotSupportCapability() (gas: 163357)
 CapabilitiesRegistry_UpdateDONTest:test_UpdatesDON() (gas: 371909)
-CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_CalledByNonAdminAndNonOwner() (gas: 20631)
+CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_CalledByNonAdminAndNonOwner() (gas: 20728)
 CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_NodeOperatorAdminIsZeroAddress() (gas: 20052)
 CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_NodeOperatorDoesNotExist() (gas: 19790)
 CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_NodeOperatorIdAndParamLengthsMismatch() (gas: 15430)
-CapabilitiesRegistry_UpdateNodeOperatorTest:test_UpdatesNodeOperator() (gas: 36937)
-CapabilitiesRegistry_UpdateNodesTest:test_CanUpdateParamsIfNodeSignerAddressNoLongerUsed() (gas: 256157)
-CapabilitiesRegistry_UpdateNodesTest:test_OwnerCanUpdateNodes() (gas: 162059)
-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: 470803)
-CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_RemovingCapabilityRequiredByWorkflowDON() (gas: 341084)
-CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_SignerAddressEmpty() (gas: 26951)
-CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_UpdatingNodeWithoutCapabilities() (gas: 25480)
-CapabilitiesRegistry_UpdateNodesTest:test_UpdatesNodeParams() (gas: 162113)
-KeystoneForwarder_ReportTest:test_Report_ConfigVersion() (gas: 1797755)
+CapabilitiesRegistry_UpdateNodeOperatorTest:test_UpdatesNodeOperator() (gas: 37034)
+CapabilitiesRegistry_UpdateNodesTest:test_CanUpdateParamsIfNodeSignerAddressNoLongerUsed() (gas: 256371)
+CapabilitiesRegistry_UpdateNodesTest:test_OwnerCanUpdateNodes() (gas: 162166)
+CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_AddingNodeWithInvalidCapability() (gas: 35873)
+CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_CalledByAnotherNodeOperatorAdmin() (gas: 29200)
+CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_CalledByNonNodeOperatorAdminAndNonOwner() (gas: 29377)
+CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_NodeDoesNotExist() (gas: 29199)
+CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_NodeSignerAlreadyAssignedToAnotherNode() (gas: 31326)
+CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_P2PIDEmpty() (gas: 29165)
+CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_RemovingCapabilityRequiredByCapabilityDON() (gas: 470910)
+CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_RemovingCapabilityRequiredByWorkflowDON() (gas: 341191)
+CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_SignerAddressEmpty() (gas: 29058)
+CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_UpdatingNodeWithoutCapabilities() (gas: 27587)
+CapabilitiesRegistry_UpdateNodesTest:test_UpdatesNodeParams() (gas: 162220)
+KeystoneForwarder_ReportTest:test_Report_ConfigVersion() (gas: 1798375)
 KeystoneForwarder_ReportTest:test_Report_FailedDeliveryWhenReceiverInterfaceNotSupported() (gas: 125910)
 KeystoneForwarder_ReportTest:test_Report_FailedDeliveryWhenReceiverNotContract() (gas: 127403)
 KeystoneForwarder_ReportTest:test_Report_SuccessfulDelivery() (gas: 155928)
@@ -96,10 +97,11 @@ KeystoneForwarder_ReportTest:test_RevertWhen_TooManySignatures() (gas: 56050)
 KeystoneForwarder_SetConfigTest:test_RevertWhen_ExcessSigners() (gas: 20184)
 KeystoneForwarder_SetConfigTest:test_RevertWhen_FaultToleranceIsZero() (gas: 88057)
 KeystoneForwarder_SetConfigTest:test_RevertWhen_InsufficientSigners() (gas: 14533)
-KeystoneForwarder_SetConfigTest:test_RevertWhen_NotOwner() (gas: 88788)
-KeystoneForwarder_SetConfigTest:test_RevertWhen_ProvidingDuplicateSigners() (gas: 114507)
-KeystoneForwarder_SetConfigTest:test_SetConfig_FirstTime() (gas: 1539921)
-KeystoneForwarder_SetConfigTest:test_SetConfig_WhenSignersAreRemoved() (gas: 1534476)
+KeystoneForwarder_SetConfigTest:test_RevertWhen_NotOwner() (gas: 88766)
+KeystoneForwarder_SetConfigTest:test_RevertWhen_ProvidingDuplicateSigners() (gas: 114570)
+KeystoneForwarder_SetConfigTest:test_RevertWhen_ProvidingZeroAddressSigner() (gas: 114225)
+KeystoneForwarder_SetConfigTest:test_SetConfig_FirstTime() (gas: 1540541)
+KeystoneForwarder_SetConfigTest:test_SetConfig_WhenSignersAreRemoved() (gas: 1535211)
 KeystoneForwarder_TypeAndVersionTest:test_TypeAndVersion() (gas: 9641)
 KeystoneRouter_SetConfigTest:test_AddForwarder_RevertWhen_NotOwner() (gas: 10978)
 KeystoneRouter_SetConfigTest:test_RemoveForwarder_RevertWhen_NotOwner() (gas: 10923)

diff --git a/contracts/src/v0.8/keystone/KeystoneForwarder.sol b/contracts/src/v0.8/keystone/KeystoneForwarder.sol
@@ -49,7 +49,7 @@ contract KeystoneForwarder is OwnerIsCreator, ITypeAndVersion, IRouter {
   error InvalidConfig(uint64 configId);
 
   /// @notice This error is thrown whenever a signer address is not in the
-  /// configuration.
+  /// configuration or when trying to set a zero address as a signer.
   /// @param signer The signer address that was not in the configuration
   error InvalidSigner(address signer);
 
@@ -187,6 +187,7 @@ contract KeystoneForwarder is OwnerIsCreator, ITypeAndVersion, IRouter {
     for (uint256 i = 0; i < signers.length; ++i) {
       // assign indices, detect duplicates
       address signer = signers[i];
+      if (signer == address(0)) revert InvalidSigner(signer);
       if (s_configs[configId]._positions[signer] != 0) revert DuplicateSigner(signer);
       s_configs[configId]._positions[signer] = i + 1;
     }

diff --git a/contracts/src/v0.8/keystone/OCR3Capability.sol b/contracts/src/v0.8/keystone/OCR3Capability.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {OCR2Base} from "./ocr/OCR2Base.sol";
 

diff --git a/contracts/src/v0.8/keystone/interfaces/ICapabilityConfiguration.sol b/contracts/src/v0.8/keystone/interfaces/ICapabilityConfiguration.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 /// @notice Interface for capability configuration contract. It MUST be
 /// implemented for a contract to be used as a capability configuration.

diff --git a/contracts/src/v0.8/keystone/interfaces/IReceiver.sol b/contracts/src/v0.8/keystone/interfaces/IReceiver.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 /// @title IReceiver - receives keystone reports
 interface IReceiver {

diff --git a/contracts/src/v0.8/keystone/interfaces/IRouter.sol b/contracts/src/v0.8/keystone/interfaces/IRouter.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 /// @title IRouter - delivers keystone reports to receiver
 interface IRouter {

diff --git a/contracts/src/v0.8/keystone/ocr/OCR2Abstract.sol b/contracts/src/v0.8/keystone/ocr/OCR2Abstract.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {ITypeAndVersion} from "../../shared/interfaces/ITypeAndVersion.sol";
 

diff --git a/contracts/src/v0.8/keystone/test/BaseTest.t.sol b/contracts/src/v0.8/keystone/test/BaseTest.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {Test} from "forge-std/Test.sol";
 import {Constants} from "./Constants.t.sol";

diff --git a/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_AddDONTest.t.sol b/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_AddDONTest.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {BaseTest} from "./BaseTest.t.sol";
 import {ICapabilityConfiguration} from "../interfaces/ICapabilityConfiguration.sol";

diff --git a/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_DeprecateCapabilitiesTest.t.sol b/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_DeprecateCapabilitiesTest.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {BaseTest} from "./BaseTest.t.sol";
 import {CapabilitiesRegistry} from "../CapabilitiesRegistry.sol";

diff --git a/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_GetCapabilitiesTest.t.sol b/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_GetCapabilitiesTest.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {BaseTest} from "./BaseTest.t.sol";
 import {CapabilitiesRegistry} from "../CapabilitiesRegistry.sol";

diff --git a/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_GetDONsTest.t.sol b/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_GetDONsTest.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {BaseTest} from "./BaseTest.t.sol";
 

diff --git a/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_GetHashedCapabilityIdTest.t.sol b/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_GetHashedCapabilityIdTest.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {BaseTest} from "./BaseTest.t.sol";
 import {CapabilityConfigurationContract} from "./mocks/CapabilityConfigurationContract.sol";

diff --git a/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_GetNodeOperatorsTest.t.sol b/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_GetNodeOperatorsTest.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {BaseTest} from "./BaseTest.t.sol";
 import {CapabilitiesRegistry} from "../CapabilitiesRegistry.sol";

diff --git a/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_GetNodesTest.t.sol b/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_GetNodesTest.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {BaseTest} from "./BaseTest.t.sol";
 import {CapabilitiesRegistry} from "../CapabilitiesRegistry.sol";

diff --git a/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_UpdateNodeOperatorsTest.t.sol b/contracts/src/v0.8/keystone/test/CapabilitiesRegistry_UpdateNodeOperatorsTest.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {BaseTest} from "./BaseTest.t.sol";
 import {CapabilitiesRegistry} from "../CapabilitiesRegistry.sol";

diff --git a/contracts/src/v0.8/keystone/test/Constants.t.sol b/contracts/src/v0.8/keystone/test/Constants.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 contract Constants {
   address internal constant ADMIN = address(1);

diff --git a/contracts/src/v0.8/keystone/test/KeystoneForwarderBaseTest.t.sol b/contracts/src/v0.8/keystone/test/KeystoneForwarderBaseTest.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {Test} from "forge-std/Test.sol";
 import {Receiver} from "./mocks/Receiver.sol";

diff --git a/contracts/src/v0.8/keystone/test/KeystoneForwarder_ReportTest.t.sol b/contracts/src/v0.8/keystone/test/KeystoneForwarder_ReportTest.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {BaseTest} from "./KeystoneForwarderBaseTest.t.sol";
 import {IRouter} from "../interfaces/IRouter.sol";

diff --git a/contracts/src/v0.8/keystone/test/KeystoneForwarder_SetConfigTest.t.sol b/contracts/src/v0.8/keystone/test/KeystoneForwarder_SetConfigTest.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {BaseTest} from "./KeystoneForwarderBaseTest.t.sol";
 import {KeystoneForwarder} from "../KeystoneForwarder.sol";
@@ -41,6 +41,14 @@ contract KeystoneForwarder_SetConfigTest is BaseTest {
     s_forwarder.setConfig(DON_ID, CONFIG_VERSION, F, signers);
   }
 
+  function test_RevertWhen_ProvidingZeroAddressSigner() public {
+    address[] memory signers = _getSignerAddresses();
+    signers[1] = address(0);
+
+    vm.expectRevert(abi.encodeWithSelector(KeystoneForwarder.InvalidSigner.selector, signers[1]));
+    s_forwarder.setConfig(DON_ID, CONFIG_VERSION, F, signers);
+  }
+
   function test_SetConfig_FirstTime() public {
     s_forwarder.setConfig(DON_ID, CONFIG_VERSION, F, _getSignerAddresses());
   }

diff --git a/contracts/src/v0.8/keystone/test/KeystoneForwarder_TypeAndVersionTest.t.sol b/contracts/src/v0.8/keystone/test/KeystoneForwarder_TypeAndVersionTest.t.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {BaseTest} from "./KeystoneForwarderBaseTest.t.sol";
 

diff --git a/contracts/src/v0.8/keystone/test/mocks/Receiver.sol b/contracts/src/v0.8/keystone/test/mocks/Receiver.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.19;
+pragma solidity 0.8.24;
 
 import {IReceiver} from "../../interfaces/IReceiver.sol";
 

diff --git a/core/gethwrappers/keystone/generated/forwarder/forwarder.go b/core/gethwrappers/keystone/generated/forwarder/forwarder.go
diff --git a/core/gethwrappers/keystone/generation/generated-wrapper-dependency-versions-do-not-edit.txt b/core/gethwrappers/keystone/generation/generated-wrapper-dependency-versions-do-not-edit.txt
@@ -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 6d2e3aa3a6f3aed2cf24b613743bb9ae4b9558f48a6864dc03b8b0ebb37235e3
 feeds_consumer: ../../../contracts/solc/v0.8.24/KeystoneFeedsConsumer/KeystoneFeedsConsumer.abi ../../../contracts/solc/v0.8.24/KeystoneFeedsConsumer/KeystoneFeedsConsumer.bin f098e25df6afc100425fcad7f5107aec0844cc98315117e49da139a179d0eead
-forwarder: ../../../contracts/solc/v0.8.24/KeystoneForwarder/KeystoneForwarder.abi ../../../contracts/solc/v0.8.24/KeystoneForwarder/KeystoneForwarder.bin dc98a86a3775ead987b79d5b6079ee0e26f31c0626032bdd6508f986e2423227
+forwarder: ../../../contracts/solc/v0.8.24/KeystoneForwarder/KeystoneForwarder.abi ../../../contracts/solc/v0.8.24/KeystoneForwarder/KeystoneForwarder.bin 21a203d62a69338a5ca260907a31727421114ca25679330ada5d68f0092725bf
 ocr3_capability: ../../../contracts/solc/v0.8.24/OCR3Capability/OCR3Capability.abi ../../../contracts/solc/v0.8.24/OCR3Capability/OCR3Capability.bin 8bf0f53f222efce7143dea6134552eb26ea1eef845407b4475a0d79b7d7ba9f8