Skip to content

Commit

Permalink
ir/config: Fix unenforced morph.consensus.p2p.peers.min config defa…
Browse files Browse the repository at this point in the history
…ult (#2856)
  • Loading branch information
roman-khimov authored May 26, 2024
2 parents 9a9a5d5 + 3930c67 commit ea78a2d
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Changelog for NeoFS Node
### Added

### Fixed
- Unenforced IR `morph.consensus.p2p.peers.min` config default (#2856)

### Changed

Expand Down
14 changes: 10 additions & 4 deletions pkg/innerring/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ func parseBlockchainConfig(v *viper.Viper, _logger *zap.Logger) (c blockchain.Co
}
}

minPeersConfigured := false
const p2pSection = cfgPathFSChainLocalConsensus + ".p2p"
if v.IsSet(p2pSection) {
c.P2P.DialTimeout, err = parseConfigDurationPositive(v, p2pSection+".dial_timeout", "P2P dial timeout")
Expand All @@ -202,11 +203,12 @@ func parseBlockchainConfig(v *viper.Viper, _logger *zap.Logger) (c blockchain.Co
if !errors.Is(err, errMissingConfig) {
return c, err
}
// we calculate default here since explicit 0 is also a valid setting
n := uint64(len(c.Committee))
minPeers = n - (n-1)/3 - 1
// defaults below
} else {
c.P2P.MinPeers = uint(minPeers)
// zero can be set explicitly, so prevent overriding it
minPeersConfigured = true
}
c.P2P.MinPeers = uint(minPeers)
maxPeers, err := parseConfigUint64Range(v, p2pPeersSection+".max", "maximum number of P2P peers", 1, math.MaxInt32)
if err != nil && !errors.Is(err, errMissingConfig) {
return c, err
Expand All @@ -230,6 +232,10 @@ func parseBlockchainConfig(v *viper.Viper, _logger *zap.Logger) (c blockchain.Co
}
}
}
if !minPeersConfigured {
n := uint(len(c.Committee))
c.P2P.MinPeers = n - (n-1)/3 - 1
}

c.SetRolesInGenesis, err = parseConfigBool(v, cfgPathFSChainLocalConsensus+".set_roles_in_genesis", "flag to set roles for the committee in the genesis block")
if err != nil && !errors.Is(err, errMissingConfig) {
Expand Down
61 changes: 61 additions & 0 deletions pkg/innerring/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,16 @@ func resetConfig(tb testing.TB, v *viper.Viper, key string) {
}
}

func commiteeN(t testing.TB, n int) []string {
res := make([]string, n)
for i := range res {
k, err := keys.NewPrivateKey()
require.NoError(t, err)
res[i] = k.PublicKey().StringCompressed()
}
return res
}

func TestParseBlockchainConfig(t *testing.T) {
fullConfig := true
_logger := zap.NewNop()
Expand All @@ -143,6 +153,7 @@ func TestParseBlockchainConfig(t *testing.T) {
NetworkMagic: 15405,
Committee: validCommittee,
Storage: blockchain.BoltDB("chain.db"),
P2P: blockchain.P2PConfig{MinPeers: 2},
}, c)
})

Expand Down Expand Up @@ -570,3 +581,53 @@ fschain_autodeploy: true
require.False(t, b)
})
}

func TestP2PMinPeers(t *testing.T) {
l := zap.NewNop()
assert := func(t testing.TB, v *viper.Viper, exp uint) {
c, err := parseBlockchainConfig(v, l)
require.NoError(t, err)
require.EqualValues(t, exp, c.P2P.MinPeers)
}

v := newValidBlockchainConfig(t, true)
v.Set("morph.consensus.p2p.peers.min", 123)
assert(t, v, 123)

t.Run("explicit zero", func(t *testing.T) {
v := newValidBlockchainConfig(t, false)
v.Set("morph.consensus.p2p.peers.min", 0)
assert(t, v, 0)
})
t.Run("default", func(t *testing.T) {
assertDefault := func(t testing.TB, v *viper.Viper) {
setCommitteeN := func(n int) {
v.Set("morph.consensus.committee", commiteeN(t, n))
resetConfig(t, v, "morph.consensus.validators_history") // checked against committee size
}
setCommitteeN(4)
assert(t, v, 2)
setCommitteeN(7)
assert(t, v, 4)
setCommitteeN(21)
assert(t, v, 14)
}
t.Run("missing P2P section", func(t *testing.T) {
v := newValidBlockchainConfig(t, true)
resetConfig(t, v, "morph.consensus.p2p")
assertDefault(t, v)
})
t.Run("missing peers section", func(t *testing.T) {
v := newValidBlockchainConfig(t, true)
resetConfig(t, v, "morph.consensus.p2p.peers")
require.True(t, v.IsSet("morph.consensus.p2p"))
assertDefault(t, v)
})
t.Run("missing config itself", func(t *testing.T) {
v := newValidBlockchainConfig(t, true)
resetConfig(t, v, "morph.consensus.p2p.peers.min")
require.True(t, v.IsSet("morph.consensus.p2p.peers"))
assertDefault(t, v)
})
})
}

0 comments on commit ea78a2d

Please sign in to comment.