Remove Functions OCR2Base config digest check & use custom errors (#1…

…1249)

contracts/gas-snapshots/functions.gas-snapshot

@@ -1,12 +1,12 @@
-ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumGoerli() (gas: 14534206)
-ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumMainnet() (gas: 14534184)
-ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumSepolia() (gas: 14534200)
-ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseGoerli() (gas: 14545620)
-ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseMainnet() (gas: 14545597)
-ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseSepolia() (gas: 14545569)
-ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismGoerli() (gas: 14545520)
-ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismMainnet() (gas: 14545509)
-ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismSepolia() (gas: 14545553)
+ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumGoerli() (gas: 14600662)
+ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumMainnet() (gas: 14600640)
+ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumSepolia() (gas: 14600656)
+ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseGoerli() (gas: 14612076)
+ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseMainnet() (gas: 14612053)
+ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseSepolia() (gas: 14612025)
+ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismGoerli() (gas: 14611976)
+ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismMainnet() (gas: 14611965)
+ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismSepolia() (gas: 14612009)
 FunctionsBilling_Constructor:test_Constructor_Success() (gas: 14812)
 FunctionsBilling_DeleteCommitment:test_DeleteCommitment_RevertIfNotRouter() (gas: 13282)
 FunctionsBilling_DeleteCommitment:test_DeleteCommitment_Success() (gas: 15897)
