Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FUN-1234 (feat): Chainlink Functions premium fees in USD denomination #12000

Closed
wants to merge 19 commits into from
Closed

FUN-1234 (feat): Chainlink Functions premium fees in USD denomination #12000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
b2927a3
FUN-1234 (feat): Chainlink Functions premium fees in USD denomination
justinkaseman Feb 12, 2024
84a2675
(test): Update Fucntions Hardhat tests for USD premium fees
justinkaseman Feb 12, 2024
8aec7fe
Soothe SonarQube. Set supported typeAndVersion to constants
justinkaseman Feb 15, 2024
9e50fa9
Remove more duplicated code: commitment ABI
justinkaseman Feb 15, 2024
86321d6
Remove even more duplicated code: OracleRequest evmRelayType
justinkaseman Feb 15, 2024
97fea36
Changes from review
justinkaseman Feb 16, 2024
e26672d
(test): Update LogPollerWrapper tests with Coordinator V1 vs V2
justinkaseman Feb 17, 2024
09df392
Changes from review
justinkaseman Feb 19, 2024
1ba2c06
(test): Run LogPollerWrapperTest v1/v2 tests sequentially
justinkaseman Feb 19, 2024
f5afdb8
Use typeAndVersion go wrapper more genericly
justinkaseman Feb 19, 2024
2adfc1b
[FUN-1234] refactor logpoller_wrapper by injecting a coordinator dep …
agparadiso Feb 19, 2024
753122a
chore: internal abitypes to avoid pkg variables and init function
agparadiso Feb 20, 2024
688b5f4
chore: reuse abi.Arguments to remove duplication
agparadiso Feb 20, 2024
bcc39f2
chore: rollback func addr() change
agparadiso Feb 20, 2024
a1d93bc
chore: improve logs
agparadiso Feb 20, 2024
361893f
fix: no changes if proposed is nil
agparadiso Feb 20, 2024
4991b13
chore: extract building of versioned coordinator to a helper function
agparadiso Feb 20, 2024
2c47244
chore: simplify Coordinator interface
agparadiso Feb 20, 2024
6ace734
chore: extract FilterName to a const
agparadiso Feb 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 74 additions & 72 deletions contracts/gas-snapshots/functions.gas-snapshot
View file
Open in desktop

Large diffs are not rendered by default.

127 changes: 94 additions & 33 deletions contracts/src/v0.8/functions/dev/v1_X/FunctionsBilling.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {ChainSpecificUtil} from "./libraries/ChainSpecificUtil.sol";
abstract contract FunctionsBilling is Routable, IFunctionsBilling {
using FunctionsResponse for FunctionsResponse.RequestMeta;
using FunctionsResponse for FunctionsResponse.Commitment;
using FunctionsResponse for FunctionsResponse.CommitmentWithOperationFee;
using FunctionsResponse for FunctionsResponse.FulfillResult;

uint256 private constant REASONABLE_GAS_PRICE_CEILING = 1_000_000_000_000_000; // 1 million gwei
Expand All @@ -26,7 +27,9 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
uint96 juelsPerGas,
uint256 l1FeeShareWei,
uint96 callbackCostJuels,
uint96 totalCostJuels
uint72 donFeeJuels,
uint72 adminFeeJuels,
uint72 operationFeeJuels
);

// ================================================================
Expand All @@ -47,6 +50,7 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
error UnauthorizedSender();
error MustBeSubOwner(address owner);
error InvalidLinkWeiPrice(int256 linkWei);
error InvalidUsdLinkPrice(int256 usdLink);
error PaymentTooLarge();
error NoTransmittersSet();
error InvalidCalldata();
Expand All @@ -61,12 +65,19 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
uint96 internal s_feePool;

AggregatorV3Interface private s_linkToNativeFeed;
AggregatorV3Interface private s_linkToUsdFeed;

