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.

Sign up for GitHub

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

Jump to bottom

feat: added evm_setBlockGasLimit #332

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feat: added evm_setBlockGasLimit #332

wants to merge 11 commits into from

Conversation

vbar
Copy link
Contributor

@vbar vbar commented Aug 16, 2024

What 💻

Implemented the evm_setBlockGasLimit API.

Why ✋

Fixes #324 .

Evidence 📷

Added test, which passes:

$ npx hardhat test --grep evm_setBlockGasLimit


  evm_setBlockGasLimit
    ✔ Should change block gas limit (52ms)


  1 passing (73ms)

hardhat testsuite also can use the API:

$ mocha test/helpers/setBlockGasLimit.ts 


  setBlockGasLimit
    ✔ should allow to set the block gas limit (130ms)
    accepted parameter types for block gas limit
      ✔ should accept blockGasLimit of type number
      ✔ should accept blockGasLimit of type bigint
      ✔ should accept blockGasLimit of type hex encoded
      ✔ should accept blockGasLimit of type hex encoded with leading zeros
      ✔ should accept blockGasLimit of type ethers's bignumber instances
      ✔ should accept blockGasLimit of type bn.js instances
      ✔ should not accept strings that are not 0x-prefixed


  8 passing (717ms)

@vbar vbar requested a review from a team as a code owner August 16, 2024 12:18
@vbar vbar changed the title fead: added evm_setBlockGasLimit feat: added evm_setBlockGasLimit Aug 16, 2024
@AnastasiiaVashchuk
Copy link
Contributor

i do not see test for endpoint in the file diff

@AnastasiiaVashchuk
Copy link
Contributor

i can not find evm_setBlockGasLimit as well.

@vbar
Copy link
Contributor Author

vbar commented Aug 19, 2024

i do not see test for endpoint in the file diff

sorry, forgot to commit - should be there now

@vbar
Copy link
Contributor Author

vbar commented Aug 19, 2024

i can not find evm_setBlockGasLimit as well.

ditto

vbar and others added 2 commits August 19, 2024 09:25
@vbar @AnastasiiaVashchuk
Update SUPPORTED_APIS.md
3bf0950 
Co-authored-by: AnastasiiaVashchuk <[email protected]>
@vbar @AnastasiiaVashchuk
Update SUPPORTED_APIS.md
9ce506a 
Co-authored-by: AnastasiiaVashchuk <[email protected]>
@AnastasiiaVashchuk
Copy link
Contributor

I see the implementation, but we have the following situation.
On ZKsync, the l1 batch limit is not dynamic like on Ethereum—it's static. The default batch limit is equal to the maximum available one. Also, there are no sequencer in the in-memory node, and we can not tell him to use another block limit, so this endpoint seems not feasible to implement(i do not see a solution). Probably we have to document this for developers.

@vbar
Copy link
Contributor Author

vbar commented Aug 21, 2024

Well, one could still lower the limit in the test environment, even if the real blockchain doesn't support it, for testing... But OK, if you think it would be too confusing, just close the issue...

@AnastasiiaVashchuk
Copy link
Contributor

one could still lower the limit

In what way?
VM will always use the same value. You can not pass a lower value into it.

@vbar
Copy link
Contributor Author

vbar commented Aug 27, 2024 •
edited
Loading

one could still lower the limit

In what way? VM will always use the same value. You can not pass a lower value into it.

OK, but the test node environment should use the variable, affecting (e.g.) gas estimation - IOW although the value inside the VM doesn't change, it is possible to make a transaction fail just by lowering the limit... I can imagine testing how much higher is the maximum than some specific transactions (those in the test suite, for example)...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnastasiiaVashchuk AnastasiiaVashchuk AnastasiiaVashchuk left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

hardhat_api: Implement evm_setBlockGasLimit method
2 participants
@vbar @AnastasiiaVashchuk