Skip to content

Commit

Permalink
CCIP-4359 fee quoter supports interface for keystone (#15448)
Browse files Browse the repository at this point in the history
* fix: enforce supports interface from IReceiver.sol and impliment in FeeQuoter.sol

* fix: enforce supports interface from IReceiver.sol and impliment in FeeQuoter.sol

* Update gethwrappers

* CCIP-4359 changeset + minor fixes and wrappers

* snapshot update

* [Bot] Update changeset file with jira issues

* lint fix

* chore: import arrangement and additional comments

* fix : comment in IReceiver

* fix : comment in KeystoneFeedsPermissionHandler

* fmt

* add fee_quoter_interface

* add fee_quoter_interface

* remove incorrectly added mock interface

* add mock interface

* add fee_quoter_interface.go

* added test with Keystone Forwarder

* update cod cov ignore for ccip

* cleanup: removed newline

* update coverage

* Revert "update cod cov ignore for ccip"

This reverts commit 4a62610.

* update lcov_prine to exclude keystone contract in coverage for CCIP

* Revert "update lcov_prine to exclude keystone contract in coverage for CCIP"

This reverts commit 5a0d650.

* remove keystone test setup dependency

* chore: update test name according to style guide

* update snapshot

* Revert "Revert "update lcov_prine to exclude keystone contract in coverage for CCIP""

This reverts commit 2c86461.

---------

Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
  • Loading branch information
1 parent 22c6cf7 commit c1ee7ab
Show file tree
Hide file tree
Showing 15 changed files with 344 additions and 208 deletions.
9 changes: 9 additions & 0 deletions contracts/.changeset/tender-lemons-punch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@chainlink/contracts': minor
---

#internal Add supportsInterface to FeeQuoter for Keystone

PR issue: CCIP-4359

Solidity Review issue: CCIP-3966
317 changes: 158 additions & 159 deletions contracts/gas-snapshots/ccip.gas-snapshot

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions contracts/scripts/lcov_prune
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ exclusion_list_ccip=(
"src/v0.8/ConfirmedOwnerWithProposal.sol"
"src/v0.8/tests/MockV3Aggregator.sol"
"src/v0.8/ccip/applications/CCIPClientExample.sol"
"src/v0.8/keystone/*"
)

exclusion_list_shared=(
Expand Down
9 changes: 9 additions & 0 deletions contracts/src/v0.8/ccip/FeeQuoter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {Internal} from "./libraries/Internal.sol";
import {Pool} from "./libraries/Pool.sol";
import {USDPriceWith18Decimals} from "./libraries/USDPriceWith18Decimals.sol";

import {IERC165} from "../vendor/openzeppelin-solidity/v5.0.2/contracts/interfaces/IERC165.sol";
import {EnumerableSet} from "../vendor/openzeppelin-solidity/v5.0.2/contracts/utils/structs/EnumerableSet.sol";

/// @notice The FeeQuoter contract responsibility is to:
Expand Down Expand Up @@ -493,6 +494,14 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
}
}

/// @notice Signals which version of the pool interface is supported
function supportsInterface(
bytes4 interfaceId
) public pure override returns (bool) {
return interfaceId == type(IReceiver).interfaceId || interfaceId == type(IFeeQuoter).interfaceId
|| interfaceId == type(ITypeAndVersion).interfaceId || interfaceId == type(IERC165).interfaceId;
}

/// @inheritdoc IReceiver
/// @notice Handles the report containing price feeds and updates the internal price storage.
/// @dev This function is called to process incoming price feed data.
Expand Down
84 changes: 50 additions & 34 deletions contracts/src/v0.8/ccip/test/feeQuoter/FeeQuoter.onReport.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,42 @@
pragma solidity 0.8.24;

import {KeystoneFeedsPermissionHandler} from "../../../keystone/KeystoneFeedsPermissionHandler.sol";

import {KeystoneForwarder} from "../../../keystone/KeystoneForwarder.sol";
import {FeeQuoter} from "../../FeeQuoter.sol";
import {FeeQuoterSetup} from "./FeeQuoterSetup.t.sol";

