Skip to content

Commit

Permalink
FeeQuoter : Timestamp Usage and Ensure Recent Price Selection (#14854)
Browse files Browse the repository at this point in the history
* Fix: Ensure the correct timestamp is used from the oracle feed and select the most recent price between local and oracle data.

* Lint fix

* fix variable name
  • Loading branch information
0xsuryansh authored Oct 21, 2024
1 parent e725f05 commit 3594a94
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 25 deletions.
5 changes: 5 additions & 0 deletions contracts/.changeset/loud-penguins-float.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@chainlink/contracts': minor
---

# internal Fix: Ensure the correct timestamp is used from the oracle feed and select the most recent price between local and oracle data.
31 changes: 16 additions & 15 deletions contracts/gas-snapshots/ccip.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,10 @@ FeeQuoter_applyTokenTransferFeeConfigUpdates:test_ApplyTokenTransferFeeConfig_Su
FeeQuoter_applyTokenTransferFeeConfigUpdates:test_ApplyTokenTransferFeeZeroInput() (gas: 13255)
FeeQuoter_applyTokenTransferFeeConfigUpdates:test_InvalidDestBytesOverhead_Revert() (gas: 17366)
FeeQuoter_applyTokenTransferFeeConfigUpdates:test_OnlyCallableByOwnerOrAdmin_Revert() (gas: 12352)
FeeQuoter_constructor:test_InvalidLinkTokenEqZeroAddress_Revert() (gas: 106579)
FeeQuoter_constructor:test_InvalidMaxFeeJuelsPerMsg_Revert() (gas: 110929)
FeeQuoter_constructor:test_InvalidStalenessThreshold_Revert() (gas: 110982)
FeeQuoter_constructor:test_Setup_Success() (gas: 5013710)
FeeQuoter_constructor:test_InvalidLinkTokenEqZeroAddress_Revert() (gas: 106589)
FeeQuoter_constructor:test_InvalidMaxFeeJuelsPerMsg_Revert() (gas: 110939)
FeeQuoter_constructor:test_InvalidStalenessThreshold_Revert() (gas: 110992)
FeeQuoter_constructor:test_Setup_Success() (gas: 5020729)
FeeQuoter_convertTokenAmount:test_ConvertTokenAmount_Success() (gas: 68361)
FeeQuoter_convertTokenAmount:test_LinkTokenNotSupported_Revert() (gas: 29076)
FeeQuoter_getDataAvailabilityCost:test_EmptyMessageCalculatesDataAvailabilityCost_Success() (gas: 96045)
Expand All @@ -120,7 +120,8 @@ FeeQuoter_getTokenAndGasPrices:test_StaleGasPrice_Revert() (gas: 26309)
FeeQuoter_getTokenAndGasPrices:test_StalenessCheckDisabled_Success() (gas: 111759)
FeeQuoter_getTokenAndGasPrices:test_UnsupportedChain_Revert() (gas: 16081)
FeeQuoter_getTokenAndGasPrices:test_ZeroGasPrice_Success() (gas: 109015)
FeeQuoter_getTokenPrice:test_GetTokenPriceFromFeed_Success() (gas: 66273)
FeeQuoter_getTokenPrice:test_GetTokenPriceFromFeed_Success() (gas: 67299)
FeeQuoter_getTokenPrice:test_GetTokenPrice_LocalMoreRecent_Success() (gas: 33219)
FeeQuoter_getTokenPrices:test_GetTokenPrices_Success() (gas: 78388)
FeeQuoter_getTokenTransferCost:test_CustomTokenBpsFee_Success() (gas: 39239)
FeeQuoter_getTokenTransferCost:test_FeeTokenBpsFee_Success() (gas: 34876)
Expand All @@ -143,20 +144,20 @@ FeeQuoter_getValidatedFee:test_NotAFeeToken_Revert() (gas: 21211)
FeeQuoter_getValidatedFee:test_SingleTokenMessage_Success() (gas: 113804)
FeeQuoter_getValidatedFee:test_TooManyTokens_Revert() (gas: 22729)
FeeQuoter_getValidatedFee:test_ZeroDataAvailabilityMultiplier_Success() (gas: 63687)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedErc20Above18Decimals_Success() (gas: 1973956)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedErc20Below18Decimals_Success() (gas: 1973914)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedFeedAt0Decimals_Success() (gas: 1954033)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedFeedAt18Decimals_Success() (gas: 1973688)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedFlippedDecimals_Success() (gas: 1973892)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedMaxInt224Value_Success() (gas: 1973704)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedOverStalenessPeriod_Success() (gas: 64610)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeed_Success() (gas: 64490)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedErc20Above18Decimals_Success() (gas: 1974036)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedErc20Below18Decimals_Success() (gas: 1973994)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedFeedAt0Decimals_Success() (gas: 1954113)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedFeedAt18Decimals_Success() (gas: 1973768)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedFlippedDecimals_Success() (gas: 1973972)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedMaxInt224Value_Success() (gas: 1973784)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedOverStalenessPeriod_Success() (gas: 64690)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeed_Success() (gas: 64570)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPrice_Success() (gas: 58894)
FeeQuoter_getValidatedTokenPrice:test_OverflowFeedPrice_Revert() (gas: 1973401)
FeeQuoter_getValidatedTokenPrice:test_OverflowFeedPrice_Revert() (gas: 1973410)
FeeQuoter_getValidatedTokenPrice:test_StaleFeeToken_Success() (gas: 61764)
FeeQuoter_getValidatedTokenPrice:test_TokenNotSupportedFeed_Revert() (gas: 116495)
FeeQuoter_getValidatedTokenPrice:test_TokenNotSupported_Revert() (gas: 14103)
FeeQuoter_getValidatedTokenPrice:test_UnderflowFeedPrice_Revert() (gas: 1972078)
FeeQuoter_getValidatedTokenPrice:test_UnderflowFeedPrice_Revert() (gas: 1972087)
FeeQuoter_onReport:test_OnReport_StaleUpdate_Revert() (gas: 43675)
FeeQuoter_onReport:test_onReport_InvalidForwarder_Reverts() (gas: 23514)
FeeQuoter_onReport:test_onReport_Success() (gas: 80116)
Expand Down
12 changes: 6 additions & 6 deletions contracts/src/v0.8/ccip/FeeQuoter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,10 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
return tokenPrice;
}

// If the token price feed is set, return the price from the feed
// The price feed is the fallback because we do not expect it to be the default source due to the gas cost of reading from it
return _getTokenPriceFromDataFeed(priceFeedConfig);
// If the token price feed is set, retrieve the price from the feed
Internal.TimestampedPackedUint224 memory feedPrice = _getTokenPriceFromDataFeed(priceFeedConfig);

return feedPrice.timestamp >= tokenPrice.timestamp ? feedPrice : tokenPrice;
}

/// @notice Get the `tokenPrice` for a given token, checks if the price is valid.
Expand Down Expand Up @@ -368,8 +369,7 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
int256 dataFeedAnswer,
/* uint startedAt */
,
/* uint256 updatedAt */
,
uint256 updatedAt,
/* uint80 answeredInRound */
) = dataFeedContract.latestRoundData();

Expand All @@ -380,7 +380,7 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
_calculateRebasedValue(dataFeedContract.decimals(), priceFeedConfig.tokenDecimals, uint256(dataFeedAnswer));

// Data feed staleness is unchecked to decouple the FeeQuoter from data feed delay issues
return Internal.TimestampedPackedUint224({value: rebasedValue, timestamp: uint32(block.timestamp)});
return Internal.TimestampedPackedUint224({value: rebasedValue, timestamp: uint32(updatedAt)});
}

/// @dev Gets the fee token price and the gas price, both denominated in dollars.
Expand Down
32 changes: 31 additions & 1 deletion contracts/src/v0.8/ccip/test/feeQuoter/FeeQuoter.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,41 @@ contract FeeQuoter_getTokenPrice is FeeQuoterSetup {
vm.warp(originalTimestampValue + s_feeQuoter.getStaticConfig().tokenPriceStalenessThreshold + 1);

address sourceToken = _initialiseSingleTokenPriceFeed();

vm.expectCall(s_dataFeedByToken[sourceToken], abi.encodeWithSelector(MockV3Aggregator.latestRoundData.selector));

Internal.TimestampedPackedUint224 memory tokenPriceAnswer = s_feeQuoter.getTokenPrice(sourceToken);

// Price answer is 1e8 (18 decimal token) - unit is (1e18 * 1e18 / 1e18) -> expected 1e18
assertEq(tokenPriceAnswer.value, uint224(1e18));
assertEq(tokenPriceAnswer.timestamp, uint32(block.timestamp));
assertEq(tokenPriceAnswer.timestamp, uint32(originalTimestampValue));
}

