Merge pull request #2968 from nspcc-dev/bad-cfg-panic

config: do not allow negative validators/committee count

docs/node-configuration.md

@@ -341,7 +341,7 @@ protocol-related settings described in the table below.
 
 | Section | Type | Default value | Description | Notes |
 | --- | --- | --- | --- | --- |
-| CommitteeHistory | map[uint32]int | none | Number of committee members after the given height, for example `{0: 1, 20: 4}` sets up a chain with one committee member since the genesis and then changes the setting to 4 committee members at the height of 20. `StandbyCommittee` committee setting must have the number of keys equal or exceeding the highest value in this option. Blocks numbers where the change happens must be divisible by the old and by the new values simultaneously. If not set, committee size is derived from the `StandbyCommittee` setting and never changes. |
+| CommitteeHistory | map[uint32]uint32 | none | Number of committee members after the given height, for example `{0: 1, 20: 4}` sets up a chain with one committee member since the genesis and then changes the setting to 4 committee members at the height of 20. `StandbyCommittee` committee setting must have the number of keys equal or exceeding the highest value in this option. Blocks numbers where the change happens must be divisible by the old and by the new values simultaneously. If not set, committee size is derived from the `StandbyCommittee` setting and never changes. |
 | GarbageCollectionPeriod | `uint32` | 10000 | Controls MPT garbage collection interval (in blocks) for configurations with `RemoveUntraceableBlocks` enabled and `KeepOnlyLatestState` disabled. In this mode the node stores a number of MPT trees (corresponding to `MaxTraceableBlocks` and `StateSyncInterval`), but the DB needs to be clean from old entries from time to time. Doing it too often will cause too much processing overhead, doing it too rarely will leave more useless data in the DB. This setting is deprecated in favor of the same setting in the ApplicationConfiguration and will be removed in future node versions. If both settings are used, ApplicationConfiguration is prioritized over this one. |
 | Hardforks | `map[string]uint32` | [] | The set of incompatible changes that affect node behaviour starting from the specified height. The default value is an empty set which should be interpreted as "each known hard-fork is applied from the zero blockchain height". The list of valid hard-fork names:<br>• `Aspidochelone` represents hard-fork introduced in [#2469](https://github.com/nspcc-dev/neo-go/pull/2469) (ported from the [reference](https://github.com/neo-project/neo/pull/2712)). It adjusts the prices of `System.Contract.CreateStandardAccount` and `System.Contract.CreateMultisigAccount` interops so that the resulting prices are in accordance with `sha256` method of native `CryptoLib` contract. It also includes [#2519](https://github.com/nspcc-dev/neo-go/pull/2519) (ported from the [reference](https://github.com/neo-project/neo/pull/2749)) that adjusts the price of `System.Runtime.GetRandom` interop and fixes its vulnerability. A special NeoGo-specific change is included as well for ContractManagement's update/deploy call flags behaviour to be compatible with pre-0.99.0 behaviour that was changed because of the [3.2.0 protocol change](https://github.com/neo-project/neo/pull/2653). |
 | KeepOnlyLatestState | `bool` | `false` | Specifies if MPT should only store the latest state (or a set of latest states, see `P2PStateExcangeExtensions` section for details). If true, DB size will be smaller, but older roots won't be accessible. This value should remain the same for the same database. | This setting is deprecated in favor of the same setting in the ApplicationConfiguration and will be removed in future node versions. If both settings are used, setting any of them to true enables the function. |
@@ -365,7 +365,7 @@ protocol-related settings described in the table below.
 | StateRootInHeader | `bool` | `false` | Enables storing state root in block header. | Experimental protocol extension! |
 | StateSyncInterval | `int` | `40000` | The number of blocks between state heights available for MPT state data synchronization. | `P2PStateExchangeExtensions` should be enabled to use this setting. |
 | TimePerBlock | `Duration` | `15s` | Minimal (and targeted for) time interval between blocks. Must be an integer number of milliseconds. |
-| ValidatorsCount | `int` | `0` | Number of validators set for the whole network lifetime, can't be set if `ValidatorsHistory` setting is used. |
-| ValidatorsHistory | map[uint32]int | none | Number of consensus nodes to use after given height (see `CommitteeHistory` also). Heights where the change occurs must be divisible by the number of committee members at that height. Can't be used with `ValidatorsCount` not equal to zero. |
+| ValidatorsCount | `uint32` | `0` | Number of validators set for the whole network lifetime, can't be set if `ValidatorsHistory` setting is used. |
+| ValidatorsHistory | map[uint32]uint32 | none | Number of consensus nodes to use after given height (see `CommitteeHistory` also). Heights where the change occurs must be divisible by the number of committee members at that height. Can't be used with `ValidatorsCount` not equal to zero. |
 | VerifyBlocks | `bool` | `false` | This setting is deprecated and no longer works, please use `SkipBlockVerification` in the `ApplicationConfiguration`, it will be removed in future node versions. |
 | VerifyTransactions | `bool` | `false` | Denotes whether to verify transactions in the received blocks. |

pkg/config/protocol_config.go

@@ -15,7 +15,7 @@ import (
 type (
 	ProtocolConfiguration struct {
 		// CommitteeHistory stores committee size change history (height: size).
-		CommitteeHistory map[uint32]int `yaml:"CommitteeHistory"`
+		CommitteeHistory map[uint32]uint32 `yaml:"CommitteeHistory"`
 		// GarbageCollectionPeriod sets the number of blocks to wait before
 		// starting the next MPT garbage collection cycle when RemoveUntraceableBlocks
 		// option is used.
@@ -82,9 +82,9 @@ type (
 		// TimePerBlock is the time interval between blocks that consensus nodes work with.
 		// It must be an integer number of milliseconds.
 		TimePerBlock    time.Duration `yaml:"TimePerBlock"`
-		ValidatorsCount int           `yaml:"ValidatorsCount"`
+		ValidatorsCount uint32        `yaml:"ValidatorsCount"`
 		// Validators stores history of changes to consensus node number (height: number).
-		ValidatorsHistory map[uint32]int `yaml:"ValidatorsHistory"`
+		ValidatorsHistory map[uint32]uint32 `yaml:"ValidatorsHistory"`
 		// Whether to verify received blocks.
 		//
 		// Deprecated: please use the same setting in the ApplicationConfiguration, this field will be removed in future versions.
@@ -97,7 +97,7 @@ type (
 // heightNumber is an auxiliary structure for configuration checks.
 type heightNumber struct {
 	h uint32
-	n int
+	n uint32
 }
 
 // Validate checks ProtocolConfiguration for internal consistency and returns
@@ -125,11 +125,14 @@ func (p *ProtocolConfiguration) Validate() error {
 	if p.ValidatorsCount != 0 && len(p.ValidatorsHistory) != 0 {
 		return errors.New("configuration should either have ValidatorsCount or ValidatorsHistory, not both")
 	}
-	if len(p.StandbyCommittee) < p.ValidatorsCount {
+	if len(p.StandbyCommittee) < int(p.ValidatorsCount) {
 		return errors.New("validators count can't exceed the size of StandbyCommittee")
 	}
 	var arr = make([]heightNumber, 0, len(p.CommitteeHistory))
 	for h, n := range p.CommitteeHistory {
+		if n == 0 {
+			return fmt.Errorf("invalid CommitteeHistory: bad members count (%d) for height %d", n, h)
+		}
 		if int(n) > len(p.StandbyCommittee) {
 			return fmt.Errorf("too small StandbyCommittee for required number of committee members at %d", h)
 		}
@@ -148,6 +151,9 @@ func (p *ProtocolConfiguration) Validate() error {
 	}
 	arr = arr[:0]
 	for h, n := range p.ValidatorsHistory {
+		if n == 0 {
+			return fmt.Errorf("invalid ValidatorsHistory: bad members count (%d) for height %d", n, h)
+		}
 		if int(n) > len(p.StandbyCommittee) {
 			return fmt.Errorf("too small StandbyCommittee for required number of validators at %d", h)
 		}
@@ -187,11 +193,11 @@ func (p *ProtocolConfiguration) GetCommitteeSize(height uint32) int {
 	if len(p.CommitteeHistory) == 0 {
 		return len(p.StandbyCommittee)
 	}
-	return getBestFromMap(p.CommitteeHistory, height)
+	return int(getBestFromMap(p.CommitteeHistory, height))
 }
 
-func getBestFromMap(dict map[uint32]int, height uint32) int {
-	var res int
+func getBestFromMap(dict map[uint32]uint32, height uint32) uint32 {
+	var res uint32
 	var bestH = uint32(0)
 	for h, n := range dict {
 		if h >= bestH && h <= height {
@@ -206,9 +212,9 @@ func getBestFromMap(dict map[uint32]int, height uint32) int {
 // It implies valid configuration file.
 func (p *ProtocolConfiguration) GetNumOfCNs(height uint32) int {
 	if len(p.ValidatorsHistory) == 0 {
-		return p.ValidatorsCount
+		return int(p.ValidatorsCount)
 	}
-	return getBestFromMap(p.ValidatorsHistory, height)
+	return int(getBestFromMap(p.ValidatorsHistory, height))
 }
 
 // ShouldUpdateCommitteeAt answers the question of whether the committee

pkg/config/protocol_config_test.go

@@ -52,7 +52,7 @@ func TestProtocolConfigurationValidation(t *testing.T) {
 			"02a7bc55fe8684e0119768d104ba30795bdcc86619e864add26156723ed185cd62",
 		},
 		ValidatorsCount:   4,
-		ValidatorsHistory: map[uint32]int{0: 4},
+		ValidatorsHistory: map[uint32]uint32{0: 4},
 	}
 	require.Error(t, p.Validate())
 	p = &ProtocolConfiguration{
@@ -62,8 +62,8 @@ func TestProtocolConfigurationValidation(t *testing.T) {
 			"03d90c07df63e690ce77912e10ab51acc944b66860237b608c4f8f8309e71ee699",
 			"02a7bc55fe8684e0119768d104ba30795bdcc86619e864add26156723ed185cd62",
 		},
-		CommitteeHistory:  map[uint32]int{0: 4},
-		ValidatorsHistory: map[uint32]int{0: 4, 1000: 5},
+		CommitteeHistory:  map[uint32]uint32{0: 4},
+		ValidatorsHistory: map[uint32]uint32{0: 4, 1000: 5},
 	}
 	require.Error(t, p.Validate())
 	p = &ProtocolConfiguration{
@@ -73,8 +73,8 @@ func TestProtocolConfigurationValidation(t *testing.T) {
 			"03d90c07df63e690ce77912e10ab51acc944b66860237b608c4f8f8309e71ee699",
 			"02a7bc55fe8684e0119768d104ba30795bdcc86619e864add26156723ed185cd62",
 		},
-		CommitteeHistory:  map[uint32]int{0: 4, 1000: 5},
-		ValidatorsHistory: map[uint32]int{0: 4},
+		CommitteeHistory:  map[uint32]uint32{0: 4, 1000: 5},
+		ValidatorsHistory: map[uint32]uint32{0: 4},
 	}
 	require.Error(t, p.Validate())
 	p = &ProtocolConfiguration{
@@ -84,8 +84,8 @@ func TestProtocolConfigurationValidation(t *testing.T) {
 			"03d90c07df63e690ce77912e10ab51acc944b66860237b608c4f8f8309e71ee699",
 			"02a7bc55fe8684e0119768d104ba30795bdcc86619e864add26156723ed185cd62",
 		},
-		CommitteeHistory:  map[uint32]int{0: 1, 999: 4},
-		ValidatorsHistory: map[uint32]int{0: 1},
+		CommitteeHistory:  map[uint32]uint32{0: 1, 999: 4},
+		ValidatorsHistory: map[uint32]uint32{0: 1},
 	}
 	require.Error(t, p.Validate())
 	p = &ProtocolConfiguration{
@@ -95,8 +95,8 @@ func TestProtocolConfigurationValidation(t *testing.T) {
 			"03d90c07df63e690ce77912e10ab51acc944b66860237b608c4f8f8309e71ee699",
 			"02a7bc55fe8684e0119768d104ba30795bdcc86619e864add26156723ed185cd62",
 		},
-		CommitteeHistory:  map[uint32]int{0: 1, 1000: 4},
-		ValidatorsHistory: map[uint32]int{0: 1, 999: 4},
+		CommitteeHistory:  map[uint32]uint32{0: 1, 1000: 4},
+		ValidatorsHistory: map[uint32]uint32{0: 1, 999: 4},
 	}
 	require.Error(t, p.Validate())
 	p = &ProtocolConfiguration{
@@ -106,8 +106,30 @@ func TestProtocolConfigurationValidation(t *testing.T) {
 			"03d90c07df63e690ce77912e10ab51acc944b66860237b608c4f8f8309e71ee699",
 			"02a7bc55fe8684e0119768d104ba30795bdcc86619e864add26156723ed185cd62",
 		},
-		CommitteeHistory:  map[uint32]int{0: 1, 100: 4},
-		ValidatorsHistory: map[uint32]int{0: 4, 100: 4},
+		CommitteeHistory:  map[uint32]uint32{0: 1, 100: 4},
+		ValidatorsHistory: map[uint32]uint32{0: 4, 100: 4},
+	}
+	require.Error(t, p.Validate())
+	p = &ProtocolConfiguration{
+		StandbyCommittee: []string{
+			"02b3622bf4017bdfe317c58aed5f4c753f206b7db896046fa7d774bbc4bf7f8dc2",
+			"02103a7f7dd016558597f7960d27c516a4394fd968b9e65155eb4b013e4040406e",
+			"03d90c07df63e690ce77912e10ab51acc944b66860237b608c4f8f8309e71ee699",
+			"02a7bc55fe8684e0119768d104ba30795bdcc86619e864add26156723ed185cd62",
+		},
+		CommitteeHistory:  map[uint32]uint32{0: 0, 100: 4},
+		ValidatorsHistory: map[uint32]uint32{0: 1, 100: 4},
+	}
+	require.Error(t, p.Validate())
+	p = &ProtocolConfiguration{
+		StandbyCommittee: []string{
+			"02b3622bf4017bdfe317c58aed5f4c753f206b7db896046fa7d774bbc4bf7f8dc2",
+			"02103a7f7dd016558597f7960d27c516a4394fd968b9e65155eb4b013e4040406e",
+			"03d90c07df63e690ce77912e10ab51acc944b66860237b608c4f8f8309e71ee699",
+			"02a7bc55fe8684e0119768d104ba30795bdcc86619e864add26156723ed185cd62",
+		},
+		CommitteeHistory:  map[uint32]uint32{0: 1, 100: 4},
+		ValidatorsHistory: map[uint32]uint32{0: 0, 100: 4},
 	}
 	require.Error(t, p.Validate())
 	p = &ProtocolConfiguration{
@@ -123,8 +145,8 @@ func TestProtocolConfigurationValidation(t *testing.T) {
 			"03d90c07df63e690ce77912e10ab51acc944b66860237b608c4f8f8309e71ee699",
 			"02a7bc55fe8684e0119768d104ba30795bdcc86619e864add26156723ed185cd62",
 		},
-		CommitteeHistory:  map[uint32]int{0: 1, 100: 4},
-		ValidatorsHistory: map[uint32]int{0: 1, 100: 4},
+		CommitteeHistory:  map[uint32]uint32{0: 1, 100: 4},
+		ValidatorsHistory: map[uint32]uint32{0: 1, 100: 4},
 	}
 	require.NoError(t, p.Validate())
 }
@@ -137,8 +159,8 @@ func TestGetCommitteeAndCNs(t *testing.T) {
 			"03d90c07df63e690ce77912e10ab51acc944b66860237b608c4f8f8309e71ee699",
 			"02a7bc55fe8684e0119768d104ba30795bdcc86619e864add26156723ed185cd62",
 		},
-		CommitteeHistory:  map[uint32]int{0: 1, 100: 4},
-		ValidatorsHistory: map[uint32]int{0: 1, 200: 4},
+		CommitteeHistory:  map[uint32]uint32{0: 1, 100: 4},
+		ValidatorsHistory: map[uint32]uint32{0: 1, 200: 4},
 	}
 	require.Equal(t, 1, p.GetCommitteeSize(0))
 	require.Equal(t, 1, p.GetCommitteeSize(99))
@@ -173,8 +195,8 @@ func TestProtocolConfigurationEquals(t *testing.T) {
 	o = &cfg2.ProtocolConfiguration
 	require.True(t, p.Equals(o))
 
-	o.CommitteeHistory = map[uint32]int{111: 7}
-	p.CommitteeHistory = map[uint32]int{111: 7}
+	o.CommitteeHistory = map[uint32]uint32{111: 7}
+	p.CommitteeHistory = map[uint32]uint32{111: 7}
 	require.True(t, p.Equals(o))
 	p.CommitteeHistory[111] = 8
 	require.False(t, p.Equals(o))
@@ -220,9 +242,9 @@ func TestProtocolConfigurationEquals(t *testing.T) {
 	p.StandbyCommittee = nil
 	o.StandbyCommittee = nil
 
-	o.ValidatorsHistory = map[uint32]int{111: 0}
-	p.ValidatorsHistory = map[uint32]int{111: 0}
+	o.ValidatorsHistory = map[uint32]uint32{111: 0}
+	p.ValidatorsHistory = map[uint32]uint32{111: 0}
 	require.True(t, p.Equals(o))
-	p.ValidatorsHistory = map[uint32]int{112: 0}
+	p.ValidatorsHistory = map[uint32]uint32{112: 0}
 	require.False(t, p.Equals(o))
 }

pkg/core/blockchain_core_test.go

@@ -247,12 +247,12 @@ func TestBlockchain_InitWithIncompleteStateJump(t *testing.T) {
 func TestChainWithVolatileNumOfValidators(t *testing.T) {
 	bc := newTestChainWithCustomCfg(t, func(c *config.Config) {
 		c.ProtocolConfiguration.ValidatorsCount = 0
-		c.ProtocolConfiguration.CommitteeHistory = map[uint32]int{
+		c.ProtocolConfiguration.CommitteeHistory = map[uint32]uint32{
 			0:  1,
 			4:  4,
 			24: 6,
 		}
-		c.ProtocolConfiguration.ValidatorsHistory = map[uint32]int{
+		c.ProtocolConfiguration.ValidatorsHistory = map[uint32]uint32{
 			0: 1,
 			4: 4,
 		}

pkg/neorpc/result/version.go

@@ -34,13 +34,13 @@ type (
 		// returned by the server in case they're enabled.
 
 		// CommitteeHistory stores height:size map of the committee size.
-		CommitteeHistory map[uint32]int
+		CommitteeHistory map[uint32]uint32
 		// P2PSigExtensions is true when Notary subsystem is enabled on the network.
 		P2PSigExtensions bool
 		// StateRootInHeader is true if state root is contained in block header.
 		StateRootInHeader bool
 		// ValidatorsHistory stores height:size map of the validators count.
-		ValidatorsHistory map[uint32]int
+		ValidatorsHistory map[uint32]uint32
 	}
 
 	// protocolMarshallerAux is an auxiliary struct used for Protocol JSON marshalling.
@@ -55,10 +55,10 @@ type (
 		ValidatorsCount             byte          `json:"validatorscount"`
 		InitialGasDistribution      int64         `json:"initialgasdistribution"`
 
-		CommitteeHistory  map[uint32]int `json:"committeehistory,omitempty"`
-		P2PSigExtensions  bool           `json:"p2psigextensions,omitempty"`
-		StateRootInHeader bool           `json:"staterootinheader,omitempty"`
-		ValidatorsHistory map[uint32]int `json:"validatorshistory,omitempty"`
+		CommitteeHistory  map[uint32]uint32 `json:"committeehistory,omitempty"`
+		P2PSigExtensions  bool              `json:"p2psigextensions,omitempty"`
+		StateRootInHeader bool              `json:"staterootinheader,omitempty"`
+		ValidatorsHistory map[uint32]uint32 `json:"validatorshistory,omitempty"`
 	}
 )
 

pkg/rpcclient/actor/maker.go

@@ -214,7 +214,7 @@ func (a *Actor) CalculateValidUntilBlock() (uint32, error) {
 	if err != nil {
 		return 0, fmt.Errorf("can't get block count: %w", err)
 	}
-	var vc = int(a.version.Protocol.ValidatorsCount)
+	var vc = uint32(a.version.Protocol.ValidatorsCount)
 	var bestH = uint32(0)
 	for h, n := range a.version.Protocol.ValidatorsHistory { // In case it's enabled.
 		if h >= bestH && h <= blockCount {
@@ -223,5 +223,5 @@ func (a *Actor) CalculateValidUntilBlock() (uint32, error) {
 		}
 	}
 
-	return blockCount + uint32(vc+1), nil
+	return blockCount + vc + 1, nil
 }

pkg/rpcclient/actor/maker_test.go

@@ -25,7 +25,7 @@ func TestCalculateValidUntilBlock(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, uint32(42+7+1), vub)
 
-	client.version.Protocol.ValidatorsHistory = map[uint32]int{
+	client.version.Protocol.ValidatorsHistory = map[uint32]uint32{
 		0:  7,
 		40: 4,
 		80: 10,