@@ -29,8 +29,8 @@ FunctionsBilling__DisperseFeePool:test__DisperseFeePool_RevertIfNotSet() (gas: 8
 FunctionsBilling__FulfillAndBill:test__FulfillAndBill_RevertIfInvalidCommitment() (gas: 13302)
 FunctionsBilling__FulfillAndBill:test__FulfillAndBill_Success() (gas: 180763)
 FunctionsClient_Constructor:test_Constructor_Success() (gas: 7573)
-FunctionsClient_FulfillRequest:test_FulfillRequest_MaximumGas() (gas: 504354)
-FunctionsClient_FulfillRequest:test_FulfillRequest_MinimumGas() (gas: 205558)
+FunctionsClient_FulfillRequest:test_FulfillRequest_MaximumGas() (gas: 497786)
+FunctionsClient_FulfillRequest:test_FulfillRequest_MinimumGas() (gas: 198990)
 FunctionsClient_HandleOracleFulfillment:test_HandleOracleFulfillment_RevertIfNotRouter() (gas: 14623)
 FunctionsClient_HandleOracleFulfillment:test_HandleOracleFulfillment_Success() (gas: 22923)
 FunctionsClient__SendRequest:test__SendRequest_RevertIfInvalidCallbackGasLimit() (gas: 55059)
@@ -51,17 +51,17 @@ FunctionsRequest_DEFAULT_BUFFER_SIZE:test_DEFAULT_BUFFER_SIZE() (gas: 246)
 FunctionsRequest_EncodeCBOR:test_EncodeCBOR_Success() (gas: 223)
 FunctionsRequest_REQUEST_DATA_VERSION:test_REQUEST_DATA_VERSION() (gas: 225)
 FunctionsRouter_Constructor:test_Constructor_Success() (gas: 12007)
-FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedCostExceedsCommitment() (gas: 174027)
-FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedInsufficientGas() (gas: 164358)
+FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedCostExceedsCommitment() (gas: 167459)
+FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedInsufficientGas() (gas: 157790)
 FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedInvalidCommitment() (gas: 38115)
 FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedInvalidRequestId() (gas: 35238)
-FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedSubscriptionBalanceInvariant() (gas: 182503)
+FunctionsRouter_Fulfill:test_Fulfill_RequestNotProcessedSubscriptionBalanceInvariant() (gas: 175935)
 FunctionsRouter_Fulfill:test_Fulfill_RevertIfNotCommittedCoordinator() (gas: 28086)
-FunctionsRouter_Fulfill:test_Fulfill_RevertIfPaused() (gas: 158046)
-FunctionsRouter_Fulfill:test_Fulfill_SuccessClientNoLongerExists() (gas: 327605)
-FunctionsRouter_Fulfill:test_Fulfill_SuccessFulfilled() (gas: 341226)
-FunctionsRouter_Fulfill:test_Fulfill_SuccessUserCallbackReverts() (gas: 2516507)
-FunctionsRouter_Fulfill:test_Fulfill_SuccessUserCallbackRunsOutOfGas() (gas: 546986)
+FunctionsRouter_Fulfill:test_Fulfill_RevertIfPaused() (gas: 151478)
+FunctionsRouter_Fulfill:test_Fulfill_SuccessClientNoLongerExists() (gas: 321037)
+FunctionsRouter_Fulfill:test_Fulfill_SuccessFulfilled() (gas: 334658)
+FunctionsRouter_Fulfill:test_Fulfill_SuccessUserCallbackReverts() (gas: 2509939)
+FunctionsRouter_Fulfill:test_Fulfill_SuccessUserCallbackRunsOutOfGas() (gas: 540418)
 FunctionsRouter_GetAdminFee:test_GetAdminFee_Success() (gas: 17983)
 FunctionsRouter_GetAllowListId:test_GetAllowListId_Success() (gas: 12904)
 FunctionsRouter_GetConfig:test_GetConfig_Success() (gas: 37159)
@@ -141,11 +141,11 @@ FunctionsSubscriptions_GetSubscriptionsInRange:test_GetSubscriptionsInRange_Reve
 FunctionsSubscriptions_GetSubscriptionsInRange:test_GetSubscriptionsInRange_RevertIfStartIsAfterEnd() (gas: 13459)
 FunctionsSubscriptions_GetSubscriptionsInRange:test_GetSubscriptionsInRange_Success() (gas: 59592)
 FunctionsSubscriptions_GetTotalBalance:test_GetTotalBalance_Success() (gas: 15010)
-FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoCalldata(uint96) (runs: 256, μ: 43685, ~: 45548)
-FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoSubscription(uint96) (runs: 256, μ: 46197, ~: 48060)
+FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoCalldata(uint96) (runs: 256, μ: 43863, ~: 45548)
+FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoSubscription(uint96) (runs: 256, μ: 46375, ~: 48060)
 FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNotLink(uint96) (runs: 256, μ: 14295, ~: 14295)
 FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfPaused(uint96) (runs: 256, μ: 51177, ~: 53040)
-FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_Success(uint96) (runs: 256, μ: 85879, ~: 89604)
+FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_Success(uint96) (runs: 256, μ: 86057, ~: 89604)
 FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfAmountMoreThanBalance() (gas: 20745)
 FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfBalanceInvariant() (gas: 189)
 FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfNoAmount() (gas: 15638)

contracts/src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol

@@ -44,7 +44,7 @@ contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilli
     address router,
     Config memory config,
     address linkToNativeFeed
-  ) OCR2Base(true) FunctionsBilling(router, config, linkToNativeFeed) {}
+  ) OCR2Base() FunctionsBilling(router, config, linkToNativeFeed) {}
 
   /// @inheritdoc IFunctionsCoordinator
   function getThresholdPublicKey() external view override returns (bytes memory) {
@@ -151,7 +151,9 @@ contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilli
       numberOfFulfillments != onchainMetadata.length ||
       numberOfFulfillments != offchainMetadata.length
     ) {
-      revert ReportInvalid();
+      revert ReportInvalid(
+        "All fields on the report must be of equal length: requestIds, results, errors, onchainMetadata, offchainMetadata"
+      );
     }
 
     // Bounded by "MaxRequestBatchSize" on the Job's ReportingPluginConfig

contracts/src/v0.8/functions/dev/v1_X/ocr/OCR2Base.sol

