Skip to content

Commit

Permalink
fix: cap tx size
Browse files Browse the repository at this point in the history
  • Loading branch information
ninabarbakadze committed Dec 5, 2024
1 parent 62f232e commit ab11446
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 108 deletions.
9 changes: 9 additions & 0 deletions app/check_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ import (
// transactions that contain blobs.
func (app *App) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
tx := req.Tx

// all txs should be under the max tx size limit
maxTxSize := appconsts.MaxTxSize(app.AppVersion())
currentTxSize := len(tx)
if currentTxSize > appconsts.MaxTxSize(app.AppVersion()) {
err := fmt.Errorf("tx size %d bytes is larger than the application's configured threshold of %d bytes", currentTxSize, maxTxSize)
return sdkerrors.ResponseCheckTxWithEvents(err, 0, 0, []abci.Event{}, false)
}

// check if the transaction contains blobs
btx, isBlob, err := blobtx.UnmarshalBlobTx(tx)
if isBlob && err != nil {
Expand Down
25 changes: 25 additions & 0 deletions app/test/check_tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,31 @@ func TestCheckTx(t *testing.T) {
},
expectedABCICode: abci.CodeTypeOK,
},
{
name: "v1 blob over 2MiB",
checkType: abci.CheckTxType_New,
getTx: func() []byte {
signer := createSigner(t, kr, accs[10], encCfg.TxConfig, 11)
blob, err := share.NewV1Blob(share.RandomBlobNamespace(), bytes.Repeat([]byte{1}, 2_000_000), signer.Account(accs[10]).Address())
require.NoError(t, err)
blobTx, _, err := signer.CreatePayForBlobs(accs[10], []*share.Blob{blob}, opts...)
require.NoError(t, err)
return blobTx
},
expectedABCICode: blobtypes.ErrBlobsTooLarge.ABCICode(),
},
{
name: "v0 blob over 2MiB",
checkType: abci.CheckTxType_New,
getTx: func() []byte {
signer := createSigner(t, kr, accs[10], encCfg.TxConfig, 11)
blob, err := share.NewV0Blob(share.RandomBlobNamespace(), bytes.Repeat([]byte{1}, 2_000_000))
require.NoError(t, err)
blobTx, _, err := signer.CreatePayForBlobs(accs[10], []*share.Blob{blob}, opts...)
require.NoError(t, err)
return blobTx
},
},
}

