Merge pull request #3104 from nspcc-dev/hardforks-check

core: adjust hardfork enabling logic

pkg/config/hardfork.go

@@ -15,14 +15,23 @@ const (
 	// https://github.com/neo-project/neo/pull/2883) and #3085 (ported from
 	// https://github.com/neo-project/neo/pull/2810).
 	HFBasilisk // Basilisk
+	// hfLast denotes the end of hardforks enum. Consider adding new hardforks
+	// before hfLast.
+	hfLast
 )
 
+// Hardforks represents the ordered slice of all possible hardforks.
+var Hardforks []Hardfork
+
 // hardforks holds a map of Hardfork string representation to its type.
 var hardforks map[string]Hardfork
 
 func init() {
-	hardforks = make(map[string]Hardfork)
+	for i := HFAspidochelone; i < hfLast; i = i << 1 {
-	for _, hf := range []Hardfork{HFAspidochelone, HFBasilisk} {
+		Hardforks = append(Hardforks, i)
+	}
+	hardforks = make(map[string]Hardfork, len(Hardforks))
+	for _, hf := range Hardforks {
 		hardforks[hf.String()] = hf
 	}
 }

pkg/config/protocol_config.go

@@ -122,6 +122,24 @@ func (p *ProtocolConfiguration) Validate() error {
 			return fmt.Errorf("Hardforks configuration section contains unexpected hardfork: %s", name)
 		}
 	}
+	var (
+		prev             uint32
+		shouldBeDisabled bool
+	)
+	for _, cfgHf := range Hardforks {
+		h := p.Hardforks[cfgHf.String()]
+		if h != 0 && shouldBeDisabled {
+			return fmt.Errorf("missing previous hardfork configuration with %s present", cfgHf.String())
+		}
+		if h != 0 && h < prev {
+			return fmt.Errorf("hardfork %s has inconsistent enabling height %d (lower than the previouse one)", cfgHf.String(), h)
+		}
+		if h != 0 {
+			prev = h
+		} else if prev != 0 {
+			shouldBeDisabled = true
+		}
+	}
 	if p.ValidatorsCount != 0 && len(p.ValidatorsHistory) != 0 {
 		return errors.New("configuration should either have ValidatorsCount or ValidatorsHistory, not both")
 	}

pkg/config/protocol_config_test.go

@@ -132,12 +132,6 @@ func TestProtocolConfigurationValidation(t *testing.T) {
 		ValidatorsHistory: map[uint32]uint32{0: 0, 100: 4},
 	}
 	require.Error(t, p.Validate())
