Repository: celestiaorg/celestia-app

Issue:
Remove usage of `BroadcastModeBlock`
        [question] should this be `flags.BroadcastMode` per https://github.com/cosmos/cosmos-sdk/blob/7781cdb3d20bc7ebac017452897ce1e6ab3903ef/client/flags/flags.go#L114

```suggestion
// should be submitted async, sync, or block (see flags.BroadcastMode). If
```

Note: `block` mode was removed in https://github.com/cosmos/cosmos-sdk/pull/12659. Since celestia-app currently uses cosmos-sdk v0.46.0 we can still use `block` here but note to future selves that we'll need to remove it when we upgrade cosmos-sdk to a release with that change

_Originally posted by @rootulp in https://github.com/celestiaorg/celestia-app/pull/866#discussion_r994025733_

blocking until a tx is included in a block is useful for testing purposes. Since its being deprecated, we need to create a replacement and use it across the repo

Discussion:
@rach-id: The QGB also rely on it to wait for signatures to be included in blocks. We will need to update that also

Edit: no longer relevant with new design@rootulp: Done. We're on Cosmos SDK v0.50.x and we don't use BroadcastModeBlock.

Solution patch:
diff --git a/docs/architecture/adr-007-universal-share-prefix.md b/docs/architecture/adr-007-universal-share-prefix.md
index c31b6f27..f4d6b800 100644
--- a/docs/architecture/adr-007-universal-share-prefix.md
+++ b/docs/architecture/adr-007-universal-share-prefix.md
@@ -114,7 +114,7 @@ Types
 Logic
 
 1. Account for the new `InfoByte` in all share splitting and merging code.
-1. Encode a total data length varint into the first compact share of a sequence.
+1. Encode a total sequence length varint into the first compact share of a sequence.
 1. Introduce a new `ParseShares` API that can accept any type of share (compact or sparse).
 1. Introduce new block validity rules:
     1. All shares contain a share version that belongs to a list of supported versions (initially this list contains version `0`)
