devmode: Allow DevMode + Follower configurations. (algorand#5157)

netdeploy/networkTemplate.go

@@ -209,7 +209,6 @@ func (t NetworkTemplate) Validate() error {
 	}
 
 	// No wallet can be assigned to more than one node
-	// At least one relay is required
 	wallets := make(map[string]bool)
 	for _, cfg := range t.Nodes {
 		for _, wallet := range cfg.Wallets {
@@ -221,17 +220,49 @@ func (t NetworkTemplate) Validate() error {
 		}
 	}
 
+	// At least one relay is required
 	if len(t.Nodes) > 1 && countRelayNodes(t.Nodes) == 0 {
 		return fmt.Errorf("invalid template: at least one relay is required when more than a single node presents")
 	}
 
+	// Validate JSONOverride decoding
+	for _, cfg := range t.Nodes {
+		local := config.GetDefaultLocal()
+		err := decodeJSONOverride(cfg.ConfigJSONOverride, &local)
+		if err != nil {
+			return fmt.Errorf("invalid template: unable to decode JSONOverride: %w", err)
+		}
+	}
+
+	// Follow nodes cannot be relays
+	for _, cfg := range t.Nodes {
+		if cfg.IsRelay && isEnableFollowMode(cfg.ConfigJSONOverride) {
+			return fmt.Errorf("invalid template: follower nodes may not be relays")
+		}
+	}
+
 	if t.Genesis.DevMode && len(t.Nodes) != 1 {
-		return fmt.Errorf("invalid template: DevMode should only have a single node")
+		if countRelayNodes(t.Nodes) != 1 {
+			return fmt.Errorf("invalid template: devmode configurations may have at most one relay")
+		}
+
+		for _, cfg := range t.Nodes {
+			if !cfg.IsRelay && !isEnableFollowMode(cfg.ConfigJSONOverride) {
+				return fmt.Errorf("invalid template: devmode configurations may only contain one relay and follower nodes")
+			}
+		}
 	}
 
 	return nil
 }
 
+func isEnableFollowMode(JSONOverride string) bool {
+	local := config.GetDefaultLocal()
+	// decode error is checked elsewhere
+	_ = decodeJSONOverride(JSONOverride, &local)
+	return local.EnableFollowMode
+}
+
 // countRelayNodes counts the total number of relays
 func countRelayNodes(nodeCfgs []remote.NodeConfigGoal) (relayCount int) {
 	for _, cfg := range nodeCfgs {
@@ -242,6 +273,18 @@ func countRelayNodes(nodeCfgs []remote.NodeConfigGoal) (relayCount int) {
 	return
 }
 
+func decodeJSONOverride(override string, cfg *config.Local) error {
+	if override != "" {
+		reader := strings.NewReader(override)
+		dec := json.NewDecoder(reader)
+		dec.DisallowUnknownFields()
+		if err := dec.Decode(&cfg); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func createConfigFile(node remote.NodeConfigGoal, configFile string, numNodes int, relaysCount int) error {
 	cfg := config.GetDefaultLocal()
 	cfg.GossipFanout = numNodes
@@ -266,12 +309,10 @@ func createConfigFile(node remote.NodeConfigGoal, configFile string, numNodes in
 		cfg.DeadlockDetection = node.DeadlockDetection
 	}
 
-	if node.ConfigJSONOverride != "" {
-		reader := strings.NewReader(node.ConfigJSONOverride)
-		dec := json.NewDecoder(reader)
-		if err := dec.Decode(&cfg); err != nil {
-			return err
-		}
+	err := decodeJSONOverride(node.ConfigJSONOverride, &cfg)
+	if err != nil {
+		return err
 	}
+
 	return cfg.SaveToFile(configFile)
 }

netdeploy/networkTemplates_test.go

@@ -26,6 +26,8 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/algorand/go-algorand/config"
+	"github.com/algorand/go-algorand/gen"
+	"github.com/algorand/go-algorand/netdeploy/remote"
 	"github.com/algorand/go-algorand/test/partitiontest"
 )
 
@@ -98,6 +100,133 @@ func TestValidate(t *testing.T) {
 	a.NoError(err)
 }
 
+func TestDevModeValidate(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	// same genesis configuration for all tests.
+	devmodeGenesis := gen.GenesisData{
+		DevMode: true,
+		Wallets: []gen.WalletData{
+			{
+				Stake: 100,
+			},
+		},
+	}
+
+	t.Run("DevMode two relays", func(t *testing.T) {
+		t.Parallel()
+		tmpl := NetworkTemplate{
+			Genesis: devmodeGenesis,
+			Nodes: []remote.NodeConfigGoal{
+				{
+					IsRelay: true,
+				},
+				{
+					IsRelay: true,
+				},
+			},
+		}
+		require.ErrorContains(t, tmpl.Validate(), "devmode configurations may have at most one relay")
+	})
+
+	t.Run("FollowMode relay", func(t *testing.T) {
+		t.Parallel()
+		tmpl := NetworkTemplate{
+			Genesis: devmodeGenesis,
+			Nodes: []remote.NodeConfigGoal{
+				{
+					IsRelay:            true,
+					ConfigJSONOverride: "{\"EnableFollowMode\":true}",
+				},
+			},
+		}
+		require.ErrorContains(t, tmpl.Validate(), "follower nodes may not be relays")
+	})
+
+	t.Run("DevMode multiple regular nodes", func(t *testing.T) {
+		t.Parallel()
+		tmpl := NetworkTemplate{
+			Genesis: devmodeGenesis,
+			Nodes: []remote.NodeConfigGoal{
+				{
+					IsRelay: true,
+				},
+				{},
+			},
+		}
+		require.ErrorContains(t, tmpl.Validate(), "devmode configurations may only contain one relay and follower nodes")
+	})
+
+	t.Run("ConfigJSONOverride does not parse", func(t *testing.T) {
+		t.Parallel()
+		tmpl := NetworkTemplate{
+			Genesis: devmodeGenesis,
+			Nodes: []remote.NodeConfigGoal{
+				{
+					IsRelay:            false,
+					ConfigJSONOverride: "DOES NOT PARSE",
+				},
+			},
+		}
+		require.ErrorContains(t, tmpl.Validate(), "unable to decode JSONOverride")
+	})
+
+	t.Run("ConfigJSONOverride unknown key", func(t *testing.T) {
+		t.Parallel()
+		tmpl := NetworkTemplate{
+			Genesis: devmodeGenesis,
+			Nodes: []remote.NodeConfigGoal{
+				{
+					IsRelay:            false,
+					ConfigJSONOverride: "{\"Unknown Key\": \"Valid JSON\"}",
+				},
+			},
+		}
+		require.ErrorContains(t, tmpl.Validate(), "json: unknown field \"Unknown Key\"")
+	})
+
+	t.Run("Valid multi-node DevMode", func(t *testing.T) {
+		t.Parallel()
+		tmpl := NetworkTemplate{
+			Genesis: devmodeGenesis,
+			Nodes: []remote.NodeConfigGoal{
+				{
+					IsRelay: true,
+				},
+				{
+					IsRelay:            false,
+					ConfigJSONOverride: "{\"EnableFollowMode\":true}",
+				},
+			},
+		}
+		// this one is fine.
+		require.NoError(t, tmpl.Validate())
+	})
+
+	t.Run("Valid two-follower DevMode", func(t *testing.T) {
+		t.Parallel()
+		tmpl := NetworkTemplate{
+			Genesis: devmodeGenesis,
+			Nodes: []remote.NodeConfigGoal{
+				{
+					IsRelay: true,
+				},
+				{
+					IsRelay:            false,
+					ConfigJSONOverride: "{\"EnableFollowMode\":true}",
+				},
+				{
+					IsRelay:            false,
+					ConfigJSONOverride: "{\"EnableFollowMode\":true}",
+				},
+			},
+		}
+		// this one is fine.
+		require.NoError(t, tmpl.Validate())
+	})
+}
+
 type overlayTestStruct struct {
 	A string
 	B string

node/follower_node.go

@@ -87,8 +87,7 @@ func MakeFollower(log logging.Logger, rootDir string, cfg config.Local, phoneboo
 	node.devMode = genesis.DevMode
 
 	if node.devMode {
-		log.Errorf("Cannot run follower node in devMode--submitting txns won't work")
-		return nil, fmt.Errorf("cannot run with both EnableFollowMode and DevMode")
+		log.Warn("Follower running on a devMode network. Must submit txns to a different node.")
 	}
 	node.config = cfg
 

node/follower_node_test.go

@@ -19,6 +19,8 @@ package node
 import (
 	"testing"
 
+	"github.com/sirupsen/logrus"
+	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/require"
 
 	"github.com/algorand/go-algorand/agreement"
@@ -32,10 +34,8 @@ import (
 	"github.com/algorand/go-algorand/test/partitiontest"
 )
 
-func setupFollowNode(t *testing.T) *AlgorandFollowerNode {
-	cfg := config.GetDefaultLocal()
-	cfg.EnableFollowMode = true
-	genesis := bookkeeping.Genesis{
+func followNodeDefaultGenesis() bookkeeping.Genesis {
+	return bookkeeping.Genesis{
 		SchemaID:    "go-test-follower-node-genesis",
 		Proto:       protocol.ConsensusCurrentVersion,
 		Network:     config.Devtestnet,
@@ -50,6 +50,12 @@ func setupFollowNode(t *testing.T) *AlgorandFollowerNode {
 			},
 		},
 	}
+}
+
+func setupFollowNode(t *testing.T) *AlgorandFollowerNode {
+	cfg := config.GetDefaultLocal()
+	cfg.EnableFollowMode = true
+	genesis := followNodeDefaultGenesis()
 	node, err := MakeFollower(logging.Base(), t.TempDir(), cfg, []string{}, genesis)
 	require.NoError(t, err)
 	return node
@@ -95,3 +101,29 @@ func TestErrors(t *testing.T) {
 	_, err = node.InstallParticipationKey([]byte{})
 	require.Error(t, err)
 }
+
+func TestDevModeWarning(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	cfg := config.GetDefaultLocal()
+	cfg.EnableFollowMode = true
+	genesis := followNodeDefaultGenesis()
+	genesis.DevMode = true
+
+	logger, hook := test.NewNullLogger()
+	tlogger := logging.NewWrappedLogger(logger)
+	_, err := MakeFollower(tlogger, t.TempDir(), cfg, []string{}, genesis)
+	require.NoError(t, err)
+
+	// check for the warning
+	var foundEntry *logrus.Entry
+	entries := hook.AllEntries()
+	for i := range entries {
+		if entries[i].Level == logrus.WarnLevel {
+			foundEntry = entries[i]
+		}
+	}
+	require.NotNil(t, foundEntry)
+	require.Contains(t, foundEntry.Message, "Follower running on a devMode network. Must submit txns to a different node.")
+}

node/node.go

@@ -183,10 +183,6 @@ func MakeFull(log logging.Logger, rootDir string, cfg config.Local, phonebookAdd
 	node.genesisID = genesis.ID()
 	node.genesisHash = genesis.Hash()
 	node.devMode = genesis.DevMode
-
-	if node.devMode {
-		cfg.DisableNetworking = true
-	}
 	node.config = cfg
 
 	// tie network, block fetcher, and agreement services together
@@ -485,7 +481,7 @@ func (node *AlgorandFullNode) writeDevmodeBlock() (err error) {
 	}
 
 	// add the newly generated block to the ledger
-	err = node.ledger.AddValidatedBlock(*vb, agreement.Certificate{})
+	err = node.ledger.AddValidatedBlock(*vb, agreement.Certificate{Round: vb.Block().Round()})
 	return err
 }