Miscellaneous fixes and cleanups (#1421)

## Motivation

The goal of this PR is to fix several miscellaneous items.

## Solution

- Replaces some magic numbers with constants
- Default to `private` visibility for state variables that are not used
in test helpers
- Standardizes some natspec tags (consistent use of `@dev` instead of
`@notice` for state variables)
- Add a `ConfigSet` event in `CCIPConfig`
- Asserts full equality of `OCRConfigWithMeta[]` in `CCIPConfigTests`
- Add clarifying comments in `NonceManager` `applyPreviousRampsUpdates`

contracts/src/v0.8/ccip/FeeQuoter.sol

@@ -58,7 +58,7 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
   event DestChainConfigUpdated(uint64 indexed destChainSelector, DestChainConfig destChainConfig);
   event DestChainAdded(uint64 indexed destChainSelector, DestChainConfig destChainConfig);
 
-  /// @notice Token price data feed update
+  /// @dev Token price data feed update
   struct TokenPriceFeedUpdate {
     address sourceToken; // Source token to update feed for
     IFeeQuoter.TokenPriceFeedConfig feedConfig; // Feed config update data
@@ -73,7 +73,7 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
     uint32 stalenessThreshold; // The amount of time a gas price can be stale before it is considered invalid.
   }
 
-  /// @notice The struct representing the received CCIP feed report from keystone IReceiver.onReport()
+  /// @dev The struct representing the received CCIP feed report from keystone IReceiver.onReport()
   struct ReceivedCCIPFeedReport {
     address token; // Token address
     uint224 price; // ─────────╮ Price of the token in USD with 18 decimals
@@ -151,6 +151,11 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
     uint64 premiumMultiplierWeiPerEth; // ──╯ Multiplier for destination chain specific premiums.
   }
 
+  /// @dev The base decimals for cost calculations
+  uint256 public constant FEE_BASE_DECIMALS = 36;
+  /// @dev The decimals that Keystone reports prices in
+  uint256 public constant KEYSTONE_PRICE_DECIMALS = 18;
+
   string public constant override typeAndVersion = "FeeQuoter 1.6.0-dev";
 
   /// @dev The gas price per unit of gas for a given destination chain, in USD with 18 decimals.
@@ -175,13 +180,13 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
   mapping(address token => IFeeQuoter.TokenPriceFeedConfig dataFeedAddress) private s_usdPriceFeedsPerToken;
 
   /// @dev The multiplier for destination chain specific premiums that can be set by the owner or fee admin
-  mapping(address token => uint64 premiumMultiplierWeiPerEth) internal s_premiumMultiplierWeiPerEth;
+  mapping(address token => uint64 premiumMultiplierWeiPerEth) private s_premiumMultiplierWeiPerEth;
 
   /// @dev The destination chain specific fee configs
   mapping(uint64 destChainSelector => DestChainConfig destChainConfig) internal s_destChainConfigs;
 
   /// @dev The token transfer fee config that can be set by the owner or fee admin
-  mapping(uint64 destChainSelector => mapping(address token => TokenTransferFeeConfig tranferFeeConfig)) internal
+  mapping(uint64 destChainSelector => mapping(address token => TokenTransferFeeConfig tranferFeeConfig)) private
     s_tokenTransferFeeConfig;
 
   /// @dev Maximum fee that can be charged for a message. This is a guard to prevent massively overcharging due to misconfiguation.
@@ -530,7 +535,7 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
     _applyPremiumMultiplierWeiPerEthUpdates(premiumMultiplierWeiPerEthArgs);
   }
 