// ================================================================
// | Initialization |
// ================================================================
constructor(address router, FunctionsBillingConfig memory config, address linkToNativeFeed) Routable(router) {
constructor(
address router,
FunctionsBillingConfig memory config,
address linkToNativeFeed,
address linkToUsdFeed
) Routable(router) {
s_linkToNativeFeed = AggregatorV3Interface(linkToNativeFeed);
s_linkToUsdFeed = AggregatorV3Interface(linkToUsdFeed);

updateConfig(config);
}
Expand Down Expand Up @@ -95,22 +106,28 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
// ================================================================

/// @inheritdoc IFunctionsBilling
function getDONFee(bytes memory /* requestData */) public view override returns (uint72) {
return s_config.donFee;
function getDONFeeJuels(bytes memory /* requestData */) public view override returns (uint72) {
// s_config.donFee is in cents of USD. Get Juel amount then convert to dollars.
return SafeCast.toUint72(_getJuelsFromUsd(s_config.donFeeCentsUsd) / 100);
}

/// @inheritdoc IFunctionsBilling
function getAdminFee() public view override returns (uint72) {
function getOperationFeeJuels() public view override returns (uint72) {
// s_config.donFee is in cents of USD. Get Juel amount then convert to dollars.
return SafeCast.toUint72(_getJuelsFromUsd(s_config.operationFeeCentsUsd) / 100);
}

/// @inheritdoc IFunctionsBilling
function getAdminFeeJuels() public view override returns (uint72) {
return _getRouter().getAdminFee();
}

/// @inheritdoc IFunctionsBilling
function getWeiPerUnitLink() public view returns (uint256) {
FunctionsBillingConfig memory config = s_config;
(, int256 weiPerUnitLink, , uint256 timestamp, ) = s_linkToNativeFeed.latestRoundData();
// solhint-disable-next-line not-rely-on-time
if (config.feedStalenessSeconds < block.timestamp - timestamp && config.feedStalenessSeconds > 0) {
return config.fallbackNativePerUnitLink;
if (s_config.feedStalenessSeconds < block.timestamp - timestamp && s_config.feedStalenessSeconds > 0) {
return s_config.fallbackNativePerUnitLink;
}
if (weiPerUnitLink <= 0) {
revert InvalidLinkWeiPrice(weiPerUnitLink);
Expand All @@ -124,6 +141,26 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
return SafeCast.toUint96((1e18 * amountWei) / getWeiPerUnitLink());
}

/// @inheritdoc IFunctionsBilling
function getUsdPerUnitLink() public view returns (uint256, uint8) {
(, int256 usdPerUnitLink, , uint256 timestamp, ) = s_linkToUsdFeed.latestRoundData();
// solhint-disable-next-line not-rely-on-time
if (s_config.feedStalenessSeconds < block.timestamp - timestamp && s_config.feedStalenessSeconds > 0) {
return (s_config.fallbackUsdPerUnitLink, s_config.fallbackUsdPerUnitLinkDecimals);
}
if (usdPerUnitLink <= 0) {
revert InvalidUsdLinkPrice(usdPerUnitLink);
}
return (uint256(usdPerUnitLink), s_linkToUsdFeed.decimals());
}

function _getJuelsFromUsd(uint256 amountUsd) private view returns (uint96) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to work through this math by hand to verify.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubled checked. I think this is good.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also have tests that triple and quadruple check this, right?

Copy link
Contributor Author

@justinkaseman justinkaseman Feb 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are in this PR, yes. They should also be reviewed.

(uint256 usdPerLink, uint8 decimals) = getUsdPerUnitLink();
// (usd) * (10**18 juels/link) * (10**decimals) / (link / usd) = juels
// There are only 1e9*1e18 = 1e27 juels in existence, should not exceed uint96 (2^96 ~ 7e28)
return SafeCast.toUint96((amountUsd * 10 ** (18 + decimals)) / usdPerLink);
}

// ================================================================
// | Cost Estimation |
// ================================================================
Expand All @@ -140,9 +177,10 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
if (gasPriceWei > REASONABLE_GAS_PRICE_CEILING) {
revert InvalidCalldata();
}
uint72 adminFee = getAdminFee();
uint72 donFee = getDONFee(data);
return _calculateCostEstimate(callbackGasLimit, gasPriceWei, donFee, adminFee);
uint72 adminFee = getAdminFeeJuels();
uint72 donFee = getDONFeeJuels(data);
uint72 operationFee = getOperationFeeJuels();
return _calculateCostEstimate(callbackGasLimit, gasPriceWei, donFee, adminFee, operationFee);
}

/// @notice Estimate the cost in Juels of LINK
Expand All @@ -151,8 +189,9 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
function _calculateCostEstimate(
uint32 callbackGasLimit,
uint256 gasPriceWei,
uint72 donFee,
uint72 adminFee
uint72 donFeeJuels,
uint72 adminFeeJuels,
uint72 operationFeeJuels
) internal view returns (uint96) {
// If gas price is less than the minimum fulfillment gas price, override to using the minimum
if (gasPriceWei < s_config.minimumEstimateGasPriceWei) {
Expand All @@ -167,7 +206,7 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
uint256 l1FeeWei = ChainSpecificUtil._getCurrentTxL1GasFees(msg.data);
uint96 estimatedGasReimbursementJuels = _getJuelsFromWei((gasPriceWithOverestimation * executionGas) + l1FeeWei);

uint96 feesJuels = uint96(donFee) + uint96(adminFee);
uint96 feesJuels = uint96(donFeeJuels) + uint96(adminFeeJuels) + uint96(operationFeeJuels);

return estimatedGasReimbursementJuels + feesJuels;
}
Expand All @@ -182,28 +221,28 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
/// @return commitment - The parameters of the request that must be held consistent at response time
function _startBilling(
FunctionsResponse.RequestMeta memory request
) internal returns (FunctionsResponse.Commitment memory commitment) {
FunctionsBillingConfig memory config = s_config;

) internal returns (FunctionsResponse.CommitmentWithOperationFee memory commitment) {
// Nodes should support all past versions of the structure
if (request.dataVersion > config.maxSupportedRequestDataVersion) {
if (request.dataVersion > s_config.maxSupportedRequestDataVersion) {
revert UnsupportedRequestDataVersion();
}

uint72 donFee = getDONFee(request.data);
uint72 donFee = getDONFeeJuels(request.data);
uint72 operationFee = getOperationFeeJuels();
uint96 estimatedTotalCostJuels = _calculateCostEstimate(
request.callbackGasLimit,
tx.gasprice,
donFee,
request.adminFee
request.adminFee,
operationFee
);

// Check that subscription can afford the estimated cost
if ((request.availableBalance) < estimatedTotalCostJuels) {
revert InsufficientBalance();
}

uint32 timeoutTimestamp = uint32(block.timestamp + config.requestTimeoutSeconds);
uint32 timeoutTimestamp = uint32(block.timestamp + s_config.requestTimeoutSeconds);
bytes32 requestId = keccak256(
abi.encode(
address(this),
Expand All @@ -220,18 +259,19 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
)
);

commitment = FunctionsResponse.Commitment({
adminFee: request.adminFee,
commitment = FunctionsResponse.CommitmentWithOperationFee({
adminFeeJuels: request.adminFee,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are deprecating adminFee (right?), could we save all this effort with a new Commitment struct by putting the new operationFee into adminFee field? Still passing a zero to the router so everything matches there. Not the most elegant approach but maybe a lot simpler?

Copy link
Contributor

@KuphJr KuphJr Feb 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hadn't considered this. I'd be very curious to know if this is possible as it would be great to eliminate the changes to the log_poller_wrapper.

coordinator: address(this),
client: request.requestingContract,
subscriptionId: request.subscriptionId,
callbackGasLimit: request.callbackGasLimit,
estimatedTotalCostJuels: estimatedTotalCostJuels,
timeoutTimestamp: timeoutTimestamp,
requestId: requestId,
donFee: donFee,
gasOverheadBeforeCallback: config.gasOverheadBeforeCallback,
gasOverheadAfterCallback: config.gasOverheadAfterCallback
donFeeJuels: donFee,
operationFeeJuels: operationFee,
gasOverheadBeforeCallback: s_config.gasOverheadBeforeCallback,
gasOverheadAfterCallback: s_config.gasOverheadAfterCallback
});

s_requestCommitments[requestId] = keccak256(abi.encode(commitment));
Expand All @@ -255,7 +295,10 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
bytes memory /* offchainMetadata TODO: use in getDonFee() for dynamic billing */,
uint8 reportBatchSize
) internal returns (FunctionsResponse.FulfillResult) {
FunctionsResponse.Commitment memory commitment = abi.decode(onchainMetadata, (FunctionsResponse.Commitment));
FunctionsResponse.CommitmentWithOperationFee memory commitment = abi.decode(
onchainMetadata,
(FunctionsResponse.CommitmentWithOperationFee)
);

uint256 gasOverheadWei = (commitment.gasOverheadBeforeCallback + commitment.gasOverheadAfterCallback) * tx.gasprice;
uint256 l1FeeShareWei = ChainSpecificUtil._getCurrentTxL1GasFees(msg.data) / reportBatchSize;
Expand All @@ -268,9 +311,21 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
response,
err,
juelsPerGas,
gasOverheadJuels + commitment.donFee, // cost without callback or admin fee, those will be added by the Router
gasOverheadJuels + commitment.donFeeJuels + commitment.operationFeeJuels, // cost without callback or admin fee, those will be added by the Router
msg.sender,
commitment
FunctionsResponse.Commitment({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a comment here explaining that we re-pack everything into an older Commitment struct compatible with the Router?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion, I agree that it'll make it more clear

adminFee: commitment.adminFeeJuels,
coordinator: commitment.coordinator,
client: commitment.client,
subscriptionId: commitment.subscriptionId,
callbackGasLimit: commitment.callbackGasLimit,
estimatedTotalCostJuels: commitment.estimatedTotalCostJuels,
timeoutTimestamp: commitment.timeoutTimestamp,
requestId: commitment.requestId,
donFee: commitment.donFeeJuels,
gasOverheadBeforeCallback: commitment.gasOverheadBeforeCallback,
gasOverheadAfterCallback: commitment.gasOverheadAfterCallback
})
);

// The router will only pay the DON on successfully processing the fulfillment
Expand All @@ -282,19 +337,22 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
) {
delete s_requestCommitments[requestId];
// Reimburse the transmitter for the fulfillment gas cost
s_withdrawableTokens[msg.sender] = gasOverheadJuels + callbackCostJuels;
s_withdrawableTokens[msg.sender] += gasOverheadJuels + callbackCostJuels;
// Put donFee into the pool of fees, to be split later
// Saves on storage writes that would otherwise be charged to the user
s_feePool += commitment.donFee;
s_feePool += commitment.donFeeJuels;
// Pay the operation fee to the Coordinator owner
s_withdrawableTokens[_owner()] += commitment.operationFeeJuels;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there already a proposal for concrete values of all 3 fees in prod? Can we discuss that in the Config sheet maybe?

Copy link
Contributor Author

@justinkaseman justinkaseman Feb 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's in another PR in tooling. It will be added to the Config sheet once finalized.

emit RequestBilled({
requestId: requestId,
juelsPerGas: juelsPerGas,
l1FeeShareWei: l1FeeShareWei,
callbackCostJuels: callbackCostJuels,
totalCostJuels: gasOverheadJuels + callbackCostJuels + commitment.donFee + commitment.adminFee
donFeeJuels: commitment.donFeeJuels,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this affect Atlas or other data collection? Any other event changes that we are coordinating (or will) with other teams?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will not. I already double checked that Atlas only uses Router events.

adminFeeJuels: commitment.adminFeeJuels,
operationFeeJuels: commitment.operationFeeJuels
});
}

return resultCode;
}

Expand Down Expand Up @@ -377,4 +435,7 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
function _isExistingRequest(bytes32 requestId) internal view returns (bool) {
return s_requestCommitments[requestId] != bytes32(0);
}

// Overriden in FunctionsCoordinator.sol
function _owner() internal view virtual returns (address owner);
}
42 changes: 30 additions & 12 deletions contracts/src/v0.8/functions/dev/v1_X/FunctionsCoordinator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilli

/// @inheritdoc ITypeAndVersion
// solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables
string public constant override typeAndVersion = "Functions Coordinator v1.2.0";
string public constant override typeAndVersion = "Functions Coordinator v2.0.0";

event OracleRequest(
bytes32 indexed requestId,
Expand All @@ -29,7 +29,7 @@ contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilli
uint16 dataVersion,
bytes32 flags,
uint64 callbackGasLimit,
FunctionsResponse.Commitment commitment
FunctionsResponse.CommitmentWithOperationFee commitment
);
event OracleResponse(bytes32 indexed requestId, address transmitter);

Expand All @@ -43,8 +43,9 @@ contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilli
constructor(
address router,
FunctionsBillingConfig memory config,
address linkToNativeFeed
) OCR2Base() FunctionsBilling(router, config, linkToNativeFeed) {}
address linkToNativeFeed,
address linkToUsdFeed
) OCR2Base() FunctionsBilling(router, config, linkToNativeFeed, linkToUsdFeed) {}

/// @inheritdoc IFunctionsCoordinator
function getThresholdPublicKey() external view override returns (bytes memory) {
Expand Down Expand Up @@ -80,10 +81,9 @@ contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilli

/// @dev check if node is in current transmitter list
function _isTransmitter(address node) internal view returns (bool) {
address[] memory nodes = s_transmitters;
// Bounded by "maxNumOracles" on OCR2Abstract.sol
for (uint256 i = 0; i < nodes.length; ++i) {
if (nodes[i] == node) {
for (uint256 i = 0; i < s_transmitters.length; ++i) {
if (s_transmitters[i] == node) {
return true;
}
}
Expand All @@ -93,11 +93,11 @@ contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilli
/// @inheritdoc IFunctionsCoordinator
function startRequest(
FunctionsResponse.RequestMeta calldata request
) external override onlyRouter returns (FunctionsResponse.Commitment memory commitment) {
commitment = _startBilling(request);
) external override onlyRouter returns (FunctionsResponse.Commitment memory) {
FunctionsResponse.CommitmentWithOperationFee memory commitmentWithOperationFee = _startBilling(request);

emit OracleRequest(
commitment.requestId,
commitmentWithOperationFee.requestId,
request.requestingContract,
// solhint-disable-next-line avoid-tx-origin
tx.origin,
Expand All @@ -107,10 +107,23 @@ contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilli
request.dataVersion,
request.flags,
request.callbackGasLimit,
commitment
commitmentWithOperationFee
);

return commitment;
return
FunctionsResponse.Commitment({
adminFee: commitmentWithOperationFee.adminFeeJuels,
coordinator: commitmentWithOperationFee.coordinator,
client: commitmentWithOperationFee.client,
subscriptionId: commitmentWithOperationFee.subscriptionId,
callbackGasLimit: commitmentWithOperationFee.callbackGasLimit,
estimatedTotalCostJuels: commitmentWithOperationFee.estimatedTotalCostJuels,
timeoutTimestamp: commitmentWithOperationFee.timeoutTimestamp,
requestId: commitmentWithOperationFee.requestId,
donFee: commitmentWithOperationFee.donFeeJuels,
gasOverheadBeforeCallback: commitmentWithOperationFee.gasOverheadBeforeCallback,
gasOverheadAfterCallback: commitmentWithOperationFee.gasOverheadAfterCallback
});
}

/// @dev DON fees are pooled together. If the OCR configuration is going to change, these need to be distributed.
Expand Down Expand Up @@ -205,4 +218,9 @@ contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilli
function _onlyOwner() internal view override {
_validateOwnership();
}

/// @dev Used in FunctionsBilling.sol
function _owner() internal view override returns (address owner) {
return this.owner();
}
}
Loading
Loading