for _, tt := range tests {
Expand Down
223 changes: 121 additions & 102 deletions app/test/prepare_proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package app_test

import (
"crypto/rand"
"fmt"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -51,7 +52,6 @@ func TestPrepareProposalPutsPFBsAtEnd(t *testing.T) {
accnts[:numBlobTxs],
infos[:numBlobTxs],
testfactory.Repeat([]*share.Blob{protoBlob}, numBlobTxs),
blobfactory.DefaultTxOpts()...,
)

normalTxs := testutil.SendTxsWithAccounts(
Expand Down Expand Up @@ -97,75 +97,74 @@ func TestPrepareProposalFiltering(t *testing.T) {

// create 3 single blob blobTxs that are signed with valid account numbers
// and sequences
blobTxs := blobfactory.ManyMultiBlobTx(
t,
encConf.TxConfig,
kr,
testutil.ChainID,
accounts[:3],
infos[:3],
blobfactory.NestedBlobs(
t,
testfactory.RandomBlobNamespaces(tmrand.NewRand(), 3),
[][]int{{100}, {1000}, {420}},
),
blobfactory.DefaultTxOpts()...,
)
// blobTxs := blobfactory.ManyMultiBlobTx(
// t,
// encConf.TxConfig,
// kr,
// testutil.ChainID,
// accounts[:3],
// infos[:3],
// blobfactory.NestedBlobs(
// t,
// testfactory.RandomBlobNamespaces(tmrand.NewRand(), 3),
// [][]int{{100}, {1000}, {420}},
// ),
// )

// create 3 MsgSend transactions that are signed with valid account numbers
// and sequences
sendTxs := coretypes.Txs(testutil.SendTxsWithAccounts(
t,
testApp,
encConf.TxConfig,
kr,
1000,
accounts[0],
accounts[len(accounts)-3:],
testutil.ChainID,
)).ToSliceOfBytes()

validTxs := func() [][]byte {
txs := make([][]byte, 0, len(sendTxs)+len(blobTxs))
txs = append(txs, blobTxs...)
txs = append(txs, sendTxs...)
return txs
}
// sendTxs := coretypes.Txs(testutil.SendTxsWithAccounts(
// t,
// testApp,
// encConf.TxConfig,
// kr,
// 1000,
// accounts[0],
// accounts[len(accounts)-3:],
// testutil.ChainID,
// )).ToSliceOfBytes()

// validTxs := func() [][]byte {
// txs := make([][]byte, 0, len(sendTxs)+len(blobTxs))
// txs = append(txs, blobTxs...)
// txs = append(txs, sendTxs...)
// return txs
// }

// create 3 MsgSend transactions that are using the same sequence as the
// first three blob transactions above
duplicateSeqSendTxs := coretypes.Txs(testutil.SendTxsWithAccounts(
t,
testApp,
encConf.TxConfig,
kr,
1000,
accounts[0],
accounts[:3],
testutil.ChainID,
)).ToSliceOfBytes()
// duplicateSeqSendTxs := coretypes.Txs(testutil.SendTxsWithAccounts(
// t,
// testApp,
// encConf.TxConfig,
// kr,
// 1000,
// accounts[0],
// accounts[:3],
// testutil.ChainID,
// )).ToSliceOfBytes()

// create a transaction with an account that doesn't exist. This will cause the increment nonce
nilAccount := "carmon san diego"
_, _, err := kr.NewMnemonic(nilAccount, keyring.English, "", "", hd.Secp256k1)
require.NoError(t, err)
noAccountTx := []byte(testutil.SendTxWithManualSequence(t, encConf.TxConfig, kr, nilAccount, accounts[0], 1000, "", 0, 6))
// 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]
// 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)
Expand All @@ -184,62 +183,76 @@ func TestPrepareProposalFiltering(t *testing.T) {
blobfactory.NestedBlobs(
t,
testfactory.RandomBlobNamespaces(tmrand.NewRand(), 3),
[][]int{{100}, {1000}, {420}},
[][]int{{699050}, {699050}, {699050}}, // over 2MiB, equal to 2 MiB, and under 2 MiB
),
user.SetMemo(largeString),
)

// 3 blobTxs where one is over MaxTxSize limit
// mixedSizeBlobTxs := blobfactory.ManyMultiBlobTx(
// t,
// encConf.TxConfig,
// kr,
// testutil.ChainID,
// accounts[:2],
// infos[:2],
// blobfactory.NestedBlobs(
// t,
// testfactory.RandomBlobNamespaces(tmrand.NewRand(), 2),
// [][]int{{2}, {2800}, {2097152}},
// ),
// )

type test struct {
name string
txs func() [][]byte
prunedTxs [][]byte
}

tests := []test{
{
name: "all valid txs, none are pruned",
txs: func() [][]byte { return validTxs() },
prunedTxs: [][]byte{},
},
{
// even though duplicateSeqSendTxs are getting appended to the end of the
// block, and we do not check the signatures of the standard txs,
// the blob txs still get pruned because we are separating the
// normal and blob txs, and checking/executing the normal txs first.
name: "duplicate sequence appended to the end of the block",
txs: func() [][]byte {
return append(validTxs(), duplicateSeqSendTxs...)
},
prunedTxs: blobTxs,
},
{
name: "duplicate sequence txs",
txs: func() [][]byte {
txs := make([][]byte, 0, len(sendTxs)+len(blobTxs)+len(duplicateSeqSendTxs))
// these should increment the nonce of the accounts that are
// signing the blobtxs, which should make those signatures
// invalid.
txs = append(txs, duplicateSeqSendTxs...)
txs = append(txs, blobTxs...)
txs = append(txs, sendTxs...)
return txs
},
prunedTxs: blobTxs,
},
{
name: "nil account panic catch",
txs: func() [][]byte {
return [][]byte{noAccountTx}
},
prunedTxs: [][]byte{noAccountTx},
},
{
name: "blob tx with too many shares",
txs: func() [][]byte {
return [][]byte{tooManyShareBtx}
},
prunedTxs: [][]byte{tooManyShareBtx},
},
// {
// name: "all valid txs, none are pruned",
// txs: func() [][]byte { return validTxs() },
// prunedTxs: [][]byte{},
// },
// {
// // even though duplicateSeqSendTxs are getting appended to the end of the
// // block, and we do not check the signatures of the standard txs,
// // the blob txs still get pruned because we are separating the
// // normal and blob txs, and checking/executing the normal txs first.
// name: "duplicate sequence appended to the end of the block",
// txs: func() [][]byte {
// return append(validTxs(), duplicateSeqSendTxs...)
// },
// prunedTxs: blobTxs,
// },
// {
// name: "duplicate sequence txs",
// txs: func() [][]byte {
// txs := make([][]byte, 0, len(sendTxs)+len(blobTxs)+len(duplicateSeqSendTxs))
// // these should increment the nonce of the accounts that are
// // signing the blobtxs, which should make those signatures
// // invalid.
// txs = append(txs, duplicateSeqSendTxs...)
// txs = append(txs, blobTxs...)
// txs = append(txs, sendTxs...)
// return txs
// },
// prunedTxs: blobTxs,
// },
// {
// name: "nil account panic catch",
// txs: func() [][]byte {
// return [][]byte{noAccountTx}
// },
// 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 {
Expand All @@ -260,10 +273,16 @@ func TestPrepareProposalFiltering(t *testing.T) {
Height: height,
Time: blockTime,
})
fmt.Println(
"EXPECTED TXS", len(tt.txs())-len(tt.prunedTxs),
"RESPONSE TXS", len(resp.BlockData.Txs),
)
// check that we have the expected number of transactions
require.Equal(t, len(tt.txs())-len(tt.prunedTxs), len(resp.BlockData.Txs))
// check that the expected txs were removed
for _, ptx := range tt.prunedTxs {
fmt.Println("PTX: ", len(ptx))
fmt.Println(len(tt.prunedTxs), "PRUNED TXS")
require.NotContains(t, resp.BlockData.Txs, ptx)
}
})
Expand Down
4 changes: 1 addition & 3 deletions app/test/process_proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ 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))
Expand All @@ -64,8 +63,7 @@ func TestProcessProposal(t *testing.T) {
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
Expand Down
16 changes: 15 additions & 1 deletion app/validate_txs.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package app

import (
"fmt"

"github.com/celestiaorg/celestia-app/v3/pkg/appconsts"
"github.com/celestiaorg/go-square/v2/tx"
"github.com/cosmos/cosmos-sdk/client"
Expand Down Expand Up @@ -34,7 +36,19 @@ func separateTxs(_ client.TxConfig, rawTxs [][]byte) ([][]byte, []*tx.BlobTx) {
//
// Side-effect: arranges all normal transactions before all blob transactions.
func FilterTxs(logger log.Logger, ctx sdk.Context, handler sdk.AnteHandler, txConfig client.TxConfig, txs [][]byte) [][]byte {
normalTxs, blobTxs := separateTxs(txConfig, txs)
// all transactions should be below the max tx size
maxTxSize := appconsts.MaxTxSize(ctx.BlockHeader().Version.App)
var txsBelowLimit [][]byte

Check failure on line 41 in app/validate_txs.go

View workflow job for this annotation

GitHub Actions / lint / golangci-lint

Consider pre-allocating `txsBelowLimit` (prealloc)
for _, tx := range txs {
if len(tx) > maxTxSize {
err := fmt.Sprintf("tx size %d bytes is larger than the application's configured threshold of %d bytes", len(tx), maxTxSize)
logger.Error(err)
continue
}
txsBelowLimit = append(txsBelowLimit, tx)
}

normalTxs, blobTxs := separateTxs(txConfig, txsBelowLimit)
normalTxs, ctx = filterStdTxs(logger, txConfig.TxDecoder(), ctx, handler, normalTxs)
blobTxs, _ = filterBlobTxs(logger, txConfig.TxDecoder(), ctx, handler, blobTxs)
return append(normalTxs, encodeBlobTxs(blobTxs)...)
Expand Down
3 changes: 1 addition & 2 deletions test/util/blobfactory/payforblob_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,14 +245,13 @@ func ManyMultiBlobTx(
accounts []string,
accInfos []AccountInfo,
blobs [][]*share.Blob,
opts ...user.TxOption,
) [][]byte {
t.Helper()
txs := make([][]byte, len(accounts))
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)
txs[i], _, err = signer.CreatePayForBlobs(acc, blobs[i], opts...)
txs[i], _, err = signer.CreatePayForBlobs(acc, blobs[i], DefaultTxOpts()...)
require.NoError(t, err)
}
return txs
Expand Down

0 comments on commit ab11446

Please sign in to comment.