Skip to content

Commit

Permalink
[FUN-1094] Minor fixes (#11434)
Browse files Browse the repository at this point in the history
* fix: do not emit AddedAccess if the recipient already had access & minor doc improvements

* chore: save gas by avoiding declaring a new var
  • Loading branch information
agparadiso authored Dec 6, 2023
1 parent 346448e commit 4d8e093
Show file tree
Hide file tree
Showing 19 changed files with 124 additions and 113 deletions.
40 changes: 20 additions & 20 deletions contracts/gas-snapshots/functions.gas-snapshot
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: 14579333)
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumMainnet() (gas: 14579311)
ChainSpecificUtil__getCurrentTxL1GasFees_Arbitrum:test__getCurrentTxL1GasFees_SuccessWhenArbitrumSepolia() (gas: 14579327)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseGoerli() (gas: 14590747)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseMainnet() (gas: 14590724)
ChainSpecificUtil__getCurrentTxL1GasFees_Base:test__getCurrentTxL1GasFees_SuccessWhenBaseSepolia() (gas: 14590696)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismGoerli() (gas: 14590647)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismMainnet() (gas: 14590636)
ChainSpecificUtil__getCurrentTxL1GasFees_Optimism:test__getCurrentTxL1GasFees_SuccessWhenOptimismSepolia() (gas: 14590680)
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: 87177)
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: 89287)
FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_RevertIfPaused() (gas: 20148)
FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_RevertIfPendingRequests() (gas: 194325)
FunctionsSubscriptions_CancelSubscription:test_CancelSubscription_SuccessForfeitAllBalanceAsDeposit() (gas: 114506)
Expand All @@ -142,10 +142,10 @@ 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_RevertIfPaused(uint96) (runs: 256, μ: 51443, ~: 53040)
FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_Success(uint96) (runs: 256, μ: 86057, ~: 89604)
FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfAmountMoreThanBalance() (gas: 20745)
FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfBalanceInvariant() (gas: 189)
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: 87223)
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: 94830)
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: 1946562)
FunctionsTermsOfServiceAllowList_AcceptTermsOfService:test_AcceptTermsOfService_SuccessIfAcceptingForSelf() (gas: 103411)
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: 95199)
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: 84690)
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
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 FunctionsBillingConfig struct in IFunctionsBilling.sol 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
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 All @@ -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.1.0";
string public constant override typeAndVersion = "Functions Coordinator v1.2.0";

event OracleRequest(
bytes32 indexed requestId,
Expand All @@ -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
10 changes: 5 additions & 5 deletions contracts/src/v0.8/functions/dev/v1_X/FunctionsRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ contract FunctionsRouter is IFunctionsRouter, FunctionsSubscriptions, Pausable,
using FunctionsResponse for FunctionsResponse.FulfillResult;

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

// We limit return data to a selector plus 4 words. This is to avoid
// malicious contracts from returning large amounts of data and causing
Expand Down 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 costWithoutFulfillment,
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 + costWithoutFulfillment + 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
costWithoutFulfillment
);

emit RequestProcessed({
Expand Down
Loading

0 comments on commit 4d8e093

Please sign in to comment.