diff --git a/pkg/appconsts/appconsts.go b/pkg/appconsts/appconsts.go
index 108a1a2c..1014c065 100644
--- a/pkg/appconsts/appconsts.go
+++ b/pkg/appconsts/appconsts.go
@@ -114,24 +114,24 @@ var (
 	// defaults.
 	NameSpacedPaddedShareBytes = bytes.Repeat([]byte{0}, SparseShareContentSize)
 
-	// FirstCompactShareDataLengthBytes is the number of bytes reserved for the total
-	// data length that is stored in the first compact share of a reserved
-	// namespace. This value is the maximum number of bytes required to store
-	// the data length of a block that only contains compact shares of one type.
-	// For example, if a block contains only evidence then it could contain:
-	// MaxSquareSize * MaxSquareSize * ShareSize bytes of evidence.
+	// FirstCompactShareSequenceLengthBytes is the number of bytes reserved for the total
+	// sequence length that is stored in the first compact share of a sequence. This
+	// value is the maximum number of bytes required to store the sequence
+	// length of a block that only contains shares of one type. For example, if
+	// a block contains only evidence then it could contain: MaxSquareSize *
+	// MaxSquareSize * ShareSize bytes of evidence.
 	//
 	// Assuming MaxSquareSize is 128 and ShareSize is 256, this is 4194304 bytes
 	// of evidence. It takes 4 bytes to store a varint of 4194304.
 	//
 	// https://go.dev/play/p/MynwcDHQ_me
-	FirstCompactShareDataLengthBytes = numberOfBytesVarint(MaxSquareSize * MaxSquareSize * ShareSize)
+	FirstCompactShareSequenceLengthBytes = numberOfBytesVarint(MaxSquareSize * MaxSquareSize * ShareSize)
 
 	// FirstCompactShareContentSize is the number of bytes usable for data in
 	// the first compact share of a reserved namespace. This type of share
 	// contains less space for data than a ContinuationCompactShare because the
-	// first compact share includes a total data length varint.
-	FirstCompactShareContentSize = ContinuationCompactShareContentSize - FirstCompactShareDataLengthBytes
+	// first compact share includes a total sequence length varint.
+	FirstCompactShareContentSize = ContinuationCompactShareContentSize - FirstCompactShareSequenceLengthBytes
 
 	// SupportedShareVersions is a list of supported share versions.
 	SupportedShareVersions = []uint8{ShareVersion}
diff --git a/pkg/shares/parse_compact_shares.go b/pkg/shares/parse_compact_shares.go
index b988a633..9ea94787 100644
--- a/pkg/shares/parse_compact_shares.go
+++ b/pkg/shares/parse_compact_shares.go
@@ -59,7 +59,7 @@ func (ss *shareStack) resolve() ([][]byte, error) {
 	if !infoByte.IsMessageStart() {
 		return nil, errors.New("first share is not a message start")
 	}
-	err = ss.peel(ss.shares[0][appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareDataLengthBytes+appconsts.CompactShareReservedBytes:], true)
+	err = ss.peel(ss.shares[0][appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareSequenceLengthBytes+appconsts.CompactShareReservedBytes:], true)
 	return ss.data, err
 }
 
diff --git a/pkg/shares/shares.go b/pkg/shares/shares.go
index 23fbb919..e454f630 100644
--- a/pkg/shares/shares.go
+++ b/pkg/shares/shares.go
@@ -43,7 +43,7 @@ func (s Share) MessageLength() (uint64, error) {
 	if !infoByte.IsMessageStart() {
 		return 0, nil
 	}
-	if s.isCompactShare() || len(s) < appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareDataLengthBytes {
+	if s.isCompactShare() || len(s) < appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareSequenceLengthBytes {
 		panic(fmt.Sprintf("compact share %s is too short to contain message length", s))
 	}
 	reader := bytes.NewReader(s[appconsts.NamespaceSize+appconsts.ShareInfoBytes:])
diff --git a/pkg/shares/split_compact_shares.go b/pkg/shares/split_compact_shares.go
index 72211985..41b5ff29 100644
--- a/pkg/shares/split_compact_shares.go
+++ b/pkg/shares/split_compact_shares.go
@@ -28,7 +28,7 @@ func NewCompactShareSplitter(ns namespace.ID, version uint8) *CompactShareSplitt
 	if err != nil {
 		panic(err)
 	}
-	placeholderDataLength := make([]byte, appconsts.FirstCompactShareDataLengthBytes)
+	placeholderDataLength := make([]byte, appconsts.FirstCompactShareSequenceLengthBytes)
 	placeholderReservedBytes := make([]byte, appconsts.CompactShareReservedBytes)
 
 	pendingShare = append(pendingShare, ns...)
@@ -136,9 +136,9 @@ func (css *CompactShareSplitter) dataLengthVarint(bytesOfPadding int) []byte {
 	}
 
 	// declare and initialize the data length
-	dataLengthVarint := make([]byte, appconsts.FirstCompactShareDataLengthBytes)
+	dataLengthVarint := make([]byte, appconsts.FirstCompactShareSequenceLengthBytes)
 	binary.PutUvarint(dataLengthVarint, css.dataLength(bytesOfPadding))
-	zeroPadIfNecessary(dataLengthVarint, appconsts.FirstCompactShareDataLengthBytes)
+	zeroPadIfNecessary(dataLengthVarint, appconsts.FirstCompactShareSequenceLengthBytes)
 
 	return dataLengthVarint
 }
@@ -150,7 +150,7 @@ func (css *CompactShareSplitter) writeDataLengthVarintToFirstShare(dataLengthVar
 
 	// write the data length varint to the first share
 	firstShare := css.shares[0]
-	for i := 0; i < appconsts.FirstCompactShareDataLengthBytes; i++ {
+	for i := 0; i < appconsts.FirstCompactShareSequenceLengthBytes; i++ {
 		firstShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes+i] = dataLengthVarint[i]
 	}
 
@@ -173,7 +173,7 @@ func (css *CompactShareSplitter) maybeWriteReservedByteToPendingShare() {
 
 	// write the location of next unit to the reserved byte of the pending share
 	if css.isPendingShareTheFirstShare() {
-		css.pendingShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareDataLengthBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareContentSize+appconsts.CompactShareReservedBytes][0] = byte(locationOfNextUnit)
+		css.pendingShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareSequenceLengthBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareContentSize+appconsts.CompactShareReservedBytes][0] = byte(locationOfNextUnit)
 	} else {
 		css.pendingShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.CompactShareReservedBytes][0] = byte(locationOfNextUnit)
 	}
@@ -184,7 +184,7 @@ func (css *CompactShareSplitter) isEmptyReservedByte() bool {
 	var reservedByte byte
 
 	if css.isPendingShareTheFirstShare() {
-		reservedByte = css.pendingShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareDataLengthBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareContentSize+appconsts.CompactShareReservedBytes][0]
+		reservedByte = css.pendingShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareSequenceLengthBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareContentSize+appconsts.CompactShareReservedBytes][0]
 	} else {
 		reservedByte = css.pendingShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.CompactShareReservedBytes][0]
 	}
