Producer MaxGas from genesis (autobahn); ConsensusParams on RPC ctx (CON-257)#3341
Merged
Conversation
…esults eth_getBlockByNumber's gasLimit field was sourced from blockRes.ConsensusParamUpdates.Block.MaxGas. Two problems with that: - Under CometBFT, ConsensusParamUpdates is only populated when the app proposes a consensus-param update — most blocks have it nil. The code path nil-derefs in that case (or relies on Sei's per-block synthesis in app.go's getFinalizeBlockResponse, which always populates it but with whatever endBlockResp returned, not strictly the active params). - Under Autobahn, /block_results is synthesized in sei-tendermint's GigaRouter path with whatever the autobahn config's MaxGasPerBlock is. That can drift from the genesis ConsensusParams.Block.MaxGas the EVM runtime actually uses for the GASLIMIT opcode (x/evm/keeper/keeper.go BlockContext.GasLimit reads ctx.ConsensusParams().Block.MaxGas). Switch to ctx.ConsensusParams().Block.MaxGas — the same source the EVM uses internally — so eth_getBlockByNumber.gasLimit and the GASLIMIT opcode return the same number under both consensus engines. Adds nil guards so the path is safe if ConsensusParams hasn't been populated yet (very early init). Unblocks hardhat tests: - Should return correct gas limit via GASLIMIT opcode - Should access gas limit directly via inline assembly - Should access gas limit correctly within a state-changing transaction Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Autobahn producer's mempool reaping budget (Producer.MaxGasPerBlock, used by ReapMaxBytesMaxGas via MaxGasWanted/MaxGasEstimated) was sourced from autobahn.json's max_gas_per_block field. The chain's gas-limit consensus rule lives in genesis (consensus_params.block.max_gas), which is the same number x/evm reads via ctx.ConsensusParams().Block.MaxGas for the GASLIMIT opcode. Two independently configurable sources for the same physical rule could silently drift — a leader sized to one limit while verifiers (and the EVM) enforced the other. Source the producer from genesis so there's one canonical value, and drop the autobahn.json field entirely. Validate that genesis ConsensusParams.Block.MaxGas > 0 at node startup and fail fast if it isn't. The autobahn-integration-test cluster boots cleanly and all 5 subtests (BlockProduction / BankTransfer / LivenessUnderMaxFaults / HaltsBeyondMaxFaults / Recovery) pass with the new path. Non-app-hash-breaking: producer reaping affects which txs the leader proposes, not block validity. Verifiers re-check against genesis (unchanged), so any working deployment already had the two sources matching — at most this removes a way to silently misconfigure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
evmrpc handlers that read ctx.ConsensusParams() — EncodeTmBlock's gasLimit (eth_getBlockByNumber), InfoAPI.CalculateGasUsedRatio, x/evm BlockContext.GasLimit when reached via debug_trace*, etc. — were getting nil and falling through to zero or hardcoded 10M defaults. Reason: app.GetCheckCtx() returns app.checkState.ctx, which setCheckState builds without WithConsensusParams; CreateQueryContext likewise omits it. Only the tx-execution path (getContextForTx in sei-cosmos baseapp) sets it. So every RPC ctx returned by RPCContextProvider lacked the params even though the param store had them. Populate it in both branches of RPCContextProvider via WithConsensusParams(app.GetConsensusParams(ctx)). The existing case-by-case workarounds (e.g. the cascade in evmrpc/info.go's CalculateGasUsedRatio) become redundant; they can be cleaned up separately. Cost: ~7 in-memory paramStore reads per RPC ctx (already paid on every tx via getContextForTx, well-trodden). Negligible against the JSON serialization cost of a typical handler. Adds TestRPCContextProviderPopulatesConsensusParams covering the latest-height and historical-height branches; verified to fail without the fix. Also updates evmrpc/tests/regression_test.go: the captured trace ground truths were recorded under specific block consensus_params (visible in mock_data/.../*.json -> consensus_param_updates). Previously the test happened to match because the fallback to DefaultBlockGasLimit (10M) coincidentally equaled the captured mainnet block's max_gas. With this fix, ctx now carries the test fixture's DefaultConsensusParams (100M), so traces drift unless we honor the captured per-block params. Add withCapturedConsensusParams that reads them from the already-loaded mockedBlockResultsResults and applies them to the ctx — making the regression replay faithful to mainnet conditions, not coincidentally equal to a default fallback. Non-app-hash-breaking: RPCContextProvider is query-only; no state-transition path uses it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
wen-coding
changed the title
wen-coding
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3341 +/- ##
==========================================
- Coverage 59.21% 59.16% -0.05%
==========================================
Files 2097 2097
Lines 172509 172451 -58
==========================================
- Hits 102152 102032 -120
- Misses 61501 61558 +57
- Partials 8856 8861 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
pompon0
reviewed
Apr 30, 2026
| genDoc := &types.GenesisDoc{ChainID: "test-chain", InitialHeight: 1} | ||
| genDoc := &types.GenesisDoc{ | ||
| ChainID: "test-chain", | ||
| InitialHeight: 1, |
Contributor
There was a problem hiding this comment.
while at it, can we make InitialHeight nontrivial? Or will it break tests?
Contributor
Author
There was a problem hiding this comment.
Changed, I don't think it matters here
pompon0
reviewed
Apr 30, 2026
| genDoc.ConsensusParams = nil | ||
| _, err := buildGigaConfig(cfgFile, nodeKey, valKey, txMempool, genDoc) | ||
| assert.Error(t, err) | ||
| assert.Contains(t, err.Error(), "max_gas") |
Contributor
There was a problem hiding this comment.
pls, no error parsing, use errors.Is instead.
pompon0
approved these changes
Apr 30, 2026
Reviewer feedback on #3341: replace substring-matching error text (`assert.Contains(err.Error(), "max_gas")`) with a typed sentinel. Define ErrGenesisMaxGasInvalid and wrap via fmt.Errorf("...: %w", ...) so callers can `errors.Is(err, ErrGenesisMaxGasInvalid)`. Tests now use assert.ErrorIs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reviewer feedback on #3341: the test fixture used InitialHeight: 1. Bump to 100 (with a comment) so any future code in buildGigaConfig that implicitly assumes the genesis is at height 1 fails the test instead of accidentally working. buildGigaConfig itself only reads genDoc.ConsensusParams and genDoc.ChainID — InitialHeight flows untouched into result.GenDoc, so the existing equality assertion still holds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
arajasek
approved these changes
May 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent fixes on the path from
genesis.consensus_params.block.max_gasto its consumers.Fix 1 — Autobahn producer sources
MaxGasPerBlockfrom genesisAutobahn-only.
Producer.MaxGasPerBlockwas populated fromautobahn.json'smax_gas_per_block, while the EVM runtime and CometBFT's reaping read fromgenDoc.ConsensusParams.Block.MaxGas— two configurable sources for the same chain rule.buildGigaConfignow sources from genesis (validated > 0), matching CometBFT. The autobahn.json field is removed.Fix 2 —
RPCContextProviderpopulates ConsensusParamsBoth engines.
app.GetCheckCtx()andCreateQueryContextbuild ctxs withoutWithConsensusParams— only the tx-execution path (getContextForTx) does. So every RPC ctx hadctx.ConsensusParams() == nil, masked under the oldblock.go(which readblockRes.ConsensusParamUpdates) and worked around ininfo.gowith a fallback cascade. After2593ceb28switchedEncodeTmBlocktoctx.ConsensusParams(),eth_getBlockByNumber.gasLimitreturns 0.RPCContextProvidernow callsWithConsensusParams(app.GetConsensusParams(ctx))on both branches.eth_getBlockByNumber.gasLimitmatches the GASLIMIT opcode bit-exactly. Theinfo.gocascade can be cleaned up in a follow-up.Test plan
go test ./sei-tendermint/node/— incl. newTestBuildGigaConfig_GenesisMaxGas(nil / zero / negative)go test ./app/— incl. newTestRPCContextProviderPopulatesConsensusParams(verified to fail without Fix 2)go test ./evmrpc/...— all 4 packagesmake autobahn-integration-test— 5/5 subtests, full cluster boot/teardowngofmt -s -l .clean,go vetclean🤖 Generated with Claude Code