Validate transmitter lengths against F values

contracts/src/v0.8/ccip/capability/CCIPConfig.sol

@@ -34,11 +34,11 @@ contract CCIPConfig is ITypeAndVersion, ICapabilityConfiguration, OwnerIsCreator
   error ChainSelectorNotSet();
   error TooManyOCR3Configs();
   error TooManySigners();
-  error P2PIdsLengthNotMatching(uint256 p2pIdsLength, uint256 signersLength, uint256 transmittersLength);
+  error InvalidNode(CCIPConfigTypes.OCR3Node node);
   error NotEnoughTransmitters(uint256 got, uint256 minimum);
-  error FMustBePositive();
   error FChainMustBePositive();
   error FTooHigh();
+  error FChainTooHigh(uint256 fChain, uint256 FRoleDON);
   error InvalidPluginType();
   error OfframpAddressCannotBeZero();
   error InvalidConfigLength(uint256 length);
@@ -167,6 +167,7 @@ contract CCIPConfig is ITypeAndVersion, ICapabilityConfiguration, OwnerIsCreator
   }
 
   /// @notice Called by the registry prior to the config being set for a particular DON.
+  /// @dev precondition Requires destination chain config to be set
   function beforeCapabilityConfigSet(
     bytes32[] calldata, /* nodes */
     bytes calldata config,
@@ -213,7 +214,32 @@ contract CCIPConfig is ITypeAndVersion, ICapabilityConfiguration, OwnerIsCreator
     // We won't run out of gas from this delete since the array is at most 2 elements long.
     delete s_ocr3Configs[donId][pluginType];
     for (uint256 i = 0; i < newConfigWithMeta.length; ++i) {
-      s_ocr3Configs[donId][pluginType].push(newConfigWithMeta[i]);
+      // Struct has to be manually copied since there is a nested OCR3Node array. Direct assignment
+      // will result in Unimplemented Feature issue.
+      CCIPConfigTypes.OCR3ConfigWithMeta storage ocr3ConfigWithMeta = s_ocr3Configs[donId][pluginType].push();
+      ocr3ConfigWithMeta.configDigest = newConfigWithMeta[i].configDigest;
+      ocr3ConfigWithMeta.configCount = newConfigWithMeta[i].configCount;
+
+      CCIPConfigTypes.OCR3Config storage ocr3Config = ocr3ConfigWithMeta.config;
+      CCIPConfigTypes.OCR3Config memory newOcr3Config = newConfigWithMeta[i].config;
+      ocr3Config.pluginType = newOcr3Config.pluginType;
+      ocr3Config.chainSelector = newOcr3Config.chainSelector;
+      ocr3Config.FRoleDON = newOcr3Config.FRoleDON;
+      ocr3Config.offchainConfigVersion = newOcr3Config.offchainConfigVersion;
+      ocr3Config.offrampAddress = newOcr3Config.offrampAddress;
+      ocr3Config.offchainConfig = newOcr3Config.offchainConfig;
+
+      // Remove all excess nodes
+      while (ocr3Config.nodes.length > newOcr3Config.nodes.length) {
+        ocr3Config.nodes.pop();
+      }
+
+      // Assign nodes
+      for (uint256 j = 0; j < newOcr3Config.nodes.length; ++j) {
+        if (j >= ocr3Config.nodes.length) {
+          ocr3Config.nodes.push(newOcr3Config.nodes[j]);
+        }
+      }
     }
 
     emit ConfigSet(donId, uint8(pluginType), newConfigWithMeta);
@@ -402,21 +428,49 @@ contract CCIPConfig is ITypeAndVersion, ICapabilityConfiguration, OwnerIsCreator
     }
     if (!s_remoteChainSelectors.contains(cfg.chainSelector)) revert ChainSelectorNotFound(cfg.chainSelector);
 
