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.

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

VRF V2.5 gas optimisations #11932

Merged
merged 7 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
39 changes: 21 additions & 18 deletions contracts/src/v0.8/vrf/dev/SubscriptionAPI.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,25 @@ abstract contract SubscriptionAPI is ConfirmedOwner, IERC677Receiver, IVRFSubscr

/// @dev may not be provided upon construction on some chains due to lack of availability
LinkTokenInterface public LINK;
// s_totalBalance tracks the total link sent to/from
// this contract through onTokenTransfer, cancelSubscription and oracleWithdraw.
// A discrepancy with this contract's link balance indicates someone
// sent tokens using transfer and so we may need to use recoverFunds.
uint96 public s_totalBalance;

/// @dev may not be provided upon construction on some chains due to lack of availability
AggregatorV3Interface public LINK_NATIVE_FEED;
// s_totalNativeBalance tracks the total native sent to/from
// this contract through fundSubscription, cancelSubscription and oracleWithdrawNative.
// A discrepancy with this contract's native balance indicates someone
// sent native using transfer and so we may need to use recoverNativeFunds.
uint96 public s_totalNativeBalance;

uint96 internal s_withdrawableTokens;
uint96 internal s_withdrawableNative;
// subscription nonce used to construct subId. Rises monotonically
uint64 public s_currentSubNonce;
Copy link
Contributor

Choose a reason for hiding this comment

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

as discussed offline, I think this results in gas optimization for other code paths but not hot paths (request/fulfillment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah VRF doesn't have Foundry tests? A quick snapshot would give you gas for every path :D (yes I'm trying to convert you, the last project that still used HH!)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Upon running:

$ forge snapshot --gas-report > gas_report.txt
$ git checkout ec1479e07de09340239a8ab1b7a6a80dce22d31f
$ forge snapshot --snap .develop-gas-snapshot
$ forge snapshot --diff .develop-gas-snapshot
Running 2 tests for test/v0.8/foundry/vrf/VRFV2Wrapper_Migration.t.sol:VRFV2PlusWrapperTest
[PASS] testMigrateWrapperLINKPayment() (gas: 540249)
[PASS] testMigrateWrapperNativePayment() (gas: 487257)
Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 2.58ms

Running 3 tests for test/v0.8/foundry/vrf/VRFV2Wrapper.t.sol:VRFV2PlusWrapperTest
[PASS] testRequestAndFulfillRandomWordsLINKWrapper() (gas: 367113)
[PASS] testRequestAndFulfillRandomWordsNativeWrapper() (gas: 299229)
[PASS] testSetLinkAndLinkNativeFeed() (gas: 3508860)
Test result: ok. 3 passed; 0 failed; 0 skipped; finished in 1.94ms

Running 11 tests for test/v0.8/foundry/vrf/VRFV2Plus.t.sol:VRFV2Plus
[PASS] testCancelSubWithNoLink() (gas: 160553)
[PASS] testCreateSubscription() (gas: 164143)
[PASS] testDeregisterProvingKey() (gas: 86749)
[PASS] testGetActiveSubscriptionIds() (gas: 3460238)
[PASS] testRegisterProvingKey() (gas: 101470)
[PASS] testRequestAndFulfillRandomWordsLINK() (gas: 725068)
[PASS] testRequestAndFulfillRandomWordsNative() (gas: 671454)
[PASS] testRequestAndFulfillRandomWords_NetworkGasPriceExceedsGasLane() (gas: 597450)
[PASS] testRequestAndFulfillRandomWords_OnlyPremium_LinkPayment() (gas: 725502)
[PASS] testRequestAndFulfillRandomWords_OnlyPremium_NativePayment() (gas: 671928)
[PASS] testSetConfig() (gas: 67933)
Test result: ok. 11 passed; 0 failed; 0 skipped; finished in 4.91ms

Running 14 tests for test/v0.8/foundry/vrf/VRFCoordinatorV2Mock.t.sol:VRFCoordinatorV2MockTest
[PASS] testAddConsumer() (gas: 113218)
[PASS] testAddConsumerToInvalidSub() (gas: 16216)
[PASS] testAddMaxConsumers() (gas: 4706289)
[PASS] testCancelSubscription() (gas: 56244)
[PASS] testCreateSubscription() (gas: 92220)
[PASS] testFundInvalidSubscription() (gas: 16206)
[PASS] testFundSubscription() (gas: 70523)
[PASS] testRemoveConsumerAgain() (gas: 94336)
[PASS] testRemoveConsumerFromInvalidSub() (gas: 16183)
[PASS] testRemoveConsumerFromSub() (gas: 92264)
[PASS] testRequestRandomWordsHappyPath() (gas: 242705)
[PASS] testRequestRandomWordsInsufficientFunds() (gas: 160571)
[PASS] testRequestRandomWordsInvalidConsumer() (gas: 70829)
[PASS] testRequestRandomWordsUserOverride() (gas: 229957)
Test result: ok. 14 passed; 0 failed; 0 skipped; finished in 11.60ms

