gas optimizations and nitpick fixes

contracts/gas-snapshots/ccip.gas-snapshot

@@ -34,16 +34,16 @@ BurnWithFromMintTokenPool_lockOrBurn:test_ChainNotAllowed_Revert() (gas: 28675)
 BurnWithFromMintTokenPool_lockOrBurn:test_PoolBurnRevertNotHealthy_Revert() (gas: 55158)
 BurnWithFromMintTokenPool_lockOrBurn:test_PoolBurn_Success() (gas: 243568)
 BurnWithFromMintTokenPool_lockOrBurn:test_Setup_Success() (gas: 24260)
-CCIPClientTest:test_HappyPath_Success() (gas: 192504)
+CCIPClientTest:test_HappyPath_Success() (gas: 192126)
 CCIPClientTest:test_ccipSend_withNonNativeFeetoken_andDestTokens_Success() (gas: 326927)
 CCIPClientTest:test_ccipSend_withNonNativeFeetoken_andNoDestTokens_Success() (gas: 219968)
 CCIPClientTest:test_ccipSend_with_NativeFeeToken_andDestTokens_Success() (gas: 376836)
 CCIPClientTest:test_modifyFeeToken_Success() (gas: 74452)
-CCIPClientWithACKTest:test_ccipReceiveAndSendAck_Success() (gas: 331795)
-CCIPClientWithACKTest:test_ccipReceiver_ack_with_invalidAckMessageHeaderBytes_Revert() (gas: 438714)
-CCIPClientWithACKTest:test_ccipSendAndReceiveAck_in_return_Success() (gas: 349366)
-CCIPClientWithACKTest:test_ccipSend_withNonNativeFeetoken_andNoDestTokens_Success() (gas: 242666)
-CCIPClientWithACKTest:test_send_tokens_that_are_not_feeToken_Success() (gas: 553089)
+CCIPClientWithACKTest:test_ccipReceiveAndSendAck_Success() (gas: 331807)
+CCIPClientWithACKTest:test_ccipReceiver_ack_with_invalidAckMessageHeaderBytes_Revert() (gas: 416038)
+CCIPClientWithACKTest:test_ccipSendAndReceiveAck_in_return_Success() (gas: 348558)
+CCIPClientWithACKTest:test_ccipSend_withNonNativeFeetoken_andNoDestTokens_Success() (gas: 242678)
+CCIPClientWithACKTest:test_send_tokens_that_are_not_feeToken_Success() (gas: 553108)
 CCIPConfigSetup:test_getCapabilityConfiguration_Success() (gas: 9495)
 CCIPConfig_ConfigStateMachine:test__computeConfigDigest_Success() (gas: 70755)
 CCIPConfig_ConfigStateMachine:test__computeNewConfigWithMeta_InitToRunning_Success() (gas: 363647)
