fix(state): CoreAccessor manages its own context (#1247)

Resolves #1248 and
#1246

Author: renaynay

Diff for: nodebuilder/node_test.go

@@ -36,8 +36,14 @@ func TestLifecycle(t *testing.T) {
 			err := node.Start(ctx)
 			require.NoError(t, err)
 
+			// ensure the state service is running
+			require.False(t, node.StateServ.IsStopped())
+
 			err = node.Stop(ctx)
 			require.NoError(t, err)
+
+			// ensure the state service is stopped
+			require.True(t, node.StateServ.IsStopped())
 		})
 	}
 }

Diff for: state/core_access.go

@@ -73,10 +73,11 @@ func NewCoreAccessor(
 }
 
 func (ca *CoreAccessor) Start(ctx context.Context) error {
-	ca.ctx, ca.cancel = context.WithCancel(ctx)
 	if ca.coreConn != nil {
 		return fmt.Errorf("core-access: already connected to core endpoint")
 	}
+	ca.ctx, ca.cancel = context.WithCancel(context.Background())
+
 	// dial given celestia-core endpoint
 	endpoint := fmt.Sprintf("%s:%s", ca.coreIP, ca.grpcPort)
 	client, err := grpc.DialContext(ctx, endpoint, grpc.WithTransportCredentials(insecure.NewCredentials()))
@@ -100,21 +101,32 @@ func (ca *CoreAccessor) Start(ctx context.Context) error {
 }
 
 func (ca *CoreAccessor) Stop(context.Context) error {
-	defer ca.cancel()
+	if ca.cancel == nil {
+		log.Warn("core accessor already stopped")
+		return nil
+	}
 	if ca.coreConn == nil {
 		log.Warn("no connection found to close")
 		return nil
 	}
+	defer ca.cancelCtx()
+
 	// close out core connection
 	err := ca.coreConn.Close()
 	if err != nil {
 		return err
 	}
+
 	ca.coreConn = nil
 	ca.queryCli = nil
 	return nil
 }
 
+func (ca *CoreAccessor) cancelCtx() {
+	ca.cancel()
+	ca.cancel = nil
+}
+
 func (ca *CoreAccessor) constructSignedTx(
 	ctx context.Context,
 	msg sdktypes.Msg,

Diff for: state/core_access_test.go

@@ -0,0 +1,33 @@
+package state
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestLifecycle(t *testing.T) {
+	ca := NewCoreAccessor(nil, nil, "", "", "")
+	ctx, cancel := context.WithCancel(context.Background())
+	// start the accessor
+	err := ca.Start(ctx)
+	require.NoError(t, err)
+	// ensure accessor isn't stopped
+	require.False(t, ca.IsStopped())
+	// cancel the top level context (this should not affect the lifecycle of the
+	// accessor as it should manage its own internal context)
+	cancel()
+	// ensure accessor was unaffected by top-level context cancellation
+	require.False(t, ca.IsStopped())
+	// stop the accessor
+	stopCtx, stopCancel := context.WithCancel(context.Background())
+	t.Cleanup(stopCancel)
+	err = ca.Stop(stopCtx)
+	require.NoError(t, err)
+	// ensure accessor is stopped
+	require.True(t, ca.IsStopped())
+	// ensure that stopping the accessor again does not return an error
+	err = ca.Stop(stopCtx)
+	require.NoError(t, err)
+}