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

refactor!: increase max gas limitation #1630

Closed
wants to merge 1 commit into from
Closed

refactor!: increase max gas limitation #1630

wants to merge 1 commit into from

Conversation

KaoImin
Copy link
Contributor

@KaoImin KaoImin commented Dec 7, 2023 •
edited
Loading

What this PR does / why we need it?

This PR increase max gas limitation to 50 million.

What is the impact of this PR?

PR relation:

CI Settings

CI Usage

Tip: Check the CI you want to run below, and then comment /run-ci.

CI Switch

  • Web3 Compatible Tests
  • OpenZeppelin tests
  • v3 Core Tests

CI Description

CI Name Description
Web3 Compatible Test Test the Web3 compatibility of Axon
v3 Core Test Run the compatibility tests provided by Uniswap V3
OpenZeppelin tests Run the compatibility tests provided by OpenZeppelin, including OCT 1-5 | 6-10 | 11 | 12-15 | 16-19

@KaoImin KaoImin marked this pull request as ready for review December 11, 2023 02:14
@KaoImin KaoImin requested a review from a team as a code owner December 11, 2023 02:14
@KaoImin KaoImin requested review from jjyr and wenyuanhust December 11, 2023 02:14
@KaoImin KaoImin added this pull request to the merge queue Dec 11, 2023
@Flouse Flouse removed this pull request from the merge queue due to a manual request Dec 11, 2023
github-merge-queue bot pushed a commit that referenced this pull request Dec 11, 2023
refactor!: increase max gas limitation (#1630)
aea9fbe
@Flouse Flouse marked this pull request as draft December 11, 2023 02:15
@Flouse Flouse added the d:confirmation Discussion required to confirm whether it's a bug label Dec 11, 2023
@Flouse
Copy link
Contributor

Flouse commented Dec 11, 2023 •
edited
Loading

  1. Update AXON_MAX_GAS_LIMIT in test/Rollback.js godwokenrises/godwoken-tests#319
    This PR has fixed the test in a simple way.
    I think it's should be discussed if this PR could be merged, because it might be a breaking change.

  2. The root cause may not be the max gas limitation
    @KaoImin

    Why did eth_estimateGas cost all gas in that test case?

@KaoImin KaoImin closed this Dec 13, 2023
@KaoImin KaoImin deleted the change-max-gas-limit branch December 13, 2023 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sunchengzhu sunchengzhu sunchengzhu approved these changes

@yangby-cryptape yangby-cryptape yangby-cryptape approved these changes

@jjyr jjyr Awaiting requested review from jjyr jjyr is a code owner automatically assigned from axonweb3/axon-dev

@wenyuanhust wenyuanhust Awaiting requested review from wenyuanhust wenyuanhust is a code owner automatically assigned from axonweb3/axon-dev

Assignees
No one assigned
Labels
d:confirmation Discussion required to confirm whether it's a bug refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@KaoImin @Flouse @yangby-cryptape @sunchengzhu