-    // We check for chain config presence above, so fChain here must be non-zero.
-    uint256 minTransmittersLength = 3 * s_chainConfigurations[cfg.chainSelector].fChain + 1;
-    if (cfg.transmitters.length < minTransmittersLength) {
-      revert NotEnoughTransmitters(cfg.transmitters.length, minTransmittersLength);
+    // fChain cannot exceed FRoleDON, since it is a subcommittee in the larger DON
+    uint256 FRoleDON = cfg.FRoleDON;
+    uint256 fChain = s_chainConfigurations[cfg.chainSelector].fChain;
+    // fChain > 0 is enforced in applyChainConfigUpdates, and the presence of a chain config is checked above
+    // FRoleDON != 0 because FRoleDON >= fChain is enforced here
+    if (fChain > FRoleDON) {
+      revert FChainTooHigh(fChain, FRoleDON);
+    }
+
+    // len(nodes) >= 3 * FRoleDON + 1
+    // len(nodes) == numberOfSigners
+    uint256 numberOfNodes = cfg.nodes.length;
+    if (numberOfNodes > MAX_NUM_ORACLES) revert TooManySigners();
+    if (numberOfNodes <= 3 * FRoleDON) revert FTooHigh();
+
+    uint256 nonZeroTransmitters = 0;
+    bytes32[] memory p2pIds = new bytes32[](numberOfNodes);
+    for (uint256 i = 0; i < numberOfNodes; ++i) {
+      CCIPConfigTypes.OCR3Node memory node = cfg.nodes[i];
+
+      // 3 * fChain + 1 <= nonZeroTransmitters <= 3 * FRoleDON + 1
+      // Transmitters can be set to 0 since there can be more signers than transmitters,
+      if (node.transmitterKey.length != 0) {
+        nonZeroTransmitters++;
+      }
+
+      // Signer key and p2pIds must always be present
+      if (node.signerKey.length == 0 || node.p2pId == bytes32(0)) {
+        revert InvalidNode(node);
+      }
+
+      p2pIds[i] = node.p2pId;
     }
-    uint256 numberOfSigners = cfg.signers.length;
-    if (numberOfSigners > MAX_NUM_ORACLES) revert TooManySigners();
-    if (numberOfSigners != cfg.p2pIds.length || numberOfSigners != cfg.transmitters.length) {
-      revert P2PIdsLengthNotMatching(cfg.p2pIds.length, cfg.signers.length, cfg.transmitters.length);
+
+    // We check for chain config presence above, so fChain here must be non-zero. fChain <= FRoleDON due to the checks above.
+    // There can be less transmitters than signers - so they can be set to zero (which indicates that a node is a signer, but not a transmitter).
+    uint256 minTransmittersLength = 3 * fChain + 1;
+    if (nonZeroTransmitters < minTransmittersLength) {
+      revert NotEnoughTransmitters(nonZeroTransmitters, minTransmittersLength);
     }
-    if (cfg.F == 0) revert FMustBePositive();
-    if (numberOfSigners <= 3 * cfg.F) revert FTooHigh();
 
     // Check that the readers are in the capabilities registry.
-    _ensureInRegistry(cfg.p2pIds);
+    _ensureInRegistry(p2pIds);
   }
 
   /// @notice Computes the digest of the provided configuration.
@@ -441,10 +495,8 @@ contract CCIPConfig is ITypeAndVersion, ICapabilityConfiguration, OwnerIsCreator
           ocr3Config.pluginType,
           ocr3Config.offrampAddress,
           configCount,
-          ocr3Config.p2pIds,
-          ocr3Config.signers,
-          ocr3Config.transmitters,
-          ocr3Config.F,
+          ocr3Config.nodes,
+          ocr3Config.FRoleDON,
           ocr3Config.offchainConfigVersion,
           ocr3Config.offchainConfig
         )
@@ -459,6 +511,7 @@ contract CCIPConfig is ITypeAndVersion, ICapabilityConfiguration, OwnerIsCreator
   // ================================================================
 
   /// @notice Sets and/or removes chain configurations.
+  /// Does not validate that fChain <= FRoleDON and relies on OCR3Configs to be changed in case fChain becomes larger than the FRoleDON value.
   /// @param chainSelectorRemoves The chain configurations to remove.
   /// @param chainConfigAdds The chain configurations to add.
   function applyChainConfigUpdates(

contracts/src/v0.8/ccip/capability/libraries/CCIPConfigTypes.sol

@@ -37,19 +37,25 @@ library CCIPConfigTypes {
     ChainConfig chainConfig;
   }
 
+  /// @notice Represents an oracle node in OCR3 configs part of the role DON.
+  /// Every configured node should be a signer, but does not have to be a transmitter.
+  struct OCR3Node {
+    bytes32 p2pId; // Peer2Peer connection ID of the oracle
+    bytes signerKey; // On-chain signer public key
+    bytes transmitterKey; // On-chain transmitter public key. Can be set to empty bytes to represent that the node is a signer but not a transmitter.
+  }
+
   /// @notice OCR3 configuration.
+  /// Note that FRoleDON >= fChain, since FRoleDON represents the role DON, and fChain represents sub-committees.
+  /// FRoleDON values are typically identical across multiple OCR3 configs since the chains pertain to one role DON,
+  /// but FRoleDON values can change across OCR3 configs to indicate role DON splits.
   struct OCR3Config {
     Internal.OCRPluginType pluginType; // ────────╮ The plugin that the configuration is for.
     uint64 chainSelector; //                      | The (remote) chain that the configuration is for.
-    uint8 F; //                                   | The "big F" parameter for the role DON.
+    uint8 FRoleDON; //                            | The "big F" parameter for the role DON.
     uint64 offchainConfigVersion; // ─────────────╯ The version of the offchain configuration.
     bytes offrampAddress; // The remote chain offramp address.
-    // 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.
-    // This is crucial in order to build the oracle ID <-> peer ID mapping offchain.
-    bytes32[] p2pIds; // The P2P IDs of the oracles that are part of the role DON.
-    bytes[] signers; // The onchain signing keys of nodes in the don.
-    bytes[] transmitters; // The onchain transmitter keys of nodes in the don.
+    OCR3Node[] nodes; // Keys & IDs of nodes part of the role DON
     bytes offchainConfig; // The offchain configuration for the OCR3 protocol. Protobuf encoded.
   }
 

contracts/src/v0.8/ccip/ocr/MultiOCR3Base.sol

@@ -28,7 +28,8 @@ abstract contract MultiOCR3Base is ITypeAndVersion, OwnerIsCreator {
     TOO_MANY_TRANSMITTERS,
     TOO_MANY_SIGNERS,
     F_TOO_HIGH,
-    REPEATED_ORACLE_ADDRESS
+    REPEATED_ORACLE_ADDRESS,
+    NO_TRANSMITTERS
   }
 
   error InvalidConfig(InvalidConfigErrorType errorType);
@@ -74,6 +75,7 @@ abstract contract MultiOCR3Base is ITypeAndVersion, OwnerIsCreator {
   /// @notice OCR configuration for a single OCR plugin within a DON.
   struct OCRConfig {
     ConfigInfo configInfo; //  latest OCR config
+    // NOTE: len(signers) can be different from len(transmitters). There is no index relationship between the two arrays
     address[] signers; //      addresses oracles use to sign the reports
     address[] transmitters; // addresses oracles use to transmit the reports
   }
@@ -123,6 +125,9 @@ abstract contract MultiOCR3Base is ITypeAndVersion, OwnerIsCreator {
   /// NOTE: The OCR3 config must be sanity-checked against the home-chain registry configuration, to ensure
   /// home-chain and remote-chain parity!
   /// @param ocrConfigArgs OCR config update args.
+  /// @dev precondition number of transmitters should match the expected F/fChain relationship.
+  /// For transmitters, the function only validates that len(transmitters) > 0 && len(transmitters) <= MAX_NUM_ORACLES
+  /// && len(transmitters) <= len(signers) [if sig verification is enabled]
   function setOCR3Configs(OCRConfigArgs[] memory ocrConfigArgs) external onlyOwner {
     for (uint256 i; i < ocrConfigArgs.length; ++i) {
       _setOCR3Config(ocrConfigArgs[i]);
@@ -147,6 +152,7 @@ abstract contract MultiOCR3Base is ITypeAndVersion, OwnerIsCreator {
 
     address[] memory transmitters = ocrConfigArgs.transmitters;
     if (transmitters.length > MAX_NUM_ORACLES) revert InvalidConfig(InvalidConfigErrorType.TOO_MANY_TRANSMITTERS);
+    if (transmitters.length == 0) revert InvalidConfig(InvalidConfigErrorType.NO_TRANSMITTERS);
 
     _clearOracleRoles(ocrPluginType, ocrConfig.transmitters);
 
@@ -157,6 +163,9 @@ abstract contract MultiOCR3Base is ITypeAndVersion, OwnerIsCreator {
 
       if (signers.length > MAX_NUM_ORACLES) revert InvalidConfig(InvalidConfigErrorType.TOO_MANY_SIGNERS);
       if (signers.length <= 3 * ocrConfigArgs.F) revert InvalidConfig(InvalidConfigErrorType.F_TOO_HIGH);
+      // NOTE: Transmitters cannot exceed signers. Transmitters do not have to be >= 3F + 1 because they can match >= 3fChain + 1, where fChain <= F.
+      // fChain is not represented in MultiOCR3Base - so we skip this check.
+      if (signers.length < transmitters.length) revert InvalidConfig(InvalidConfigErrorType.TOO_MANY_TRANSMITTERS);
 
       configInfo.n = uint8(signers.length);
       ocrConfig.signers = signers;