diff --git a/x/vm/keeper/call_evm.go b/x/vm/keeper/call_evm.go index c7f97b9d7..727f83a9b 100644 --- a/x/vm/keeper/call_evm.go +++ b/x/vm/keeper/call_evm.go @@ -274,6 +274,18 @@ func (k Keeper) DerivedEVMCallWithData( attrs = append(attrs, sdk.NewAttribute(types.AttributeKeyEthereumTxFailed, res.VmError)) } + // adding txData for more info in rpc methods in order to parse derived txs + attrs = append(attrs, sdk.NewAttribute(types.AttributeKeyTxData, hexutil.Encode(msg.Data()))) + // adding nonce for more info in rpc methods in order to parse derived txs + attrs = append(attrs, sdk.NewAttribute(types.AttributeKeyTxNonce, strconv.FormatUint(nonce, 10))) + attrs = append(attrs, sdk.NewAttribute(types.AttributeKeyTxGasLimit, strconv.FormatUint(gasCap, 10))) + // Build the tx_log attributes. On a reverted execution res.Logs is empty, + // so txLogAttrs ends up empty — but the tx_log event is still emitted below. + // The JSON-RPC log builder (TxLogsFromEvents) matches logs to txs by + // position: the Nth tx_log event belongs to the Nth ethereum_tx. So every + // ethereum_tx must be paired with exactly one tx_log event — an empty one on + // failure — otherwise logs get misattributed across derived txs in the same + // block. The failed tx therefore shows a status-0 receipt with no logs. txLogAttrs := make([]sdk.Attribute, len(res.Logs)) for i, log := range res.Logs { log.TxHash = ethTxHash @@ -284,11 +296,6 @@ func (k Keeper) DerivedEVMCallWithData( txLogAttrs[i] = sdk.NewAttribute(types.AttributeKeyTxLog, string(value)) } - // adding txData for more info in rpc methods in order to parse derived txs - attrs = append(attrs, sdk.NewAttribute(types.AttributeKeyTxData, hexutil.Encode(msg.Data()))) - // adding nonce for more info in rpc methods in order to parse derived txs - attrs = append(attrs, sdk.NewAttribute(types.AttributeKeyTxNonce, strconv.FormatUint(nonce, 10))) - attrs = append(attrs, sdk.NewAttribute(types.AttributeKeyTxGasLimit, strconv.FormatUint(gasCap, 10))) ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypeEthereumTx, @@ -306,14 +313,17 @@ func (k Keeper) DerivedEVMCallWithData( ), }) - logs := types.LogsToEthereum(res.Logs) - var bloomReceipt ethtypes.Bloom - if len(logs) > 0 { - bloom := k.GetBlockBloomTransient(ctx) - bloom.Or(bloom, big.NewInt(0).SetBytes(ethtypes.LogsBloom(logs))) - bloomReceipt = ethtypes.BytesToBloom(bloom.Bytes()) - k.SetBlockBloomTransient(ctx, bloomReceipt.Big()) - k.SetLogSizeTransient(ctx, (k.GetLogSizeTransient(ctx))+uint64(len(logs))) + // Only successful executions contribute to the block bloom / log size. + // res.Logs is empty on a revert, so a failed tx never touches the bloom. + if !res.Failed() { + logs := types.LogsToEthereum(res.Logs) + if len(logs) > 0 { + bloom := k.GetBlockBloomTransient(ctx) + bloom.Or(bloom, big.NewInt(0).SetBytes(ethtypes.LogsBloom(logs))) + bloomReceipt := ethtypes.BytesToBloom(bloom.Bytes()) + k.SetBlockBloomTransient(ctx, bloomReceipt.Big()) + k.SetLogSizeTransient(ctx, (k.GetLogSizeTransient(ctx))+uint64(len(logs))) + } } } diff --git a/x/vm/keeper/call_evm_test.go b/x/vm/keeper/call_evm_test.go index 21b5e4bae..40ac08070 100644 --- a/x/vm/keeper/call_evm_test.go +++ b/x/vm/keeper/call_evm_test.go @@ -6,6 +6,7 @@ import ( "github.com/ethereum/go-ethereum/common" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/evm/contracts" testconstants "github.com/cosmos/evm/testutil/constants" utiltx "github.com/cosmos/evm/testutil/tx" @@ -151,6 +152,122 @@ func (suite *KeeperTestSuite) TestCallEVMWithData() { } } +// derivedTransfer issues a single ERC20 transfer through DerivedEVMCall (commit=true) +// on the shared ctx and returns the resulting error (non-nil when the call reverts). +func (suite *KeeperTestSuite) derivedTransfer(ctx sdk.Context, from, contract, recipient common.Address) error { + erc20Contract, err := testdata.LoadERC20Contract() + suite.Require().NoError(err) + _, err = suite.network.App.EVMKeeper.DerivedEVMCall( + ctx, + erc20Contract.ABI, + from, + contract, + big.NewInt(0), // value + big.NewInt(200000), // gasLimit (explicit so a reverting call still reaches + // execution + event emission instead of failing in gas estimation) + true, // commit + false, // gasless + false, // isModuleSender + nil, // manualNonce + "transfer", + recipient, big.NewInt(100), + ) + return err +} + +// countEthTxAndLogEvents returns how many ethereum_tx and tx_log events are present. +func countEthTxAndLogEvents(events []sdk.Event) (ethTx, txLog int) { + for _, e := range events { + switch e.Type { + case evmtypes.EventTypeEthereumTx: + ethTx++ + case evmtypes.EventTypeTxLog: + txLog++ + } + } + return ethTx, txLog +} + +// TestDerivedEVMCallEthTxLogEventsStayPaired is a regression test for F-2026-17738. +// Every derived ethereum_tx event must be paired with exactly one tx_log event — +// even on failure, where the tx_log is empty. The JSON-RPC log builder matches logs +// to txs positionally, so a missing tx_log on a failed derived tx would desync logs +// across the other derived txs in the same block. +func (suite *KeeperTestSuite) TestDerivedEVMCallEthTxLogEventsStayPaired() { + suite.SetupTest() + + owner := suite.keyring.GetAddr(0) // holds the supply + broke := suite.keyring.GetAddr(1) // holds 0 tokens -> transfer reverts + recipient := utiltx.GenerateAddress() + + contractAddr := suite.DeployTestContract(suite.T(), suite.network.GetContext(), owner, big.NewInt(1_000_000)) + // Fresh event manager so only the calls below are counted (not the deploy). + ctx := suite.network.GetContext().WithEventManager(sdk.NewEventManager()) + + // Interleave success / failure / success so a dropped tx_log on the middle + // (failed) call would leave the counts unequal. + suite.Require().NoError(suite.derivedTransfer(ctx, owner, contractAddr, recipient)) + suite.Require().Error(suite.derivedTransfer(ctx, broke, contractAddr, recipient)) + suite.Require().NoError(suite.derivedTransfer(ctx, owner, contractAddr, recipient)) + + ethTx, txLog := countEthTxAndLogEvents(ctx.EventManager().Events()) + suite.Require().Equal(3, ethTx, "each derived call must emit exactly one ethereum_tx event") + suite.Require().Equal(ethTx, txLog, + "every ethereum_tx must be paired with a tx_log event (empty on failure) to preserve positional log alignment") +} + +// TestDerivedEVMCallFailedExecutionNoBloomSideEffect is a regression test for +// F-2026-17738: a reverted derived execution must not contribute to the block bloom +// or log size, while still emitting the ethereum_tx + (empty) tx_log pair. +func (suite *KeeperTestSuite) TestDerivedEVMCallFailedExecutionNoBloomSideEffect() { + suite.SetupTest() + + owner := suite.keyring.GetAddr(0) + broke := suite.keyring.GetAddr(1) // 0 tokens -> transfer reverts + recipient := utiltx.GenerateAddress() + + contractAddr := suite.DeployTestContract(suite.T(), suite.network.GetContext(), owner, big.NewInt(1_000_000)) + ctx := suite.network.GetContext().WithEventManager(sdk.NewEventManager()) + + bloomBefore := new(big.Int).Set(suite.network.App.EVMKeeper.GetBlockBloomTransient(ctx)) + logSizeBefore := suite.network.App.EVMKeeper.GetLogSizeTransient(ctx) + + // reverting transfer (broke has no tokens) + suite.Require().Error(suite.derivedTransfer(ctx, broke, contractAddr, recipient)) + + suite.Require().Equal(0, bloomBefore.Cmp(suite.network.App.EVMKeeper.GetBlockBloomTransient(ctx)), + "failed derived tx must not mutate the block bloom") + suite.Require().Equal(logSizeBefore, suite.network.App.EVMKeeper.GetLogSizeTransient(ctx), + "failed derived tx must not mutate the log size") + + ethTx, txLog := countEthTxAndLogEvents(ctx.EventManager().Events()) + suite.Require().Equal(1, ethTx, "failed derived tx still emits its ethereum_tx receipt") + suite.Require().Equal(1, txLog, "failed derived tx still emits an (empty) tx_log to preserve alignment") + + // The tx_log emitted on failure MUST carry no log attributes — otherwise the fix + // would publish phantom logs for state that was never committed. + suite.Require().Equal(0, txLogAttrCount(ctx.EventManager().Events()), + "a reverted derived tx must not emit any log attributes") + + // Sanity: a successful transfer DOES produce a non-empty tx_log (ERC20 Transfer + // event), so the empty-on-failure result above is not trivially always-empty. + okCtx := suite.network.GetContext().WithEventManager(sdk.NewEventManager()) + suite.Require().NoError(suite.derivedTransfer(okCtx, owner, contractAddr, recipient)) + suite.Require().Positive(txLogAttrCount(okCtx.EventManager().Events()), + "a successful derived tx must emit its logs") +} + +// txLogAttrCount returns the total number of tx_log attributes across all tx_log events. +func txLogAttrCount(events []sdk.Event) int { + n := 0 + for _, e := range events { + if e.Type == evmtypes.EventTypeTxLog { + n += len(e.Attributes) + } + } + return n +} + // TestDerivedEVMCallCommitFlag is a regression test for F-2026-17736: a derived // call with commit=false must not persist any state changes, while commit=true // must persist them. It deploys an ERC20, performs a state-changing transfer via