-	p = &ProtocolConfiguration{
-		Hardforks: map[string]uint32{
-			"Unknown": 123, // Unknown hard-fork.
-		},
-	}
-	require.Error(t, p.Validate())
 	p = &ProtocolConfiguration{
 		StandbyCommittee: []string{
 			"02b3622bf4017bdfe317c58aed5f4c753f206b7db896046fa7d774bbc4bf7f8dc2",
@@ -151,6 +145,48 @@ func TestProtocolConfigurationValidation(t *testing.T) {
 	require.NoError(t, p.Validate())
 }
 
+func TestProtocolConfigurationValidation_Hardforks(t *testing.T) {
+	p := &ProtocolConfiguration{
+		Hardforks: map[string]uint32{
+			"Unknown": 123, // Unknown hard-fork.
+		},
+	}
+	require.Error(t, p.Validate())
+	p = &ProtocolConfiguration{
+		Hardforks: map[string]uint32{
+			"Aspidochelone": 2,
+			"Basilisk":      1, // Lower height in higher hard-fork.
+		},
+	}
+	require.Error(t, p.Validate())
+	p = &ProtocolConfiguration{
+		Hardforks: map[string]uint32{
+			"Aspidochelone": 2,
+			"Basilisk":      2, // Same height is OK.
+		},
+	}
+	require.NoError(t, p.Validate())
+	p = &ProtocolConfiguration{
+		Hardforks: map[string]uint32{
+			"Aspidochelone": 2,
+			"Basilisk":      3, // Larger height is OK.
+		},
+	}
+	require.NoError(t, p.Validate())
+	p = &ProtocolConfiguration{
+		Hardforks: map[string]uint32{
+			"Aspidochelone": 2,
+		},
+	}
+	require.NoError(t, p.Validate())
+	p = &ProtocolConfiguration{
+		Hardforks: map[string]uint32{
+			"Basilisk": 2,
+		},
+	}
+	require.NoError(t, p.Validate())
+}
+
 func TestGetCommitteeAndCNs(t *testing.T) {
 	p := &ProtocolConfiguration{
 		StandbyCommittee: []string{

pkg/core/blockchain.go

@@ -290,7 +290,20 @@ func NewBlockchain(s storage.Store, cfg config.Blockchain, log *zap.Logger) (*Bl
 	}
 	if cfg.Hardforks == nil {
 		cfg.Hardforks = map[string]uint32{}
+		for _, hf := range config.Hardforks {
+			cfg.Hardforks[hf.String()] = 0
+		}
 		log.Info("Hardforks are not set, using default value")
+	} else {
+		// Explicitly set the height of all old omitted hardforks to 0 for proper
+		// IsHardforkEnabled behaviour.
+		for _, hf := range config.Hardforks {
+			if _, ok := cfg.Hardforks[hf.String()]; !ok {
+				cfg.Hardforks[hf.String()] = 0
+				continue
+			}
+			break
+		}
 	}
 	// Compatibility with the old ProtocolConfiguration.
 	if cfg.ProtocolConfiguration.GarbageCollectionPeriod > 0 && cfg.Ledger.GarbageCollectionPeriod == 0 { //nolint:staticcheck // SA1019: cfg.ProtocolConfiguration.GarbageCollectionPeriod is deprecated

pkg/core/blockchain_core_test.go

@@ -360,3 +360,45 @@ func TestBlockchain_IsRunning(t *testing.T) {
 	chain.Close()
 	require.False(t, chain.isRunning.Load().(bool))
 }
+
+func TestNewBlockchain_InitHardforks(t *testing.T) {
+	t.Run("empty set", func(t *testing.T) {
+		bc := newTestChainWithCustomCfg(t, func(c *config.Config) {
+			c.ProtocolConfiguration.Hardforks = map[string]uint32{}
+			require.NoError(t, c.ProtocolConfiguration.Validate())
+		})
+		require.Equal(t, map[string]uint32{
+			config.HFAspidochelone.String(): 0,
+			config.HFBasilisk.String():      0,
+		}, bc.GetConfig().Hardforks)
+	})
+	t.Run("missing old", func(t *testing.T) {
+		bc := newTestChainWithCustomCfg(t, func(c *config.Config) {
+			c.ProtocolConfiguration.Hardforks = map[string]uint32{config.HFBasilisk.String(): 5}
+			require.NoError(t, c.ProtocolConfiguration.Validate())
+		})
+		require.Equal(t, map[string]uint32{
+			config.HFAspidochelone.String(): 0,
+			config.HFBasilisk.String():      5,
+		}, bc.GetConfig().Hardforks)
+	})
+	t.Run("missing new", func(t *testing.T) {
+		bc := newTestChainWithCustomCfg(t, func(c *config.Config) {
+			c.ProtocolConfiguration.Hardforks = map[string]uint32{config.HFAspidochelone.String(): 5}
+			require.NoError(t, c.ProtocolConfiguration.Validate())
+		})
+		require.Equal(t, map[string]uint32{
+			config.HFAspidochelone.String(): 5,
+		}, bc.GetConfig().Hardforks)
+	})
+	t.Run("all present", func(t *testing.T) {
+		bc := newTestChainWithCustomCfg(t, func(c *config.Config) {
+			c.ProtocolConfiguration.Hardforks = map[string]uint32{config.HFAspidochelone.String(): 5, config.HFBasilisk.String(): 10}
+			require.NoError(t, c.ProtocolConfiguration.Validate())
+		})
+		require.Equal(t, map[string]uint32{
+			config.HFAspidochelone.String(): 5,
+			config.HFBasilisk.String():      10,
+		}, bc.GetConfig().Hardforks)
+	})
+}

pkg/core/interop/context.go

@@ -405,7 +405,8 @@ func (ic *Context) IsHardforkEnabled(hf config.Hardfork) bool {
 	if ok {
 		return ic.BlockHeight() >= height
 	}
-	return len(ic.Hardforks) == 0 // Enable each hard-fork by default.
+	// Completely rely on proper hardforks initialisation made by core.NewBlockchain.
+	return false
 }
 
 // AddNotification creates notification event and appends it to the notification list.

pkg/core/interop/context_test.go

@@ -0,0 +1,35 @@
+package interop
+
+import (
+	"testing"
+
+	"github.com/nspcc-dev/neo-go/pkg/config"
+	"github.com/nspcc-dev/neo-go/pkg/core/block"
+	"github.com/stretchr/testify/require"
+)
+
+func TestIsHardforkEnabled(t *testing.T) {
+	t.Run("not configured", func(t *testing.T) {
+		ic := &Context{Hardforks: map[string]uint32{config.HFAspidochelone.String(): 0, config.HFBasilisk.String(): 0}, Block: &block.Block{Header: block.Header{Index: 10}}}
+		require.True(t, ic.IsHardforkEnabled(config.HFAspidochelone))
+		require.True(t, ic.IsHardforkEnabled(config.HFBasilisk))
+	})
+	t.Run("new disabled", func(t *testing.T) {
+		ic := &Context{Hardforks: map[string]uint32{config.HFAspidochelone.String(): 5}, Block: &block.Block{Header: block.Header{Index: 10}}}
+		require.True(t, ic.IsHardforkEnabled(config.HFAspidochelone))
+		require.False(t, ic.IsHardforkEnabled(config.HFBasilisk))
+	})
+	t.Run("old enabled", func(t *testing.T) {
+		ic := &Context{Hardforks: map[string]uint32{config.HFAspidochelone.String(): 0, config.HFBasilisk.String(): 10}, Block: &block.Block{Header: block.Header{Index: 5}}}
+		require.True(t, ic.IsHardforkEnabled(config.HFAspidochelone))
+		require.False(t, ic.IsHardforkEnabled(config.HFBasilisk))
+	})
+	t.Run("not yet enabled", func(t *testing.T) {
+		ic := &Context{Hardforks: map[string]uint32{config.HFAspidochelone.String(): 10}, Block: &block.Block{Header: block.Header{Index: 5}}}
+		require.False(t, ic.IsHardforkEnabled(config.HFAspidochelone))
+	})
+	t.Run("already enabled", func(t *testing.T) {
+		ic := &Context{Hardforks: map[string]uint32{config.HFAspidochelone.String(): 10}, Block: &block.Block{Header: block.Header{Index: 15}}}
+		require.True(t, ic.IsHardforkEnabled(config.HFAspidochelone))
+	})
+}