-  /// @dev Set the fee config.
+  /// @dev Sets the fee config.
   /// @param premiumMultiplierWeiPerEthArgs The multiplier for destination chain specific premiums.
   function _applyPremiumMultiplierWeiPerEthUpdates(
     PremiumMultiplierWeiPerEthArgs[] memory premiumMultiplierWeiPerEthArgs
@@ -646,10 +651,10 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
     uint8 excessDecimals = dataFeedDecimal + tokenDecimal;
     uint256 rebasedVal;
 
-    if (excessDecimals > 36) {
-      rebasedVal = feedValue / (10 ** (excessDecimals - 36));
+    if (excessDecimals > FEE_BASE_DECIMALS) {
+      rebasedVal = feedValue / (10 ** (excessDecimals - FEE_BASE_DECIMALS));
     } else {
-      rebasedVal = feedValue * (10 ** (36 - excessDecimals));
+      rebasedVal = feedValue * (10 ** (FEE_BASE_DECIMALS - excessDecimals));
     }
 
     if (rebasedVal > type(uint224).max) {

contracts/src/v0.8/ccip/MultiAggregateRateLimiter.sol

@@ -59,14 +59,14 @@ contract MultiAggregateRateLimiter is IMessageInterceptor, AuthorizedCallers, IT
 
   /// @dev Tokens that should be included in Aggregate Rate Limiting (from local chain (this chain) -> remote),
   /// grouped per-remote chain.
-  mapping(uint64 remoteChainSelector => EnumerableMapAddresses.AddressToBytesMap tokensLocalToRemote) internal
+  mapping(uint64 remoteChainSelector => EnumerableMapAddresses.AddressToBytesMap tokensLocalToRemote) private
     s_rateLimitedTokensLocalToRemote;
 
   /// @notice The address of the FeeQuoter used to query token values for ratelimiting.
   address internal s_feeQuoter;
 
   /// @notice Rate limiter token bucket states per chain, with separate buckets for inbound and outbound lanes.
-  mapping(uint64 remoteChainSelector => RateLimiterBuckets buckets) internal s_rateLimitersByChainSelector;
+  mapping(uint64 remoteChainSelector => RateLimiterBuckets buckets) private s_rateLimitersByChainSelector;
 
   /// @param feeQuoter the fee quoter to set.
   /// @param authorizedCallers the authorized callers to set.

contracts/src/v0.8/ccip/NonceManager.sol

@@ -129,7 +129,9 @@ contract NonceManager is INonceManager, AuthorizedCallers, ITypeAndVersion {
 
       PreviousRamps storage prevRamps = s_previousRamps[previousRampsArg.remoteChainSelector];
 
-      // If the previous ramps are already set then they should not be updated
+      // If the previous ramps are already set then they should not be updated.
+      // In versions prior to the introduction of the NonceManager contract, nonces were tracked in the on/off ramps.
+      // This config does a 1-time migration to move the nonce from on/off ramps into NonceManager
       if (prevRamps.prevOnRamp != address(0) || prevRamps.prevOffRamp != address(0)) {
         revert PreviousRampAlreadySet();
       }

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

@@ -51,10 +51,12 @@ contract CCIPConfig is ITypeAndVersion, ICapabilityConfiguration, OwnerIsCreator
   error WrongConfigDigestBlueGreen(bytes32 got, bytes32 expected);
   error ZeroAddressNotAllowed();
 
-  /// @notice Type and version override.
+  event ConfigSet(uint32 indexed donId, uint8 indexed pluginType, CCIPConfigTypes.OCR3ConfigWithMeta[] config);
+
+  /// @dev Type and version override.
   string public constant override typeAndVersion = "CCIPConfig 1.6.0-dev";
 
-  /// @notice The canonical capabilities registry address.
+  /// @dev The canonical capabilities registry address.
   address internal immutable i_capabilitiesRegistry;
 
   uint8 internal constant MAX_OCR3_CONFIGS_PER_PLUGIN = 2;
@@ -66,18 +68,18 @@ contract CCIPConfig is ITypeAndVersion, ICapabilityConfiguration, OwnerIsCreator
   /// @dev 256 is the hard limit due to the bit encoding of their indexes into a uint256.
   uint256 internal constant MAX_NUM_ORACLES = 256;
 
-  /// @notice chain configuration for each chain that CCIP is deployed on.
-  mapping(uint64 chainSelector => CCIPConfigTypes.ChainConfig chainConfig) internal s_chainConfigurations;
+  /// @dev chain configuration for each chain that CCIP is deployed on.
+  mapping(uint64 chainSelector => CCIPConfigTypes.ChainConfig chainConfig) private s_chainConfigurations;
 
-  /// @notice All chains that are configured.
-  EnumerableSet.UintSet internal s_remoteChainSelectors;
+  /// @dev All chains that are configured.
+  EnumerableSet.UintSet private s_remoteChainSelectors;
 
-  /// @notice OCR3 configurations for each DON.
+  /// @dev OCR3 configurations for each DON.
   /// Each CR DON will have a commit and execution configuration.
   /// This means that a DON can have up to 4 configurations, since we are implementing blue/green deployments.
   mapping(
     uint32 donId => mapping(Internal.OCRPluginType pluginType => CCIPConfigTypes.OCR3ConfigWithMeta[] ocr3Configs)
-  ) internal s_ocr3Configs;
+  ) private s_ocr3Configs;
 
   /// @param capabilitiesRegistry the canonical capabilities registry address.
   constructor(address capabilitiesRegistry) {
@@ -213,6 +215,8 @@ contract CCIPConfig is ITypeAndVersion, ICapabilityConfiguration, OwnerIsCreator
     for (uint256 i = 0; i < newConfigWithMeta.length; ++i) {
       s_ocr3Configs[donId][pluginType].push(newConfigWithMeta[i]);
     }
+
+    emit ConfigSet(donId, uint8(pluginType), newConfigWithMeta);
   }
 
   // ================================================================

contracts/src/v0.8/ccip/interfaces/IFeeQuoter.sol

@@ -6,7 +6,7 @@ import {Internal} from "../libraries/Internal.sol";
 import {IPriceRegistry} from "./IPriceRegistry.sol";
 
 interface IFeeQuoter is IPriceRegistry {
-  /// @notice Token price data feed configuration
+  /// @dev Token price data feed configuration
   struct TokenPriceFeedConfig {
     address dataFeedAddress; // ──╮ AggregatorV3Interface contract (0 - feed is unset)
     uint8 tokenDecimals; // ──────╯ Decimals of the token that the feed represents

contracts/src/v0.8/ccip/offRamp/OffRamp.sol

@@ -83,7 +83,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
   event RootRemoved(bytes32 root);
   event SkippedReportExecution(uint64 sourceChainSelector);
 
-  /// @notice Struct that contains the static configuration
+  /// @dev Struct that contains the static configuration
   /// @dev RMN depends on this struct, if changing, please notify the RMN maintainers.
   /// @dev not sure why solhint complains about this, seems like a buggy detector
   /// https://github.com/protofire/solhint/issues/597
@@ -95,15 +95,15 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
     address nonceManager; // Nonce manager address
   }
 
-  /// @notice Per-chain source config (defining a lane from a Source Chain -> Dest OffRamp)
+  /// @dev Per-chain source config (defining a lane from a Source Chain -> Dest OffRamp)
   struct SourceChainConfig {
     IRouter router; // ──────────╮  Local router to use for messages coming from this source chain
     bool isEnabled; //           |  Flag whether the source chain is enabled or not
     uint64 minSeqNr; // ─────────╯  The min sequence number expected for future messages
     bytes onRamp; // OnRamp address on the source chain
   }
 
-  /// @notice Same as SourceChainConfig but with source chain selector so that an array of these
+  /// @dev Same as SourceChainConfig but with source chain selector so that an array of these
   /// can be passed in the constructor and the applySourceChainConfigUpdates function.
   struct SourceChainConfigArgs {
     IRouter router; // ────────────────╮  Local router to use for messages coming from this source chain
@@ -112,7 +112,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
     bytes onRamp; // OnRamp address on the source chain
   }
 
-  /// @notice Dynamic offRamp config
+  /// @dev Dynamic offRamp config
   /// @dev Since DynamicConfig is part of DynamicConfigSet event, if changing it, we should update the ABI on Atlas
   struct DynamicConfig {
     address feeQuoter; // ──────────────────────────────╮ FeeQuoter address on the local chain
@@ -122,7 +122,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
     address messageValidator; // Optional message validator to validate incoming messages (zero address = no validator)
   }
 
-  /// @notice Report that is committed by the observing DON at the committing phase
+  /// @dev Report that is committed by the observing DON at the committing phase
   /// @dev RMN depends on this struct, if changing, please notify the RMN maintainers.
   struct CommitReport {
     Internal.PriceUpdates priceUpdates; // Collection of gas and price updates to commit
@@ -146,7 +146,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
 
   /// @notice SourceChainConfig per chain
   /// (forms lane configurations from sourceChainSelector => StaticConfig.chainSelector)
-  mapping(uint64 sourceChainSelector => SourceChainConfig sourceChainConfig) internal s_sourceChainConfigs;
+  mapping(uint64 sourceChainSelector => SourceChainConfig sourceChainConfig) private s_sourceChainConfigs;
 
   // STATE
   /// @dev A mapping of sequence numbers (per source chain) to execution state using a bitmap with each execution

contracts/src/v0.8/ccip/onRamp/OnRamp.sol

@@ -108,20 +108,20 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator {
   // STATIC CONFIG
   string public constant override typeAndVersion = "OnRamp 1.6.0-dev";
   /// @dev The chain ID of the source chain that this contract is deployed to
-  uint64 internal immutable i_chainSelector;
+  uint64 private immutable i_chainSelector;
   /// @dev The rmn contract
-  IRMNV2 internal immutable i_rmn;
+  IRMNV2 private immutable i_rmn;
   /// @dev The address of the nonce manager
-  address internal immutable i_nonceManager;
+  address private immutable i_nonceManager;
   /// @dev The address of the token admin registry
-  address internal immutable i_tokenAdminRegistry;
+  address private immutable i_tokenAdminRegistry;
 
   // DYNAMIC CONFIG
   /// @dev The dynamic config for the onRamp
-  DynamicConfig internal s_dynamicConfig;
+  DynamicConfig private s_dynamicConfig;
 
   /// @dev The destination chain specific configs
-  mapping(uint64 destChainSelector => DestChainConfig destChainConfig) internal s_destChainConfigs;
+  mapping(uint64 destChainSelector => DestChainConfig destChainConfig) private s_destChainConfigs;
 
   constructor(
     StaticConfig memory staticConfig,