@@ -100,21 +100,21 @@ CCIPConfig_validateConfig:test__validateConfig_TooManySigners_Reverts() (gas: 12
 CCIPConfig_validateConfig:test__validateConfig_TooManyTransmitters_Reverts() (gas: 1214264)
 CCIPConfig_validateConfig:test_getCapabilityConfiguration_Success() (gas: 9562)
 CCIPReceiverTest:test_HappyPath_Success() (gas: 193794)
-CCIPReceiverTest:test_Recovery_from_invalid_sender_Success() (gas: 430905)
-CCIPReceiverTest:test_Recovery_with_intentional_Revert() (gas: 445121)
-CCIPReceiverTest:test_disableChain_andRevert_onccipReceive_Revert() (gas: 199985)
-CCIPReceiverTest:test_modifyRouter_Success() (gas: 26446)
-CCIPReceiverTest:test_removeSender_from_approvedList_and_revert_Success() (gas: 427653)
-CCIPReceiverTest:test_retryFailedMessage_Success() (gas: 454419)
-CCIPReceiverTest:test_withdraw_nativeToken_to_owner_Success() (gas: 20667)
-CCIPReceiverWithAckTest:test_ccipReceive_ack_message_Success() (gas: 56278)
-CCIPReceiverWithAckTest:test_ccipReceive_and_respond_with_ack_Success() (gas: 331829)
-CCIPReceiverWithAckTest:test_ccipReceiver_ack_with_invalidAckMessageHeaderBytes_Revert() (gas: 438738)
-CCIPReceiverWithAckTest:test_feeTokenApproval_in_constructor_Success() (gas: 2956788)
-CCIPReceiverWithAckTest:test_modifyFeeToken_Success() (gas: 74867)
-CCIPSenderTest:test_ccipSend_withNonNativeFeetoken_andDestTokens_Success() (gas: 339615)
-CCIPSenderTest:test_ccipSend_withNonNativeFeetoken_andNoDestTokens_Success() (gas: 224489)
-CCIPSenderTest:test_ccipSend_with_NativeFeeToken_andDestTokens_Success() (gas: 368159)
+CCIPReceiverTest:test_Recovery_from_invalid_sender_Success() (gas: 408968)
+CCIPReceiverTest:test_Recovery_with_intentional_Revert() (gas: 414468)
+CCIPReceiverTest:test_disableChain_andRevert_onccipReceive_Revert() (gas: 200263)
+CCIPReceiverTest:test_modifyRouter_Success() (gas: 26449)
+CCIPReceiverTest:test_removeSender_from_approvedList_and_revert_Success() (gas: 405385)
+CCIPReceiverTest:test_retryFailedMessage_Success() (gas: 423644)
+CCIPReceiverTest:test_withdraw_nativeToken_to_owner_Success() (gas: 20854)
+CCIPReceiverWithAckTest:test_ccipReceive_ack_message_Success() (gas: 55404)
+CCIPReceiverWithAckTest:test_ccipReceive_and_respond_with_ack_Success() (gas: 331763)
+CCIPReceiverWithAckTest:test_ccipReceiver_ack_with_invalidAckMessageHeaderBytes_Revert() (gas: 416017)
+CCIPReceiverWithAckTest:test_feeTokenApproval_in_constructor_Success() (gas: 2879800)
+CCIPReceiverWithAckTest:test_modifyFeeToken_Success() (gas: 74845)
+CCIPSenderTest:test_ccipSend_withNonNativeFeetoken_andDestTokens_Success() (gas: 339353)
+CCIPSenderTest:test_ccipSend_withNonNativeFeetoken_andNoDestTokens_Success() (gas: 224534)
+CCIPSenderTest:test_ccipSend_with_NativeFeeToken_andDestTokens_Success() (gas: 367897)
 CommitStore_constructor:test_Constructor_Success() (gas: 3091326)
 CommitStore_isUnpausedAndRMNHealthy:test_RMN_Success() (gas: 73420)
 CommitStore_report:test_InvalidIntervalMinLargerThanMax_Revert() (gas: 28670)
@@ -219,7 +219,7 @@ EVM2EVMMultiOffRamp_executeSingleReport:test_NonExistingSourceChain_Revert() (ga
 EVM2EVMMultiOffRamp_executeSingleReport:test_ReceiverError_Success() (gas: 181628)
 EVM2EVMMultiOffRamp_executeSingleReport:test_RetryFailedMessageWithoutManualExecution_Revert() (gas: 190908)
 EVM2EVMMultiOffRamp_executeSingleReport:test_RootNotCommitted_Revert() (gas: 48050)
-EVM2EVMMultiOffRamp_executeSingleReport:test_RouterYULCall_Revert() (gas: 1697359)
+EVM2EVMMultiOffRamp_executeSingleReport:test_RouterYULCall_Revert() (gas: 1587918)
 EVM2EVMMultiOffRamp_executeSingleReport:test_SingleMessageNoTokensOtherChain_Success() (gas: 252012)
 EVM2EVMMultiOffRamp_executeSingleReport:test_SingleMessageNoTokensUnordered_Success() (gas: 174226)
 EVM2EVMMultiOffRamp_executeSingleReport:test_SingleMessageNoTokens_Success() (gas: 193899)
@@ -245,8 +245,8 @@ EVM2EVMMultiOffRamp_manuallyExecute:test_manuallyExecute_DoesNotRevertIfUntouche
 EVM2EVMMultiOffRamp_manuallyExecute:test_manuallyExecute_FailedTx_Revert() (gas: 207956)
 EVM2EVMMultiOffRamp_manuallyExecute:test_manuallyExecute_ForkedChain_Revert() (gas: 26001)
 EVM2EVMMultiOffRamp_manuallyExecute:test_manuallyExecute_GasLimitMismatchMultipleReports_Revert() (gas: 152913)
-EVM2EVMMultiOffRamp_manuallyExecute:test_manuallyExecute_LowGasLimit_Success() (gas: 1768860)
-EVM2EVMMultiOffRamp_manuallyExecute:test_manuallyExecute_ReentrancyFails() (gas: 3413460)
+EVM2EVMMultiOffRamp_manuallyExecute:test_manuallyExecute_LowGasLimit_Success() (gas: 1659419)
+EVM2EVMMultiOffRamp_manuallyExecute:test_manuallyExecute_ReentrancyFails() (gas: 3343871)
 EVM2EVMMultiOffRamp_manuallyExecute:test_manuallyExecute_Success() (gas: 210050)
 EVM2EVMMultiOffRamp_manuallyExecute:test_manuallyExecute_WithGasOverride_Success() (gas: 210613)
 EVM2EVMMultiOffRamp_manuallyExecute:test_manuallyExecute_WithMultiReportGasOverride_Success() (gas: 670625)
@@ -397,7 +397,7 @@ EVM2EVMOffRamp_execute:test_Paused_Revert() (gas: 101458)
 EVM2EVMOffRamp_execute:test_ReceiverError_Success() (gas: 165192)
 EVM2EVMOffRamp_execute:test_RetryFailedMessageWithoutManualExecution_Revert() (gas: 177948)
 EVM2EVMOffRamp_execute:test_RootNotCommitted_Revert() (gas: 41317)
-EVM2EVMOffRamp_execute:test_RouterYULCall_Revert() (gas: 1663435)
+EVM2EVMOffRamp_execute:test_RouterYULCall_Revert() (gas: 1553994)
 EVM2EVMOffRamp_execute:test_SingleMessageNoTokensUnordered_Success() (gas: 159863)
 EVM2EVMOffRamp_execute:test_SingleMessageNoTokens_Success() (gas: 175094)
 EVM2EVMOffRamp_execute:test_SingleMessageToNonCCIPReceiver_Success() (gas: 248764)
@@ -429,14 +429,14 @@ EVM2EVMOffRamp_execute_upgrade:test_V2_Success() (gas: 131906)
 EVM2EVMOffRamp_getAllRateLimitTokens:test_GetAllRateLimitTokens_Success() (gas: 38408)
 EVM2EVMOffRamp_getExecutionState:test_FillExecutionState_Success() (gas: 3213556)
 EVM2EVMOffRamp_getExecutionState:test_GetExecutionState_Success() (gas: 83091)
-EVM2EVMOffRamp_manuallyExecute:test_LowGasLimitManualExec_Success() (gas: 1744898)
+EVM2EVMOffRamp_manuallyExecute:test_LowGasLimitManualExec_Success() (gas: 1635457)
 EVM2EVMOffRamp_manuallyExecute:test_ManualExecFailedTx_Revert() (gas: 186809)
 EVM2EVMOffRamp_manuallyExecute:test_ManualExecForkedChain_Revert() (gas: 25894)
 EVM2EVMOffRamp_manuallyExecute:test_ManualExecGasLimitMismatch_Revert() (gas: 43519)
 EVM2EVMOffRamp_manuallyExecute:test_ManualExecInvalidGasLimit_Revert() (gas: 26009)
 EVM2EVMOffRamp_manuallyExecute:test_ManualExecWithGasOverride_Success() (gas: 189003)
 EVM2EVMOffRamp_manuallyExecute:test_ManualExec_Success() (gas: 188464)
-EVM2EVMOffRamp_manuallyExecute:test_ReentrancyManualExecuteFails() (gas: 3156679)
+EVM2EVMOffRamp_manuallyExecute:test_ReentrancyManualExecuteFails() (gas: 3087887)
 EVM2EVMOffRamp_manuallyExecute:test_manuallyExecute_DoesNotRevertIfUntouched_Success() (gas: 144106)
 EVM2EVMOffRamp_metadataHash:test_MetadataHash_Success() (gas: 8871)
 EVM2EVMOffRamp_setDynamicConfig:test_NonOwner_Revert() (gas: 40429)
@@ -703,10 +703,10 @@ OCR2Base_transmit:test_UnAuthorizedTransmitter_Revert() (gas: 23484)
 OCR2Base_transmit:test_UnauthorizedSigner_Revert() (gas: 39665)
 OCR2Base_transmit:test_WrongNumberOfSignatures_Revert() (gas: 20557)
 OnRampTokenPoolReentrancy:test_OnRampTokenPoolReentrancy_Success() (gas: 380711)
-PingPong_example_ccipReceive:test_CcipReceive_Success() (gas: 179093)
-PingPong_example_plumbing:test_Pausing_Success() (gas: 17917)
-PingPong_example_plumbing:test_typeAndVersion() (gas: 9786)
-PingPong_example_startPingPong:test_StartPingPong_Success() (gas: 203459)
+PingPong_example_ccipReceive:test_CcipReceive_Success() (gas: 179111)
+PingPong_example_plumbing:test_Pausing_Success() (gas: 17833)
+PingPong_example_plumbing:test_typeAndVersion() (gas: 9741)
+PingPong_example_startPingPong:test_StartPingPong_Success() (gas: 203453)
 PriceRegistry_applyFeeTokensUpdates:test_ApplyFeeTokensUpdates_Success() (gas: 79823)
 PriceRegistry_applyFeeTokensUpdates:test_OnlyCallableByOwner_Revert() (gas: 12580)
 PriceRegistry_constructor:test_InvalidStalenessThreshold_Revert() (gas: 67418)
@@ -853,8 +853,8 @@ Router_routeMessage:test_ManualExec_Success() (gas: 35381)
 Router_routeMessage:test_OnlyOffRamp_Revert() (gas: 25116)
 Router_routeMessage:test_WhenNotHealthy_Revert() (gas: 44724)
 Router_setWrappedNative:test_OnlyOwner_Revert() (gas: 10985)
-SelfFundedPingPong_ccipReceive:test_FundingIfNotANop_Revert() (gas: 291110)
-SelfFundedPingPong_ccipReceive:test_Funding_Success() (gas: 445731)
+SelfFundedPingPong_ccipReceive:test_FundingIfNotANop_Revert() (gas: 268794)
+SelfFundedPingPong_ccipReceive:test_Funding_Success() (gas: 445605)
 SelfFundedPingPong_setCountIncrBeforeFunding:test_setCountIncrBeforeFunding() (gas: 20273)
 TokenAdminRegistry_acceptAdminRole:test_acceptAdminRole_OnlyPendingAdministrator_Revert() (gas: 51085)
 TokenAdminRegistry_acceptAdminRole:test_acceptAdminRole_Success() (gas: 43947)

contracts/src/v0.8/ccip/applications/external/CCIPBase.sol

@@ -12,7 +12,7 @@ import {Address} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/ut
 /// @dev This contract is abstract, but does not have any functions which must be implemented by a child.
 abstract contract CCIPBase is OwnerIsCreator {
   using SafeERC20 for IERC20;
-  using Address for address;
+  using Address for address payable;
 
   error ZeroAddressNotAllowed();
   error InvalidRouter(address router);
@@ -24,8 +24,10 @@ abstract contract CCIPBase is OwnerIsCreator {
   event TokensWithdrawnByOwner(address indexed token, address indexed to, uint256 amount);
 
   // Parameters are indexed to simplify indexing of cross-chain dapps where contracts may be deployed with the same address.
-  // Since the updateApprovedSenders() function should be used sparingly by the contract owner, the additional gas cost should be negligible. If this function is needed to be used constantly, or with a large number of
-  // contracts, then an alternative and more gas-efficient method should be implemented instead, e.g. with merkle trees or indexing the parameters can be removed.
+  // Since the updateApprovedSenders() function should be used sparingly by the contract owner, the additional gas cost
+  // should be negligible. If this function is needed to be used constantly, or with a large number of
+  // contracts, then an alternative and more gas-efficient method should be implemented instead, e.g. with merkle trees
+  // or removing the indexed parameters
   event ApprovedSenderAdded(uint64 indexed destChainSelector, bytes indexed recipient);
   event ApprovedSenderRemoved(uint64 indexed destChainSelector, bytes indexed recipient);
 
@@ -108,33 +110,26 @@ abstract contract CCIPBase is OwnerIsCreator {
     return s_chainConfigs[sourceChainSelector].approvedSender[senderAddr];
   }
 
-  // ================================================================
+  // ===============================================================
   // │                  Fee Token Management                       │
   // ===============================================================
 
   /// @notice Accepts incoming native-tokens to support prefunding in native fee token.
   /// @dev All the example applications accept prefunding. This function should be removed if prefunding in native fee token is not required.
   receive() external payable {}
 
-  /// @notice Allow the owner to recover any native-tokens sent to this contract out of error, or to withdraw any native-tokens which were used for pre-funding if the fee-token is switched away from native-tokens.
-  /// @dev Function should not be used to recover tokens from failed messages, abandonFailedMessage() should be used instead
-  /// @param to A payable address to send the recovered tokens to
-  /// @param amount the amount of native tokens to recover, denominated in wei
-  function withdrawNativeToken(address payable to, uint256 amount) external onlyOwner {
-    Address.sendValue(to, amount);
-
-    // Use the same withdrawal event signature as withdrawTokens() but use address(0) to denote native-tokens.
-    emit TokensWithdrawnByOwner(address(0), to, amount);
-  }
-
   /// @notice Allow the owner to recover any ERC-20 tokens sent to this contract out of error or withdraw any fee-tokens which were sent as a source of fee-token pre-funding
   /// @dev This should NOT be used for recovering tokens from a failed message. Token recoveries can happen only if
   /// the failed message is guaranteed to not succeed upon retry, otherwise this can lead to double spend.
   /// For implementation of token recovery, see inheriting contracts.
   /// @param to A payable address to send the recovered tokens to
   /// @param amount the amount of native tokens to recover, denominated in wei  function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
   function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
-    IERC20(token).safeTransfer(to, amount);
+    if (token == address(0)) {
+      payable(to).sendValue(amount);
+    } else {
+      IERC20(token).safeTransfer(to, amount);
+    }
 
     emit TokensWithdrawnByOwner(token, to, amount);
   }
@@ -160,30 +155,31 @@ abstract contract CCIPBase is OwnerIsCreator {
   /// @notice Enable a remote-chain to send and receive messages to/from this contract via CCIP
   function applyChainUpdates(ChainUpdate[] calldata chains) external onlyOwner {
     for (uint256 i = 0; i < chains.length; ++i) {
-      if (!chains[i].allowed) {
-        delete s_chainConfigs[chains[i].chainSelector].recipient;
-        emit ChainRemoved(chains[i].chainSelector);
+      ChainUpdate memory chain = chains[i];
+
+      if (!chain.allowed) {
+        delete s_chainConfigs[chain.chainSelector].recipient;
+        emit ChainRemoved(chain.chainSelector);
       } else {
         // The existence of a stored recipient is used to denote a chain being enabled, so the length here cannot be zero
-        if (chains[i].recipient.length == 0) revert ZeroAddressNotAllowed();
+        if (chain.recipient.length == 0) revert ZeroAddressNotAllowed();
 
-        ChainConfig storage currentConfig = s_chainConfigs[chains[i].chainSelector];
+        ChainConfig storage currentConfig = s_chainConfigs[chain.chainSelector];
 
-        currentConfig.recipient = chains[i].recipient;
+        currentConfig.recipient = chain.recipient;
 
         // Set any additional args such as enabling out-of-order execution or manual gas-limit
-        if (chains[i].extraArgsBytes.length != 0) currentConfig.extraArgsBytes = chains[i].extraArgsBytes;
+        currentConfig.extraArgsBytes = chain.extraArgsBytes;
 
-        emit ChainAdded(chains[i].chainSelector, chains[i].recipient, chains[i].extraArgsBytes);
+        emit ChainAdded(chain.chainSelector, chain.recipient, chain.extraArgsBytes);
       }
     }
   }
 
   /// @notice Reverts if the specified chainSelector is not approved to send/receive messages to/from this contract
   /// @param chainSelector the CCIP specific chain selector for a given remote-chain.
   modifier isValidChain(uint64 chainSelector) {
-    // Must be storage and not memory because the struct contains a nested mapping which is not capable of being copied to memory
-    ChainConfig storage currentConfig = s_chainConfigs[chainSelector];
+    ChainConfig storage currentConfig = s_chainConfigs[chainSelector]; // Must be storage because the nested mapping cannot be copied to memory
     if (currentConfig.recipient.length == 0) revert InvalidChain(chainSelector);
     _;
   }

contracts/src/v0.8/ccip/applications/external/CCIPClient.sol

@@ -20,6 +20,7 @@ contract CCIPClient is CCIPReceiver {
   event MessageSent(bytes32 messageId);
   event FeeTokenUpdated(address oldFeeToken, address newFeeToken);
 
+  /// @dev A check for the zero-address is not explicitly performed since it is included in the CCIPBase parent constructor
   constructor(address router, IERC20 feeToken) CCIPReceiver(router) {
     s_feeToken = feeToken;
 
@@ -47,15 +48,16 @@ contract CCIPClient is CCIPReceiver {
 
     uint256 fee = IRouterClient(s_ccipRouter).getFee(destChainSelector, message);
 
-    // Additional tokens for fees do not need to be approved to the router since it is already handled by setting s_feeToken
-    // Fee transfers need first, that way balanceOf(address(this)) does not conflict with any tokens sent in tokenAmounts
-    // to support fee-token pre-funding
+    // To support pre-funding, if the contract already posesses enough tokens to pay the fee, then a transferFrom is
+    // not necesarry. This branch must be performed before transfering tokens in tokenAmounts[], since in the case
+    // where tokenAmounts[] contains the fee token, then balanceOf may double count tokens improperly and cause
+    // unexpected behavior.
     if ((address(s_feeToken) != address(0)) && (s_feeToken.balanceOf(address(this)) < fee)) {
       IERC20(s_feeToken).safeTransferFrom(msg.sender, address(this), fee);
     }
 
     for (uint256 i = 0; i < tokenAmounts.length; ++i) {
-      // Transfer the tokens to pay for tokens in tokenAmounts
+      // Transfer the tokens specified in TokenAmounts[] so that it can be forwarded to the router
       IERC20(tokenAmounts[i].token).safeTransferFrom(msg.sender, address(this), tokenAmounts[i].amount);
 
       // Do not approve the tokens if it is the feeToken, otherwise the approval amount may overflow
@@ -75,17 +77,6 @@ contract CCIPClient is CCIPReceiver {
     return messageId;
   }
 
-  /// @notice Contains arbitrary application-logic for incoming CCIP messages.
-  /// @dev It has to be external because of the try/catch of ccipReceive() which invokes it
-  function processMessage(Client.Any2EVMMessage calldata message)
-    external
-    virtual
-    override
-    onlySelf
-    isValidSender(message.sourceChainSelector, message.sender)
-    isValidChain(message.sourceChainSelector)
-  {}
-
   function updateFeeToken(address token) external onlyOwner {
     // If the current fee token is not-native, zero out the allowance to the router for safety
     if (address(s_feeToken) != address(0)) {