core: Remove embedded core node option (celestiaorg#567)

* refactor: remove embedded core node functionality

* bug: hook start and stop for remote client via fx

* fix: state constructor relies on new core config vals

cmd/cel-shed/header.go

@@ -41,7 +41,7 @@ Custom store path is not supported yet.`,
 			return fmt.Errorf("invalid height: %w", err)
 		}
 
-		store, err := node.OpenStore(fmt.Sprintf("~/.celestia-%s", strings.ToLower(tp.String())), tp)
+		store, err := node.OpenStore(fmt.Sprintf("~/.celestia-%s", strings.ToLower(tp.String())))
 		if err != nil {
 			return err
 		}

cmd/start.go

@@ -25,7 +25,7 @@ Options passed on start override configuration options only on start and are not
 				return err
 			}
 
-			store, err := node.OpenStore(env.StorePath, env.NodeType)
+			store, err := node.OpenStore(env.StorePath)
 			if err != nil {
 				return err
 			}

core/client.go

@@ -5,10 +5,8 @@ import (
 
 	retryhttp "github.com/hashicorp/go-retryablehttp"
 
-	corenode "github.com/tendermint/tendermint/node"
 	"github.com/tendermint/tendermint/rpc/client"
 	"github.com/tendermint/tendermint/rpc/client/http"
-	"github.com/tendermint/tendermint/rpc/client/local"
 )
 
 // Client is an alias to Core Client.
@@ -27,33 +25,3 @@ func NewRemote(protocol, remoteAddr string) (Client, error) {
 		httpClient.StandardClient(),
 	)
 }
-
-// NewEmbedded returns a new Client from an embedded Core node process.
-func NewEmbedded(cfg *Config) (Client, error) {
-	node, err := corenode.DefaultNewNode(cfg, adaptedLogger())
-	if err != nil {
-		return nil, err
-	}
-
-	return &embeddedWrapper{local.New(node), node}, nil
-}
-
-// NewEmbeddedFromNode wraps a given Core node process to be able to control its lifecycle.
-func NewEmbeddedFromNode(node *corenode.Node) Client {
-	return &embeddedWrapper{local.New(node), node}
-}
-
-// embeddedWrapper is a small wrapper around local Client which ensures the embedded Core node
-// can be started/stopped.
-type embeddedWrapper struct {
-	*local.Local
-	node *corenode.Node
-}
-
-func (e *embeddedWrapper) Start() error {
-	return e.node.Start()
-}
-
-func (e *embeddedWrapper) Stop() error {
-	return e.node.Stop()
-}

core/client_test.go

@@ -7,46 +7,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestEmbeddedClientLifecycle(t *testing.T) {
-	client := MockEmbeddedClient()
-	require.NoError(t, client.Stop())
-}
-
-func TestEmbeddedClient_Status(t *testing.T) {
-	client := MockEmbeddedClient()
-
-	ctx, cancel := context.WithCancel(context.Background())
-	t.Cleanup(cancel)
-
-	status, err := client.Status(ctx)
-	require.NoError(t, err)
-	require.NotNil(t, status)
-
-	require.NoError(t, client.Stop())
-}
-
-func TestEmbeddedClient_StartBlockSubscription_And_GetBlock(t *testing.T) {
-	client := MockEmbeddedClient()
-
-	ctx, cancel := context.WithCancel(context.Background())
-	t.Cleanup(cancel)
-
-	eventChan, err := client.Subscribe(ctx, newBlockSubscriber, newBlockEventQuery)
-	require.NoError(t, err)
-
-	for i := 1; i <= 3; i++ {
-		<-eventChan
-		// check that `Block` works as intended (passing nil to get block at latest height)
-		block, err := client.Block(ctx, nil)
-		require.NoError(t, err)
-		require.Equal(t, int64(i), block.Block.Height)
-	}
-
-	// unsubscribe to event channel
-	require.NoError(t, client.Unsubscribe(ctx, newBlockSubscriber, newBlockEventQuery))
-	require.NoError(t, client.Stop())
-}
-
 func TestRemoteClientLifecycle(t *testing.T) {
 	remote, client, err := StartRemoteClient()
 	require.NoError(t, err)

core/fetcher.go

@@ -4,13 +4,17 @@ import (
 	"context"
 	"fmt"
 
+	logging "github.com/ipfs/go-log/v2"
 	tmbytes "github.com/tendermint/tendermint/libs/bytes"
 	"github.com/tendermint/tendermint/types"
 )
 
 const newBlockSubscriber = "NewBlock/Events"
 
-var newBlockEventQuery = types.QueryForEvent(types.EventNewBlock).String()
+var (
+	log                = logging.Logger("core/fetcher")
+	newBlockEventQuery = types.QueryForEvent(types.EventNewBlock).String()
+)
 
 type BlockFetcher struct {
 	client Client

core/fetcher_test.go

@@ -11,7 +11,15 @@ import (
 )
 
 func TestBlockFetcher_GetBlock_and_SubscribeNewBlockEvent(t *testing.T) {
-	client := MockEmbeddedClient()
+	nd, client, err := StartRemoteClient()
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		client.Stop() //nolint:errcheck
+		nd.Stop()     //nolint:errcheck
+	})
+	err = client.Start()
+	require.NoError(t, err)
+
 	fetcher := NewBlockFetcher(client)
 
 	ctx, cancel := context.WithCancel(context.Background())
@@ -37,7 +45,15 @@ func TestBlockFetcher_GetBlock_and_SubscribeNewBlockEvent(t *testing.T) {
 // TestBlockFetcherHeaderValues tests that both the Commit and ValidatorSet
 // endpoints are working as intended.
 func TestBlockFetcherHeaderValues(t *testing.T) {
-	client := MockEmbeddedClient()
+	nd, client, err := StartRemoteClient()
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		client.Stop() //nolint:errcheck
+		nd.Stop()     //nolint:errcheck
+	})
+	err = client.Start()
+	require.NoError(t, err)
+
 	fetcher := NewBlockFetcher(client)
 
 	ctx, cancel := context.WithCancel(context.Background())

core/testing.go

@@ -1,41 +1,19 @@
 package core
 
 import (
-	"os"
-	"testing"
-
 	"github.com/tendermint/tendermint/abci/example/kvstore"
 	"github.com/tendermint/tendermint/abci/types"
-	"github.com/tendermint/tendermint/config"
 	"github.com/tendermint/tendermint/node"
 	rpctest "github.com/tendermint/tendermint/rpc/test"
 )
 
 const defaultRetainBlocks int64 = 10
 
-// MockConfig provides a testing configuration for embedded Core Client.
-func MockConfig(t *testing.T) *Config {
-	cfg := config.ResetTestRoot(t.Name())
-	t.Cleanup(func() {
-		os.RemoveAll(cfg.RootDir)
-	})
-	return cfg
-}
-
 // StartMockNode starts a mock Core node background process and returns it.
 func StartMockNode(app types.Application) *node.Node {
 	return rpctest.StartTendermint(app, rpctest.SuppressStdout, rpctest.RecreateConfig)
 }
 
-func EphemeralMockEmbeddedClient(t *testing.T) Client {
-	nd := StartMockNode(CreateKvStore(defaultRetainBlocks))
-	t.Cleanup(func() {
-		nd.Stop() //nolint:errcheck
-		rpctest.StopTendermint(nd)
-	})
-	return NewEmbeddedFromNode(nd)
-}
-
 // CreateKvStore creates a simple kv store app and gives the user
 // ability to set desired amount of blocks to be retained.
 func CreateKvStore(retainBlocks int64) *kvstore.Application {
@@ -44,11 +22,6 @@ func CreateKvStore(retainBlocks int64) *kvstore.Application {
 	return app
 }
 
-// MockEmbeddedClient returns a started mock Core Client.
-func MockEmbeddedClient() Client {
-	return NewEmbeddedFromNode(StartMockNode(CreateKvStore(defaultRetainBlocks)))
-}
-
 // StartRemoteClient returns a started remote Core node process, as well its
 // mock Core Client.
 func StartRemoteClient() (*node.Node, Client, error) {
@@ -58,14 +31,6 @@ func StartRemoteClient() (*node.Node, Client, error) {
 	return remote, client, err
 }
 
-// StartRemoteCore starts a remote core and returns its protocol and address
-func StartRemoteCore() (*node.Node, string, string) {
-	app := CreateKvStore(defaultRetainBlocks)
-	remote := StartMockNode(app)
-	protocol, ip := GetRemoteEndpoint(remote)
-	return remote, protocol, ip
-}
-
 // GetRemoteEndpoint returns the protocol and ip of the remote node.
 func GetRemoteEndpoint(remote *node.Node) (string, string) {
 	endpoint := remote.Config().RPC.ListenAddress

node/components.go

@@ -35,7 +35,7 @@ func bridgeComponents(cfg *Config, store Store) fxutil.Option {
 	return fxutil.Options(
 		fxutil.Supply(Bridge),
 		baseComponents(cfg, store),
-		nodecore.Components(cfg.Core, store.Core),
+		nodecore.Components(cfg.Core),
 		fxutil.Provide(services.LightAvailability), // TODO(@Wondertan): Remove strict requirements to have Availability
 	)
 }
@@ -70,13 +70,13 @@ func baseComponents(cfg *Config, store Store) fxutil.Option {
 		fxutil.Invoke(invokeWatchdog(store.Path())),
 		p2p.Components(cfg.P2P),
 		// state components
-		fxutil.ProvideIf(cfg.Core.Remote, state.NewService),
-		fxutil.ProvideIf(cfg.Core.Remote, func(lc fx.Lifecycle) (state.Accessor, error) {
+		fxutil.ProvideIf(cfg.Core.RemoteAddr != "", state.NewService),
+		fxutil.ProvideIf(cfg.Core.RemoteAddr != "", func(lc fx.Lifecycle) (state.Accessor, error) {
 			ks, err := store.Keystore()
 			if err != nil {
 				return nil, err
 			}
-			ca, err := statecomponents.CoreAccessor(ks, cfg.Core.RemoteConfig.RemoteAddr, params.DefaultNetwork())
+			ca, err := statecomponents.CoreAccessor(ks, cfg.Core.RemoteAddr, params.DefaultNetwork())
 			if err != nil {
 				return nil, err
 			}

node/config_opts.go

@@ -3,9 +3,8 @@ package node
 // WithRemoteCore configures Node to start with remote Core.
 func WithRemoteCore(protocol string, address string) Option {
 	return func(cfg *Config, _ *settings) (_ error) {
-		cfg.Core.Remote = true
-		cfg.Core.RemoteConfig.Protocol = protocol
-		cfg.Core.RemoteConfig.RemoteAddr = address
+		cfg.Core.Protocol = protocol
+		cfg.Core.RemoteAddr = address
 		return
 	}
 }