Running 2 tests for test/v0.8/foundry/vrf/TrustedBlockhashStore.t.sol:TrustedBlockhashStoreTest
[PASS] testGenericBHSFunctions() (gas: 53684)
[PASS] testTrustedBHSFunctions() (gas: 99343)
Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 24.41ms

Running 8 tests for test/v0.8/foundry/vrf/ChainSpecificUtil.t.sol:ChainSpecificUtilTest
[PASS] testGetBlockNumberArbitrum() (gas: 7930)
[PASS] testGetBlockNumberOptimism() (gas: 419)
[PASS] testGetBlockhashArbitrum() (gas: 18896)
[PASS] testGetBlockhashOptimism() (gas: 612)
[PASS] testGetCurrentTxL1GasFeesArbitrum() (gas: 10586)
[PASS] testGetCurrentTxL1GasFeesOptimism() (gas: 39204)
[PASS] testGetL1CalldataGasCostArbitrum() (gas: 12278)
[PASS] testGetL1CalldataGasCostOptimism() (gas: 45883)
Test result: ok. 8 passed; 0 failed; 0 skipped; finished in 12.26ms
Running 2 tests for test/v0.8/foundry/vrf/TrustedBlockhashStore.t.sol:TrustedBlockhashStoreTest
[PASS] testGenericBHSFunctions() (gas: 53684)
[PASS] testTrustedBHSFunctions() (gas: 99343)
Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 24.41ms

Running 8 tests for test/v0.8/foundry/vrf/ChainSpecificUtil.t.sol:ChainSpecificUtilTest
[PASS] testGetBlockNumberArbitrum() (gas: 7930)
[PASS] testGetBlockNumberOptimism() (gas: 419)
[PASS] testGetBlockhashArbitrum() (gas: 18896)
[PASS] testGetBlockhashOptimism() (gas: 612)
[PASS] testGetCurrentTxL1GasFeesArbitrum() (gas: 10586)
[PASS] testGetCurrentTxL1GasFeesOptimism() (gas: 39204)
[PASS] testGetL1CalldataGasCostArbitrum() (gas: 12278)
[PASS] testGetL1CalldataGasCostOptimism() (gas: 45883)
Test result: ok. 8 passed; 0 failed; 0 skipped; finished in 12.26ms

Running 32 tests for test/v0.8/foundry/vrf/VRFV2PlusSubscriptionAPI.t.sol:VRFV2PlusSubscriptionAPITest
[PASS] testAddConsumer() (gas: 226772)
[PASS] testAddConsumerReaddSameConsumer() (gas: 231240)
[PASS] testAddConsumerTooManyConsumers() (gas: 5402706)
[PASS] testCreateSubscription() (gas: 149655)
[PASS] testCreateSubscriptionRecreate() (gas: 219584)                                                                                                            [83/12605]
[PASS] testDefaultState() (gas: 20161)
[PASS] testFundSubscriptionWithNative() (gas: 199347)
[PASS] testFundSubscriptionWithNativeInvalidSubscriptionId() (gas: 26350)
[PASS] testOnTokenTransferCallerNotLink() (gas: 15389)
[PASS] testOnTokenTransferInvalidCalldata() (gas: 324431)
[PASS] testOnTokenTransferInvalidSubscriptionId() (gas: 326632)
[PASS] testOnTokenTransferSuccess() (gas: 484262)
[PASS] testOwnerCancelSubscriptionLinkFundsOnly() (gas: 415139)
[PASS] testOwnerCancelSubscriptionNativeAndLinkFunds() (gas: 452297)
[PASS] testOwnerCancelSubscriptionNativeFundsOnly() (gas: 175888)
[PASS] testOwnerCancelSubscriptionNoFunds() (gas: 126259)
[PASS] testRecoverFundsAmountToTransfer() (gas: 307417)
[PASS] testRecoverFundsBalanceInvariantViolated() (gas: 302392)
[PASS] testRecoverFundsLINKNotSet() (gas: 12856)
[PASS] testRecoverFundsNothingToTransfer() (gas: 487295)
[PASS] testRecoverNativeFundsAmountToTransfer() (gas: 20650)
[PASS] testRecoverNativeFundsBalanceInvariantViolated() (gas: 34603)
[PASS] testRecoverNativeFundsNothingToTransfer() (gas: 204137)
[PASS] testSetLINKAndLINKNativeFeed() (gas: 62660)
[PASS] testSubscriptionOwnershipTransfer() (gas: 160656)
[PASS] testWithdrawInsufficientBalance() (gas: 301593)
[PASS] testWithdrawLinkInvalidOwner() (gas: 19372)
[PASS] testWithdrawNativeInsufficientBalance() (gas: 19622)
[PASS] testWithdrawNativeInvalidOwner() (gas: 19348)
[PASS] testWithdrawNativeSufficientBalance() (gas: 54570)
[PASS] testWithdrawNoLink() (gas: 14997)
[PASS] testWithdrawSufficientBalanceLinkSet() (gas: 341143)
Test result: ok. 32 passed; 0 failed; 0 skipped; finished in 17.97ms

