feat: reject tx if total blob size too large (backport #2202) (#2268)

app/ante.go → app/ante/ante.go

@@ -1,4 +1,4 @@
-package app
+package ante
 
 import (
 	blobante "github.com/celestiaorg/celestia-app/x/blob/ante"
@@ -36,7 +36,11 @@ func NewAnteHandler(
 		ante.NewSigGasConsumeDecorator(accountKeeper, sigGasConsumer),
 		ante.NewSigVerificationDecorator(accountKeeper, signModeHandler),
 		blobante.NewMinGasPFBDecorator(blobKeeper),
+		blobante.NewMaxBlobSizeDecorator(blobKeeper),
+		NewGovProposalDecorator(),
 		ante.NewIncrementSequenceDecorator(accountKeeper),
 		ibcante.NewRedundantRelayDecorator(channelKeeper),
 	)
 }
+
+var DefaultSigVerificationGasConsumer = ante.DefaultSigVerificationGasConsumer

app/ante/gov.go

@@ -0,0 +1,27 @@
+package ante
+
+import (
+	"cosmossdk.io/errors"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	gov "github.com/cosmos/cosmos-sdk/x/gov/types"
+	govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
+)
+
+type GovProposalDecorator struct{}
+
+func NewGovProposalDecorator() GovProposalDecorator {
+	return GovProposalDecorator{}
+}
+
+// AnteHandle implements the AnteHandler interface. It ensures that MsgSubmitProposal has at least one message
+func (d GovProposalDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
+	for _, m := range tx.GetMsgs() {
+		if proposal, ok := m.(*govv1.MsgSubmitProposal); ok {
+			if len(proposal.Messages) == 0 {
+				return ctx, errors.Wrapf(gov.ErrNoProposalMsgs, "must include at least one message in proposal")
+			}
+		}
+	}
+
+	return next(ctx, tx, simulate)
+}

app/ante/gov_test.go

@@ -0,0 +1,75 @@
+package ante_test
+
+import (
+	"testing"
+
+	"github.com/celestiaorg/celestia-app/app"
+	"github.com/celestiaorg/celestia-app/app/ante"
+	"github.com/celestiaorg/celestia-app/app/encoding"
+	"github.com/celestiaorg/celestia-app/test/util/testfactory"
+	"github.com/cosmos/cosmos-sdk/types"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGovDecorator(t *testing.T) {
+	decorator := ante.NewGovProposalDecorator()
+	anteHandler := types.ChainAnteDecorators(decorator)
+	accounts := testfactory.GenerateAccounts(1)
+	coins := types.NewCoins(types.NewCoin("utia", types.NewInt(10)))
+
+	msgSend := banktypes.NewMsgSend(
+		testfactory.RandomAddress().(types.AccAddress),
+		testfactory.RandomAddress().(types.AccAddress),
+		coins,
+	)
+	encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...)
+
+	msgProposal, err := govtypes.NewMsgSubmitProposal([]types.Msg{msgSend}, coins, accounts[0], "")
+	require.NoError(t, err)
+	msgEmptyProposal, err := govtypes.NewMsgSubmitProposal([]types.Msg{}, coins, accounts[0], "do nothing")
+	require.NoError(t, err)
+
+	testCases := []struct {
+		name   string
+		msg    []types.Msg
+		expErr bool
+	}{
+		{
+			name:   "good proposal; has at least one message",
+			msg:    []types.Msg{msgProposal},
+			expErr: false,
+		},
+		{
+			name:   "bad proposal; has no messages",
+			msg:    []types.Msg{msgEmptyProposal},
+			expErr: true,
+		},
+		{
+			name:   "good proposal; multiple messages in tx",
+			msg:    []types.Msg{msgProposal, msgSend},
+			expErr: false,
+		},
+		{
+			name:   "bad proposal; multiple messages in tx but proposal has no messages",
+			msg:    []types.Msg{msgEmptyProposal, msgSend},
+			expErr: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := types.Context{}
+			builder := encCfg.TxConfig.NewTxBuilder()
+			require.NoError(t, builder.SetMsgs(tc.msg...))
+			tx := builder.GetTx()
+			_, err := anteHandler(ctx, tx, false)
+			if tc.expErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}

app/app.go

@@ -22,7 +22,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/types/module"
 	"github.com/cosmos/cosmos-sdk/version"
 	"github.com/cosmos/cosmos-sdk/x/auth"
-	"github.com/cosmos/cosmos-sdk/x/auth/ante"
 	authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
 	"github.com/cosmos/cosmos-sdk/x/auth/posthandler"
 	authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
@@ -85,6 +84,7 @@ import (
 	tmos "github.com/tendermint/tendermint/libs/os"
 	dbm "github.com/tendermint/tm-db"
 
+	"github.com/celestiaorg/celestia-app/app/ante"
 	"github.com/celestiaorg/celestia-app/app/encoding"
 	"github.com/celestiaorg/celestia-app/pkg/proof"
 	blobmodule "github.com/celestiaorg/celestia-app/x/blob"
@@ -532,7 +532,7 @@ func New(
 	app.SetInitChainer(app.InitChainer)
 	app.SetBeginBlocker(app.BeginBlocker)
 	app.SetEndBlocker(app.EndBlocker)
-	app.SetAnteHandler(NewAnteHandler(
+	app.SetAnteHandler(ante.NewAnteHandler(
 		app.AccountKeeper,
 		app.BankKeeper,
 		app.BlobKeeper,

app/prepare_proposal.go

@@ -1,10 +1,10 @@
 package app
 
 import (
+	"github.com/celestiaorg/celestia-app/app/ante"
 	"github.com/celestiaorg/celestia-app/pkg/da"
 	"github.com/celestiaorg/celestia-app/pkg/shares"
 	"github.com/celestiaorg/celestia-app/pkg/square"
-	"github.com/cosmos/cosmos-sdk/x/auth/ante"
 	abci "github.com/tendermint/tendermint/abci/types"
 	core "github.com/tendermint/tendermint/proto/tendermint/types"
 )
@@ -23,7 +23,7 @@ func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePr
 	// TODO: we can remove all state independent checks from the ante handler here such as signature verification
 	// and only check the state dependent checks like fees and nonces as all these transactions have already
 	// passed CheckTx.
-	handler := NewAnteHandler(
+	handler := ante.NewAnteHandler(
 		app.AccountKeeper,
 		app.BankKeeper,
 		app.BlobKeeper,

app/process_proposal.go

@@ -4,12 +4,12 @@ import (
 	"bytes"
 	"fmt"
 
+	"github.com/celestiaorg/celestia-app/app/ante"
 	"github.com/celestiaorg/celestia-app/pkg/da"
 	"github.com/celestiaorg/celestia-app/pkg/shares"
 	"github.com/celestiaorg/celestia-app/pkg/square"
 	blobtypes "github.com/celestiaorg/celestia-app/x/blob/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/cosmos/cosmos-sdk/x/auth/ante"
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/libs/log"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
@@ -32,7 +32,7 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp
 	// transactions. All transactions need to be equally validated here
 	// so that the nonce number is always correctly incremented (which
 	// may affect the validity of future transactions).
-	handler := NewAnteHandler(
+	handler := ante.NewAnteHandler(
 		app.AccountKeeper,
 		app.BankKeeper,
 		app.BlobKeeper,

app/test/check_tx_test.go

@@ -24,7 +24,7 @@ func TestCheckTx(t *testing.T) {
 	encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...)
 	ns1 := appns.MustNewV0(bytes.Repeat([]byte{1}, appns.NamespaceVersionZeroIDSize))
 
-	accs := []string{"a", "b", "c", "d", "e", "f"}
+	accs := []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j"}
 
 	testApp, kr := testutil.SetupTestAppWithGenesisValSet(app.DefaultConsensusParams(), accs...)
 
@@ -107,6 +107,51 @@ func TestCheckTx(t *testing.T) {
 			},
 			expectedABCICode: abci.CodeTypeOK,
 		},
+		{
+			name:      "1,000 byte blob",
+			checkType: abci.CheckTxType_New,
+			getTx: func() []byte {
+				tx := blobfactory.RandBlobTxsWithAccounts(encCfg.TxConfig.TxEncoder(), tmrand.NewRand(), kr, nil, 1_000, 1, false, testutil.ChainID, accs[4:5])[0]
+				return tx
+			},
+			expectedABCICode: abci.CodeTypeOK,
+		},
+		{
+			name:      "10,000 byte blob",
+			checkType: abci.CheckTxType_New,
+			getTx: func() []byte {
+				tx := blobfactory.RandBlobTxsWithAccounts(encCfg.TxConfig.TxEncoder(), tmrand.NewRand(), kr, nil, 10_000, 1, false, testutil.ChainID, accs[5:6])[0]
+				return tx
+			},
+			expectedABCICode: abci.CodeTypeOK,
+		},
+		{
+			name:      "100,000 byte blob",
+			checkType: abci.CheckTxType_New,
+			getTx: func() []byte {
+				tx := blobfactory.RandBlobTxsWithAccounts(encCfg.TxConfig.TxEncoder(), tmrand.NewRand(), kr, nil, 100_000, 1, false, testutil.ChainID, accs[6:7])[0]
+				return tx
+			},
+			expectedABCICode: abci.CodeTypeOK,
+		},
+		{
+			name:      "1,000,000 byte blob",
+			checkType: abci.CheckTxType_New,
+			getTx: func() []byte {
+				tx := blobfactory.RandBlobTxsWithAccounts(encCfg.TxConfig.TxEncoder(), tmrand.NewRand(), kr, nil, 1_000_000, 1, false, testutil.ChainID, accs[7:8])[0]
+				return tx
+			},
+			expectedABCICode: abci.CodeTypeOK,
+		},
+		{
+			name:      "10,000,000 byte blob",
+			checkType: abci.CheckTxType_New,
+			getTx: func() []byte {
+				tx := blobfactory.RandBlobTxsWithAccounts(encCfg.TxConfig.TxEncoder(), tmrand.NewRand(), kr, nil, 10_000_000, 1, false, testutil.ChainID, accs[8:9])[0]
+				return tx
+			},
+			expectedABCICode: blobtypes.ErrTotalBlobSizeTooLarge.ABCICode(),
+		},
 	}
 
 	for _, tt := range tests {

app/test/integration_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/cosmos/cosmos-sdk/client"
+	"github.com/cosmos/cosmos-sdk/types/errors"
 
 	"github.com/stretchr/testify/suite"
 
@@ -29,6 +30,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	abci "github.com/tendermint/tendermint/abci/types"
 	tmrand "github.com/tendermint/tendermint/libs/rand"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	coretypes "github.com/tendermint/tendermint/types"
 )
 
@@ -374,3 +376,64 @@ func (s *IntegrationTestSuite) TestEmptyBlock() {
 		ExtendBlobTest(t, blockRes.Block)
 	}
 }
+
+// TestSubmitPayForBlob_blobSizes verifies the tx response ABCI code when
+// SubmitPayForBlob is invoked with different blob sizes.
+func (s *IntegrationTestSuite) TestSubmitPayForBlob_blobSizes() {
+	t := s.T()
+	require.NoError(t, s.cctx.WaitForBlocks(3))
+
+	type testCase struct {
+		name string
+		blob *tmproto.Blob
+		// txResponseCode is the expected tx response ABCI code.
+		txResponseCode uint32
+	}
+	testCases := []testCase{
+		{
+			name:           "1,000 byte blob",
+			blob:           mustNewBlob(t, 1_000),
+			txResponseCode: abci.CodeTypeOK,
+		},
+		{
+			name:           "10,000 byte blob",
+			blob:           mustNewBlob(t, 10_000),
+			txResponseCode: abci.CodeTypeOK,
+		},
+		{
+			name:           "100,000 byte blob",
+			blob:           mustNewBlob(t, 100_000),
+			txResponseCode: abci.CodeTypeOK,
+		},
+		{
+			name:           "1,000,000 byte blob",
+			blob:           mustNewBlob(t, 1_000_000),
+			txResponseCode: abci.CodeTypeOK,
+		},
+		{
+			name:           "10,000,000 byte blob returns err tx too large",
+			blob:           mustNewBlob(t, 10_000_000),
+			txResponseCode: errors.ErrTxTooLarge.ABCICode(),
+		},
+	}
+
+	for _, tc := range testCases {
+		s.Run(tc.name, func() {
+			signer := blobtypes.NewKeyringSigner(s.cctx.Keyring, s.accounts[141], s.cctx.ChainID)
+			options := []blobtypes.TxBuilderOption{blobtypes.SetGasLimit(1_000_000_000)}
+			res, err := blob.SubmitPayForBlob(context.TODO(), signer, s.cctx.GRPCClient, []*blobtypes.Blob{tc.blob}, options...)
+
+			require.NoError(t, err)
+			require.NotNil(t, res)
+			require.Equal(t, tc.txResponseCode, res.Code, res.Logs)
+		})
+	}
+}
+
+func mustNewBlob(t *testing.T, blobSize int) *tmproto.Blob {
+	ns1 := appns.MustNewV0(bytes.Repeat([]byte{1}, appns.NamespaceVersionZeroIDSize))
+	data := tmrand.Bytes(blobSize)
+	result, err := blobtypes.NewBlob(ns1, data, appconsts.ShareVersionZero)
+	require.NoError(t, err)
+	return result
+}