pending request counter in vrf v2.5 coordinator #12425
Conversation
pending request counter in vrf v2.5 coordinator
|
I see that you haven't updated any README files. Would it make sense to do so? |
jinhoonbang
changed the base branch from
develop
to
vrfv2plus-unique-request-id
jinhoonbang
requested review from
kidambisrinivas,
ibrajer,
leeyikjiun and
vreff
and removed request for
a team
|
Go solidity wrappers are out-of-date, regenerate them via the |
| if (s_consumers[consumers[i]][subId].pendingReqCount > 0) { | ||
| return true; |
There was a problem hiding this comment.
Since you removed the code above, that removed the only place where we had to recompute the request ID based on the consumer nonce. Now, I'm thinking if we can use s_subscriptions[rc.subId].reqCount together with pendingReqCount as a part of the request ID generation (instead of consumer nonce), would that also ensure uniqueness? So, reqCount is incremented every time a fulfillment is made on a subscription level (at the same time when pendingReqCount decrements) and pendingReqCount is incremented every time a new request has been made (but reqCount stays the same). Somehow, they always complement each other in a way that could ensure a unique combination every time a request has been made. Of course, in case I didn't miss any edge cases or started with wrong assumptions.
There was a problem hiding this comment.
I discussed this with @leeyikjiun and seems like this approach won't introduce any additional "wins" compared to the current solution we have. I think we can dismiss it.
| if (s_requestCommitments[reqId] != 0) { | ||
| return true; | ||
| } | ||
| if (s_consumers[consumers[i]][subId].pendingReqCount > 0) { |
There was a problem hiding this comment.
Currently both removeConsumer and cancelSubscription call pendingRequestExists, which checks if a subId has pendingRequests or not.
This means we would not be able to delete a consumer of a subId if another consumer of the same subId has pendingRequests. Is this a limiting factor?
There was a problem hiding this comment.
The pendingRequestsExists method tracks pending requests on an entire subscriptionID when we check to removeConsumer.
Shall we add a new method that just checks s_consumers[address][subId].pendingReqCounter when removeConsumer is called
There was a problem hiding this comment.
Discussed this offline. We will leave as is because customers only removeConsumer when they want to cancel subscriptions anyways.
jinhoonbang
dismissed stale reviews from ibrajer and leeyikjiun
The base branch was changed.
|
| assertTrue(s_testCoordinator.pendingRequestExists(subId)); | ||
| } | ||
|
|
||
| function testRequestRandomWords_MultipleConsumers_PendingRequestExists() public { |
There was a problem hiding this comment.
Do we have a test to check what happens when you have pending requests and you try to remove the consumer or cancel an entire subscription? Do we need it?
There was a problem hiding this comment.
Doesn't look like we have those tests. I can add those in follow up PR
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
* 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
* 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
* 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
* 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
* 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
Implement pending request counter in vrf v2.5 coordinator