From f0ffd77c38d51edaf8010033379edcf3c80048c7 Mon Sep 17 00:00:00 2001 From: 0xsuryansh Date: Wed, 10 Jul 2024 15:12:19 +0530 Subject: [PATCH] feat: CCIPConfig, adding exhaustive checks for p2pIds and bootstrapP2PIds Signed-off-by: 0xsuryansh --- contracts/gas-snapshots/ccip.gas-snapshot | 53 ++++--- .../src/v0.8/ccip/capability/CCIPConfig.sol | 7 +- .../capability/libraries/CCIPConfigTypes.sol | 1 + .../ccip/test/capability/CCIPConfig.t.sol | 145 ++++++++++++++++++ 4 files changed, 180 insertions(+), 26 deletions(-) diff --git a/contracts/gas-snapshots/ccip.gas-snapshot b/contracts/gas-snapshots/ccip.gas-snapshot index d76d62a606..993ada61f9 100644 --- a/contracts/gas-snapshots/ccip.gas-snapshot +++ b/contracts/gas-snapshots/ccip.gas-snapshot @@ -37,32 +37,32 @@ BurnWithFromMintTokenPool_lockOrBurn:test_Setup_Success() (gas: 24260) CCIPClientExample_sanity:test_ImmutableExamples_Success() (gas: 2132183) CCIPConfigSetup:test_getCapabilityConfiguration_Success() (gas: 9495) CCIPConfig_ConfigStateMachine:test__computeConfigDigest_Success() (gas: 70755) -CCIPConfig_ConfigStateMachine:test__computeNewConfigWithMeta_InitToRunning_Success() (gas: 357994) -CCIPConfig_ConfigStateMachine:test__computeNewConfigWithMeta_RunningToStaging_Success() (gas: 481619) -CCIPConfig_ConfigStateMachine:test__computeNewConfigWithMeta_StagingToRunning_Success() (gas: 447731) +CCIPConfig_ConfigStateMachine:test__computeNewConfigWithMeta_InitToRunning_Success() (gas: 363647) +CCIPConfig_ConfigStateMachine:test__computeNewConfigWithMeta_RunningToStaging_Success() (gas: 488774) +CCIPConfig_ConfigStateMachine:test__computeNewConfigWithMeta_StagingToRunning_Success() (gas: 453384) CCIPConfig_ConfigStateMachine:test__groupByPluginType_TooManyOCR3Configs_Reverts() (gas: 37027) CCIPConfig_ConfigStateMachine:test__groupByPluginType_threeCommitConfigs_Reverts() (gas: 61043) CCIPConfig_ConfigStateMachine:test__groupByPluginType_threeExecutionConfigs_Reverts() (gas: 60963) CCIPConfig_ConfigStateMachine:test__stateFromConfigLength_Success() (gas: 11764) CCIPConfig_ConfigStateMachine:test__validateConfigStateTransition_Success() (gas: 8765) -CCIPConfig_ConfigStateMachine:test__validateConfigTransition_InitToRunning_Success() (gas: 307840) +CCIPConfig_ConfigStateMachine:test__validateConfigTransition_InitToRunning_Success() (gas: 311991) CCIPConfig_ConfigStateMachine:test__validateConfigTransition_InitToRunning_WrongConfigCount_Reverts() (gas: 49663) CCIPConfig_ConfigStateMachine:test__validateConfigTransition_NonExistentConfigTransition_Reverts() (gas: 32275) -CCIPConfig_ConfigStateMachine:test__validateConfigTransition_RunningToStaging_Success() (gas: 372425) +CCIPConfig_ConfigStateMachine:test__validateConfigTransition_RunningToStaging_Success() (gas: 376576) CCIPConfig_ConfigStateMachine:test__validateConfigTransition_RunningToStaging_WrongConfigCount_Reverts() (gas: 120943) CCIPConfig_ConfigStateMachine:test__validateConfigTransition_RunningToStaging_WrongConfigDigestBlueGreen_Reverts() (gas: 157105) -CCIPConfig_ConfigStateMachine:test__validateConfigTransition_StagingToRunning_Success() (gas: 372201) +CCIPConfig_ConfigStateMachine:test__validateConfigTransition_StagingToRunning_Success() (gas: 376352) CCIPConfig_ConfigStateMachine:test__validateConfigTransition_StagingToRunning_WrongConfigDigest_Reverts() (gas: 157172) CCIPConfig_ConfigStateMachine:test_getCapabilityConfiguration_Success() (gas: 9583) -CCIPConfig__updatePluginConfig:test__updatePluginConfig_InitToRunning_Success() (gas: 1051740) +CCIPConfig__updatePluginConfig:test__updatePluginConfig_InitToRunning_Success() (gas: 1057393) CCIPConfig__updatePluginConfig:test__updatePluginConfig_InvalidConfigLength_Reverts() (gas: 27539) CCIPConfig__updatePluginConfig:test__updatePluginConfig_InvalidConfigStateTransition_Reverts() (gas: 23105) -CCIPConfig__updatePluginConfig:test__updatePluginConfig_RunningToStaging_Success() (gas: 2002384) -CCIPConfig__updatePluginConfig:test__updatePluginConfig_StagingToRunning_Success() (gas: 2608050) +CCIPConfig__updatePluginConfig:test__updatePluginConfig_RunningToStaging_Success() (gas: 2009309) +CCIPConfig__updatePluginConfig:test__updatePluginConfig_StagingToRunning_Success() (gas: 2616177) CCIPConfig__updatePluginConfig:test_getCapabilityConfiguration_Success() (gas: 9583) -CCIPConfig_beforeCapabilityConfigSet:test_beforeCapabilityConfigSet_CommitAndExecConfig_Success() (gas: 1844033) -CCIPConfig_beforeCapabilityConfigSet:test_beforeCapabilityConfigSet_CommitConfigOnly_Success() (gas: 1062709) -CCIPConfig_beforeCapabilityConfigSet:test_beforeCapabilityConfigSet_ExecConfigOnly_Success() (gas: 1062740) +CCIPConfig_beforeCapabilityConfigSet:test_beforeCapabilityConfigSet_CommitAndExecConfig_Success() (gas: 1851188) +CCIPConfig_beforeCapabilityConfigSet:test_beforeCapabilityConfigSet_CommitConfigOnly_Success() (gas: 1068362) +CCIPConfig_beforeCapabilityConfigSet:test_beforeCapabilityConfigSet_ExecConfigOnly_Success() (gas: 1068393) CCIPConfig_beforeCapabilityConfigSet:test_beforeCapabilityConfigSet_OnlyCapabilitiesRegistryCanCall_Reverts() (gas: 9599) CCIPConfig_beforeCapabilityConfigSet:test_beforeCapabilityConfigSet_ZeroLengthConfig_Success() (gas: 16070) CCIPConfig_beforeCapabilityConfigSet:test_getCapabilityConfiguration_Success() (gas: 9583) @@ -72,18 +72,23 @@ CCIPConfig_chainConfig:test_applyChainConfigUpdates_nodeNotInRegistry_Reverts() CCIPConfig_chainConfig:test_applyChainConfigUpdates_removeChainConfigs_Success() (gas: 267558) CCIPConfig_chainConfig:test_applyChainConfigUpdates_selectorNotFound_Reverts() (gas: 14829) CCIPConfig_chainConfig:test_getCapabilityConfiguration_Success() (gas: 9626) -CCIPConfig_validateConfig:test__validateConfig_ChainSelectorNotFound_Reverts() (gas: 290206) -CCIPConfig_validateConfig:test__validateConfig_ChainSelectorNotSet_Reverts() (gas: 287302) -CCIPConfig_validateConfig:test__validateConfig_FMustBePositive_Reverts() (gas: 288245) -CCIPConfig_validateConfig:test__validateConfig_FTooHigh_Reverts() (gas: 288389) -CCIPConfig_validateConfig:test__validateConfig_NodeNotInRegistry_Reverts() (gas: 293767) -CCIPConfig_validateConfig:test__validateConfig_NotEnoughTransmitters_Reverts() (gas: 1107053) -CCIPConfig_validateConfig:test__validateConfig_OfframpAddressCannotBeZero_Reverts() (gas: 287109) -CCIPConfig_validateConfig:test__validateConfig_P2PIdsLengthNotMatching_Reverts() (gas: 289078) -CCIPConfig_validateConfig:test__validateConfig_Success() (gas: 296533) -CCIPConfig_validateConfig:test__validateConfig_TooManyBootstrapP2PIds_Reverts() (gas: 290321) -CCIPConfig_validateConfig:test__validateConfig_TooManySigners_Reverts() (gas: 1160583) -CCIPConfig_validateConfig:test__validateConfig_TooManyTransmitters_Reverts() (gas: 1158919) +CCIPConfig_validateConfig:test__validateConfig_BootstrapP2PIdsHasDuplicates_Reverts() (gas: 294893) +CCIPConfig_validateConfig:test__validateConfig_BootstrapP2PIdsNotASubsetOfP2PIds_Reverts() (gas: 298325) +CCIPConfig_validateConfig:test__validateConfig_BootstrapP2PIdsNotSorted_Reverts() (gas: 295038) +CCIPConfig_validateConfig:test__validateConfig_ChainSelectorNotFound_Reverts() (gas: 294357) +CCIPConfig_validateConfig:test__validateConfig_ChainSelectorNotSet_Reverts() (gas: 291431) +CCIPConfig_validateConfig:test__validateConfig_FMustBePositive_Reverts() (gas: 292396) +CCIPConfig_validateConfig:test__validateConfig_FTooHigh_Reverts() (gas: 292540) +CCIPConfig_validateConfig:test__validateConfig_NodeNotInRegistry_Reverts() (gas: 299420) +CCIPConfig_validateConfig:test__validateConfig_NotEnoughTransmitters_Reverts() (gas: 1160094) +CCIPConfig_validateConfig:test__validateConfig_OfframpAddressCannotBeZero_Reverts() (gas: 291260) +CCIPConfig_validateConfig:test__validateConfig_P2PIdsHasDuplicates_Reverts() (gas: 295907) +CCIPConfig_validateConfig:test__validateConfig_P2PIdsLengthNotMatching_Reverts() (gas: 293229) +CCIPConfig_validateConfig:test__validateConfig_P2PIdsNotSorted_Reverts() (gas: 295623) +CCIPConfig_validateConfig:test__validateConfig_Success() (gas: 302186) +CCIPConfig_validateConfig:test__validateConfig_TooManyBootstrapP2PIds_Reverts() (gas: 294539) +CCIPConfig_validateConfig:test__validateConfig_TooManySigners_Reverts() (gas: 1215861) +CCIPConfig_validateConfig:test__validateConfig_TooManyTransmitters_Reverts() (gas: 1214264) CCIPConfig_validateConfig:test_getCapabilityConfiguration_Success() (gas: 9562) CommitStore_constructor:test_Constructor_Success() (gas: 3091326) CommitStore_isUnpausedAndRMNHealthy:test_RMN_Success() (gas: 73420) diff --git a/contracts/src/v0.8/ccip/capability/CCIPConfig.sol b/contracts/src/v0.8/ccip/capability/CCIPConfig.sol index 795db0c3e4..40b7a4a2f9 100644 --- a/contracts/src/v0.8/ccip/capability/CCIPConfig.sol +++ b/contracts/src/v0.8/ccip/capability/CCIPConfig.sol @@ -7,6 +7,7 @@ import {ICapabilitiesRegistry} from "./interfaces/ICapabilitiesRegistry.sol"; import {OwnerIsCreator} from "../../shared/access/OwnerIsCreator.sol"; +import {SortedSetValidationUtil} from "../../shared/util/SortedSetValidationUtil.sol"; import {Internal} from "../libraries/Internal.sol"; import {CCIPConfigTypes} from "./libraries/CCIPConfigTypes.sol"; @@ -370,9 +371,11 @@ contract CCIPConfig is ITypeAndVersion, ICapabilityConfiguration, OwnerIsCreator } if (cfg.bootstrapP2PIds.length > cfg.p2pIds.length) revert TooManyBootstrapP2PIds(); + // check for duplicate p2p ids and bootstrapP2PIds. + // check that p2p ids in cfg.bootstrapP2PIds are included in cfg.p2pIds. + SortedSetValidationUtil._checkIsValidUniqueSubset(cfg.bootstrapP2PIds, cfg.p2pIds); + // Check that the readers are in the capabilities registry. - // TODO: check for duplicate signers, duplicate p2p ids, etc. - // TODO: check that p2p ids in cfg.bootstrapP2PIds are included in cfg.p2pIds. for (uint256 i = 0; i < cfg.signers.length; ++i) { _ensureInRegistry(cfg.p2pIds[i]); } diff --git a/contracts/src/v0.8/ccip/capability/libraries/CCIPConfigTypes.sol b/contracts/src/v0.8/ccip/capability/libraries/CCIPConfigTypes.sol index 5f052b3509..99adef84b1 100644 --- a/contracts/src/v0.8/ccip/capability/libraries/CCIPConfigTypes.sol +++ b/contracts/src/v0.8/ccip/capability/libraries/CCIPConfigTypes.sol @@ -37,6 +37,7 @@ library CCIPConfigTypes { uint8 F; // | The "big F" parameter for the role DON. uint64 offchainConfigVersion; // ─────────────╯ The version of the offchain configuration. bytes offrampAddress; // The remote chain offramp address. + // NOTE: bootstrapP2PIds and p2pIds should be sent as sorted sets bytes32[] bootstrapP2PIds; // The bootstrap P2P IDs of the oracles that are part of the role DON. // len(p2pIds) == len(signers) == len(transmitters) == 3 * F + 1 // NOTE: indexes matter here! The p2p ID at index i corresponds to the signer at index i and the transmitter at index i. diff --git a/contracts/src/v0.8/ccip/test/capability/CCIPConfig.t.sol b/contracts/src/v0.8/ccip/test/capability/CCIPConfig.t.sol index 4caa4394be..0c3108d279 100644 --- a/contracts/src/v0.8/ccip/test/capability/CCIPConfig.t.sol +++ b/contracts/src/v0.8/ccip/test/capability/CCIPConfig.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.24; import {Test} from "forge-std/Test.sol"; +import {SortedSetValidationUtil} from "../../../shared/util/SortedSetValidationUtil.sol"; import {CCIPConfig} from "../../capability/CCIPConfig.sol"; import {ICapabilitiesRegistry} from "../../capability/interfaces/ICapabilitiesRegistry.sol"; import {CCIPConfigTypes} from "../../capability/libraries/CCIPConfigTypes.sol"; @@ -44,11 +45,31 @@ contract CCIPConfigSetup is Test { return subset; } + //TODO: Use OZ's Arrays.sort when we upgrade to OZ v5 + function _sort(bytes32[] memory arr, int256 left, int256 right) private pure { + int256 i = left; + int256 j = right; + if (i == j) return; + bytes32 pivot = arr[uint256(left + (right - left) / 2)]; + while (i <= j) { + while (arr[uint256(i)] < pivot) i++; + while (pivot < arr[uint256(j)]) j--; + if (i <= j) { + (arr[uint256(i)], arr[uint256(j)]) = (arr[uint256(j)], arr[uint256(i)]); + i++; + j--; + } + } + if (left < j) _sort(arr, left, j); + if (i < right) _sort(arr, i, right); + } + function _addChainConfig(uint256 numNodes) internal returns (bytes32[] memory p2pIds, bytes[] memory signers, bytes[] memory transmitters) { p2pIds = _makeBytes32Array(numNodes, 0); + _sort(p2pIds, 0, int256(numNodes - 1)); signers = _makeBytesArray(numNodes, 10); transmitters = _makeBytesArray(numNodes, 20); for (uint256 i = 0; i < numNodes; i++) { @@ -536,6 +557,130 @@ contract CCIPConfig_validateConfig is CCIPConfigSetup { vm.expectRevert(abi.encodeWithSelector(CCIPConfig.NodeNotInRegistry.selector, nonExistentP2PId)); s_ccipCC.validateConfig(config); } + + function test__validateConfig_P2PIdsNotSorted_Reverts() public { + (bytes32[] memory p2pIds, bytes[] memory signers, bytes[] memory transmitters) = _addChainConfig(4); + // Config is for 4 nodes, so f == 1. + + //swapping two adjacent p2pIds to make it unsorted + (p2pIds[2], p2pIds[3]) = (p2pIds[3], p2pIds[2]); + + CCIPConfigTypes.OCR3Config memory config = CCIPConfigTypes.OCR3Config({ + pluginType: Internal.OCRPluginType.Commit, + offrampAddress: abi.encodePacked(keccak256(abi.encode("offramp"))), + chainSelector: 1, + bootstrapP2PIds: _subset(p2pIds, 0, 1), + p2pIds: p2pIds, + signers: signers, + transmitters: transmitters, + F: 1, + offchainConfigVersion: 30, + offchainConfig: bytes("offchainConfig") + }); + + vm.expectRevert(abi.encodeWithSelector(SortedSetValidationUtil.NotASortedSet.selector, p2pIds)); + s_ccipCC.validateConfig(config); + } + + function test__validateConfig_BootstrapP2PIdsNotSorted_Reverts() public { + (bytes32[] memory p2pIds, bytes[] memory signers, bytes[] memory transmitters) = _addChainConfig(4); + // Config is for 4 nodes, so f == 1. + + bytes32[] memory bootstrapP2PIds = _subset(p2pIds, 0, 2); + + //swapping bootstrapP2PIds to make it unsorted + (bootstrapP2PIds[0], bootstrapP2PIds[1]) = (bootstrapP2PIds[1], bootstrapP2PIds[0]); + + CCIPConfigTypes.OCR3Config memory config = CCIPConfigTypes.OCR3Config({ + pluginType: Internal.OCRPluginType.Commit, + offrampAddress: abi.encodePacked(keccak256(abi.encode("offramp"))), + chainSelector: 1, + bootstrapP2PIds: bootstrapP2PIds, + p2pIds: p2pIds, + signers: signers, + transmitters: transmitters, + F: 1, + offchainConfigVersion: 30, + offchainConfig: bytes("offchainConfig") + }); + + vm.expectRevert(abi.encodeWithSelector(SortedSetValidationUtil.NotASortedSet.selector, bootstrapP2PIds)); + s_ccipCC.validateConfig(config); + } + + function test__validateConfig_P2PIdsHasDuplicates_Reverts() public { + (bytes32[] memory p2pIds, bytes[] memory signers, bytes[] memory transmitters) = _addChainConfig(4); + // Config is for 4 nodes, so f == 1. + + //forcing duplicate p2pIds + p2pIds[1] = p2pIds[2]; + + CCIPConfigTypes.OCR3Config memory config = CCIPConfigTypes.OCR3Config({ + pluginType: Internal.OCRPluginType.Commit, + offrampAddress: abi.encodePacked(keccak256(abi.encode("offramp"))), + chainSelector: 1, + bootstrapP2PIds: _subset(p2pIds, 0, 2), + p2pIds: p2pIds, + signers: signers, + transmitters: transmitters, + F: 1, + offchainConfigVersion: 30, + offchainConfig: bytes("offchainConfig") + }); + + vm.expectRevert(abi.encodeWithSelector(SortedSetValidationUtil.NotASortedSet.selector, p2pIds)); + s_ccipCC.validateConfig(config); + } + + function test__validateConfig_BootstrapP2PIdsHasDuplicates_Reverts() public { + (bytes32[] memory p2pIds, bytes[] memory signers, bytes[] memory transmitters) = _addChainConfig(4); + // Config is for 4 nodes, so f == 1. + + bytes32[] memory bootstrapP2PIds = _subset(p2pIds, 0, 2); + //forcing duplicate bootstrapP2PIds + bootstrapP2PIds[1] = bootstrapP2PIds[0]; + + CCIPConfigTypes.OCR3Config memory config = CCIPConfigTypes.OCR3Config({ + pluginType: Internal.OCRPluginType.Commit, + offrampAddress: abi.encodePacked(keccak256(abi.encode("offramp"))), + chainSelector: 1, + bootstrapP2PIds: bootstrapP2PIds, + p2pIds: p2pIds, + signers: signers, + transmitters: transmitters, + F: 1, + offchainConfigVersion: 30, + offchainConfig: bytes("offchainConfig") + }); + + vm.expectRevert(abi.encodeWithSelector(SortedSetValidationUtil.NotASortedSet.selector, bootstrapP2PIds)); + s_ccipCC.validateConfig(config); + } + + function test__validateConfig_BootstrapP2PIdsNotASubsetOfP2PIds_Reverts() public { + (bytes32[] memory p2pIds, bytes[] memory signers, bytes[] memory transmitters) = _addChainConfig(4); + // Config is for 4 nodes, so f == 1. + + //forcing invalid bootstrapP2PIds where the bootstrapP2PIds is sorted, but one of the element is not in the p2pIdsSet + bytes32[] memory bootstrapP2PIds = _subset(p2pIds, 0, 2); + p2pIds[1] = bytes32(uint256(p2pIds[0]) + 100); + + CCIPConfigTypes.OCR3Config memory config = CCIPConfigTypes.OCR3Config({ + pluginType: Internal.OCRPluginType.Commit, + offrampAddress: abi.encodePacked(keccak256(abi.encode("offramp"))), + chainSelector: 1, + bootstrapP2PIds: bootstrapP2PIds, + p2pIds: p2pIds, + signers: signers, + transmitters: transmitters, + F: 1, + offchainConfigVersion: 30, + offchainConfig: bytes("offchainConfig") + }); + + vm.expectRevert(abi.encodeWithSelector(SortedSetValidationUtil.NotASubset.selector, bootstrapP2PIds, p2pIds)); + s_ccipCC.validateConfig(config); + } } contract CCIPConfig_ConfigStateMachine is CCIPConfigSetup {