[#352] Add explicit max number of alphabet nodes #354

Merged
fyrchik merged 1 commit from ale64bit/frostfs-node:fix/352-clarify-too-many-alpha-nodes into master 2023-05-17 15:01:28 +00:00
4 changed files with 41 additions and 10 deletions
Showing only changes of commit 99b4464674 - Show all commits

View file

@ -18,6 +18,7 @@ To start a network, you need a set of consensus nodes, the same number of
Alphabet nodes and any number of Storage nodes. While the number of Storage
nodes can be scaled almost infinitely, the number of consensus and Alphabet
nodes can't be changed so easily right now. Consider this before going any further.
Note also that there is an upper limit on the number of alphabet nodes (currently 22).
It is easier to use`frostfs-adm` with a predefined configuration. First, create
a network configuration file. In this example, there is going to be only one

View file

@ -39,6 +39,9 @@ func generateAlphabetCreds(cmd *cobra.Command, _ []string) error {
if size == 0 {
return errors.New("size must be > 0")
}
if size > maxAlphabetNodes {
return ErrTooManyAlphabetNodes
}
v := viper.GetViper()
walletDir := config.ResolveHomePath(viper.GetString(alphabetWalletsFlag))

View file

@ -23,6 +23,13 @@ import (
"github.com/spf13/viper"
)
const (
// maxAlphabetNodes is the maximum number of candidates allowed, which is currently limited by the size
// of the invocation script.
// See: https://github.com/nspcc-dev/neo-go/blob/740488f7f35e367eaa99a71c0a609c315fe2b0fc/pkg/core/transaction/witness.go#L10
fyrchik marked this conversation as resolved Outdated

I believe, it is invocation script, not verification.

I believe, it is invocation script, not verification.

done

done
maxAlphabetNodes = 22
)
type cache struct {
nnsCs *state.Contract
groupKey *keys.PublicKey
@ -45,6 +52,8 @@ type initializeContext struct {
ContractPath string
}
var ErrTooManyAlphabetNodes = fmt.Errorf("too many alphabet nodes (maximum allowed is %d)", maxAlphabetNodes)
func initializeSideChainCmd(cmd *cobra.Command, _ []string) error {
initCtx, err := newInitializeContext(cmd, viper.GetViper())
if err != nil {
@ -111,6 +120,10 @@ func newInitializeContext(cmd *cobra.Command, v *viper.Viper) (*initializeContex
return nil, err
}
if len(wallets) > maxAlphabetNodes {
return nil, ErrTooManyAlphabetNodes
}
needContracts := cmd.Name() == "update-contracts" || cmd.Name() == "init"
var w *wallet.Wallet

View file

@ -2,6 +2,7 @@ package morph
import (
"encoding/hex"
"fmt"
"os"
"path/filepath"
"strconv"
@ -40,8 +41,11 @@ func TestInitialize(t *testing.T) {
t.Run("16 nodes", func(t *testing.T) {
testInitialize(t, 16)
})
t.Run("22 nodes", func(t *testing.T) {
testInitialize(t, 22)
t.Run("max nodes", func(t *testing.T) {
testInitialize(t, maxAlphabetNodes)
})
t.Run("too many nodes", func(t *testing.T) {
require.ErrorIs(t, generateTestData(t, t.TempDir(), maxAlphabetNodes+1), ErrTooManyAlphabetNodes)
})
}
@ -49,7 +53,7 @@ func testInitialize(t *testing.T, committeeSize int) {
testdataDir := t.TempDir()
v := viper.GetViper()
generateTestData(t, testdataDir, committeeSize)
require.NoError(t, generateTestData(t, testdataDir, committeeSize))
v.Set(protoConfigPath, filepath.Join(testdataDir, protoFileName))
// Set to the path or remove the next statement to download from the network.
@ -80,25 +84,33 @@ func testInitialize(t *testing.T, committeeSize int) {
})
}
func generateTestData(t *testing.T, dir string, size int) {
func generateTestData(t *testing.T, dir string, size int) error {
v := viper.GetViper()
v.Set(alphabetWalletsFlag, dir)
sizeStr := strconv.FormatUint(uint64(size), 10)
require.NoError(t, generateAlphabetCmd.Flags().Set(alphabetSizeFlag, sizeStr))
if err := generateAlphabetCmd.Flags().Set(alphabetSizeFlag, sizeStr); err != nil {
return err
fyrchik marked this conversation as resolved Outdated

Why do we need this line?

Why do we need this line?

Since this file is initialize_test.go and it tests initialization, I don't think this function should be calling test assertions since it's just a helper. This function marks it as such, so that it's skipped in the log to avoid verbose output.
We already have separate generate_test.go for testing generation specifically.

Since this file is `initialize_test.go` and it tests initialization, I don't think this function should be calling test assertions since it's just a helper. This function marks it as such, so that it's skipped in the log to avoid verbose output. We already have separate `generate_test.go` for testing generation specifically.

Actually, I apologize. I had this call in an earlier version but in current form ofc it's not necessary. I removed it.

Actually, I apologize. I had this call in an earlier version but in current form ofc it's not necessary. I removed it.
}
setTestCredentials(v, size)
require.NoError(t, generateAlphabetCreds(generateAlphabetCmd, nil))
if err := generateAlphabetCreds(generateAlphabetCmd, nil); err != nil {
return err
}
var pubs []string
for i := 0; i < size; i++ {
p := filepath.Join(dir, innerring.GlagoliticLetter(i).String()+".json")
w, err := wallet.NewWalletFromFile(p)
require.NoError(t, err, "wallet doesn't exist")
if err != nil {
return fmt.Errorf("wallet doesn't exist: %w", err)
}
for _, acc := range w.Accounts {
if acc.Label == singleAccountName {
pub, ok := vm.ParseSignatureContract(acc.Contract.Script)
require.True(t, ok)
if !ok {
return fmt.Errorf("could not parse signature script for %s", acc.Address)
}
pubs = append(pubs, hex.EncodeToString(pub))
continue
}
@ -113,10 +125,12 @@ func generateTestData(t *testing.T, dir string, size int) {
cfg.ProtocolConfiguration.P2PSigExtensions = true
cfg.ProtocolConfiguration.VerifyTransactions = true
data, err := yaml.Marshal(cfg)
require.NoError(t, err)
if err != nil {
return err
}
protoPath := filepath.Join(dir, protoFileName)
require.NoError(t, os.WriteFile(protoPath, data, os.ModePerm))
return os.WriteFile(protoPath, data, os.ModePerm)
}
func setTestCredentials(v *viper.Viper, size int) {