Skip to content

Commit

Permalink
chore: log errors and fix gas limits in tests
Browse files Browse the repository at this point in the history
  • Loading branch information
evan-forbes committed Sep 18, 2023
1 parent b59e876 commit f7d8f26
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 7 deletions.
2 changes: 1 addition & 1 deletion app/prepare_proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePr
if app.LastBlockHeight() == 0 {
txs = make([][]byte, 0)
} else {
txs = FilterTxs(sdkCtx, handler, app.txConfig, req.BlockData.Txs)
txs = FilterTxs(app.Logger(), sdkCtx, handler, app.txConfig, req.BlockData.Txs)
}

// build the square from the set of valid and prioritised transactions.
Expand Down
3 changes: 3 additions & 0 deletions app/test/fuzz_abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/celestiaorg/celestia-app/app"
"github.com/celestiaorg/celestia-app/app/encoding"
"github.com/celestiaorg/celestia-app/pkg/appconsts"
"github.com/celestiaorg/celestia-app/pkg/user"
testutil "github.com/celestiaorg/celestia-app/test/util"
"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
Expand Down Expand Up @@ -102,6 +103,7 @@ func TestPrepareProposalConsistency(t *testing.T) {
true,
testutil.ChainID,
accounts[:tt.count],
user.SetGasLimitAndFee(1_000_000_000, 0.1),
)
// create 100 send transactions
sendTxs := testutil.SendTxsWithAccounts(
Expand All @@ -113,6 +115,7 @@ func TestPrepareProposalConsistency(t *testing.T) {
accounts[0],
accounts[len(accounts)-sendTxCount:],
testutil.ChainID,
user.SetGasLimitAndFee(1_000_000, 0.1),
)
txs = append(txs, sendTxs...)
resp := testApp.PrepareProposal(abci.RequestPrepareProposal{
Expand Down
14 changes: 9 additions & 5 deletions app/validate_txs.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package app
import (
"github.com/cosmos/cosmos-sdk/client"
sdk "github.com/cosmos/cosmos-sdk/types"
tmbytes "github.com/tendermint/tendermint/libs/bytes"
"github.com/tendermint/tendermint/libs/log"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
coretypes "github.com/tendermint/tendermint/types"
)
Expand All @@ -23,17 +25,17 @@ func separateTxs(_ client.TxConfig, rawTxs [][]byte) ([][]byte, []tmproto.BlobTx
}

// FilterTxs applies the antehandler to all proposed transactions and removes transactions that return an error.
func FilterTxs(ctx sdk.Context, handler sdk.AnteHandler, txConfig client.TxConfig, txs [][]byte) [][]byte {
func FilterTxs(logger log.Logger, ctx sdk.Context, handler sdk.AnteHandler, txConfig client.TxConfig, txs [][]byte) [][]byte {
normalTxs, blobTxs := separateTxs(txConfig, txs)
normalTxs, ctx = filterStdTxs(txConfig.TxDecoder(), ctx, handler, normalTxs)
blobTxs, _ = filterBlobTxs(txConfig.TxDecoder(), ctx, handler, blobTxs)
normalTxs, ctx = filterStdTxs(logger, txConfig.TxDecoder(), ctx, handler, normalTxs)
blobTxs, _ = filterBlobTxs(logger, txConfig.TxDecoder(), ctx, handler, blobTxs)
return append(normalTxs, encodeBlobTxs(blobTxs)...)
}

// filterStdTxs applies the provided antehandler to each transaction and removes
// transactions that return an error. Panics are caught by the checkTxValidity
// function used to apply the ante handler.
func filterStdTxs(dec sdk.TxDecoder, ctx sdk.Context, handler sdk.AnteHandler, txs [][]byte) ([][]byte, sdk.Context) {
func filterStdTxs(logger log.Logger, dec sdk.TxDecoder, ctx sdk.Context, handler sdk.AnteHandler, txs [][]byte) ([][]byte, sdk.Context) {
n := 0
var err error
for _, tx := range txs {
Expand All @@ -42,6 +44,7 @@ func filterStdTxs(dec sdk.TxDecoder, ctx sdk.Context, handler sdk.AnteHandler, t
// simply want to remove this tx, or we're catching a panic from one
// of the anteHanders which is logged.
if err != nil {
logger.Error("filtering already checked transaction", "tx", tmbytes.FromBytes(tx), "error", err)
continue
}
txs[n] = tx
Expand All @@ -55,7 +58,7 @@ func filterStdTxs(dec sdk.TxDecoder, ctx sdk.Context, handler sdk.AnteHandler, t
// filterBlobTxs applies the provided antehandler to each transaction
// and removes transactions that return an error. Panics are caught by the checkTxValidity
// function used to apply the ante handler.
func filterBlobTxs(dec sdk.TxDecoder, ctx sdk.Context, handler sdk.AnteHandler, txs []tmproto.BlobTx) ([]tmproto.BlobTx, sdk.Context) {
func filterBlobTxs(logger log.Logger, dec sdk.TxDecoder, ctx sdk.Context, handler sdk.AnteHandler, txs []tmproto.BlobTx) ([]tmproto.BlobTx, sdk.Context) {
n := 0
var err error
for _, tx := range txs {
Expand All @@ -64,6 +67,7 @@ func filterBlobTxs(dec sdk.TxDecoder, ctx sdk.Context, handler sdk.AnteHandler,
// simply want to remove this tx, or we're catching a panic from one
// of the anteHanders which is logged.
if err != nil {
logger.Error("filtering already checked blob transaction", "tx", tmbytes.FromBytes(tx.Tx), "error", err)
continue
}
txs[n] = tx
Expand Down
2 changes: 1 addition & 1 deletion test/util/malicious/out_of_order_prepare.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (a *App) OutOfOrderPrepareProposal(req abci.RequestPrepareProposal) abci.Re
a.IBCKeeper,
)

txs := app.FilterTxs(sdkCtx, handler, a.GetTxConfig(), req.BlockData.Txs)
txs := app.FilterTxs(a.Logger(), sdkCtx, handler, a.GetTxConfig(), req.BlockData.Txs)

// build the square from the set of valid and prioritised transactions.
// The txs returned are the ones used in the square and block
Expand Down

0 comments on commit f7d8f26

Please sign in to comment.