Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: implement FillSquare method for the testnode #866

Merged
merged 8 commits into from
Oct 13, 2022
3 changes: 2 additions & 1 deletion testutil/testnode/full_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
26 changes: 25 additions & 1 deletion testutil/testnode/full_node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
rach-id marked this conversation as resolved.
Show resolved Hide resolved
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)
}
}
43 changes: 39 additions & 4 deletions testutil/testnode/node_interaction_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
}
Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[question] should this be flags.BroadcastMode per https://github.com/cosmos/cosmos-sdk/blob/7781cdb3d20bc7ebac017452897ce1e6ab3903ef/client/flags/flags.go#L114

Suggested change
// should be submitted async, sync, or block. (see flags.BroadcastModeSync). If
// should be submitted async, sync, or block (see flags.BroadcastMode). If

Note: block mode was removed in cosmos/cosmos-sdk#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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, blocking is just convenient atm and we will have to fix this in the future across the board.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Non Blocking][Question]
The (2 * squareSize) refers to the size that the transaction will take, aside from the message?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assume squareSize=16 so maxShareCount=256:

msgSize := (maxShareCount - (2 * squareSize)) * appconsts.SparseShareContentSize
msgSize := (256 - (2 * 16)) * appconsts.SparseShareContentSize
msgSize := (256 - 32) * appconsts.SparseShareContentSize
msgSize := (224) * appconsts.SparseShareContentSize

I don't think the transaction is guaranteed to take (2 * squareSize) but I think it is an approximation for the other shares in the square (transactions, intermediate state roots, evidence).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, what Rootul said. In order to get the square size we want, we just have to make sure that we're using some number of share in between maxShareCount and minShareCount

maxShareCount := squareSize * squareSize
minShareCount := maxShareCount / 2 

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your help 🙏

I think it is an approximation for the other shares in the square (transactions, intermediate state roots, evidence).

Another question, are ISRs and evidence currently added to blocks?

minShareCount := maxShareCount / 2

Why is that? is it to accommodate ISRs and evidence?

// 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))
}