fix: swap mainnet/testnet chain IDs and enforce 0.01 Gwei min 1559 tip#8
fix: swap mainnet/testnet chain IDs and enforce 0.01 Gwei min 1559 tip#8maoaixiao1314 merged 5 commits intomainfrom
Conversation
WalkthroughGenesis/init script reorders max_gas application and updates feemarket and gentx fees; RPC gas tip cap gains a 10,000,000 wei lower bound; mainnet/testnet chain ID constants are swapped; TempDir no longer auto-removes; EVM chain-ID checks broadened to allow 560000, 560001, 560002, and 565000. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
utils/utils.go (1)
109-117:TempDirreturns a directory that is immediately scheduled for deletion
TempDirdefersos.RemoveAll(dir)before returningdir, so by the time the caller uses the path, the directory may already have been deleted (or will be deleted at the end of the function’s stack frame).This is a correctness issue: any caller that expects to write into the returned temp directory will fail unpredictably.
Consider removing the
defer os.RemoveAll(dir)and letting callers explicitly clean up, or returning a cleanup function alongside the path.func TempDir(defaultHome string) string { dir, err := os.MkdirTemp("", "hetu") if err != nil { dir = defaultHome } - defer os.RemoveAll(dir) - return dir }
♻️ Duplicate comments (3)
x/evm/types/legacy_tx.go (1)
217-237: Chain ID validation matches dynamic-fee tx behaviorThe allowlist for chain IDs and the loop-based validation are correct and consistent with
DynamicFeeTx.Validate(supporting560000,560001,560002,565000with a clear error message).The duplication of this block across several files is already noted in the
DynamicFeeTxreview; consider centralizing the allowlist and predicate there to avoid drift.x/evm/types/access_list_tx.go (1)
240-260: Access-list tx chain ID checks are aligned with the rest of the EVM stackThis allowlist (560000, 560001, 560002, 565000) and the membership loop are correct and consistent with the other transaction types and keeper logic.
As mentioned in the
DynamicFeeTxreview, you might want to centralize this allowlist / predicate to avoid having to edit multiple call sites when supported networks change.x/evm/keeper/keeper.go (1)
144-162: Keeper’s chain ID enforcement is correct and consistent with tx validationThe
WithChainIDmethod now enforces the same supported set (560000,560001,560002,565000) as the tx validators, panicking if the context’s chain ID falls outside this set. That’s a good guard to prevent running the EVM on unexpected networks.As with the other sites, consider centralizing the supported chain ID list / predicate so this logic doesn’t need to be kept in sync manually across multiple files.
🧹 Nitpick comments (3)
x/evm/types/dynamic_fee_tx.go (1)
272-292: Chain ID allowlist logic is correct but duplicated across multiple filesThe new loop-based validation over
validChainIDsis functionally correct and matches the intended supported set (560000,560001,560002,565000). However, the exact same allowlist and loop now appear in several places (dynamic fee, legacy, access list txs, and the keeper).To avoid drift when adding/removing supported IDs, consider centralizing this into a shared helper (e.g.
types.IsSupportedChainID(*big.Int)) or a shared[]*big.Intconstant, and call that from each validation site. This keeps the single source of truth for supported Hetu chain IDs.rpc/backend/chain_info.go (1)
284-295: Min tip cap behavior matches intent; consider extracting a named constantClamping
maxDeltato a minimum of10_000_000wei (0.01 Gwei) ensuresSuggestGasTipCapnever returns zero once EIP-1559 is active, which should help keep MetaMask in 1559 mode on very low base fees. The placement after the existing negative-guard is sensible.For clarity and future reuse, consider:
- Extracting this into a named constant, e.g.
const minPriorityFeeWei int64 = 10_000_000, with a short doc comment about MetaMask behavior and units.- (Optional, longer-term) If you ever expect base fees that approach
math.MaxInt64, consider rewritingmaxDeltain terms ofbig.Intinstead ofInt64()to avoid potential overflow; this is a pre-existing pattern, but the clamp won’t fix it.init.sh (1)
105-105: Script no longer auto-starts the nodeWith
hetud startcommented out, this script now only initializes and validates the genesis, then exits. That can be desirable for automation, but it’s a behavior change compared to an auto-starting init script.If consumers previously relied on it to start the node, consider updating any docs or wrappers to explicitly start
hetudafter running this script.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
init.sh(3 hunks)rpc/backend/chain_info.go(1 hunks)utils/utils.go(1 hunks)x/evm/keeper/keeper.go(1 hunks)x/evm/types/access_list_tx.go(1 hunks)x/evm/types/dynamic_fee_tx.go(1 hunks)x/evm/types/legacy_tx.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
x/evm/types/legacy_tx.go (1)
types/errors.go (1)
ErrInvalidChainID(27-27)
x/evm/types/dynamic_fee_tx.go (1)
types/errors.go (1)
ErrInvalidChainID(27-27)
x/evm/types/access_list_tx.go (1)
types/errors.go (1)
ErrInvalidChainID(27-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
utils/utils.go (1)
38-40: Mainnet/Testnet chain ID strings now align with numeric IDsSwapping
MainnetChainIDto"hetu_560000"andTestnetChainIDto"hetu_560001"looks consistent with the EIP-155 numeric IDs used elsewhere (560000 mainnet, 560001 testnet). TheIsMainnet/IsTestnetprefix checks will still work correctly with suffixed IDs likehetu_560000-1.init.sh (2)
64-64: Feemarketmin_gas_priceand gentx fees are now consistent
min_gas_priceis set to"100000.0"and the gentx uses--fees 20000000000ahetu --gas 200000, which yields an effective gas price of exactly100000ahetu/gas. That keeps the initial validator transaction in line with the fee market’s minimum and avoids startup failures due to underpriced gentx fees.No functional issues here; just keep these two values in sync if you tune gas economics in the future.
Also applies to: 86-86
71-72: Settingconsensus_params.block.max_gasaftercollect-gentxsis the right orderingCommenting out the earlier
max_gasmutation and applying it afterhetud collect-gentxsavoids the genesis builder overwriting your block gas limit. The jq path and value ("10000000") are unchanged, just moved.This ordering change is sound and should make the configured block gas limit stable across init runs.
Also applies to: 91-93
… default directory that should have been kept. After removing defer, the directory will be left for the caller to use, and cleaning needs to be handled by the caller themselves.
1 participant
Summary
mainnet and testnet.
from falling back to legacy mode when base-fee is very low.
Changes
utils/utils.goMainnetChainID↔TestnetChainIDvaluesrpc/backend/chain_info.gominTipCap = 10 000 000 weiinSuggestGasTipCapBreaking Change
None. The new tip floor only applies when the computed
maxDeltaissmaller than 0.01 Gwei, so existing clients are unaffected.
Test Plan
make test-backendPASSSummary by CodeRabbit
New Features
Chores
Other
✏️ Tip: You can customize this high-level summary in your review settings.