Skip to content

Commit

Permalink
pending request counter in vrf v2.5 coordinator (#12425)
Browse files Browse the repository at this point in the history 
* soft delete nonce in s_consumers so that request IDs do not repeat. WIP

* forge unit tests passing except for exact billing amount

* add forge tests and optimize gas

* regenerate wrappers

* remove comment

* address comments

* fix test

* add changeset

* implement pending request counter
pending request counter in vrf v2.5 coordinator

* run prettier and regenerate wrappers

* add changeset

* bump gas required for remove consumer
  • Loading branch information
@jinhoonbang @kidambisrinivas
jinhoonbang authored and kidambisrinivas committed Mar 18, 2024
1 parent fc64f0c commit 83d14e5
Show file tree
Hide file tree
Showing 9 changed files with 193 additions and 20 deletions.
5 changes: 5 additions & 0 deletions .changeset/nasty-humans-promise.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

add pending request counter for vrf v2.5 coordinator
1 change: 1 addition & 0 deletions contracts/src/v0.8/vrf/dev/SubscriptionAPI.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ abstract contract SubscriptionAPI is ConfirmedOwner, IERC677Receiver, IVRFSubscr
struct ConsumerConfig {
bool active;
uint64 nonce;
uint64 pendingReqCount;
}
// Note a nonce of 0 indicates an the consumer is not assigned to that subscription.
mapping(address => mapping(uint256 => ConsumerConfig)) /* consumerAddress */ /* subId */ /* consumerConfig */
Expand Down
17 changes: 5 additions & 12 deletions contracts/src/v0.8/vrf/dev/VRFCoordinatorV2_5.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ contract VRFCoordinatorV2_5 is VRF, SubscriptionAPI, IVRFCoordinatorV2Plus {
// The consequence for users is that they can send requests
// for invalid keyHashes which will simply not be fulfilled.
++consumerConfig.nonce;
++consumerConfig.pendingReqCount;
uint256 preSeed;
(requestId, preSeed) = _computeRequestId(req.keyHash, msg.sender, subId, consumerConfig.nonce);

Expand Down Expand Up @@ -500,6 +501,8 @@ contract VRFCoordinatorV2_5 is VRF, SubscriptionAPI, IVRFCoordinatorV2Plus {

// Increment the req count for the subscription.
++s_subscriptions[rc.subId].reqCount;
// Decrement the pending req count for the consumer.
--s_consumers[rc.sender][rc.subId].pendingReqCount;

bool nativePayment = uint8(rc.extraArgs[rc.extraArgs.length - 1]) == 1;

Expand Down Expand Up @@ -625,19 +628,9 @@ contract VRFCoordinatorV2_5 is VRF, SubscriptionAPI, IVRFCoordinatorV2Plus {
if (consumersLength == 0) {
return false;
}
uint256 provingKeyHashesLength = s_provingKeyHashes.length;
for (uint256 i = 0; i < consumersLength; ++i) {
address consumer = consumers[i];
for (uint256 j = 0; j < provingKeyHashesLength; ++j) {
(uint256 reqId, ) = _computeRequestId(
s_provingKeyHashes[j],
consumer,
subId,
s_consumers[consumer][subId].nonce
);
if (s_requestCommitments[reqId] != 0) {
return true;
}
if (s_consumers[consumers[i]][subId].pendingReqCount > 0) {
return true;
}
}
return false;
Expand Down
6 changes: 5 additions & 1 deletion contracts/src/v0.8/vrf/dev/testhelpers/VRFCoordinatorV2PlusUpgradedVersion.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,11 @@ contract VRFCoordinatorV2PlusUpgradedVersion is
}

for (uint256 i = 0; i < migrationData.consumers.length; i++) {
s_consumers[migrationData.consumers[i]][migrationData.subId] = ConsumerConfig({active: true, nonce: 0});
s_consumers[migrationData.consumers[i]][migrationData.subId] = ConsumerConfig({
active: true,
nonce: 0,
pendingReqCount: 0
});
}

s_subscriptions[migrationData.subId] = Subscription({
Expand Down
174 changes: 172 additions & 2 deletions contracts/test/v0.8/foundry/vrf/VRFV2Plus.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ contract VRFV2Plus is BaseTest {
// 1e15 is less than 1 percent discrepancy
assertApproxEqAbs(payment, 5.138 * 1e17, 1e15);
assertApproxEqAbs(nativeBalanceAfter, nativeBalanceBefore - 5.138 * 1e17, 1e15);
assertFalse(s_testCoordinator.pendingRequestExists(subId));
}

function testRequestAndFulfillRandomWordsLINK() public {
Expand Down Expand Up @@ -453,6 +454,7 @@ contract VRFV2Plus is BaseTest {
// 1e15 is less than 1 percent discrepancy
assertApproxEqAbs(payment, 8.2992 * 1e17, 1e15);
assertApproxEqAbs(linkBalanceAfter, linkBalanceBefore - 8.2992 * 1e17, 1e15);
assertFalse(s_testCoordinator.pendingRequestExists(subId));
}

function testRequestAndFulfillRandomWordsLINK_FallbackWeiPerUnitLinkUsed() public {
Expand Down Expand Up @@ -508,6 +510,7 @@ contract VRFV2Plus is BaseTest {
s_testConsumer.requestRandomWords(CALLBACK_GAS_LIMIT, MIN_CONFIRMATIONS, NUM_WORDS, vrfKeyHash, false);
(bool fulfilled, , ) = s_testConsumer.s_requests(requestId);
assertEq(fulfilled, false);
assertTrue(s_testCoordinator.pendingRequestExists(subId));

// Uncomment these console logs to see info about the request:
// console.log("requestId: ", requestId);
Expand Down Expand Up @@ -596,6 +599,7 @@ contract VRFV2Plus is BaseTest {
s_testConsumer.requestRandomWords(CALLBACK_GAS_LIMIT, MIN_CONFIRMATIONS, NUM_WORDS, vrfKeyHash, true);
(bool fulfilled, , ) = s_testConsumer.s_requests(requestId);
assertEq(fulfilled, false);
assertTrue(s_testCoordinator.pendingRequestExists(subId));

// Uncomment these console logs to see info about the request:
// console.log("requestId: ", requestId);
Expand Down Expand Up @@ -715,6 +719,7 @@ contract VRFV2Plus is BaseTest {
// 1e15 is less than 1 percent discrepancy
assertApproxEqAbs(payment, 5.9 * 1e17, 1e15);
assertApproxEqAbs(nativeBalanceAfter, nativeBalanceBefore - 5.9 * 1e17, 1e15);
assertFalse(s_testCoordinator.pendingRequestExists(subId));
}

function testRequestAndFulfillRandomWords_OnlyPremium_LinkPayment() public {
Expand Down Expand Up @@ -764,6 +769,7 @@ contract VRFV2Plus is BaseTest {
// 1e15 is less than 1 percent discrepancy
assertApproxEqAbs(payment, 9.36 * 1e17, 1e15);
assertApproxEqAbs(linkBalanceAfter, linkBalanceBefore - 9.36 * 1e17, 1e15);
assertFalse(s_testCoordinator.pendingRequestExists(subId));
}

function testRequestRandomWords_InvalidConsumer() public {
Expand All @@ -783,6 +789,7 @@ contract VRFV2Plus is BaseTest {
NUM_WORDS,
1 /* requestCount */
);
assertFalse(s_testCoordinator.pendingRequestExists(subId));
}

function testRequestRandomWords_ReAddConsumer_AssertRequestID() public {
Expand All @@ -792,8 +799,7 @@ contract VRFV2Plus is BaseTest {
address subOwner = makeAddr("subOwner");
changePrank(subOwner);
uint256 subId = s_testCoordinator.createSubscription();
VRFV2PlusLoadTestWithMetrics consumer = new VRFV2PlusLoadTestWithMetrics(address(s_testCoordinator));
s_testCoordinator.addConsumer(subId, address(consumer));
VRFV2PlusLoadTestWithMetrics consumer = createAndAddLoadTestWithMetricsConsumer(subId);
uint32 requestBlock = 10;
vm.roll(requestBlock);
changePrank(LINK_WHALE);
Expand Down Expand Up @@ -924,5 +930,169 @@ contract VRFV2Plus is BaseTest {
);
assertNotEq(requestId, requestId2);
assertNotEq(preSeed, preSeed2);
assertTrue(s_testCoordinator.pendingRequestExists(subId));
}

function testRequestRandomWords_MultipleConsumers_PendingRequestExists() public {
// 1. setup consumer and subscription
setConfig();
registerProvingKey();
address subOwner = makeAddr("subOwner");
changePrank(subOwner);
uint256 subId = s_testCoordinator.createSubscription();
VRFV2PlusLoadTestWithMetrics consumer1 = createAndAddLoadTestWithMetricsConsumer(subId);
VRFV2PlusLoadTestWithMetrics consumer2 = createAndAddLoadTestWithMetricsConsumer(subId);
uint32 requestBlock = 10;
vm.roll(requestBlock);
changePrank(LINK_WHALE);
s_testCoordinator.fundSubscriptionWithNative{value: 10 ether}(subId);

// 2. Request random words.
changePrank(subOwner);
(uint256 requestId1, uint256 preSeed1) = s_testCoordinator.computeRequestIdExternal(
vrfKeyHash,
address(consumer1),
subId,
1
);
(uint256 requestId2, uint256 preSeed2) = s_testCoordinator.computeRequestIdExternal(
vrfKeyHash,
address(consumer2),
subId,
1
);
assertNotEq(requestId1, requestId2);
assertNotEq(preSeed1, preSeed2);
consumer1.requestRandomWords(
subId,
MIN_CONFIRMATIONS,
vrfKeyHash,
CALLBACK_GAS_LIMIT,
true /* nativePayment */,
NUM_WORDS,
1 /* requestCount */
);
consumer2.requestRandomWords(
subId,
MIN_CONFIRMATIONS,
vrfKeyHash,
CALLBACK_GAS_LIMIT,
true /* nativePayment */,
NUM_WORDS,
1 /* requestCount */
);
assertTrue(s_testCoordinator.pendingRequestExists(subId));

// Move on to the next block.
// Store the previous block's blockhash, and assert that it is as expected.
vm.roll(requestBlock + 1);
s_bhs.store(requestBlock);
assertEq(hex"000000000000000000000000000000000000000000000000000000000000000a", s_bhs.getBlockhash(requestBlock));

// 3. Fulfill the 1st request above
console.log("requestId: ", requestId1);
console.log("preSeed: ", preSeed1);
console.log("sender: ", address(consumer1));

// Fulfill the request.
// Proof generated via the generate-proof-v2-plus script command. Example usage:
/*
go run . generate-proof-v2-plus \
-key-hash 0x9f2353bde94264dbc3d554a94cceba2d7d2b4fdce4304d3e09a1fea9fbeb1528 \
-pre-seed 94043941380654896554739370173616551044559721638888689173752661912204412136884 \
-block-hash 0x000000000000000000000000000000000000000000000000000000000000000a \
-block-num 10 \
-sender 0x44CAfC03154A0708F9DCf988681821f648dA74aF \
-native-payment true
*/
VRF.Proof memory proof = VRF.Proof({
pk: [
72488970228380509287422715226575535698893157273063074627791787432852706183111,
62070622898698443831883535403436258712770888294397026493185421712108624767191
],
gamma: [
18593555375562408458806406536059989757338587469093035962641476877033456068708,
55675218112764789548330682504442195066741636758414578491295297591596761905475
],
c: 56595337384472359782910435918403237878894172750128610188222417200315739516270,
s: 60666722370046279064490737533582002977678558769715798604164042022636022215663,
seed: 94043941380654896554739370173616551044559721638888689173752661912204412136884,
uWitness: 0xEdbE15fd105cfEFb9CCcbBD84403d1F62719E50d,
cGammaWitness: [
11752391553651713021860307604522059957920042356542944931263270793211985356642,
14713353048309058367510422609936133400473710094544154206129568172815229277104
],
sHashWitness: [
109716108880570827107616596438987062129934448629902940427517663799192095060206,
79378277044196229730810703755304140279837983575681427317104232794580059801930
],
zInv: 18898957977631212231148068121702167284572066246731769473720131179584458697812
});
VRFCoordinatorV2_5.RequestCommitment memory rc = VRFCoordinatorV2_5.RequestCommitment({
blockNum: requestBlock,
subId: subId,
callbackGasLimit: CALLBACK_GAS_LIMIT,
numWords: NUM_WORDS,
sender: address(consumer1),
extraArgs: VRFV2PlusClient._argsToBytes(VRFV2PlusClient.ExtraArgsV1({nativePayment: true}))
});
s_testCoordinator.fulfillRandomWords(proof, rc, true /* onlyPremium */);
assertTrue(s_testCoordinator.pendingRequestExists(subId));

//4. Fulfill the 2nd request
console.log("requestId: ", requestId2);
console.log("preSeed: ", preSeed2);
console.log("sender: ", address(consumer2));

// Fulfill the request.
// Proof generated via the generate-proof-v2-plus script command. Example usage:
/*
go run . generate-proof-v2-plus \
-key-hash 0x9f2353bde94264dbc3d554a94cceba2d7d2b4fdce4304d3e09a1fea9fbeb1528 \
-pre-seed 60086281972849674111646805013521068579710860774417505336898013292594859262126 \
-block-hash 0x000000000000000000000000000000000000000000000000000000000000000a \
-block-num 10 \
-sender 0xf5a165378E120f93784395aDF1E08a437e902865 \
-native-payment true
*/
proof = VRF.Proof({
pk: [
72488970228380509287422715226575535698893157273063074627791787432852706183111,
62070622898698443831883535403436258712770888294397026493185421712108624767191
],
gamma: [
8781676794493524976318989249067879326013864868749595045909181134740761572122,
70144896394968351242907510966944756907625107566821127114847472296460405612124
],
c: 67847193668837615807355025316836592349514589069599294392546721746916067719949,
s: 114946531382736685625345450298146929067341928840493664822961336014597880904075,
seed: 60086281972849674111646805013521068579710860774417505336898013292594859262126,
uWitness: 0xe1de4fD69277D0C5516cAE4d760b1d08BC340A28,
cGammaWitness: [
90301582727701442026215692513959255065128476395727596945643431833363167168678,
61501369717028493801369453424028509804064958915788808540582630993703331669978
],
sHashWitness: [
98738650825542176387169085844714248077697103572877410412808249468787326424906,
85647963391545223707301702874240345890884970941786094239896961457539737216630
],
zInv: 29080001901010358083725892808339807464533563010468652346220922643802059192842
});
rc = VRFCoordinatorV2_5.RequestCommitment({
blockNum: requestBlock,
subId: subId,
callbackGasLimit: CALLBACK_GAS_LIMIT,
numWords: NUM_WORDS,
sender: address(consumer2),
extraArgs: VRFV2PlusClient._argsToBytes(VRFV2PlusClient.ExtraArgsV1({nativePayment: true}))
});
s_testCoordinator.fulfillRandomWords(proof, rc, true /* onlyPremium */);
assertFalse(s_testCoordinator.pendingRequestExists(subId));
}

function createAndAddLoadTestWithMetricsConsumer(uint256 subId) internal returns (VRFV2PlusLoadTestWithMetrics) {
VRFV2PlusLoadTestWithMetrics consumer = new VRFV2PlusLoadTestWithMetrics(address(s_testCoordinator));
s_testCoordinator.addConsumer(subId, address(consumer));
return consumer;
}
}
2 changes: 1 addition & 1 deletion core/gethwrappers/generated/vrf_coordinator_v2_5/vrf_coordinator_v2_5.go
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion core/gethwrappers/generated/vrf_v2plus_upgraded_version/vrf_v2plus_upgraded_version.go
View file

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions core/gethwrappers/generation/generated-wrapper-dependency-versions-do-not-edit.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ vrf_consumer_v2_upgradeable_example: ../../contracts/solc/v0.8.6/VRFConsumerV2Up
vrf_coordinator_mock: ../../contracts/solc/v0.8.6/VRFCoordinatorMock/VRFCoordinatorMock.abi ../../contracts/solc/v0.8.6/VRFCoordinatorMock/VRFCoordinatorMock.bin 5c495cf8df1f46d8736b9150cdf174cce358cb8352f60f0d5bb9581e23920501
vrf_coordinator_test_v2: ../../contracts/solc/v0.8.6/VRFCoordinatorTestV2/VRFCoordinatorTestV2.abi ../../contracts/solc/v0.8.6/VRFCoordinatorTestV2/VRFCoordinatorTestV2.bin eaefd785c38bac67fb11a7fc2737ab2da68c988ca170e7db8ff235c80893e01c
vrf_coordinator_v2: ../../contracts/solc/v0.8.6/VRFCoordinatorV2/VRFCoordinatorV2.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2/VRFCoordinatorV2.bin 295f35ce282060317dfd01f45959f5a2b05ba26913e422fbd4fb6bf90b107006
vrf_coordinator_v2_5: ../../contracts/solc/v0.8.6/VRFCoordinatorV2_5/VRFCoordinatorV2_5.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2_5/VRFCoordinatorV2_5.bin 6b4c50e8c8bbe877e5450d679e968dbde896f7c9043d29f3ecf79aefc28a0ef3
vrf_coordinator_v2_5: ../../contracts/solc/v0.8.6/VRFCoordinatorV2_5/VRFCoordinatorV2_5.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2_5/VRFCoordinatorV2_5.bin d794eb0968ee16f6660eb7a4fd30cc423427377f272ae6f83224e023fbeb5f47
vrf_coordinator_v2_plus_v2_example: ../../contracts/solc/v0.8.6/VRFCoordinatorV2Plus_V2Example/VRFCoordinatorV2Plus_V2Example.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2Plus_V2Example/VRFCoordinatorV2Plus_V2Example.bin 4a5b86701983b1b65f0a8dfa116b3f6d75f8f706fa274004b57bdf5992e4cec3
vrf_coordinator_v2plus_interface: ../../contracts/solc/v0.8.6/IVRFCoordinatorV2PlusInternal/IVRFCoordinatorV2PlusInternal.abi ../../contracts/solc/v0.8.6/IVRFCoordinatorV2PlusInternal/IVRFCoordinatorV2PlusInternal.bin 86b8e23aab28c5b98e3d2384dc4f702b093e382dc985c88101278e6e4bf6f7b8
vrf_external_sub_owner_example: ../../contracts/solc/v0.8.6/VRFExternalSubOwnerExample/VRFExternalSubOwnerExample.abi ../../contracts/solc/v0.8.6/VRFExternalSubOwnerExample/VRFExternalSubOwnerExample.bin 14f888eb313930b50233a6f01ea31eba0206b7f41a41f6311670da8bb8a26963
Expand All @@ -102,7 +102,7 @@ vrf_v2_consumer_wrapper: ../../contracts/solc/v0.8.6/VRFv2Consumer/VRFv2Consumer
vrf_v2plus_load_test_with_metrics: ../../contracts/solc/v0.8.6/VRFV2PlusLoadTestWithMetrics/VRFV2PlusLoadTestWithMetrics.abi ../../contracts/solc/v0.8.6/VRFV2PlusLoadTestWithMetrics/VRFV2PlusLoadTestWithMetrics.bin 42e1b3fb61a9d09dfa6d63beeab3f55d0744c38f6d4b4503d7f3fe77b32231c5
vrf_v2plus_single_consumer: ../../contracts/solc/v0.8.6/VRFV2PlusSingleConsumerExample/VRFV2PlusSingleConsumerExample.abi ../../contracts/solc/v0.8.6/VRFV2PlusSingleConsumerExample/VRFV2PlusSingleConsumerExample.bin 12b5d322db7dbf8af71955699e411109a4cc40811b606273ea0b5ecc8dbc639d
vrf_v2plus_sub_owner: ../../contracts/solc/v0.8.6/VRFV2PlusExternalSubOwnerExample/VRFV2PlusExternalSubOwnerExample.abi ../../contracts/solc/v0.8.6/VRFV2PlusExternalSubOwnerExample/VRFV2PlusExternalSubOwnerExample.bin 7b4f5ffe8fc293d2f4294d3d8348ed8dd480e909cef0743393095e5b20dc9c34
vrf_v2plus_upgraded_version: ../../contracts/solc/v0.8.6/VRFCoordinatorV2PlusUpgradedVersion/VRFCoordinatorV2PlusUpgradedVersion.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2PlusUpgradedVersion/VRFCoordinatorV2PlusUpgradedVersion.bin 1695b5f9990dfe1c7d71c6f47f4be3488be15d09def9d984ffbc1db0f207a08a
vrf_v2plus_upgraded_version: ../../contracts/solc/v0.8.6/VRFCoordinatorV2PlusUpgradedVersion/VRFCoordinatorV2PlusUpgradedVersion.abi ../../contracts/solc/v0.8.6/VRFCoordinatorV2PlusUpgradedVersion/VRFCoordinatorV2PlusUpgradedVersion.bin cd294fdedbd834f888de71d900c21783c8824962217aa2632c542f258c8fca14
vrfv2_proxy_admin: ../../contracts/solc/v0.8.6/VRFV2ProxyAdmin/VRFV2ProxyAdmin.abi ../../contracts/solc/v0.8.6/VRFV2ProxyAdmin/VRFV2ProxyAdmin.bin 402b1103087ffe1aa598854a8f8b38f8cd3de2e3aaa86369e28017a9157f4980
vrfv2_reverting_example: ../../contracts/solc/v0.8.6/VRFV2RevertingExample/VRFV2RevertingExample.abi ../../contracts/solc/v0.8.6/VRFV2RevertingExample/VRFV2RevertingExample.bin 1ae46f80351d428bd85ba58b9041b2a608a1845300d79a8fed83edf96606de87
vrfv2_transparent_upgradeable_proxy: ../../contracts/solc/v0.8.6/VRFV2TransparentUpgradeableProxy/VRFV2TransparentUpgradeableProxy.abi ../../contracts/solc/v0.8.6/VRFV2TransparentUpgradeableProxy/VRFV2TransparentUpgradeableProxy.bin fe1a8e6852fbd06d91f64315c5cede86d340891f5b5cc981fb5b86563f7eac3f
Expand Down
2 changes: 1 addition & 1 deletion core/services/vrf/v2/integration_v2_plus_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,7 @@ func TestVRFV2PlusIntegration_MaxConsumersCost(t *testing.T) {
uni.rootContractAddress, uni.coordinatorABI,
"removeConsumer", subId, carolContractAddress)
t.Log(estimate)
assert.Less(t, estimate, uint64(320000))
assert.Less(t, estimate, uint64(540000))
estimate = estimateGas(t, uni.backend, carolContractAddress,
uni.rootContractAddress, uni.coordinatorABI,
"addConsumer", subId, testutils.NewAddress())
Expand Down

0 comments on commit 83d14e5

Please sign in to comment.