From 231e2cf63e13c2fdc78ad6d61ee55a70b3a37ec6 Mon Sep 17 00:00:00 2001 From: Evaldas Latoskinas Date: Thu, 11 Jul 2024 17:48:22 +0200 Subject: [PATCH] refactor: move staleness threshold to StaticConfig --- contracts/src/v0.8/ccip/PriceRegistry.sol | 23 ++-- .../ccip/test/helpers/PriceRegistryHelper.sol | 2 - .../ccip/test/onRamp/EVM2EVMMultiOnRamp.t.sol | 102 +++++++----------- .../test/priceRegistry/PriceRegistry.t.sol | 32 ++++-- 4 files changed, 77 insertions(+), 82 deletions(-) diff --git a/contracts/src/v0.8/ccip/PriceRegistry.sol b/contracts/src/v0.8/ccip/PriceRegistry.sol index dfd553951c..8ace43d13e 100644 --- a/contracts/src/v0.8/ccip/PriceRegistry.sol +++ b/contracts/src/v0.8/ccip/PriceRegistry.sol @@ -32,12 +32,12 @@ contract PriceRegistry is AuthorizedCallers, IPriceRegistry, ITypeAndVersion { struct StaticConfig { uint96 maxFeeJuelsPerMsg; // ─╮ Maximum fee that can be charged for a message address linkToken; // ────────╯ LINK token address + uint32 stalenessThreshold; // The amount of time a gas price can be stale before it is considered invalid. } error TokenNotSupported(address token); error ChainNotSupported(uint64 chain); error StaleGasPrice(uint64 destChainSelector, uint256 threshold, uint256 timePassed); - error InvalidStalenessThreshold(); error DataFeedValueOutOfUint224Range(); error NotAFeeToken(address token); error InvalidDestBytesOverhead(address token, uint32 destBytesOverhead); @@ -190,23 +190,24 @@ contract PriceRegistry is AuthorizedCallers, IPriceRegistry, ITypeAndVersion { StaticConfig memory staticConfig, address[] memory priceUpdaters, address[] memory feeTokens, - uint32 stalenessThreshold, TokenPriceFeedUpdate[] memory tokenPriceFeeds, TokenTransferFeeConfigArgs[] memory tokenTransferFeeConfigArgs, PremiumMultiplierWeiPerEthArgs[] memory premiumMultiplierWeiPerEthArgs, DestChainDynamicConfigArgs[] memory destChainConfigArgs ) AuthorizedCallers(priceUpdaters) { - if (staticConfig.linkToken == address(0) || staticConfig.maxFeeJuelsPerMsg == 0) { + if ( + staticConfig.linkToken == address(0) || staticConfig.maxFeeJuelsPerMsg == 0 + || staticConfig.stalenessThreshold == 0 + ) { revert InvalidStaticConfig(); } i_linkToken = staticConfig.linkToken; i_maxFeeJuelsPerMsg = staticConfig.maxFeeJuelsPerMsg; - i_stalenessThreshold = stalenessThreshold; + i_stalenessThreshold = staticConfig.stalenessThreshold; _applyFeeTokensUpdates(feeTokens, new address[](0)); _updateTokenPriceFeeds(tokenPriceFeeds); - if (stalenessThreshold == 0) revert InvalidStalenessThreshold(); _applyDestChainConfigUpdates(destChainConfigArgs); _applyPremiumMultiplierWeiPerEthUpdates(premiumMultiplierWeiPerEthArgs); _applyTokenTransferFeeConfigUpdates(tokenTransferFeeConfigArgs, new TokenTransferFeeConfigRemoveArgs[](0)); @@ -256,12 +257,6 @@ contract PriceRegistry is AuthorizedCallers, IPriceRegistry, ITypeAndVersion { return s_usdPriceFeedsPerToken[token]; } - /// @notice Get the staleness threshold. - /// @return stalenessThreshold The staleness threshold. - function getStalenessThreshold() external view returns (uint128) { - return i_stalenessThreshold; - } - /// @inheritdoc IPriceRegistry function getDestinationChainGasPrice(uint64 destChainSelector) external @@ -873,7 +868,11 @@ contract PriceRegistry is AuthorizedCallers, IPriceRegistry, ITypeAndVersion { /// @dev RMN depends on this function, if changing, please notify the RMN maintainers. /// @return the configuration. function getStaticConfig() external view returns (StaticConfig memory) { - return StaticConfig({maxFeeJuelsPerMsg: i_maxFeeJuelsPerMsg, linkToken: i_linkToken}); + return StaticConfig({ + maxFeeJuelsPerMsg: i_maxFeeJuelsPerMsg, + linkToken: i_linkToken, + stalenessThreshold: i_stalenessThreshold + }); } // ================================================================ diff --git a/contracts/src/v0.8/ccip/test/helpers/PriceRegistryHelper.sol b/contracts/src/v0.8/ccip/test/helpers/PriceRegistryHelper.sol index 7303d3f5e5..6a8d739097 100644 --- a/contracts/src/v0.8/ccip/test/helpers/PriceRegistryHelper.sol +++ b/contracts/src/v0.8/ccip/test/helpers/PriceRegistryHelper.sol @@ -9,7 +9,6 @@ contract PriceRegistryHelper is PriceRegistry { StaticConfig memory staticConfig, address[] memory priceUpdaters, address[] memory feeTokens, - uint32 stalenessThreshold, TokenPriceFeedUpdate[] memory tokenPriceFeeds, TokenTransferFeeConfigArgs[] memory tokenTransferFeeConfigArgs, PremiumMultiplierWeiPerEthArgs[] memory premiumMultiplierWeiPerEthArgs, @@ -19,7 +18,6 @@ contract PriceRegistryHelper is PriceRegistry { staticConfig, priceUpdaters, feeTokens, - stalenessThreshold, tokenPriceFeeds, tokenTransferFeeConfigArgs, premiumMultiplierWeiPerEthArgs, diff --git a/contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRamp.t.sol b/contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRamp.t.sol index 8c2b5e8b2f..5324eabaef 100644 --- a/contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRamp.t.sol +++ b/contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRamp.t.sol @@ -592,67 +592,47 @@ contract EVM2EVMMultiOnRamp_getFee is EVM2EVMMultiOnRampSetup { s_onRamp.getFee(DEST_CHAIN_SELECTOR, _generateEmptyMessage()); } - // TODO: re-implement tests - // function test_Fuzz_EnforceOutOfOrder(bool enforce, bool allowOutOfOrderExecution) public { - // // Update dynamic config to enforce allowOutOfOrderExecution = defaultVal. - // vm.stopPrank(); - // vm.startPrank(OWNER); - - // PriceRegistry.DestChainDynamicConfigArgs[] memory destChainConfigArgs = - // _generatePriceRegistryDestChainDynamicConfigArgs(); - // destChainConfigArgs[0].dynamicConfig.enforceOutOfOrder = enforce; - // s_priceRegistry.applyDestChainConfigUpdates(destChainConfigArgs); - - // vm.stopPrank(); - - // vm.startPrank(address(s_sourceRouter)); - // Client.EVM2AnyMessage memory message = _generateEmptyMessage(); - // message.extraArgs = abi.encodeWithSelector( - // Client.EVM_EXTRA_ARGS_V2_TAG, - // Client.EVMExtraArgsV2({gasLimit: GAS_LIMIT * 2, allowOutOfOrderExecution: allowOutOfOrderExecution}) - // ); - // uint256 feeAmount = 1234567890; - // IERC20(s_sourceFeeToken).transferFrom(OWNER, address(s_onRamp), feeAmount); - - // if (enforce) { - // // If enforcement is on, only true should be allowed. - // if (allowOutOfOrderExecution) { - // vm.expectEmit(); - // emit EVM2EVMMultiOnRamp.CCIPSendRequested(DEST_CHAIN_SELECTOR, _messageToEvent(message, 1, 1, feeAmount, OWNER)); - // s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, feeAmount, OWNER); - // } else { - // vm.expectRevert(PriceRegistry.ExtraArgOutOfOrderExecutionMustBeTrue.selector); - // s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, feeAmount, OWNER); - // } - // } else { - // // no enforcement should allow any value. - // vm.expectEmit(); - // emit EVM2EVMMultiOnRamp.CCIPSendRequested(DEST_CHAIN_SELECTOR, _messageToEvent(message, 1, 1, feeAmount, OWNER)); - // s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, feeAmount, OWNER); - // } - // } - - // function test_EnforceOutOfOrder_Revert() public { - // // Update dynamic config to enforce allowOutOfOrderExecution = true. - // vm.stopPrank(); - // vm.startPrank(OWNER); - - // PriceRegistry.DestChainDynamicConfigArgs[] memory destChainConfigArgs = - // _generatePriceRegistryDestChainDynamicConfigArgs(); - // destChainConfigArgs[0].dynamicConfig.enforceOutOfOrder = true; - // s_priceRegistry.applyDestChainConfigUpdates(destChainConfigArgs); - // vm.stopPrank(); - - // vm.startPrank(address(s_sourceRouter)); - // Client.EVM2AnyMessage memory message = _generateEmptyMessage(); - // // Empty extraArgs to should revert since it enforceOutOfOrder is true. - // message.extraArgs = ""; - // uint256 feeAmount = 1234567890; - // IERC20(s_sourceFeeToken).transferFrom(OWNER, address(s_onRamp), feeAmount); - - // vm.expectRevert(PriceRegistry.ExtraArgOutOfOrderExecutionMustBeTrue.selector); - // s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, feeAmount, OWNER); - // } + function test_EnforceOutOfOrder_Revert() public { + // Update dynamic config to enforce allowOutOfOrderExecution = true. + vm.stopPrank(); + vm.startPrank(OWNER); + + PriceRegistry.DestChainDynamicConfigArgs[] memory destChainConfigArgs = + _generatePriceRegistryDestChainDynamicConfigArgs(); + destChainConfigArgs[0].dynamicConfig.enforceOutOfOrder = true; + s_priceRegistry.applyDestChainConfigUpdates(destChainConfigArgs); + vm.stopPrank(); + + Client.EVM2AnyMessage memory message = _generateEmptyMessage(); + // Empty extraArgs to should revert since it enforceOutOfOrder is true. + message.extraArgs = ""; + + vm.expectRevert(PriceRegistry.ExtraArgOutOfOrderExecutionMustBeTrue.selector); + s_onRamp.getFee(DEST_CHAIN_SELECTOR, message); + } + + function test_Fuzz_EnforceOutOfOrder(bool enforce, bool allowOutOfOrderExecution) public { + // Update dynamic config to enforce allowOutOfOrderExecution = defaultVal. + vm.stopPrank(); + vm.startPrank(OWNER); + + PriceRegistry.DestChainDynamicConfigArgs[] memory destChainConfigArgs = + _generatePriceRegistryDestChainDynamicConfigArgs(); + destChainConfigArgs[0].dynamicConfig.enforceOutOfOrder = enforce; + s_priceRegistry.applyDestChainConfigUpdates(destChainConfigArgs); + + Client.EVM2AnyMessage memory message = _generateEmptyMessage(); + message.extraArgs = abi.encodeWithSelector( + Client.EVM_EXTRA_ARGS_V2_TAG, + Client.EVMExtraArgsV2({gasLimit: GAS_LIMIT * 2, allowOutOfOrderExecution: allowOutOfOrderExecution}) + ); + + // If enforcement is on, only true should be allowed. + if (enforce && !allowOutOfOrderExecution) { + vm.expectRevert(PriceRegistry.ExtraArgOutOfOrderExecutionMustBeTrue.selector); + } + s_onRamp.getFee(DEST_CHAIN_SELECTOR, message); + } } contract EVM2EVMMultiOnRamp_setDynamicConfig is EVM2EVMMultiOnRampSetup { diff --git a/contracts/src/v0.8/ccip/test/priceRegistry/PriceRegistry.t.sol b/contracts/src/v0.8/ccip/test/priceRegistry/PriceRegistry.t.sol index 2d02f50695..0781363df5 100644 --- a/contracts/src/v0.8/ccip/test/priceRegistry/PriceRegistry.t.sol +++ b/contracts/src/v0.8/ccip/test/priceRegistry/PriceRegistry.t.sol @@ -155,10 +155,13 @@ contract PriceRegistrySetup is TokenSetup { ); s_priceRegistry = new PriceRegistryHelper( - PriceRegistry.StaticConfig({linkToken: s_sourceTokens[0], maxFeeJuelsPerMsg: MAX_MSG_FEES_JUELS}), + PriceRegistry.StaticConfig({ + linkToken: s_sourceTokens[0], + maxFeeJuelsPerMsg: MAX_MSG_FEES_JUELS, + stalenessThreshold: uint32(TWELVE_HOURS) + }), priceUpdaters, feeTokens, - uint32(TWELVE_HOURS), tokenPriceFeedUpdates, s_priceRegistryTokenTransferFeeConfigArgs, s_priceRegistryPremiumMultiplierWeiPerEthArgs, @@ -445,23 +448,26 @@ contract PriceRegistry_constructor is PriceRegistrySetup { s_priceRegistryPremiumMultiplierWeiPerEthArgs[1].premiumMultiplierWeiPerEth ); + PriceRegistry.StaticConfig memory staticConfig = PriceRegistry.StaticConfig({ + linkToken: s_sourceTokens[0], + maxFeeJuelsPerMsg: MAX_MSG_FEES_JUELS, + stalenessThreshold: uint32(TWELVE_HOURS) + }); s_priceRegistry = new PriceRegistryHelper( - PriceRegistry.StaticConfig({linkToken: s_sourceTokens[0], maxFeeJuelsPerMsg: MAX_MSG_FEES_JUELS}), + staticConfig, priceUpdaters, feeTokens, - uint32(TWELVE_HOURS), tokenPriceFeedUpdates, s_priceRegistryTokenTransferFeeConfigArgs, s_priceRegistryPremiumMultiplierWeiPerEthArgs, _generatePriceRegistryDestChainDynamicConfigArgs() ); - // TODO: assert staticConfig equality // TODO: assert dynamicConfig equality - // TODO: assert tokenTransferFeeConfig equality + // TODO: assert tokenTransferFeeConfigArgs (emit events) + _assertPriceRegistryStaticConfigsEqual(s_priceRegistry.getStaticConfig(), staticConfig); assertEq(feeTokens, s_priceRegistry.getFeeTokens()); - assertEq(uint32(TWELVE_HOURS), s_priceRegistry.getStalenessThreshold()); assertEq(priceUpdaters, s_priceRegistry.getAllAuthorizedCallers()); assertEq(s_priceRegistry.typeAndVersion(), "PriceRegistry 1.6.0-dev"); @@ -480,6 +486,18 @@ contract PriceRegistry_constructor is PriceRegistrySetup { uint64 gotFeeTokenConfig1 = s_priceRegistry.getPremiumMultiplierWeiPerEth(s_priceRegistryPremiumMultiplierWeiPerEthArgs[1].token); assertEq(s_priceRegistryPremiumMultiplierWeiPerEthArgs[1].premiumMultiplierWeiPerEth, gotFeeTokenConfig1); + + PriceRegistry.TokenTransferFeeConfigArgs memory tokenTransferFeeConfigArg = + s_priceRegistryTokenTransferFeeConfigArgs[0]; + for (uint256 i = 0; i < tokenTransferFeeConfigArg.tokenTransferFeeConfigs.length; ++i) { + PriceRegistry.TokenTransferFeeConfigSingleTokenArgs memory tokenFeeArgs = + s_priceRegistryTokenTransferFeeConfigArgs[0].tokenTransferFeeConfigs[i]; + + _assertTokenTransferFeeConfigEqual( + tokenFeeArgs.tokenTransferFeeConfig, + s_priceRegistry.getTokenTransferFeeConfig(tokenTransferFeeConfigArg.destChainSelector, tokenFeeArgs.token) + ); + } } // TODO: re-add test