feat: Multi VM Gas Token Flexibility #2384
Conversation
…ling - Added FeegrantKeeper interface in expected_keeper.go for fee management. - Refactored handler_opts.go to remove unused imports and clean up code. - Enhanced gas consumption logic in evmante_gas_consume.go to support WNIBI for transaction fees. - Updated tests in evmante_gas_consume_test.go to validate WNIBI interactions. - Modified evmante_handler.go to accommodate changes in gas consumption handling. - Improved account verification in evmante_verify_eth_acc.go to check WNIBI balance. - Added new tests in evmante_verify_eth_acc_test.go for WNIBI balance checks. - Updated localnet.sh to initialize WNIBI in the genesis state. - Created wnibi.json to define WNIBI parameters and initial balances. - Enhanced setup scripts in evm-e2e/test/setup.ts for multi-account testing. - Added utility functions in evm-e2e/test/utils.ts for deploying WNIBI contracts. - Implemented WNIBI gas tests in evm-e2e/test/wnibi_gas.test.ts to validate gas payment scenarios. - Updated keeper.go to include mock GETH message for EVM operations.
📝 WalkthroughWalkthroughAdds WNIBI/ERC20-backed fee deduction and fallback paths: new DeductFeeDecorator and helpers, Feegrant interface, EVM ante wiring to use AccountKeeper for multi-token gas payment, updated EVM ante verification and tests, localnet WNIBI genesis/script, E2E WNIBI gas test, and a changelog entry. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Mempool as Mempool/CheckTx
participant Ante as AnteChain
participant DFD as DeductFeeDecorator
participant AK as AccountKeeper
participant EK as EvmKeeper
participant BK as BankKeeper
participant FK as FeegrantKeeper
participant FC as FeeCollector
User->>Mempool: Submit Tx (FeeTx)
Mempool->>Ante: Run ante-handlers
Ante->>DFD: Deduct fees
DFD->>DFD: Validate FeeTx & compute fee/priority
alt Fee granter present
DFD->>FK: UseGrantedFees(granter, grantee, fee, msgs)
end
alt Native funds sufficient
DFD->>BK: SendCoins payer -> FC
DFD-->>Ante: success
else Fallback to WNIBI
DFD->>AK: Get fee payer account
DFD->>EK: Query WNIBI balance via StateDB/ERC20
DFD->>EK: Transfer WNIBI payer -> FC
DFD-->>Ante: success or ErrInsufficientFunds
end
Ante-->>Mempool: proceed or reject
sequenceDiagram
actor User
participant Verify as VerifyEthAcc (CheckTx)
participant BK as Bank
participant EK as EvmKeeper
participant WN as WNIBI Contract
User->>Verify: Submit Ethereum Tx
Verify->>BK: Get native balance(sender)
alt Native sufficient
Verify-->>User: ok
else
Verify->>EK: Init temp StateDB/EVM (mock message)
Verify->>WN: balanceOf(sender)
alt WNIBI balance >= cost
Verify-->>User: ok
else
Verify-->>User: ErrInsufficientFunds
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2384 +/- ##
==========================================
- Coverage 56.95% 56.54% -0.42%
==========================================
Files 335 336 +1
Lines 27445 27691 +246
==========================================
+ Hits 15632 15657 +25
- Misses 10740 10958 +218
- Partials 1073 1076 +3
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (15)
CHANGELOG.md (1)
62-62: Changelog entry: add scope and hyphenation for consistency.Prefer subsystem-scoped, hyphenated entries.
-- [#2384](https://github.com/NibiruChain/nibiru/pull/2384) - feat: Multi VM Gas Token Flexibility +- [#2384](https://github.com/NibiruChain/nibiru/pull/2384) - feat(ante,evm): Multi-VM gas token flexibility (WNIBI fallback)x/evm/keeper/keeper.go (1)
9-11: Import alias: prefer gethtypes for clarity; drop unused core if possible.If core is only used for the interface type in MOCK_GETH_MESSAGE, keep it; otherwise remove.
- "github.com/ethereum/go-ethereum/core" - gethcore "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/core" + gethcore "github.com/ethereum/go-ethereum/core/types" // aka gethtypesevm-e2e/test/setup.ts (2)
8-10: Fail fast on missing env; avoid non-null assertion on MNEMONIC.Improves DX and avoids cryptic runtime errors.
-const provider = new ethers.JsonRpcProvider(process.env.JSON_RPC_ENDPOINT) -const mnemonic = Mnemonic.fromPhrase(process.env.MNEMONIC!) -const account = HDNodeWallet.fromMnemonic(mnemonic, "m/44'/60'/0'/0/0").connect(provider) +const rpc = process.env.JSON_RPC_ENDPOINT +if (!rpc) throw new Error("JSON_RPC_ENDPOINT is required") +const provider = new ethers.JsonRpcProvider(rpc) +const phrase = process.env.MNEMONIC +if (!phrase) throw new Error("MNEMONIC is required") +const mnemonic = Mnemonic.fromPhrase(phrase) +const account = HDNodeWallet.fromMnemonic(mnemonic, "m/44'/60'/0'/0/0").connect(provider)
10-10: Keep derivations together; minor readability nit.Co-locate account2 derivation next to account creation after the env checks.
-const account2 = HDNodeWallet.fromMnemonic(mnemonic, "m/44'/60'/0'/0/1").connect(provider) +const account2 = HDNodeWallet.fromMnemonic(mnemonic, "m/44'/60'/0'/0/1").connect(provider)contrib/wnibi.json (1)
7-13: Static account_number in EthAccount; prefer 0 or omit to avoid collisions.Hardcoding "12" can conflict with other genesis accounts in merges.
- "account_number": "12", + "account_number": "0",evm-e2e/test/wnibi_gas.test.ts (1)
1-6: Remove unused imports to satisfy strict TS builds.
toBigInt,account, andTX_WAIT_TIMEOUTare unused.-import { parseUnits, toBigInt, Wallet } from "ethers" +import { parseUnits, Wallet } from "ethers" -import { account, account2, provider, TEST_TIMEOUT, TX_WAIT_TIMEOUT } from "./setup" +import { account2, provider, TEST_TIMEOUT } from "./setup"app/evmante/evmante_verify_eth_acc.go (1)
99-102: Improve error message to reflect both native and WNIBI balances.Aids debugging when fallback was attempted.
- return ctx, sdkioerrors.Wrapf( - sdkerrors.ErrInsufficientFunds, - "sender balance < tx cost (%s < %s)", acct.BalanceNative.ToBig(), cost, - ) + return ctx, sdkioerrors.Wrapf( + sdkerrors.ErrInsufficientFunds, + "sender funds < tx cost (native: %s, wnibi: %s, cost: %s)", + acct.BalanceNative.ToBig(), wnibiBal, cost, + )evm-e2e/test/utils.ts (2)
3-10: Add getAddress import for address normalization/validation.You call provider.getCode with user‑supplied addresses. Normalize early to avoid checksum/format issues and reduce noisy warnings.
-import { - ethers, - ContractFactory, - ContractTransactionResponse, - parseEther, - toBigInt, - Wallet, - type TransactionRequest, -} from "ethers" +import { + ethers, + getAddress, + ContractFactory, + ContractTransactionResponse, + parseEther, + toBigInt, + Wallet, + type TransactionRequest, +} from "ethers"
110-138: Harden reuse path: normalize address and prefer factory.attach over new ethers.Contract.
- Normalize maybeAddress and short‑circuit on valid code.
- factory.attach keeps signer/bytecode context consistent.
export const deployContractWNIBIIfNeeded = async ( maybeAddress?: string ): Promise<{ contract: WNIBI & DeploymentTx }> => { const { abi, bytecode } = WNIBI_JSON const factory = new ContractFactory(abi, bytecode, account) - let contract: ethers.Contract + let contract: ethers.Contract if (maybeAddress) { try { - const code = await account.provider.getCode(maybeAddress) + const addr = getAddress(maybeAddress) + const code = await account.provider.getCode(addr) if (code !== "0x") { // Contract already deployed - console.log(`Contract already deployed at ${maybeAddress}`) - contract = new ethers.Contract(maybeAddress, abi, account) + console.log(`Contract already deployed at ${addr}`) + contract = factory.attach(addr) as unknown as ethers.Contract return { contract: contract as unknown as WNIBI & DeploymentTx } } } catch (error) { console.warn(`Failed to check code at ${maybeAddress}:`, error) // Fall through to deploy new contract } } contract = (await factory.deploy()) as ethers.Contract await contract.waitForDeployment() return { contract: contract as unknown as WNIBI & DeploymentTx } }app/evmante/evmante_gas_consume.go (1)
4-17: Remove unused fmt import after switching to SDK error wrapping.-import ( - "fmt" - "math" +import ( + "math"app/evmante/evmante_verify_eth_acc_test.go (1)
33-45: Avoid magic storage slot (“3”) for WNIBI balances; document or centralize.Hard‑coding the mapping slot is brittle if WNIBI storage layout changes. Define a named const near usage (or in a shared testutil) and reference it.
- slot := CalcMappingSlot(deps.Sender.EthAddr, 3) + const balancesSlot uint64 = 3 // WNIBI: mapping(address => uint) balanceOf storage slot + slot := CalcMappingSlot(deps.Sender.EthAddr, balancesSlot)Also, please confirm slot index 3 matches the compiled WNIBI.sol layout being used in tests.
app/evmante/evmante_gas_consume_test.go (2)
40-55: Mirror the storage‑slot constant to reduce brittleness.Use a named const for the WNIBI balances slot rather than literal 3 to make intent clear and future changes localized.
- slot := CalcMappingSlot(deps.Sender.EthAddr, 3) + const balancesSlot uint64 = 3 + slot := CalcMappingSlot(deps.Sender.EthAddr, balancesSlot)
107-111: Deduplicate CalcMappingSlot across tests.It’s duplicated in multiple test files. Extract to a shared test helper (e.g., x/evm/evmtest/testutil/storage.go) and import from there.
app/ante/deductfee.go (2)
58-69: Remove duplicate zero-height guard in AnteHandle.The block-height==0 early return appears twice; keep only the first to avoid confusion.
- if ctx.BlockHeight() == 0 { - return next(ctx, tx, simulate) - }
141-177: Consider restricting WNIBI fallback to insufficient-funds only.DeductFees falls back to WNIBI for any bank error. For clearer semantics and less surprising behavior, gate the fallback on ErrInsufficientFunds (similar to the EVM ante suggestion).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
CHANGELOG.md(1 hunks)app/ante.go(1 hunks)app/ante/deductfee.go(1 hunks)app/ante/expected_keeper.go(1 hunks)app/ante/handler_opts.go(0 hunks)app/evmante/evmante_gas_consume.go(3 hunks)app/evmante/evmante_gas_consume_test.go(5 hunks)app/evmante/evmante_handler.go(1 hunks)app/evmante/evmante_verify_eth_acc.go(1 hunks)app/evmante/evmante_verify_eth_acc_test.go(4 hunks)contrib/scripts/localnet.sh(1 hunks)contrib/wnibi.json(1 hunks)evm-e2e/test/setup.ts(1 hunks)evm-e2e/test/utils.ts(2 hunks)evm-e2e/test/wnibi_gas.test.ts(1 hunks)x/evm/keeper/keeper.go(2 hunks)
💤 Files with no reviewable changes (1)
- app/ante/handler_opts.go
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2024-10-31T18:28:26.466Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2095
File: x/evm/keeper/bank_extension.go:0-0
Timestamp: 2024-10-31T18:28:26.466Z
Learning: When implementing the `bankkeeper.Keeper` interface in `x/evm/keeper/bank_extension.go`, methods must use non-pointer receivers to satisfy the interface requirements.
Applied to files:
app/ante/expected_keeper.go
📚 Learning: 2024-07-27T01:11:54.422Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#1680
File: x/perp/v2/keeper/msg_server.go:210-210
Timestamp: 2024-07-27T01:11:54.422Z
Learning: The user indicated that error handling for `sdk.AccAddressFromBech32(msg.Sender)` is covered by prior `ValidateBasic` checks in the code flow.
Applied to files:
app/evmante/evmante_verify_eth_acc.go
📚 Learning: 2025-05-21T21:47:04.451Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2315
File: app/upgrades/v2_5_0/v2_5_0.go:0-0
Timestamp: 2025-05-21T21:47:04.451Z
Learning: In the v2.5.0 upgrade handler for Nibiru Chain, excess stNIBI ERC20 token balances (those not belonging to explicitly tracked holders) are sent to a Nibiru team-controlled Gnosis Safe for redistribution to ensure no tokens are lost during contract upgrades.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-09-07T16:02:13.748Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2345
File: x/evm/keeper/msg_convert_evm_to_coin.go:311-315
Timestamp: 2025-09-07T16:02:13.748Z
Learning: WNIBI (Wrapped NIBI) does not use module account escrow like regular FunTokens. WNIBI works through direct contract interactions where users deposit/withdraw native NIBI directly to/from the WNIBI contract. The fund transfers use SendCoins between user accounts, not SendCoinsFromModuleToAccount like regular FunToken conversions.
Applied to files:
contrib/wnibi.json
📚 Learning: 2024-12-06T16:10:34.174Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2119
File: x/evm/keeper/bank_extension.go:0-0
Timestamp: 2024-12-06T16:10:34.174Z
Learning: In `x/evm/keeper/bank_extension.go`, assigning a new `GasMeter` to the context within the `NibiruBankKeeper.ForceGasInvariant` function is intentional and acceptable. This ensures that subsequent operations do not affect the parent `ctx`'s gas consumption.
Applied to files:
app/evmante/evmante_gas_consume.goapp/evmante/evmante_gas_consume_test.go
📚 Learning: 2024-12-06T11:54:38.275Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2119
File: x/evm/keeper/bank_extension_test.go:67-71
Timestamp: 2024-12-06T11:54:38.275Z
Learning: When writing tests in `x/evm/keeper/bank_extension_test.go`, prefer comparing gas consumption values as formatted strings using `fmt.Sprintf("%d", value)` instead of numeric values directly in `s.Equalf`. This avoids hexadecimal representations that are hard to read in shell outputs.
Applied to files:
app/evmante/evmante_gas_consume_test.go
🧬 Code graph analysis (12)
app/ante/expected_keeper.go (1)
x/common/testutil/sample.go (1)
AccAddress(24-27)
app/evmante/evmante_verify_eth_acc.go (3)
x/evm/statedb/statedb.go (1)
StateDB(45-85)x/evm/statedb/config.go (1)
TxConfig(15-20)x/evm/keeper/keeper.go (1)
MOCK_GETH_MESSAGE(187-200)
app/evmante/evmante_verify_eth_acc_test.go (3)
x/evm/evmtest/test_deps.go (1)
TestDeps(27-33)eth/eth_account.go (1)
EthAddrToNibiruAddr(15-17)app/evmante/evmante_gas_consume_test.go (1)
CalcMappingSlot(107-111)
x/evm/keeper/keeper.go (2)
x/evm/const.go (2)
EVM_MODULE_ADDRESS(125-125)Big0(236-236)x/evm/tx_data_access_list.go (1)
AccessList(18-18)
evm-e2e/test/wnibi_gas.test.ts (1)
evm-e2e/test/utils.ts (1)
deployContractWNIBIIfNeeded(110-138)
evm-e2e/test/utils.ts (3)
evm-core-ts/src/ethers/typechain/index.ts (1)
WNIBI(14-14)evm-core-ts/src/ethers/typechain/WNIBI.ts (1)
WNIBI(158-367)evm-e2e/test/setup.ts (1)
account(16-16)
app/evmante/evmante_handler.go (1)
app/evmante/evmante_gas_consume.go (1)
NewAnteDecEthGasConsume(28-38)
app/evmante/evmante_gas_consume.go (1)
app/ante/deductfee.go (1)
DeductFeesWithWNIBI(180-227)
app/ante.go (3)
app/ante/deductfee.go (1)
NewDeductFeeDecorator(34-46)x/evm/deps.go (1)
AccountKeeper(11-32)app/ante/expected_keeper.go (1)
FeegrantKeeper(8-10)
app/ante/deductfee.go (7)
x/evm/keeper/keeper.go (2)
Keeper(30-57)MOCK_GETH_MESSAGE(187-200)app/ante/expected_keeper.go (1)
FeegrantKeeper(8-10)app/appconst/appconst.go (1)
BondDenom(22-22)eth/gas_limit.go (1)
NewInfiniteGasMeter(54-56)x/evm/statedb/statedb.go (1)
StateDB(45-85)eth/eth_account.go (1)
NibiruAddrToEthAddr(19-21)x/evm/const.go (1)
NativeToWei(142-145)
contrib/scripts/localnet.sh (1)
contrib/scripts/chaosnet.sh (1)
add_genesis_param(39-45)
app/evmante/evmante_gas_consume_test.go (6)
x/evm/evmtest/test_deps.go (1)
TestDeps(27-33)x/evm/statedb/statedb.go (1)
StateDB(45-85)eth/eth_account.go (1)
EthAddrToNibiruAddr(15-17)x/evm/const.go (1)
NativeToWei(142-145)x/evm/evmtest/evmante.go (1)
HappyCreateContractTx(69-81)app/evmante/evmante_gas_consume.go (1)
NewAnteDecEthGasConsume(28-38)
🔇 Additional comments (9)
contrib/wnibi.json (2)
47-51: Confirm canonical_wnibi correctness across environments.Ensure this address matches the deployed WNIBI for localnet; mismatch breaks WNIBI fallback.
19-43: Code hash verified — no action required.
Keccak256(code) equals auth.accounts[0].code_hash in contrib/wnibi.json (0xffb88e0eb48147949565e65de3ec8a54b746214da7b9dd5b9a8a3ae7df46193b).contrib/scripts/localnet.sh (1)
231-233: Make merges idempotent — dedupe accounts/balancesEnsure repeated runs don’t create duplicate accounts or balances; use jq unique_by on accounts and balances.
File: contrib/scripts/localnet.sh Lines: 231-233
-jq --slurpfile evm contrib/wnibi.json '.app_state.evm = $evm[0].evm | .app_state.auth.accounts += $evm[0].auth.accounts |.app_state.bank.balances += $evm[0].bank.balances' $CHAIN_DIR/config/genesis.json > $CHAIN_DIR/config/genesis_tmp.json +jq --slurpfile evm contrib/wnibi.json ' + .app_state.evm = $evm[0].evm + | .app_state.auth.accounts += $evm[0].auth.accounts + | .app_state.auth.accounts |= unique_by(.base_account.address // .address) + | .app_state.bank.balances += $evm[0].bank.balances + | .app_state.bank.balances |= unique_by(.address) +' $CHAIN_DIR/config/genesis.json > $CHAIN_DIR/config/genesis_tmp.json mv $CHAIN_DIR/config/genesis_tmp.json $CHAIN_DIR/config/genesis.jsonVerification couldn’t be performed here — $HOME/.nibid/config/genesis.json was not present in the verification environment. After applying the patch, run the supplied supply-check locally to confirm balances sum equals supply.
x/evm/keeper/keeper.go (1)
187-200: MOCK_GETH_MESSAGE construction is invalid; use types.NewMessage to build a core.Message.core.Message is an interface in recent geth; composite literal won’t compile. Build via types.NewMessage and assign to core.Message.
-var MOCK_GETH_MESSAGE = core.Message{ - To: nil, - From: evm.EVM_MODULE_ADDRESS, - Nonce: 0, - Value: evm.Big0, // amount - GasLimit: 0, - GasPrice: evm.Big0, - GasFeeCap: evm.Big0, - GasTipCap: evm.Big0, - Data: []byte{}, - AccessList: gethcore.AccessList{}, - SkipNonceChecks: false, - SkipFromEOACheck: false, -} +var MOCK_GETH_MESSAGE core.Message = gethcore.NewMessage( + evm.EVM_MODULE_ADDRESS, // from + nil, // to + 0, // nonce + evm.Big0, // value + 0, // gasLimit + evm.Big0, // gasPrice + evm.Big0, // gasFeeCap + evm.Big0, // gasTipCap + []byte{}, // data + gethcore.AccessList{}, // access list + false, // checkNonce +)Likely an incorrect or invalid review comment.
app/evmante/evmante_verify_eth_acc.go (1)
73-77: Short‑circuit on sufficient native balance is correct.This preserves the fast path and only triggers WNIBI fallback when needed.
app/evmante/evmante_handler.go (1)
22-23: Approve — AccountKeeper wiring and ante ordering OK; call sites updated. Confirmed NewAnteDecEthGasConsume is invoked with the three-arg signature in app/evmante/evmante_handler.go and app/evmante/evmante_gas_consume_test.go; ante-chain ordering matches Ethermint.app/ante/expected_keeper.go (1)
7-10: Interface shape LGTM — verify concrete keeper implements UseGrantedFees
- Wiring: app/keepers/all_keepers.go:44 sets FeeGrantKeeper = feegrantkeeper.Keeper and it's passed into NewDeductFeeDecorator in app/ante.go:82; call site using the method: app/ante/deductfee.go:107.
- No local implementation found in this repo; confirm github.com/cosmos/cosmos-sdk/x/feegrant/keeper.Keeper exposes UseGrantedFees(ctx sdk.Context, granter, grantee sdk.AccAddress, fee sdk.Coins, msgs []sdk.Msg) error or run go build to validate.
app/ante.go (1)
82-84: Approve: DeductFeeDecorator placement — verify EvmKeeper non‑nilGood integration of EvmKeeper for WNIBI fallback. Ran the grep you suggested — EvmKeeper is declared as a *evmkeeper.Keeper (app/ante/handler_opts.go) and used in NewAnteHandlerNonEVM (app/ante.go:82); app/app.go passes app.EvmKeeper into ante options. Confirm NewNibiruApp always instantiates app.EvmKeeper (search for evmkeeper.NewKeeper or an assignment to app.EvmKeeper); if it can be nil, add a nil-guard before calling NewDeductFeeDecorator or make the decorator handle a nil EvmKeeper.
evm-e2e/test/utils.ts (1)
120-129: Confirm expectations when canonical address has no code.If the canonical WNIBI isn’t predeployed, this helper deploys to a different address, which may break tests relying on the canonical address being used. Confirm tests only depend on the returned contract and not on maybeAddress being honored for new deployments.
…BI transfer tests
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/evmante/evmante_verify_eth_acc.go (2)
81-87: Critical: don’t clobber global Bank.StateDB; restore original instead.The defer unconditionally sets Bank.StateDB to nil even if it was non‑nil, breaking subsequent EVM usage. Capture and restore the original value.
- stateDB := anteDec.evmKeeper.Bank.StateDB - if stateDB == nil { - stateDB = anteDec.evmKeeper.NewStateDB(ctx, anteDec.evmKeeper.TxConfig(ctx, gethcommon.Hash{})) - } - defer func() { - anteDec.evmKeeper.Bank.StateDB = nil - }() + origStateDB := anteDec.evmKeeper.Bank.StateDB + stateDB := origStateDB + if stateDB == nil { + stateDB = anteDec.evmKeeper.NewStateDB(ctx, anteDec.evmKeeper.TxConfig(ctx, gethcommon.Hash{})) + } + defer func() { + anteDec.evmKeeper.Bank.StateDB = origStateDB + }()
79-90: Guard against unset CanonicalWnibi and handle BalanceOf errors deterministically.If CanonicalWnibi isn’t configured or BalanceOf errors, don’t proceed with fallback; return the original insufficient funds error.
- wnibi := anteDec.evmKeeper.GetParams(ctx).CanonicalWnibi + wnibi := anteDec.evmKeeper.GetParams(ctx).CanonicalWnibi + if wnibi.Address == (gethcommon.Address{}) { + // WNIBI not configured: keep original InsufficientFunds semantics + return ctx, sdkerrors.ErrInsufficientFunds + } @@ - wnibiBal, err := anteDec.evmKeeper.ERC20().BalanceOf(wnibi.Address, fromAddr, ctx, evmObj) + wnibiBal, balErr := anteDec.evmKeeper.ERC20().BalanceOf(wnibi.Address, fromAddr, ctx, evmObj) + if balErr != nil { + // Don’t mask errors from BalanceOf as sufficient funds + return ctx, sdkerrors.ErrInsufficientFunds + }
🧹 Nitpick comments (2)
app/evmante/evmante_verify_eth_acc.go (1)
97-100: Preserve typed insufficient-funds error; consider enhancing context.Typed ErrInsufficientFunds is good. Optionally include both native and WNIBI balances in the message to aid diagnostics.
app/evmante/evmante_gas_consume.go (1)
181-191: WNIBI deduction flow: mostly good; preserve detailed error when available.If DeductFeesWithWNIBI returns a typed ErrInsufficientFunds, rewrapping loses context. Prefer returning err as-is when it’s already classified.
- err = ante.DeductFeesWithWNIBI(ctx, anteDec.accountKeeper, anteDec.evmKeeper, acc, fees) - if err == nil { - return nil - } - - return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "insufficient balance across supported gas tokens to cover %s", fees[0].Amount) + err = ante.DeductFeesWithWNIBI(ctx, anteDec.accountKeeper, anteDec.evmKeeper, acc, fees) + if err == nil { + return nil + } + // Preserve original typed error if it’s already ErrInsufficientFunds + if sdkioerrors.IsOf(err, sdkerrors.ErrInsufficientFunds) { + return err + } + return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFunds, + "insufficient balance across supported gas tokens to cover %s: %v", + fees[0].Amount, err)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/evmante/evmante_gas_consume.go(3 hunks)app/evmante/evmante_verify_eth_acc.go(1 hunks)evm-e2e/test/wnibi_gas.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- evm-e2e/test/wnibi_gas.test.ts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2024-10-31T18:28:26.466Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2095
File: x/evm/keeper/bank_extension.go:0-0
Timestamp: 2024-10-31T18:28:26.466Z
Learning: When implementing the `bankkeeper.Keeper` interface in `x/evm/keeper/bank_extension.go`, methods must use non-pointer receivers to satisfy the interface requirements.
Applied to files:
app/evmante/evmante_verify_eth_acc.go
📚 Learning: 2024-12-06T16:09:01.767Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2119
File: x/evm/keeper/bank_extension_test.go:161-165
Timestamp: 2024-12-06T16:09:01.767Z
Learning: In Go test assertions within `x/evm/keeper/bank_extension_test.go`, when comparing `uint64` values using `s.Equalf`, prefer formatting the values as decimal strings with `fmt.Sprintf("%d", value)` to avoid them being printed in hexadecimal.
Applied to files:
app/evmante/evmante_verify_eth_acc.go
📚 Learning: 2024-12-06T16:10:34.174Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2119
File: x/evm/keeper/bank_extension.go:0-0
Timestamp: 2024-12-06T16:10:34.174Z
Learning: In `x/evm/keeper/bank_extension.go`, assigning a new `GasMeter` to the context within the `NibiruBankKeeper.ForceGasInvariant` function is intentional and acceptable. This ensures that subsequent operations do not affect the parent `ctx`'s gas consumption.
Applied to files:
app/evmante/evmante_verify_eth_acc.goapp/evmante/evmante_gas_consume.go
📚 Learning: 2024-12-06T11:54:38.275Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2119
File: x/evm/keeper/bank_extension_test.go:67-71
Timestamp: 2024-12-06T11:54:38.275Z
Learning: When writing tests in `x/evm/keeper/bank_extension_test.go`, prefer comparing gas consumption values as formatted strings using `fmt.Sprintf("%d", value)` instead of numeric values directly in `s.Equalf`. This avoids hexadecimal representations that are hard to read in shell outputs.
Applied to files:
app/evmante/evmante_gas_consume.go
📚 Learning: 2024-11-04T20:38:45.909Z
Learnt from: k-yang
PR: NibiruChain/nibiru#2093
File: x/evm/precompile/funtoken.go:85-87
Timestamp: 2024-11-04T20:38:45.909Z
Learning: In the `Run` methods of precompiled contracts in `x/evm/precompile/`, after handling errors from `OnRunStart`, it's unnecessary to check if `err` is nil before consuming gas with `contract.UseGas()`, as `err` will always be nil at that point.
Applied to files:
app/evmante/evmante_gas_consume.go
🧬 Code graph analysis (2)
app/evmante/evmante_verify_eth_acc.go (3)
x/evm/statedb/statedb.go (1)
StateDB(45-85)x/evm/statedb/config.go (1)
TxConfig(15-20)x/evm/keeper/keeper.go (1)
MOCK_GETH_MESSAGE(187-200)
app/evmante/evmante_gas_consume.go (2)
app/evmante/interfaces.go (1)
EVMKeeper(10-10)app/ante/deductfee.go (1)
DeductFeesWithWNIBI(180-227)
🔇 Additional comments (5)
app/evmante/evmante_gas_consume.go (4)
12-12: New ante import: LGTM.Importing app/ante to use DeductFeesWithWNIBI is appropriate.
21-24: Struct field additions: LGTM.Wiring AccountKeeper into the decorator aligns with the new deduction path.
171-179: Correctly gate WNIBI fallback only on insufficient funds.This prevents masking real bank errors. Good fix.
27-36: Constructor signature change: LGTM — call sites updated.
- Updated call sites: app/evmante/evmante_handler.go:22; app/evmante/evmante_gas_consume_test.go:84-86.
app/evmante/evmante_verify_eth_acc.go (1)
92-95: Compare WNIBI against gas-only fees, not tx Cost().txData.Cost() = value + gas; WNIBI only pays fees. Replace the Cost() comparison with a fee-only verification (convert returned fee to wei) and separately ensure the sender’s native/EVM balance covers tx value.
File: app/evmante/evmante_verify_eth_acc.go:92-95
- cost := txData.Cost() - if wnibi.Address != (gethcommon.Address{}) && err == nil && wnibiBal.Cmp(cost) >= 0 { - continue - } + fees, feeErr := keeper.VerifyFee(txData, anteDec.evmKeeper.BaseFeeMicronibiPerGas(ctx), ctx) + if feeErr != nil { + return ctx, sdkioerrors.Wrap(feeErr, "failed to verify fees for WNIBI fallback") + } + gasWei := evm.NativeToWei(fees[0].Amount.BigInt()) + if wnibiBal.Cmp(gasWei) >= 0 { + continue + }Follow-up: enforce native balance covers tx value when WNIBI only covers gas; add tests for value>0 with gas in WNIBI and value>0 without native balance.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
evm-e2e/test/setup.ts (1)
8-12: Consider safer environment variable handling.The non-null assertion on
process.env.MNEMONIC!will throw a runtime error if the environment variable is undefined. Consider adding validation or a fallback to provide a clearer error message in development environments.-const mnemonic = Mnemonic.fromPhrase(process.env.MNEMONIC!) +if (!process.env.MNEMONIC) { + throw new Error("MNEMONIC environment variable is required") +} +const mnemonic = Mnemonic.fromPhrase(process.env.MNEMONIC)app/evmante/evmante_verify_eth_acc_test.go (1)
43-43: Consider optimizing WNIBI address retrieval.Based on the past review comment, you could potentially remove the explicit
wnibiparameter and retrieve the WNIBI address from the context. However, the current approach is clear and functional for testing purposes.Based on learnings
x/evm/mock_msg.go (1)
8-21: Use Go‑idiomatic exported name and add a brief doc commentPublic identifiers should be MixedCaps. Also document intended usage to avoid misuse outside tests.
-var MOCK_GETH_MESSAGE = core.Message{ +// MockGethMessage is a zero-gas, module-signed message used for read-only EVM calls in tests/utilities. +var MockGethMessage = core.Message{Follow-up: update call sites (e.g., evmante) to evm.MockGethMessage.
contrib/scripts/localnet.sh (1)
247-248: Merge from the temp file and clean up; recompute supply after merge-jq --slurpfile evm contrib/wnibi.json '.app_state.evm = $evm[0].evm | .app_state.auth.accounts += $evm[0].auth.accounts |.app_state.bank.balances += $evm[0].bank.balances' $CHAIN_DIR/config/genesis.json > $CHAIN_DIR/config/genesis_tmp.json +jq --slurpfile evm "$WNIBI_TMP" ' + .app_state.evm = $evm[0].evm + | .app_state.auth.accounts += $evm[0].auth.accounts + | .app_state.bank.balances += $evm[0].bank.balances +' "$CHAIN_DIR/config/genesis.json" > "$CHAIN_DIR/config/genesis_tmp.json" mv $CHAIN_DIR/config/genesis_tmp.json $CHAIN_DIR/config/genesis.json +rm -f "$WNIBI_TMP" + +# Recompute bank supply now that balances changed +jq ' + .app_state.bank.supply = + ( + [ .app_state.bank.balances[].coins[] ] + | sort_by(.denom) + | group_by(.denom) + | map({denom: .[0].denom, amount: (map(.amount|tonumber) | add | tostring)}) + ) +' $CHAIN_DIR/config/genesis.json > $CHAIN_DIR/config/genesis_tmp.json && \ +mv $CHAIN_DIR/config/genesis_tmp.json $CHAIN_DIR/config/genesis.jsoncontrib/wnibi.json (1)
7-13: Avoid hardcoding account_number in committed JSONSince the script sets account_number dynamically, commit this as "0" or omit the field to reduce accidental conflicts. If you keep it, document that localnet.sh overwrites it.
- "account_number": "3", + "account_number": "0",app/evmante/evmante_verify_eth_acc.go (2)
89-95: Use the new evm.MockGethMessage (if renamed) and guard EVM init failuresIf NewEVM can error via config/state issues, surface that instead of proceeding with a nil evmObj.
64-68: Verify native-balance path when EVM account is nilCreating a Cosmos account then using NewEmptyAccount() sets BalanceNative=0, which forces WNIBI fallback even if the user holds unibi. Confirm this is intended or populate acct.BalanceNative from bank for first-time EOAs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/ante/deductfee.go(1 hunks)app/evmante/evmante_verify_eth_acc.go(1 hunks)app/evmante/evmante_verify_eth_acc_test.go(3 hunks)contrib/scripts/localnet.sh(1 hunks)contrib/wnibi.json(1 hunks)evm-e2e/test/setup.ts(1 hunks)evm-e2e/test/utils.ts(2 hunks)evm-e2e/test/wnibi_gas.test.ts(1 hunks)x/evm/evmtest/tx.go(1 hunks)x/evm/mock_msg.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- evm-e2e/test/utils.ts
🧰 Additional context used
🧠 Learnings (13)
📚 Learning: 2025-09-07T16:02:13.748Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2345
File: x/evm/keeper/msg_convert_evm_to_coin.go:311-315
Timestamp: 2025-09-07T16:02:13.748Z
Learning: WNIBI (Wrapped NIBI) does not use module account escrow like regular FunTokens. WNIBI works through direct contract interactions where users deposit/withdraw native NIBI directly to/from the WNIBI contract. The fund transfers use SendCoins between user accounts, not SendCoinsFromModuleToAccount like regular FunToken conversions.
Applied to files:
contrib/wnibi.jsonevm-e2e/test/wnibi_gas.test.ts
📚 Learning: 2024-10-31T18:28:26.466Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2095
File: x/evm/keeper/bank_extension.go:0-0
Timestamp: 2024-10-31T18:28:26.466Z
Learning: When implementing the `bankkeeper.Keeper` interface in `x/evm/keeper/bank_extension.go`, methods must use non-pointer receivers to satisfy the interface requirements.
Applied to files:
app/evmante/evmante_verify_eth_acc.go
📚 Learning: 2024-12-06T16:09:01.767Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2119
File: x/evm/keeper/bank_extension_test.go:161-165
Timestamp: 2024-12-06T16:09:01.767Z
Learning: In Go test assertions within `x/evm/keeper/bank_extension_test.go`, when comparing `uint64` values using `s.Equalf`, prefer formatting the values as decimal strings with `fmt.Sprintf("%d", value)` to avoid them being printed in hexadecimal.
Applied to files:
app/evmante/evmante_verify_eth_acc.go
📚 Learning: 2024-12-06T16:10:34.174Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2119
File: x/evm/keeper/bank_extension.go:0-0
Timestamp: 2024-12-06T16:10:34.174Z
Learning: In `x/evm/keeper/bank_extension.go`, assigning a new `GasMeter` to the context within the `NibiruBankKeeper.ForceGasInvariant` function is intentional and acceptable. This ensures that subsequent operations do not affect the parent `ctx`'s gas consumption.
Applied to files:
app/evmante/evmante_verify_eth_acc.go
📚 Learning: 2024-10-08T15:45:59.488Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#1680
File: x/perp/v2/keeper/msg_server.go:210-210
Timestamp: 2024-10-08T15:45:59.488Z
Learning: The user indicated that error handling for `sdk.AccAddressFromBech32(msg.Sender)` is covered by prior `ValidateBasic` checks in the code flow.
Applied to files:
app/evmante/evmante_verify_eth_acc.go
📚 Learning: 2024-10-13T05:48:51.771Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2076
File: x/evm/keeper/msg_server.go:115-116
Timestamp: 2024-10-13T05:48:51.771Z
Learning: In the `ApplyEvmTx` function within `x/evm/keeper/msg_server.go`, a zero value for `weiPerGas` is acceptable as it signifies that gas is free. There is no need to check for zero before performing the gas refund in this context.
Applied to files:
app/evmante/evmante_verify_eth_acc.go
📚 Learning: 2024-10-08T15:45:59.488Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#1680
File: x/perp/v2/keeper/msg_server.go:198-198
Timestamp: 2024-10-08T15:45:59.488Z
Learning: The user Unique-Divine has indicated that the error check for `sdk.AccAddressFromBech32(msg.Sender)` is not necessary in the context of their code because it is covered by the `ValidateBasic` method.
Applied to files:
app/evmante/evmante_verify_eth_acc.go
📚 Learning: 2025-09-16T00:14:44.279Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2385
File: x/evm/keeper/msg_server.go:92-97
Timestamp: 2025-09-16T00:14:44.279Z
Learning: In the EthereumTx function in x/evm/keeper/msg_server.go, the existing pattern of checking `if evmResp != nil` before calling SafeConsumeGas is correct because the control flow already handles the nil case through an early return when there's a consensus error (applyErr != nil && evmResp == nil).
Applied to files:
app/evmante/evmante_verify_eth_acc.go
📚 Learning: 2024-12-06T11:54:38.275Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2119
File: x/evm/keeper/bank_extension_test.go:67-71
Timestamp: 2024-12-06T11:54:38.275Z
Learning: When writing tests in `x/evm/keeper/bank_extension_test.go`, prefer comparing gas consumption values as formatted strings using `fmt.Sprintf("%d", value)` instead of numeric values directly in `s.Equalf`. This avoids hexadecimal representations that are hard to read in shell outputs.
Applied to files:
app/evmante/evmante_verify_eth_acc_test.go
📚 Learning: 2025-09-25T05:01:20.715Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2384
File: contrib/scripts/localnet.sh:225-225
Timestamp: 2025-09-25T05:01:20.715Z
Learning: In Cosmos SDK genesis files, bank.supply should be computed from balances rather than hardcoded, as hardcoded values can become inconsistent when merging additional balance data from external sources like wnibi.json.
Applied to files:
contrib/scripts/localnet.sh
📚 Learning: 2025-09-10T07:13:02.800Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2379
File: x/tokenfactory/types/genesis.go:43-45
Timestamp: 2025-09-10T07:13:02.800Z
Learning: In x/tokenfactory/types/genesis.go, the error handling in GenesisState.Validate() intentionally uses fmt.Errorf with %w to wrap the underlying sdk.AccAddressFromBech32 error rather than wrapping with ErrInvalidGenesis. This allows callers to unwrap and inspect the specific address parsing error while keeping genesis-level errors as contextual strings.
Applied to files:
contrib/scripts/localnet.sh
📚 Learning: 2024-10-01T16:58:59.204Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2054
File: x/evm/embeds/artifacts/contracts/Wasm.sol/IWasm.json:175-198
Timestamp: 2024-10-01T16:58:59.204Z
Learning: In the IWasm interface, the `contractAddr` parameter is a bech32 nibi-prefixed address and should be of type `string` in Solidity, not `address`.
Applied to files:
evm-e2e/test/wnibi_gas.test.ts
📚 Learning: 2024-10-01T16:59:08.812Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2054
File: x/evm/embeds/artifacts/contracts/Wasm.sol/IWasm.json:6-46
Timestamp: 2024-10-01T16:59:08.812Z
Learning: In the `IWasm` interface, the `contractAddr` parameter should be of type `string` because it's a bech32 nibi-prefixed address, not an Ethereum address.
Applied to files:
evm-e2e/test/wnibi_gas.test.ts
🧬 Code graph analysis (7)
app/evmante/evmante_verify_eth_acc.go (3)
x/evm/statedb/statedb.go (1)
StateDB(45-85)x/evm/statedb/config.go (1)
TxConfig(15-20)x/evm/evmtest/tx.go (1)
MOCK_GETH_MESSAGE(383-396)
app/evmante/evmante_verify_eth_acc_test.go (6)
x/evm/evmtest/test_deps.go (1)
TestDeps(27-33)x/evm/const.go (1)
NativeToWei(142-145)x/evm/evmtest/evmante.go (1)
HappyCreateContractTx(69-81)x/common/testutil/testapp/testapp.go (1)
FundAccount(156-165)x/evm/evmtest/tx.go (1)
ExecuteNibiTransferTo(50-68)x/evm/query.pb.go (3)
QueryBalanceRequest(277-280)QueryBalanceRequest(284-284)QueryBalanceRequest(285-287)
contrib/scripts/localnet.sh (1)
contrib/scripts/chaosnet.sh (1)
add_genesis_param(39-45)
x/evm/mock_msg.go (2)
x/evm/const.go (2)
EVM_MODULE_ADDRESS(125-125)Big0(236-236)x/evm/tx_data_access_list.go (1)
AccessList(18-18)
x/evm/evmtest/tx.go (3)
x/evm/evmtest/test_deps.go (1)
TestDeps(27-33)x/evm/tx.pb.go (6)
MsgEthereumTx(40-51)MsgEthereumTx(55-55)MsgEthereumTx(56-58)MsgEthereumTxResponse(333-348)MsgEthereumTxResponse(352-352)MsgEthereumTxResponse(353-355)x/evm/json_tx_args.go (1)
JsonTxArgs(24-46)
evm-e2e/test/wnibi_gas.test.ts (1)
evm-e2e/test/utils.ts (1)
getWNIBI(110-130)
app/ante/deductfee.go (8)
x/evm/keeper/keeper.go (1)
Keeper(27-54)app/ante/expected_keeper.go (1)
FeegrantKeeper(8-10)app/appconst/appconst.go (1)
BondDenom(22-22)x/evm/statedb/statedb.go (1)
StateDB(45-85)x/evm/statedb/config.go (1)
TxConfig(15-20)x/evm/evmtest/tx.go (1)
MOCK_GETH_MESSAGE(383-396)eth/eth_account.go (1)
NibiruAddrToEthAddr(19-21)x/evm/const.go (1)
NativeToWei(142-145)
🪛 Gitleaks (8.28.0)
contrib/wnibi.json
[high] 35-35: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 39-39: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (6)
- GitHub Check: proto-gen-rs
- GitHub Check: build
- GitHub Check: unit-tests
- GitHub Check: integration-tests
- GitHub Check: e2e-evm
- GitHub Check: lint
🔇 Additional comments (14)
x/evm/evmtest/tx.go (1)
50-68: LGTM! Well-designed test helper function.The new
ExecuteNibiTransferTofunction follows the established pattern fromExecuteNibiTransferand provides useful flexibility for testing specific recipient addresses and transfer values. The implementation correctly handles transaction construction, signing, and execution with proper error checking.evm-e2e/test/setup.ts (1)
9-12: Excellent documentation for account derivations.The comments clearly explain the purpose of each account and their respective derivation paths. This addresses the previous review feedback and makes the setup much more understandable for developers.
app/ante/deductfee.go (3)
29-30: Add compile-time interface assertion.Based on previous review feedback, add a compile-time interface assertion to ensure DeductFeeDecorator implements sdk.AnteDecorator correctly.
var ( - _ sdk.AnteDecorator = DeductFeeDecorator{} + _ sdk.AnteDecorator = (*DeductFeeDecorator)(nil) )
155-160: Gas meter manipulation looks correct.The defer function properly restores the gas meter state by refunding consumed gas and then re-consuming the baseline amount. This pattern ensures that EVM operations don't interfere with the ante handler's gas accounting.
183-231: WNIBI fallback implementation is well-structured.The
DeductFeesWithWNIBIfunction properly:
- Retrieves WNIBI contract parameters
- Sets up StateDB for EVM operations
- Checks WNIBI balance using ERC20 interface
- Transfers WNIBI to fee collector via
ConvertEvmToCoinForWNIBI- Updates account sequence correctly
The implementation aligns with the WNIBI fee payment requirements.
app/evmante/evmante_verify_eth_acc_test.go (1)
33-57: Well-designed WNIBI gas payment test case.The new test case properly validates the WNIBI fallback mechanism by:
- Funding the account with WNIBI tokens via
ExecuteNibiTransferTo- Explicitly verifying zero native balance before the transaction
- Testing that gas can be paid using WNIBI when native funds are insufficient
The test setup correctly uses
deps.DeployWNIBIand follows established testing patterns.evm-e2e/test/wnibi_gas.test.ts (3)
19-23: Previous review feedback has been addressed.The code now correctly uses
account2.addressas the recipient for the ERC-20 transfer, addressing the type safety concern from previous reviews.
26-39: Gas fee handling properly implemented.The test now includes explicit fee overrides (
maxFeePerGasandmaxPriorityFeePerGas) to ensure deterministic gas consumption, and usesalice.addressas the recipient. This addresses the previous review feedback about potential test flakiness and type safety.
7-42: Comprehensive WNIBI gas payment validation.This end-to-end test effectively validates the multi-token gas payment feature by:
- Verifying zero native balance initially
- Funding account with WNIBI tokens
- Executing a transfer with explicit gas fees
- Confirming gas was paid from WNIBI balance (balance drops below expected amount)
The test provides good coverage of the WNIBI gas payment flow introduced in this PR.
x/evm/mock_msg.go (1)
13-17: Confirm gas semantics for zero GasLimit/Price in read-only callsIf BalanceOf uses the provided Message gas, GasLimit:0 could cause OOG on some execution paths. If your call path enforces eth_call semantics (no meter), this is fine; otherwise set a small non-zero gas limit.
contrib/wnibi.json (1)
21-44: Sanity check: deployed bytecode and storage are public test fixtures, not secretsGitleaks “Generic API Key” is a false positive on contract storage keys. Ensure this file contains only testnet/localnet data and no production private keys.
contrib/scripts/localnet.sh (1)
225-225: Do not hardcode bank.supply; recompute from balances after merges- add_genesis_param '.app_state.bank.supply[0].amount = "30000420000000"' +# Recompute bank.supply from balances (all denoms) after any merges +jq ' + .app_state.bank.supply = + ( + [ .app_state.bank.balances[].coins[] ] + | sort_by(.denom) + | group_by(.denom) + | map({denom: .[0].denom, amount: (map(.amount|tonumber) | add | tostring)}) + ) +' $CHAIN_DIR/config/genesis.json > $CHAIN_DIR/config/genesis_tmp.json && \ +mv $CHAIN_DIR/config/genesis_tmp.json $CHAIN_DIR/config/genesis.jsonOptional: run again after collect-gentxs to include staking/bonding edits.
#!/bin/bash # Verify supply equals sum(balances) per denom jq -r ' def sum_balances: ([.app_state.bank.balances[].coins[]] | group_by(.denom) | map({denom: .[0].denom, amount: (map(.amount|tonumber)|add)})); {supply: (.app_state.bank.supply|map({denom,amount:(.amount|tonumber)})), sums: sum_balances} ' "$HOME/.nibid/config/genesis.json"app/evmante/evmante_verify_eth_acc.go (2)
73-77: Fallback only on insufficient funds; return other errors- if err := keeper.CheckSenderBalance( - evm.NativeToWei(acct.BalanceNative.ToBig()), txData, - ); err == nil { - continue - } + err := keeper.CheckSenderBalance( + evm.NativeToWei(acct.BalanceNative.ToBig()), txData, + ) + if err == nil { + continue + } + if !sdkioerrors.IsOf(err, sdkerrors.ErrInsufficientFunds) { + return ctx, err + }
81-88: Don’t clobber shared Bank.StateDB; restore originalCurrently the deferred nil assignment can wipe a preexisting StateDB set elsewhere.
- stateDB := anteDec.evmKeeper.Bank.StateDB + origStateDB := anteDec.evmKeeper.Bank.StateDB + stateDB := origStateDB if stateDB == nil { stateDB = anteDec.evmKeeper.NewStateDB(ctx, anteDec.evmKeeper.TxConfig(ctx, gethcommon.Hash{})) } - defer func() { - anteDec.evmKeeper.Bank.StateDB = nil - }() + // No mutation to Bank.StateDB required here; if you ever set it, restore it: + // defer func() { anteDec.evmKeeper.Bank.StateDB = origStateDB }()
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
contrib/scripts/localnet.sh (1)
241-243: Merge safety: dedupe accounts by address; then recompute supply and validate
- The merge can duplicate an existing account. Deduplicate by
.base_account.address.- Immediately recompute
bank.supplyfrom balances (big‑int safe) and rungenesis validate.Apply:
-jq --slurpfile evm contrib/wnibi.json '.app_state.evm = $evm[0].evm | .app_state.auth.accounts += $evm[0].auth.accounts |.app_state.bank.balances += $evm[0].bank.balances' $CHAIN_DIR/config/genesis.json > $CHAIN_DIR/config/genesis_tmp.json +jq --slurpfile evm "$WNIBI_TMP" ' + .app_state.evm = $evm[0].evm + | .app_state.auth.accounts = + (.app_state.auth.accounts + $evm[0].auth.accounts + | unique_by(.base_account.address)) + | .app_state.bank.balances += $evm[0].bank.balances +' "$CHAIN_DIR/config/genesis.json" > "$CHAIN_DIR/config/genesis_tmp.json" mv $CHAIN_DIR/config/genesis_tmp.json $CHAIN_DIR/config/genesis.json + +# Recompute bank.supply (big-int safe) and validate +python - <<'PY' "$CHAIN_DIR/config/genesis.json" +import json,sys +p=sys.argv[1]; g=json.load(open(p)); s={} +for b in g["app_state"]["bank"]["balances"]: + for c in b.get("coins",[]): + s[c["denom"]]=s.get(c["denom"],0)+int(c["amount"]) +g["app_state"]["bank"]["supply"]=[{"denom":d,"amount":str(a)} for d,a in sorted(s.items())] +json.dump(g,open(p,"w"),separators=(", ",": ")) +PY +"$BINARY" genesis validateOptionally, clean up the temp wnibi file after use:
rm -f "$WNIBI_TMP"app/ante/deductfee.go (2)
171-176: Error message clarity for unsupported fee denomIf multiple denoms are supplied, users will see “fee denom must be unibi” even if unibi is present but not at index 0. The earlier single‑denom check mitigates this; ensure message guidance is consistent.
- return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "fee denom must be %s, got %s", appconst.BondDenom, fees[0].Denom) + return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "only %s fees are accepted", appconst.BondDenom)
52-60: Minor: gas > 0 check is good; consider IsReCheckTx short‑circuitOptional: emulate SDK behavior to skip certain checks on ReCheckTx to reduce mempool churn.
- if !simulate && ctx.BlockHeight() > 0 && feeTx.GetGas() == 0 { + if !simulate && !ctx.IsReCheckTx() && ctx.BlockHeight() > 0 && feeTx.GetGas() == 0 { return ctx, sdkioerrors.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/ante/deductfee.go(1 hunks)contrib/scripts/localnet.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-25T05:01:20.715Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2384
File: contrib/scripts/localnet.sh:225-225
Timestamp: 2025-09-25T05:01:20.715Z
Learning: In Cosmos SDK genesis files, bank.supply should be computed from balances rather than hardcoded, as hardcoded values can become inconsistent when merging additional balance data from external sources like wnibi.json.
Applied to files:
contrib/scripts/localnet.sh
📚 Learning: 2025-09-10T07:13:02.800Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2379
File: x/tokenfactory/types/genesis.go:43-45
Timestamp: 2025-09-10T07:13:02.800Z
Learning: In x/tokenfactory/types/genesis.go, the error handling in GenesisState.Validate() intentionally uses fmt.Errorf with %w to wrap the underlying sdk.AccAddressFromBech32 error rather than wrapping with ErrInvalidGenesis. This allows callers to unwrap and inspect the specific address parsing error while keeping genesis-level errors as contextual strings.
Applied to files:
contrib/scripts/localnet.sh
📚 Learning: 2024-10-31T09:55:25.630Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2097
File: x/oracle/keeper/keeper.go:195-195
Timestamp: 2024-10-31T09:55:25.630Z
Learning: In `x/oracle/keeper/keeper.go`, within the `GetDatedExchangeRate` method, it's not possible for `exchangeRate.CreatedBlock` to be in the future relative to `ctx.BlockHeight()`, so additional validation before time calculation is unnecessary.
Applied to files:
app/ante/deductfee.go
🧬 Code graph analysis (2)
contrib/scripts/localnet.sh (1)
contrib/scripts/chaosnet.sh (1)
add_genesis_param(39-45)
app/ante/deductfee.go (8)
x/evm/keeper/keeper.go (1)
Keeper(27-54)app/ante/expected_keeper.go (1)
FeegrantKeeper(8-10)app/appconst/appconst.go (1)
BondDenom(22-22)x/evm/statedb/statedb.go (1)
StateDB(45-85)x/evm/statedb/config.go (1)
TxConfig(15-20)x/evm/mock_msg.go (1)
MOCK_GETH_MESSAGE(8-21)eth/eth_account.go (1)
NibiruAddrToEthAddr(19-21)x/evm/const.go (1)
NativeToWei(142-145)
⏰ 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). (6)
- GitHub Check: integration-tests
- GitHub Check: e2e-evm
- GitHub Check: lint
- GitHub Check: proto-gen-rs
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (4)
contrib/scripts/localnet.sh (3)
236-240: Do not mutate contrib/wnibi.json; write to a temp file and keep repo cleanEditing files under contrib/ in place dirties the working tree and is fragile across runs. Generate a temp copy with the computed account_number and use that for merging.
Apply:
-echo_info "Setting WNIBI auth account_number to next open index" -NEXT_ACC_NUM=$(get_auth_accounts_len) -jq ".auth.accounts[0].base_account.account_number=\"${NEXT_ACC_NUM}\"" contrib/wnibi.json > contrib/wnibi.tmp.json -mv contrib/wnibi.tmp.json contrib/wnibi.json +echo_info "Preparing WNIBI json with computed account_number" +NEXT_ACC_NUM=$(get_next_open_account_number) +WNIBI_TMP="$(mktemp "${CHAIN_DIR}/wnibi.XXXXXX.json")" +jq --argjson n "$NEXT_ACC_NUM" \ + '.auth.accounts[0].base_account.account_number = ($n|tostring)' \ + contrib/wnibi.json > "$WNIBI_TMP"
227-230: Length ≠ next account_number; derive max(account_number)+1Using
.auth.accounts | lengthcan collide if numbers are sparse. Compute the next open number from the maximum existing account_number instead.Apply:
get_auth_accounts_len() { - cat $CHAIN_DIR/config/genesis.json | jq '.app_state.auth.accounts | length' + jq '.app_state.auth.accounts | length' "$CHAIN_DIR/config/genesis.json" } + +# Next open auth account_number = max(existing)+1 (or 0 if none) +get_next_open_account_number() { + jq -r ' + ( [ .app_state.auth.accounts[] + | ( .base_account.account_number // .account_number // "0" ) | tonumber + ] ) as $nums + | ( ($nums|length) == 0 ) ? 0 : (($nums|max) + 1) + ' "$CHAIN_DIR/config/genesis.json" +}And update usage at line 237 to call
get_next_open_account_number(see next comment).#!/bin/bash jq -r '[.app_state.auth.accounts[]|.base_account.account_number//"0"|tonumber]|max' "$CHAIN_DIR/config/genesis.json"
225-225: Request manual verification of genesis validation
Ensure theBINARYenvironment variable points to your node binary (e.g.nibid) and then re-run:export BINARY="/path/to/your/binary" "$BINARY" genesis validate --home "$CHAIN_DIR"Confirm that the command exits cleanly and reports no validation errors.
app/ante/deductfee.go (1)
22-24: Good: compile‑time interface assertion presentThis guarantees DeductFeeDecorator implements sdk.AnteDecorator at build time.
| import ( | ||
| "fmt" | ||
| "math" | ||
|
|
||
| sdkioerrors "cosmossdk.io/errors" | ||
| sdkmath "cosmossdk.io/math" | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | ||
| authante "github.com/cosmos/cosmos-sdk/x/auth/ante" | ||
| authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" | ||
| "github.com/cosmos/cosmos-sdk/x/auth/types" | ||
| gethcommon "github.com/ethereum/go-ethereum/common" | ||
|
|
||
| "github.com/NibiruChain/nibiru/v2/app/appconst" | ||
| "github.com/NibiruChain/nibiru/v2/eth" | ||
| "github.com/NibiruChain/nibiru/v2/x/evm" | ||
| evmkeeper "github.com/NibiruChain/nibiru/v2/x/evm/keeper" | ||
| ) |
There was a problem hiding this comment.
Fix imports: use authtypes and banktypes (current types.BankKeeper won’t compile)
You're importing x/auth/types as "types" and later using types.BankKeeper, which doesn't exist in that package. Use explicit aliases and import bank types.
Apply:
@@
-import (
+import (
"fmt"
"math"
sdkioerrors "cosmossdk.io/errors"
sdkmath "cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
authante "github.com/cosmos/cosmos-sdk/x/auth/ante"
authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
- "github.com/cosmos/cosmos-sdk/x/auth/types"
+ authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+ banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
gethcommon "github.com/ethereum/go-ethereum/common"
"github.com/NibiruChain/nibiru/v2/app/appconst"
"github.com/NibiruChain/nibiru/v2/eth"
"github.com/NibiruChain/nibiru/v2/x/evm"
evmkeeper "github.com/NibiruChain/nibiru/v2/x/evm/keeper"
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import ( | |
| "fmt" | |
| "math" | |
| sdkioerrors "cosmossdk.io/errors" | |
| sdkmath "cosmossdk.io/math" | |
| sdk "github.com/cosmos/cosmos-sdk/types" | |
| sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | |
| authante "github.com/cosmos/cosmos-sdk/x/auth/ante" | |
| authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" | |
| "github.com/cosmos/cosmos-sdk/x/auth/types" | |
| gethcommon "github.com/ethereum/go-ethereum/common" | |
| "github.com/NibiruChain/nibiru/v2/app/appconst" | |
| "github.com/NibiruChain/nibiru/v2/eth" | |
| "github.com/NibiruChain/nibiru/v2/x/evm" | |
| evmkeeper "github.com/NibiruChain/nibiru/v2/x/evm/keeper" | |
| ) | |
| import ( | |
| "fmt" | |
| "math" | |
| sdkioerrors "cosmossdk.io/errors" | |
| sdkmath "cosmossdk.io/math" | |
| sdk "github.com/cosmos/cosmos-sdk/types" | |
| sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | |
| authante "github.com/cosmos/cosmos-sdk/x/auth/ante" | |
| authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" | |
| authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" | |
| banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" | |
| gethcommon "github.com/ethereum/go-ethereum/common" | |
| "github.com/NibiruChain/nibiru/v2/app/appconst" | |
| "github.com/NibiruChain/nibiru/v2/eth" | |
| "github.com/NibiruChain/nibiru/v2/x/evm" | |
| evmkeeper "github.com/NibiruChain/nibiru/v2/x/evm/keeper" | |
| ) |
🤖 Prompt for AI Agents
In app/ante/deductfee.go around lines 3 to 20, the import alias "types" for
x/auth is incorrect because types.BankKeeper does not exist there; replace the
imports to explicitly alias x/auth/types as authtypes and import x/bank/types as
banktypes (or similar names used elsewhere), then update any usages from
types.BankKeeper to banktypes.BankKeeper and any auth-specific types to
authtypes to match the correct packages so the file compiles.
| accountKeeper authkeeper.AccountKeeper | ||
| evmkeeper *evmkeeper.Keeper | ||
| bankKeeper types.BankKeeper | ||
| feegrantKeeper FeegrantKeeper | ||
| txFeeChecker authante.TxFeeChecker | ||
| } |
There was a problem hiding this comment.
Use the correct BankKeeper interface type
Update the field to the bank module’s interface; current type reference is invalid.
type DeductFeeDecorator struct {
accountKeeper authkeeper.AccountKeeper
evmkeeper *evmkeeper.Keeper
- bankKeeper types.BankKeeper
+ bankKeeper banktypes.BankKeeper
feegrantKeeper FeegrantKeeper
txFeeChecker authante.TxFeeChecker
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| accountKeeper authkeeper.AccountKeeper | |
| evmkeeper *evmkeeper.Keeper | |
| bankKeeper types.BankKeeper | |
| feegrantKeeper FeegrantKeeper | |
| txFeeChecker authante.TxFeeChecker | |
| } | |
| type DeductFeeDecorator struct { | |
| accountKeeper authkeeper.AccountKeeper | |
| evmkeeper *evmkeeper.Keeper | |
| bankKeeper banktypes.BankKeeper | |
| feegrantKeeper FeegrantKeeper | |
| txFeeChecker authante.TxFeeChecker | |
| } |
🤖 Prompt for AI Agents
In app/ante/deductfee.go around lines 31 to 36, the bankKeeper field is
currently typed as types.BankKeeper which is not the bank module's keeper
interface; change the field type to the bank module keeper (e.g.
bankkeeper.Keeper or the bank keeper interface exported by x/bank) and update
imports accordingly so the field uses the correct bank keeper interface from the
bank module package.
| if addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { | ||
| return fmt.Errorf("fee collector module account (%s) has not been set", types.FeeCollectorName) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use authtypes.FeeCollectorName after alias change
Replace references to types.FeeCollectorName accordingly.
- if addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil {
+ if addr := dfd.accountKeeper.GetModuleAddress(authtypes.FeeCollectorName); addr == nil {
return fmt.Errorf("fee collector module account (%s) has not been set", types.FeeCollectorName)
}
@@
- err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees)
+ err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), authtypes.FeeCollectorName, fees)
@@
- feeCollector := eth.NibiruAddrToEthAddr(accountKeeper.GetModuleAddress(types.FeeCollectorName))
+ feeCollector := eth.NibiruAddrToEthAddr(accountKeeper.GetModuleAddress(authtypes.FeeCollectorName))
@@
- ctx, stateDB, wnibi, sender, accountKeeper.GetModuleAddress(types.FeeCollectorName),
+ ctx, stateDB, wnibi, sender, accountKeeper.GetModuleAddress(authtypes.FeeCollectorName),Also applies to: 158-160, 203-205, 213-216
🤖 Prompt for AI Agents
In app/ante/deductfee.go around lines 93-95 (and similarly at 158-160, 203-205,
213-216), replace usages of types.FeeCollectorName with
authtypes.FeeCollectorName and update imports to include the auth types package
aliased as authtypes (e.g. import authtypes
"github.com/cosmos/cosmos-sdk/x/auth/types"); ensure any existing types import
isn’t clashing and adjust references accordingly so all FeeCollectorName usages
point to authtypes.FeeCollectorName.
| // DeductFees deducts fees from the given account. | ||
| func DeductFees(accountKeeper authante.AccountKeeper, evmKeeper *evmkeeper.Keeper, bankKeeper types.BankKeeper, ctx sdk.Context, acc types.AccountI, fees sdk.Coins) error { | ||
| if !fees.IsValid() { | ||
| return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) | ||
| } | ||
|
|
There was a problem hiding this comment.
Correct signatures and validate single‑denom fees
Make types explicit (banktypes.BankKeeper, authtypes.AccountI) and enforce exactly one fee denom (BondDenom) to avoid indexing fees[0] on multi‑denom inputs.
-// DeductFees deducts fees from the given account.
-func DeductFees(accountKeeper authante.AccountKeeper, evmKeeper *evmkeeper.Keeper, bankKeeper types.BankKeeper, ctx sdk.Context, acc types.AccountI, fees sdk.Coins) error {
+// DeductFees deducts fees from the given account.
+func DeductFees(accountKeeper authante.AccountKeeper, evmKeeper *evmkeeper.Keeper, bankKeeper banktypes.BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error {
if !fees.IsValid() {
return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees)
}
+ if fees.Len() != 1 {
+ return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "exactly 1 fee denom (%s) supported; got %s", appconst.BondDenom, fees.String())
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // DeductFees deducts fees from the given account. | |
| func DeductFees(accountKeeper authante.AccountKeeper, evmKeeper *evmkeeper.Keeper, bankKeeper types.BankKeeper, ctx sdk.Context, acc types.AccountI, fees sdk.Coins) error { | |
| if !fees.IsValid() { | |
| return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) | |
| } | |
| // DeductFees deducts fees from the given account. | |
| func DeductFees( | |
| accountKeeper authante.AccountKeeper, | |
| evmKeeper *evmkeeper.Keeper, | |
| bankKeeper banktypes.BankKeeper, | |
| ctx sdk.Context, | |
| acc authtypes.AccountI, | |
| fees sdk.Coins, | |
| ) error { | |
| if !fees.IsValid() { | |
| return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) | |
| } | |
| if fees.Len() != 1 { | |
| return sdkioerrors.Wrapf( | |
| sdkerrors.ErrInsufficientFee, | |
| "exactly 1 fee denom (%s) supported; got %s", | |
| appconst.BondDenom, | |
| fees.String(), | |
| ) | |
| } | |
| // ... rest of existing logic ... | |
| } |
🤖 Prompt for AI Agents
In app/ante/deductfee.go around lines 141-146, update the DeductFees signature
to use concrete types banktypes.BankKeeper and authtypes.AccountI (replace
types.BankKeeper and types.AccountI), and add validation to ensure fees contains
exactly one coin and that coin.Denom equals the chain BondDenom before indexing
fees[0]; if not, return an appropriate wrapped sdkerror (e.g.
ErrInvalidCoins/ErrInsufficientFee). After validating single-denom and denom
equality, proceed to extract the coin and continue existing fee deduction logic.
Ensure necessary imports (banktypes, authtypes, and the BondDenom constant
source) are added.
| gasMeterBefore := ctx.GasMeter() | ||
| gasConsumedBefore := gasMeterBefore.GasConsumed() | ||
| baseOpGasConsumed := uint64(0) | ||
|
|
||
| defer func() { | ||
| // NOTE: we have to refund the entire gasMeterBefore because it's modified by AfterOp | ||
| // stateDB.getStateObject() reads from state using the local root ctx which affects the gas meter | ||
| gasMeterBefore.RefundGas(gasMeterBefore.GasConsumed(), "") | ||
| gasMeterBefore.ConsumeGas(gasConsumedBefore+baseOpGasConsumed, "DeductFeeDecorator invariant") | ||
| }() |
There was a problem hiding this comment.
Gas refund math double‑counts; overcharges gas
You snapshot gas twice (gasConsumedBefore + baseOpGasConsumed where baseOp already includes the former). Preserve the pre‑fallback consumption only and ignore EVM fallback gas as intended.
- gasMeterBefore := ctx.GasMeter()
- gasConsumedBefore := gasMeterBefore.GasConsumed()
- baseOpGasConsumed := uint64(0)
+ gasMeterBefore := ctx.GasMeter()
+ preConsumed := gasMeterBefore.GasConsumed()
defer func() {
- // NOTE: we have to refund the entire gasMeterBefore because it's modified by AfterOp
- // stateDB.getStateObject() reads from state using the local root ctx which affects the gas meter
- gasMeterBefore.RefundGas(gasMeterBefore.GasConsumed(), "")
- gasMeterBefore.ConsumeGas(gasConsumedBefore+baseOpGasConsumed, "DeductFeeDecorator invariant")
+ // Reset to the pre‑fallback consumption; EVM reads should not affect gas here.
+ gasMeterBefore.RefundGas(gasMeterBefore.GasConsumed(), "")
+ gasMeterBefore.ConsumeGas(preConsumed, "DeductFeeDecorator invariant")
}()
@@
- baseOpGasConsumed = ctx.GasMeter().GasConsumed()
- ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
+ ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())Also applies to: 164-166
🤖 Prompt for AI Agents
In app/ante/deductfee.go around lines 147-156, the defer currently refunds then
reconsumes gas using gasConsumedBefore+baseOpGasConsumed which double-counts gas
(baseOpGasConsumed already includes the pre-fallback consumption); change the
logic to refund the entire current meter and then only consume the original
pre-fallback consumption (gasConsumedBefore) so EVM fallback gas is ignored as
intended; make the same correction at the other occurrence around lines 164-166.
| // DeductFeesWithWNIBI tries to deduct fees from WNIBI balance if native deduction fails. | ||
| func DeductFeesWithWNIBI( | ||
| ctx sdk.Context, | ||
| accountKeeper authante.AccountKeeper, | ||
| evmKeeper *evmkeeper.Keeper, | ||
| acc types.AccountI, | ||
| fees sdk.Coins, | ||
| ) error { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Correct DeductFeesWithWNIBI signature (AccountI type)
Use authtypes.AccountI to match the auth module interface after alias changes.
-func DeductFeesWithWNIBI(
+func DeductFeesWithWNIBI(
ctx sdk.Context,
- accountKeeper authante.AccountKeeper,
- evmKeeper *evmkeeper.Keeper,
- acc types.AccountI,
+ accountKeeper authante.AccountKeeper,
+ evmKeeper *evmkeeper.Keeper,
+ acc authtypes.AccountI,
fees sdk.Coins,
) error {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // DeductFeesWithWNIBI tries to deduct fees from WNIBI balance if native deduction fails. | |
| func DeductFeesWithWNIBI( | |
| ctx sdk.Context, | |
| accountKeeper authante.AccountKeeper, | |
| evmKeeper *evmkeeper.Keeper, | |
| acc types.AccountI, | |
| fees sdk.Coins, | |
| ) error { | |
| // DeductFeesWithWNIBI tries to deduct fees from WNIBI balance if native deduction fails. | |
| func DeductFeesWithWNIBI( | |
| ctx sdk.Context, | |
| accountKeeper authante.AccountKeeper, | |
| evmKeeper *evmkeeper.Keeper, | |
| acc authtypes.AccountI, | |
| fees sdk.Coins, | |
| ) error { | |
| // ...existing implementation... | |
| } |
🤖 Prompt for AI Agents
In app/ante/deductfee.go around lines 179 to 186, the DeductFeesWithWNIBI
function parameter uses types.AccountI but the auth module now exposes
authtypes.AccountI; update the function signature to accept authtypes.AccountI,
change any local references accordingly, and add/adjust the import for authtypes
(or update alias) so the file compiles with the new auth alias. Ensure any
callers are updated if necessary to pass authtypes.AccountI (or keep existing
variables but cast to authtypes.AccountI where appropriate).
| if wnibiBal.Cmp(evm.NativeToWei(feesAmount.BigInt())) >= 0 { | ||
| nonce := evmKeeper.GetAccNonce(ctx, sender.Eth) | ||
| _, err = evmKeeper.ConvertEvmToCoinForWNIBI( | ||
| ctx, stateDB, wnibi, sender, accountKeeper.GetModuleAddress(types.FeeCollectorName), | ||
| sdkmath.NewIntFromBigInt(evm.NativeToWei(feesAmount.BigInt())), | ||
| nil, | ||
| ) | ||
| if err != nil { | ||
| return sdkioerrors.Wrapf(err, "failed to transfer WNIBI from %s to %s", sender.Eth.Hex(), feeCollector.Hex()) | ||
| } | ||
| if err := acc.SetSequence(nonce); err != nil { | ||
| return sdkioerrors.Wrapf(err, "failed to set sequence to %d", nonce) | ||
| } | ||
| accountKeeper.SetAccount(ctx, acc) | ||
| return nil |
There was a problem hiding this comment.
Do not set Cosmos account sequence from EVM nonce
Setting acc.SetSequence(nonce) will corrupt the Cosmos sequence for the fee payer (affects signature verification and replay protection). Remove the nonce fetch and sequence mutation.
- if wnibiBal.Cmp(evm.NativeToWei(feesAmount.BigInt())) >= 0 {
- nonce := evmKeeper.GetAccNonce(ctx, sender.Eth)
+ if wnibiBal.Cmp(evm.NativeToWei(feesAmount.BigInt())) >= 0 {
_, err = evmKeeper.ConvertEvmToCoinForWNIBI(
ctx, stateDB, wnibi, sender, accountKeeper.GetModuleAddress(types.FeeCollectorName),
sdkmath.NewIntFromBigInt(evm.NativeToWei(feesAmount.BigInt())),
nil,
)
if err != nil {
return sdkioerrors.Wrapf(err, "failed to transfer WNIBI from %s to %s", sender.Eth.Hex(), feeCollector.Hex())
}
- if err := acc.SetSequence(nonce); err != nil {
- return sdkioerrors.Wrapf(err, "failed to set sequence to %d", nonce)
- }
- accountKeeper.SetAccount(ctx, acc)
return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if wnibiBal.Cmp(evm.NativeToWei(feesAmount.BigInt())) >= 0 { | |
| nonce := evmKeeper.GetAccNonce(ctx, sender.Eth) | |
| _, err = evmKeeper.ConvertEvmToCoinForWNIBI( | |
| ctx, stateDB, wnibi, sender, accountKeeper.GetModuleAddress(types.FeeCollectorName), | |
| sdkmath.NewIntFromBigInt(evm.NativeToWei(feesAmount.BigInt())), | |
| nil, | |
| ) | |
| if err != nil { | |
| return sdkioerrors.Wrapf(err, "failed to transfer WNIBI from %s to %s", sender.Eth.Hex(), feeCollector.Hex()) | |
| } | |
| if err := acc.SetSequence(nonce); err != nil { | |
| return sdkioerrors.Wrapf(err, "failed to set sequence to %d", nonce) | |
| } | |
| accountKeeper.SetAccount(ctx, acc) | |
| return nil | |
| if wnibiBal.Cmp(evm.NativeToWei(feesAmount.BigInt())) >= 0 { | |
| _, err = evmKeeper.ConvertEvmToCoinForWNIBI( | |
| ctx, stateDB, wnibi, sender, accountKeeper.GetModuleAddress(types.FeeCollectorName), | |
| sdkmath.NewIntFromBigInt(evm.NativeToWei(feesAmount.BigInt())), | |
| nil, | |
| ) | |
| if err != nil { | |
| return sdkioerrors.Wrapf(err, "failed to transfer WNIBI from %s to %s", sender.Eth.Hex(), feeCollector.Hex()) | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In app/ante/deductfee.go around lines 210 to 224, remove the logic that fetches
the EVM nonce and mutates the Cosmos account sequence: delete the nonce :=
evmKeeper.GetAccNonce(...) line and the block that calls acc.SetSequence(nonce)
and its error handling. Do not modify the Cosmos account's sequence from EVM
data; just perform the WNIBI transfer, keep accountKeeper.SetAccount(ctx, acc)
if needed, and return nil on success.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
app/evmante/evmante_verify_eth_acc.go (2)
76-80: Only fallback when the native balance check truly lacks fundsRight now every error from
CheckSenderBalancetriggers the WNIBI fallback, masking real failures. Bubble up non-ErrInsufficientFundserrors so we only fallback on genuine insufficiency.- if err := keeper.CheckSenderBalance( - evm.NativeToWei(acct.BalanceNative.ToBig()), txData, - ); err == nil { - continue - } + err := keeper.CheckSenderBalance( + evm.NativeToWei(acct.BalanceNative.ToBig()), txData, + ) + if err == nil { + continue + } + if !sdkioerrors.IsOf(err, sdkerrors.ErrInsufficientFunds) { + return ctx, err + }
84-90: Restore the original Bank.StateDB instead of always nil-ing itThe deferred cleanup currently sets
Bank.StateDBtonileven when a caller already provided a populated instance, breaking subsequent calls. Capture the original pointer and restore it in the defer.- stateDB := anteDec.evmKeeper.Bank.StateDB - if stateDB == nil { - stateDB = anteDec.evmKeeper.NewStateDB(ctx, anteDec.evmKeeper.TxConfig(ctx, gethcommon.Hash{})) - } - defer func() { - anteDec.evmKeeper.Bank.StateDB = nil - }() + origStateDB := anteDec.evmKeeper.Bank.StateDB + stateDB := origStateDB + if stateDB == nil { + stateDB = anteDec.evmKeeper.NewStateDB(ctx, anteDec.evmKeeper.TxConfig(ctx, gethcommon.Hash{})) + } + defer func() { + anteDec.evmKeeper.Bank.StateDB = origStateDB + }()app/ante/deductfee.go (3)
153-172: Stop double-counting gas during fallback
gasConsumedBefore + baseOpGasConsumedreconsumes the same amount twice, overcharging users. Preserve only the pre-fallback consumption and drop the redundant bookkeeping.- gasMeterBefore := ctx.GasMeter() - gasConsumedBefore := gasMeterBefore.GasConsumed() - baseOpGasConsumed := uint64(0) + gasMeterBefore := ctx.GasMeter() + preConsumed := gasMeterBefore.GasConsumed() @@ - gasMeterBefore.RefundGas(gasMeterBefore.GasConsumed(), "") - gasMeterBefore.ConsumeGas(gasConsumedBefore+baseOpGasConsumed, "DeductFeeDecorator invariant") + gasMeterBefore.RefundGas(gasMeterBefore.GasConsumed(), "") + gasMeterBefore.ConsumeGas(preConsumed, "DeductFeeDecorator invariant") @@ - baseOpGasConsumed = ctx.GasMeter().GasConsumed() - ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) + ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
217-229: Do not overwrite the Cosmos sequence with the EVM nonceSetting
acc.SetSequence(nonce)ties the Cosmos seq to the EVM nonce, breaking signature verification and replay protection for future Cosmos msgs. Just perform the WNIBI transfer; leave the sequence alone.- if err := acc.SetSequence(nonce); err != nil { - return sdkioerrors.Wrapf(err, "failed to set sequence to %d", nonce) - } - accountKeeper.SetAccount(ctx, acc) return nil(Remove the
nonce := …line above as well.)
13-36: Fix keeper imports and type usage so the file compiles
types.BankKeeperdoesn’t exist in the auth package, so this file won’t build. Alias auth types, import the bank keeper, and update every usage accordingly (including FeeCollectorName and account interfaces).-import ( +import ( "fmt" "math" sdkioerrors "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" authante "github.com/cosmos/cosmos-sdk/x/auth/ante" authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" - "github.com/cosmos/cosmos-sdk/x/auth/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper" gethcommon "github.com/ethereum/go-ethereum/common" @@ type DeductFeeDecorator struct { accountKeeper authkeeper.AccountKeeper evmkeeper *evmkeeper.Keeper - bankKeeper types.BankKeeper + bankKeeper *bankkeeper.Keeper feegrantKeeper FeegrantKeeper txFeeChecker authante.TxFeeChecker } @@ -func NewDeductFeeDecorator(ak authkeeper.AccountKeeper, ek *evmkeeper.Keeper, bk types.BankKeeper, fk FeegrantKeeper, tfc authante.TxFeeChecker) DeductFeeDecorator { +func NewDeductFeeDecorator(ak authkeeper.AccountKeeper, ek *evmkeeper.Keeper, bk *bankkeeper.Keeper, fk FeegrantKeeper, tfc authante.TxFeeChecker) DeductFeeDecorator { @@ - if addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { - return fmt.Errorf("fee collector module account (%s) has not been set", types.FeeCollectorName) + if addr := dfd.accountKeeper.GetModuleAddress(authtypes.FeeCollectorName); addr == nil { + return fmt.Errorf("fee collector module account (%s) has not been set", authtypes.FeeCollectorName) @@ -func DeductFees(accountKeeper authante.AccountKeeper, evmKeeper *evmkeeper.Keeper, bankKeeper types.BankKeeper, ctx sdk.Context, acc types.AccountI, fees sdk.Coins) error { +func DeductFees(accountKeeper authante.AccountKeeper, evmKeeper *evmkeeper.Keeper, bankKeeper *bankkeeper.Keeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error {(Apply the same
authtypes.FeeCollectorNameandauthtypes.AccountIreplacement in the rest of the file.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/ante/deductfee.go(1 hunks)app/evmante/evmante_verify_eth_acc.go(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-31T18:28:26.466Z
Learnt from: Unique-Divine
PR: NibiruChain/nibiru#2095
File: x/evm/keeper/bank_extension.go:0-0
Timestamp: 2024-10-31T18:28:26.466Z
Learning: When implementing the `bankkeeper.Keeper` interface in `x/evm/keeper/bank_extension.go`, methods must use non-pointer receivers to satisfy the interface requirements.
Applied to files:
app/ante/deductfee.go
🧬 Code graph analysis (2)
app/evmante/evmante_verify_eth_acc.go (3)
x/evm/statedb/statedb.go (1)
StateDB(45-85)x/evm/statedb/config.go (1)
TxConfig(15-20)x/evm/mock_msg.go (1)
MOCK_GETH_MESSAGE(8-21)
app/ante/deductfee.go (7)
x/evm/keeper/keeper.go (1)
Keeper(27-54)app/ante/expected_keeper.go (1)
FeegrantKeeper(8-10)app/appconst/appconst.go (1)
BondDenom(22-22)x/evm/statedb/statedb.go (1)
StateDB(45-85)x/evm/mock_msg.go (1)
MOCK_GETH_MESSAGE(8-21)eth/eth_account.go (1)
NibiruAddrToEthAddr(19-21)x/evm/const.go (1)
NativeToWei(142-145)
⏰ 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). (6)
- GitHub Check: unit-tests
- GitHub Check: build
- GitHub Check: proto-gen-rs
- GitHub Check: e2e-evm
- GitHub Check: integration-tests
- GitHub Check: lint
| // DeductFees deducts fees from the given account. | ||
| func DeductFees(accountKeeper authante.AccountKeeper, evmKeeper *evmkeeper.Keeper, bankKeeper types.BankKeeper, ctx sdk.Context, acc types.AccountI, fees sdk.Coins) error { | ||
| if !fees.IsValid() { | ||
| return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) | ||
| } | ||
|
|
||
| gasMeterBefore := ctx.GasMeter() | ||
| gasConsumedBefore := gasMeterBefore.GasConsumed() | ||
| baseOpGasConsumed := uint64(0) | ||
|
|
||
| defer func() { | ||
| // NOTE: we have to refund the entire gasMeterBefore because it's modified by AfterOp | ||
| // stateDB.getStateObject() reads from state using the local root ctx which affects the gas meter | ||
| gasMeterBefore.RefundGas(gasMeterBefore.GasConsumed(), "") | ||
| gasMeterBefore.ConsumeGas(gasConsumedBefore+baseOpGasConsumed, "DeductFeeDecorator invariant") | ||
| }() | ||
|
|
||
| if fees[0].Denom == appconst.BondDenom { | ||
| err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) | ||
| if err == nil { | ||
| return nil | ||
| } | ||
|
|
||
| baseOpGasConsumed = ctx.GasMeter().GasConsumed() | ||
| ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) | ||
|
|
||
| // fallback to WNIBI | ||
| err = DeductFeesWithWNIBI(ctx, accountKeeper, evmKeeper, acc, fees) | ||
| if err == nil { | ||
| return nil | ||
| } | ||
|
|
||
| return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "insufficient balance across supported gas tokens to cover %s", fees[0].Amount) | ||
| } else { | ||
| return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "fee denom must be %s, got %s", appconst.BondDenom, fees[0].Denom) | ||
| } | ||
| } | ||
|
|
||
| // DeductFeesWithWNIBI tries to deduct fees from WNIBI balance if native deduction fails. |
There was a problem hiding this comment.
Validate single-denom fees before indexing
Both deduction paths assume fees[0] exists, so multi-denom or empty inputs panic or misclassify valid fees. Enforce exactly one coin before touching fees[0].
if !fees.IsValid() {
return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees)
}
+ if fees.Len() != 1 {
+ return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "exactly one fee denom (%s) is supported; got %s", appconst.BondDenom, fees)
+ }
@@
func DeductFeesWithWNIBI(
ctx sdk.Context,
accountKeeper authante.AccountKeeper,
evmKeeper *evmkeeper.Keeper,
- acc types.AccountI,
+ acc authtypes.AccountI,
fees sdk.Coins,
) error {
+ if fees.Len() != 1 {
+ return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "exactly one fee denom (%s) is supported; got %s", appconst.BondDenom, fees)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // DeductFees deducts fees from the given account. | |
| func DeductFees(accountKeeper authante.AccountKeeper, evmKeeper *evmkeeper.Keeper, bankKeeper types.BankKeeper, ctx sdk.Context, acc types.AccountI, fees sdk.Coins) error { | |
| if !fees.IsValid() { | |
| return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) | |
| } | |
| gasMeterBefore := ctx.GasMeter() | |
| gasConsumedBefore := gasMeterBefore.GasConsumed() | |
| baseOpGasConsumed := uint64(0) | |
| defer func() { | |
| // NOTE: we have to refund the entire gasMeterBefore because it's modified by AfterOp | |
| // stateDB.getStateObject() reads from state using the local root ctx which affects the gas meter | |
| gasMeterBefore.RefundGas(gasMeterBefore.GasConsumed(), "") | |
| gasMeterBefore.ConsumeGas(gasConsumedBefore+baseOpGasConsumed, "DeductFeeDecorator invariant") | |
| }() | |
| if fees[0].Denom == appconst.BondDenom { | |
| err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) | |
| if err == nil { | |
| return nil | |
| } | |
| baseOpGasConsumed = ctx.GasMeter().GasConsumed() | |
| ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) | |
| // fallback to WNIBI | |
| err = DeductFeesWithWNIBI(ctx, accountKeeper, evmKeeper, acc, fees) | |
| if err == nil { | |
| return nil | |
| } | |
| return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "insufficient balance across supported gas tokens to cover %s", fees[0].Amount) | |
| } else { | |
| return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "fee denom must be %s, got %s", appconst.BondDenom, fees[0].Denom) | |
| } | |
| } | |
| // DeductFeesWithWNIBI tries to deduct fees from WNIBI balance if native deduction fails. | |
| // DeductFees deducts fees from the given account. | |
| func DeductFees( | |
| accountKeeper authante.AccountKeeper, | |
| evmKeeper *evmkeeper.Keeper, | |
| bankKeeper types.BankKeeper, | |
| ctx sdk.Context, | |
| acc types.AccountI, | |
| fees sdk.Coins, | |
| ) error { | |
| if !fees.IsValid() { | |
| return sdkioerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) | |
| } | |
| if fees.Len() != 1 { | |
| return sdkioerrors.Wrapf( | |
| sdkerrors.ErrInsufficientFee, | |
| "exactly one fee denom (%s) is supported; got %s", | |
| appconst.BondDenom, | |
| fees, | |
| ) | |
| } | |
| gasMeterBefore := ctx.GasMeter() | |
| gasConsumedBefore := gasMeterBefore.GasConsumed() | |
| baseOpGasConsumed := uint64(0) | |
| defer func() { | |
| // NOTE: we have to refund the entire gasMeterBefore because it's modified by AfterOp | |
| // stateDB.getStateObject() reads from state using the local root ctx which affects the gas meter | |
| gasMeterBefore.RefundGas(gasMeterBefore.GasConsumed(), "") | |
| gasMeterBefore.ConsumeGas(gasConsumedBefore+baseOpGasConsumed, "DeductFeeDecorator invariant") | |
| }() | |
| if fees[0].Denom == appconst.BondDenom { | |
| err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) | |
| if err == nil { | |
| return nil | |
| } | |
| baseOpGasConsumed = ctx.GasMeter().GasConsumed() | |
| ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) | |
| // fallback to WNIBI | |
| err = DeductFeesWithWNIBI(ctx, accountKeeper, evmKeeper, acc, fees) | |
| if err == nil { | |
| return nil | |
| } | |
| return sdkioerrors.Wrapf( | |
| sdkerrors.ErrInsufficientFunds, | |
| "insufficient balance across supported gas tokens to cover %s", | |
| fees[0].Amount, | |
| ) | |
| } | |
| return sdkioerrors.Wrapf( | |
| sdkerrors.ErrInsufficientFee, | |
| "fee denom must be %s, got %s", | |
| appconst.BondDenom, | |
| fees[0].Denom, | |
| ) | |
| } | |
| // DeductFeesWithWNIBI tries to deduct fees from WNIBI balance if native deduction fails. | |
| func DeductFeesWithWNIBI( | |
| ctx sdk.Context, | |
| accountKeeper authante.AccountKeeper, | |
| evmKeeper *evmkeeper.Keeper, | |
| acc authtypes.AccountI, | |
| fees sdk.Coins, | |
| ) error { | |
| if fees.Len() != 1 { | |
| return sdkioerrors.Wrapf( | |
| sdkerrors.ErrInsufficientFee, | |
| "exactly one fee denom (%s) is supported; got %s", | |
| appconst.BondDenom, | |
| fees, | |
| ) | |
| } | |
| // ... rest of implementation unchanged | |
| } |
🤖 Prompt for AI Agents
In app/ante/deductfee.go around lines 147 to 185, the code indexes fees[0]
without ensuring the coins slice has exactly one entry, which can panic or
misclassify valid fees; add a guard after validating fees to require len(fees)
== 1 (and non-empty) and return a clear sdkerrors.ErrInsufficientFee-wrapped
error if not, so both branches safely use fees[0] only when a single-denom fee
is provided. Ensure the new check runs before any access to fees[0] and keep
existing error types/messages consistent with surrounding logic.
| stateDB := evmKeeper.Bank.StateDB | ||
| if stateDB == nil { | ||
| stateDB = evmKeeper.NewStateDB(ctx, evmKeeper.TxConfig(ctx, gethcommon.Hash{})) | ||
| } | ||
| defer func() { | ||
| evmKeeper.Bank.StateDB = nil | ||
| }() |
There was a problem hiding this comment.
Don’t clobber EVM bank state after the fallback
This defer always sets Bank.StateDB to nil, wiping any existing shared state. Track the original pointer and restore it just like the verify decorator.
- stateDB := evmKeeper.Bank.StateDB
- if stateDB == nil {
- stateDB = evmKeeper.NewStateDB(ctx, evmKeeper.TxConfig(ctx, gethcommon.Hash{}))
- }
- defer func() {
- evmKeeper.Bank.StateDB = nil
- }()
+ origStateDB := evmKeeper.Bank.StateDB
+ stateDB := origStateDB
+ if stateDB == nil {
+ stateDB = evmKeeper.NewStateDB(ctx, evmKeeper.TxConfig(ctx, gethcommon.Hash{}))
+ }
+ defer func() {
+ evmKeeper.Bank.StateDB = origStateDB
+ }()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| stateDB := evmKeeper.Bank.StateDB | |
| if stateDB == nil { | |
| stateDB = evmKeeper.NewStateDB(ctx, evmKeeper.TxConfig(ctx, gethcommon.Hash{})) | |
| } | |
| defer func() { | |
| evmKeeper.Bank.StateDB = nil | |
| }() | |
| origStateDB := evmKeeper.Bank.StateDB | |
| stateDB := origStateDB | |
| if stateDB == nil { | |
| stateDB = evmKeeper.NewStateDB(ctx, evmKeeper.TxConfig(ctx, gethcommon.Hash{})) | |
| } | |
| defer func() { | |
| evmKeeper.Bank.StateDB = origStateDB | |
| }() |
🤖 Prompt for AI Agents
In app/ante/deductfee.go around lines 195 to 201, the defer unconditionally sets
evmKeeper.Bank.StateDB = nil which clobbers any preexisting shared StateDB;
instead capture the original StateDB pointer before the fallback (origStateDB :=
evmKeeper.Bank.StateDB), only set the fallback when nil, and in the defer
restore the original pointer (evmKeeper.Bank.StateDB = origStateDB) so you don't
wipe an existing shared state — follow the same save/restore pattern used in the
verify decorator.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(1 hunks)
⏰ 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). (6)
- GitHub Check: lint
- GitHub Check: proto-gen-rs
- GitHub Check: build
- GitHub Check: unit-tests
- GitHub Check: integration-tests
- GitHub Check: e2e-evm
Purpose / Abstract
This PR enable the usage of WNIBI for gas. Modifying the current auth
DeductFeeDecoratorto fallback toWNIBIwhen users doesn't have enoughunibiin their account.Related issues:
Key changes