Running 7 tests for test/v0.8/foundry/vrf/VRFCoordinatorV2Plus_Migration.t.sol:VRFCoordinatorV2Plus_Migration
[PASS] testDeregister() (gas: 101372)
[PASS] testMigrateRevertsWhenInvalidCaller() (gas: 33273)
[PASS] testMigrateRevertsWhenInvalidCoordinator() (gas: 19960)
[PASS] testMigrateRevertsWhenPendingFulfillment() (gas: 237917)
[PASS] testMigrateRevertsWhenReentrant() (gas: 358016)
[PASS] testMigration() (gas: 465964)
[PASS] testMigrationNoLink() (gas: 436780)
Test result: ok. 7 passed; 0 failed; 0 skipped; finished in 16.22ms
Test result: ok. 7 passed; 0 failed; 0 skipped; finished in 16.22ms

Ran 8 test suites: 79 tests passed, 0 failed, 0 skipped (79 total tests)
testGetBlockNumberArbitrum() (gas: 0 (0.000%))
testGetBlockNumberOptimism() (gas: 0 (0.000%))
testGetBlockhashArbitrum() (gas: 0 (0.000%))
testGetBlockhashOptimism() (gas: 0 (0.000%))                                                                                                                     [39/12605]
testGetCurrentTxL1GasFeesArbitrum() (gas: 0 (0.000%))
testGetCurrentTxL1GasFeesOptimism() (gas: 0 (0.000%))
testGetL1CalldataGasCostArbitrum() (gas: 0 (0.000%))
testGetL1CalldataGasCostOptimism() (gas: 0 (0.000%))
testGenericBHSFunctions() (gas: 0 (0.000%))
testTrustedBHSFunctions() (gas: 0 (0.000%))
testAddConsumer() (gas: 0 (0.000%))
testAddConsumerToInvalidSub() (gas: 0 (0.000%))
testAddMaxConsumers() (gas: 0 (0.000%))
testCancelSubscription() (gas: 0 (0.000%))
testCreateSubscription() (gas: 0 (0.000%))
testFundInvalidSubscription() (gas: 0 (0.000%))
testFundSubscription() (gas: 0 (0.000%))
testRemoveConsumerAgain() (gas: 0 (0.000%))
testRemoveConsumerFromInvalidSub() (gas: 0 (0.000%))
testRemoveConsumerFromSub() (gas: 0 (0.000%))
testRequestRandomWordsHappyPath() (gas: 0 (0.000%))
testRequestRandomWordsInsufficientFunds() (gas: 0 (0.000%))
testRequestRandomWordsInvalidConsumer() (gas: 0 (0.000%))
testRequestRandomWordsUserOverride() (gas: 0 (0.000%))
testDeregister() (gas: 0 (0.000%))
testMigrateRevertsWhenInvalidCaller() (gas: 0 (0.000%))
testMigrateRevertsWhenInvalidCoordinator() (gas: 0 (0.000%))
testRecoverFundsLINKNotSet() (gas: 0 (0.000%))
testWithdrawLinkInvalidOwner() (gas: 0 (0.000%))
testWithdrawNoLink() (gas: 0 (0.000%))
testAddConsumerTooManyConsumers() (gas: 103 (0.002%))
testSetLinkAndLinkNativeFeed() (gas: 103 (0.003%))
testMigrationNoLink() (gas: -14 (-0.003%))
testMigrateRevertsWhenPendingFulfillment() (gas: 22 (0.009%))
testCreateSubscription() (gas: -17 (-0.011%))
testOnTokenTransferInvalidSubscriptionId() (gas: -44 (-0.013%))
testOnTokenTransferInvalidCalldata() (gas: -44 (-0.014%))
testDeregisterProvingKey() (gas: 17 (0.020%))
testWithdrawInsufficientBalance() (gas: -66 (-0.022%))
testMigrateWrapperLINKPayment() (gas: 151 (0.028%))
testMigrateWrapperNativePayment() (gas: 151 (0.031%))
testAddConsumer() (gas: -95 (-0.042%))
testRegisterProvingKey() (gas: 44 (0.043%))
testWithdrawNativeSufficientBalance() (gas: 27 (0.050%))
testMigrateRevertsWhenReentrant() (gas: 197 (0.055%))
testSubscriptionOwnershipTransfer() (gas: -94 (-0.058%))
testRecoverNativeFundsNothingToTransfer() (gas: 158 (0.077%))
testFundSubscriptionWithNative() (gas: 158 (0.079%))
testCancelSubWithNoLink() (gas: 143 (0.089%))
testAddConsumerReaddSameConsumer() (gas: -227 (-0.098%))
testRecoverNativeFundsBalanceInvariantViolated() (gas: 34 (0.098%))
testRecoverNativeFundsAmountToTransfer() (gas: 22 (0.107%))
testOwnerCancelSubscriptionNativeFundsOnly() (gas: 188 (0.107%))
testWithdrawNativeInsufficientBalance() (gas: 22 (0.112%))
testWithdrawNativeInvalidOwner() (gas: 22 (0.114%))
testGetActiveSubscriptionIds() (gas: 4681 (0.135%))
testOnTokenTransferCallerNotLink() (gas: 22 (0.143%))
testCreateSubscriptionRecreate() (gas: 323 (0.147%))
testOwnerCancelSubscriptionNoFunds() (gas: 207 (0.164%))
testFundSubscriptionWithNativeInvalidSubscriptionId() (gas: -45 (-0.170%))
testSetConfig() (gas: -180 (-0.264%))
testSetLINKAndLINKNativeFeed() (gas: -177 (-0.282%))
testOwnerCancelSubscriptionNativeAndLinkFunds() (gas: -1350 (-0.298%))
testRecoverFundsAmountToTransfer() (gas: -2049 (-0.662%))
testMigration() (gas: -11356 (-2.379%))
testRequestAndFulfillRandomWords_NetworkGasPriceExceedsGasLane() (gas: -16909 (-2.752%))
testOwnerCancelSubscriptionLinkFundsOnly() (gas: -17251 (-3.990%))
testRecoverFundsNothingToTransfer() (gas: -21693 (-4.262%))
testOnTokenTransferSuccess() (gas: -21641 (-4.278%))
testRequestAndFulfillRandomWordsLINKWrapper() (gas: -18934 (-4.905%))
testRequestAndFulfillRandomWords_OnlyPremium_LinkPayment() (gas: -40981 (-5.347%))
testRequestAndFulfillRandomWordsLINK() (gas: -40981 (-5.350%))
testRequestAndFulfillRandomWordsNativeWrapper() (gas: -17101 (-5.406%))
testRequestAndFulfillRandomWords_OnlyPremium_NativePayment() (gas: -39141 (-5.505%))
testRequestAndFulfillRandomWordsNative() (gas: -39141 (-5.508%))
testWithdrawSufficientBalanceLinkSet() (gas: -21821 (-6.012%))
testRecoverFundsBalanceInvariantViolated() (gas: -21901 (-6.753%))
testDefaultState() (gas: -2053 (-9.242%))
testCreateSubscription() (gas: -17042 (-9.406%))
Overall gas change: -345553 (-1.078%)