@@ -10,17 +10,10 @@ import {OCR2Abstract} from "./OCR2Abstract.sol";
  * doc, which refers to this contract as simply the "contract".
  */
 abstract contract OCR2Base is ConfirmedOwner, OCR2Abstract {
-  error ReportInvalid();
+  error ReportInvalid(string message);
   error InvalidConfig(string message);
 
-  bool internal immutable i_uniqueReports;
-
-  constructor(bool uniqueReports) ConfirmedOwner(msg.sender) {
-    i_uniqueReports = uniqueReports;
-  }
-
-  // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables
-  uint256 private constant maxUint32 = (1 << 32) - 1;
+  constructor() ConfirmedOwner(msg.sender) {}
 
   // incremented each time a new config is posted. This count is incorporated
   // into the config digest, to prevent replay attacks.
@@ -144,12 +137,12 @@ abstract contract OCR2Base is ConfirmedOwner, OCR2Abstract {
 
     // Bounded by MAX_NUM_ORACLES in OCR2Abstract.sol
     for (uint256 i = 0; i < args.signers.length; i++) {
+      if (args.signers[i] == address(0)) revert InvalidConfig("signer must not be empty");
+      if (args.transmitters[i] == address(0)) revert InvalidConfig("transmitter must not be empty");
       // add new signer/transmitter addresses
-      // solhint-disable-next-line custom-errors
-      require(s_oracles[args.signers[i]].role == Role.Unset, "repeated signer address");
+      if (s_oracles[args.signers[i]].role != Role.Unset) revert InvalidConfig("repeated signer address");
       s_oracles[args.signers[i]] = Oracle(uint8(i), Role.Signer);
-      // solhint-disable-next-line custom-errors
-      require(s_oracles[args.transmitters[i]].role == Role.Unset, "repeated transmitter address");
+      if (s_oracles[args.transmitters[i]].role != Role.Unset) revert InvalidConfig("repeated transmitter address");
       s_oracles[args.transmitters[i]] = Oracle(uint8(i), Role.Transmitter);
       s_signers.push(args.signers[i]);
       s_transmitters.push(args.transmitters[i]);
@@ -287,8 +280,7 @@ abstract contract OCR2Base is ConfirmedOwner, OCR2Abstract {
       ss.length *
       32 + // 32 bytes per entry in _ss
       0; // placeholder
-    // solhint-disable-next-line custom-errors
-    require(msg.data.length == expected, "calldata length mismatch");
+    if (msg.data.length != expected) revert ReportInvalid("calldata length mismatch");
   }
 
   /**
@@ -319,30 +311,20 @@ abstract contract OCR2Base is ConfirmedOwner, OCR2Abstract {
 
       emit Transmitted(configDigest, uint32(epochAndRound >> 8));
 
-      ConfigInfo memory configInfo = s_configInfo;
-      // solhint-disable-next-line custom-errors
-      require(configInfo.latestConfigDigest == configDigest, "configDigest mismatch");
+      // The following check is disabled to allow both current and proposed routes to submit reports using the same OCR config digest
+      // Chainlink Functions uses globally unique request IDs. Metadata about the request is stored and checked in the Coordinator and Router
+      // require(configInfo.latestConfigDigest == configDigest, "configDigest mismatch");
 
       _requireExpectedMsgDataLength(report, rs, ss);
 
-      uint256 expectedNumSignatures;
-      if (i_uniqueReports) {
-        expectedNumSignatures = (configInfo.n + configInfo.f) / 2 + 1;
-      } else {
-        expectedNumSignatures = configInfo.f + 1;
-      }
+      uint256 expectedNumSignatures = (s_configInfo.n + s_configInfo.f) / 2 + 1;
 
-      // solhint-disable-next-line custom-errors
-      require(rs.length == expectedNumSignatures, "wrong number of signatures");
-      // solhint-disable-next-line custom-errors
-      require(rs.length == ss.length, "signatures out of registration");
+      if (rs.length != expectedNumSignatures) revert ReportInvalid("wrong number of signatures");
+      if (rs.length != ss.length) revert ReportInvalid("report rs and ss must be of equal length");
 
       Oracle memory transmitter = s_oracles[msg.sender];
-      // solhint-disable-next-line custom-errors
-      require( // Check that sender is authorized to report
-        transmitter.role == Role.Transmitter && msg.sender == s_transmitters[transmitter.index],
-        "unauthorized transmitter"
-      );
+      if (transmitter.role != Role.Transmitter && msg.sender != s_transmitters[transmitter.index])
+        revert ReportInvalid("unauthorized transmitter");
     }
 
     address[MAX_NUM_ORACLES] memory signed;
@@ -357,10 +339,8 @@ abstract contract OCR2Base is ConfirmedOwner, OCR2Abstract {
       for (uint256 i = 0; i < rs.length; ++i) {
         address signer = ecrecover(h, uint8(rawVs[i]) + 27, rs[i], ss[i]);
         o = s_oracles[signer];
-        // solhint-disable-next-line custom-errors
-        require(o.role == Role.Signer, "address not authorized to sign");
-        // solhint-disable-next-line custom-errors
-        require(signed[o.index] == address(0), "non-unique signature");
+        if (o.role != Role.Signer) revert ReportInvalid("address not authorized to sign");
+        if (signed[o.index] != address(0)) revert ReportInvalid("non-unique signature");
         signed[o.index] = signer;
         signerCount += 1;
       }