diff --git a/app/app_test.go b/app/app_test.go index e7db1db562..f6611b8734 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -21,10 +21,14 @@ import ( ethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/sei-protocol/sei-chain/app" + clienttx "github.com/sei-protocol/sei-chain/sei-cosmos/client/tx" cryptocodec "github.com/sei-protocol/sei-chain/sei-cosmos/crypto/codec" cosmosed25519 "github.com/sei-protocol/sei-chain/sei-cosmos/crypto/keys/ed25519" "github.com/sei-protocol/sei-chain/sei-cosmos/crypto/keys/secp256k1" + cryptotypes "github.com/sei-protocol/sei-chain/sei-cosmos/crypto/types" sdk "github.com/sei-protocol/sei-chain/sei-cosmos/types" + "github.com/sei-protocol/sei-chain/sei-cosmos/types/tx/signing" + xauthsigning "github.com/sei-protocol/sei-chain/sei-cosmos/x/auth/signing" authtypes "github.com/sei-protocol/sei-chain/sei-cosmos/x/auth/types" banktypes "github.com/sei-protocol/sei-chain/sei-cosmos/x/bank/types" abci "github.com/sei-protocol/sei-chain/sei-tendermint/abci/types" @@ -763,6 +767,134 @@ func TestDeliverTxWithNilTypedTxDoesNotPanic(t *testing.T) { }) } +func signCosmosTx( + t *testing.T, + txCfg client.TxConfig, + txBuilder client.TxBuilder, + privKey cryptotypes.PrivKey, + acc authtypes.AccountI, +) []byte { + // Set signatures with empty sig first to populate SignerInfos + sigV2 := signing.SignatureV2{ + PubKey: privKey.PubKey(), + Data: &signing.SingleSignatureData{ + SignMode: txCfg.SignModeHandler().DefaultMode(), + Signature: nil, + }, + Sequence: acc.GetSequence(), + } + err := txBuilder.SetSignatures(sigV2) + require.NoError(t, err) + + // Sign for real + signerData := xauthsigning.SignerData{ + ChainID: "sei-test", + AccountNumber: acc.GetAccountNumber(), + Sequence: acc.GetSequence(), + } + sigV2, err = clienttx.SignWithPrivKey( + txCfg.SignModeHandler().DefaultMode(), + signerData, txBuilder, privKey, txCfg, acc.GetSequence(), + ) + require.NoError(t, err) + err = txBuilder.SetSignatures(sigV2) + require.NoError(t, err) + + txBytes, err := txCfg.TxEncoder()(txBuilder.GetTx()) + require.NoError(t, err) + return txBytes +} + +func TestDecodeFailureTxReportsZeroGas(t *testing.T) { + // Set up app with a funded genesis account + senderPriv := secp256k1.GenPrivKey() + senderPub := senderPriv.PubKey() + senderAddr := sdk.AccAddress(senderPub.Address()) + receiverAddr := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + + genAcc := authtypes.NewBaseAccount(senderAddr, senderPub, 0, 0) + balance := banktypes.Balance{ + Address: senderAddr.String(), + Coins: sdk.NewCoins(sdk.NewCoin("usei", sdk.NewInt(1_000_000_000))), + } + tmPub, err := cryptocodec.ToTmPubKeyInterface(cosmosed25519.GenPrivKey().PubKey()) + require.NoError(t, err) + valSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{tmtypes.NewValidator(tmPub, 1)}) + + testApp := app.SetupWithGenesisValSet(t, valSet, []authtypes.GenesisAccount{genAcc}, balance) + txCfg := testApp.GetTxConfig() + + // Look up the account from committed state so we have the right sequence + ctx := testApp.NewUncachedContext(false, types.Header{Height: testApp.LastBlockHeight()}) + acc := testApp.AccountKeeper.GetAccount(ctx, senderAddr) + require.NotNil(t, acc) + + // Build a signed bank send tx (should succeed) + txBuilder := txCfg.NewTxBuilder() + err = txBuilder.SetMsgs(&banktypes.MsgSend{ + FromAddress: senderAddr.String(), + ToAddress: receiverAddr.String(), + Amount: sdk.NewCoins(sdk.NewInt64Coin("usei", 1)), + }) + require.NoError(t, err) + txBuilder.SetGasLimit(200000) + txBuilder.SetFeeAmount(sdk.NewCoins(sdk.NewInt64Coin("usei", 20000))) + normalTx := signCosmosTx(t, txCfg, txBuilder, senderPriv, acc) + + // Build a tx that will decode and pass ante handler but fail in message + // execution (insufficient funds). Use same sequence since OCC may run + // this tx before tx 0 commits. + failAcc := authtypes.NewBaseAccount(senderAddr, senderPub, acc.GetAccountNumber(), acc.GetSequence()) + failTxBuilder := txCfg.NewTxBuilder() + err = failTxBuilder.SetMsgs(&banktypes.MsgSend{ + FromAddress: senderAddr.String(), + ToAddress: receiverAddr.String(), + Amount: sdk.NewCoins(sdk.NewInt64Coin("usei", 999_999_999_999)), // way more than balance + }) + require.NoError(t, err) + failTxBuilder.SetGasLimit(200000) + failTxBuilder.SetFeeAmount(sdk.NewCoins(sdk.NewInt64Coin("usei", 20000))) + failMsgTx := signCosmosTx(t, txCfg, failTxBuilder, senderPriv, failAcc) + + malformedTx := []byte("invalid tx bytes that cannot be decoded") + + txs := [][]byte{ + normalTx, // tx 0: signed bank send (should succeed, reports gas) + malformedTx, // tx 1: decode failure (should report zero gas) + failMsgTx, // tx 2: decoded OK, ante handler runs, but msg execution fails + } + + height := testApp.LastBlockHeight() + 1 + req := &abci.RequestFinalizeBlock{ + Header: &types.Header{ChainID: "sei-test", Height: height}, + } + _, txResults, _, _ := testApp.ProcessBlock( + ctx.WithBlockHeight(height).WithChainID("sei-test"), + txs, + finalizeToBlockProcessReq(req), + req.DecidedLastCommit, + false, + ) + + require.Equal(t, 3, len(txResults)) + + // tx 0: signed bank send — should succeed with nonzero gas + require.Equal(t, uint32(0), txResults[0].Code, "signed bank send should succeed") + require.Greater(t, txResults[0].GasUsed, int64(0), "successful tx should report nonzero GasUsed") + require.Greater(t, txResults[0].GasWanted, int64(0), "successful tx should report nonzero GasWanted") + + // tx 1: malformed tx — ante handler never ran, must report zero gas + require.NotEqual(t, uint32(0), txResults[1].Code, "malformed tx should fail") + require.Equal(t, int64(0), txResults[1].GasUsed, "decode failure must report zero GasUsed") + require.Equal(t, int64(0), txResults[1].GasWanted, "decode failure must report zero GasWanted") + + // tx 2: failed execution (insufficient funds) but the ante handler already + // installed a per-tx gas meter, so gas is reported correctly. + require.NotEqual(t, uint32(0), txResults[2].Code, "insufficient funds tx should fail") + require.Greater(t, txResults[2].GasUsed, int64(0), "failed-after-ante tx should report nonzero GasUsed") + require.Greater(t, txResults[2].GasWanted, int64(0), "failed-after-ante tx should report nonzero GasWanted") +} + func finalizeToBlockProcessReq(req *abci.RequestFinalizeBlock) *app.BlockProcessRequest { var height int64 var blockTime time.Time diff --git a/app/legacyabci/check_tx.go b/app/legacyabci/check_tx.go index 1dada1a3d3..1ce6f0b151 100644 --- a/app/legacyabci/check_tx.go +++ b/app/legacyabci/check_tx.go @@ -72,11 +72,15 @@ func CheckTx( var gasWanted uint64 var gasEstimate uint64 + blockGasMeter := ctx.GasMeter() defer func() { if r := recover(); r != nil { recoveryMW := newOutOfGasRecoveryMiddleware(gasWanted, ctx, defaultRecoveryMiddleware) err, result = processRecovery(r, recoveryMW), nil } + if ctx.GasMeter() == blockGasMeter { + return + } gInfo = sdk.GasInfo{GasWanted: gasWanted, GasUsed: ctx.GasMeter().GasConsumed(), GasEstimate: gasEstimate} }() diff --git a/app/legacyabci/deliver_tx.go b/app/legacyabci/deliver_tx.go index 50541a928b..39b552a379 100644 --- a/app/legacyabci/deliver_tx.go +++ b/app/legacyabci/deliver_tx.go @@ -65,12 +65,16 @@ func DeliverTx( span.SetAttributes(attribute.String("txHash", fmt.Sprintf("%X", checksum))) var gasWanted uint64 ms := ctx.MultiStore() + blockGasMeter := ctx.GasMeter() defer func() { if r := recover(); r != nil { recoveryMW := newOutOfGasRecoveryMiddleware(gasWanted, ctx, defaultRecoveryMiddleware) recoveryMW = newOCCAbortRecoveryMiddleware(recoveryMW) // TODO: do we have to wrap with occ enabled check? err, result = processRecovery(r, recoveryMW), nil } + if ctx.GasMeter() == blockGasMeter { + return + } gInfo = sdk.GasInfo{GasWanted: gasWanted, GasUsed: ctx.GasMeter().GasConsumed()} }() diff --git a/sei-cosmos/baseapp/baseapp.go b/sei-cosmos/baseapp/baseapp.go index 258082ff72..6885ecdf09 100644 --- a/sei-cosmos/baseapp/baseapp.go +++ b/sei-cosmos/baseapp/baseapp.go @@ -841,12 +841,16 @@ func (app *BaseApp) runTx(ctx sdk.Context, mode runTxMode, tx sdk.Tx, checksum [ ms := ctx.MultiStore() + blockGasMeter := ctx.GasMeter() defer func() { if r := recover(); r != nil { recoveryMW := newOutOfGasRecoveryMiddleware(gasWanted, ctx, app.runTxRecoveryMiddleware) recoveryMW = newOCCAbortRecoveryMiddleware(recoveryMW) // TODO: do we have to wrap with occ enabled check? err, result = processRecovery(r, recoveryMW), nil } + if ctx.GasMeter() == blockGasMeter { + return + } gInfo = sdk.GasInfo{GasWanted: gasWanted, GasUsed: ctx.GasMeter().GasConsumed(), GasEstimate: gasEstimate} }() diff --git a/sei-cosmos/baseapp/deliver_tx_test.go b/sei-cosmos/baseapp/deliver_tx_test.go index d13681036e..1df2c53817 100644 --- a/sei-cosmos/baseapp/deliver_tx_test.go +++ b/sei-cosmos/baseapp/deliver_tx_test.go @@ -639,6 +639,23 @@ func TestRunInvalidTransaction(t *testing.T) { } } +func TestRunTxDecodeError(t *testing.T) { + app := setupBaseApp(t) + + header := tmproto.Header{Height: 1} + app.setDeliverState(header) + + // Consume some gas on the block-level meter to simulate prior operations + ctx := app.deliverState.ctx + ctx.GasMeter().ConsumeGas(5000, "simulated prior gas") + + // A decode failure should not report block-level gas as its own + gInfo, _, _, _, _, _, _, _, err := app.runTx(ctx, runTxModeDeliver, nil, [32]byte{}) + require.Error(t, err) + require.Equal(t, uint64(0), gInfo.GasUsed) + require.Equal(t, uint64(0), gInfo.GasWanted) +} + // Test that transactions exceeding gas limits fail func TestTxGasLimits(t *testing.T) { gasGranted := uint64(10)