From 2030a07ef3a61e542cc57dd4d1540d2c3f613e1a Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Sat, 25 Nov 2023 17:14:30 -0500 Subject: [PATCH 01/14] refactor: testnode config --- test/util/testnode/config.go | 37 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/test/util/testnode/config.go b/test/util/testnode/config.go index bb04f44ee4..be0f86cf56 100644 --- a/test/util/testnode/config.go +++ b/test/util/testnode/config.go @@ -17,6 +17,8 @@ import ( ) const ( + kibibyte = 1024 + mebibyte = kibibyte * 1024 DefaultValidatorAccountName = "validator" ) @@ -70,7 +72,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,9 +110,8 @@ 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( @@ -132,14 +133,14 @@ type KVAppOptions struct { options map[string]interface{} } -// Get implements AppOptions -func (ao *KVAppOptions) Get(o string) interface{} { - return ao.options[o] +// Get returns the option value for the given option key. +func (ao *KVAppOptions) Get(option string) interface{} { + return ao.options[option] } -// Set adds an option to the KVAppOptions -func (ao *KVAppOptions) Set(o string, v interface{}) { - ao.options[o] = v +// 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. @@ -159,22 +160,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 } From 1e184e64ec12b2d89be7cbba4bf16a4fb61d7e59 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Sat, 25 Nov 2023 17:17:16 -0500 Subject: [PATCH 02/14] refactor: extract newLogger --- test/util/testnode/full_node.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/test/util/testnode/full_node.go b/test/util/testnode/full_node.go index 183d7a5f6d..4a2b0a296a 100644 --- a/test/util/testnode/full_node.go +++ b/test/util/testnode/full_node.go @@ -24,14 +24,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) @@ -113,3 +106,12 @@ func GetFreePort() int { } panic("while getting free port: " + err.Error()) } + +func newLogger(cfg *Config) log.Logger { + if cfg.SupressLogs { + return log.NewNopLogger() + } + logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout)) + logger = log.NewFilter(logger, log.AllowError()) + return logger +} From 0f04d5764cbd590ae6d49fe7e7321c30c1251f8b Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Sat, 25 Nov 2023 17:18:59 -0500 Subject: [PATCH 03/14] refactor: extract app options --- test/util/testnode/app_options.go | 27 +++++++++++++++++++++++++++ test/util/testnode/config.go | 23 ----------------------- 2 files changed, 27 insertions(+), 23 deletions(-) create mode 100644 test/util/testnode/app_options.go 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 be0f86cf56..dcd4a418c3 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" @@ -129,27 +127,6 @@ func DefaultConfig() *Config { WithSupressLogs(true) } -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 -} - func DefaultParams() *tmproto.ConsensusParams { cparams := types.DefaultConsensusParams() cparams.Block.TimeIotaMs = 1 From 0f7d31775b7a10feb62bd47c173928ed47e6b873 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Sat, 25 Nov 2023 17:32:19 -0500 Subject: [PATCH 04/14] refactor: extract new network --- test/util/testnode/full_node.go | 59 ----------------------------- test/util/testnode/network.go | 66 +++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 59 deletions(-) create mode 100644 test/util/testnode/network.go diff --git a/test/util/testnode/full_node.go b/test/util/testnode/full_node.go index 4a2b0a296a..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" @@ -52,61 +48,6 @@ 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 - } - } - panic("while getting free port: " + err.Error()) -} - func newLogger(cfg *Config) log.Logger { if cfg.SupressLogs { return log.NewNopLogger() diff --git a/test/util/testnode/network.go b/test/util/testnode/network.go new file mode 100644 index 0000000000..114606d86f --- /dev/null +++ b/test/util/testnode/network.go @@ -0,0 +1,66 @@ +package testnode + +import ( + "context" + "fmt" + "net" + "testing" + + "github.com/celestiaorg/celestia-app/test/util/genesis" + "github.com/stretchr/testify/require" +) + +// 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 + } + } + panic("while getting free port: " + err.Error()) +} From 14f907fac591dbd2529aed7bb5987cfdbdacbc14 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Sat, 25 Nov 2023 17:34:22 -0500 Subject: [PATCH 05/14] refactor: unexport getFreePort --- test/util/testnode/network.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/util/testnode/network.go b/test/util/testnode/network.go index 114606d86f..be02c0ac23 100644 --- a/test/util/testnode/network.go +++ b/test/util/testnode/network.go @@ -21,9 +21,9 @@ func NewNetwork(t testing.TB, cfg *Config) (cctx Context, rpcAddr, grpcAddr stri 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()) + 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) @@ -38,8 +38,8 @@ func NewNetwork(t testing.TB, cfg *Config) (cctx Context, rpcAddr, grpcAddr stri 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()) + 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) @@ -53,7 +53,7 @@ func NewNetwork(t testing.TB, cfg *Config) (cctx Context, rpcAddr, grpcAddr stri return cctx, tmCfg.RPC.ListenAddress, appCfg.GRPC.Address } -func GetFreePort() int { +func getFreePort() int { a, err := net.ResolveTCPAddr("tcp", "localhost:0") if err == nil { var l *net.TCPListener From 9129fd98eb50134644a7221bfbe8a15baa80e7a7 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Sat, 25 Nov 2023 17:38:52 -0500 Subject: [PATCH 06/14] refactor: prefer context.Background https://stackoverflow.com/questions/74239074/context-todo-or-context-background-which-one-should-i-prefer --- test/util/testnode/network.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/util/testnode/network.go b/test/util/testnode/network.go index be02c0ac23..8b15eb7f62 100644 --- a/test/util/testnode/network.go +++ b/test/util/testnode/network.go @@ -32,7 +32,7 @@ func NewNetwork(t testing.TB, cfg *Config) (cctx Context, rpcAddr, grpcAddr stri tmNode, app, err := NewCometNode(t, baseDir, cfg) require.NoError(t, err) - cctx = NewContext(context.TODO(), cfg.Genesis.Keyring(), tmCfg, cfg.Genesis.ChainID) + cctx = NewContext(context.Background(), cfg.Genesis.Keyring(), tmCfg, cfg.Genesis.ChainID) cctx, stopNode, err := StartNode(tmNode, cctx) require.NoError(t, err) From b483860ec3817da1115cf7dbfa07ab83af9dc4cc Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Sat, 25 Nov 2023 17:41:44 -0500 Subject: [PATCH 07/14] refactor: rename to DefaultConsensusParams --- app/test/max_total_blob_size_test.go | 2 +- test/cmd/txsim/cli_test.go | 2 +- test/util/testnode/config.go | 4 ++-- x/mint/test/mint_test.go | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/test/max_total_blob_size_test.go b/app/test/max_total_blob_size_test.go index 8c7875f2f9..ebbdc31f90 100644 --- a/app/test/max_total_blob_size_test.go +++ b/app/test/max_total_blob_size_test.go @@ -48,7 +48,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/config.go b/test/util/testnode/config.go index dcd4a418c3..928973cad6 100644 --- a/test/util/testnode/config.go +++ b/test/util/testnode/config.go @@ -116,7 +116,7 @@ func DefaultConfig() *Config { genesis.NewDefaultGenesis(). WithChainID(tmrand.Str(6)). WithGenesisTime(time.Now()). - WithConsensusParams(DefaultParams()). + WithConsensusParams(DefaultConsensusParams()). WithModifiers(). WithValidators(genesis.NewDefaultValidator(DefaultValidatorAccountName)), ). @@ -127,7 +127,7 @@ func DefaultConfig() *Config { WithSupressLogs(true) } -func DefaultParams() *tmproto.ConsensusParams { +func DefaultConsensusParams() *tmproto.ConsensusParams { cparams := types.DefaultConsensusParams() cparams.Block.TimeIotaMs = 1 cparams.Block.MaxBytes = appconsts.DefaultMaxBytes 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 From 649c2ebcb4b8b13fac26185b22083d5ac733da7f Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Sat, 25 Nov 2023 18:57:27 -0500 Subject: [PATCH 08/14] refactor: extract noOpCleanup --- test/util/testnode/rpc_client.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/util/testnode/rpc_client.go b/test/util/testnode/rpc_client.go index 458b410331..23eaea00ef 100644 --- a/test/util/testnode/rpc_client.go +++ b/test/util/testnode/rpc_client.go @@ -15,13 +15,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) { if err := tmNode.Start(); err != nil { - return cctx, func() error { return nil }, err + return cctx, noOpCleanup, err } coreClient := local.New(tmNode) From 60eb210c5223bda3ebeaa93b0818120e6255259e Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Sat, 25 Nov 2023 19:12:56 -0500 Subject: [PATCH 09/14] test: add more logs to debug delete dir issue See https://github.com/celestiaorg/celestia-app/issues/2858#issuecomment-1825915927 --- test/util/testnode/network.go | 2 +- test/util/testnode/rpc_client.go | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/test/util/testnode/network.go b/test/util/testnode/network.go index 8b15eb7f62..2ef2f95380 100644 --- a/test/util/testnode/network.go +++ b/test/util/testnode/network.go @@ -34,7 +34,7 @@ func NewNetwork(t testing.TB, cfg *Config) (cctx Context, rpcAddr, grpcAddr stri cctx = NewContext(context.Background(), cfg.Genesis.Keyring(), tmCfg, cfg.Genesis.ChainID) - cctx, stopNode, err := StartNode(tmNode, cctx) + cctx, stopNode, err := StartNode(t, tmNode, cctx) require.NoError(t, err) appCfg := cfg.AppConfig diff --git a/test/util/testnode/rpc_client.go b/test/util/testnode/rpc_client.go index 23eaea00ef..0822386f33 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" @@ -23,7 +24,7 @@ var noOpCleanup = func() error { return nil } // 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, noOpCleanup, err } @@ -35,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("waiting for tmNode to stop") + return removeDir(t, path.Join([]string{cctx.HomeDir, "config"}...)) } return cctx, cleanup, nil @@ -86,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) } From bd4ed939b5f13d24e48bc25330665651baf0c813 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Sat, 25 Nov 2023 19:36:41 -0500 Subject: [PATCH 10/14] refactor: mustGetFreePort --- test/util/testnode/network.go | 39 +++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/test/util/testnode/network.go b/test/util/testnode/network.go index 2ef2f95380..31999be935 100644 --- a/test/util/testnode/network.go +++ b/test/util/testnode/network.go @@ -21,9 +21,9 @@ func NewNetwork(t testing.TB, cfg *Config) (cctx Context, rpcAddr, grpcAddr stri 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()) + 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) @@ -38,8 +38,8 @@ func NewNetwork(t testing.TB, cfg *Config) (cctx Context, rpcAddr, grpcAddr stri 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()) + 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) @@ -53,14 +53,27 @@ func NewNetwork(t testing.TB, cfg *Config) (cctx Context, rpcAddr, grpcAddr stri return cctx, tmCfg.RPC.ListenAddress, appCfg.GRPC.Address } -func getFreePort() int { +// getFreePort returns a free port and optionally an error. +func getFreePort() (int, error) { 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 - } + if err != nil { + return 0, err } - panic("while getting free port: " + err.Error()) + + 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 } From 03a27bb833cc145199e6d99747a1035407f96d17 Mon Sep 17 00:00:00 2001 From: Rootul P Date: Sun, 26 Nov 2023 20:15:10 -0500 Subject: [PATCH 11/14] Update test/util/testnode/network.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- test/util/testnode/network.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/util/testnode/network.go b/test/util/testnode/network.go index 31999be935..8658cbac76 100644 --- a/test/util/testnode/network.go +++ b/test/util/testnode/network.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/require" ) -// NewNetwork starts a single valiator celestia-app network using the provided +// 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 From aba38413211cab5d81273967826c05adbe07242e Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Sun, 26 Nov 2023 20:16:48 -0500 Subject: [PATCH 12/14] address coderabbit feedback --- test/util/testnode/rpc_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/util/testnode/rpc_client.go b/test/util/testnode/rpc_client.go index 0822386f33..a1e1913d0c 100644 --- a/test/util/testnode/rpc_client.go +++ b/test/util/testnode/rpc_client.go @@ -42,7 +42,7 @@ func StartNode(t testing.TB, tmNode *node.Node, cctx Context) (Context, func() e return err } tmNode.Wait() - t.Log("waiting for tmNode to stop") + t.Log("tmNode has stopped") return removeDir(t, path.Join([]string{cctx.HomeDir, "config"}...)) } From aee75a35fae8a32703a4d2aefc062b955caa0f33 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Sun, 26 Nov 2023 20:22:11 -0500 Subject: [PATCH 13/14] test: do not fail test if cleanup fails Based on CodeRabbit feedback https://github.com/celestiaorg/celestia-app/pull/2871#discussion_r1405532151 --- test/util/testnode/network.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/util/testnode/network.go b/test/util/testnode/network.go index 8658cbac76..3d2e427e21 100644 --- a/test/util/testnode/network.go +++ b/test/util/testnode/network.go @@ -46,8 +46,18 @@ func NewNetwork(t testing.TB, cfg *Config) (cctx Context, rpcAddr, grpcAddr stri t.Cleanup(func() { t.Log("tearing down testnode") - require.NoError(t, stopNode()) - require.NoError(t, cleanupGRPC()) + 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 From 1480500b0421a7e4f5c826bb561e8160547a251b Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Wed, 29 Nov 2023 12:27:17 -0500 Subject: [PATCH 14/14] fix: remove duplicate declaration --- test/util/testnode/config.go | 4 ++-- test/util/testnode/full_node_test.go | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/test/util/testnode/config.go b/test/util/testnode/config.go index 928973cad6..0f8f1f4504 100644 --- a/test/util/testnode/config.go +++ b/test/util/testnode/config.go @@ -15,8 +15,8 @@ import ( ) const ( - kibibyte = 1024 - mebibyte = kibibyte * 1024 + kibibyte = 1024 // bytes + mebibyte = 1_048_576 // bytes DefaultValidatorAccountName = "validator" ) 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.")