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

Add gas overhead per word to wrapper #12669

Merged
merged 10 commits into from
Apr 16, 2024

Conversation

leeyikjiun
Copy link
Contributor

@leeyikjiun leeyikjiun commented Apr 3, 2024

  • Change uint256[] memory randomWords to uint256[] calldata randomWords as calldata is cheaper. (around 200 to 300 less gas from forge test)
  • Add wrapper gas overhead per word and coordinator gas overhead per word to wrapper.

Currently the wrapper overheads are a fix cost so it doesn't matter if you are requesting 0 word, 1 word, or a maximum of 255 words; they cost the same.

The wrapper gas overhead per word isn't that much and is mostly attributed to just copying the randomWords array around. Rough estimation works out to be around 7 gas per word.

The coordinator gas overhead per word is much more expensive and works out to be around 440 gas per word. Other than copying around the randomWords array, there's actual work done in generating the random word.

for (uint256 i = 0; i < numWords; ++i) {
  randomWords[i] = uint256(keccak256(abi.encode(randomness, i)));
}

This 440 gas per word (calculated by making a transaction with 0 num words, a transaction with 255 num words, take the difference and divide by 255) isn't too implausible as well because a quick forge test calculates that line to be around 232 to 235 gas.

for (uint256 i = 0; i < numWords; ++i) {
  uint256 start = gasleft();
  randomWords[i] = uint256(keccak256(abi.encode(randomness, i)));
  uint256 end = gasleft();
  console.log("gas used: ", start - end);
}

Whereas the gas used before and after the loop works out to be around 333 gas per loop

uint256 start = gasleft();
for (uint256 i = 0; i < numWords; ++i) {
  randomWords[i] = uint256(keccak256(abi.encode(randomness, i)));
}
uint256 end = gasleft();
console.log("gas used: ", start - end);

@leeyikjiun leeyikjiun requested review from a team as code owners April 3, 2024 08:20
Copy link
Contributor

github-actions bot commented Apr 3, 2024

I see you updated files related to core. Please run pnpm changeset in the root directory to add a changeset.

Copy link
Contributor

github-actions bot commented Apr 3, 2024

I see you updated files related to contracts. Please run pnpm changeset in the contracts directory to add a changeset.

@@ -84,6 +84,7 @@ contract VRFV2PlusWrapper is ConfirmedOwner, TypeAndVersionInterface, VRFConsume
// s_wrapperGasOverhead reflects the gas overhead of the wrapper's fulfillRandomWords
// function. The cost for this gas is passed to the user.
uint32 private s_wrapperGasOverhead;
uint16 private s_wrapperGasOverheadPerWord;
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is only 7 gas units per word, we probably don't need it right? we can just add some extra buffer to s_wrapperGasOverhead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, if my calculation is correct and the overhead is that small then we can just add some buffer for it. It's at most 2.5k more gas if it's 10 gas per word. I'll need to run more transactions.

@RensR
Copy link
Contributor

RensR commented Apr 11, 2024

Please run git checkout v1.8.1 in the foundry dependency folder to undo the dependency downgrade

@cl-sonarqube-production
Copy link

@leeyikjiun leeyikjiun added this pull request to the merge queue Apr 16, 2024
Merged via the queue into develop with commit 3134ce8 Apr 16, 2024
106 checks passed
@leeyikjiun leeyikjiun deleted the vrfv2pluswrapper-overhead-per-word branch April 16, 2024 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants