Skip to content

Fix: Correct JSON path for consensus max_gas setting #39

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

Merged
merged 2 commits into from
Apr 11, 2025

Conversation

xosnet
Copy link
Contributor

@xosnet xosnet commented Apr 2, 2025

Fix: Correct JSON path for consensus max_gas setting

Issue

The current implementation in local_node.sh uses an incorrect JSON path to set the maximum gas limit for blocks in the genesis file. This results in the max_gas parameter remaining at the default value -1 (unlimited) instead of being set to the intended value of 10000000.

Root Cause

The script is using the path .consensus_params["block"]["max_gas"] which doesn't match the actual structure in the genesis.json file. The correct path should be .consensus.params.block.max_gas.

Changes

  • Updated the jq command in local_node.sh to use the correct JSON path structure
  • Changed from:
    jq '.consensus_params["block"]["max_gas"]="10000000"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS"
  • To:
    jq '.consensus.params.block.max_gas="10000000"' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS"

Testing

I've tested this change by:

  1. Running the updated script
  2. Verifying the genesis.json file has the correct value set:
    "consensus": {
      "params": {
        "block": {
          "max_bytes": "22020096",
          "max_gas": "10000000"
        },
  3. Confirming the chain starts with the intended gas limit

This fix ensures that nodes are properly configured with the specified block gas limit rather than using unlimited gas, which could lead to block processing issues.

@xosnet xosnet requested a review from a team as a code owner April 2, 2025 07:54
Copy link
Member

@vladjdk vladjdk left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for catching this :)

@vladjdk vladjdk merged commit a717fd7 into cosmos:main Apr 11, 2025
14 checks passed
Eric-Warehime added a commit that referenced this pull request Apr 17, 2025
* chore: add docs to readme (#60)

Link mention to Cosmos EVM docs' page on the main readme of the repository.

* Fix: Correct JSON path for setting max_gas in genesis (#39)

Co-authored-by: Vlad J <[email protected]>

* chore: Move copied files back to geth fork (#40)

* Change all the things

* Fix test

* Remove activators nolint

* Fix test

* Update readme

* Fix readme lint

* chore: utest -> atest (#25)

* utest -> atest

* example chain script fix

* fix node script

---------

Co-authored-by: Eric Warehime <[email protected]>

* fix name() query on native ERC20 failing out (#72)

* fix name() query on native ERC20 failing out

* fix indent

* lint fix

* lint

* tab character

* another hidden tab

* static link (#77)

solving the problem using different versions of glibc

* Remove comma from Makefile phony (#78)

* refactor: Remove old params object from x/feemarket (#83)

* Remove old params object from feemarket

* Add changelog entry

---------

Co-authored-by: Nico Poggi <[email protected]>
Co-authored-by: xosnet <[email protected]>
Co-authored-by: Vlad J <[email protected]>
Co-authored-by: valli_0x <[email protected]>
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.

2 participants