From 30e907f219a4966081204f7f979fc15e11398d99 Mon Sep 17 00:00:00 2001 From: Rootul P Date: Mon, 4 Dec 2023 10:30:24 -0500 Subject: [PATCH] test!: refactor testnode (#2871) Miscelaneous refactors to testnode while working on https://github.com/celestiaorg/celestia-app/issues/2858 but doesn't actually close that issue. This PR adds more logging to help debug the second scenario alluded to in the original post. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- app/test/max_total_blob_size_test.go | 2 +- test/cmd/txsim/cli_test.go | 2 +- test/util/testnode/app_options.go | 27 +++++++++ test/util/testnode/config.go | 52 +++++----------- test/util/testnode/full_node.go | 71 +++------------------- test/util/testnode/full_node_test.go | 4 -- test/util/testnode/network.go | 89 ++++++++++++++++++++++++++++ test/util/testnode/rpc_client.go | 20 +++++-- x/mint/test/mint_test.go | 2 +- 9 files changed, 155 insertions(+), 114 deletions(-) create mode 100644 test/util/testnode/app_options.go create mode 100644 test/util/testnode/network.go diff --git a/app/test/max_total_blob_size_test.go b/app/test/max_total_blob_size_test.go index 9c9bafcef7..7c08bfce9f 100644 --- a/app/test/max_total_blob_size_test.go +++ b/app/test/max_total_blob_size_test.go @@ -39,7 +39,7 @@ func (s *MaxTotalBlobSizeSuite) SetupSuite() { tmConfig := testnode.DefaultTendermintConfig() tmConfig.Mempool.MaxTxBytes = 10 * mebibyte - cParams := testnode.DefaultParams() + cParams := testnode.DefaultConsensusParams() cParams.Block.MaxBytes = 10 * mebibyte cfg := testnode.DefaultConfig(). diff --git a/test/cmd/txsim/cli_test.go b/test/cmd/txsim/cli_test.go index 491b9c850e..6b17b3824a 100644 --- a/test/cmd/txsim/cli_test.go +++ b/test/cmd/txsim/cli_test.go @@ -61,7 +61,7 @@ func setup(t testing.TB) (keyring.Keyring, string, string) { cdc := encoding.MakeConfig(app.ModuleEncodingRegisters...).Codec // set the consensus params to allow for the max square size - cparams := testnode.DefaultParams() + cparams := testnode.DefaultConsensusParams() cparams.Block.MaxBytes = int64(appconsts.DefaultSquareSizeUpperBound*appconsts.DefaultSquareSizeUpperBound) * appconsts.ContinuationSparseShareContentSize cfg := testnode.DefaultConfig(). diff --git a/test/util/testnode/app_options.go b/test/util/testnode/app_options.go new file mode 100644 index 0000000000..8774b0cf48 --- /dev/null +++ b/test/util/testnode/app_options.go @@ -0,0 +1,27 @@ +package testnode + +import ( + pruningtypes "github.com/cosmos/cosmos-sdk/pruning/types" + "github.com/cosmos/cosmos-sdk/server" +) + +type KVAppOptions struct { + options map[string]interface{} +} + +// Get returns the option value for the given option key. +func (ao *KVAppOptions) Get(option string) interface{} { + return ao.options[option] +} + +// Set sets a key-value app option. +func (ao *KVAppOptions) Set(option string, value interface{}) { + ao.options[option] = value +} + +// DefaultAppOptions returns the default application options. +func DefaultAppOptions() *KVAppOptions { + opts := &KVAppOptions{options: make(map[string]interface{})} + opts.Set(server.FlagPruning, pruningtypes.PruningOptionNothing) + return opts +} diff --git a/test/util/testnode/config.go b/test/util/testnode/config.go index bb04f44ee4..0f8f1f4504 100644 --- a/test/util/testnode/config.go +++ b/test/util/testnode/config.go @@ -6,8 +6,6 @@ import ( "github.com/celestiaorg/celestia-app/cmd/celestia-appd/cmd" "github.com/celestiaorg/celestia-app/pkg/appconsts" "github.com/celestiaorg/celestia-app/test/util/genesis" - pruningtypes "github.com/cosmos/cosmos-sdk/pruning/types" - "github.com/cosmos/cosmos-sdk/server" srvconfig "github.com/cosmos/cosmos-sdk/server/config" srvtypes "github.com/cosmos/cosmos-sdk/server/types" tmconfig "github.com/tendermint/tendermint/config" @@ -17,6 +15,8 @@ import ( ) const ( + kibibyte = 1024 // bytes + mebibyte = 1_048_576 // bytes DefaultValidatorAccountName = "validator" ) @@ -70,7 +70,7 @@ func (c *Config) WithSupressLogs(sl bool) *Config { return c } -// WithTimeoutCommit sets the CommitTimeout and returns the Config. +// WithTimeoutCommit sets the TimeoutCommit and returns the Config. func (c *Config) WithTimeoutCommit(d time.Duration) *Config { c.TmConfig.Consensus.TimeoutCommit = d return c @@ -108,16 +108,15 @@ func (c *Config) WithConsensusParams(params *tmproto.ConsensusParams) *Config { return c } +// DefaultConfig returns the default configuration of a test node. func DefaultConfig() *Config { - tmcfg := DefaultTendermintConfig() - tmcfg.Consensus.TimeoutCommit = 1 * time.Millisecond cfg := &Config{} return cfg. WithGenesis( genesis.NewDefaultGenesis(). WithChainID(tmrand.Str(6)). WithGenesisTime(time.Now()). - WithConsensusParams(DefaultParams()). + WithConsensusParams(DefaultConsensusParams()). WithModifiers(). WithValidators(genesis.NewDefaultValidator(DefaultValidatorAccountName)), ). @@ -128,28 +127,7 @@ func DefaultConfig() *Config { WithSupressLogs(true) } -type KVAppOptions struct { - options map[string]interface{} -} - -// Get implements AppOptions -func (ao *KVAppOptions) Get(o string) interface{} { - return ao.options[o] -} - -// Set adds an option to the KVAppOptions -func (ao *KVAppOptions) Set(o string, v interface{}) { - ao.options[o] = v -} - -// DefaultAppOptions returns the default application options. -func DefaultAppOptions() *KVAppOptions { - opts := &KVAppOptions{options: make(map[string]interface{})} - opts.Set(server.FlagPruning, pruningtypes.PruningOptionNothing) - return opts -} - -func DefaultParams() *tmproto.ConsensusParams { +func DefaultConsensusParams() *tmproto.ConsensusParams { cparams := types.DefaultConsensusParams() cparams.Block.TimeIotaMs = 1 cparams.Block.MaxBytes = appconsts.DefaultMaxBytes @@ -159,22 +137,20 @@ func DefaultParams() *tmproto.ConsensusParams { func DefaultTendermintConfig() *tmconfig.Config { tmCfg := tmconfig.DefaultConfig() - // TimeoutCommit is the duration the node waits after committing a block - // before starting the next height. This duration influences the time - // interval between blocks. A smaller TimeoutCommit value could lead to - // less time between blocks (i.e. shorter block intervals). + // Reduce the timeout commit to 1ms to speed up the rate at which the test + // node produces blocks. tmCfg.Consensus.TimeoutCommit = 1 * time.Millisecond - // set the mempool's MaxTxBytes to allow the testnode to accept a + // Override the mempool's MaxTxBytes to allow the testnode to accept a // transaction that fills the entire square. Any blob transaction larger // than the square size will still fail no matter what. - upperBoundBytes := appconsts.DefaultSquareSizeUpperBound * appconsts.DefaultSquareSizeUpperBound * appconsts.ContinuationSparseShareContentSize - tmCfg.Mempool.MaxTxBytes = upperBoundBytes + maxTxBytes := appconsts.DefaultSquareSizeUpperBound * appconsts.DefaultSquareSizeUpperBound * appconsts.ContinuationSparseShareContentSize + tmCfg.Mempool.MaxTxBytes = maxTxBytes - // remove all barriers from the testnode being able to accept very large - // transactions and respond to very queries with large responses (~200MB was + // Override the MaxBodyBytes to allow the testnode to accept very large + // transactions and respond to queries with large responses (200 MiB was // chosen only as an arbitrary large number). - tmCfg.RPC.MaxBodyBytes = 200_000_000 + tmCfg.RPC.MaxBodyBytes = 200 * mebibyte return tmCfg } diff --git a/test/util/testnode/full_node.go b/test/util/testnode/full_node.go index 183d7a5f6d..0de8c4385a 100644 --- a/test/util/testnode/full_node.go +++ b/test/util/testnode/full_node.go @@ -1,14 +1,10 @@ package testnode import ( - "context" - "fmt" - "net" "os" "path/filepath" "testing" - "github.com/celestiaorg/celestia-app/test/util/genesis" "github.com/cosmos/cosmos-sdk/client/flags" srvtypes "github.com/cosmos/cosmos-sdk/server/types" "github.com/stretchr/testify/require" @@ -24,14 +20,7 @@ import ( // validator celestia-app network. It expects that all configuration files are // already initialized and saved to the baseDir. func NewCometNode(t testing.TB, baseDir string, cfg *Config) (*node.Node, srvtypes.Application, error) { - var logger log.Logger - if cfg.SupressLogs { - logger = log.NewNopLogger() - } else { - logger = log.NewTMLogger(log.NewSyncWriter(os.Stdout)) - logger = log.NewFilter(logger, log.AllowError()) - } - + logger := newLogger(cfg) dbPath := filepath.Join(cfg.TmConfig.RootDir, "data") db, err := dbm.NewGoLevelDB("application", dbPath) require.NoError(t, err) @@ -59,57 +48,11 @@ func NewCometNode(t testing.TB, baseDir string, cfg *Config) (*node.Node, srvtyp return tmNode, app, err } -// NewNetwork starts a single valiator celestia-app network using the provided -// configurations. Configured accounts will be funded and their keys can be -// accessed in keyring returned client.Context. All rpc, p2p, and grpc addresses -// in the provided configs are overwritten to use open ports. The node can be -// accessed via the returned client.Context or via the returned rpc and grpc -// addresses. Configured genesis options will be applied after all accounts have -// been initialized. -func NewNetwork(t testing.TB, cfg *Config) (cctx Context, rpcAddr, grpcAddr string) { - t.Helper() - - tmCfg := cfg.TmConfig - tmCfg.RPC.ListenAddress = fmt.Sprintf("tcp://127.0.0.1:%d", GetFreePort()) - tmCfg.P2P.ListenAddress = fmt.Sprintf("tcp://127.0.0.1:%d", GetFreePort()) - tmCfg.RPC.GRPCListenAddress = fmt.Sprintf("tcp://127.0.0.1:%d", GetFreePort()) - - // initialize the genesis file and validator files for the first validator. - baseDir, err := genesis.InitFiles(t.TempDir(), tmCfg, cfg.Genesis, 0) - require.NoError(t, err) - - tmNode, app, err := NewCometNode(t, baseDir, cfg) - require.NoError(t, err) - - cctx = NewContext(context.TODO(), cfg.Genesis.Keyring(), tmCfg, cfg.Genesis.ChainID) - - cctx, stopNode, err := StartNode(tmNode, cctx) - require.NoError(t, err) - - appCfg := cfg.AppConfig - appCfg.GRPC.Address = fmt.Sprintf("127.0.0.1:%d", GetFreePort()) - appCfg.API.Address = fmt.Sprintf("tcp://127.0.0.1:%d", GetFreePort()) - - cctx, cleanupGRPC, err := StartGRPCServer(app, appCfg, cctx) - require.NoError(t, err) - - t.Cleanup(func() { - t.Log("tearing down testnode") - require.NoError(t, stopNode()) - require.NoError(t, cleanupGRPC()) - }) - - return cctx, tmCfg.RPC.ListenAddress, appCfg.GRPC.Address -} - -func GetFreePort() int { - a, err := net.ResolveTCPAddr("tcp", "localhost:0") - if err == nil { - var l *net.TCPListener - if l, err = net.ListenTCP("tcp", a); err == nil { - defer l.Close() - return l.Addr().(*net.TCPAddr).Port - } +func newLogger(cfg *Config) log.Logger { + if cfg.SupressLogs { + return log.NewNopLogger() } - panic("while getting free port: " + err.Error()) + logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout)) + logger = log.NewFilter(logger, log.AllowError()) + return logger } diff --git a/test/util/testnode/full_node_test.go b/test/util/testnode/full_node_test.go index 7d040a07d6..fcfaca5a75 100644 --- a/test/util/testnode/full_node_test.go +++ b/test/util/testnode/full_node_test.go @@ -19,10 +19,6 @@ import ( coretypes "github.com/tendermint/tendermint/rpc/core/types" ) -const ( - kibibyte = 1024 -) - func TestIntegrationTestSuite(t *testing.T) { if testing.Short() { t.Skip("skipping full node integration test in short mode.") diff --git a/test/util/testnode/network.go b/test/util/testnode/network.go new file mode 100644 index 0000000000..3d2e427e21 --- /dev/null +++ b/test/util/testnode/network.go @@ -0,0 +1,89 @@ +package testnode + +import ( + "context" + "fmt" + "net" + "testing" + + "github.com/celestiaorg/celestia-app/test/util/genesis" + "github.com/stretchr/testify/require" +) + +// NewNetwork starts a single validator celestia-app network using the provided +// configurations. Configured accounts will be funded and their keys can be +// accessed in keyring returned client.Context. All rpc, p2p, and grpc addresses +// in the provided configs are overwritten to use open ports. The node can be +// accessed via the returned client.Context or via the returned rpc and grpc +// addresses. Configured genesis options will be applied after all accounts have +// been initialized. +func NewNetwork(t testing.TB, cfg *Config) (cctx Context, rpcAddr, grpcAddr string) { + t.Helper() + + tmCfg := cfg.TmConfig + tmCfg.RPC.ListenAddress = fmt.Sprintf("tcp://127.0.0.1:%d", mustGetFreePort()) + tmCfg.P2P.ListenAddress = fmt.Sprintf("tcp://127.0.0.1:%d", mustGetFreePort()) + tmCfg.RPC.GRPCListenAddress = fmt.Sprintf("tcp://127.0.0.1:%d", mustGetFreePort()) + + // initialize the genesis file and validator files for the first validator. + baseDir, err := genesis.InitFiles(t.TempDir(), tmCfg, cfg.Genesis, 0) + require.NoError(t, err) + + tmNode, app, err := NewCometNode(t, baseDir, cfg) + require.NoError(t, err) + + cctx = NewContext(context.Background(), cfg.Genesis.Keyring(), tmCfg, cfg.Genesis.ChainID) + + cctx, stopNode, err := StartNode(t, tmNode, cctx) + require.NoError(t, err) + + appCfg := cfg.AppConfig + appCfg.GRPC.Address = fmt.Sprintf("127.0.0.1:%d", mustGetFreePort()) + appCfg.API.Address = fmt.Sprintf("tcp://127.0.0.1:%d", mustGetFreePort()) + + cctx, cleanupGRPC, err := StartGRPCServer(app, appCfg, cctx) + require.NoError(t, err) + + t.Cleanup(func() { + t.Log("tearing down testnode") + err := stopNode() + if err != nil { + // the test has already completed so log the error instead of + // failing the test. + t.Logf("error stopping node %v", err) + } + err = cleanupGRPC() + if err != nil { + // the test has already completed so just log the error instead of + // failing the test. + t.Logf("error when cleaning up GRPC %v", err) + } + }) + + return cctx, tmCfg.RPC.ListenAddress, appCfg.GRPC.Address +} + +// getFreePort returns a free port and optionally an error. +func getFreePort() (int, error) { + a, err := net.ResolveTCPAddr("tcp", "localhost:0") + if err != nil { + return 0, err + } + + l, err := net.ListenTCP("tcp", a) + if err != nil { + return 0, err + } + defer l.Close() + return l.Addr().(*net.TCPAddr).Port, nil +} + +// mustGetFreePort returns a free port. Panics if no free ports are available or +// an error is encountered. +func mustGetFreePort() int { + port, err := getFreePort() + if err != nil { + panic(err) + } + return port +} diff --git a/test/util/testnode/rpc_client.go b/test/util/testnode/rpc_client.go index 458b410331..a1e1913d0c 100644 --- a/test/util/testnode/rpc_client.go +++ b/test/util/testnode/rpc_client.go @@ -5,6 +5,7 @@ import ( "os" "path" "strings" + "testing" srvconfig "github.com/cosmos/cosmos-sdk/server/config" srvgrpc "github.com/cosmos/cosmos-sdk/server/grpc" @@ -15,13 +16,17 @@ import ( "google.golang.org/grpc/credentials/insecure" ) +// noOpCleanup is a function that conforms to the cleanup function signature and +// performs no operation. +var noOpCleanup = func() error { return nil } + // StartNode starts the tendermint node along with a local core rpc client. The // rpc is returned via the client.Context. The function returned should be // called during cleanup to teardown the node, core client, along with canceling // the internal context.Context in the returned Context. -func StartNode(tmNode *node.Node, cctx Context) (Context, func() error, error) { +func StartNode(t testing.TB, tmNode *node.Node, cctx Context) (Context, func() error, error) { if err := tmNode.Start(); err != nil { - return cctx, func() error { return nil }, err + return cctx, noOpCleanup, err } coreClient := local.New(tmNode) @@ -31,12 +36,14 @@ func StartNode(tmNode *node.Node, cctx Context) (Context, func() error, error) { cctx.rootCtx = goCtx cleanup := func() error { cancel() + t.Log("stopping tmNode") err := tmNode.Stop() if err != nil { return err } tmNode.Wait() - return removeDir(path.Join([]string{cctx.HomeDir, "config"}...)) + t.Log("tmNode has stopped") + return removeDir(t, path.Join([]string{cctx.HomeDir, "config"}...)) } return cctx, cleanup, nil @@ -82,16 +89,19 @@ func DefaultAppConfig() *srvconfig.Config { // the config folder of the tendermint node. // This will manually go over the files contained inside the provided `rootDir` // and delete them one by one. -func removeDir(rootDir string) error { +func removeDir(t testing.TB, rootDir string) error { dir, err := os.ReadDir(rootDir) if err != nil { return err } for _, d := range dir { - err := os.RemoveAll(path.Join([]string{rootDir, d.Name()}...)) + path := path.Join([]string{rootDir, d.Name()}...) + t.Logf("removing %v", d.Name()) + err := os.RemoveAll(path) if err != nil { return err } } + t.Logf("removing %v", rootDir) return os.RemoveAll(rootDir) } diff --git a/x/mint/test/mint_test.go b/x/mint/test/mint_test.go index 8b728e5429..60cd5830e7 100644 --- a/x/mint/test/mint_test.go +++ b/x/mint/test/mint_test.go @@ -26,7 +26,7 @@ func (s *IntegrationTestSuite) SetupSuite() { t := s.T() t.Log("setting up mint integration test suite") - cparams := testnode.DefaultParams() + cparams := testnode.DefaultConsensusParams() oneDay := time.Hour * 24 oneMonth := oneDay * 30 sixMonths := oneMonth * 6