// We need to maintain a list of consuming addresses.
// This bound ensures we are able to loop over them as needed.
// Should a user require more consumers, they can use multiple subscriptions.
uint16 public constant MAX_CONSUMERS = 100;
error TooManyConsumers();
error InsufficientBalance();
error InvalidConsumer(uint256 subId, address consumer);
Expand Down Expand Up @@ -64,27 +76,18 @@ abstract contract SubscriptionAPI is ConfirmedOwner, IERC677Receiver, IVRFSubscr
mapping(address => mapping(uint256 => uint64)) /* consumer */ /* subId */ /* nonce */ internal s_consumers;
mapping(uint256 => SubscriptionConfig) /* subId */ /* subscriptionConfig */ internal s_subscriptionConfigs;
mapping(uint256 => Subscription) /* subId */ /* subscription */ internal s_subscriptions;
// subscription nonce used to construct subId. Rises monotonically
uint64 public s_currentSubNonce;
// track all subscription id's that were created by this contract
// note: access should be through the getActiveSubscriptionIds() view function
// which takes a starting index and a max number to fetch in order to allow
// "pagination" of the subscription ids. in the event a very large number of
// subscription id's are stored in this set, they cannot be retrieved in a
// single RPC call without violating various size limits.
EnumerableSet.UintSet internal s_subIds;
// s_totalBalance tracks the total link sent to/from
// this contract through onTokenTransfer, cancelSubscription and oracleWithdraw.
// A discrepancy with this contract's link balance indicates someone
// sent tokens using transfer and so we may need to use recoverFunds.
uint96 public s_totalBalance;
// s_totalNativeBalance tracks the total native sent to/from
// this contract through fundSubscription, cancelSubscription and oracleWithdrawNative.
// A discrepancy with this contract's native balance indicates someone
// sent native using transfer and so we may need to use recoverNativeFunds.
uint96 public s_totalNativeBalance;
uint96 internal s_withdrawableTokens;
uint96 internal s_withdrawableNative;

