[Functions] Small comment & checks clean up from audit (#12388)

* (fix): Remove redundant check from TermsOfServiceAllowList.getAllowedSendersInRange()

* (doc): Add missing param natpsec for FunctionsClient._sendRequest()

* (doc): Amend comment line in Functions OCR2Base about ConfigInfo being 32bytes

* (doc): Amend comment line in FunctionsBilling.sol regarding getFee in Juels

* (fix): Add extra check to FunctionsBilling _disperseFeePool to save gas when attempting to disperse dust

* (doc): Add comment lines to reflect that a feedStalenessSeconds of 0 is a feature disabled state

* (refactor): Remove duplicated prefix expression in Functions OCR2Base

* (chore): Prettier

* (chore): Regen snapshot & wrappers

* (chore): Changeset

.changeset/lazy-cooks-agree.md

@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+Chainlink Functions contracts v1.3 audit findings

contracts/gas-snapshots/functions.gas-snapshot

@@ -1,12 +1,12 @@
-ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumGoerli() (gas: 15924776)
-ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumMainnet() (gas: 15924754)
-ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumSepolia() (gas: 15924770)
-ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseGoerli() (gas: 15936218)
-ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseMainnet() (gas: 15936195)
-ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseSepolia() (gas: 15936167)
-ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismGoerli() (gas: 15936118)
-ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismMainnet() (gas: 15936107)
-ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismSepolia() (gas: 15936151)
+ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumGoerli() (gas: 15926591)
+ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumMainnet() (gas: 15926569)
+ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumSepolia() (gas: 15926585)
+ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseGoerli() (gas: 15938033)
+ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseMainnet() (gas: 15938010)
+ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseSepolia() (gas: 15937982)
+ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismGoerli() (gas: 15937933)
+ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismMainnet() (gas: 15937922)
+ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismSepolia() (gas: 15937966)
 FunctionsBilling_Constructor:test_Constructor_Success() (gas: 14823)
 FunctionsBilling_DeleteCommitment:test_DeleteCommitment_RevertIfNotRouter() (gas: 13260)
 FunctionsBilling_DeleteCommitment:test_DeleteCommitment_Success() (gas: 15875)
@@ -18,13 +18,13 @@ FunctionsBilling_GetConfig:test_GetConfig_Success() (gas: 27553)
 FunctionsBilling_GetDONFeeJuels:test_GetDONFeeJuels_Success() (gas: 40831)
 FunctionsBilling_GetOperationFee:test_GetOperationFeeJuels_Success() (gas: 40211)
 FunctionsBilling_GetWeiPerUnitLink:test_GetWeiPerUnitLink_Success() (gas: 29414)
-FunctionsBilling_OracleWithdraw:test_OracleWithdraw_RevertIfInsufficientBalance() (gas: 70107)
-FunctionsBilling_OracleWithdraw:test_OracleWithdraw_RevertWithNoBalance() (gas: 106264)
-FunctionsBilling_OracleWithdraw:test_OracleWithdraw_SuccessCoordinatorOwner() (gas: 129542)
-FunctionsBilling_OracleWithdraw:test_OracleWithdraw_SuccessTransmitterWithBalanceNoAmountGiven() (gas: 169241)
-FunctionsBilling_OracleWithdraw:test_OracleWithdraw_SuccessTransmitterWithBalanceValidAmountGiven() (gas: 142476)
+FunctionsBilling_OracleWithdraw:test_OracleWithdraw_RevertIfInsufficientBalance() (gas: 70136)
+FunctionsBilling_OracleWithdraw:test_OracleWithdraw_RevertWithNoBalance() (gas: 106293)
+FunctionsBilling_OracleWithdraw:test_OracleWithdraw_SuccessCoordinatorOwner() (gas: 129571)
+FunctionsBilling_OracleWithdraw:test_OracleWithdraw_SuccessTransmitterWithBalanceNoAmountGiven() (gas: 169270)
+FunctionsBilling_OracleWithdraw:test_OracleWithdraw_SuccessTransmitterWithBalanceValidAmountGiven() (gas: 142505)
 FunctionsBilling_OracleWithdrawAll:test_OracleWithdrawAll_RevertIfNotOwner() (gas: 13297)
-FunctionsBilling_OracleWithdrawAll:test_OracleWithdrawAll_SuccessPaysTransmittersWithBalance() (gas: 217168)
+FunctionsBilling_OracleWithdrawAll:test_OracleWithdrawAll_SuccessPaysTransmittersWithBalance() (gas: 217197)
 FunctionsBilling_UpdateConfig:test_UpdateConfig_RevertIfNotOwner() (gas: 21521)
 FunctionsBilling_UpdateConfig:test_UpdateConfig_Success() (gas: 49192)
 FunctionsBilling__DisperseFeePool:test__DisperseFeePool_RevertIfNotSet() (gas: 8810)
@@ -208,12 +208,12 @@ FunctionsTermsOfServiceAllowList_BlockSender:test_BlockSender_Success() (gas: 96
 FunctionsTermsOfServiceAllowList_Constructor:test_Constructor_Success() (gas: 12253)
 FunctionsTermsOfServiceAllowList_GetAllAllowedSenders:test_GetAllAllowedSenders_Success() (gas: 19199)
 FunctionsTermsOfServiceAllowList_GetAllowedSendersCount:test_GetAllowedSendersCount_Success() (gas: 12995)
-FunctionsTermsOfServiceAllowList_GetAllowedSendersInRange:test_GetAllowedSendersInRange_RevertIfAllowedSendersIsEmpty() (gas: 13158901)
-FunctionsTermsOfServiceAllowList_GetAllowedSendersInRange:test_GetAllowedSendersInRange_RevertIfEndIsAfterLastAllowedSender() (gas: 16571)
-FunctionsTermsOfServiceAllowList_GetAllowedSendersInRange:test_GetAllowedSendersInRange_RevertIfStartIsAfterEnd() (gas: 13301)
-FunctionsTermsOfServiceAllowList_GetAllowedSendersInRange:test_GetAllowedSendersInRange_Success() (gas: 20448)
+FunctionsTermsOfServiceAllowList_GetAllowedSendersInRange:test_GetAllowedSendersInRange_RevertIfAllowedSendersIsEmpty() (gas: 13160699)
+FunctionsTermsOfServiceAllowList_GetAllowedSendersInRange:test_GetAllowedSendersInRange_RevertIfEndIsAfterLastAllowedSender() (gas: 16554)
+FunctionsTermsOfServiceAllowList_GetAllowedSendersInRange:test_GetAllowedSendersInRange_RevertIfStartIsAfterEnd() (gas: 13284)
+FunctionsTermsOfServiceAllowList_GetAllowedSendersInRange:test_GetAllowedSendersInRange_Success() (gas: 20268)
 FunctionsTermsOfServiceAllowList_GetBlockedSendersCount:test_GetBlockedSendersCount_Success() (gas: 12931)
-FunctionsTermsOfServiceAllowList_GetBlockedSendersInRange:test_GetBlockedSendersInRange_RevertIfAllowedSendersIsEmpty() (gas: 13158905)
+FunctionsTermsOfServiceAllowList_GetBlockedSendersInRange:test_GetBlockedSendersInRange_RevertIfAllowedSendersIsEmpty() (gas: 13160720)
 FunctionsTermsOfServiceAllowList_GetBlockedSendersInRange:test_GetBlockedSendersInRange_RevertIfEndIsAfterLastAllowedSender() (gas: 16549)
 FunctionsTermsOfServiceAllowList_GetBlockedSendersInRange:test_GetBlockedSendersInRange_RevertIfStartIsAfterEnd() (gas: 13367)
 FunctionsTermsOfServiceAllowList_GetBlockedSendersInRange:test_GetBlockedSendersInRange_Success() (gas: 18493)

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

@@ -106,13 +106,13 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
 
   /// @inheritdoc IFunctionsBilling
   function getDONFeeJuels(bytes memory /* requestData */) public view override returns (uint72) {
-    // s_config.donFee is in cents of USD. Get Juel amount then convert to dollars.
+    // s_config.donFee is in cents of USD. Convert to dollars amount then get amount of Juels.
     return SafeCast.toUint72(_getJuelsFromUsd(s_config.donFeeCentsUsd) / 100);
   }
 
   /// @inheritdoc IFunctionsBilling
   function getOperationFeeJuels() public view override returns (uint72) {
-    // s_config.donFee is in cents of USD. Get Juel amount then convert to dollars.
+    // s_config.donFee is in cents of USD. Convert to dollars then get amount of Juels.
     return SafeCast.toUint72(_getJuelsFromUsd(s_config.operationFeeCentsUsd) / 100);
   }
 
@@ -124,6 +124,7 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
   /// @inheritdoc IFunctionsBilling
   function getWeiPerUnitLink() public view returns (uint256) {
     (, int256 weiPerUnitLink, , uint256 timestamp, ) = s_linkToNativeFeed.latestRoundData();
+    // Only fallback if feedStalenessSeconds is set
     // solhint-disable-next-line not-rely-on-time
     if (s_config.feedStalenessSeconds < block.timestamp - timestamp && s_config.feedStalenessSeconds > 0) {
       return s_config.fallbackNativePerUnitLink;
@@ -143,6 +144,7 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
   /// @inheritdoc IFunctionsBilling
   function getUsdPerUnitLink() public view returns (uint256, uint8) {
     (, int256 usdPerUnitLink, , uint256 timestamp, ) = s_linkToUsdFeed.latestRoundData();
+    // Only fallback if feedStalenessSeconds is set
     // solhint-disable-next-line not-rely-on-time
     if (s_config.feedStalenessSeconds < block.timestamp - timestamp && s_config.feedStalenessSeconds > 0) {
       return (s_config.fallbackUsdPerUnitLink, s_config.fallbackUsdPerUnitLinkDecimals);
@@ -420,6 +422,10 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
       revert NoTransmittersSet();
     }
     uint96 feePoolShare = s_feePool / uint96(numberOfTransmitters);
+    if (feePoolShare == 0) {
+      // Dust cannot be evenly distributed to all transmitters
+      return;
+    }
     // Bounded by "maxNumOracles" on OCR2Abstract.sol
     for (uint256 i = 0; i < numberOfTransmitters; ++i) {
       s_withdrawableTokens[transmitters[i]] += feePoolShare;

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

@@ -25,7 +25,8 @@ abstract contract FunctionsClient is IFunctionsClient {
   /// @notice Sends a Chainlink Functions request
   /// @param data The CBOR encoded bytes data for a Functions request
   /// @param subscriptionId The subscription ID that will be charged to service the request
-  /// @param callbackGasLimit the amount of gas that will be available for the fulfillment callback
+  /// @param callbackGasLimit - The amount of gas that will be available for the fulfillment callback
+  /// @param donId - An identifier used to determine which route to send the request along
   /// @return requestId The generated request ID for this request
   function _sendRequest(
     bytes memory data,

contracts/src/v0.8/functions/dev/v1_X/accessControl/TermsOfServiceAllowList.sol

@@ -128,11 +128,7 @@ contract TermsOfServiceAllowList is ITermsOfServiceAllowList, IAccessController,
     uint64 allowedSenderIdxStart,
     uint64 allowedSenderIdxEnd
   ) external view override returns (address[] memory allowedSenders) {
-    if (
-      allowedSenderIdxStart > allowedSenderIdxEnd ||
-      allowedSenderIdxEnd >= s_allowedSenders.length() ||
-      s_allowedSenders.length() == 0
-    ) {
+    if (allowedSenderIdxStart > allowedSenderIdxEnd || allowedSenderIdxEnd >= s_allowedSenders.length()) {
       revert InvalidCalldata();
     }
 

contracts/src/v0.8/functions/dev/v1_X/interfaces/IFunctionsBilling.sol

@@ -59,7 +59,7 @@ interface IFunctionsBilling {
 
 struct FunctionsBillingConfig {
   uint32 fulfillmentGasPriceOverEstimationBP; // ══╗ Percentage of gas price overestimation to account for changes in gas price between request and response. Held as basis points (one hundredth of 1 percentage point)
-  uint32 feedStalenessSeconds; //                  ║ How long before we consider the feed price to be stale and fallback to fallbackNativePerUnitLink.
+  uint32 feedStalenessSeconds; //                  ║ How long before we consider the feed price to be stale and fallback to fallbackNativePerUnitLink. Default of 0 means no fallback.
   uint32 gasOverheadBeforeCallback; //             ║ Represents the average gas execution cost before the fulfillment callback. This amount is always billed for every request.
   uint32 gasOverheadAfterCallback; //              ║ Represents the average gas execution cost after the fulfillment callback. This amount is always billed for every request.
   uint40 minimumEstimateGasPriceWei; //            ║ The lowest amount of wei that will be used as the tx.gasprice when estimating the cost to fulfill the request

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

@@ -22,12 +22,12 @@ abstract contract OCR2Base is ConfirmedOwner, OCR2Abstract {
   // to extract config from logs.
 
   // Storing these fields used on the hot path in a ConfigInfo variable reduces the
-  // retrieval of all of them to a single SLOAD. If any further fields are
-  // added, make sure that storage of the struct still takes at most 32 bytes.
+  // retrieval of all of them into two SLOADs. If any further fields are
+  // added, make sure that storage of the struct still takes at most 64 bytes.
   struct ConfigInfo {
     bytes32 latestConfigDigest;
-    uint8 f; // TODO: could be optimized by squeezing into one slot
-    uint8 n;
+    uint8 f; // ───╮
+    uint8 n; // ───╯
   }
   ConfigInfo internal s_configInfo;
 
@@ -215,7 +215,7 @@ abstract contract OCR2Base is ConfirmedOwner, OCR2Abstract {
     );
     uint256 prefixMask = type(uint256).max << (256 - 16); // 0xFFFF00..00
     uint256 prefix = 0x0001 << (256 - 16); // 0x000100..00
-    return bytes32((prefix & prefixMask) | (h & ~prefixMask));
+    return bytes32(prefix | (h & ~prefixMask));
   }
 
   /**

core/gethwrappers/functions/generation/generated-wrapper-dependency-versions-do-not-edit.txt

@@ -1,10 +1,10 @@
 GETH_VERSION: 1.13.8
 functions: ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsRequest.abi ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsRequest.bin 3c972870b0afeb6d73a29ebb182f24956a2cebb127b21c4f867d1ecf19a762db
-functions_allow_list: ../../../contracts/solc/v0.8.19/functions/v1_X/TermsOfServiceAllowList.abi ../../../contracts/solc/v0.8.19/functions/v1_X/TermsOfServiceAllowList.bin 0c2156289e11f884ca6e92bf851192d3917c9094a0a301bcefa61266678d0e57
+functions_allow_list: ../../../contracts/solc/v0.8.19/functions/v1_X/TermsOfServiceAllowList.abi ../../../contracts/solc/v0.8.19/functions/v1_X/TermsOfServiceAllowList.bin 268de8b3c061b53a1a2a1ccc0149eff68545959e29cd41b5f2e9f5dab19075cf
 functions_billing_registry_events_mock: ../../../contracts/solc/v0.8.6/functions/v0_0_0/FunctionsBillingRegistryEventsMock.abi ../../../contracts/solc/v0.8.6/functions/v0_0_0/FunctionsBillingRegistryEventsMock.bin 50deeb883bd9c3729702be335c0388f9d8553bab4be5e26ecacac496a89e2b77
 functions_client: ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsClient.abi ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsClient.bin 2368f537a04489c720a46733f8596c4fc88a31062ecfa966d05f25dd98608aca
 functions_client_example: ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsClientExample.abi ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsClientExample.bin abf32e69f268f40e8530eb8d8e96bf310b798a4c0049a58022d9d2fb527b601b
-functions_coordinator: ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsCoordinator.abi ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsCoordinator.bin 292d4742d039a154ed7875a0167c9725e2a90674ad9a05f152377819bb991082
+functions_coordinator: ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsCoordinator.abi ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsCoordinator.bin 97a625c7ce8c8c167faad5e532a5894a52af5dee722b2da7e7528f5eaa32a0fe
 functions_load_test_client: ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsLoadTestClient.abi ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsLoadTestClient.bin c8dbbd5ebb34435800d6674700068837c3a252db60046a14b0e61e829db517de
 functions_oracle_events_mock: ../../../contracts/solc/v0.8.6/functions/v0_0_0/FunctionsOracleEventsMock.abi ../../../contracts/solc/v0.8.6/functions/v0_0_0/FunctionsOracleEventsMock.bin 3ca70f966f8fe751987f0ccb50bebb6aa5be77e4a9f835d1ae99e0e9bfb7d52c
 functions_router: ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsRouter.abi ../../../contracts/solc/v0.8.19/functions/v1_X/FunctionsRouter.bin 1f6d18f9e0846ad74b37a0a6acef5942ab73ace1e84307f201899f69e732e776