fix: CCIP-3020 natspec and snapshot corrections for Onramp contract

smartcontractkit · Aug 23, 2024 · 64c4f0f · 64c4f0f
1 parent fcfdb46
commit 64c4f0f
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 19 deletions.
diff --git a/contracts/gas-snapshots/ccip.gas-snapshot b/contracts/gas-snapshots/ccip.gas-snapshot
@@ -621,26 +621,26 @@ OffRamp_verify:test_NotBlessedWrongChainSelector_Success() (gas: 178688)
 OffRamp_verify:test_NotBlessed_Success() (gas: 141549)
 OffRamp_verify:test_TooManyLeaves_Revert() (gas: 51510)
 OnRampTokenPoolReentrancy:test_OnRampTokenPoolReentrancy_Success() (gas: 390322)
-OnRamp_allowListConfigUpdates:test_applyAllowList_Success() (gas: 324961)
-OnRamp_allowListConfigUpdates:test_applyAllowList_Revert() (gas: 66547)
+OnRamp_allowListConfigUpdates:test_applyAllowList_Success() (gas: 324969)
+OnRamp_allowListConfigUpdates:test_applyAllowList_Revert() (gas: 66557)
 OnRamp_allowListConfigUpdates:test_setAllowListAdmin_ByNonOwner_Revert() (gas: 11833)
 OnRamp_allowListConfigUpdates:test_setAllowListAdmin_Success() (gas: 36159)
 OnRamp_applyDestChainConfigUpdates:test_ApplyDestChainConfigUpdates_WithInvalidChainSelector_Revert() (gas: 13242)
-OnRamp_applyDestChainConfigUpdates:test_ApplyDestChainConfigUpdates_Success() (gas: 64558)
+OnRamp_applyDestChainConfigUpdates:test_ApplyDestChainConfigUpdates_Success() (gas: 64573)
 OnRamp_applyDestChainConfigUpdates:test_ApplyDestChainConfigUpdates_WithInalidChainSelector_Revert() (gas: 13308)
-OnRamp_constructor:test_Constructor_InvalidConfigChainSelectorEqZero_Revert() (gas: 94806)
-OnRamp_constructor:test_Constructor_InvalidConfigNonceManagerEqAddressZero_Revert() (gas: 92740)
-OnRamp_constructor:test_Constructor_InvalidConfigRMNProxyEqAddressZero_Revert() (gas: 97755)
-OnRamp_constructor:test_Constructor_InvalidConfigTokenAdminRegistryEqAddressZero_Revert() (gas: 92798)
-OnRamp_constructor:test_Constructor_Success() (gas: 2807415)
+OnRamp_constructor:test_Constructor_InvalidConfigChainSelectorEqZero_Revert() (gas: 94814)
+OnRamp_constructor:test_Constructor_InvalidConfigNonceManagerEqAddressZero_Revert() (gas: 92748)
+OnRamp_constructor:test_Constructor_InvalidConfigRMNProxyEqAddressZero_Revert() (gas: 97763)
+OnRamp_constructor:test_Constructor_InvalidConfigTokenAdminRegistryEqAddressZero_Revert() (gas: 92806)
+OnRamp_constructor:test_Constructor_Success() (gas: 2808028)
 OnRamp_forwardFromRouter:test_CannotSendZeroTokens_Revert() (gas: 72097)
 OnRamp_forwardFromRouter:test_ForwardFromRouterExtraArgsV2AllowOutOfOrderTrue_Success() (gas: 114462)
 OnRamp_forwardFromRouter:test_ForwardFromRouterExtraArgsV2_Success() (gas: 145231)
 OnRamp_forwardFromRouter:test_ForwardFromRouterSuccessCustomExtraArgs() (gas: 144827)
 OnRamp_forwardFromRouter:test_ForwardFromRouterSuccessEmptyExtraArgs() (gas: 143028)
 OnRamp_forwardFromRouter:test_ForwardFromRouterSuccessLegacyExtraArgs() (gas: 145059)
 OnRamp_forwardFromRouter:test_ForwardFromRouter_Success() (gas: 144428)
