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

Jump to bottom

[FUN-1094] Resolve informational finding from audit #11429

Closed
wants to merge 9 commits into from
Closed

[FUN-1094] Resolve informational finding from audit #11429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
48c8053
chore: correct comment mentioning MAX_CALLBACK_GAS_LIMIT_FLAGS_INDEX
agparadiso Nov 29, 2023
ea5e0b8
chore: rename costWithoutCallback to costWithoutFulfilment
agparadiso Nov 29, 2023
eca4862
chore: add comment about transmitter address supporting link
agparadiso Nov 30, 2023
17da090
fix: do not emit AddedAccess if the recipient already had access
agparadiso Nov 30, 2023
8926dc6
chore: extract config of FunctionsBilling and TermsOfServiceAllowList…
agparadiso Nov 30, 2023
f11cc02
chore: update wrappers
agparadiso Nov 30, 2023
8af9aae
chore: update snapshot
agparadiso Nov 30, 2023
297cc36
chore: no newline at end of file
agparadiso Nov 30, 2023
22c052f
chore: fix formatting
agparadiso Nov 30, 2023
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
42 changes: 21 additions & 21 deletions contracts/gas-snapshots/functions.gas-snapshot
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumGoerli() (gas: 14578318)
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumMainnet() (gas: 14578296)
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumSepolia() (gas: 14578312)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseGoerli() (gas: 14589732)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseMainnet() (gas: 14589709)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseSepolia() (gas: 14589681)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismGoerli() (gas: 14589632)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismMainnet() (gas: 14589621)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismSepolia() (gas: 14589665)
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumGoerli() (gas: 14580546)
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumMainnet() (gas: 14580524)
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumSepolia() (gas: 14580540)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseGoerli() (gas: 14591960)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseMainnet() (gas: 14591937)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseSepolia() (gas: 14591909)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismGoerli() (gas: 14591860)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismMainnet() (gas: 14591849)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismSepolia() (gas: 14591893)
FunctionsBilling_Constructor:test_Constructor_Success() (gas: 14812)
FunctionsBilling_DeleteCommitment:test_DeleteCommitment_RevertIfNotRouter() (gas: 13282)
FunctionsBilling_DeleteCommitment:test_DeleteCommitment_Success() (gas: 15897)
Expand Down Expand Up @@ -118,12 +118,12 @@ FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfMaximumConsumers() (
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfMaximumConsumersAfterConfigUpdate() (gas: 164837)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfNoSubscription() (gas: 12946)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfNotAllowedSender() (gas: 57809)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfNotSubscriptionOwner() (gas: 87162)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfNotSubscriptionOwner() (gas: 87190)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_RevertIfPaused() (gas: 18094)
FunctionsSubscriptions_AddConsumer:test_AddConsumer_Success() (gas: 95480)
FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_RevertIfNoSubscription() (gas: 15041)
FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_RevertIfNotAllowedSender() (gas: 57885)
FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_RevertIfNotSubscriptionOwner() (gas: 89272)
FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_RevertIfNotSubscriptionOwner() (gas: 89300)
FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_RevertIfPaused() (gas: 20148)
FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_RevertIfPendingRequests() (gas: 194325)
FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_SuccessForfeitAllBalanceAsDeposit() (gas: 114506)
Expand All @@ -142,11 +142,11 @@ FunctionsSubscriptions_GetSubscriptionsInRange:test_GetSubscriptionsInRange_Reve
FunctionsSubscriptions_GetSubscriptionsInRange:test_GetSubscriptionsInRange_RevertIfStartIsAfterEnd() (gas: 13459)
FunctionsSubscriptions_GetSubscriptionsInRange:test_GetSubscriptionsInRange_Success() (gas: 59592)
FunctionsSubscriptions_GetTotalBalance:test_GetTotalBalance_Success() (gas: 15010)
FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoCalldata(uint96) (runs: 256, μ: 43508, ~: 45548)
FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoSubscription(uint96) (runs: 256, μ: 46020, ~: 48060)
FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoCalldata(uint96) (runs: 256, μ: 43774, ~: 45548)
FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoSubscription(uint96) (runs: 256, μ: 46286, ~: 48060)
FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNotLink(uint96) (runs: 256, μ: 14295, ~: 14295)
FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfPaused(uint96) (runs: 256, μ: 51089, ~: 53040)
FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_Success(uint96) (runs: 256, μ: 86057, ~: 89604)
FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfPaused(uint96) (runs: 256, μ: 51266, ~: 53040)
FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_Success(uint96) (runs: 256, μ: 86234, ~: 89604)
FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfAmountMoreThanBalance() (gas: 20745)
FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfBalanceInvariant() (gas: 189)
FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfNoAmount() (gas: 15638)
Expand Down Expand Up @@ -181,7 +181,7 @@ FunctionsSubscriptions_RecoverFunds:test_RecoverFunds_Success(uint64) (runs: 256
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfInvalidConsumer() (gas: 30260)
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfNoSubscription() (gas: 15019)
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfNotAllowedSender() (gas: 57800)
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfNotSubscriptionOwner() (gas: 87208)
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfNotSubscriptionOwner() (gas: 87236)
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfPaused() (gas: 18049)
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_RevertIfPendingRequests() (gas: 191858)
FunctionsSubscriptions_RemoveConsumer:test_RemoveConsumer_Success() (gas: 41979)
Expand All @@ -195,14 +195,14 @@ FunctionsSubscriptions_TimeoutRequests:test_TimeoutRequests_Success() (gas: 5775
FunctionsSubscriptions_createSubscription:test_CreateSubscription_RevertIfNotAllowedSender() (gas: 26390)
FunctionsSubscriptions_createSubscription:test_CreateSubscription_RevertIfPaused() (gas: 15759)
FunctionsSubscriptions_createSubscription:test_CreateSubscription_Success() (gas: 152576)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:testAcceptTermsOfService_InvalidSigner_vuln() (gas: 94815)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:testAcceptTermsOfService_InvalidSigner_vuln() (gas: 94843)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_RevertIfAcceptorIsNotSender() (gas: 25837)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_RevertIfBlockedSender() (gas: 44348)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_RevertIfInvalidSigner() (gas: 23597)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_RevertIfRecipientContractIsNotSender() (gas: 1866530)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_RevertIfRecipientIsNotSender() (gas: 26003)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_SuccessIfAcceptingForContract() (gas: 1946547)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_SuccessIfAcceptingForSelf() (gas: 104851)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_SuccessIfAcceptingForContract() (gas: 1946575)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_SuccessIfAcceptingForSelf() (gas: 103437)
FunctionsTermsOfServiceAllowList_BlockSender:test_BlockSender_RevertIfNotOwner() (gas: 15469)
FunctionsTermsOfServiceAllowList_BlockSender:test_BlockSender_Success() (gas: 51794)
FunctionsTermsOfServiceAllowList_Constructor:test_Constructor_Success() (gas: 12187)
Expand All @@ -214,10 +214,10 @@ FunctionsTermsOfServiceAllowList_HasAccess:test_HasAccess_TrueWhenDisabled() (ga
FunctionsTermsOfServiceAllowList_IsBlockedSender:test_IsBlockedSender_SuccessFalse() (gas: 15354)
FunctionsTermsOfServiceAllowList_IsBlockedSender:test_IsBlockedSender_SuccessTrue() (gas: 41957)
FunctionsTermsOfServiceAllowList_UnblockSender:test_UnblockSender_RevertIfNotOwner() (gas: 13525)
FunctionsTermsOfServiceAllowList_UnblockSender:test_UnblockSender_Success() (gas: 95184)
FunctionsTermsOfServiceAllowList_UnblockSender:test_UnblockSender_Success() (gas: 95212)
FunctionsTermsOfServiceAllowList_UpdateConfig:test_UpdateConfig_RevertIfNotOwner() (gas: 13727)
FunctionsTermsOfServiceAllowList_UpdateConfig:test_UpdateConfig_Success() (gas: 22073)
Gas_AcceptTermsOfService:test_AcceptTermsOfService_Gas() (gas: 84675)
Gas_AcceptTermsOfService:test_AcceptTermsOfService_Gas() (gas: 84703)
Gas_AddConsumer:test_AddConsumer_Gas() (gas: 79087)
Gas_CreateSubscription:test_CreateSubscription_Gas() (gas: 73375)
Gas_FundSubscription:test_FundSubscription_Gas() (gas: 38546)
Expand Down
34 changes: 9 additions & 25 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 @@ -3,7 +3,7 @@ pragma solidity ^0.8.19;

import {IFunctionsSubscriptions} from "./interfaces/IFunctionsSubscriptions.sol";
import {AggregatorV3Interface} from "../../../shared/interfaces/AggregatorV3Interface.sol";
import {IFunctionsBilling} from "./interfaces/IFunctionsBilling.sol";
import {IFunctionsBilling, FunctionsBillingConfig} from "./interfaces/IFunctionsBilling.sol";

import {Routable} from "./Routable.sol";
import {FunctionsResponse} from "./libraries/FunctionsResponse.sol";
Expand Down Expand Up @@ -37,25 +37,9 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {

event CommitmentDeleted(bytes32 requestId);

// ================================================================
// | Configuration state |
// ================================================================

struct Config {
uint32 fulfillmentGasPriceOverEstimationBP; // ══╗ Percentage of gas price overestimation to account for changes in gas price between request and response. Held as basis points (one hundredth of 1 percentage point)
uint32 feedStalenessSeconds; // ║ How long before we consider the feed price to be stale and fallback to fallbackNativePerUnitLink.
uint32 gasOverheadBeforeCallback; // ║ Represents the average gas execution cost before the fulfillment callback. This amount is always billed for every request.
uint32 gasOverheadAfterCallback; // ║ Represents the average gas execution cost after the fulfillment callback. This amount is always billed for every request.
uint72 donFee; // ║ Additional flat fee (in Juels of LINK) that will be split between Node Operators. Max value is 2^80 - 1 == 1.2m LINK.
uint40 minimumEstimateGasPriceWei; // ║ The lowest amount of wei that will be used as the tx.gasprice when estimating the cost to fulfill the request
uint16 maxSupportedRequestDataVersion; // ═══════╝ The highest support request data version supported by the node. All lower versions should also be supported.
uint224 fallbackNativePerUnitLink; // ═══════════╗ Fallback NATIVE CURRENCY / LINK conversion rate if the data feed is stale
uint32 requestTimeoutSeconds; // ════════════════╝ How many seconds it takes before we consider a request to be timed out
}

Config private s_config;
FunctionsBillingConfig private s_config;

event ConfigUpdated(Config config);
event ConfigUpdated(FunctionsBillingConfig config);

error UnsupportedRequestDataVersion();
error InsufficientBalance();
Expand All @@ -81,7 +65,7 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
// ================================================================
// | Initialization |
// ================================================================
constructor(address router, Config memory config, address linkToNativeFeed) Routable(router) {
constructor(address router, FunctionsBillingConfig memory config, address linkToNativeFeed) Routable(router) {
s_linkToNativeFeed = AggregatorV3Interface(linkToNativeFeed);

updateConfig(config);
Expand All @@ -93,13 +77,13 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {

/// @notice Gets the Chainlink Coordinator's billing configuration
/// @return config
function getConfig() external view returns (Config memory) {
function getConfig() external view returns (FunctionsBillingConfig memory) {
return s_config;
}

/// @notice Sets the Chainlink Coordinator's billing configuration
/// @param config - See the contents of the Config struct in IFunctionsBilling.Config for more information
function updateConfig(Config memory config) public {
/// @param config - See the contents of the Config struct in FunctionsBillingConfig for more information
function updateConfig(FunctionsBillingConfig memory config) public {
_onlyOwner();

s_config = config;
Expand All @@ -122,7 +106,7 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {

/// @inheritdoc IFunctionsBilling
function getWeiPerUnitLink() public view returns (uint256) {
Config memory config = s_config;
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) {
Expand Down Expand Up @@ -199,7 +183,7 @@ abstract contract FunctionsBilling is Routable, IFunctionsBilling {
function _startBilling(
FunctionsResponse.RequestMeta memory request
) internal returns (FunctionsResponse.Commitment memory commitment) {
Config memory config = s_config;
FunctionsBillingConfig memory config = s_config;

// Nodes should support all past versions of the structure
if (request.dataVersion > config.maxSupportedRequestDataVersion) {
Expand Down
4 changes: 2 additions & 2 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 @@ -4,7 +4,7 @@ pragma solidity ^0.8.19;
import {IFunctionsCoordinator} from "./interfaces/IFunctionsCoordinator.sol";
import {ITypeAndVersion} from "../../../shared/interfaces/ITypeAndVersion.sol";

import {FunctionsBilling} from "./FunctionsBilling.sol";
import {FunctionsBilling, FunctionsBillingConfig} from "./FunctionsBilling.sol";
import {OCR2Base} from "./ocr/OCR2Base.sol";
import {FunctionsResponse} from "./libraries/FunctionsResponse.sol";

Expand Down Expand Up @@ -42,7 +42,7 @@ contract FunctionsCoordinator is OCR2Base, IFunctionsCoordinator, FunctionsBilli

constructor(
address router,
Config memory config,
FunctionsBillingConfig memory config,
address linkToNativeFeed
) OCR2Base() FunctionsBilling(router, config, linkToNativeFeed) {}

Expand Down
8 changes: 4 additions & 4 deletions contracts/src/v0.8/functions/dev/v1_X/FunctionsRouter.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable,
uint72 adminFee; // ║ Flat fee (in Juels of LINK) that will be paid to the Router owner for operation of the network
bytes4 handleOracleFulfillmentSelector; // ║ The function selector that is used when calling back to the Client contract
uint16 gasForCallExactCheck; // ════════════════╝ Used during calling back to the client. Ensures we have at least enough gas to be able to revert if gasAmount > 63//64*gas available.
uint32[] maxCallbackGasLimits; // ══════════════╸ List of max callback gas limits used by flag with GAS_FLAG_INDEX
uint32[] maxCallbackGasLimits; // ══════════════╸ List of max callback gas limits used by flag with MAX_CALLBACK_GAS_LIMIT_FLAGS_INDEX
uint16 subscriptionDepositMinimumRequests; //═══╗ Amount of requests that must be completed before the full subscription balance will be released when closing a subscription account.
uint72 subscriptionDepositJuels; // ════════════╝ Amount of subscription funds that are held as a deposit until Config.subscriptionDepositMinimumRequests are made using the subscription.
}
Expand Down Expand Up @@ -306,7 +306,7 @@ contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable,
bytes memory response,
bytes memory err,
uint96 juelsPerGas,
uint96 costWithoutCallback,
uint96 costWithoutFulfilment,
address transmitter,
FunctionsResponse.Commitment memory commitment
) external override returns (FunctionsResponse.FulfillResult resultCode, uint96) {
Expand Down Expand Up @@ -341,7 +341,7 @@ contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable,

{
uint96 callbackCost = juelsPerGas * SafeCast.toUint96(commitment.callbackGasLimit);
uint96 totalCostJuels = commitment.adminFee + costWithoutCallback + callbackCost;
uint96 totalCostJuels = commitment.adminFee + costWithoutFulfilment + callbackCost;

// Check that the subscription can still afford to fulfill the request
if (totalCostJuels > getSubscription(commitment.subscriptionId).balance) {
Expand Down Expand Up @@ -379,7 +379,7 @@ contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable,
commitment.adminFee,
juelsPerGas,
SafeCast.toUint96(result.gasUsed),
costWithoutCallback
costWithoutFulfilment
);

emit RequestProcessed({
Expand Down
Loading
Loading