Address comments

Author: Manav-Aggarwal

da/jsonrpc/client.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"strings"
 
 	"cosmossdk.io/log"
 	"github.com/filecoin-project/go-jsonrpc"
@@ -61,7 +60,7 @@ func (api *API) GetIDs(ctx context.Context, height uint64, _ []byte) (*da.GetIDs
 			api.Logger.Debug("RPC call indicates blobs not found", "method", "GetIDs", "height", height)
 			return nil, err // Return the specific ErrBlobNotFound
 		}
-		if strings.Contains(err.Error(), da.ErrHeightFromFuture.Error()) {
+		if errors.Is(err, da.ErrHeightFromFuture) {
 			api.Logger.Debug("RPC call indicates height from future", "method", "GetIDs", "height", height)
 			return nil, err // Return the specific ErrHeightFromFuture
 		}

da/jsonrpc/proxy_test.go

@@ -73,6 +73,7 @@ func TestProxy(t *testing.T) {
 	t.Run("Given height is from the future", func(t *testing.T) {
 		HeightFromFutureTest(t, &client.DA)
 	})
+	dummy.StopHeightTicker()
 }
 
 // BasicDATest tests round trip of messages to DA and back.

node/full_node_integration_test.go

@@ -328,7 +328,7 @@ func testSingleSequencerTwoFullNodes(t *testing.T, source Source) {
 	shutdownAndWait(t, cancels, &runningWg, 5*time.Second)
 }
 
-// testSingleSequencerSingleFullNode sets up a single sequencer and a single full node with a trusted hash, starts the sequencer, waits for it to produce a block, then starts the full node with the trusted hash.
+// testSingleSequencerSingleFullNodeTrustedHash sets up a single sequencer and a single full node with a trusted hash, starts the sequencer, waits for it to produce a block, then starts the full node with the trusted hash.
 // It waits for both nodes to reach a target block height (using the provided 'source' to determine block inclusion), verifies that both nodes are fully synced, and then shuts them down.
 func testSingleSequencerSingleFullNodeTrustedHash(t *testing.T, source Source) {
 	require := require.New(t)

node/helpers_test.go

@@ -42,12 +42,16 @@ const (
 )
 
 // createTestComponents creates test components for node initialization
-func createTestComponents(t *testing.T, config rollkitconfig.Config) (coreexecutor.Executor, coresequencer.Sequencer, coreda.DA, *p2p.Client, datastore.Batching, *key.NodeKey) {
+func createTestComponents(t *testing.T, config rollkitconfig.Config) (coreexecutor.Executor, coresequencer.Sequencer, coreda.DA, *p2p.Client, datastore.Batching, *key.NodeKey, func()) {
 	executor := coreexecutor.NewDummyExecutor()
 	sequencer := coresequencer.NewDummySequencer()
 	dummyDA := coreda.NewDummyDA(100_000, 0, 0, config.DA.BlockTime.Duration)
 	dummyDA.StartHeightTicker()
 
+	stopDAHeightTicker := func() {
+		dummyDA.StopHeightTicker()
+	}
+
 	// Create genesis and keys for P2P client
 	_, genesisValidatorKey, _ := types.GetGenesisWithPrivkey("test-chain")
 	p2pKey := &key.NodeKey{
@@ -59,7 +63,7 @@ func createTestComponents(t *testing.T, config rollkitconfig.Config) (coreexecut
 	require.NotNil(t, p2pClient)
 	ds := dssync.MutexWrap(datastore.NewMapDatastore())
 
-	return executor, sequencer, dummyDA, p2pClient, ds, p2pKey
+	return executor, sequencer, dummyDA, p2pClient, ds, p2pKey, stopDAHeightTicker
 }
 
 func getTestConfig(t *testing.T, n int) rollkitconfig.Config {
@@ -97,7 +101,7 @@ func createNodeWithCleanup(t *testing.T, config rollkitconfig.Config) (*FullNode
 	remoteSigner, err := remote_signer.NewNoopSigner(genesisValidatorKey)
 	require.NoError(t, err)
 
-	executor, sequencer, dac, p2pClient, ds, _ := createTestComponents(t, config)
+	executor, sequencer, dac, p2pClient, ds, _, stopDAHeightTicker := createTestComponents(t, config)
 	nodeKey, err := InitFiles(config.RootDir)
 	require.NoError(t, err)
 
@@ -121,6 +125,7 @@ func createNodeWithCleanup(t *testing.T, config rollkitconfig.Config) (*FullNode
 	cleanup := func() {
 		// Cancel the context to stop the node
 		cancel()
+		stopDAHeightTicker()
 	}
 
 	return node.(*FullNode), cleanup
@@ -143,7 +148,7 @@ func createNodesWithCleanup(t *testing.T, num int, config rollkitconfig.Config)
 
 	aggListenAddress := config.P2P.ListenAddress
 	aggPeers := config.P2P.Peers
-	executor, sequencer, dac, p2pClient, ds, aggP2PKey := createTestComponents(t, config)
+	executor, sequencer, dac, p2pClient, ds, aggP2PKey, stopDAHeightTicker := createTestComponents(t, config)
 	aggPeerID, err := peer.IDFromPrivateKey(aggP2PKey.PrivKey)
 	require.NoError(err)
 
@@ -170,6 +175,7 @@ func createNodesWithCleanup(t *testing.T, num int, config rollkitconfig.Config)
 	cleanup := func() {
 		// Cancel the context to stop the node
 		aggCancel()
+		stopDAHeightTicker()
 	}
 
 	nodes[0], cleanups[0] = aggNode.(*FullNode), cleanup
@@ -186,7 +192,7 @@ func createNodesWithCleanup(t *testing.T, num int, config rollkitconfig.Config)
 		}
 		config.P2P.ListenAddress = fmt.Sprintf("/ip4/127.0.0.1/tcp/%d", 40001+i)
 		config.RPC.Address = fmt.Sprintf("127.0.0.1:%d", 8001+i)
-		executor, sequencer, _, p2pClient, _, nodeP2PKey := createTestComponents(t, config)
+		executor, sequencer, _, p2pClient, _, nodeP2PKey, stopDAHeightTicker := createTestComponents(t, config)
 		node, err := NewNode(
 			ctx,
 			config,
@@ -206,6 +212,7 @@ func createNodesWithCleanup(t *testing.T, num int, config rollkitconfig.Config)
 		cleanup := func() {
 			// Cancel the context to stop the node
 			cancel()
+			stopDAHeightTicker()
 		}
 		nodes[i], cleanups[i] = node.(*FullNode), cleanup
 		nodePeerID, err := peer.IDFromPrivateKey(nodeP2PKey.PrivKey)

pkg/cmd/run_node_test.go

@@ -25,11 +25,14 @@ import (
 
 const MockDANamespace = "test"
 
-func createTestComponents(_ context.Context, t *testing.T) (coreexecutor.Executor, coresequencer.Sequencer, coreda.DA, signer.Signer, *p2p.Client, datastore.Batching) {
+func createTestComponents(_ context.Context, t *testing.T) (coreexecutor.Executor, coresequencer.Sequencer, coreda.DA, signer.Signer, *p2p.Client, datastore.Batching, func()) {
 	executor := coreexecutor.NewDummyExecutor()
 	sequencer := coresequencer.NewDummySequencer()
 	dummyDA := coreda.NewDummyDA(100_000, 0, 0, 10*time.Second)
 	dummyDA.StartHeightTicker()
+	stopDAHeightTicker := func() {
+		dummyDA.StopHeightTicker()
+	}
 	tmpDir := t.TempDir()
 	keyProvider, err := filesigner.CreateFileSystemSigner(filepath.Join(tmpDir, "config"), []byte{})
 	if err != nil {
@@ -39,7 +42,7 @@ func createTestComponents(_ context.Context, t *testing.T) (coreexecutor.Executo
 	p2pClient := &p2p.Client{}
 	ds := datastore.NewMapDatastore()
 
-	return executor, sequencer, dummyDA, keyProvider, p2pClient, ds
+	return executor, sequencer, dummyDA, keyProvider, p2pClient, ds, stopDAHeightTicker
 }
 
 func TestParseFlags(t *testing.T) {
@@ -79,7 +82,8 @@ func TestParseFlags(t *testing.T) {
 
 	args := append([]string{"start"}, flags...)
 
-	executor, sequencer, dac, keyProvider, p2pClient, ds := createTestComponents(context.Background(), t)
+	executor, sequencer, dac, keyProvider, p2pClient, ds, stopDAHeightTicker := createTestComponents(context.Background(), t)
+	defer stopDAHeightTicker()
 	nodeKey, err := key.GenerateNodeKey()
 	if err != nil {
 		t.Fatalf("Error: %v", err)
@@ -159,7 +163,8 @@ func TestAggregatorFlagInvariants(t *testing.T) {
 	for i, flags := range flagVariants {
 		args := append([]string{"start"}, flags...)
 
-		executor, sequencer, dac, keyProvider, p2pClient, ds := createTestComponents(context.Background(), t)
+		executor, sequencer, dac, keyProvider, p2pClient, ds, stopDAHeightTicker := createTestComponents(context.Background(), t)
+		defer stopDAHeightTicker()
 
 		nodeKey, err := key.GenerateNodeKey()
 		if err != nil {
@@ -199,7 +204,8 @@ func TestDefaultAggregatorValue(t *testing.T) {
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			executor, sequencer, dac, keyProvider, p2pClient, ds := createTestComponents(context.Background(), t)
+			executor, sequencer, dac, keyProvider, p2pClient, ds, stopDAHeightTicker := createTestComponents(context.Background(), t)
+			defer stopDAHeightTicker()
 
 			nodeKey, err := key.GenerateNodeKey()
 			if err != nil {
@@ -283,7 +289,8 @@ func TestCentralizedAddresses(t *testing.T) {
 		"--rollkit.da.address=http://central-da:26657",
 	}
 
-	executor, sequencer, dac, keyProvider, p2pClient, ds := createTestComponents(context.Background(), t)
+	executor, sequencer, dac, keyProvider, p2pClient, ds, stopDAHeightTicker := createTestComponents(context.Background(), t)
+	defer stopDAHeightTicker()
 
 	tmpDir := t.TempDir()
 	nodeKey, err := key.LoadOrGenNodeKey(filepath.Join(tmpDir, "config", "node_key.json"))
@@ -312,7 +319,9 @@ func TestStartNodeErrors(t *testing.T) {
 	logger := log.NewNopLogger() // Use NopLogger for tests unless specific logging output is needed
 
 	// Common setup
-	executor, sequencer, dac, _, p2pClient, ds := createTestComponents(baseCtx, t)
+	executor, sequencer, dac, _, p2pClient, ds, stopDAHeightTicker := createTestComponents(baseCtx, t)
+	defer stopDAHeightTicker()
+
 	nodeKey, err := key.GenerateNodeKey()
 	assert.NoError(t, err)
 	tmpDir := t.TempDir()