-OnRamp_forwardFromRouter:test_ForwardFromRouter_Success_ConfigurableSourceRouter() (gas: 139523)
+OnRamp_forwardFromRouter:test_ForwardFromRouter_Success_ConfigurableSourceRouter() (gas: 139528)
 OnRamp_forwardFromRouter:test_InvalidExtraArgsTag_Revert() (gas: 28777)
 OnRamp_forwardFromRouter:test_MessageValidationError_Revert() (gas: 138683)
 OnRamp_forwardFromRouter:test_MesssageFeeTooHigh_Revert() (gas: 26866)

diff --git a/contracts/src/v0.8/ccip/onRamp/OnRamp.sol b/contracts/src/v0.8/ccip/onRamp/OnRamp.sol
@@ -73,10 +73,12 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator {
   }
 
   /// @dev Struct to hold the configs for a destination chain
+  /// @dev sequenceNumber, allowListEnabled, router will all be packed in 1 slot
   struct DestChainConfig {
     // The last used sequence number. This is zero in the case where no messages have yet been sent.
     // 0 is not a valid sequence number for any real transaction.
     uint64 sequenceNumber;
+    // boolean indicator to specify if allowList check is enabled
     bool allowListEnabled;
     // This is the local router address that is allowed to send messages to the destination chain.
     // This is NOT the receiving router address on the destination chain.
@@ -87,6 +89,8 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator {
 
   /// @dev Struct used as a return type for config query on Destination chain
   /// @dev sequenceNumber, allowListEnabled, router will all be packed in 1 slot
+  /// @dev DestChainConfig stores allowedList of senders as enumeratedSet. enumeratedSet cant be returned as-is,
+  /// @dev DestChainConfigInfo is used as a struct to return DestinationConfig info
   struct DestChainConfigInfo {
     // The last used sequence number. This is zero in the case where no messages has been sent yet.
     // 0 is not a valid sequence number for any real transaction.
@@ -373,21 +377,21 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator {
 
       s_destChainConfigs[destChainSelector].router = destChainConfigArg.router;
 
+      DestChainConfig storage destChainConfig = s_destChainConfigs[destChainSelector];
+
       emit DestChainConfigSet(
-        destChainSelector,
-        s_destChainConfigs[destChainSelector].sequenceNumber,
-        s_destChainConfigs[destChainSelector].router,
-        s_destChainConfigs[destChainSelector].allowListEnabled
+        destChainSelector, destChainConfig.sequenceNumber, destChainConfigArg.router, destChainConfig.allowListEnabled
       );
     }
   }
 
-  // ================================================================
-  // │                          Allowlist                           │
-  // ================================================================
-
-  function getDestChainConfig(uint64 destinationChainSelector) public view returns (DestChainConfigInfo memory) {
-    DestChainConfig storage config = s_destChainConfigs[destinationChainSelector];
+  /// @notice get ChainConfig configured for the DestinationChainSelector
+  /// @param destChainSelector The destination chain selector
+  /// @dev DestChainConfig stores allowedList of senders as enumeratedSet.
+  /// @dev As enumeratedSet cant be returned as-is, an array of allowedSenders returned in DestChainConfigInfo struct
+  /// @return DestChainConfigInfo returns DestinationChainConfig with array of allowedList of Senders
+  function getDestChainConfig(uint64 destChainSelector) public view returns (DestChainConfigInfo memory) {
+    DestChainConfig storage config = s_destChainConfigs[destChainSelector];
     return DestChainConfigInfo({
       sequenceNumber: config.sequenceNumber,
       allowListEnabled: config.allowListEnabled,
@@ -396,6 +400,13 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator {
     });
   }
 
+  // ================================================================
+  // │                          Allowlist                           │
+  // ================================================================
+
+  /// @notice Updates allowListConfig for Senders
+  /// @dev configuration used to set the list of senders who are authorized to send messages
+  /// @param allowListConfigArgsItems Array of AllowListConfigArguments where each item is for a destChainSelector
   function applyAllowListUpdates(AllowListConfigArgs[] calldata allowListConfigArgsItems) external {
     if (msg.sender != owner()) {
       if (msg.sender != s_dynamicConfig.allowListAdmin) {