Skip to content

Commit

Permalink
feat!: limit the max tx size to 2 MiB (#3909)
Browse files Browse the repository at this point in the history
## Overview

Fixes [#3686](#3686)

---------

Co-authored-by: Rootul P <[email protected]>
  • Loading branch information
ninabarbakadze and rootulp authored Oct 11, 2024
1 parent d61a686 commit dcea313
Show file tree
Hide file tree
Showing 14 changed files with 213 additions and 1 deletion.
2 changes: 2 additions & 0 deletions app/ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ func NewAnteHandler(
// Set up the context with a gas meter.
// Must be called before gas consumption occurs in any other decorator.
ante.NewSetUpContextDecorator(),
// Ensure the tx is not larger than the configured threshold.
NewMaxTxSizeDecorator(),
// Ensure the tx does not contain any extension options.
ante.NewExtensionOptionsDecorator(nil),
// Ensure the tx passes ValidateBasic.
Expand Down
33 changes: 33 additions & 0 deletions app/ante/max_tx_size.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package ante

import (
"fmt"

"github.com/celestiaorg/celestia-app/v3/pkg/appconsts"
v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3"
sdk "github.com/cosmos/cosmos-sdk/types"
)

// MaxTxSizeDecorator ensures that a tx can not be larger than
// application's configured versioned constant.
type MaxTxSizeDecorator struct{}

func NewMaxTxSizeDecorator() MaxTxSizeDecorator {
return MaxTxSizeDecorator{}
}

// AnteHandle implements the AnteHandler interface. It ensures that tx size is under application's configured threshold.
func (d MaxTxSizeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
// Tx size rule applies to app versions v3 and onwards.
if ctx.BlockHeader().Version.App < v3.Version {
return next(ctx, tx, simulate)
}

currentTxSize := len(ctx.TxBytes())
maxTxBytes := appconsts.MaxTxBytes(ctx.BlockHeader().Version.App)
if currentTxSize > maxTxBytes {
bytesOverLimit := currentTxSize - maxTxBytes
return ctx, fmt.Errorf("tx size %d bytes is larger than the application's configured threshold of %d bytes. Please reduce the size by %d bytes", currentTxSize, maxTxBytes, bytesOverLimit)
}
return next(ctx, tx, simulate)
}
78 changes: 78 additions & 0 deletions app/ante/max_tx_size_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package ante_test

import (
"testing"

"github.com/celestiaorg/celestia-app/v3/app/ante"
v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2"
v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/require"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
version "github.com/tendermint/tendermint/proto/tendermint/version"
)

func TestMaxTxSizeDecorator(t *testing.T) {
decorator := ante.NewMaxTxSizeDecorator()
anteHandler := sdk.ChainAnteDecorators(decorator)

testCases := []struct {
name string
txSize int
expectError bool
appVersion uint64
isCheckTx []bool
}{
{
name: "good tx; under max tx bytes threshold",
txSize: v3.MaxTxBytes - 1,
appVersion: v3.Version,
expectError: false,
isCheckTx: []bool{true, false},
},
{
name: "bad tx; over max tx bytes threshold",
txSize: v3.MaxTxBytes + 1,
appVersion: v3.Version,
expectError: true,
isCheckTx: []bool{true, false},
},
{
name: "good tx; equal to max tx bytes threshold",
txSize: v3.MaxTxBytes,
appVersion: v3.Version,
expectError: false,
isCheckTx: []bool{true, false},
},
{
name: "good tx; limit only applies to v3 and above",
txSize: v3.MaxTxBytes + 10,
appVersion: v2.Version,
expectError: false,
isCheckTx: []bool{true, false},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
for _, isCheckTx := range tc.isCheckTx {

ctx := sdk.NewContext(nil, tmproto.Header{
Version: version.Consensus{
App: tc.appVersion,
},
}, isCheckTx, nil)

txBytes := make([]byte, tc.txSize)

ctx = ctx.WithTxBytes(txBytes)
_, err := anteHandler(ctx, nil, false)
if tc.expectError {
require.Error(t, err)
} else {
require.NoError(t, err)
}
}
})
}
}
File renamed without changes.
File renamed without changes.
5 changes: 5 additions & 0 deletions app/process_proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp
// sdkCtx = sdkCtx.WithTxBytes(tx)

sdkTx, err := app.txConfig.TxDecoder()(tx)
// Set the tx bytes in the context for app version v3 and greater
if sdkCtx.BlockHeader().Version.App >= 3 {
sdkCtx = sdkCtx.WithTxBytes(tx)
}

if err != nil {
if req.Header.Version.App == v1 {
// For appVersion 1, there was no block validity rule that all
Expand Down
33 changes: 33 additions & 0 deletions app/test/prepare_proposal_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package app_test

import (
"strings"
"testing"
"time"

Expand All @@ -16,6 +17,7 @@ import (
"github.com/celestiaorg/celestia-app/v3/app"
"github.com/celestiaorg/celestia-app/v3/app/encoding"
"github.com/celestiaorg/celestia-app/v3/pkg/appconsts"
"github.com/celestiaorg/celestia-app/v3/pkg/user"
testutil "github.com/celestiaorg/celestia-app/v3/test/util"
"github.com/celestiaorg/celestia-app/v3/test/util/blobfactory"
"github.com/celestiaorg/celestia-app/v3/test/util/testfactory"
Expand All @@ -39,6 +41,7 @@ func TestPrepareProposalPutsPFBsAtEnd(t *testing.T) {
accnts[:numBlobTxs],
infos[:numBlobTxs],
testfactory.Repeat([]*share.Blob{protoBlob}, numBlobTxs),
blobfactory.DefaultTxOpts()...,
)

normalTxs := testutil.SendTxsWithAccounts(
Expand Down Expand Up @@ -96,6 +99,7 @@ func TestPrepareProposalFiltering(t *testing.T) {
testfactory.RandomBlobNamespaces(tmrand.NewRand(), 3),
[][]int{{100}, {1000}, {420}},
),
blobfactory.DefaultTxOpts()...,
)

// create 3 MsgSend transactions that are signed with valid account numbers
Expand Down Expand Up @@ -153,6 +157,28 @@ func TestPrepareProposalFiltering(t *testing.T) {
),
)[0]

// memo is 2 MiB resulting in the transaction being over limit
largeString := strings.Repeat("a", 2*1024*1024)

// 3 transactions over MaxTxBytes limit
largeTxs := coretypes.Txs(testutil.SendTxsWithAccounts(t, testApp, encConf.TxConfig, kr, 1000, accounts[0], accounts[:3], testutil.ChainID, user.SetMemo(largeString))).ToSliceOfBytes()

// 3 blobTxs over MaxTxBytes limit
largeBlobTxs := blobfactory.ManyMultiBlobTx(
t,
encConf.TxConfig,
kr,
testutil.ChainID,
accounts[:3],
infos[:3],
blobfactory.NestedBlobs(
t,
testfactory.RandomBlobNamespaces(tmrand.NewRand(), 3),
[][]int{{100}, {1000}, {420}},
),
user.SetMemo(largeString),
)

type test struct {
name string
txs func() [][]byte
Expand Down Expand Up @@ -204,6 +230,13 @@ func TestPrepareProposalFiltering(t *testing.T) {
},
prunedTxs: [][]byte{tooManyShareBtx},
},
{
name: "blobTxs and sendTxs that exceed MaxTxBytes limit",
txs: func() [][]byte {
return append(largeTxs, largeBlobTxs...) // All txs are over MaxTxBytes limit
},
prunedTxs: append(largeTxs, largeBlobTxs...),
},
}

for _, tt := range tests {
Expand Down
41 changes: 41 additions & 0 deletions app/test/process_proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package app_test
import (
"bytes"
"fmt"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -50,6 +51,26 @@ func TestProcessProposal(t *testing.T) {
testfactory.RandomBlobNamespaces(tmrand.NewRand(), 4),
[][]int{{100}, {1000}, {420}, {300}},
),
blobfactory.DefaultTxOpts()...,
)

largeMemo := strings.Repeat("a", appconsts.MaxTxBytes(appconsts.LatestVersion))

// create 2 single blobTxs that include a large memo making the transaction
// larger than the configured max tx bytes
largeBlobTxs := blobfactory.ManyMultiBlobTx(
t, enc, kr, testutil.ChainID, accounts[3:], infos[3:],
blobfactory.NestedBlobs(
t,
testfactory.RandomBlobNamespaces(tmrand.NewRand(), 4),
[][]int{{100}, {1000}, {420}, {300}},
),
user.SetMemo(largeMemo))

// create 1 large sendTx that includes a large memo making the
// transaction over the configured max tx bytes limit
largeSendTx := testutil.SendTxsWithAccounts(
t, testApp, enc, kr, 1000, accounts[0], accounts[1:2], testutil.ChainID, user.SetMemo(largeMemo),
)

// create 3 MsgSend transactions that are signed with valid account numbers
Expand Down Expand Up @@ -327,6 +348,26 @@ func TestProcessProposal(t *testing.T) {
appVersion: v3.Version,
expectedResult: abci.ResponseProcessProposal_REJECT,
},
{
name: "blob txs larger than configured max tx bytes",
input: validData(),
mutator: func(d *tmproto.Data) {
d.Txs = append(d.Txs, largeBlobTxs...)
d.Hash = calculateNewDataHash(t, d.Txs)
},
appVersion: appconsts.LatestVersion,
expectedResult: abci.ResponseProcessProposal_REJECT,
},
{
name: "send tx larger than configured max tx bytes",
input: validData(),
mutator: func(d *tmproto.Data) {
d.Txs = append(coretypes.Txs(largeSendTx).ToSliceOfBytes(), d.Txs...)
d.Hash = calculateNewDataHash(t, d.Txs)
},
appVersion: appconsts.LatestVersion,
expectedResult: abci.ResponseProcessProposal_REJECT,
},
}

for _, tt := range tests {
Expand Down
8 changes: 8 additions & 0 deletions app/validate_txs.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ func filterStdTxs(logger log.Logger, dec sdk.TxDecoder, ctx sdk.Context, handler
logger.Error("decoding already checked transaction", "tx", tmbytes.HexBytes(coretypes.Tx(tx).Hash()), "error", err)
continue
}

// Set the tx size on the context before calling the AnteHandler
ctx = ctx.WithTxBytes(tx)

ctx, err = handler(ctx, sdkTx, false)
// either the transaction is invalid (ie incorrect nonce) and we
// simply want to remove this tx, or we're catching a panic from one
Expand Down Expand Up @@ -85,6 +89,10 @@ func filterBlobTxs(logger log.Logger, dec sdk.TxDecoder, ctx sdk.Context, handle
logger.Error("decoding already checked blob transaction", "tx", tmbytes.HexBytes(coretypes.Tx(tx.Tx).Hash()), "error", err)
continue
}

// Set the tx size on the context before calling the AnteHandler
ctx = ctx.WithTxBytes(tx.Tx)

ctx, err = handler(ctx, sdkTx, false)
// either the transaction is invalid (ie incorrect nonce) and we
// simply want to remove this tx, or we're catching a panic from one
Expand Down
1 change: 1 addition & 0 deletions pkg/appconsts/v3/app_consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ const (
SubtreeRootThreshold int = 64
TxSizeCostPerByte uint64 = 10
GasPerBlobByte uint32 = 8
MaxTxBytes int = 2097152 // 2 MiB in bytes
)
4 changes: 4 additions & 0 deletions pkg/appconsts/versioned_consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ func GasPerBlobByte(_ uint64) uint32 {
return v3.GasPerBlobByte
}

func MaxTxBytes(_ uint64) int {
return v3.MaxTxBytes
}

var (
DefaultSubtreeRootThreshold = SubtreeRootThreshold(LatestVersion)
DefaultSquareSizeUpperBound = SquareSizeUpperBound(LatestVersion)
Expand Down
6 changes: 6 additions & 0 deletions pkg/appconsts/versioned_consts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ func TestVersionedConsts(t *testing.T) {
expectedConstant: v3.GasPerBlobByte,
got: appconsts.GasPerBlobByte(v3.Version),
},
{
name: "MaxTxBytes v3",
version: v3.Version,
expectedConstant: v3.MaxTxBytes,
got: appconsts.MaxTxBytes(v3.Version),
},
}

for _, tc := range testCases {
Expand Down
1 change: 1 addition & 0 deletions specs/src/ante_handler_v3.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
The AnteHandler chains together several decorators to ensure the following criteria are met for app version 3:

- The tx does not contain any messages that are unsupported by the current app version. See `MsgVersioningGateKeeper`.
- The tx size is not larger than the application's configured versioned constant MaxTxBytes.
- The tx does not contain any [extension options](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L119-L122).
- The tx passes `ValidateBasic()`.
- The tx's [timeout_height](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L115-L117) has not been reached if one is specified.
Expand Down
2 changes: 1 addition & 1 deletion test/util/blobfactory/payforblob_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,10 @@ func ManyMultiBlobTx(
accounts []string,
accInfos []AccountInfo,
blobs [][]*share.Blob,
opts ...user.TxOption,
) [][]byte {
t.Helper()
txs := make([][]byte, len(accounts))
opts := DefaultTxOpts()
for i, acc := range accounts {
signer, err := user.NewSigner(kr, enc, chainid, appconsts.LatestVersion, user.NewAccount(acc, accInfos[i].AccountNum, accInfos[i].Sequence))
require.NoError(t, err)
Expand Down

0 comments on commit dcea313

Please sign in to comment.