feat: paying fee in wnibi

[onikonychev]

• related to feat: native support for WNIBI gas consumption #2321

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (9)

contrib/scripts/localnet-uniswap.sh (6)

110-113: Use SEDOPTION with BSD/macOS sed; current SEDOPTION is unused.

-SEDOPTION=""
+SEDOPTION=""
 if [[ "$OSTYPE" == "darwin"* ]]; then
-  SEDOPTION="''"
+  SEDOPTION="''" # BSD sed requires a backup extension
 fi
@@
-sed -i "s/\"chain_id\": \".*\"/\"chain_id\": \"$CHAIN_ID\"/" "$CHAIN_DIR/config/genesis.json"
+sed -i${SEDOPTION} "s/\"chain_id\": \".*\"/\"chain_id\": \"$CHAIN_ID\"/" "$CHAIN_DIR/config/genesis.json"

Also applies to: 154-154

---

22-35: Fix color init: broken conditional, wrong package, and no graceful fallback.

The [ console_log_text_color ] test is always true; colors never initialize. Also avoid apt-get in scripts and handle missing tput gracefully.

-# Console log text colour
-console_log_text_color() {
-  red=$(tput setaf 9)
-  green=$(tput setaf 10)
-  blue=$(tput setaf 12)
-  reset=$(tput sgr0)
-}
-
-if [ console_log_text_color ]; then
-  echo "succesfully toggled console coloring"
-else
-  # For Ubuntu and Debian. MacOS has tput by default.
-  apt-get install libncurses5-dbg -y
-fi
+# Console log text colour
+console_log_text_color() {
+  if command -v tput >/dev/null 2>&1 && [ "$(tput colors 2>/dev/null)" -ge 8 ]; then
+    red="$(tput setaf 1)"
+    green="$(tput setaf 2)"
+    blue="$(tput setaf 4)"
+    reset="$(tput sgr0)"
+    return 0
+  else
+    red=""; green=""; blue=""; reset=""
+    return 1
+  fi
+}
+
+if console_log_text_color; then
+  echo "successfully enabled console coloring"
+else
+  echo "tput not available or terminal lacks color support; continuing without colors"
+fi

---

126-130: Scope reset to the configured home.

-$BINARY tendermint unsafe-reset-all
+$BINARY tendermint unsafe-reset-all --home "$CHAIN_DIR"

---

156-158: Use $BINARY consistently; avoid hardcoding nibid.

-nibid tendermint unsafe-reset-all --home "$CHAIN_DIR"
+"$BINARY" tendermint unsafe-reset-all --home "$CHAIN_DIR"

---

168-169: Use $BINARY consistently when starting the node.

-echo "=== Starting chain $CHAIN_ID ==="
-nibid start --home "$CHAIN_DIR"
+echo "=== Starting chain $CHAIN_ID ==="
+"$BINARY" start --home "$CHAIN_DIR"

---

4-10: Undefined $STAKE causes broken gentx/genesis account. Define it and make key ops non-interactive.

 GENESIS_EXPORT="./contrib/genesis.json"
 
+STAKE="100000000unibi"
@@
-if ! "$BINARY" keys show validator --home "$CHAIN_DIR" >/dev/null 2>&1; then
-    "$BINARY" keys add validator --home "$CHAIN_DIR"
+if ! "$BINARY" keys show validator --home "$CHAIN_DIR" >/dev/null 2>&1; then
+    "$BINARY" keys add validator --home "$CHAIN_DIR" --keyring-backend test
     "$BINARY" add-genesis-account validator "$STAKE" --home "$CHAIN_DIR"
     "$BINARY" gentx validator "$STAKE" --chain-id "$CHAIN_ID" --home "$CHAIN_DIR"
     "$BINARY" collect-gentxs --home "$CHAIN_DIR"
 fi

Also applies to: 160-165

x/gastoken/ante/decorator.go (3)

37-37: Wrong BankKeeper type — use x/gastoken/types.BankKeeper (compilation blocker).

