fixup! [GH-659] Basic protocol version change handling.

Author: Un1oR

nil/internal/collate/syncer.go

@@ -88,7 +88,7 @@ func (s *Syncer) WaitComplete() {
 	s.waitForSync.Wait()
 }
 
-func (s *Syncer) readLocalVersion(ctx context.Context) (*NodeVersion, error) {
+func (s *Syncer) getLocalVersion(ctx context.Context) (*NodeVersion, error) {
 	protocolVersion := s.networkManager.ProtocolVersion()
 
 	rotx, err := s.db.CreateRoTx(ctx)
@@ -134,7 +134,7 @@ func (s *Syncer) fetchRemoteVersion(ctx context.Context) (NodeVersion, error) {
 			return NodeVersion{protocolVersion, res}, nil
 		}
 	}
-	return NodeVersion{"", common.EmptyHash}, fmt.Errorf("failed to fetch version from all peers; last error: %w", err)
+	return NodeVersion{}, fmt.Errorf("failed to fetch version from all peers; last error: %w", err)
 }
 
 func (s *Syncer) fetchSnapshot(ctx context.Context) error {
@@ -158,7 +158,7 @@ func (s *Syncer) Init(ctx context.Context, allowDbDrop bool) error {
 		return nil
 	}
 
-	version, err := s.readLocalVersion(ctx)
+	version, err := s.getLocalVersion(ctx)
 	if err != nil {
 		return err
 	}

nil/internal/network/config.go

@@ -33,6 +33,8 @@ type Config struct {
 	Reachability network.Reachability `yaml:"-"`
 }
 
+type Option func(cfg *Config) error
+
 func NewDefaultConfig() *Config {
 	return &Config{
 		KeysPath:                "network-keys.yaml",
@@ -45,3 +47,17 @@ func NewDefaultConfig() *Config {
 func (c *Config) Enabled() bool {
 	return c.TcpPort != 0 || c.QuicPort != 0
 }
+
+// Apply applies the given options to the config, returning the first error
+// encountered (if any).
+func (c *Config) Apply(opts ...Option) error {
+	for _, opt := range opts {
+		if opt == nil {
+			continue
+		}
+		if err := opt(c); err != nil {
+			return err
+		}
+	}
+	return nil
+}

nil/internal/network/testaide.go

@@ -93,36 +93,6 @@ func topLevelTestName(t *testing.T) string {
 	return strings.Split(t.Name(), "/")[0]
 }
 
-// ConfigOverrides is a helper structure for configuring the config in tests.
-//
-// Why was it needed at all?
-// To understand this, let's consider how the config is filled when the application is running.
-// We have several layers of overrides:
-// - Default values set in the code
-// - They are merged with the values from the yaml file (overwriting in favor of the file)
-// - And they can be overridden by command line options (again with overwriting)
-//
-// What do the yaml file and command line options have in common?
-// Both of these sources have the semantics of absence of value.
-// That is, we can clearly distinguish, for example, false from the absence of a value,
-// and if a true was set at an earlier layer and there was no override at a later layer,
-// we should not turn true into false.
-//
-// It is important that the semantics of the absence of a value are not represented at the level
-// of the config structure itself in Go, so the task of merging two configs presented in this way becomes non-trivial.
-// We can easily accidentally overwrite the value from the base config with the default value from the override config.
-//
-// This is exactly why a separate structure was introduced, having an explicit semantics of absence of value.
-type ConfigOverrides struct {
-	ProtocolVersion *string
-}
-
-func (c *ConfigOverrides) ApplyToConfig(cfg *Config) {
-	if c.ProtocolVersion != nil {
-		cfg.ProtocolVersion = *c.ProtocolVersion
-	}
-}
-
 func GenerateConfig(t *testing.T, port int) (*Config, AddrInfo) {
 	t.Helper()
 

nil/tests/archive_node/archive_node_test.go renamed to nil/tests/archive_node/suite_archive_node.go

@@ -1,3 +1,5 @@
+//go:build test
+
 package tests
 
 import (
@@ -16,6 +18,13 @@ var (
 	newProtocolVersion = "new"
 )
 
+func protocolVersionOption(protocolVersion string) network.Option {
+	return func(c *network.Config) error {
+		c.ProtocolVersion = protocolVersion
+		return nil
+	}
+}
+
 type SuiteArchiveNode struct {
 	tests.ShardedSuite
 
@@ -43,12 +52,12 @@ func (s *SuiteArchiveNode) runArchiveNode(database db.DB) (*nilservice.Config, n
 	ctx, cancel := context.WithCancel(s.Context)
 	s.cancel = cancel
 	return s.RunArchiveNode(&tests.ArchiveNodeConfig{
-		Ctx:                    ctx,
-		Wg:                     &s.wg,
-		AllowDbDrop:            true,
-		Port:                   s.port + int(s.nShards),
-		WithBootstrapPeers:     s.withBootstrapPeers,
-		NetworkConfigOverrides: network.ConfigOverrides{ProtocolVersion: &oldProtocolVersion},
+		Ctx:                ctx,
+		Wg:                 &s.wg,
+		AllowDbDrop:        true,
+		Port:               s.port + int(s.nShards),
+		WithBootstrapPeers: s.withBootstrapPeers,
+		NetworkOptions:     []network.Option{protocolVersionOption(oldProtocolVersion)},
 	})
 }
 
@@ -69,10 +78,8 @@ func (s *SuiteArchiveNode) startCluster(protocolVersion string) {
 			NShards:              s.nShards,
 			CollatorTickPeriodMs: 200,
 		},
-		network.ConfigOverrides{
-			ProtocolVersion: &protocolVersion,
-		},
-		s.port)
+		s.port,
+		protocolVersionOption(protocolVersion))
 }
 
 func (s *SuiteArchiveNode) checkBlocksGeneration() {

nil/tests/async_await/async_await_test.go

@@ -83,7 +83,6 @@ func (s *SuiteAsyncAwait) SetupSuite() {
 			ZeroState:        zeroState,
 			DisableConsensus: disableConsensus,
 		},
-		network.ConfigOverrides{},
 		port)
 
 	_, archiveNodeAddr := s.StartArchiveNode(&tests.ArchiveNodeConfig{

nil/tests/cli/cli_test.go

@@ -14,7 +14,6 @@ import (
 	"github.com/NilFoundation/nil/nil/internal/contracts"
 	nilcrypto "github.com/NilFoundation/nil/nil/internal/crypto"
 	"github.com/NilFoundation/nil/nil/internal/execution"
-	"github.com/NilFoundation/nil/nil/internal/network"
 	"github.com/NilFoundation/nil/nil/internal/types"
 	"github.com/NilFoundation/nil/nil/services/cliservice"
 	"github.com/NilFoundation/nil/nil/services/cometa"
@@ -52,7 +51,6 @@ func (s *SuiteCliBase) SetupTest() {
 			NShards:              3,
 			CollatorTickPeriodMs: 200,
 		},
-		network.ConfigOverrides{},
 		s.basePort)
 
 	s.DefaultClient, s.endpoint = s.StartRPCNode(tests.WithDhtBootstrapByValidators, nil)

nil/tests/cli_call/cli_call_test.go

@@ -9,7 +9,6 @@ import (
 	"github.com/NilFoundation/nil/nil/internal/contracts"
 	nilcrypto "github.com/NilFoundation/nil/nil/internal/crypto"
 	"github.com/NilFoundation/nil/nil/internal/execution"
-	"github.com/NilFoundation/nil/nil/internal/network"
 	"github.com/NilFoundation/nil/nil/internal/types"
 	"github.com/NilFoundation/nil/nil/services/nilservice"
 	"github.com/NilFoundation/nil/nil/tests"
@@ -50,7 +49,6 @@ func (s *SuiteCliTestCall) SetupTest() {
 			CollatorTickPeriodMs: 200,
 			ZeroState:            zeroState,
 		},
-		network.ConfigOverrides{},
 		10525)
 
 	s.client, s.endpoint = s.StartRPCNode(tests.WithDhtBootstrapByValidators, nil)

nil/tests/faucet/faucet_test.go

@@ -8,7 +8,6 @@ import (
 	"github.com/NilFoundation/nil/nil/common"
 	"github.com/NilFoundation/nil/nil/common/logging"
 	"github.com/NilFoundation/nil/nil/internal/contracts"
-	"github.com/NilFoundation/nil/nil/internal/network"
 	"github.com/NilFoundation/nil/nil/internal/types"
 	"github.com/NilFoundation/nil/nil/services/faucet"
 	"github.com/NilFoundation/nil/nil/services/nilservice"
@@ -30,7 +29,6 @@ func (s *SuiteFaucet) SetupSuite() {
 			NShards:              3,
 			CollatorTickPeriodMs: 200,
 		},
-		network.ConfigOverrides{},
 		10225)
 
 	s.DefaultClient, _ = s.StartRPCNode(tests.WithDhtBootstrapByValidators, nil)

nil/tests/rpc_node/rpc_node_test.go

@@ -24,7 +24,6 @@ func (s *SuiteRpcNode) SetupTest() {
 			NShards: nShards,
 			RunMode: nilservice.NormalRunMode,
 		},
-		network.ConfigOverrides{},
 		port)
 
 	_, archiveNodeAddr := s.StartArchiveNode(&tests.ArchiveNodeConfig{

nil/tests/shard/shard_test.go

@@ -3,7 +3,6 @@ package main
 import (
 	"testing"
 
-	"github.com/NilFoundation/nil/nil/internal/network"
 	"github.com/NilFoundation/nil/nil/internal/types"
 	"github.com/NilFoundation/nil/nil/services/nilservice"
 	"github.com/NilFoundation/nil/nil/services/rpc/transport"
@@ -21,7 +20,6 @@ func (s *BasicShardSuite) SetupSuite() {
 			NShards:              3,
 			CollatorTickPeriodMs: 1000,
 		},
-		network.ConfigOverrides{},
 		10000)
 }
 

nil/tests/sharded_suite.go

@@ -172,7 +172,6 @@ func createAllShardsAllValidatorsCfg(
 
 func (s *ShardedSuite) start(
 	cfg *nilservice.Config,
-	networkConfigOverrides network.ConfigOverrides,
 	port int,
 	shardCfgGen func(
 		*ShardedSuite,
@@ -181,6 +180,7 @@ func (s *ShardedSuite) start(
 		*network.Config,
 		map[InstanceId]*keys.ValidatorKeysManager,
 	) *nilservice.Config,
+	options ...network.Option,
 ) {
 	s.T().Helper()
 	s.Context, s.ctxCancel = context.WithCancel(context.Background())
@@ -196,7 +196,7 @@ func (s *ShardedSuite) start(
 	instanceCount := cfg.NShards - 1
 	networkConfigs, p2pAddresses := network.GenerateConfigs(s.T(), instanceCount, port)
 	for _, networkConfig := range networkConfigs {
-		networkConfigOverrides.ApplyToConfig(networkConfig)
+		s.Require().NoError(networkConfig.Apply(options...))
 	}
 	keysManagers := make(map[InstanceId]*keys.ValidatorKeysManager)
 	s.Instances = make([]Instance, instanceCount)
@@ -255,16 +255,16 @@ func (s *ShardedSuite) start(
 	s.waitShardsTick(cfg.NShards)
 }
 
-func (s *ShardedSuite) Start(cfg *nilservice.Config, networkConfigOverrides network.ConfigOverrides, port int) {
+func (s *ShardedSuite) Start(cfg *nilservice.Config, port int, options ...network.Option) {
 	s.T().Helper()
 
-	s.start(cfg, networkConfigOverrides, port, createOneShardOneValidatorCfg)
+	s.start(cfg, port, createOneShardOneValidatorCfg, options...)
 }
 
 func (s *ShardedSuite) StartShardAllValidators(cfg *nilservice.Config, port int) {
 	s.T().Helper()
 
-	s.start(cfg, network.ConfigOverrides{}, port, createAllShardsAllValidatorsCfg)
+	s.start(cfg, port, createAllShardsAllValidatorsCfg)
 }
 
 func (s *ShardedSuite) connectToInstances(nm *network.Manager) {
@@ -288,13 +288,13 @@ func (s *ShardedSuite) GetNShards() uint32 {
 }
 
 type ArchiveNodeConfig struct {
-	Ctx                    context.Context
-	Wg                     *sync.WaitGroup
-	AllowDbDrop            bool
-	Port                   int
-	WithBootstrapPeers     bool
-	DisableConsensus       bool
-	NetworkConfigOverrides network.ConfigOverrides
+	Ctx                context.Context
+	Wg                 *sync.WaitGroup
+	AllowDbDrop        bool
+	Port               int
+	WithBootstrapPeers bool
+	DisableConsensus   bool
+	NetworkOptions     []network.Option
 }
 
 func (s *ShardedSuite) RunArchiveNode(params *ArchiveNodeConfig) (*nilservice.Config, network.AddrInfo, chan error) {
@@ -311,7 +311,7 @@ func (s *ShardedSuite) RunArchiveNode(params *ArchiveNodeConfig) (*nilservice.Co
 
 	s.Require().NotEmpty(s.Instances)
 	netCfg, addr := network.GenerateConfig(s.T(), params.Port)
-	params.NetworkConfigOverrides.ApplyToConfig(netCfg)
+	s.Require().NoError(netCfg.Apply(params.NetworkOptions...))
 	serviceName := fmt.Sprintf("archive-%d", params.Port)
 
 	cfg := &nilservice.Config{