test!: attempt to resolve flakes

app/test/integration_test.go

@@ -36,6 +36,11 @@ import (
 	coretypes "github.com/tendermint/tendermint/types"
 )
 
+const (
+	Kibibyte = 1024
+	Mebibyte = 1_048_576
+)
+
 func TestIntegrationTestSuite(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping app/test/integration_test in short mode.")
@@ -53,12 +58,7 @@ type IntegrationTestSuite struct {
 
 func (s *IntegrationTestSuite) SetupSuite() {
 	t := s.T()
-
-	numAccounts := 142
-	s.accounts = make([]string, numAccounts)
-	for i := 0; i < numAccounts; i++ {
-		s.accounts[i] = tmrand.Str(20)
-	}
+	s.accounts = testnode.RandomAccounts(142)
 
 	cfg := testnode.DefaultConfig().WithFundedAccounts(s.accounts...)
 
@@ -79,47 +79,41 @@ func (s *IntegrationTestSuite) SetupSuite() {
 func (s *IntegrationTestSuite) TestMaxBlockSize() {
 	t := s.T()
 
-	// tendermint's default tx size limit is 1Mb, so we get close to that
-	equallySized1MbTxGen := func(c client.Context) []coretypes.Tx {
+	singleBlobTxGen := func(c client.Context) []coretypes.Tx {
 		return blobfactory.RandBlobTxsWithAccounts(
 			s.ecfg,
 			tmrand.NewRand(),
 			s.cctx.Keyring,
 			c.GRPCClient,
-			950000,
+			600*Kibibyte,
 			1,
 			false,
 			s.accounts[:20],
 		)
 	}
 
-	// Tendermint's default tx size limit is 1 MiB, so we get close to that by
-	// generating transactions of size 600 KB because 3 blobs per transaction *
-	// 200,000 bytes each = 600,000 total bytes = 600 KB per transaction.
-	randMultiBlob1MbTxGen := func(c client.Context) []coretypes.Tx {
+	// This tx generator generates txs that contain 3 blobs each of 200 KiB so
+	// 600 KiB total per transaction.
+	multiBlobTxGen := func(c client.Context) []coretypes.Tx {
 		return blobfactory.RandBlobTxsWithAccounts(
 			s.ecfg,
 			tmrand.NewRand(),
 			s.cctx.Keyring,
 			c.GRPCClient,
-			200000, // 200 KB
+			200*Kibibyte,
 			3,
 			false,
 			s.accounts[20:40],
 		)
 	}
 
-	// Generate 80 randomly sized txs (max size == 50 KB). Generate these
-	// transactions using some of the same accounts as the previous generator to
-	// ensure that the sequence number is being utilized correctly in blob
-	// txs
-	randoTxGen := func(c client.Context) []coretypes.Tx {
+	randomTxGen := func(c client.Context) []coretypes.Tx {
 		return blobfactory.RandBlobTxsWithAccounts(
 			s.ecfg,
 			tmrand.NewRand(),
 			s.cctx.Keyring,
 			c.GRPCClient,
-			50000,
+			50*Kibibyte,
 			8,
 			true,
 			s.accounts[40:120],
@@ -130,27 +124,21 @@ func (s *IntegrationTestSuite) TestMaxBlockSize() {
 		name        string
 		txGenerator func(clientCtx client.Context) []coretypes.Tx
 	}
-
 	tests := []test{
-		{
-			"20 1Mb txs",
-			equallySized1MbTxGen,
-		},
-		{
-			"20 1Mb multiblob txs",
-			randMultiBlob1MbTxGen,
-		},
-		{
-			"80 random txs",
-			randoTxGen,
-		},
+		{"singleBlobTxGen", singleBlobTxGen},
+		{"multiBlobTxGen", multiBlobTxGen},
+		{"randomTxGen", randomTxGen},
 	}
 	for _, tc := range tests {
 		s.Run(tc.name, func() {
 			txs := tc.txGenerator(s.cctx.Context)
 			hashes := make([]string, len(txs))
 
 			for i, tx := range txs {
+				// The default CometBFT mempool MaxTxBytes is 1 MiB so the generators in
+				// this test must create transactions that are smaller than that.
+				require.LessOrEqual(t, len(tx), 1*mebibyte)
+
 				res, err := s.cctx.Context.BroadcastTxSync(tx)
 				require.NoError(t, err)
 				assert.Equal(t, abci.CodeTypeOK, res.Code, res.RawLog)
@@ -166,10 +154,7 @@ func (s *IntegrationTestSuite) TestMaxBlockSize() {
 			for _, hash := range hashes {
 				resp, err := testnode.QueryTx(s.cctx.Context, hash, true)
 				require.NoError(t, err)
-				assert.NotNil(t, resp)
-				if resp == nil {
-					continue
-				}
+				require.NotNil(t, resp)
 				require.Equal(t, abci.CodeTypeOK, resp.TxResult.Code, resp.TxResult.Log)
 				heights[resp.Height]++
 				// ensure that some gas was used
@@ -191,13 +176,8 @@ func (s *IntegrationTestSuite) TestMaxBlockSize() {
 				require.LessOrEqual(t, size, uint64(appconsts.DefaultGovMaxSquareSize))
 				require.GreaterOrEqual(t, size, uint64(appconsts.MinSquareSize))
 
-				// assert that the app version is correctly set
-				// FIXME: This should return the latest version but tendermint v0.34.x doesn't copy
-				// over the version when converting from proto so it disappears
-				require.EqualValues(t, 0, blockRes.Block.Header.Version.App)
-
 				sizes = append(sizes, size)
-				ExtendBlobTest(t, blockRes.Block)
+				ExtendBlockTest(t, blockRes.Block)
 			}
 			// ensure that at least one of the blocks used the max square size
 			assert.Contains(t, sizes, uint64(appconsts.DefaultGovMaxSquareSize))
@@ -210,10 +190,6 @@ func (s *IntegrationTestSuite) TestSubmitPayForBlob() {
 	t := s.T()
 	ns1 := appns.MustNewV0(bytes.Repeat([]byte{1}, appns.NamespaceVersionZeroIDSize))
 
-	mustNewBlob := func(ns appns.Namespace, data []byte, shareVersion uint8) *blob.Blob {
-		return blob.New(ns, data, shareVersion)
-	}
-
 	type test struct {
 		name string
 		blob *blob.Blob
@@ -223,31 +199,31 @@ func (s *IntegrationTestSuite) TestSubmitPayForBlob() {
 	tests := []test{
 		{
 			"small random typical",
-			mustNewBlob(ns1, tmrand.Bytes(3000), appconsts.ShareVersionZero),
+			blob.New(ns1, tmrand.Bytes(3000), appconsts.ShareVersionZero),
 			[]user.TxOption{
 				user.SetFeeAmount(sdk.NewCoins(sdk.NewCoin(app.BondDenom, sdk.NewInt(1)))),
 				user.SetGasLimit(1_000_000_000),
 			},
 		},
 		{
 			"large random typical",
-			mustNewBlob(ns1, tmrand.Bytes(350000), appconsts.ShareVersionZero),
+			blob.New(ns1, tmrand.Bytes(350000), appconsts.ShareVersionZero),
 			[]user.TxOption{
 				user.SetFeeAmount(sdk.NewCoins(sdk.NewCoin(app.BondDenom, sdk.NewInt(10)))),
 				user.SetGasLimit(1_000_000_000),
 			},
 		},
 		{
 			"medium random with memo",
-			mustNewBlob(ns1, tmrand.Bytes(100000), appconsts.ShareVersionZero),
+			blob.New(ns1, tmrand.Bytes(100000), appconsts.ShareVersionZero),
 			[]user.TxOption{
 				user.SetMemo("lol I could stick the rollup block here if I wanted to"),
 				user.SetGasLimit(1_000_000_000),
 			},
 		},
 		{
 			"medium random with timeout height",
-			mustNewBlob(ns1, tmrand.Bytes(100000), appconsts.ShareVersionZero),
+			blob.New(ns1, tmrand.Bytes(100000), appconsts.ShareVersionZero),
 			[]user.TxOption{
 				user.SetTimeoutHeight(10000),
 				user.SetGasLimit(1_000_000_000),
@@ -296,13 +272,12 @@ func (s *IntegrationTestSuite) TestUnwrappedPFBRejection() {
 func (s *IntegrationTestSuite) TestShareInclusionProof() {
 	t := s.T()
 
-	// generate 100 randomly sized txs (max size == 100kb)
 	txs := blobfactory.RandBlobTxsWithAccounts(
 		s.ecfg,
 		tmrand.NewRand(),
 		s.cctx.Keyring,
 		s.cctx.GRPCClient,
-		100000,
+		100*Kibibyte,
 		1,
 		true,
 		s.accounts[120:140],
@@ -352,9 +327,9 @@ func (s *IntegrationTestSuite) TestShareInclusionProof() {
 	}
 }
 
-// ExtendBlobTest re-extends the block and compares the data roots to ensure
+// ExtendBlockTest re-extends the block and compares the data roots to ensure
 // that the public functions for extending the block are working correctly.
-func ExtendBlobTest(t *testing.T, block *coretypes.Block) {
+func ExtendBlockTest(t *testing.T, block *coretypes.Block) {
 	eds, err := app.ExtendBlock(block.Data, block.Header.Version.App)
 	require.NoError(t, err)
 	dah, err := da.NewDataAvailabilityHeader(eds)
@@ -374,7 +349,7 @@ func (s *IntegrationTestSuite) TestEmptyBlock() {
 		blockRes, err := s.cctx.Client.Block(s.cctx.GoContext(), &h)
 		require.NoError(t, err)
 		require.True(t, app.IsEmptyBlock(blockRes.Block.Data, blockRes.Block.Header.Version.App))
-		ExtendBlobTest(t, blockRes.Block)
+		ExtendBlockTest(t, blockRes.Block)
 	}
 }
 
@@ -395,35 +370,35 @@ func (s *IntegrationTestSuite) TestSubmitPayForBlob_blobSizes() {
 	}
 	testCases := []testCase{
 		{
-			name:           "1,000 byte blob",
-			blob:           mustNewBlob(1_000),
+			name:           "1 kibibyte blob",
+			blob:           newBlobWithSize(Kibibyte),
 			txResponseCode: abci.CodeTypeOK,
 		},
 		{
-			name:           "10,000 byte blob",
-			blob:           mustNewBlob(10_000),
+			name:           "10 kibibyte blob",
+			blob:           newBlobWithSize(10 * Kibibyte),
 			txResponseCode: abci.CodeTypeOK,
 		},
 		{
-			name:           "100,000 byte blob",
-			blob:           mustNewBlob(100_000),
+			name:           "100 kibibyte blob",
+			blob:           newBlobWithSize(100 * Kibibyte),
 			txResponseCode: abci.CodeTypeOK,
 		},
 		{
-			name:           "1,000,000 byte blob",
-			blob:           mustNewBlob(1_000_000),
+			name:           "1 mebibyte blob",
+			blob:           newBlobWithSize(1 * mebibyte),
 			txResponseCode: abci.CodeTypeOK,
 		},
 		{
 			name:           "10,000,000 byte blob returns err tx too large",
-			blob:           mustNewBlob(10_000_000),
+			blob:           newBlobWithSize(10_000_000),
 			txResponseCode: errors.ErrTxTooLarge.ABCICode(),
 		},
 	}
 
 	for _, tc := range testCases {
 		s.Run(tc.name, func() {
-			subCtx, cancel := context.WithTimeout(s.cctx.GoContext(), 30*time.Second)
+			subCtx, cancel := context.WithTimeout(s.cctx.GoContext(), 60*time.Second)
 			defer cancel()
 			res, err := signer.SubmitPayForBlob(subCtx, []*blob.Blob{tc.blob}, user.SetGasLimit(1_000_000_000))
 			if tc.txResponseCode == abci.CodeTypeOK {
@@ -437,8 +412,8 @@ func (s *IntegrationTestSuite) TestSubmitPayForBlob_blobSizes() {
 	}
 }
 
-func mustNewBlob(blobSize int) *blob.Blob {
-	ns1 := appns.MustNewV0(bytes.Repeat([]byte{1}, appns.NamespaceVersionZeroIDSize))
-	data := tmrand.Bytes(blobSize)
-	return blob.New(ns1, data, appconsts.ShareVersionZero)
+func newBlobWithSize(size int) *blob.Blob {
+	ns := appns.MustNewV0(bytes.Repeat([]byte{1}, appns.NamespaceVersionZeroIDSize))
+	data := tmrand.Bytes(size)
+	return blob.New(ns, data, appconsts.ShareVersionZero)
 }

app/test/max_total_blob_size_test.go

@@ -7,17 +7,13 @@ 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/blob"
-	"github.com/celestiaorg/celestia-app/pkg/square"
 	"github.com/celestiaorg/celestia-app/pkg/user"
 	"github.com/celestiaorg/celestia-app/test/util/testfactory"
 	"github.com/celestiaorg/celestia-app/test/util/testnode"
 	blobtypes "github.com/celestiaorg/celestia-app/x/blob/types"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
-	abci "github.com/tendermint/tendermint/abci/types"
 )
 
 const (
@@ -63,9 +59,9 @@ func (s *MaxTotalBlobSizeSuite) SetupSuite() {
 	require.NoError(t, cctx.WaitForNextBlock())
 }
 
-// TestSubmitPayForBlob_blobSizes verifies the tx response ABCI code when
-// SubmitPayForBlob is invoked with different blob sizes.
-func (s *MaxTotalBlobSizeSuite) TestSubmitPayForBlob_blobSizes() {
+// TestErrTotalBlobSizeTooLarge verifies that submitting a 2 MiB blob hits
+// ErrTotalBlobSizeTooLarge.
+func (s *MaxTotalBlobSizeSuite) TestErrTotalBlobSizeTooLarge() {
 	t := s.T()
 
 	type testCase struct {
@@ -75,19 +71,9 @@ func (s *MaxTotalBlobSizeSuite) TestSubmitPayForBlob_blobSizes() {
 		want uint32
 	}
 	testCases := []testCase{
-		{
-			name: "1 byte blob",
-			blob: mustNewBlob(1),
-			want: abci.CodeTypeOK,
-		},
-		{
-			name: "1 mebibyte blob",
-			blob: mustNewBlob(mebibyte),
-			want: abci.CodeTypeOK,
-		},
 		{
 			name: "2 mebibyte blob",
-			blob: mustNewBlob(2 * mebibyte),
+			blob: newBlobWithSize(2 * mebibyte),
 			want: blobtypes.ErrTotalBlobSizeTooLarge.ABCICode(),
 		},
 	}
@@ -105,16 +91,6 @@ func (s *MaxTotalBlobSizeSuite) TestSubmitPayForBlob_blobSizes() {
 			require.NoError(t, err)
 			require.NotNil(t, res)
 			require.Equal(t, tc.want, res.Code, res.Logs)
-
-			sq, err := square.Construct([][]byte{blobTx}, appconsts.LatestVersion, squareSize)
-			if tc.want == abci.CodeTypeOK {
-				// verify that if the tx was accepted, the blob can fit in a square
-				assert.NoError(t, err)
-				assert.False(t, sq.IsEmpty())
-			} else {
-				// verify that if the tx was rejected, the blob can not fit in a square
-				assert.Error(t, err)
-			}
 		})
 	}
 }

app/test/square_size_test.go

@@ -60,8 +60,7 @@ func (s *SquareSizeIntegrationTest) SetupSuite() {
 
 // TestSquareSizeUpperBound sets the app's params to specific sizes, then fills the
 // block with spam txs to measure that the desired max is getting hit
-// The "_Flaky" suffix indicates that the test may fail non-deterministically especially when executed in CI.
-func (s *SquareSizeIntegrationTest) TestSquareSizeUpperBound_Flaky() {
+func (s *SquareSizeIntegrationTest) TestSquareSizeUpperBound() {
 	t := s.T()
 
 	type test struct {

pkg/square/square_test.go

@@ -134,12 +134,10 @@ func generateBlobTxsWithNamespaces(t *testing.T, namespaces []ns.Namespace, blob
 	)
 }
 
-// The "_Flaky" suffix indicates that the test may fail non-deterministically especially when executed in CI.
-func TestSquareBlobShareRange_Flaky(t *testing.T) {
-	rand := tmrand.NewRand()
+func TestSquareBlobShareRange(t *testing.T) {
 	signer, err := testnode.NewOfflineSigner()
 	require.NoError(t, err)
-	txs := blobfactory.RandBlobTxsRandomlySized(signer, rand, 10, 1000, 10).ToSliceOfBytes()
+	txs := blobfactory.RandBlobTxsRandomlySized(signer, tmrand.NewRand(), 10, 1000, 10).ToSliceOfBytes()
 
 	builder, err := square.NewBuilder(appconsts.DefaultSquareSizeUpperBound, appconsts.LatestVersion, txs...)
 	require.NoError(t, err)
@@ -153,6 +151,7 @@ func TestSquareBlobShareRange_Flaky(t *testing.T) {
 		for blobIdx := range blobTx.Blobs {
 			shareRange, err := square.BlobShareRange(txs, pfbIdx, blobIdx, appconsts.LatestVersion)
 			require.NoError(t, err)
+			require.LessOrEqual(t, shareRange.End, len(dataSquare))
 			blobShares := dataSquare[shareRange.Start:shareRange.End]
 			blobSharesBytes, err := rawData(blobShares)
 			require.NoError(t, err)