authtypes.BankKeeper does not exist; the expected interface sits in x/gastoken/types.

 type DeductFeeDecorator struct {
 	ak             types.AccountKeeper
 	evmkeeper      *evmkeeper.Keeper
-	bankKeeper     authtypes.BankKeeper
+	bankKeeper     types.BankKeeper
 	feegrantKeeper types.FeegrantKeeper
 	gasTokenKeeper *keeper.Keeper

-func NewDeductFeeDecorator(gtk *keeper.Keeper, ek *evmkeeper.Keeper, ak types.AccountKeeper, bk authtypes.BankKeeper, fk types.FeegrantKeeper, tfc authante.TxFeeChecker) DeductFeeDecorator {
+func NewDeductFeeDecorator(gtk *keeper.Keeper, ek *evmkeeper.Keeper, ak types.AccountKeeper, bk types.BankKeeper, fk types.FeegrantKeeper, tfc authante.TxFeeChecker) DeductFeeDecorator {

-func DeductFees(accountkeeper types.AccountKeeper, ek *evmkeeper.Keeper, gtk *keeper.Keeper, bankKeeper authtypes.BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error {
+func DeductFees(accountkeeper types.AccountKeeper, ek *evmkeeper.Keeper, gtk *keeper.Keeper, bankKeeper types.BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error {

Also applies to: 44-44, 138-138

---

176-176: Route ERC20/wnibi fees to FeeCollector module, not gastoken module.

Unify fee flows and align with how WNIBI withdraw credits native funds (no module escrow). Use FeeCollectorName (or forward after withdraw).

-	feeCollector := eth.NibiruAddrToEthAddr(accountkeeper.GetModuleAddress(types.ModuleName))
+	feeCollector := eth.NibiruAddrToEthAddr(accountkeeper.GetModuleAddress(authtypes.FeeCollectorName))

And set sequence (if kept) on the same module account:

-	acc := ak.GetModuleAccount(ctx, types.ModuleName)
+	acc := ak.GetModuleAccount(ctx, authtypes.FeeCollectorName)

Context: WNIBI uses direct contract interactions; native coins move via account balances, not module escrow. See team learnings.

Also applies to: 293-297

---

225-231: Avoid manual Cosmos sequence sync with EVM nonce here (risk of off-by-one and replay issues).

Let existing EVM ante handlers manage sequence sync, or ensure you set nonce+1 post-call and target the correct account. Safer: remove these SetSequence calls.

-				if err := acc.SetSequence(nonce); err != nil {
-					return sdkioerrors.Wrapf(err, "failed to set sequence to %d", nonce)
-				}
-
-				accountkeeper.SetAccount(ctx, acc)
+				// Sequence synchronization is handled by EVM ante; avoid manual SetSequence here.

-	acc := ak.GetModuleAccount(ctx, authtypes.FeeCollectorName)
-	if err := acc.SetSequence(nonce); err != nil {
-		return sdkioerrors.Wrapf(err, "failed to set sequence to %d", nonce)
-	}
+	// Do not manually sync module account sequence; EVM ante manages this.

To validate remaining manual sync sites across the repo, run:

#!/bin/bash
rg -n -C2 '\bSetSequence\('
rg -n -C2 '\bGetAccNonce\('

Also applies to: 293-297

🧹 Nitpick comments (10)

contrib/scripts/localnet-uniswap.sh (4)

37-53: Avoid extra blank lines; print colored messages on a single line.

-echo_info() {
-  echo "${blue}"
-  echo "$1"
-  echo "${reset}"
-}
+echo_info() { printf "%b\n" "${blue}$1${reset}"; }
 
-echo_error() {
-  echo "${red}"
-  echo "$1"
-  echo "${reset}"
-}
+echo_error() { printf "%b\n" "${red}$1${reset}"; }
 
-echo_success() {
-  echo "${green}"
-  echo "$1"
-  echo "${reset}"
-}
+echo_success() { printf "%b\n" "${green}$1${reset}"; }

---

2-2: Harden shell options.

-set -e
+set -Eeuo pipefail

Note: This pairs well with defining STAKE above; otherwise -u will exit on unset vars.

---

76-81: Feature flags are currently unused; either wire them or remove.

+# default feature flags
+FLAG_SPOT=false
@@
 enable_feature_flag() {
   case $1 in
   spot) FLAG_SPOT=true ;;
   *) echo_error "Unknown feature: $1" ;;
   esac
 }
@@
 echo_info "Features flags:"
 echo "FLAG_SKIP_BUILD: $FLAG_SKIP_BUILD"
+echo "FLAG_SPOT: $FLAG_SPOT"

If there’s intended behavior (config mutations, module params) behind FLAG_SPOT, point this script to those actions.

Also applies to: 107-109

---

146-149: Surface missing genesis via echo_error and stderr.

-if [ ! -f "$GENESIS_EXPORT" ]; then
-    echo "Error: exported genesis file $GENESIS_EXPORT not found!"
-    exit 1
-fi
+if [ ! -f "$GENESIS_EXPORT" ]; then
+  echo_error "exported genesis file not found: $GENESIS_EXPORT"
+  exit 1
+fi

x/gastoken/ante/decorator.go (6)

143-149: Remove debug print and use structured logging.

 	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")
-		fmt.Println("baseOpGasConsumed :", baseOpGasConsumed)
+		ctx.Logger().Debug("gastoken: baseOpGasConsumed", "gas", baseOpGasConsumed)
 	}()

---

209-211: Avoid magic number 3000 for the pool fee; DRY via a param or local const.

Example minimal change (local variable):

-			amountNeeded, err := GetAmountInFromUniswap(
+			poolFee := big.NewInt(3000) // TODO: source from params
+			amountNeeded, err := GetAmountInFromUniswap(
 				ctx,
 				ek,
 				gtk,
 				erc20Addr,
 				gethcommon.HexToAddress(wnibi),
-				big.NewInt(3000),
+				poolFee,
 				evm.NativeToWei(feesAmount.BigInt()),
 			)

-			Fee:               big.NewInt(3000),                                   // 0.3% pool
+			Fee:               big.NewInt(3000),                                   // TODO: source from params

Also applies to: 327-331

---

438-446: Inconsistent gas limits for quoter call; use one source of truth.

-		GasLimit:         2_500_000,
+		GasLimit:         evmkeeper.GetCallGasWithLimit(ctx, evmkeeper.Erc20GasLimitExecute),

The CallContractWithInput below already uses the same helper; this aligns both.

Also applies to: 458-458

---

69-71: Comment is misleading; fees aren’t accepted in any denom at this stage.

Clarify that the fee field is expected in base denom, with WNIBI/ERC20 fallback for payment.

-	// fees can be in any denom (checked for validity later)
+	// Fees are provided in the base denom; payment may be fulfilled via WNIBI/ERC20 fallback.

---

242-269: Drop unusedBigInt parameter from WithdrawFeeToken; initialize zeros locally.

-func WithdrawFeeToken(ctx sdk.Context, ek *evmkeeper.Keeper, ak types.AccountKeeper, contract, feeCollector gethcommon.Address, unusedBigInt *big.Int) error {
+func WithdrawFeeToken(ctx sdk.Context, ek *evmkeeper.Keeper, ak types.AccountKeeper, contract, feeCollector gethcommon.Address) error {

-		Value:            unusedBigInt, // amount
+		Value:            big.NewInt(0), // amount
-		GasPrice:         unusedBigInt,
-		GasFeeCap:        unusedBigInt,
-		GasTipCap:        unusedBigInt,
+		GasPrice:         big.NewInt(0),
+		GasFeeCap:        big.NewInt(0),
+		GasTipCap:        big.NewInt(0),

Also update call sites:

-	if err := WithdrawFeeToken(ctx, ek, accountkeeper, gethcommon.HexToAddress(wnibi), feeCollector, big.NewInt(0)); err != nil {
+	if err := WithdrawFeeToken(ctx, ek, accountkeeper, gethcommon.HexToAddress(wnibi), feeCollector); err != nil {

Also applies to: 189-191

---

156-237: Add an event for which fee path was used (native | wnibi | swap).

Emitting an explicit tx_fee_paid event improves observability and debugging. Example after each successful path:

• native: attributes path=native
• wnibi: path=wnibi
• swap: path=swap, tokenIn, amountIn

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc2f199 and 845fe98.

📒 Files selected for processing (3)

• app/evmante/evmante_verify_eth_acc_test.go (4 hunks)
• contrib/scripts/localnet-uniswap.sh (1 hunks)
• x/gastoken/ante/decorator.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• app/evmante/evmante_verify_eth_acc_test.go

🧰 Additional context used

🧠 Learnings (2)

📚 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:

• x/gastoken/ante/decorator.go

📚 Learning: 2025-09-07T16:02:13.718Z

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.718Z
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:

• x/gastoken/ante/decorator.go

🧬 Code graph analysis (1)

x/gastoken/ante/decorator.go (7)

x/gastoken/types/expected_keeper.go (3)

• AccountKeeper (11-16)
• BankKeeper (24-32)
• FeegrantKeeper (19-21)

x/gastoken/keeper/keeper.go (1)

• Keeper (23-33)

x/evm/keeper/keeper.go (1)

• Keeper (30-56)

app/appconst/appconst.go (1)

• BondDenom (20-20)

eth/eth_account.go (1)

• NibiruAddrToEthAddr (19-21)

x/evm/const.go (1)

• NativeToWei (123-126)

x/evm/embeds/embeds.go (3)

• SmartContract_WNIBI (189-192)
• SmartContract_UniswapV3SwapRouter (199-202)
• SmartContract_UniswapV3Quoter (203-206)

🪛 Shellcheck (0.10.0)

contrib/scripts/localnet-uniswap.sh

[warning] 7-7: MNEMONIC appears unused. Verify use (or export if used externally).

(SC2034)

---

[error] 30-30: This expression is constant. Did you forget a $ somewhere?

(SC2078)

---

[warning] 78-78: FLAG_SPOT appears unused. Verify use (or export if used externally).

(SC2034)

---

[warning] 112-112: SEDOPTION appears unused. Verify use (or export if used externally).

(SC2034)

🔇 Additional comments (1)

contrib/scripts/localnet-uniswap.sh (1)

14-17: Fail-fast check for binary looks good.

[coderabbitai]

Actionable comments posted: 5

♻️ Duplicate comments (1)

x/evm/keeper/keeper.go (1)

96-120: Improve error handling and use consistent error types.

The function implementation is good but has inconsistent error handling. Line 98 uses fmt.Errorf while the rest uses sdkioerrors.Wrapf.

 func (k *Keeper) GetErc20Balance(ctx sdk.Context, account, contract gethcommon.Address) (balance *big.Int, err error) {
 	if contract == (gethcommon.Address{}) {
-		return nil, fmt.Errorf("contract address is empty")
+		return nil, sdkioerrors.Wrap(sdkerrors.ErrInvalidAddress, "contract address cannot be zero")
 	}

This provides consistency with the error handling pattern used throughout the function and the broader codebase.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 845fe98 and f51d983.

📒 Files selected for processing (4)

• x/evm/embeds/embeds.go (3 hunks)
• x/evm/evmtest/test_deps.go (2 hunks)
• x/evm/keeper/erc20.go (3 hunks)
• x/evm/keeper/keeper.go (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• x/evm/evmtest/test_deps.go
• x/evm/keeper/erc20.go

🧰 Additional context used

🧠 Learnings (2)

📚 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:

• x/evm/keeper/keeper.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:

• x/evm/keeper/keeper.go

🧬 Code graph analysis (1)

x/evm/keeper/keeper.go (5)

x/evm/statedb/config.go (1)

• TxConfig (15-20)

x/evm/statedb/statedb.go (1)

• StateDB (45-85)

x/evm/embeds/embeds.go (1)

• SmartContract_ERC20MinterWithMetadataUpdates (79-82)

x/evm/keeper/erc20.go (1)

• Erc20GasLimitExecute (29-29)

x/evm/const.go (2)

• EVM_MODULE_ADDRESS (106-106)
• Big0 (217-217)

🪛 GitHub Check: unit-tests

x/evm/embeds/embeds.go

[failure] 198-198:
SmartContract_WNIBI redeclared in this block

⏰ 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). (4)

• GitHub Check: integration-tests
• GitHub Check: e2e-evm
• GitHub Check: proto-gen-rs
• GitHub Check: build

🔇 Additional comments (4)

x/evm/embeds/embeds.go (2)

203-215: New Uniswap contract embeddings look correct.

The new contract embeddings for Uniswap V3 components (Pool, SwapRouter, Quoter) follow the established pattern and use appropriate naming conventions.

---

240-242: New Uniswap contract initialization looks correct.

The MustLoad() calls for the new Uniswap contracts follow the established pattern and will properly initialize the ABI and bytecode at runtime.

x/evm/keeper/keeper.go (2)

8-11: LGTM! New imports support ERC20 functionality.

The new imports are appropriate for the added ERC20 helper functions: sdkioerrors for error wrapping, gethcommon and core for Ethereum types, gethcore for transaction types, sdkerrors for standard SDK errors, and embeds for contract ABI access.

Also applies to: 19-19, 23-23

---

285-298: LGTM! Mock message properly configured.

The MOCK_GETH_MESSAGE variable is well-structured for test scenarios and EVM operations that need a default message. Using evm.EVM_MODULE_ADDRESS as the sender and evm.Big0 for monetary fields is appropriate.

[coderabbitai]

Actionable comments posted: 6

♻️ Duplicate comments (15)

contrib/scripts/localnet-uniswap.sh (6)

11-13: Fail fast if required binary is missing.

Prevents destructive resets when nibid isn’t installed.

 echo "CHAIN_DIR: $CHAIN_DIR"
 echo "CHAIN_ID: $CHAIN_ID"
 
+if ! command -v "$BINARY" >/dev/null 2>&1; then
+  >&2 echo "Error: Binary '$BINARY' not found in PATH. Build or install it first."
+  exit 1
+fi

---

116-121: Process stop: use $BINARY consistently and tolerate absence.

Avoid hardcoded killall; don’t fail under set -e.

 if pgrep -x "$BINARY" >/dev/null; then
   echo_error "Terminating $BINARY..."
-  killall nibid
+  pkill -x "$BINARY" || true
 fi

---

123-127: Scope reset to the intended home.

Prevent nuking the wrong node home.

-$BINARY tendermint unsafe-reset-all
+$BINARY tendermint unsafe-reset-all --home "$CHAIN_DIR"
 rm -f "$CHAIN_DIR/config/genesis.json"
 rm -rf "$CHAIN_DIR/config/gentx/"

---

134-141: Init must target CHAIN_DIR and quote vars.

Ensures idempotent runs to the expected directory.

-if $BINARY init $CHAIN_ID --chain-id $CHAIN_ID --overwrite; then
+if "$BINARY" init "$CHAIN_ID" --chain-id "$CHAIN_ID" --overwrite --home "$CHAIN_DIR"; then
   echo_success "Successfully initialized $CHAIN_ID"
 else
   echo_error "Failed to initialize $CHAIN_ID"
   exit 1
 fi

---

183-189: Keys ops: quote vars and set explicit home.

Avoids dependence on global CLI state.

-if ! $BINARY keys show $val_key_name; then 
-  echo "$MNEMONIC" | $BINARY keys add $val_key_name --recover
+if ! "$BINARY" keys show "$val_key_name" --home "$CHAIN_DIR" >/dev/null 2>&1; then 
+  echo "$MNEMONIC" | "$BINARY" keys add "$val_key_name" --recover --home "$CHAIN_DIR"
   echo_success "Successfully added key: $val_key_name"
 fi
 
-val_address=$($BINARY keys show $val_key_name -a)
+val_address=$("$BINARY" keys show "$val_key_name" -a --home "$CHAIN_DIR")

---

18-31: Color init is broken and non-portable; don’t apt-get in scripts.

The test is constant-true; colors never init; avoid package installs and degrade gracefully.

-# Console log text colour
-console_log_text_color() {
-  red=$(tput setaf 9)
-  green=$(tput setaf 10)
-  blue=$(tput setaf 12)
-  reset=$(tput sgr0)
-}
-
-if [ console_log_text_color ]; then
-  echo "succesfully toggled console coloring"
-else
-  # For Ubuntu and Debian. MacOS has tput by default.
-  apt-get install libncurses5-dbg -y
-fi
+# Console log text colour
+console_log_text_color() {
+  if command -v tput >/dev/null 2>&1 && [ "$(tput colors 2>/dev/null || echo 0)" -ge 8 ]; then
+    red="$(tput setaf 1)"; green="$(tput setaf 2)"; blue="$(tput setaf 4)"; reset="$(tput sgr0)"
+    return 0
+  else
+    red=""; green=""; blue=""; reset=""
+    return 1
+  fi
+}
+
+if console_log_text_color; then
+  echo "successfully enabled console coloring"
+else
+  echo "tput not available or terminal lacks color; continuing without colors"
+fi

x/evm/keeper/keeper.go (4)

166-168: Wrap VM failure with SDK error and include VM error string.

Align with module-wide error handling and return a typed error.

-	if resp.Failed() {
-		return fmt.Errorf("ERC20 transfer failed with VM error: %s", resp.VmError)
-	}
+	if resp.Failed() {
+		return sdkioerrors.Wrapf(sdkerrors.ErrInvalidRequest, "ERC20 transfer failed with VM error: %s", resp.VmError)
+	}

---

220-222: Fix wrong operation name and wrap VM failure with SDK error.

Message says “transfer” in approve path; also prefer typed error.

-	if resp.Failed() {
-		return fmt.Errorf("ERC20 transfer failed with VM error: %s", resp.VmError)
-	}
+	if resp.Failed() {
+		return sdkioerrors.Wrapf(sdkerrors.ErrInvalidRequest, "ERC20 approve failed with VM error: %s", resp.VmError)
+	}

---

152-159: Same StateDB cleanup issue as above.

Only nil-out Bank.StateDB if you created it in this function.

-	txConfig := k.TxConfig(ctx, gethcommon.Hash{})
-	stateDB := k.Bank.StateDB
-	if stateDB == nil {
-		stateDB = k.NewStateDB(ctx, txConfig)
-	}
-	defer func() {
-		k.Bank.StateDB = nil
-	}()
+	txConfig := k.TxConfig(ctx, gethcommon.Hash{})
+	stateDB := k.Bank.StateDB
+	created := false
+	if stateDB == nil {
+		stateDB = k.NewStateDB(ctx, txConfig)
+		created = true
+	}
+	if created {
+		defer func() { k.Bank.StateDB = nil }()
+	}

---

206-213: Same StateDB cleanup guard needed here.

-	txConfig := k.TxConfig(ctx, gethcommon.Hash{})
-	stateDB := k.Bank.StateDB
-	if stateDB == nil {
-		stateDB = k.NewStateDB(ctx, txConfig)
-	}
-	defer func() {
-		k.Bank.StateDB = nil
-	}()
+	txConfig := k.TxConfig(ctx, gethcommon.Hash{})
+	stateDB := k.Bank.StateDB
+	created := false
+	if stateDB == nil {
+		stateDB = k.NewStateDB(ctx, txConfig)
+		created = true
+	}
+	if created {
+		defer func() { k.Bank.StateDB = nil }()
+	}

x/gastoken/ante/decorator.go (5)

37-38: Fix BankKeeper type: use the expected interface from gastoken/types (compilation blocker).

authtypes.BankKeeper does not exist; use types.BankKeeper consistently (struct field, constructor, and DeductFees signature).

 type DeductFeeDecorator struct {
   ak             types.AccountKeeper
   evmkeeper      *evmkeeper.Keeper
-  bankKeeper     authtypes.BankKeeper
+  bankKeeper     types.BankKeeper
   feegrantKeeper types.FeegrantKeeper
   gasTokenKeeper *keeper.Keeper

   txFeeChecker authante.TxFeeChecker
 }

-func NewDeductFeeDecorator(gtk *keeper.Keeper, ek *evmkeeper.Keeper, ak types.AccountKeeper, bk authtypes.BankKeeper, fk types.FeegrantKeeper, tfc authante.TxFeeChecker) DeductFeeDecorator {
+func NewDeductFeeDecorator(gtk *keeper.Keeper, ek *evmkeeper.Keeper, ak types.AccountKeeper, bk types.BankKeeper, fk types.FeegrantKeeper, tfc authante.TxFeeChecker) DeductFeeDecorator {

 func DeductFees(accountkeeper types.AccountKeeper, ek *evmkeeper.Keeper, gtk *keeper.Keeper, bankKeeper
-  authtypes.BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error {
+  types.BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error {

Also applies to: 44-56, 138-139

---

176-179: Send ERC20 fees to FeeCollector module account, not the gastoken module.

Unify fee flows; credit authtypes.FeeCollectorName directly.

- feeCollector := eth.NibiruAddrToEthAddr(accountkeeper.GetModuleAddress(types.ModuleName))
+ feeCollector := eth.NibiruAddrToEthAddr(accountkeeper.GetModuleAddress(authtypes.FeeCollectorName))

---

293-297: Set sequence on the correct module account (FeeCollector), not gastoken module.

Withdraw is initiated from FeeCollector; update that account’s sequence.

- acc := ak.GetModuleAccount(ctx, types.ModuleName)
+ acc := ak.GetModuleAccount(ctx, authtypes.FeeCollectorName)
 if err := acc.SetSequence(nonce); err != nil {
   return sdkioerrors.Wrapf(err, "failed to set sequence to %d", nonce)
 }

---

121-127: Do not deduct fees in CheckTx (mempool).

Guard DeductFees so it only runs in DeliverTx; otherwise funds are charged during admission.

- // deducts the fees and transfer them to the module account
- if !fees.IsZero() && !simulate {
+ // Deduct only during DeliverTx (never in CheckTx or simulation)
+ if !fees.IsZero() && !simulate && !ctx.IsCheckTx() {
   err = DeductFees(dfd.ak, dfd.evmkeeper, dfd.gasTokenKeeper, dfd.bankKeeper, ctx, deductFeesFromAcc, fees)
   if err != nil {
     return ctx, err
   }
 }

---

225-231: Remove manual sequence sync in gastoken ante decorator

The EVM ante handlers in evmante_gas_consume.go and evmante_increment_sender_seq.go already sync and increment the Cosmos account sequence from the EVM nonce, making this block redundant and prone to desynchronization. Remove the SetSequence/SetAccount calls and simply return nil.

- if err := acc.SetSequence(nonce); err != nil {
-   return sdkioerrors.Wrapf(err, "failed to set sequence to %d", nonce)
- }
-
- accountkeeper.SetAccount(ctx, acc)
-
- return nil
+ return nil

🧹 Nitpick comments (17)

contrib/scripts/localnet-uniswap.sh (8)

2-2: Harden shell execution flags.

Use strict mode for safer failure handling.

-set -e
+set -Eeuo pipefail

---

33-49: Logging helpers print colors on separate lines; use printf.

Keeps messages on one line with colors.

-echo_info() {
-  echo "${blue}"
-  echo "$1"
-  echo "${reset}"
-}
+echo_info()   { printf '%b\n' "${blue}$1${reset}"; }
 
-echo_error() {
-  echo "${red}"
-  echo "$1"
-  echo "${reset}"
-}
+echo_error()  { printf '%b\n' "${red}$1${reset}"; }
 
-echo_success() {
-  echo "${green}"
-  echo "$1"
-  echo "${reset}"
-}
+echo_success(){ printf '%b\n' "${green}$1${reset}"; }

---

145-151: Config commands: quote vars and prefer explicit home.

Avoids surprises if multiple homes exist.

-$BINARY config keyring-backend test
-$BINARY config chain-id $CHAIN_ID
+$BINARY config keyring-backend test
+$BINARY config chain-id "$CHAIN_ID"
 $BINARY config broadcast-mode sync
 $BINARY config output json
-$BINARY config node "http://localhost:26657"
+$BINARY config node "http://localhost:26657"
 $BINARY config # Prints config.

---

201-211: Genesis param helper: avoid useless cat, quote paths, check jq.

Prevents failures on spaces and missing jq.

-add_genesis_param() {
-  echo "jq input $1"
-  # copy param ($1) to tmp_genesis.json
-  cat $CHAIN_DIR/config/genesis.json | jq "$1" >$CHAIN_DIR/config/tmp_genesis.json
-  # rewrite genesis.json with the contents of tmp_genesis.json
-  mv $CHAIN_DIR/config/tmp_genesis.json $CHAIN_DIR/config/genesis.json
-}
+add_genesis_param() {
+  command -v jq >/dev/null 2>&1 || { echo_error "jq not found"; exit 1; }
+  local tmp="$CHAIN_DIR/config/tmp_genesis.json"
+  jq "$1" "$CHAIN_DIR/config/genesis.json" > "$tmp"
+  mv "$tmp" "$CHAIN_DIR/config/genesis.json"
+}

---

231-233: Gastoken config likely incomplete.

Only erc20_address and name are set; confirm required fields (denom, decimals, enabled, etc.) per module schema.

Would you like me to cross-check the gastoken feetoken schema and propose a full init block?

---

260-261: Optional: exec the node process.

Replaces the shell with the node for better signal handling.

-$BINARY start --home "$CHAIN_DIR" --pruning nothing
+exec "$BINARY" start --home "$CHAIN_DIR" --pruning nothing

---

14-17: Add basic prereq checks (jq/sed).

Early, clear failures help.

 # ------------------------------------------------------------
 # Set up colored text logging
 # ------------------------------------------------------------
 
+for cmd in jq sed; do
+  command -v "$cmd" >/dev/null 2>&1 || { >&2 echo "Error: '$cmd' not found"; exit 1; }
+done

---

75-76: Drop or implement FLAG_SPOT handling.
ShellCheck SC2034 confirms FLAG_SPOT is set in the spot) case but never read (contrib/scripts/localnet-uniswap.sh:75). Remove the spot) branch or wire the flag into the script’s logic.

x/evm/keeper/keeper.go (5)

97-99: Use Cosmos SDK error wrapping for zero contract validation.

Prefer sdkioerrors.Wrap with a concrete sdk error for consistency across the module.

-	if contract == (gethcommon.Address{}) {
-		return nil, fmt.Errorf("contract address is empty")
-	}
+	if contract == (gethcommon.Address{}) {
+		return nil, sdkioerrors.Wrap(sdkerrors.ErrInvalidAddress, "contract address cannot be zero")
+	}

---

111-116: Minor: format contract address with String() for %s verb.

Keeps formatting explicit and consistent with account.

-		return nil, sdkioerrors.Wrapf(
-			err, "failed to get balance of account %s for token %s",
-			account.String(), contract,
-		)
+		return nil, sdkioerrors.Wrapf(
+			err, "failed to get balance of account %s for token %s",
+			account.String(), contract.String(),
+		)

---

137-147: Remove temp big.Int and fix misleading comment on Value.

Use evm.Big0 and clarify that no ETH value is sent in ERC20 calls.

-	unusedBigInt := big.NewInt(0)
-	evmMsg := core.Message{
+	evmMsg := core.Message{
 		To:               &contract,
 		From:             sender,
 		Nonce:            nonce,
-		Value:            unusedBigInt, // amount
+		Value:            evm.Big0, // no ETH value for ERC20 call
-		GasLimit:         Erc20GasLimitExecute,
-		GasPrice:         unusedBigInt,
-		GasFeeCap:        unusedBigInt,
-		GasTipCap:        unusedBigInt,
+		GasLimit:         Erc20GasLimitExecute,
+		GasPrice:         evm.Big0,
+		GasFeeCap:        evm.Big0,
+		GasTipCap:        evm.Big0,
 		Data:             input,
 		AccessList:       gethcore.AccessList{},
 		SkipNonceChecks:  false,
 		SkipFromEOACheck: false,
 	}

---

191-201: Use evm.Big0 and fix Value comment in approve path too.

-	unusedBigInt := big.NewInt(0)
-	evmMsg := core.Message{
+	evmMsg := core.Message{
 		To:               &contract,
 		From:             sender,
 		Nonce:            nonce,
-		Value:            unusedBigInt, // amount
+		Value:            evm.Big0, // no ETH value for ERC20 call
 		GasLimit:         Erc20GasLimitExecute,
-		GasPrice:         unusedBigInt,
-		GasFeeCap:        unusedBigInt,
-		GasTipCap:        unusedBigInt,
+		GasPrice:         evm.Big0,
+		GasFeeCap:        evm.Big0,
+		GasTipCap:        evm.Big0,
 		Data:             input,
 		AccessList:       gethcore.AccessList{},
 		SkipNonceChecks:  false,
 		SkipFromEOACheck: false,
 	}

---

285-298: Set mock message to skip nonce/from checks for read-only calls.

Static balance lookups shouldn’t require nonce/EOA checks; relax them on the mock.

 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,
+	SkipNonceChecks:  true,
+	SkipFromEOACheck: true,
 }

x/gastoken/ante/decorator.go (4)

438-446: Align gas limits in quoter call (avoid under-gassing).

You set evmMsg.GasLimit to 2,500,000 but pass 300,000 to CallContractWithInput. Use the same higher limit.

-resp, err := ek.CallContractWithInput(ctx, evmObj, gethcommon.Address{}, &quoterAddr, false, input, evmkeeper.GetCallGasWithLimit(ctx, evmkeeper.Erc20GasLimitExecute), nil)
+resp, err := ek.CallContractWithInput(ctx, evmObj, gethcommon.Address{}, &quoterAddr, false, input, evmkeeper.GetCallGasWithLimit(ctx, 2_500_000), nil)

Also applies to: 458-461

---

148-149: Remove stray println; use structured logging if needed.

Avoid stdout in ante handlers.

- fmt.Println("baseOpGasConsumed :", baseOpGasConsumed)

---

69-70: Comment contradicts behavior; clarify fee denom requirement.

Code enforces fees[0].Denom == appconst.BondDenom. Update the comment.

- // fees can be in any denom (checked for validity later)
+ // The tx declares fees in the base denom; alternative tokens are handled internally.

---

182-193: Minor: ensure consistent sequence handling for the direct WNIBI path.

You perform EVM transfers/withdraw but don’t adjust sequences here, while the swap path does. After removing manual sequence management per above, both paths will be consistent.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f51d983 and 2037a10.

📒 Files selected for processing (5)

• contrib/scripts/localnet-uniswap.sh (1 hunks)
• x/evm/embeds/embeds.go (3 hunks)
• x/evm/keeper/erc20.go (3 hunks)
• x/evm/keeper/keeper.go (3 hunks)
• x/gastoken/ante/decorator.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• x/evm/keeper/erc20.go
• x/evm/embeds/embeds.go

🧰 Additional context used

🧠 Learnings (3)

📚 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:

• x/evm/keeper/keeper.go
• x/gastoken/ante/decorator.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:

• x/evm/keeper/keeper.go

📚 Learning: 2025-09-07T16:02:13.718Z

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.718Z
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:

• x/gastoken/ante/decorator.go

🧬 Code graph analysis (2)

x/evm/keeper/keeper.go (5)

x/evm/statedb/config.go (1)

• TxConfig (15-20)

x/evm/statedb/statedb.go (1)

• StateDB (45-85)

x/evm/embeds/embeds.go (1)

• SmartContract_ERC20MinterWithMetadataUpdates (77-80)

x/evm/keeper/erc20.go (1)

• Erc20GasLimitExecute (29-29)

x/evm/const.go (2)

• EVM_MODULE_ADDRESS (106-106)
• Big0 (217-217)

x/gastoken/ante/decorator.go (9)

x/gastoken/types/expected_keeper.go (3)

• AccountKeeper (11-16)
• BankKeeper (24-32)
• FeegrantKeeper (19-21)

x/evm/keeper/keeper.go (1)

• Keeper (30-57)

x/gastoken/keeper/keeper.go (1)

• Keeper (23-33)

app/ante/handler_opts.go (1)

• TxFeeChecker (59-59)

app/appconst/appconst.go (1)

• BondDenom (22-22)

eth/eth_account.go (1)

• NibiruAddrToEthAddr (19-21)

x/evm/const.go (1)

• NativeToWei (123-126)

x/evm/embeds/embeds.go (3)

• SmartContract_WNIBI (102-105)
• SmartContract_UniswapV3SwapRouter (201-204)
• SmartContract_UniswapV3Quoter (205-208)

x/evm/keeper/erc20.go (1)

• Erc20GasLimitExecute (29-29)

🪛 Shellcheck (0.10.0)

contrib/scripts/localnet-uniswap.sh

[error] 26-26: This expression is constant. Did you forget a $ somewhere?

(SC2078)

---

[warning] 75-75: FLAG_SPOT appears unused. Verify use (or export if used externally).

(SC2034)

---

[warning] 109-109: Quotes/backslashes will be treated literally. Use an array.

(SC2089)

---

[warning] 154-154: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 158-158: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 163-163: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 166-166: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 169-169: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 173-173: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 177-177: Quotes/backslashes in this variable will not be respected.

(SC2090)

⏰ 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: build
• GitHub Check: integration-tests
• GitHub Check: proto-gen-rs
• GitHub Check: e2e-evm
• GitHub Check: unit-tests

🔇 Additional comments (3)

x/evm/keeper/keeper.go (2)

122-134: LGTM: validations and ABI pack.

Address and amount validation look good, and packing “transfer” args is correct.

---

176-188: LGTM: validations and ABI pack for approve.

Consistent with transfer, and the approve pack message is correct.

x/gastoken/ante/decorator.go (1)

314-333: Verify Uniswap router ABI; exactOutputSingle likely requires a deadline param.

If embeds.SmartContract_UniswapV3SwapRouter uses ISwapRouter.ExactOutputSingleParams, you must include deadline.

 input, err := embeds.SmartContract_UniswapV3SwapRouter.ABI.Pack(
-  "exactOutputSingle",
-  struct {
+  "exactOutputSingle",
+  struct {
     TokenIn           gethcommon.Address
     TokenOut          gethcommon.Address
-    Fee               *big.Int
+    Fee               *big.Int
     Recipient         gethcommon.Address
+    Deadline          *big.Int
     AmountOut         *big.Int
     AmountInMaximum   *big.Int
     SqrtPriceLimitX96 *big.Int
   }{
     TokenIn:           feeToken,
     TokenOut:          gethcommon.HexToAddress(txFeesParams.WnibiAddress),
     Fee:               big.NewInt(3000),
     Recipient:         feeCollector,
+    Deadline:          big.NewInt(ctx.BlockTime().Unix() + 600), // +10 min
     AmountOut:         amountOut,
     AmountInMaximum:   amountIn,
     SqrtPriceLimitX96: big.NewInt(0),
   },
 )

If your embedded router omits deadline (custom), ignore this change.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (5)

contrib/scripts/localnet-uniswap.sh (5)

107-110: Make sed -i macOS-compatible and quote targets.

Use an array for in-place args and apply it to every sed call; fixes SC2089/SC2090 and path quoting.

-SEDOPTION=""
-if [[ "$OSTYPE" == "darwin"* ]]; then
-  SEDOPTION="''"
-fi
+if [[ "$OSTYPE" == "darwin"* ]]; then
+  SED_INPLACE=(-i '')
+else
+  SED_INPLACE=(-i)
+fi
@@
-sed -i $SEDOPTION '/\[api\]/,+3 s/enable = false/enable = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[api\]/,+3 s/enable = false/enable = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[grpc\]/,+3 s/enable = false/enable = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[grpc\]/,+3 s/enable = false/enable = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[json\-rpc\]/,+3 s/enable = false/enable = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[json\-rpc\]/,+3 s/enable = false/enable = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[json\-rpc\]/,+13 s/api = "eth,net,web3"/api = "eth,net,web3,debug"/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[json\-rpc\]/,+13 s/api = "eth,net,web3"/api = "eth,net,web3,debug"/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[json\-rpc\]/,+51 s/enable-indexer = false/enable-indexer = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[json\-rpc\]/,+51 s/enable-indexer = false/enable-indexer = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION 's/swagger = false/swagger = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" 's/swagger = false/swagger = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION 's/enabled-unsafe-cors = false/enabled-unsafe-cors = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" 's/enabled-unsafe-cors = false/enabled-unsafe-cors = true/' "$CHAIN_DIR/config/app.toml"

Also applies to: 152-177

---

11-13: Add a fail-fast check for required binaries (nibid, jq).

Prevent destructive resets when dependencies are missing; verify early and exit with a clear error.

 echo "CHAIN_DIR: $CHAIN_DIR"
 echo "CHAIN_ID: $CHAIN_ID"
 
+for dep in "$BINARY" jq; do
+  if ! command -v "$dep" >/dev/null 2>&1; then
+    >&2 echo "Error: Required dependency '$dep' not found in PATH."
+    exit 1
+  fi
+done

---

191-194: Genesis accounts: quote, use explicit home.

-$BINARY add-genesis-account $val_address $GENESIS_COINS
+"$BINARY" add-genesis-account "$val_address" "$GENESIS_COINS" --home "$CHAIN_DIR"
 # EVM encoded nibi address for the same account
-$BINARY add-genesis-account nibi1cr6tg4cjvux00pj6zjqkh6d0jzg7mksaywxyl3 $GENESIS_COINS
-$BINARY add-genesis-account nibi1ltez0kkshywzm675rkh8rj2eaf8et78cqjqrhc $GENESIS_COINS
+"$BINARY" add-genesis-account "nibi1cr6tg4cjvux00pj6zjqkh6d0jzg7mksaywxyl3" "$GENESIS_COINS" --home "$CHAIN_DIR"
+"$BINARY" add-genesis-account "nibi1ltez0kkshywzm675rkh8rj2eaf8et78cqjqrhc" "$GENESIS_COINS" --home "$CHAIN_DIR"

---

242-253: Gentx/collect must target explicit home and quote args.

-if $BINARY genesis gentx $val_key_name 900000000unibi --chain-id $CHAIN_ID; then
+if "$BINARY" genesis gentx "$val_key_name" "900000000unibi" --chain-id "$CHAIN_ID" --home "$CHAIN_DIR"; then
@@
-if $BINARY genesis collect-gentxs; then
+if "$BINARY" genesis collect-gentxs --home "$CHAIN_DIR"; then

---

18-31: Color init is broken; conditional is constant true and apt-get path is wrong.

The test never calls the function (SC2078). Also avoid package installs; degrade gracefully if tput isn’t available.

-# Console log text colour
-console_log_text_color() {
-  red=$(tput setaf 9)
-  green=$(tput setaf 10)
-  blue=$(tput setaf 12)
-  reset=$(tput sgr0)
-}
-
-if [ console_log_text_color ]; then
-  echo "succesfully toggled console coloring"
-else
-  # For Ubuntu and Debian. MacOS has tput by default.
-  apt-get install libncurses5-dbg -y
-fi
+# Console log text colour
+console_log_text_color() {
+  if command -v tput >/dev/null 2>&1 && [ "$(tput colors 2>/dev/null || echo 0)" -ge 8 ]; then
+    red="$(tput setaf 1)"; green="$(tput setaf 2)"; blue="$(tput setaf 4)"; reset="$(tput sgr0)"
+    return 0
+  else
+    red=""; green=""; blue=""; reset=""
+    return 1
+  fi
+}
+
+if console_log_text_color; then
+  echo "successfully enabled console coloring"
+else
+  echo "tput not available or terminal lacks color support; continuing without colors"
+fi

🧹 Nitpick comments (6)

contrib/scripts/localnet-uniswap.sh (6)

2-2: Enable pipefail to catch jq/pipe errors.

Without pipefail, failures in pipelines can be masked.

-set -e
+set -e
+set -o pipefail

---

55-70: Feature flag is parsed but unused.

Either wire FLAG_SPOT into behavior or remove to avoid dead code (SC2034).

-  spot) FLAG_SPOT=true ;;
+  spot) FLAG_SPOT=true ;; # TODO: consume this flag or remove

Also applies to: 80-97, 100-106

---

116-121: Process stop: use $BINARY consistently and avoid set -e aborts.

Replace hardcoded killall and guard failures.

 if pgrep -x "$BINARY" >/dev/null; then
   echo_error "Terminating $BINARY..."
-  killall nibid
+  pkill -x "$BINARY" || true
 fi

---

201-211: Harden add_genesis_param: quote paths and avoid UUOC.

Safer and cleaner, works with set -o pipefail.

 add_genesis_param() {
   echo "jq input $1"
-  # copy param ($1) to tmp_genesis.json
-  cat $CHAIN_DIR/config/genesis.json | jq "$1" >$CHAIN_DIR/config/tmp_genesis.json
-  # rewrite genesis.json with the contents of tmp_genesis.json
-  mv $CHAIN_DIR/config/tmp_genesis.json $CHAIN_DIR/config/genesis.json
+  jq "$1" "$CHAIN_DIR/config/genesis.json" > "$CHAIN_DIR/config/tmp_genesis.json"
+  mv "$CHAIN_DIR/config/tmp_genesis.json" "$CHAIN_DIR/config/genesis.json"
 }

---

238-239: Quote jq inputs/outputs.

Avoids issues with spaces in paths.

-jq --slurpfile evm contrib/evm.json '.app_state.evm = $evm[0].evm | .app_state.auth = $evm[0].auth' $CHAIN_DIR/config/genesis.json > $CHAIN_DIR/config/genesis_tmp.json
-mv $CHAIN_DIR/config/genesis_tmp.json $CHAIN_DIR/config/genesis.json
+jq --slurpfile evm "contrib/evm.json" '.app_state.evm = $evm[0].evm | .app_state.auth = $evm[0].auth' "$CHAIN_DIR/config/genesis.json" > "$CHAIN_DIR/config/genesis_tmp.json"
+mv "$CHAIN_DIR/config/genesis_tmp.json" "$CHAIN_DIR/config/genesis.json"

---

259-260: Consider surfacing start URL and important ports.

Minor DX boost: print RPC, API, gRPC, JSON-RPC endpoints after start.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2037a10 and 3cf605e.

📒 Files selected for processing (1)

• contrib/scripts/localnet-uniswap.sh (1 hunks)

🧰 Additional context used

🪛 Shellcheck (0.10.0)

contrib/scripts/localnet-uniswap.sh

[error] 26-26: This expression is constant. Did you forget a $ somewhere?

(SC2078)

---

[warning] 75-75: FLAG_SPOT appears unused. Verify use (or export if used externally).

(SC2034)

---

[warning] 109-109: Quotes/backslashes will be treated literally. Use an array.

(SC2089)

---

[warning] 154-154: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 158-158: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 163-163: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 166-166: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 169-169: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 173-173: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 177-177: Quotes/backslashes in this variable will not be respected.

(SC2090)

⏰ 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: unit-tests
• GitHub Check: build
• GitHub Check: e2e-evm
• GitHub Check: integration-tests
• GitHub Check: lint

🔇 Additional comments (2)

contrib/scripts/localnet-uniswap.sh (2)

215-233: Verify genesis shape before writing indexed fields.

Assumes arrays/paths exist; otherwise jq may fail. Confirm presence or create arrays defensively.

Would you like a guarded jq helper that initializes arrays/objects if missing?

---

7-9: Secrets in script: ensure this mnemonic is dev-only.

Confirm no risk of accidental reuse in non-local environments.

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (8)

contrib/scripts/localnet-uniswap.sh (8)

11-13: Fail fast when nibid is missing.

Prevents destructive steps from running without the binary.

 echo "CHAIN_DIR: $CHAIN_DIR"
 echo "CHAIN_ID: $CHAIN_ID"
 
+if ! command -v "$BINARY" >/dev/null 2>&1; then
+  >&2 echo "Error: Binary '$BINARY' not found in PATH. Build/install it first or pass --no-build."
+  exit 1
+fi

---

191-195: Quote variables for genesis-account adds.

Prevents word-splitting; keep explicit home.

-$BINARY add-genesis-account $val_address $GENESIS_COINS --home "$CHAIN_DIR"
+"$BINARY" add-genesis-account "$val_address" "$GENESIS_COINS" --home "$CHAIN_DIR"
 # EVM encoded nibi address for the same account
-$BINARY add-genesis-account nibi1cr6tg4cjvux00pj6zjqkh6d0jzg7mksaywxyl3 $GENESIS_COINS --home "$CHAIN_DIR"
-$BINARY add-genesis-account nibi1ltez0kkshywzm675rkh8rj2eaf8et78cqjqrhc $GENESIS_COINS --home "$CHAIN_DIR"
+"$BINARY" add-genesis-account "nibi1cr6tg4cjvux00pj6zjqkh6d0jzg7mksaywxyl3" "$GENESIS_COINS" --home "$CHAIN_DIR"
+"$BINARY" add-genesis-account "nibi1ltez0kkshywzm675rkh8rj2eaf8et78cqjqrhc" "$GENESIS_COINS" --home "$CHAIN_DIR"

---

241-253: Quote variables in gentx/collect; consider failing on error.

Prevents word-splitting; optionally exit on failure.

-if $BINARY genesis gentx $val_key_name 900000000unibi --chain-id $CHAIN_ID --home "$CHAIN_DIR"; then
+if "$BINARY" genesis gentx "$val_key_name" "900000000unibi" --chain-id "$CHAIN_ID" --home "$CHAIN_DIR"; then
   echo_success "Successfully added gentx"
 else
   echo_error "Failed to add gentx"
+  # exit 1
 fi
@@
-if $BINARY genesis collect-gentxs --home "$CHAIN_DIR"; then
+if "$BINARY" genesis collect-gentxs --home "$CHAIN_DIR"; then
   echo_success "Successfully collected genesis txs into genesis.json"
 else
   echo_error "Failed to collect genesis txs"
+  # exit 1
 fi

---

18-31: Color init never runs and has portability issues.

The test is constant-true (SC2078); colors not initialized; apt-get in scripts is undesirable. Make init conditional on tput and call it.

-# Console log text colour
-console_log_text_color() {
-  red=$(tput setaf 9)
-  green=$(tput setaf 10)
-  blue=$(tput setaf 12)
-  reset=$(tput sgr0)
-}
-
-if [ console_log_text_color ]; then
-  echo "succesfully toggled console coloring"
-else
-  # For Ubuntu and Debian. MacOS has tput by default.
-  apt-get install libncurses5-dbg -y
-fi
+# Console log text colour
+console_log_text_color() {
+  if command -v tput >/dev/null 2>&1 && [ "$(tput colors 2>/dev/null || echo 0)" -ge 8 ]; then
+    red="$(tput setaf 1)"
+    green="$(tput setaf 2)"
+    blue="$(tput setaf 4)"
+    reset="$(tput sgr0)"
+    return 0
+  fi
+  red=""; green=""; blue=""; reset=""
+  return 1
+}
+
+if console_log_text_color; then
+  echo "successfully enabled console coloring"
+else
+  echo "tput not available or terminal lacks color support; continuing without colors"
+fi

---

107-111: Make sed -i portable (macOS) and quote paths.

Current SEDOPTION pattern triggers SC2089/SC2090 and breaks on macOS. Use an array and apply to all sed calls.

-SEDOPTION=""
-if [[ "$OSTYPE" == "darwin"* ]]; then
-  SEDOPTION="''"
-fi
+if [[ "$OSTYPE" == "darwin"* ]]; then
+  SED_INPLACE=(-i '')
+else
+  SED_INPLACE=(-i)
+fi
@@
-sed -i $SEDOPTION '/\[api\]/,+3 s/enable = false/enable = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[api\]/,+3 s/enable = false/enable = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[grpc\]/,+3 s/enable = false/enable = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[grpc\]/,+3 s/enable = false/enable = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[json\-rpc\]/,+3 s/enable = false/enable = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[json\-rpc\]/,+3 s/enable = false/enable = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[json\-rpc\]/,+13 s/api = "eth,net,web3"/api = "eth,net,web3,debug"/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[json\-rpc\]/,+13 s/api = "eth,net,web3"/api = "eth,net,web3,debug"/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[json\-rpc\]/,+51 s/enable-indexer = false/enable-indexer = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[json\-rpc\]/,+51 s/enable-indexer = false/enable-indexer = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION 's/swagger = false/swagger = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" 's/swagger = false/swagger = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION 's/enabled-unsafe-cors = false/enabled-unsafe-cors = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" 's/enabled-unsafe-cors = false/enabled-unsafe-cors = true/' "$CHAIN_DIR/config/app.toml"

Also applies to: 154-177

---

116-121: Use pkill and keep using $BINARY consistently.

Avoids hardcoded name and handles set -e safely.

 if pgrep -x "$BINARY" >/dev/null; then
   echo_error "Terminating $BINARY..."
-  killall nibid
+  pkill -x "$BINARY" || true
 fi

---

122-151: Inconsistent --home usage can corrupt/misplace state.

Several nibid commands touch default home while files are edited under $CHAIN_DIR.

 echo_info "Removing previous chain data from $CHAIN_DIR..."
-$BINARY tendermint unsafe-reset-all
+"$BINARY" tendermint unsafe-reset-all --home "$CHAIN_DIR"
@@
 echo_info "Initializing $CHAIN_ID..."
-if $BINARY init $CHAIN_ID --chain-id $CHAIN_ID --overwrite; then
+if "$BINARY" init "$CHAIN_ID" --chain-id "$CHAIN_ID" --overwrite --home "$CHAIN_DIR"; then
@@
 echo_info "Updating nibid config..."
-$BINARY config keyring-backend test
-$BINARY config chain-id $CHAIN_ID
-$BINARY config broadcast-mode sync
-$BINARY config output json
-$BINARY config node "http://localhost:26657"
-$BINARY config # Prints config.
+"$BINARY" config keyring-backend test --home "$CHAIN_DIR"
+"$BINARY" config chain-id "$CHAIN_ID" --home "$CHAIN_DIR"
+"$BINARY" config broadcast-mode sync --home "$CHAIN_DIR"
+"$BINARY" config output json --home "$CHAIN_DIR"
+"$BINARY" config node "http://localhost:26657" --home "$CHAIN_DIR"
+"$BINARY" config --home "$CHAIN_DIR" # Prints config.

---

183-189: Keys ops should be idempotent, scoped to home, and quiet.

Avoid default keyring and noisy stderr on existence.

-if ! $BINARY keys show $val_key_name; then 
-  echo "$MNEMONIC" | $BINARY keys add $val_key_name --recover
+if ! "$BINARY" keys show "$val_key_name" --home "$CHAIN_DIR" --keyring-backend test >/dev/null 2>&1; then
+  echo "$MNEMONIC" | "$BINARY" keys add "$val_key_name" --recover --home "$CHAIN_DIR" --keyring-backend test
   echo_success "Successfully added key: $val_key_name"
 fi
 
-val_address=$($BINARY keys show $val_key_name -a)
+val_address=$("$BINARY" keys show "$val_key_name" -a --home "$CHAIN_DIR" --keyring-backend test)

🧹 Nitpick comments (3)

contrib/scripts/localnet-uniswap.sh (3)

205-211: Harden add_genesis_param and quote paths.

Avoid UUOC and unquoted paths.

 add_genesis_param() {
   echo "jq input $1"
-  # copy param ($1) to tmp_genesis.json
-  cat $CHAIN_DIR/config/genesis.json | jq "$1" >$CHAIN_DIR/config/tmp_genesis.json
-  # rewrite genesis.json with the contents of tmp_genesis.json
-  mv $CHAIN_DIR/config/tmp_genesis.json $CHAIN_DIR/config/genesis.json
+  jq "$1" "$CHAIN_DIR/config/genesis.json" > "$CHAIN_DIR/config/tmp_genesis.json"
+  mv "$CHAIN_DIR/config/tmp_genesis.json" "$CHAIN_DIR/config/genesis.json"
 }

---

238-239: Quote jq inputs/outputs.

Safer with spaces in paths; consistent with earlier quoting.

-jq --slurpfile evm contrib/evm.json '.app_state.evm = $evm[0].evm | .app_state.auth = $evm[0].auth' $CHAIN_DIR/config/genesis.json > $CHAIN_DIR/config/genesis_tmp.json
-mv $CHAIN_DIR/config/genesis_tmp.json $CHAIN_DIR/config/genesis.json
+jq --slurpfile evm contrib/evm.json \
+  '.app_state.evm = $evm[0].evm | .app_state.auth = $evm[0].auth' \
+  "$CHAIN_DIR/config/genesis.json" > "$CHAIN_DIR/config/genesis_tmp.json"
+mv "$CHAIN_DIR/config/genesis_tmp.json" "$CHAIN_DIR/config/genesis.json"

---

75-76: Unused feature flag (FLAG_SPOT).

Either wire it into behavior or remove to avoid SC2034.

Example: surface the flag in output or gate logic:

 echo_info "Features flags:"
-echo "FLAG_SKIP_BUILD: $FLAG_SKIP_BUILD"
+echo "FLAG_SKIP_BUILD: $FLAG_SKIP_BUILD"
+echo "FLAG_SPOT: ${FLAG_SPOT:-false}"

Also applies to: 104-106

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3cf605e and 71dcdee.

📒 Files selected for processing (2)

• contrib/scripts/localnet-uniswap.sh (1 hunks)
• x/gastoken/ante/decorator.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• x/gastoken/ante/decorator.go

🧰 Additional context used

🪛 Shellcheck (0.10.0)

contrib/scripts/localnet-uniswap.sh

[error] 26-26: This expression is constant. Did you forget a $ somewhere?

(SC2078)

---

[warning] 75-75: FLAG_SPOT appears unused. Verify use (or export if used externally).

(SC2034)

---

[warning] 109-109: Quotes/backslashes will be treated literally. Use an array.

(SC2089)

---

[warning] 154-154: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 158-158: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 163-163: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 166-166: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 169-169: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 173-173: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 177-177: Quotes/backslashes in this variable will not be respected.

(SC2090)

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (4)

x/evm/keeper/keeper.go (4)

223-225: Correct the operation name and wrap as typed error.

Message says “transfer” in approve path and uses fmt.Errorf.

-if resp.Failed() {
-  return fmt.Errorf("ERC20 transfer failed with VM error: %s", resp.VmError)
-}
+if resp.Failed() {
+  return sdkioerrors.Wrapf(sdkerrors.ErrInvalidRequest, "ERC20 approve failed with VM error: %v", resp.Err)
+}

---

104-111: Don’t always nil-out Bank.StateDB; only clean up if you created it.

Avoid wiping a caller-provided StateDB.

Apply:

 txConfig := k.TxConfig(ctx, gethcommon.Hash{})
 stateDB := k.Bank.StateDB
-if stateDB == nil {
-  stateDB = k.NewStateDB(ctx, txConfig)
-}
-defer func() {
-  k.Bank.StateDB = nil
-}()
+created := false
+if stateDB == nil {
+  stateDB = k.NewStateDB(ctx, txConfig)
+  created = true
+}
+if created {
+  defer func() { k.Bank.StateDB = nil }()
+}

#!/bin/bash
# Grep for unconditional StateDB cleanup patterns
rg -nC2 $'StateDB = nil' x/evm | sed -n '1,200p'

---

155-162: Repeat: only nil-out Bank.StateDB if created in this function.

Same rationale as above.

 txConfig := k.TxConfig(ctx, gethcommon.Hash{})
 stateDB := k.Bank.StateDB
-if stateDB == nil {
-  stateDB = k.NewStateDB(ctx, txConfig)
-}
-defer func() {
-  k.Bank.StateDB = nil
-}()
+created := false
+if stateDB == nil {
+  stateDB = k.NewStateDB(ctx, txConfig)
+  created = true
+}
+if created {
+  defer func() { k.Bank.StateDB = nil }()
+}

---

209-216: Repeat: only nil-out Bank.StateDB if created here.

Avoid clearing caller-provided StateDB.

 txConfig := k.TxConfig(ctx, gethcommon.Hash{})
 stateDB := k.Bank.StateDB
-if stateDB == nil {
-  stateDB = k.NewStateDB(ctx, txConfig)
-}
-defer func() {
-  k.Bank.StateDB = nil
-}()
+created := false
+if stateDB == nil {
+  stateDB = k.NewStateDB(ctx, txConfig)
+  created = true
+}
+if created {
+  defer func() { k.Bank.StateDB = nil }()
+}

🧹 Nitpick comments (5)

x/evm/keeper/keeper.go (5)

99-103: Use typed SDK error for zero contract address.

Prefer consistent taxonomy over fmt.Errorf.

Apply:

- if contract == (gethcommon.Address{}) {
-   return nil, fmt.Errorf("contract address is empty")
- }
+ if contract == (gethcommon.Address{}) {
+   return nil, sdkioerrors.Wrap(sdkerrors.ErrInvalidAddress, "contract address cannot be zero")
+ }

---

116-119: Fix format args in error message.

Ensure both placeholders are strings.

-    err, "failed to get balance of account %s for token %s",
-    account.String(), contract,
+    err, "failed to get balance of account %s for token %s",
+    account.Hex(), contract.Hex(),

---

140-154: Clarify zero values and avoid shared pointer aliasing.

Inline fresh zero big.Ints or name them clearly.

-unusedBigInt := big.NewInt(0)
 evmMsg := core.Message{
   To:               &contract,
   From:             sender,
   Nonce:            nonce,
-  Value:            unusedBigInt, // amount
+  Value:            new(big.Int), // no native value transfer
   GasLimit:         Erc20GasLimitExecute,
-  GasPrice:         unusedBigInt,
-  GasFeeCap:        unusedBigInt,
-  GasTipCap:        unusedBigInt,
+  GasPrice:         new(big.Int),
+  GasFeeCap:        new(big.Int),
+  GasTipCap:        new(big.Int),
   Data:             input,

---

169-171: Wrap VM failure with typed error and include engine error.

Keeps error taxonomy and preserves context.

-if resp.Failed() {
-  return fmt.Errorf("ERC20 transfer failed with VM error: %s", resp.VmError)
-}
+if resp.Failed() {
+  return sdkioerrors.Wrapf(sdkerrors.ErrInvalidRequest, "ERC20 transfer failed with VM error: %v", resp.Err)
+}

---

195-208: Same zero-value nit as Transfer.

Use fresh zeros and a clearer comment.

-unusedBigInt := big.NewInt(0)
 evmMsg := core.Message{
   To:               &contract,
   From:             sender,
   Nonce:            nonce,
-  Value:            unusedBigInt, // amount
+  Value:            new(big.Int), // no native value transfer
   GasLimit:         Erc20GasLimitExecute,
-  GasPrice:         unusedBigInt,
-  GasFeeCap:        unusedBigInt,
-  GasTipCap:        unusedBigInt,
+  GasPrice:         new(big.Int),
+  GasFeeCap:        new(big.Int),
+  GasTipCap:        new(big.Int),
   Data:             input,

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 71dcdee and df7d6ea.

📒 Files selected for processing (2)

• x/evm/evmtest/test_deps.go (2 hunks)
• x/evm/keeper/keeper.go (4 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• x/evm/evmtest/test_deps.go

🧰 Additional context used

🧠 Learnings (3)

📚 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:

• x/evm/keeper/keeper.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:

• x/evm/keeper/keeper.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:

• x/evm/keeper/keeper.go

🧬 Code graph analysis (1)

x/evm/keeper/keeper.go (7)

x/gastoken/keeper/keeper.go (1)

• Keeper (23-33)

x/evm/statedb/interfaces.go (1)

• Keeper (11-31)

x/evm/statedb/config.go (1)

• TxConfig (15-20)

x/evm/statedb/statedb.go (1)

• StateDB (45-85)

x/evm/embeds/embeds.go (1)

• SmartContract_ERC20MinterWithMetadataUpdates (77-80)

x/evm/keeper/erc20.go (1)

• Erc20GasLimitExecute (29-29)

x/evm/const.go (2)

• EVM_MODULE_ADDRESS (106-106)
• Big0 (217-217)

⏰ 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: lint
• GitHub Check: e2e-evm

🔇 Additional comments (6)

x/evm/keeper/keeper.go (6)

9-13: Imports LGTM.

New geth/core and sdkioerrors imports are appropriate.

---

21-21: Typed SDK errors import is correct.

Consistent with the error handling elsewhere.

---

26-26: ABI embeds import LGTM.

Required for ERC20 pack calls.

---

126-131: Input validation LGTM.

Zero-address and non-negative amount checks are good.

---

180-185: Input validation LGTM.

Matches Transfer’s checks; consistent and safe.

---

324-337: MOCK_GETH_MESSAGE LGTM.

Sane defaults for read-only calls; helpful for BalanceOf.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (9)

contrib/scripts/localnet-uniswap.sh (9)

242-254: Quote gentx args and ensure explicit --home everywhere.

Keeps operations scoped to CHAIN_DIR.

 echo_info "Adding gentx validator..."
-if $BINARY genesis gentx $val_key_name 900000000unibi --chain-id $CHAIN_ID --home "$CHAIN_DIR"; then
+if "$BINARY" genesis gentx "$val_key_name" "900000000unibi" --chain-id "$CHAIN_ID" --home "$CHAIN_DIR"; then
   echo_success "Successfully added gentx"
 else
   echo_error "Failed to add gentx"
 fi
 
 echo_info "Collecting gentx..."
-if $BINARY genesis collect-gentxs --home "$CHAIN_DIR"; then
+if "$BINARY" genesis collect-gentxs --home "$CHAIN_DIR"; then
   echo_success "Successfully collected genesis txs into genesis.json"
 else
   echo_error "Failed to collect genesis txs"
 fi

---

11-13: Add a fail-fast check for missing binary before any resets.

Prevents destructive ops if nibid isn't installed.

 echo "CHAIN_DIR: $CHAIN_DIR"
 echo "CHAIN_ID: $CHAIN_ID"
 
+if ! command -v "$BINARY" >/dev/null 2>&1; then
+  >&2 echo "Error: Binary '$BINARY' not found in PATH. Build or install it first."
+  exit 1
+fi

---

18-31: Fix color init: conditional is always-true; avoid apt-get and degrade gracefully.

Current check never calls the function; apt-get in script is undesirable.

-# Console log text colour
-console_log_text_color() {
-  red=$(tput setaf 9)
-  green=$(tput setaf 10)
-  blue=$(tput setaf 12)
-  reset=$(tput sgr0)
-}
-
-if [ console_log_text_color ]; then
-  echo "succesfully toggled console coloring"
-else
-  # For Ubuntu and Debian. MacOS has tput by default.
-  apt-get install libncurses5-dbg -y
-fi
+# Console log text colour
+console_log_text_color() {
+  if command -v tput >/dev/null 2>&1 && [ "$(tput colors 2>/dev/null)" -ge 8 ]; then
+    red="$(tput setaf 1)"; green="$(tput setaf 2)"; blue="$(tput setaf 4)"; reset="$(tput sgr0)"
+    return 0
+  else
+    red=""; green=""; blue=""; reset=""
+    return 1
+  fi
+}
+
+if console_log_text_color; then
+  echo "successfully enabled console coloring"
+else
+  echo "tput not available or no color support; continuing without colors"
+fi

---

107-111: Make sed -i portable with an array; avoid SC2089/SC2090.

Prepares for macOS/GNU sed differences.

-SEDOPTION=""
-if [[ "$OSTYPE" == "darwin"* ]]; then
-  SEDOPTION="''"
-fi
+if [[ "$OSTYPE" == "darwin"* ]]; then
+  SED_INPLACE=(-i '')
+else
+  SED_INPLACE=(-i)
+fi

---

116-121: Stop process using $BINARY consistently; prefer pkill and don’t fail on set -e.

Avoids killing wrong process; resilient if not running.

-if pgrep -x "$BINARY" >/dev/null; then
-  echo_error "Terminating $BINARY..."
-  killall nibid
-fi
+if pgrep -x "$BINARY" >/dev/null 2>&1; then
+  echo_error "Terminating $BINARY..."
+  pkill -x "$BINARY" || true
+fi

---

134-141: Init should target CHAIN_DIR and quote variables.

Prevents writing to default home.

 echo_info "Initializing $CHAIN_ID..."
-if $BINARY init $CHAIN_ID --chain-id $CHAIN_ID --overwrite; then
+if "$BINARY" init "$CHAIN_ID" --chain-id "$CHAIN_ID" --overwrite --home "$CHAIN_DIR"; then
   echo_success "Successfully initialized $CHAIN_ID"
 else
   echo_error "Failed to initialize $CHAIN_ID"
   exit 1
 fi

---

145-151: Config commands must use --home and quote args.

Ensures consistency with files edited under CHAIN_DIR.

-$BINARY config keyring-backend test
-$BINARY config chain-id $CHAIN_ID
-$BINARY config broadcast-mode sync
-$BINARY config output json
-$BINARY config node "http://localhost:26657"
-$BINARY config # Prints config.
+"$BINARY" config keyring-backend test --home "$CHAIN_DIR"
+"$BINARY" config chain-id "$CHAIN_ID" --home "$CHAIN_DIR"
+"$BINARY" config broadcast-mode sync --home "$CHAIN_DIR"
+"$BINARY" config output json --home "$CHAIN_DIR"
+"$BINARY" config node "http://localhost:26657" --home "$CHAIN_DIR"
+"$BINARY" config --home "$CHAIN_DIR"

---

152-178: Use SED_INPLACE and quote paths for macOS compatibility.

Current sed -i usage breaks on macOS and leaves paths unquoted.

-sed -i $SEDOPTION '/\[api\]/,+3 s/enable = false/enable = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[api\]/,+3 s/enable = false/enable = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[grpc\]/,+3 s/enable = false/enable = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[grpc\]/,+3 s/enable = false/enable = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[json\-rpc\]/,+3 s/enable = false/enable = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[json\-rpc\]/,+3 s/enable = false/enable = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[json\-rpc\]/,+13 s/api = "eth,net,web3"/api = "eth,net,web3,debug"/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[json\-rpc\]/,+13 s/api = "eth,net,web3"/api = "eth,net,web3,debug"/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[json\-rpc\]/,+51 s/enable-indexer = false/enable-indexer = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[json\-rpc\]/,+51 s/enable-indexer = false/enable-indexer = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION 's/swagger = false/swagger = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" 's/swagger = false/swagger = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION 's/enabled-unsafe-cors = false/enabled-unsafe-cors = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" 's/enabled-unsafe-cors = false/enabled-unsafe-cors = true/' "$CHAIN_DIR/config/app.toml"

---

183-189: Keys: add --home and keyring, quote vars, and quiet existence check.

Avoids touching default keyring and makes idempotent.

-if ! $BINARY keys show $val_key_name; then 
-  echo "$MNEMONIC" | $BINARY keys add $val_key_name --recover
+if ! "$BINARY" keys show "$val_key_name" --home "$CHAIN_DIR" --keyring-backend test >/dev/null 2>&1; then 
+  echo "$MNEMONIC" | "$BINARY" keys add "$val_key_name" --recover --home "$CHAIN_DIR" --keyring-backend test
   echo_success "Successfully added key: $val_key_name"
 fi
 
-val_address=$($BINARY keys show $val_key_name -a)
+val_address=$("$BINARY" keys show "$val_key_name" -a --home "$CHAIN_DIR" --keyring-backend test)

🧹 Nitpick comments (12)

contrib/scripts/localnet-uniswap.sh (5)

33-49: Trim extra blank lines in log helpers.

Use single-line printf to avoid empty lines.

-echo_info() {
-  echo "${blue}"
-  echo "$1"
-  echo "${reset}"
-}
+echo_info() { printf "%s%s%s\n" "${blue}" "$1" "${reset}"; }
 
-echo_error() {
-  echo "${red}"
-  echo "$1"
-  echo "${reset}"
-}
+echo_error() { printf "%s%s%s\n" "${red}" "$1" "${reset}"; }
 
-echo_success() {
-  echo "${green}"
-  echo "$1"
-  echo "${reset}"
-}
+echo_success() { printf "%s%s%s\n" "${green}" "$1" "${reset}"; }

---

72-79: Unused feature flag variable (FLAG_SPOT).

Either wire it into behavior or drop it.

-enable_feature_flag() {
+enable_feature_flag() {
   case $1 in
-  spot) FLAG_SPOT=true ;;
+  spot) FLAG_SPOT=true ;; # TODO: consume FLAG_SPOT or remove this branch
   *) echo_error "Unknown feature: $1" ;;
   esac
 }

---

191-195: Quote amounts/vars for genesis accounts and be consistent.

Protects commas and spaces.

-$BINARY add-genesis-account $val_address $GENESIS_COINS --home "$CHAIN_DIR"
+$BINARY add-genesis-account "$val_address" "$GENESIS_COINS" --home "$CHAIN_DIR"
 # EVM encoded nibi address for the same account
-$BINARY add-genesis-account nibi1cr6tg4cjvux00pj6zjqkh6d0jzg7mksaywxyl3 $GENESIS_COINS --home "$CHAIN_DIR"
-$BINARY add-genesis-account nibi1ltez0kkshywzm675rkh8rj2eaf8et78cqjqrhc $GENESIS_COINS --home "$CHAIN_DIR"
+$BINARY add-genesis-account nibi1cr6tg4cjvux00pj6zjqkh6d0jzg7mksaywxyl3 "$GENESIS_COINS" --home "$CHAIN_DIR"
+$BINARY add-genesis-account nibi1ltez0kkshywzm675rkh8rj2eaf8et78cqjqrhc "$GENESIS_COINS" --home "$CHAIN_DIR"

---

201-211: Harden add_genesis_param: quote paths and avoid UUOC.

Safer temp overwrite pattern.

 add_genesis_param() {
   echo "jq input $1"
-  # copy param ($1) to tmp_genesis.json
-  cat $CHAIN_DIR/config/genesis.json | jq "$1" >$CHAIN_DIR/config/tmp_genesis.json
-  # rewrite genesis.json with the contents of tmp_genesis.json
-  mv $CHAIN_DIR/config/tmp_genesis.json $CHAIN_DIR/config/genesis.json
+  local src="$CHAIN_DIR/config/genesis.json"
+  local tmp="$CHAIN_DIR/config/tmp_genesis.json"
+  jq "$1" "$src" >"$tmp"
+  mv "$tmp" "$src"
 }

---

239-240: Quote jq inputs/outputs and long-line readability.

Minor robustness/readability.

-jq --slurpfile evm contrib/evm.json '.app_state.evm = $evm[0].evm | .app_state.auth = $evm[0].auth | .app_state.gastoken = $evm[0].gastoken |.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
+jq --slurpfile evm "contrib/evm.json" \
+  '.app_state.evm = $evm[0].evm
+   | .app_state.auth = $evm[0].auth
+   | .app_state.gastoken = $evm[0].gastoken
+   | .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"

evm-e2e/test/gastoken.test.ts (5)

15-23: Assert native ETH stays unchanged to actually prove gas-token payment.

Capture ETH balance before and after and assert no drop. This validates the gas-token path (vs. silently paying in native).

             const walletBalWei = await provider.getBalance(account2.address)
@@
             {
                 const amountToSend = parseUnits("0.5", 18)
                 let tx = await wnibi.connect(account2).transfer(account, amountToSend)
                 await tx.wait()
                 const wnibiBal = await wnibi.balanceOf(account2.address)
-                expect(wnibiBal).toBeLessThan(parseUnits("0.5", 18))
+                expect(wnibiBal < parseUnits("0.5", 18)).toBe(true)
+                const walletBalWeiAfter = await provider.getBalance(account2.address)
+                expect(walletBalWeiAfter).toEqual(walletBalWei)
             }

             const walletBalWei = await provider.getBalance(account3.address)
@@
             {
                 const amountToSend = parseUnits("0.5", 18)
                 let tx = await usdc.connect(account3).transfer(account, amountToSend)
                 await tx.wait()
                 const usdcBal = await usdc.balanceOf(account3.address)
-                expect(usdcBal).toBeLessThan(parseUnits("0.5", 18))
+                expect(usdcBal < parseUnits("0.5", 18)).toBe(true)
+                const walletBalWeiAfter = await provider.getBalance(account3.address)
+                expect(walletBalWeiAfter).toEqual(walletBalWei)
             }

Also applies to: 32-39, 50-58, 69-74

---

11-13: Drop unused deployments/logging to cut test time and noise.

USDC isn’t used in the WNIBI test, and WNIBI isn’t used in the USDC test. Also remove the debug log.

-            const { contract: usdc } = await deployContractUSDCIfNeeded("0x869EAa3b34B51D631FB0B6B1f9586ab658C2D25F")
-            console.log("account2:", account2.address)
+            // USDC not needed in this test

-            const { contract: wnibi } = await deployContractWNIBIIfNeeded("0xF8Da4a4A57e4aFBdeA4c541DCa626a47Ed874729")
+            // WNIBI not needed in this test

Also applies to: 47-49

---

59-65: Comment mismatch (“Mint” vs “Transfer”).

Update the comment to reflect the actual operation to avoid confusion.

-            // Mint some USDC to account3 to use as gas token
+            // Transfer some USDC to account3 to use as gas token

---

24-30: Strengthen assertions: verify recipient received the amount.

Add a recipient balance check to ensure the transfer succeeded while gas was paid in tokens.

             // Send some WNIBI to account2 to use as gas token
             {
+                const recvBefore = await wnibi.balanceOf(account2.address)
                 let tx = await wnibi.transfer(account2, parseUnits("1", 18))
                 await tx.wait()
                 const wnibiBal = await wnibi.balanceOf(account2.address)
                 expect(wnibiBal).toEqual(parseUnits("1", 18))
+                expect(wnibiBal - recvBefore).toEqual(parseUnits("1", 18))
             }

             // Transfer some USDC to account3 to use as gas token
             {
+                const recvBefore = await usdc.balanceOf(account3.address)
                 let tx = await usdc.transfer(account3, parseUnits("1", 18))
                 await tx.wait()
                 const usdcBal = await usdc.balanceOf(account3.address)
                 expect(usdcBal).toEqual(parseUnits("1", 18))
+                expect(usdcBal - recvBefore).toEqual(parseUnits("1", 18))
             }

Also applies to: 59-65

---

59-65: Confirm token funding path won’t revert.

If the deployed USDC/WNIBI contracts don’t mint initial supply to account, transfer will revert. Prefer a guarded mint/deposit path when available.

Example (optional):

if ("mint" in usdc) {
  // @ts-ignore
  await (usdc as any).mint(account3.address, parseUnits("1", 18))
} else {
  await usdc.transfer(account3, parseUnits("1", 18))
}

evm-e2e/test/setup.ts (2)

10-10: Guard missing RPC URL for clearer failures.

Fail fast with a helpful message if JSON_RPC_ENDPOINT is unset.

-const provider = new ethers.JsonRpcProvider(process.env.JSON_RPC_ENDPOINT)
+const rpcUrl = process.env.JSON_RPC_ENDPOINT
+if (!rpcUrl) {
+  throw new Error("JSON_RPC_ENDPOINT is not set")
+}
+const provider = new ethers.JsonRpcProvider(rpcUrl)

---

17-17: Nit: spacing in export list.

Minor readability tweak.

-export { account, account2, account3,provider, TEST_TIMEOUT, TX_WAIT_TIMEOUT }
+export { account, account2, account3, provider, TEST_TIMEOUT, TX_WAIT_TIMEOUT }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df7d6ea and d01aac8.

📒 Files selected for processing (3)

• contrib/scripts/localnet-uniswap.sh (1 hunks)
• evm-e2e/test/gastoken.test.ts (1 hunks)
• evm-e2e/test/setup.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

evm-e2e/test/gastoken.test.ts (2)

evm-e2e/test/utils.ts (2)

• deployContractWNIBIIfNeeded (109-137)
• deployContractUSDCIfNeeded (139-167)

evm-e2e/test/setup.ts (5)

• account2 (17-17)
• provider (17-17)
• account (17-17)
• TEST_TIMEOUT (17-17)
• account3 (17-17)

🪛 Shellcheck (0.10.0)

contrib/scripts/localnet-uniswap.sh

[error] 26-26: This expression is constant. Did you forget a $ somewhere?

(SC2078)

---

[warning] 75-75: FLAG_SPOT appears unused. Verify use (or export if used externally).

(SC2034)

---

[warning] 109-109: Quotes/backslashes will be treated literally. Use an array.

(SC2089)

---

[warning] 154-154: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 158-158: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 163-163: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 166-166: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 169-169: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 173-173: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 177-177: Quotes/backslashes in this variable will not be respected.

(SC2090)

⏰ 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: unit-tests
• GitHub Check: build
• GitHub Check: e2e-evm
• GitHub Check: integration-tests

🔇 Additional comments (1)

contrib/scripts/localnet-uniswap.sh (1)

7-9: Confirm MNEMONIC/COINS are non-sensitive and local-only.

Avoid accidental reuse in non-dev environments.

Would you like me to gate execution behind a “--i-know-what-im-doing” flag when CHAIN_ID doesn’t match a known localnet pattern?

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

.github/workflows/e2e-evm.yml (1)

75-77: Switch to localnet-uniswap is correct; add a readiness wait to deflake CI.

Starting the chain in background + fixed sleep is flaky. Poll JSON-RPC until ready before running tests.

Example step body:

run: |
  just localnet-uniswap --no-build &
  for i in {1..120}; do
    if curl -sf http://127.0.0.1:8545 >/dev/null; then
      echo "JSON-RPC is up"; break
    fi
    sleep 1
  done

justfile (1)

72-74: LGTM; consider a -fast convenience alias.

Mirror localnet-fast to avoid repeating flags in CI/local use.

localnet-uniswap-fast:
  make localnet-uniswap FLAGS="--no-build"

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d01aac8 and 27301df.

📒 Files selected for processing (3)

• .github/workflows/e2e-evm.yml (1 hunks)
• contrib/make/localnet.mk (1 hunks)
• justfile (1 hunks)

🧰 Additional context used

🪛 GitHub Actions: EVM E2E tests

contrib/make/localnet.mk

[error] 11-11: GNU Make: missing separator on line 11 in contrib/make/localnet.mk. This syntax error caused the 'make install' target to fail (exit code 2).

🪛 GitHub Actions: Integration tests

contrib/make/localnet.mk

[error] 11-11: Makefile syntax error: missing separator at line 11.

⏰ 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). (2)

• GitHub Check: proto-gen-rs
• GitHub Check: lint

🔇 Additional comments (2)

.github/workflows/e2e-evm.yml (1)

68-69: Verify Node version spec.

"lts/jod" looks invalid for actions/setup-node@v4. Prefer "lts/*" or a concrete version (e.g., "22.x").

-          node-version: "lts/jod"
+          node-version: "lts/*"

contrib/make/localnet.mk (1)

6-6: PHONY target update looks good.

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

contrib/make/localnet.mk (1)

10-11: Recipe indentation fixed (TAB) — resolves “missing separator”

The command is now TAB-indented; this should clear the CI Make error noted previously.

🧹 Nitpick comments (9)

evm-e2e/test/setup.ts (2)

7-7: Fail fast if MNEMONIC is missing (avoid non-null crash).

Guard the env read so failures are clear in CI/local.

-const mnemonic = Mnemonic.fromPhrase(process.env.MNEMONIC!)
+const MNEMONIC = process.env.MNEMONIC
+if (!MNEMONIC) {
+  throw new Error("Missing MNEMONIC in env (see evm-e2e/.env.example)")
+}
+const mnemonic = Mnemonic.fromPhrase(MNEMONIC)

---

9-9: Validate JSON_RPC_ENDPOINT before constructing the provider.

Prevents opaque provider errors when the URL is unset.

-const provider = new ethers.JsonRpcProvider(process.env.JSON_RPC_ENDPOINT)
+const RPC_URL = process.env.JSON_RPC_ENDPOINT
+if (!RPC_URL) {
+  throw new Error("Missing JSON_RPC_ENDPOINT in env")
+}
+const provider = new ethers.JsonRpcProvider(RPC_URL)

evm-e2e/test/gastoken.test.ts (7)

26-29: Pass explicit address strings to ERC-20 transfer for clarity.

Signer objects are Addressable in ethers v6, but using .address is clearer and avoids surprises.

-                let tx = await wnibi.transfer(account2, parseUnits("1", 18))
+                let tx = await wnibi.transfer(account2.address, parseUnits("1", 18))

---

35-36: Same here: use .address for the recipient.

Keeps param types consistent across the file.

-                let tx = await wnibi.connect(account2).transfer(account, amountToSend)
+                let tx = await wnibi.connect(account2).transfer(account.address, amountToSend)

---

47-48: Avoid redeploying in each test; deploy once in beforeAll.

Cuts test time and flakiness. Example pattern:

let wnibi: any, usdc: any
beforeAll(async () => {
  ;({ contract: wnibi } = await deployContractWNIBIIfNeeded("0xF8Da4a4A57e4aFBdeA4c541DCa626a47Ed874729"))
  ;({ contract: usdc } = await deployContractUSDCIfNeeded("0x869EAa3b34B51D631FB0B6B1f9586ab658C2D25F"))
})

Then remove the per-test deployments on Lines 11-12 and 47-48 and use the shared refs.

---

59-65: Comment nit: it’s a transfer, not a mint.

Update the comment to match the code.

-            // Mint some USDC to account3 to use as gas token
+            // Transfer some USDC to account3 to use as gas token

---

32-39: Assert ETH stays zero to prove gas was paid in token.

Strengthens the gastoken signal beyond “< 0.5”.

                 const wnibiBal = await wnibi.balanceOf(account2.address)
                 expect(wnibiBal < parseUnits("0.5", 18)).toBe(true)
+                const walletBalWeiAfter = await provider.getBalance(account2.address)
+                expect(walletBalWeiAfter).toEqual(0n)

---

67-74: Mirror ETH-stays-zero assertion for the USDC case.

Confirms no native ETH was needed.

                 const usdcBal = await usdc.balanceOf(account3.address)
                 expect(usdcBal < parseUnits("0.5", 18)).toBe(true)
+                const walletBalWeiAfter = await provider.getBalance(account3.address)
+                expect(walletBalWeiAfter).toEqual(0n)

---

11-13: Minor: drop the extra deployment/logging in the WNIBI test.

USDC isn’t needed for this test, and console.log adds noise to CI output. Consider removing or gating behind a DEBUG flag.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 27301df and 30f8ed2.

📒 Files selected for processing (3)

• contrib/make/localnet.mk (1 hunks)
• evm-e2e/test/gastoken.test.ts (1 hunks)
• evm-e2e/test/setup.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 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:

• evm-e2e/test/gastoken.test.ts

🧬 Code graph analysis (1)

evm-e2e/test/gastoken.test.ts (2)

dex-deploy/scripts/2.create-pool.js (2)

• wnibi (62-62)
• usdc (63-63)

evm-e2e/test/utils.ts (2)

• deployContractWNIBIIfNeeded (109-137)
• deployContractUSDCIfNeeded (139-167)

⏰ 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). (5)

• GitHub Check: build
• GitHub Check: unit-tests
• GitHub Check: e2e-evm
• GitHub Check: integration-tests
• GitHub Check: lint

🔇 Additional comments (3)

contrib/make/localnet.mk (1)

6-6: .PHONY updated correctly

Adding localnet-uniswap to .PHONY is correct and avoids filename-target collisions.

evm-e2e/test/setup.ts (1)

10-12: Deriving multiple accounts from a single mnemonic: LGTM.

Cleanly replaces the prior MNEMONIC2 approach and resolves earlier comments about the unused secondary mnemonic.

evm-e2e/test/gastoken.test.ts (1)

38-38: BigInt comparison matcher: good fix.

Using a boolean comparison avoids Jest number coercion issues.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

contrib/scripts/localnet-uniswap.sh (1)

1-262: Fix shellcheck errors in contrib/scripts/localnet-uniswap.sh

• Line 26: replace the constant test if [ console_log_text_color ] with a direct function invocation and exit‐status check, e.g.
  if console_log_text_color; then …
• Line 75: remove or export/use the unused FLAG_SPOT variable.
• Line 109: don’t assign SEDOPTION="''" literally—define an array for in-place flags, e.g.
  readonly SED_INPLACE=(-i) on Linux and (-i '') on macOS.
• Lines 154–177: invoke sed with the array and quote the target path, e.g.
  sed "${SED_INPLACE[@]}" '/\[api\]/,+3 s/enable = false/enable = true/' "$CHAIN_DIR/config/app.toml"
• Line 191 & lines 208–210, 239–240: double-quote all variable expansions ($val_address, paths under $CHAIN_DIR) in add-genesis-account, cat, mv, jq invocations to prevent globbing/word splitting.

♻️ Duplicate comments (9)

contrib/scripts/localnet-uniswap.sh (9)

26-26: ShellCheck SC2078: constant test.

Fixed by the color-init refactor above; please adopt it.

---

154-177: ShellCheck SC2090: sed variable not respected.

Resolved by switching to SED_INPLACE array in the proposed diff.

---

11-13: Fail fast if required binary is missing.

Avoids destructive ops when nibid isn't installed.

 echo "CHAIN_DIR: $CHAIN_DIR"
 echo "CHAIN_ID: $CHAIN_ID"
 
+if ! command -v "$BINARY" >/dev/null 2>&1; then
+  >&2 echo "Error: Binary '$BINARY' not found in PATH. Build or install it first."
+  exit 1
+fi

---

18-31: Color init is broken; conditional always true and apt-get is inappropriate here.

The [ console_log_text_color ] test is constant; colors never initialize. Also don’t install packages from this script.

-# Console log text colour
-console_log_text_color() {
-  red=$(tput setaf 9)
-  green=$(tput setaf 10)
-  blue=$(tput setaf 12)
-  reset=$(tput sgr0)
-}
-
-if [ console_log_text_color ]; then
-  echo "succesfully toggled console coloring"
-else
-  # For Ubuntu and Debian. MacOS has tput by default.
-  apt-get install libncurses5-dbg -y
-fi
+# Console log text colour
+console_log_text_color() {
+  if command -v tput >/dev/null 2>&1 && [ "$(tput colors 2>/dev/null || echo 0)" -ge 8 ]; then
+    red="$(tput setaf 1)"; green="$(tput setaf 2)"; blue="$(tput setaf 4)"; reset="$(tput sgr0)"
+    return 0
+  else
+    red=""; green=""; blue=""; reset=""
+    return 1
+  fi
+}
+
+if console_log_text_color; then
+  echo "successfully enabled console coloring"
+else
+  echo "tput not available or terminal lacks color support; continuing without colors"
+fi

---

107-111: Make sed -i portable (macOS) and quote paths.

Use an array for in-place args to satisfy BSD sed and ShellCheck; quote app.toml path.

-SEDOPTION=""
-if [[ "$OSTYPE" == "darwin"* ]]; then
-  SEDOPTION="''"
-fi
+if [[ "$OSTYPE" == "darwin"* ]]; then
+  SED_INPLACE=(-i '')
+else
+  SED_INPLACE=(-i)
+fi
@@
-sed -i $SEDOPTION '/\[api\]/,+3 s/enable = false/enable = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[api\]/,+3 s/enable = false/enable = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[grpc\]/,+3 s/enable = false/enable = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[grpc\]/,+3 s/enable = false/enable = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[json\-rpc\]/,+3 s/enable = false/enable = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[json\-rpc\]/,+3 s/enable = false/enable = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[json\-rpc\]/,+13 s/api = "eth,net,web3"/api = "eth,net,web3,debug"/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[json\-rpc\]/,+13 s/api = "eth,net,web3"/api = "eth,net,web3,debug"/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION '/\[json\-rpc\]/,+51 s/enable-indexer = false/enable-indexer = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" '/\[json\-rpc\]/,+51 s/enable-indexer = false/enable-indexer = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION 's/swagger = false/swagger = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" 's/swagger = false/swagger = true/' "$CHAIN_DIR/config/app.toml"
@@
-sed -i $SEDOPTION 's/enabled-unsafe-cors = false/enabled-unsafe-cors = true/' $CHAIN_DIR/config/app.toml
+sed "${SED_INPLACE[@]}" 's/enabled-unsafe-cors = false/enabled-unsafe-cors = true/' "$CHAIN_DIR/config/app.toml"

Also applies to: 154-177

---

116-121: Process stop: use $BINARY consistently and don’t abort on failure.

killall hardcodes nibid and may fail under set -e.

 if pgrep -x "$BINARY" >/dev/null; then
   echo_error "Terminating $BINARY..."
-  killall nibid
+  pkill -x "$BINARY" || true
 fi

---

134-141: Init should target CHAIN_DIR and quote args.

-echo_info "Initializing $CHAIN_ID..."
-if $BINARY init $CHAIN_ID --chain-id $CHAIN_ID --overwrite; then
+echo_info "Initializing $CHAIN_ID..."
+if "$BINARY" init "$CHAIN_ID" --chain-id "$CHAIN_ID" --overwrite --home "$CHAIN_DIR"; then
   echo_success "Successfully initialized $CHAIN_ID"
 else
   echo_error "Failed to initialize $CHAIN_ID"
   exit 1
 fi

---

143-151: Config calls should use explicit --home and quotes.

-echo_info "Updating nibid config..."
-$BINARY config keyring-backend test
-$BINARY config chain-id $CHAIN_ID
-$BINARY config broadcast-mode sync
-$BINARY config output json
-$BINARY config node "http://localhost:26657"
-$BINARY config # Prints config.
+echo_info "Updating nibid config..."
+"$BINARY" config keyring-backend test --home "$CHAIN_DIR"
+"$BINARY" config chain-id "$CHAIN_ID" --home "$CHAIN_DIR"
+"$BINARY" config broadcast-mode sync --home "$CHAIN_DIR"
+"$BINARY" config output json --home "$CHAIN_DIR"
+"$BINARY" config node "http://localhost:26657" --home "$CHAIN_DIR"
+"$BINARY" config --home "$CHAIN_DIR" # Prints config.

---

183-189: Keys ops should be idempotent, scoped to CHAIN_DIR, and use keyring-backend.

-if ! $BINARY keys show $val_key_name; then 
-  echo "$MNEMONIC" | $BINARY keys add $val_key_name --recover
+if ! "$BINARY" keys show "$val_key_name" --home "$CHAIN_DIR" --keyring-backend test >/dev/null 2>&1; then
+  echo "$MNEMONIC" | "$BINARY" keys add "$val_key_name" --recover --home "$CHAIN_DIR" --keyring-backend test
   echo_success "Successfully added key: $val_key_name"
 fi
 
-val_address=$($BINARY keys show $val_key_name -a)
+val_address=$("$BINARY" keys show "$val_key_name" -a --home "$CHAIN_DIR" --keyring-backend test)

🧹 Nitpick comments (7)

contrib/scripts/localnet-uniswap.sh (7)

2-2: Harden shell options (safer defaults).

Prefer strict mode to catch subtle failures early.

-set -e
+set -Eeuo pipefail
+IFS=$'\n\t'

---

33-49: Reduce extra newlines in log helpers.

Print the message in one line with colors; current implementation outputs three lines.

 echo_info() {
-  echo "${blue}"
-  echo "$1"
-  echo "${reset}"
+  printf '%b%s%b\n' "${blue}" "$1" "${reset}"
 }
 
 echo_error() {
-  echo "${red}"
-  echo "$1"
-  echo "${reset}"
+  printf '%b%s%b\n' "${red}" "$1" "${reset}"
 }
 
 echo_success() {
-  echo "${green}"
-  echo "$1"
-  echo "${reset}"
+  printf '%b%s%b\n' "${green}" "$1" "${reset}"
 }

---

72-79: Unused feature flag (FLAG_SPOT).

Either wire it into behavior or remove to avoid dead code.

Would you like me to thread this flag through to toggle spot/DEX configs in genesis?

Also applies to: 80-97

---

191-195: Quote variables for genesis-account adds.

Prevents word-splitting and weird expansions.

-$BINARY add-genesis-account $val_address $GENESIS_COINS --home "$CHAIN_DIR"
+"$BINARY" add-genesis-account "$val_address" "$GENESIS_COINS" --home "$CHAIN_DIR"
 # EVM encoded nibi address for the same account
-$BINARY add-genesis-account nibi1cr6tg4cjvux00pj6zjqkh6d0jzg7mksaywxyl3 $GENESIS_COINS --home "$CHAIN_DIR"
-$BINARY add-genesis-account nibi1ltez0kkshywzm675rkh8rj2eaf8et78cqjqrhc $GENESIS_COINS --home "$CHAIN_DIR"
+"$BINARY" add-genesis-account "nibi1cr6tg4cjvux00pj6zjqkh6d0jzg7mksaywxyl3" "$GENESIS_COINS" --home "$CHAIN_DIR"
+"$BINARY" add-genesis-account "nibi1ltez0kkshywzm675rkh8rj2eaf8et78cqjqrhc" "$GENESIS_COINS" --home "$CHAIN_DIR"

---

239-240: Quote jq inputs/outputs.

Protects against spaces in CHAIN_DIR.

-jq --slurpfile evm contrib/evm.json '.app_state.evm = $evm[0].evm | .app_state.auth = $evm[0].auth | .app_state.gastoken = $evm[0].gastoken |.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
+jq --slurpfile evm contrib/evm.json '.app_state.evm = $evm[0].evm | .app_state.auth = $evm[0].auth | .app_state.gastoken = $evm[0].gastoken | .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"

---

260-261: Optional: background start with logs and health wait.

Consider redirecting logs and waiting for RPC to be ready to improve DX.

I can add a wait-for-http on 26657 and tail logs if helpful.

---

75-75: ShellCheck SC2034: FLAG_SPOT appears unused.

Either remove or apply it.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 30f8ed2 and 311f996.

📒 Files selected for processing (1)

• contrib/scripts/localnet-uniswap.sh (1 hunks)

🧰 Additional context used

🪛 Shellcheck (0.10.0)

contrib/scripts/localnet-uniswap.sh

[error] 26-26: This expression is constant. Did you forget a $ somewhere?

(SC2078)

---

[warning] 75-75: FLAG_SPOT appears unused. Verify use (or export if used externally).

(SC2034)

---

[warning] 109-109: Quotes/backslashes will be treated literally. Use an array.

(SC2089)

---

[warning] 154-154: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 158-158: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 163-163: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 166-166: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 169-169: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 173-173: Quotes/backslashes in this variable will not be respected.

(SC2090)

---

[warning] 177-177: Quotes/backslashes in this variable will not be respected.

(SC2090)

⏰ 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: unit-tests
• GitHub Check: build
• GitHub Check: proto-gen-rs
• GitHub Check: e2e-evm
• GitHub Check: lint