@@ -213,7 +213,7 @@ func (css *CompactShareSplitter) dataLength(bytesOfPadding int) uint64 {
 // isEmptyPendingShare returns true if the pending share is empty, false otherwise.
 func (css *CompactShareSplitter) isEmptyPendingShare() bool {
 	if css.isPendingShareTheFirstShare() {
-		return len(css.pendingShare) == appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareDataLengthBytes+appconsts.CompactShareReservedBytes
+		return len(css.pendingShare) == appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareSequenceLengthBytes+appconsts.CompactShareReservedBytes
 	}
 	return len(css.pendingShare) == appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.CompactShareReservedBytes
 }



Test patch:
diff --git a/pkg/prove/proof_test.go b/pkg/prove/proof_test.go
index eb353aab..bdcaf061 100644
--- a/pkg/prove/proof_test.go
+++ b/pkg/prove/proof_test.go
@@ -231,8 +231,8 @@ func TestTxShareIndex(t *testing.T) {
 func stripCompactShares(compactShares []shares.Share, start uint64, end uint64) (result []byte) {
 	for i := start; i <= end; i++ {
 		if i == 0 {
-			// the first compact share includes a total data length varint
-			result = append(result, compactShares[i][appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareDataLengthBytes+appconsts.CompactShareReservedBytes:]...)
+			// the first compact share includes a total sequence length varint
+			result = append(result, compactShares[i][appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareSequenceLengthBytes+appconsts.CompactShareReservedBytes:]...)
 		} else {
 			result = append(result, compactShares[i][appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.CompactShareReservedBytes:]...)
 		}
diff --git a/testutil/testnode/full_node.go b/testutil/testnode/full_node.go
index 6320204e..88199b3e 100644
--- a/testutil/testnode/full_node.go
+++ b/testutil/testnode/full_node.go
@@ -131,6 +131,7 @@ func DefaultParams() *tmproto.ConsensusParams {
 
 func DefaultTendermintConfig() *config.Config {
 	tmCfg := config.DefaultConfig()
-	tmCfg.Consensus.TimeoutCommit = time.Millisecond * 70
+	tmCfg.Consensus.TimeoutCommit = time.Millisecond * 100
+	tmCfg.Mempool.MaxTxBytes = 22020096 // 21MB
 	return tmCfg
 }
diff --git a/testutil/testnode/full_node_test.go b/testutil/testnode/full_node_test.go
index 376d64ee..c6462077 100644
--- a/testutil/testnode/full_node_test.go
+++ b/testutil/testnode/full_node_test.go
@@ -4,8 +4,12 @@ import (
 	"context"
 	"testing"
 
+	"github.com/celestiaorg/celestia-app/pkg/appconsts"
+	"github.com/celestiaorg/celestia-app/testutil"
 	"github.com/celestiaorg/celestia-app/testutil/namespace"
+	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/stretchr/testify/suite"
+	abci "github.com/tendermint/tendermint/abci/types"
 	tmrand "github.com/tendermint/tendermint/libs/rand"
 	coretypes "github.com/tendermint/tendermint/rpc/core/types"
 )
@@ -75,10 +79,30 @@ func (s *IntegrationTestSuite) Test_Liveness() {
 
 func (s *IntegrationTestSuite) Test_PostData() {
 	require := s.Require()
-	_, err := s.cctx.PostData(s.accounts[0], namespace.RandomMessageNamespace(), tmrand.Bytes(100000))
+	_, err := s.cctx.PostData(s.accounts[0], flags.BroadcastBlock, namespace.RandomMessageNamespace(), tmrand.Bytes(100000))
 	require.NoError(err)
 }
 
 func TestIntegrationTestSuite(t *testing.T) {
 	suite.Run(t, new(IntegrationTestSuite))
 }
+
+func (s *IntegrationTestSuite) Test_FillBlock() {
+	require := s.Require()
+
+	for squareSize := 2; squareSize < appconsts.MaxSquareSize; squareSize *= 2 {
+		resp, err := s.cctx.FillBlock(squareSize, s.accounts, flags.BroadcastAsync)
+		require.NoError(err)
+
+		err = s.cctx.WaitForNextBlock()
+		require.NoError(err)
+
+		res, err := testutil.QueryWithOutProof(s.cctx.Context, resp.TxHash)
+		require.NoError(err)
+		require.Equal(abci.CodeTypeOK, res.TxResult.Code)
+
+		b, err := s.cctx.Client.Block(context.TODO(), &res.Height)
+		require.NoError(err)
+		require.Equal(uint64(squareSize), b.Block.OriginalSquareSize)
+	}
+}
diff --git a/testutil/testnode/node_interaction_api.go b/testutil/testnode/node_interaction_api.go
index fdf1126b..04f4fa72 100644
--- a/testutil/testnode/node_interaction_api.go
+++ b/testutil/testnode/node_interaction_api.go
@@ -6,11 +6,15 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/celestiaorg/celestia-app/pkg/appconsts"
+	"github.com/celestiaorg/celestia-app/testutil/namespace"
 	"github.com/celestiaorg/celestia-app/x/payment"
 	"github.com/celestiaorg/celestia-app/x/payment/types"
 	"github.com/cosmos/cosmos-sdk/client"
+	"github.com/cosmos/cosmos-sdk/client/flags"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	abci "github.com/tendermint/tendermint/abci/types"
+	tmrand "github.com/tendermint/tendermint/libs/rand"
 )
 
 type Context struct {
@@ -86,7 +90,7 @@ func (c *Context) WaitForNextBlock() error {
 // namespace. This function blocks until the PFD has been included in a block
 // and returns an error if the transaction is invalid or is rejected by the
 // mempool.
-func (c *Context) PostData(account string, ns, msg []byte) (*sdk.TxResponse, error) {
+func (c *Context) PostData(account, broadcastMode string, ns, msg []byte) (*sdk.TxResponse, error) {
 	opts := []types.TxBuilderOption{
 		types.SetGasLimit(100000000000000),
 	}
@@ -130,14 +134,45 @@ func (c *Context) PostData(account string, ns, msg []byte) (*sdk.TxResponse, err
 	if err != nil {
 		return nil, err
 	}
-
-	res, err := c.BroadcastTxCommit(rawTx)
+	var res *sdk.TxResponse
+	switch broadcastMode {
+	case flags.BroadcastSync:
+		res, err = c.BroadcastTxSync(rawTx)
+	case flags.BroadcastAsync:
+		res, err = c.BroadcastTxAsync(rawTx)
+	case flags.BroadcastBlock:
+		res, err = c.BroadcastTxCommit(rawTx)
+	default:
+		return nil, fmt.Errorf("unsupported broadcast mode %s; supported modes: sync, async, block", c.BroadcastMode)
+	}
 	if err != nil {
 		return nil, err
 	}
 	if res.Code != abci.CodeTypeOK {
-		return nil, fmt.Errorf("failure to broadcast tx sync: %s", res.RawLog)
+		return res, fmt.Errorf("failure to broadcast tx sync: %s", res.RawLog)
 	}
 
 	return res, nil
 }
+
+// FillBlock creates and submits a single transaction that is large enough to
+// create a square of the desired size. broadcast mode indicates if the tx
+// should be submitted async, sync, or block. (see flags.BroadcastModeSync). If
+// broadcast mode is the string zero value, then it will be set to block. This
+// function does not perform checks on the passed squaresize arg, and only works
+// with squaresize >= 2. TODO: perform checks (is a power of 2 and is > 2) on
+// the passed squaresize arg
+func (c *Context) FillBlock(squareSize int, accounts []string, broadcastMode string) (*sdk.TxResponse, error) {
+	if broadcastMode == "" {
+		broadcastMode = flags.BroadcastBlock
+	}
+	maxShareCount := squareSize * squareSize
+	// we use a formula to guarantee that the tx is the exact size needed to force a specific square size.
+	msgSize := (maxShareCount - (2 * squareSize)) * appconsts.SparseShareContentSize
+	// this last patch allows for the formula above to work on a square size of
+	// 2.
+	if msgSize < 1 {
+		msgSize = 1
+	}
+	return c.PostData(accounts[0], broadcastMode, namespace.RandomMessageNamespace(), tmrand.Bytes(msgSize))
+}