diff --git a/docs/evm-integration/precompiles/wasm-precompile.md b/docs/evm-integration/precompiles/wasm-precompile.md index 5ac86938..6b2a71df 100644 --- a/docs/evm-integration/precompiles/wasm-precompile.md +++ b/docs/evm-integration/precompiles/wasm-precompile.md @@ -269,6 +269,29 @@ This matters because `eth_call` requires a `from` address. Using the wasm contra ## Cross-Cutting Concerns +### Balance Accounting (EVM -> CosmWasm direction) + +The wasm precompile installs **no** balance handler. Bank balance events +emitted by a CosmWasm contract are native x/bank movements of the contract's +own funds; the bank keeper is the single source of truth for them. + +They must never be mirrored into the EVM StateDB: + +1. The upstream `BalanceHandler` maps event addresses into EVM accounts via + `common.BytesToAddress`, silently truncating the 32-byte wasm contract + address. The truncated alias has no EVM balance, so `SubBalance` wraps + uint256 (x/vm/statedb has no insufficient-balance guard) and the + `SetBalance` reconciliation at commit mints ~2^256 extended-denom units + into the alias account. +2. The matching `coin_received` replay credits the recipient in the StateDB + journal on top of the native bank credit; reconciliation then mints the + amount a second time. + +Filtering only non-20-byte addresses is insufficient (case 2 persists for +20-byte recipients), so `NewPrecompile` wires +`BalanceHandlerFactory: NewBalanceHandlerFactory()`, which returns nil. +Regression tests: `precompiles/wasm/balance_test.go`. + ### Sender Identity **Cross-runtime calls always execute as the calling contract, not the outer user (tx.origin).** diff --git a/docs/evm-integration/testing/tests/integration-precompiles.md b/docs/evm-integration/testing/tests/integration-precompiles.md index b5309cf4..0d531991 100644 --- a/docs/evm-integration/testing/tests/integration-precompiles.md +++ b/docs/evm-integration/testing/tests/integration-precompiles.md @@ -55,6 +55,15 @@ Suite: `tests/integration/evm/precompiles/suite_test.go` | `WasmPrecompileExecuteFailsWithBadMessage` | Verifies unrecognized execute message causes receipt status=0x0. | | `WasmPrecompileExecuteRejectedInEthCall` | Verifies the state-changing wasm `execute` entrypoint is rejected when invoked via read-only `eth_call`. | | `WasmPrecompileQueryInvalidContract` | Verifies querying a non-existent bech32 contract returns a JSON-RPC error. | + +## Wasm Precompile Balance Accounting (unit) + +Suite: `precompiles/wasm/balance_test.go` + +| Test | Description | +| --- | --- | +| `TestWasmBalanceReplayDoesNotMintOrDoubleApply` | Regression for the EVM→Wasm accounting defect: a 32-byte wasm contract sends N to a 20-byte recipient. Asserts the precompile installs NO balance handler, the truncated EVM alias stays zero, and the recipient's StateDB balance is not double-applied. Includes a defect-demonstration subtest pinning the vulnerable behavior of the pre-fix wiring (alias wraps to ~2^256, recipient double-credited). | +| `TestWasmBalanceReplayKeepsEVMAccountBehavior` | Guards the legitimate upstream balance-handler path used by the other (unchanged) EVM precompiles for 20-byte EVM accounts. | | `WasmPrecompileContractInfoNotFound` | Verifies `contractInfo` for a non-existent contract returns a JSON-RPC error. | | `WasmPrecompileInvalidBech32Fails` | Verifies invalid bech32 address causes tx revert (status=0x0). | diff --git a/precompiles/wasm/balance.go b/precompiles/wasm/balance.go new file mode 100644 index 00000000..a55417e4 --- /dev/null +++ b/precompiles/wasm/balance.go @@ -0,0 +1,39 @@ +package wasm + +import ( + cmn "github.com/cosmos/evm/precompiles/common" +) + +// NewBalanceHandlerFactory returns the balance handler factory for the wasm +// precompile. It intentionally returns nil: the wasm precompile must install +// NO balance handler. +// +// Invariant: the EVM StateDB must never mirror bank balance events emitted +// by a CosmWasm contract execution. Every balance event a wasm contract can +// produce is a native bank movement of its own funds (dispatched from the +// 32-byte contract address); the native bank keeper is the single source of +// truth for those movements. +// +// Replaying them is doubly broken (see the regression tests in +// balance_test.go): +// +// 1. The upstream BalanceHandler maps event addresses into EVM accounts via +// common.BytesToAddress, silently truncating the 32-byte wasm contract +// address. The truncated alias has no EVM balance, so SubBalance wraps +// uint256 (x/vm/statedb has no insufficient-balance guard), and the +// SetBalance reconciliation at commit mints ~2^256 extended-denom units +// into the alias account. +// 2. The matching coin_received replay credits the recipient in the StateDB +// journal on top of the native bank credit; SetBalance reconciles the +// journal target against a bank view that does not include the in-flight +// native transfer and mints the amount a second time (observed: fixture +// sent 111, recipient got 222, plus a giant alias mint). +// +// Filtering only non-20-byte addresses is NOT sufficient (case 2 persists), +// so the correct minimal fix is to not mirror at all. This is the single +// production wiring point for the invariant: NewPrecompile calls this +// helper, and the regression tests build the replay through it, so +// re-installing a handler fails the tests. +func NewBalanceHandlerFactory() *cmn.BalanceHandlerFactory { + return nil +} diff --git a/precompiles/wasm/balance_test.go b/precompiles/wasm/balance_test.go new file mode 100644 index 00000000..3deea576 --- /dev/null +++ b/precompiles/wasm/balance_test.go @@ -0,0 +1,190 @@ +package wasm_test + +import ( + "testing" + + sdkmath "cosmossdk.io/math" + storetypes "cosmossdk.io/store/types" + sdktestutil "github.com/cosmos/cosmos-sdk/testutil" + sdk "github.com/cosmos/cosmos-sdk/types" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + + cmn "github.com/cosmos/evm/precompiles/common" + cmnmocks "github.com/cosmos/evm/precompiles/common/mocks" + "github.com/cosmos/evm/x/vm/statedb" + evmtypes "github.com/cosmos/evm/x/vm/types" + evmmocks "github.com/cosmos/evm/x/vm/types/mocks" + ethcommon "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/tracing" + "github.com/holiman/uint256" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + wasmprecompile "github.com/LumeraProtocol/lumera/precompiles/wasm" +) + +// newMockBankKeeper returns a mock cmn.BankKeeper whose blocked set is empty. +// Only BlockedAddr is exercised by the BalanceHandler; any other call fails +// the test, so the harness cannot silently grow dependencies on real bank +// state. +func newMockBankKeeper(t *testing.T) *cmnmocks.BankKeeper { + t.Helper() + bk := cmnmocks.NewBankKeeper(t) + bk.Mock.On("BlockedAddr", mock.AnythingOfType("types.AccAddress")).Return(false).Maybe() + return bk +} + +// wasmContractAddr is a realistic 32-byte CosmWasm contract address. +// evmRecipientAddr is a normal 20-byte EVM-projectable account. +// Lengths are asserted in init so a typo in a literal cannot silently change +// which branch of the replay the test exercises. +var ( + wasmContractAddr = sdk.AccAddress([]byte("wasm-contract-address-32-bytes!!")) // 32 bytes + evmRecipientAddr = sdk.AccAddress([]byte("evm-recipient-20byte")) // 20 bytes +) + +func init() { + if len(wasmContractAddr) != 32 { + panic("wasmContractAddr must be 32 bytes") + } + if len(evmRecipientAddr) != 20 { + panic("evmRecipientAddr must be 20 bytes") + } + + // sdk.Config bech32 prefix setup mirrors app config; use lumera prefix. + cfg := sdk.GetConfig() + cfg.SetBech32PrefixForAccount("lumera", "lumerapub") + // Bank balance events carry the display (6-decimal) denom; the balance + // handler parses them with the EVM coin denom. Match the app wiring: + // EVM coin denom is ulume (6 decimals) with alume as the extended denom. + evmtypes.SetDefaultEvmCoinInfo(evmtypes.EvmCoinInfo{ + Denom: "ulume", + ExtendedDenom: "alume", + DisplayDenom: "lume", + Decimals: evmtypes.SixDecimals.Uint32(), + }) +} + +// replayBankEvents drives the exact replay loop used by +// cmn.Precompile.runNativeAction for a precompile whose balance handler +// factory is `factory`: BeforeBalanceChange, emit events, AfterBalanceChange. +// It returns the resulting StateDB so tests can assert balances. +func replayBankEvents(t *testing.T, factory *cmn.BalanceHandlerFactory, events sdk.Events) *statedb.StateDB { + t.Helper() + storeKey := storetypes.NewKVStoreKey("test") + tKey := storetypes.NewTransientStoreKey("test_t") + ctx := sdktestutil.DefaultContext(storeKey, tKey) + stateDB := statedb.New(ctx, evmmocks.NewEVMKeeper(), statedb.NewEmptyTxConfig()) + + var bh *cmn.BalanceHandler + if factory != nil { + bh = factory.NewBalanceHandler() + } + if bh != nil { + bh.BeforeBalanceChange(ctx) + } + ctx.EventManager().EmitEvents(events) + if bh != nil { + require.NoError(t, bh.AfterBalanceChange(ctx, stateDB)) + } + return stateDB +} + +func wasmPayoutEvents() sdk.Events { + // Real production event pair emitted by x/bank when a wasm contract with a + // 32-byte address sends funds to a 20-byte EVM recipient. + amount := sdk.NewCoins(sdk.NewCoin("ulume", sdkmath.NewInt(111))) + return sdk.Events{ + banktypes.NewCoinSpentEvent(wasmContractAddr, amount), + banktypes.NewCoinReceivedEvent(evmRecipientAddr, amount), + } +} + +// TestWasmBalanceReplayDoesNotMintOrDoubleApply is the regression test for the +// EVM→Wasm accounting defect: a wasm contract (32-byte address) sends N to a +// 20-byte EVM account. The native bank transfer is the single source of +// truth; the precompile must install NO balance handler, so the StateDB must +// not: +// - mint into the truncated EVM alias of the wasm contract, nor +// - double-apply the transfer by adding N to the recipient's EVM balance +// (the bank keeper already moved the funds, and SetBalance reconciles the +// journal target against a bank view that does not include the in-flight +// native transfer, minting the replayed amount a second time). +// +// The assertions build the replay through the PRODUCTION wiring helper +// (wasmprecompile.NewBalanceHandlerFactory), so restoring a balance handler +// in production wiring fails these tests. +func TestWasmBalanceReplayDoesNotMintOrDoubleApply(t *testing.T) { + events := wasmPayoutEvents() + + t.Run("production wiring installs no balance handler", func(t *testing.T) { + require.Nil(t, wasmprecompile.NewBalanceHandlerFactory(), + "wasm precompile must not mirror native bank events into the EVM StateDB") + }) + + t.Run("does not mutate the truncated alias of a 32-byte wasm address", func(t *testing.T) { + stateDB := replayBankEvents(t, wasmprecompile.NewBalanceHandlerFactory(), events) + truncated := ethcommon.BytesToAddress(wasmContractAddr.Bytes()) + require.Equal(t, "0", stateDB.GetBalance(truncated).String(), + "replay must not mutate the truncated alias of a 32-byte wasm contract address") + }) + + t.Run("does not double-apply the recipient credit", func(t *testing.T) { + stateDB := replayBankEvents(t, wasmprecompile.NewBalanceHandlerFactory(), events) + require.Equal(t, "0", stateDB.GetBalance(ethcommon.BytesToAddress(evmRecipientAddr.Bytes())).String(), + "replay must not double-apply the native bank transfer to the recipient") + }) + + t.Run("unfiltered upstream replay wraps the truncated alias and double-mints (defect demonstration)", func(t *testing.T) { + // Pins the vulnerable behavior of the exact pre-fix production wiring: + // upstream BalanceHandlerFactory over an unfiltered bank keeper. This + // keeps the RED evidence in-repo and guards the assertions above from + // passing vacuously (e.g. if events stopped being parsed). + stateDB := replayBankEvents(t, cmn.NewBalanceHandlerFactory(newMockBankKeeper(t)), events) + + truncated := ethcommon.BytesToAddress(wasmContractAddr.Bytes()) + aliasBal := stateDB.GetBalance(truncated).ToBig() + require.True(t, aliasBal.Sign() > 0, + "unfiltered replay is expected to mint into the truncated alias (this is the defect)") + require.True(t, aliasBal.BitLen() > 200, + "wrapped alias balance must be astronomically large, got %s", aliasBal) + + recipientBal := stateDB.GetBalance(ethcommon.BytesToAddress(evmRecipientAddr.Bytes())).ToBig() + require.Equal(t, "111000000000000", recipientBal.String(), + "unfiltered replay adds the amount on top of the native bank transfer (second mint at reconciliation)") + }) +} + +// TestWasmBalanceReplayKeepsEVMAccountBehavior documents that legitimate +// EVM-native precompiles (staking, distribution, ...) keep their upstream +// balance handler; this test builds the upstream handler directly, matching +// those precompiles' wiring (they are unchanged by this fix). +func TestWasmBalanceReplayKeepsEVMAccountBehavior(t *testing.T) { + spender := sdk.AccAddress([]byte("evm-spender-20-byte!")) + receiver := sdk.AccAddress([]byte("evm-receiver-20-byte")) + if len(spender) != 20 || len(receiver) != 20 { + t.Fatalf("test literals must be exactly 20 bytes, got %d and %d", len(spender), len(receiver)) + } + + storeKey := storetypes.NewKVStoreKey("test") + tKey := storetypes.NewTransientStoreKey("test_t") + ctx := sdktestutil.DefaultContext(storeKey, tKey) + stateDB := statedb.New(ctx, evmmocks.NewEVMKeeper(), statedb.NewEmptyTxConfig()) + bh := cmn.NewBalanceHandlerFactory(newMockBankKeeper(t)).NewBalanceHandler() + bh.BeforeBalanceChange(ctx) + + amount := sdk.NewCoins(sdk.NewCoin("ulume", sdkmath.NewInt(5))) + // 5 ulume == 5e12 at 18 decimals; seed the spender above that so the + // legitimate path does not depend on unsigned wrap behavior. + seed := new(uint256.Int).Mul(uint256.NewInt(10), uint256.NewInt(1_000_000_000_000)) + stateDB.AddBalance(ethcommon.BytesToAddress(spender.Bytes()), seed, tracing.BalanceChangeUnspecified) + + ctx.EventManager().EmitEvents(sdk.Events{ + banktypes.NewCoinSpentEvent(spender, amount), + banktypes.NewCoinReceivedEvent(receiver, amount), + }) + + require.NoError(t, bh.AfterBalanceChange(ctx, stateDB)) + require.Equal(t, "5000000000000", stateDB.GetBalance(ethcommon.BytesToAddress(spender.Bytes())).String()) + require.Equal(t, "5000000000000", stateDB.GetBalance(ethcommon.BytesToAddress(receiver.Bytes())).String()) +} diff --git a/precompiles/wasm/wasm.go b/precompiles/wasm/wasm.go index 12d907e5..1fa3767e 100644 --- a/precompiles/wasm/wasm.go +++ b/precompiles/wasm/wasm.go @@ -44,18 +44,24 @@ type Precompile struct { } // NewPrecompile creates a new CosmWasm precompile instance. +// bankKeeper is retained in the signature for call-site compatibility and +// future query-side use; it is intentionally NOT wired into a balance +// handler (see NewBalanceHandlerFactory). func NewPrecompile( wasmKeeper *wasmkeeper.Keeper, - bankKeeper cmn.BankKeeper, + _ cmn.BankKeeper, addrCdc address.Codec, ) *Precompile { permKeeper := wasmkeeper.NewDefaultPermissionKeeper(wasmKeeper) return &Precompile{ Precompile: cmn.Precompile{ - KvGasConfig: storetypes.KVGasConfig(), - TransientKVGasConfig: storetypes.TransientGasConfig(), - ContractAddress: common.HexToAddress(WasmPrecompileAddress), - BalanceHandlerFactory: cmn.NewBalanceHandlerFactory(bankKeeper), + KvGasConfig: storetypes.KVGasConfig(), + TransientKVGasConfig: storetypes.TransientGasConfig(), + ContractAddress: common.HexToAddress(WasmPrecompileAddress), + // BalanceHandlerFactory is intentionally nil: wasm bank events are + // native movements owned by x/bank and must never be mirrored into + // the EVM StateDB. See NewBalanceHandlerFactory and balance_test.go. + BalanceHandlerFactory: NewBalanceHandlerFactory(), }, ABI: ABI, wasmKeeper: wasmKeeper,