// We need to maintain a list of consuming addresses.
// This bound ensures we are able to loop over them as needed.
// Should a user require more consumers, they can use multiple subscriptions.
uint16 public constant MAX_CONSUMERS = 100;

event SubscriptionCreated(uint256 indexed subId, address owner);
event SubscriptionFunded(uint256 indexed subId, uint256 oldBalance, uint256 newBalance);
Expand Down
14 changes: 10 additions & 4 deletions contracts/src/v0.8/vrf/dev/VRFCoordinatorV2_5.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {IVRFCoordinatorV2PlusMigration} from "./interfaces/IVRFCoordinatorV2Plus
// solhint-disable-next-line no-unused-import
import {IVRFCoordinatorV2Plus, IVRFSubscriptionV2Plus} from "./interfaces/IVRFCoordinatorV2Plus.sol";

// import {console} from "forge-std/console.sol";

// solhint-disable-next-line contract-name-camelcase
contract VRFCoordinatorV2_5 is VRF, SubscriptionAPI, IVRFCoordinatorV2Plus {
/// @dev should always be available
Expand Down Expand Up @@ -39,8 +41,8 @@ contract VRFCoordinatorV2_5 is VRF, SubscriptionAPI, IVRFCoordinatorV2Plus {
error InvalidExtraArgsTag();
error GasPriceExceeded(uint256 gasPrice, uint256 maxGas);
struct RequestCommitment {
uint64 blockNum;
uint256 subId;
uint64 blockNum;
uint32 callbackGasLimit;
uint32 numWords;
address sender;
Expand Down Expand Up @@ -468,21 +470,25 @@ contract VRFCoordinatorV2_5 is VRF, SubscriptionAPI, IVRFCoordinatorV2Plus {
// Include payment in the event for tracking costs.
emit RandomWordsFulfilled(output.requestId, output.randomness, rc.subId, payment, success, onlyPremium);

// console.log("fulfillRandomWords gas");
// console.log(startGas - gasleft());
return payment;
}

function _chargePayment(uint96 payment, bool nativePayment, uint256 subId) internal {
if (nativePayment) {
uint96 prevBal = s_subscriptions[subId].nativeBalance;
if (s_subscriptions[subId].nativeBalance < payment) {
kidambisrinivas marked this conversation as resolved.
Show resolved Hide resolved
revert InsufficientBalance();
}
s_subscriptions[subId].nativeBalance -= payment;
s_subscriptions[subId].nativeBalance = prevBal - payment;
s_withdrawableNative += payment;
} else {
if (s_subscriptions[subId].balance < payment) {
uint96 prevBal = s_subscriptions[subId].balance;
if (prevBal < payment) {
revert InsufficientBalance();
}
s_subscriptions[subId].balance -= payment;
s_subscriptions[subId].balance = prevBal - payment;
s_withdrawableTokens += payment;
}
}
Expand Down
Loading