Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into sanaz/modify-timeouts
Browse files Browse the repository at this point in the history
  • Loading branch information
staheri14 committed Oct 11, 2024
2 parents cd066e6 + cdb4856 commit e12ab20
Show file tree
Hide file tree
Showing 24 changed files with 499 additions and 130 deletions.
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())
maxTxSize := appconsts.MaxTxSize(ctx.BlockHeader().Version.App)
if currentTxSize > maxTxSize {
bytesOverLimit := currentTxSize - maxTxSize
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, maxTxSize, 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 size threshold",
txSize: v3.MaxTxSize - 1,
appVersion: v3.Version,
expectError: false,
isCheckTx: []bool{true, false},
},
{
name: "bad tx; over max tx size threshold",
txSize: v3.MaxTxSize + 1,
appVersion: v3.Version,
expectError: true,
isCheckTx: []bool{true, false},
},
{
name: "good tx; equal to max tx size threshold",
txSize: v3.MaxTxSize,
appVersion: v3.Version,
expectError: false,
isCheckTx: []bool{true, false},
},
{
name: "good tx; limit only applies to v3 and above",
txSize: v3.MaxTxSize + 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.
9 changes: 9 additions & 0 deletions app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package app_test

import (
"encoding/json"
"os"
"path/filepath"
"testing"

Expand Down Expand Up @@ -160,7 +161,15 @@ func createTestApp(t *testing.T) *app.App {
config := encoding.MakeConfig(app.ModuleEncodingRegisters...)
upgradeHeight := int64(3)
snapshotDir := filepath.Join(t.TempDir(), "data", "snapshots")
t.Cleanup(func() {
err := os.RemoveAll(snapshotDir)
require.NoError(t, err)
})
snapshotDB, err := tmdb.NewDB("metadata", tmdb.GoLevelDBBackend, snapshotDir)
t.Cleanup(func() {
err := snapshotDB.Close()
require.NoError(t, err)
})
require.NoError(t, err)
snapshotStore, err := snapshots.NewStore(snapshotDB, snapshotDir)
require.NoError(t, err)
Expand Down
8 changes: 8 additions & 0 deletions app/process_proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,15 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp
tx = blobTx.Tx
}

// todo: uncomment once we're sure this isn't consensus breaking
// 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
65 changes: 65 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 @@ -137,6 +141,44 @@ func TestPrepareProposalFiltering(t *testing.T) {
require.NoError(t, err)
noAccountTx := []byte(testutil.SendTxWithManualSequence(t, encConf.TxConfig, kr, nilAccount, accounts[0], 1000, "", 0, 6))

// create a tx that can't be included in a 64 x 64 when accounting for the
// pfb along with the shares
tooManyShareBtx := blobfactory.ManyMultiBlobTx(
t,
encConf.TxConfig,
kr,
testutil.ChainID,
accounts[3:4],
infos[3:4],
blobfactory.NestedBlobs(
t,
testfactory.RandomBlobNamespaces(tmrand.NewRand(), 4000),
[][]int{repeat(4000, 1)},
),
)[0]

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

// 3 transactions over MaxTxSize 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 MaxTxSize 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 @@ -181,6 +223,20 @@ func TestPrepareProposalFiltering(t *testing.T) {
},
prunedTxs: [][]byte{noAccountTx},
},
{
name: "blob tx with too many shares",
txs: func() [][]byte {
return [][]byte{tooManyShareBtx}
},
prunedTxs: [][]byte{tooManyShareBtx},
},
{
name: "blobTxs and sendTxs that exceed MaxTxSize limit",
txs: func() [][]byte {
return append(largeTxs, largeBlobTxs...) // All txs are over MaxTxSize limit
},
prunedTxs: append(largeTxs, largeBlobTxs...),
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -216,3 +272,12 @@ func queryAccountInfo(capp *app.App, accs []string, kr keyring.Keyring) []blobfa
}
return infos
}

// repeat returns a slice of length n with each element set to val.
func repeat[T any](n int, val T) []T {
result := make([]T, n)
for i := range result {
result[i] = val
}
return result
}
69 changes: 69 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 All @@ -20,6 +21,7 @@ import (
"github.com/celestiaorg/celestia-app/v3/pkg/appconsts"
v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1"
v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2"
v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3"
"github.com/celestiaorg/celestia-app/v3/pkg/da"
"github.com/celestiaorg/celestia-app/v3/pkg/user"
testutil "github.com/celestiaorg/celestia-app/v3/test/util"
Expand Down Expand Up @@ -49,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.MaxTxSize(appconsts.LatestVersion))

// create 2 single blobTxs that include a large memo making the transaction
// larger than the configured max tx size
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 size 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 @@ -80,6 +102,20 @@ func TestProcessProposal(t *testing.T) {
ns1 := share.MustNewV0Namespace(bytes.Repeat([]byte{1}, share.NamespaceVersionZeroIDSize))
data := bytes.Repeat([]byte{1}, 13)

tooManyShareBtx := blobfactory.ManyMultiBlobTx(
t,
enc,
kr,
testutil.ChainID,
accounts[3:4],
infos[3:4],
blobfactory.NestedBlobs(
t,
testfactory.RandomBlobNamespaces(tmrand.NewRand(), 4000),
[][]int{repeat(4000, 1)},
),
)[0]

type test struct {
name string
input *tmproto.Data
Expand Down Expand Up @@ -299,6 +335,39 @@ func TestProcessProposal(t *testing.T) {
appVersion: appconsts.LatestVersion,
expectedResult: abci.ResponseProcessProposal_REJECT,
},
{
name: "blob tx that takes up too many shares",
input: &tmproto.Data{
Txs: [][]byte{},
},
mutator: func(d *tmproto.Data) {
// this tx will get filtered out by prepare proposal before this
// so we add it here
d.Txs = append(d.Txs, tooManyShareBtx)
},
appVersion: v3.Version,
expectedResult: abci.ResponseProcessProposal_REJECT,
},
{
name: "blob txs larger than configured max tx size",
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 size",
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 @@ -50,6 +50,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 @@ -83,6 +87,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
Loading

0 comments on commit e12ab20

Please sign in to comment.