function test_GetTokenPrice_LocalMoreRecent_Success() public {
uint256 originalTimestampValue = block.timestamp;

Internal.PriceUpdates memory update = Internal.PriceUpdates({
tokenPriceUpdates: new Internal.TokenPriceUpdate[](1),
gasPriceUpdates: new Internal.GasPriceUpdate[](0)
});

update.tokenPriceUpdates[0] =
Internal.TokenPriceUpdate({sourceToken: s_sourceTokens[0], usdPerToken: uint32(originalTimestampValue + 5)});

vm.expectEmit();
emit FeeQuoter.UsdPerTokenUpdated(
update.tokenPriceUpdates[0].sourceToken, update.tokenPriceUpdates[0].usdPerToken, block.timestamp
);

s_feeQuoter.updatePrices(update);

vm.warp(originalTimestampValue + s_feeQuoter.getStaticConfig().tokenPriceStalenessThreshold + 10);

Internal.TimestampedPackedUint224 memory tokenPriceAnswer = s_feeQuoter.getTokenPrice(s_sourceTokens[0]);

//Assert that the returned price is the local price, not the oracle price
assertEq(tokenPriceAnswer.value, update.tokenPriceUpdates[0].usdPerToken);
assertEq(tokenPriceAnswer.timestamp, uint32(originalTimestampValue));
}
}

Expand Down
2 changes: 1 addition & 1 deletion core/gethwrappers/ccip/generated/fee_quoter/fee_quoter.go

Large diffs are not rendered by default.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ ccip_encoding_utils: ../../../contracts/solc/v0.8.24/ICCIPEncodingUtils/ICCIPEnc
ccip_home: ../../../contracts/solc/v0.8.24/CCIPHome/CCIPHome.abi ../../../contracts/solc/v0.8.24/CCIPHome/CCIPHome.bin 079b70ad36b4a9522518df82f01bdb8480fb9bb8de5791ef17ea1ddf044814be
ccip_reader_tester: ../../../contracts/solc/v0.8.24/CCIPReaderTester/CCIPReaderTester.abi ../../../contracts/solc/v0.8.24/CCIPReaderTester/CCIPReaderTester.bin 5db06eb7fad07ec0d1ae5ac8d39f61398687fe3cda8290716ce0cd8fb9dca1ab
ether_sender_receiver: ../../../contracts/solc/v0.8.24/EtherSenderReceiver/EtherSenderReceiver.abi ../../../contracts/solc/v0.8.24/EtherSenderReceiver/EtherSenderReceiver.bin 09510a3f773f108a3c231e8d202835c845ded862d071ec54c4f89c12d868b8de
fee_quoter: ../../../contracts/solc/v0.8.24/FeeQuoter/FeeQuoter.abi ../../../contracts/solc/v0.8.24/FeeQuoter/FeeQuoter.bin 7f1d7705617295aaba9f62211d38a06cf3099db47e6286cccf1e2412992a0042
fee_quoter: ../../../contracts/solc/v0.8.24/FeeQuoter/FeeQuoter.abi ../../../contracts/solc/v0.8.24/FeeQuoter/FeeQuoter.bin 6e647595433c98ccb33d27cc73a82804dbb5369a9b56fc87c43797cb9cf9a39a
lock_release_token_pool: ../../../contracts/solc/v0.8.24/LockReleaseTokenPool/LockReleaseTokenPool.abi ../../../contracts/solc/v0.8.24/LockReleaseTokenPool/LockReleaseTokenPool.bin e6a8ec9e8faccb1da7d90e0f702ed72975964f97dc3222b54cfcca0a0ba3fea2
maybe_revert_message_receiver: ../../../contracts/solc/v0.8.24/MaybeRevertMessageReceiver/MaybeRevertMessageReceiver.abi ../../../contracts/solc/v0.8.24/MaybeRevertMessageReceiver/MaybeRevertMessageReceiver.bin d73956c26232ebcc4a5444429fa99cbefed960e323be9b5a24925885c2e477d5
message_hasher: ../../../contracts/solc/v0.8.24/MessageHasher/MessageHasher.abi ../../../contracts/solc/v0.8.24/MessageHasher/MessageHasher.bin ec2d3a92348d8e7b8f0d359b62a45157b9d2c750c01fbcf991826c4392f6e218
Expand Down

0 comments on commit 3594a94

Please sign in to comment.