contract FeeQuoter_onReport is FeeQuoterSetup {
address internal constant FORWARDER_1 = address(0x1);
address internal constant WORKFLOW_OWNER_1 = address(0x3);
bytes10 internal constant WORKFLOW_NAME_1 = "workflow1";
bytes2 internal constant REPORT_NAME_1 = "01";
bytes32 internal constant EXECUTION_ID = hex"6d795f657865637574696f6e5f69640000000000000000000000000000000000";
address internal constant TRANSMITTER = address(50);
bytes32 internal constant WORKFLOW_ID_1 = hex"6d795f6964000000000000000000000000000000000000000000000000000000";
address internal constant WORKFLOW_OWNER_1 = address(51);
bytes10 internal constant WORKFLOW_NAME_1 = hex"000000000000DEADBEEF";
bytes2 internal constant REPORT_NAME_1 = hex"0001";
address internal s_onReportTestToken1;
address internal s_onReportTestToken2;
bytes public encodedPermissionsMetadata;
KeystoneForwarder internal s_forwarder;

function setUp() public virtual override {
super.setUp();

s_forwarder = new KeystoneForwarder();

s_onReportTestToken1 = s_sourceTokens[0];
s_onReportTestToken2 = _deploySourceToken("onReportTestToken2", 0, 20);

KeystoneFeedsPermissionHandler.Permission[] memory permissions = new KeystoneFeedsPermissionHandler.Permission[](1);
permissions[0] = KeystoneFeedsPermissionHandler.Permission({
forwarder: FORWARDER_1,
forwarder: address(s_forwarder),
workflowOwner: WORKFLOW_OWNER_1,
workflowName: WORKFLOW_NAME_1,
reportName: REPORT_NAME_1,
isAllowed: true
});

encodedPermissionsMetadata = abi.encodePacked(WORKFLOW_ID_1, WORKFLOW_NAME_1, WORKFLOW_OWNER_1, REPORT_NAME_1);

FeeQuoter.TokenPriceFeedUpdate[] memory tokenPriceFeeds = new FeeQuoter.TokenPriceFeedUpdate[](2);
tokenPriceFeeds[0] = FeeQuoter.TokenPriceFeedUpdate({
sourceToken: s_onReportTestToken1,
Expand All @@ -39,10 +51,7 @@ contract FeeQuoter_onReport is FeeQuoterSetup {
s_feeQuoter.updateTokenPriceFeeds(tokenPriceFeeds);
}

function test_onReport_Success() public {
bytes memory encodedPermissionsMetadata =
abi.encodePacked(keccak256(abi.encode("workflowCID")), WORKFLOW_NAME_1, WORKFLOW_OWNER_1, REPORT_NAME_1);

function test_onReport() public {
FeeQuoter.ReceivedCCIPFeedReport[] memory report = new FeeQuoter.ReceivedCCIPFeedReport[](2);
report[0] =
FeeQuoter.ReceivedCCIPFeedReport({token: s_onReportTestToken1, price: 4e18, timestamp: uint32(block.timestamp)});
Expand All @@ -56,7 +65,7 @@ contract FeeQuoter_onReport is FeeQuoterSetup {
vm.expectEmit();
emit FeeQuoter.UsdPerTokenUpdated(s_onReportTestToken2, expectedStoredToken2Price, block.timestamp);

changePrank(FORWARDER_1);
changePrank(address(s_forwarder));
s_feeQuoter.onReport(encodedPermissionsMetadata, abi.encode(report));

vm.assertEq(s_feeQuoter.getTokenPrice(report[0].token).value, expectedStoredToken1Price);
Expand All @@ -66,11 +75,34 @@ contract FeeQuoter_onReport is FeeQuoterSetup {
vm.assertEq(s_feeQuoter.getTokenPrice(report[1].token).timestamp, report[1].timestamp);
}

function test_OnReport_StaleUpdate_SkipPriceUpdate_Success() public {
//Creating a correct report
bytes memory encodedPermissionsMetadata =
abi.encodePacked(keccak256(abi.encode("workflowCID")), WORKFLOW_NAME_1, WORKFLOW_OWNER_1, REPORT_NAME_1);
function test_onReport_withKeystoneForwarderContract() public {
FeeQuoter.ReceivedCCIPFeedReport[] memory priceReportRaw = new FeeQuoter.ReceivedCCIPFeedReport[](2);
priceReportRaw[0] =
FeeQuoter.ReceivedCCIPFeedReport({token: s_onReportTestToken1, price: 4e18, timestamp: uint32(block.timestamp)});
priceReportRaw[1] =
FeeQuoter.ReceivedCCIPFeedReport({token: s_onReportTestToken2, price: 4e18, timestamp: uint32(block.timestamp)});

uint224 expectedStoredToken1Price = s_feeQuoter.calculateRebasedValue(18, 18, priceReportRaw[0].price);
uint224 expectedStoredToken2Price = s_feeQuoter.calculateRebasedValue(18, 20, priceReportRaw[1].price);

vm.expectEmit();
emit FeeQuoter.UsdPerTokenUpdated(s_onReportTestToken1, expectedStoredToken1Price, block.timestamp);
vm.expectEmit();
emit FeeQuoter.UsdPerTokenUpdated(s_onReportTestToken2, expectedStoredToken2Price, block.timestamp);

changePrank(address(s_forwarder));
s_forwarder.route(
EXECUTION_ID, TRANSMITTER, address(s_feeQuoter), encodedPermissionsMetadata, abi.encode(priceReportRaw)
);

vm.assertEq(s_feeQuoter.getTokenPrice(priceReportRaw[0].token).value, expectedStoredToken1Price);
vm.assertEq(s_feeQuoter.getTokenPrice(priceReportRaw[0].token).timestamp, priceReportRaw[0].timestamp);

vm.assertEq(s_feeQuoter.getTokenPrice(priceReportRaw[1].token).value, expectedStoredToken2Price);
vm.assertEq(s_feeQuoter.getTokenPrice(priceReportRaw[1].token).timestamp, priceReportRaw[1].timestamp);
}

function test_OnReport_SkipPriceUpdateWhenStaleUpdateReceived() public {
FeeQuoter.ReceivedCCIPFeedReport[] memory report = new FeeQuoter.ReceivedCCIPFeedReport[](1);
report[0] =
FeeQuoter.ReceivedCCIPFeedReport({token: s_onReportTestToken1, price: 4e18, timestamp: uint32(block.timestamp)});
Expand All @@ -80,7 +112,7 @@ contract FeeQuoter_onReport is FeeQuoterSetup {
vm.expectEmit();
emit FeeQuoter.UsdPerTokenUpdated(s_onReportTestToken1, expectedStoredTokenPrice, block.timestamp);

changePrank(FORWARDER_1);
changePrank(address(s_forwarder));
//setting the correct price and time with the correct report
s_feeQuoter.onReport(encodedPermissionsMetadata, abi.encode(report));

Expand All @@ -100,22 +132,18 @@ contract FeeQuoter_onReport is FeeQuoterSetup {
assertEq(vm.getRecordedLogs().length, 0);
}

function test_onReport_TokenNotSupported_Revert() public {
bytes memory encodedPermissionsMetadata =
abi.encodePacked(keccak256(abi.encode("workflowCID")), WORKFLOW_NAME_1, WORKFLOW_OWNER_1, REPORT_NAME_1);
function test_onReport_RevertWhen_TokenNotSupported() public {
FeeQuoter.ReceivedCCIPFeedReport[] memory report = new FeeQuoter.ReceivedCCIPFeedReport[](1);
report[0] =
FeeQuoter.ReceivedCCIPFeedReport({token: s_sourceTokens[1], price: 4e18, timestamp: uint32(block.timestamp)});

// Revert due to token config not being set with the isEnabled flag
vm.expectRevert(abi.encodeWithSelector(FeeQuoter.TokenNotSupported.selector, s_sourceTokens[1]));
vm.startPrank(FORWARDER_1);
changePrank(address(s_forwarder));
s_feeQuoter.onReport(encodedPermissionsMetadata, abi.encode(report));
}

function test_onReport_InvalidForwarder_Reverts() public {
bytes memory encodedPermissionsMetadata =
abi.encodePacked(keccak256(abi.encode("workflowCID")), WORKFLOW_NAME_1, WORKFLOW_OWNER_1, REPORT_NAME_1);
function test_onReport_RevertWhen_InvalidForwarder() public {
FeeQuoter.ReceivedCCIPFeedReport[] memory report = new FeeQuoter.ReceivedCCIPFeedReport[](1);
report[0] =
FeeQuoter.ReceivedCCIPFeedReport({token: s_sourceTokens[0], price: 4e18, timestamp: uint32(block.timestamp)});
Expand All @@ -132,16 +160,4 @@ contract FeeQuoter_onReport is FeeQuoterSetup {
changePrank(STRANGER);
s_feeQuoter.onReport(encodedPermissionsMetadata, abi.encode(report));
}

function test_onReport_UnsupportedToken_Reverts() public {
bytes memory encodedPermissionsMetadata =
abi.encodePacked(keccak256(abi.encode("workflowCID")), WORKFLOW_NAME_1, WORKFLOW_OWNER_1, REPORT_NAME_1);
FeeQuoter.ReceivedCCIPFeedReport[] memory report = new FeeQuoter.ReceivedCCIPFeedReport[](1);
report[0] =
FeeQuoter.ReceivedCCIPFeedReport({token: s_sourceTokens[1], price: 4e18, timestamp: uint32(block.timestamp)});

vm.expectRevert(abi.encodeWithSelector(FeeQuoter.TokenNotSupported.selector, s_sourceTokens[1]));
changePrank(FORWARDER_1);
s_feeQuoter.onReport(encodedPermissionsMetadata, abi.encode(report));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.24;

import {IReceiver} from "../../../keystone/interfaces/IReceiver.sol";
import {ITypeAndVersion} from "../../../shared/interfaces/ITypeAndVersion.sol";

import {IERC165} from "../../../vendor/openzeppelin-solidity/v5.0.2/contracts/interfaces/IERC165.sol";
import {IFeeQuoter} from "../../interfaces/IFeeQuoter.sol";
import {FeeQuoterSetup} from "./FeeQuoterSetup.t.sol";

contract FeeQuoter_supportsInterface is FeeQuoterSetup {
function test_SupportsInterface_Success() public view {
assertTrue(s_feeQuoter.supportsInterface(type(IReceiver).interfaceId));
assertTrue(s_feeQuoter.supportsInterface(type(ITypeAndVersion).interfaceId));
assertTrue(s_feeQuoter.supportsInterface(type(IFeeQuoter).interfaceId));
assertTrue(s_feeQuoter.supportsInterface(type(IERC165).interfaceId));
}
}
4 changes: 2 additions & 2 deletions contracts/src/v0.8/keystone/KeystoneFeedsConsumer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {OwnerIsCreator} from "../shared/access/OwnerIsCreator.sol";

import {IERC165} from "../vendor/openzeppelin-solidity/v4.8.3/contracts/interfaces/IERC165.sol";

contract KeystoneFeedsConsumer is IReceiver, OwnerIsCreator, IERC165 {
contract KeystoneFeedsConsumer is IReceiver, OwnerIsCreator {
event FeedReceived(bytes32 indexed feedId, uint224 price, uint32 timestamp);

error UnauthorizedSender(address sender);
Expand Down Expand Up @@ -101,7 +101,7 @@ contract KeystoneFeedsConsumer is IReceiver, OwnerIsCreator, IERC165 {
return (report.Price, report.Timestamp);
}

function supportsInterface(bytes4 interfaceId) public pure returns (bool) {
function supportsInterface(bytes4 interfaceId) public pure override returns (bool) {
return interfaceId == type(IReceiver).interfaceId || interfaceId == type(IERC165).interfaceId;
}
}
10 changes: 5 additions & 5 deletions contracts/src/v0.8/keystone/KeystoneFeedsPermissionHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ abstract contract KeystoneFeedsPermissionHandler is Ownable2StepMsgSender {
/// @notice Holds the details for permissions of a report
/// @dev Workflow names and report names are stored as bytes to optimize for gas efficiency.
struct Permission {
address forwarder; // ─────╮ The address of the forwarder (20 bytes)
bytes10 workflowName; // │ The name of the workflow in bytes10
bytes2 reportName; // ─────╯ The name of the report in bytes2
address workflowOwner; // ─╮ The address of the workflow owner (20 bytes)
bool isAllowed; // ────────╯ Whether the report is allowed or not (1 byte)
address forwarder; // ─────╮ The address of the forwarder (20 bytes)
bytes10 workflowName; // │ The name of the workflow in bytes10
bytes2 reportName; // ─────╯ The name of the report in bytes2
address workflowOwner; // ─╮ The address of the workflow owner (20 bytes)
bool isAllowed; // ────────╯ Whether the report is allowed or not (1 byte)
}

/// @notice Event emitted when report permissions are set
Expand Down
5 changes: 4 additions & 1 deletion contracts/src/v0.8/keystone/interfaces/IReceiver.sol
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import {IERC165} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts/utils/introspection/IERC165.sol";

/// @title IReceiver - receives keystone reports
interface IReceiver {
/// @notice Implementations must support the IReceiver interface through ERC165.
interface IReceiver is IERC165 {
/// @notice Handles incoming keystone reports.
/// @dev If this function call reverts, it can be retried with a higher gas
/// limit. The receiver is responsible for discarding stale reports.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pragma solidity 0.8.24;
import {IERC165} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/interfaces/IERC165.sol";
import {IReceiver} from "../../interfaces/IReceiver.sol";

contract MaliciousReportReceiver is IReceiver, IERC165 {
contract MaliciousReportReceiver is IReceiver {
event MessageReceived(bytes metadata, bytes[] mercuryReports);
bytes public latestReport;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {IReceiver} from "../../interfaces/IReceiver.sol";

/// A malicious receiver that uses max allowed for ERC165 checks and consumes all gas in `onReport()`
/// Causes parent Forwarder contract to revert if it doesn't handle gas tracking accurately
contract MaliciousRevertingReceiver is IReceiver, IERC165 {
contract MaliciousRevertingReceiver is IReceiver {
function onReport(bytes calldata, bytes calldata) external view override {
// consumes about 63/64 of all gas available
uint256 targetGasRemaining = 200;
Expand Down
4 changes: 2 additions & 2 deletions contracts/src/v0.8/keystone/test/mocks/Receiver.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pragma solidity 0.8.24;
import {IERC165} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/interfaces/IERC165.sol";
import {IReceiver} from "../../interfaces/IReceiver.sol";

contract Receiver is IReceiver, IERC165 {
contract Receiver is IReceiver {
event MessageReceived(bytes metadata, bytes[] mercuryReports);
bytes public latestReport;

Expand All @@ -18,7 +18,7 @@ contract Receiver is IReceiver, IERC165 {
emit MessageReceived(metadata, mercuryReports);
}

function supportsInterface(bytes4 interfaceId) public pure returns (bool) {
function supportsInterface(bytes4 interfaceId) public pure override returns (bool) {
return interfaceId == type(IReceiver).interfaceId || interfaceId == type(IERC165).interfaceId;
}
}
28 changes: 26 additions & 2 deletions core/gethwrappers/ccip/generated/fee_quoter/fee_quoter.go

Large diffs are not rendered by default.

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 02cb75b4274a5be7f4006cf2b72cc09e77eb6dba4c1a9c720af86668ff8ea1df
ccip_reader_tester: ../../../contracts/solc/v0.8.24/CCIPReaderTester/CCIPReaderTester.abi ../../../contracts/solc/v0.8.24/CCIPReaderTester/CCIPReaderTester.bin b368699ae7dbee7c21d049a641642837f18ce2cc8d4ece69509f205de673108e
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 8a0869d14bb5247fbc6d836fc20d123358373ed688e0d3b387d59e7d05496fea
fee_quoter: ../../../contracts/solc/v0.8.24/FeeQuoter/FeeQuoter.abi ../../../contracts/solc/v0.8.24/FeeQuoter/FeeQuoter.bin aee49c9246d5903e68b175516b3cdfdec5df23e25d53d604cd382b6bc0bf34f7
lock_release_token_pool: ../../../contracts/solc/v0.8.24/LockReleaseTokenPool/LockReleaseTokenPool.abi ../../../contracts/solc/v0.8.24/LockReleaseTokenPool/LockReleaseTokenPool.bin 1067f557abeb5570f1da7f050ea982ffad0f35dc064e668a8a0e6af128df490c
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
57 changes: 57 additions & 0 deletions core/gethwrappers/ccip/mocks/fee_quoter_interface.go

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

0 comments on commit c1ee7ab

Please sign in to comment.