diff --git a/cmd/seid/cmd/configmanager/check.go b/cmd/seid/cmd/configmanager/check.go index 3daf62ddf6..3693ac11a9 100644 --- a/cmd/seid/cmd/configmanager/check.go +++ b/cmd/seid/cmd/configmanager/check.go @@ -6,8 +6,6 @@ import ( "io" "io/fs" "os" - "path" - "path/filepath" "reflect" "sort" "strings" @@ -138,19 +136,11 @@ func report(out io.Writer, line string) { _, _ = fmt.Fprintln(out, line) } // there is nothing here that could be wrong. A file that exists and will not read is the opposite, and is // reported as a problem of a file that was found. func checkSeiToml(cmd *cobra.Command) (problems, notes []string, found bool, err error) { - home, err := resolveHomeDir(cmd) + // Refused rather than reported when no home is set, because the exit status is this command's answer + // and there is nothing to have an opinion about. + home, err := theHomeThisCommandRuns(cmd) if err != nil { - return nil, nil, false, fmt.Errorf("resolve the home directory: %w", err) - } - // An empty home leaves every path below relative, so the reads land in ./config under whatever - // directory this command was run from. That answers for some other node's files, and answering for - // the wrong node is worse than not answering: an operator runs this to decide whether to restart. - // - // The boot declines the same case. Refused rather than reported, because the exit status is this - // command's answer and there is nothing here to have an opinion about. - if home == "" { - return nil, nil, false, fmt.Errorf("no home directory is set, so there is no sei.toml to check. "+ - "Pass --home, or set %s", theVariableThatSetsTheHome()) + return nil, nil, false, err } file, err := readSeiTomlAt(home) switch { @@ -213,42 +203,6 @@ func checkSeiToml(cmd *cobra.Command) (problems, notes []string, found bool, err return problems, notes, true, nil } -// theVariableThatSetsTheHome names the environment variable the home resolves from. -// -// Derived from the running binary the same way the resolver derives it, so a message naming it cannot -// drift from the name that actually works. -func theVariableThatSetsTheHome() string { - exe, err := os.Executable() - if err != nil { - return "the home variable for this binary" - } - return strings.ToUpper(path.Base(exe)) + "_HOME" -} - -// theNodesOwnConfiguration reads the node's own configuration file into the struct a boot decodes it into. -// -// Decoded rather than read key by key, so the mode and the rehearsal base below come from one read and both -// answer for the same file. A boot unmarshals this file over the same defaults, so a key stated with nothing -// after it arrives empty and an absent key keeps the default, which is what a boot runs with. -// -// A file that is not there is the only absence. Every other failure is a file somebody wrote that a boot -// does not start on, so answering with defaults would pass a node that cannot boot. -func theNodesOwnConfiguration(home string) (*tmcfg.Config, error) { - cfg := tmcfg.DefaultConfig() - v := viper.New() - v.SetConfigFile(filepath.Join(home, "config", "config.toml")) - switch err := v.ReadInConfig(); { - case errors.Is(err, fs.ErrNotExist): - return cfg, nil - case err != nil: - return nil, err - } - if err := v.Unmarshal(cfg); err != nil { - return nil, err - } - return cfg, nil -} - // whatTheFileLeavesToTheDeclaration says how much of this node's configuration the file does not state. // // Every declared key the file leaves out takes the value this binary declares for the kind of node this is, diff --git a/cmd/seid/cmd/configmanager/generate.go b/cmd/seid/cmd/configmanager/generate.go new file mode 100644 index 0000000000..f634fdaa30 --- /dev/null +++ b/cmd/seid/cmd/configmanager/generate.go @@ -0,0 +1,303 @@ +package configmanager + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/spf13/cobra" + + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/config/seitoml" + tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config" +) + +// flagGenerateMode names the kind of node the written values resolve for. +// +// The same name the provisioning command takes, spelled here rather than shared, because the package +// holding that command imports this one and cannot be imported back. +const flagGenerateMode = "mode" + +// flagGenerateWrite places the file instead of printing it. +const flagGenerateWrite = "write" + +// GenerateCmd writes a sei.toml stating what this node already runs. +// +// A node under this manager answers every declared key from its sei.toml, so a sparse file is a large +// change to a node whose own files were tuned by hand: every declared key the file leaves out moves to the +// value this binary declares, and there are more than two hundred. Writing that file by hand means finding +// each of those by reading two files against a binary's defaults. +// +// This writes it instead, from what the node answers today. A key it states is a key this node answers +// differently from the declaration, so a node started against the written file runs what it ran before. +// +// Two things make the written file run what the node ran, and the divergence filter is neither of them. +// Every value it states is the node's own, read where that setting's reader reads it, so a stated key +// arrives as what the node already held. And a key nothing answers is not stated, because there is no +// value to state: its reader holds a default of its own and stating this binary's declaration instead is +// the one way a writer here changes a setting nobody decided to change. +// +// What the filter does is leave out a key whose answer is already the declared value. Those lines would +// state what the declaration states, so the file is shorter and every line in it marks a decision. +// +// What it does not read is the environment. A variable that answers a declared key goes on answering it +// after this runs, at the same precedence, so baking one into a file would state a value twice and leave +// the copy in the file with no effect. +func GenerateCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "generate", + Short: "Write a sei.toml stating what this node already runs", + Long: "Reads this node's app.toml and config.toml the way a boot reads them, and writes the " + + "sei.toml that makes a node under this manager run what this one runs today.\n\n" + + "Prints the file by default. Pass --write to place it in the node's config directory.", + Args: cobra.NoArgs, + // Nothing here is a usage error once the flags parse, and the production wiring does not silence + // usage, so cobra would print the whole usage block after the error. + SilenceUsage: true, + // A hook of its own, which stops the root one from running. Cobra runs the closest hook it finds. + // The root hook runs the boot's configuration handler, which generates config.toml and app.toml + // when they are absent and copies configuration values into flags. This command reads those files + // to answer what a node runs, so it must not be the thing that creates them, and it must read the + // flags before values are copied into them. + PersistentPreRunE: func(*cobra.Command, []string) error { return nil }, + RunE: func(cmd *cobra.Command, _ []string) error { + home, err := theHomeThisCommandRuns(cmd) + if err != nil { + return err + } + mode, err := theModeTheWrittenValuesResolveFor(cmd) + if err != nil { + return err + } + if err := theHomeHoldsANodeToDescribe(home); err != nil { + return err + } + own, err := theNodesOwnConfiguration(home) + if err != nil { + return fmt.Errorf("read this node's own configuration: %w", err) + } + if err := theKindThisNodeAlreadyRuns(mode, own); err != nil { + return err + } + running, err := whatThisNodeAlreadyRuns(cmd, home, own) + if err != nil { + return err + } + stated, err := whatTheDeclarationDoesNotAlreadySay(mode, running) + if err != nil { + return err + } + file, err := theFileStating(mode, stated) + if err != nil { + return err + } + return thisFilePlacedOrPrinted(cmd, home, file, len(stated)) + }, + } + cmd.Flags().String(flagGenerateMode, "", "the kind of node the written values resolve for: "+ + modesInOrder()) + cmd.Flags().Bool(flagGenerateWrite, false, + "place the file at config/"+seiTomlName+" instead of printing it") + return cmd +} + +// theModeTheWrittenValuesResolveFor reads the kind of node this file is for. +// +// Required rather than guessed. Every value in the file resolves for one kind, and the kinds differ on +// keys that decide whether a node prunes and whether it serves queries. A guess that lands on the wrong +// one writes a file whose values were chosen against defaults the node does not use. +func theModeTheWrittenValuesResolveFor(cmd *cobra.Command) (registry.Mode, error) { + given, err := cmd.Flags().GetString(flagGenerateMode) + if err != nil { + return "", err + } + if given == "" { + return "", fmt.Errorf("--%s is required: every value written here resolves for one kind of node, "+ + "and the kinds differ on whether a node prunes and whether it serves queries. One of %s", + flagGenerateMode, modesInOrder()) + } + for _, known := range registry.Modes() { + if registry.Mode(given) == known { + return known, nil + } + } + return "", fmt.Errorf("%q is not a kind of node this binary declares defaults for. One of %s", + given, modesInOrder()) +} + +// modesInOrder names every kind of node, for a message that has to list them. +func modesInOrder() string { + names := make([]string, 0, len(registry.Modes())) + for _, mode := range registry.Modes() { + names = append(names, string(mode)) + } + return strings.Join(names, ", ") +} + +// whatThisNodeAlreadyRuns reads the value this node answers for every declared key it answers at all. +// +// Both deliveries, read the way each is delivered. The keys a decode delivers are read off the struct +// their file is decoded into, because that is where their reader looks. Every other key is read off the +// source a lookup reads, which is app.toml over the start command's flag defaults. +// +// A key nothing answers is absent from the result rather than present and empty. Its reader holds a +// default of its own, and a caller cannot tell an unanswered key from one answered with a zero. +func whatThisNodeAlreadyRuns(cmd *cobra.Command, home string, own *tmcfg.Config) (map[string]any, error) { + source, err := theSourceThisNodeWouldBuild(cmd, home) + if err != nil { + return nil, err + } + + _, ownedByADecode := registry.ResolvedAndOwnedByDecodedSections(registry.Resolved{}) + decoded, unread, err := whatEachKeyHolds(own, ownedByADecode) + if err != nil { + return nil, err + } + if len(unread) > 0 { + return nil, fmt.Errorf("%d of %d keys a decode delivers are not present in the node's "+ + "configuration, so a file written from it would leave them out and move them to their "+ + "declared value: %v", len(unread), len(ownedByADecode), unread) + } + + running := make(map[string]any, len(registry.Keys())) + for key, value := range decoded { + running[key] = value + } + for _, key := range registry.Keys() { + if _, byADecode := decoded[key]; byADecode { + continue + } + if answer := source.Get(key); answer != nil { + running[key] = answer + } + } + return running, nil +} + +// theHomeHoldsANodeToDescribe refuses a home that no node has been created in. +// +// Both files are absent in that case, so every value would come from a flag's default and the file +// written would describe this binary rather than a node. It would still look like a node's +// configuration, and the kind recorded in it would be whichever kind was asked for. +// +// The node's own configuration file is the one checked, because a boot generates the other one and a +// home can legitimately hold only the first. +func theHomeHoldsANodeToDescribe(home string) error { + path := filepath.Join(home, "config", "config.toml") + switch _, err := os.Stat(path); { + case errors.Is(err, fs.ErrNotExist): + return fmt.Errorf("%s is not there, so this home holds no node to describe and every value "+ + "written would come from this binary rather than from anything a node runs", path) + case err != nil: + return err + } + return nil +} + +// theKindThisNodeAlreadyRuns refuses a kind of node that disagrees with the one this node's own +// configuration file records. +// +// A boot delivers nothing at all from a file whose kind disagrees with the kind the node runs as, so a +// file written under the wrong kind is not a partly-right file. Every declared key goes on reading as it +// did, and an operator holds a file they have every reason to believe is in use. Refused here, where it +// costs a message. +// +// One pairing is not a disagreement, and the shared answer is used rather than a second copy of the rule: +// the kind that keeps every version of history has no name in the node's own file. +func theKindThisNodeAlreadyRuns(mode registry.Mode, own *tmcfg.Config) error { + if own == nil || !modesDisagree(string(mode), own.Mode) { + return nil + } + return fmt.Errorf("this node's own configuration file records it running as %q and --%s says %q. A "+ + "boot delivers nothing from a file that disagrees, so the file written here would leave every "+ + "declared key reading as it does now", own.Mode, flagGenerateMode, mode) +} + +// whatTheDeclarationDoesNotAlreadySay returns the keys this node answers differently from the declaration. +// +// Compared as text, because the two sides carry different Go types for the same key often enough that +// comparing values would be comparing shapes. What is written is the node's own value, with the type it +// holds, so it reaches its setting as what it says. +func whatTheDeclarationDoesNotAlreadySay(mode registry.Mode, running map[string]any) (map[string]any, error) { + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + return nil, fmt.Errorf("resolve this binary's defaults for a %s node: %w", mode, err) + } + stated := map[string]any{} + for key, declared := range resolved.Values { + answer, answers := running[key] + if !answers { + continue + } + if fmt.Sprint(answer) == fmt.Sprint(declared) { + continue + } + stated[key] = answer + } + return stated, nil +} + +// theFileStating returns a document carrying this binary's schema version, the kind of node, and one line +// per key. +// +// Keys written in order, so two runs over one node produce the same bytes and a difference between two +// files is a difference in what they state. +func theFileStating(mode registry.Mode, stated map[string]any) (*seitoml.File, error) { + file, err := seitoml.New(string(mode)) + if err != nil { + return nil, err + } + keys := make([]string, 0, len(stated)) + for key := range stated { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + if err := file.Set(key, stated[key]); err != nil { + return nil, fmt.Errorf("state %s: %w", key, err) + } + } + return file, nil +} + +// thisFilePlacedOrPrinted prints the file, or puts it where a boot reads it. +// +// Printing is the default. This renders a file a boot reads for every setting a node has, so an operator +// reading it before it takes effect is the ordinary case, and `>` puts it where they want it. +// +// An existing file is never replaced. It holds what somebody decided, and the comments beside those +// decisions, and this command cannot tell a file it wrote last week from one edited since. +func thisFilePlacedOrPrinted(cmd *cobra.Command, home string, file *seitoml.File, stated int) error { + write, err := cmd.Flags().GetBool(flagGenerateWrite) + if err != nil { + return err + } + if !write { + body, err := file.Bytes() + if err != nil { + return err + } + _, err = cmd.OutOrStdout().Write(body) + return err + } + + // Asked before the document is rendered, so a home that already holds a file costs nothing and the + // refusal is the same whatever the file would have said. + path := filepath.Join(home, "config", seiTomlName) + if _, err := os.Stat(path); err == nil { + return fmt.Errorf("%s already exists and states what somebody decided, so it is left alone. "+ + "Print this instead and compare them", path) + } else if !os.IsNotExist(err) { + return err + } + if err := file.Save(path); err != nil { + return err + } + report(cmd.OutOrStdout(), fmt.Sprintf("wrote %s, stating %d of this node's %d declared keys", + path, stated, len(registry.Keys()))) + return nil +} diff --git a/cmd/seid/cmd/configmanager/generate_test.go b/cmd/seid/cmd/configmanager/generate_test.go new file mode 100644 index 0000000000..acb25845b1 --- /dev/null +++ b/cmd/seid/cmd/configmanager/generate_test.go @@ -0,0 +1,127 @@ +package configmanager + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/sei-protocol/sei-chain/sei-cosmos/client/flags" +) + +// runGenerate runs the command on its own, which is what makes the two refusals below reachable. +// +// A command executed with no parent has no sibling start command to read flag defaults off, and a home +// only if the caller registers one. Both are states the production wiring never produces, and both are +// states the command has to refuse rather than answer from. +func runGenerate(t *testing.T, home string, args ...string) (string, error) { + t.Helper() + cmd := GenerateCmd() + cmd.Flags().String(flags.FlagHome, home, "") + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SilenceUsage = true + cmd.SilenceErrors = true + cmd.SetArgs(args) + err := cmd.Execute() + return out.String(), err +} + +// aHomeHoldingANode returns a home with the one file this command requires, recording the kind of node +// the file's own default records. +// +// Enough to get past the checks that come before the one under test, and no more. A full node's own file +// is what a bare default carries, so the kind passed alongside is that one. +func aHomeHoldingANode(t *testing.T) string { + t.Helper() + home := t.TempDir() + if err := os.MkdirAll(filepath.Join(home, "config"), 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + path := filepath.Join(home, "config", "config.toml") + if err := os.WriteFile(path, []byte("mode = \"full\"\n"), 0o600); err != nil { + t.Fatalf("write %s: %v", path, err) + } + return home +} + +// TestGenerateRefusesWhatItCannotAnswerFrom covers the states where writing a file would be worse than +// writing none. +// +// Each one produces a plausible file rather than an error. That is what makes them worth a test: an +// operator reads a sei.toml this command printed and has no way to tell it describes their node from it +// describing a directory the command happened to be run in. +func TestGenerateRefusesWhatItCannotAnswerFrom(t *testing.T) { + t.Run("no home is set", func(t *testing.T) { + _, err := runGenerate(t, "", "--mode", "validator") + if err == nil { + t.Fatal("the command answered with no home set, so it read whichever config directory the " + + "working directory holds and described some other node's files") + } + if !strings.Contains(err.Error(), "no home directory is set") { + t.Errorf("the refusal says %q, and it has to name the home", err) + } + }) + + t.Run("this home holds no node", func(t *testing.T) { + _, err := runGenerate(t, t.TempDir(), "--mode", "validator") + if err == nil { + t.Fatal("the command described a home no node was created in, so every value in the file " + + "came from this binary rather than from a node") + } + if !strings.Contains(err.Error(), "config.toml") { + t.Errorf("the refusal says %q, and it has to name the file it looked for", err) + } + }) + + t.Run("no start command to read flag defaults off", func(t *testing.T) { + home := aHomeHoldingANode(t) + _, err := runGenerate(t, home, "--mode", "full") + if err == nil { + t.Fatal("the command answered without the start command's flags, so a key answered only by " + + "a flag's default read as answered by nothing and the file would leave it out") + } + if !strings.Contains(err.Error(), "start") { + t.Errorf("the refusal says %q, and it has to name the command it could not find", err) + } + }) + + t.Run("no kind of node is given", func(t *testing.T) { + _, err := runGenerate(t, t.TempDir()) + if err == nil { + t.Fatal("the command answered with no kind of node given, so its values were chosen against " + + "whichever defaults it picked") + } + if !strings.Contains(err.Error(), "--"+flagGenerateMode) { + t.Errorf("the refusal says %q, and it has to name the flag that fixes it", err) + } + }) + + t.Run("the kind of node is not one this binary declares", func(t *testing.T) { + _, err := runGenerate(t, t.TempDir(), "--mode", "sentry") + if err == nil { + t.Fatal("the command accepted a kind of node it declares no defaults for, so every value in " + + "the file was compared against nothing") + } + if !strings.Contains(err.Error(), "sentry") { + t.Errorf("the refusal says %q, and it has to name what was given", err) + } + }) +} + +// TestGenerateNamesTheHomeVariableThatWorks holds the message to the resolver. +// +// The refusal tells an operator which variable sets the home, and a name that does not match what the +// resolver reads sends them to change something with no effect. +func TestGenerateNamesTheHomeVariableThatWorks(t *testing.T) { + _, err := runGenerate(t, "", "--mode", "validator") + if err == nil { + t.Fatal("no refusal to read") + } + if !strings.Contains(err.Error(), theVariableThatSetsTheHome()) { + t.Errorf("the refusal says %q and the resolver reads %s, so an operator following it sets a "+ + "variable nothing looks at", err, theVariableThatSetsTheHome()) + } +} diff --git a/cmd/seid/cmd/configmanager/nodefiles.go b/cmd/seid/cmd/configmanager/nodefiles.go new file mode 100644 index 0000000000..2d786963b1 --- /dev/null +++ b/cmd/seid/cmd/configmanager/nodefiles.go @@ -0,0 +1,129 @@ +// Reading the files a node already has, without the boot's handler. +// +// The handler that builds the boot's source generates config.toml and app.toml when they are absent, and +// copies configuration values into flags. A command that answers a question about those files must do +// neither, so it reads them here instead. + +package configmanager + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path" + "path/filepath" + "strings" + + tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config" + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "github.com/spf13/viper" +) + +// theHomeThisCommandRuns resolves the home directory this command was given, and refuses an empty one. +// +// Every path a command reads joins the home. An empty one leaves those paths relative, so a read lands in +// ./config under whatever directory the command was run from, which is some other node's files. Answering +// for the wrong node is worse than not answering, so the two are resolved together and no caller can hold +// a home without having checked it. +func theHomeThisCommandRuns(cmd *cobra.Command) (string, error) { + home, err := resolveHomeDir(cmd) + if err != nil { + return "", fmt.Errorf("resolve the home directory: %w", err) + } + if home == "" { + return "", fmt.Errorf("no home directory is set, so the files read here would be whichever ones "+ + "the working directory holds. Pass --home, or set %s", theVariableThatSetsTheHome()) + } + return home, nil +} + +// theVariableThatSetsTheHome names the environment variable the home resolves from. +// +// Derived from the running binary the same way the resolver derives it, so a message naming it cannot +// drift from the name that actually works. +func theVariableThatSetsTheHome() string { + exe, err := os.Executable() + if err != nil { + return "the home variable for this binary" + } + return strings.ToUpper(path.Base(exe)) + "_HOME" +} + +// theNodesOwnConfiguration reads the node's own configuration file into the struct a boot decodes it into. +// +// Decoded rather than read key by key, so every caller sees the same shape a boot sees. A boot unmarshals +// this file over the same defaults, so a key stated with nothing after it arrives empty and an absent key +// keeps the default, which is what a boot runs with. +// +// A file that is not there is the only absence. Every other failure is a file somebody wrote that a boot +// does not start on, so answering with defaults would describe a node that cannot boot. +func theNodesOwnConfiguration(home string) (*tmcfg.Config, error) { + cfg := tmcfg.DefaultConfig() + v := viper.New() + v.SetConfigFile(filepath.Join(home, "config", "config.toml")) + switch err := v.ReadInConfig(); { + case errors.Is(err, fs.ErrNotExist): + return cfg, nil + case err != nil: + return nil, err + } + if err := v.Unmarshal(cfg); err != nil { + return nil, err + } + return cfg, nil +} + +// startCommandName is the subcommand whose flags answer a key a file leaves out. +const startCommandName = "start" + +// theSourceThisNodeWouldBuild returns what this node answers for a key looked up by name, without booting. +// +// A boot binds its start command's flags into the source it builds and then reads app.toml over them, so a +// key with a flag of its own is answered by that flag's default whether the file mentions it or not. The +// same two, in the same order, so a written value outranks a flag's default here as it does there. +// +// A file that is not there leaves the flag defaults answering, which is what a node in that state runs +// until a boot generates one. Every other failure is a file a boot does not start on. +func theSourceThisNodeWouldBuild(cmd *cobra.Command, home string) (*viper.Viper, error) { + set, err := theStartCommandsFlags(cmd) + if err != nil { + return nil, err + } + v := viper.New() + if err := v.BindPFlags(set); err != nil { + return nil, err + } + v.SetConfigFile(filepath.Join(home, "config", "app.toml")) + switch err := v.ReadInConfig(); { + case errors.Is(err, fs.ErrNotExist): + return v, nil + case err != nil: + return nil, err + } + return v, nil +} + +// theStartCommandsFlags returns the flags this binary's start command carries. +// +// Found on the root by name rather than built here, so they are the flags this binary ships and a flag +// added to the start command is carried without anything else changing. +// +// Not found is refused. Without these a key whose only answer is a flag's default reads as unanswered, and +// a caller writing a file from that would leave the key out and move it to its declared value. Pruning is +// the one to picture: the flag prunes and the declaration keeps everything, so the file would silently +// stop a node pruning. +func theStartCommandsFlags(cmd *cobra.Command) (*pflag.FlagSet, error) { + for _, sub := range cmd.Root().Commands() { + if sub.Name() != startCommandName { + continue + } + set := pflag.NewFlagSet(startCommandName, pflag.ContinueOnError) + set.AddFlagSet(sub.Flags()) + set.AddFlagSet(sub.PersistentFlags()) + return set, nil + } + return nil, fmt.Errorf("this binary has no %q command, so the defaults its flags carry cannot be "+ + "read and a key answered only by one would read as answered by nothing", startCommandName) +} diff --git a/cmd/seid/cmd/configmanager/tendermint_copy.go b/cmd/seid/cmd/configmanager/tendermint_copy.go index 32bc3200bd..b9baf02bc5 100644 --- a/cmd/seid/cmd/configmanager/tendermint_copy.go +++ b/cmd/seid/cmd/configmanager/tendermint_copy.go @@ -135,7 +135,21 @@ func join(path, field string) string { // it did not move. That is the same statement as a key an operator wrote and got, produced by having read // nothing. func describe(cfg *tmcfg.Config, keys []string) (values map[string]string, unread []string, err error) { - values = map[string]string{} + held, unread, err := whatEachKeyHolds(cfg, keys) + values = make(map[string]string, len(held)) + for key, v := range held { + values[key] = fmt.Sprint(v) + } + return values, unread, err +} + +// whatEachKeyHolds reads the value a node's configuration holds for each key, and names the keys that are +// not in it. +// +// The value as the struct holds it. A caller writing one into a file needs the type the key carries, and a +// number rendered as text reaches its setting as a zero. +func whatEachKeyHolds(cfg *tmcfg.Config, keys []string) (values map[string]any, unread []string, err error) { + values = map[string]any{} if cfg == nil { return values, keys, fmt.Errorf("no configuration to read") } @@ -151,7 +165,7 @@ func describe(cfg *tmcfg.Config, keys []string) (values map[string]string, unrea unread = append(unread, key) continue } - values[key] = fmt.Sprint(v) + values[key] = v } sort.Strings(unread) return values, unread, nil diff --git a/cmd/seid/cmd/generate_through_root_test.go b/cmd/seid/cmd/generate_through_root_test.go new file mode 100644 index 0000000000..ee3abeac72 --- /dev/null +++ b/cmd/seid/cmd/generate_through_root_test.go @@ -0,0 +1,487 @@ +package cmd + +import ( + "bytes" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "testing" + + "go.opentelemetry.io/otel/sdk/trace" + + "github.com/sei-protocol/sei-chain/cmd/seid/cmd/configmanager" + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/config/seitoml" + "github.com/sei-protocol/sei-chain/sei-cosmos/server" + svrcmd "github.com/sei-protocol/sei-chain/sei-cosmos/server/cmd" + "github.com/sei-protocol/sei-chain/testutil/configtest" +) + +// TestANodeStartedFromTheGeneratedFileRunsWhatItRanBefore is what the command is for. +// +// A node under this manager answers every declared key from its sei.toml, so a file that states too +// little moves settings to their declared value. That is the whole risk of handing an operator a +// generated file, and it is one measurement: read what the node runs, write the file, start the node +// against it, and read again. +// +// Both deliveries, because they are read in different places and a file can be right about one of them. +func TestANodeStartedFromTheGeneratedFileRunsWhatItRanBefore(t *testing.T) { + configtest.Isolate(t) + home := aNodeRunningAs(t, registry.ModeValidator) + + before := whatTheNodeAnswers(t, home) + + out, err := runGenerateThroughRoot(t, home, "--mode", "validator") + if err != nil { + t.Fatalf("generate was refused: %v\n%s", err, out) + } + seiToml := filepath.Join(home, "config", "sei.toml") + if err := os.WriteFile(seiToml, []byte(out), 0o600); err != nil { + t.Fatalf("write %s: %v", seiToml, err) + } + + after := whatTheNodeAnswers(t, home) + + var moved, lost []string + for key, was := range before.lookup { + now, answers := after.lookup[key] + switch { + case !answers: + lost = append(lost, key) + case fmt.Sprint(was) != fmt.Sprint(now): + moved = append(moved, fmt.Sprintf("%s was %v and is %v", key, was, now)) + } + } + sort.Strings(moved) + sort.Strings(lost) + for _, line := range moved { + t.Errorf("a key a lookup delivers moved: %s. The file this command wrote does not state what the "+ + "node ran, so an operator adopting it changes a setting they did not decide to change", line) + } + for _, key := range lost { + t.Errorf("%s was answered before the file and is answered by nothing after it, so its reader now "+ + "holds a default of its own where the node had a value", key) + } + + for key, was := range before.decoded { + if now := after.decoded[key]; was != now { + t.Errorf("a key a decode delivers moved: %s was %q and is %q", key, was, now) + } + } + + if !t.Failed() { + t.Logf("%d keys a lookup answers and %d a decode delivers, all unchanged across the file", + len(before.lookup), len(before.decoded)) + } +} + +// TestTheGeneratedFileStatesTheKeysThatDivergeAndNoOthers holds which keys the command writes. +// +// A file stating fewer keys than these moves a setting, and the test above catches that by starting a +// node. This catches the other direction, which that one cannot: a line stating what the declaration +// already states changes nothing, so a file full of them passes a round trip while telling an operator +// that two hundred settings were decisions. +// +// Measured against the record for every kind of node, because the command derives the set from the files +// and the record derives it from a boot. +func TestTheGeneratedFileStatesTheKeysThatDivergeAndNoOthers(t *testing.T) { + configtest.Isolate(t) + + for _, mode := range registry.Modes() { + t.Run(string(mode), func(t *testing.T) { + // A node of this kind, because the command refuses a kind the node's own file contradicts. + home := aNodeRunningAs(t, mode) + whatTheNodeAnswers(t, home) + + out, err := runGenerateThroughRoot(t, home, "--mode", string(mode)) + if err != nil { + t.Fatalf("generate was refused: %v\n%s", err, out) + } + stated := whatTheFileStates(t, out) + + want := divergences[mode] + + for key := range stated { + if _, records := want[key]; !records { + t.Errorf("the file states %s and nothing records it as diverging, so either it "+ + "states a key that changes nothing or a divergence is unrecorded", key) + } + } + for key := range want { + if _, states := stated[key]; !states { + t.Errorf("%s is recorded as diverging and the file does not state it, so a node "+ + "adopting the file moves that setting to its declared value", key) + } + } + }) + } +} + +// theTendermintKind is the kind of node the node's own configuration file records for a declared kind. +// +// The kind that keeps every version of history has no name in that file. The command that writes it +// writes the query-serving name instead, and the delivery accepts that one pairing. +func theTendermintKind(mode registry.Mode) string { + if mode == registry.ModeArchive { + return string(registry.ModeFull) + } + return string(mode) +} + +// aNodeRunningAs prepares a home whose own configuration file records this kind of node. +// +// The boot writes both files and then one line is replaced, rather than a file being rendered here. The +// two writers in this binary disagree on several keys, and rendering a third file would produce a node +// matching neither, so a measurement against what either writer produces would be measuring this helper. +// +// The kind matters because the delivery refuses a sei.toml whose kind disagrees with the kind the node +// runs as, and it refuses the whole file. A measurement over a node whose file says something else reads +// the same before and after, and holds for any file at all. +func aNodeRunningAs(t *testing.T, mode registry.Mode) string { + t.Helper() + home := configtest.NewHome(t) + if err := os.MkdirAll(filepath.Join(home.Root, "config"), 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + whatTheNodeAnswers(t, home.Root) + + path := filepath.Join(home.Root, "config", "config.toml") + body, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read the node's configuration file: %v", err) + } + lines := strings.Split(string(body), "\n") + replaced := false + for i, line := range lines { + // At the file's root, so the search stops at the first table header. A key of this name inside a + // section is a different setting. + if strings.HasPrefix(strings.TrimSpace(line), "[") { + break + } + if !strings.HasPrefix(strings.TrimSpace(line), "mode ") { + continue + } + lines[i] = "mode = \"" + theTendermintKind(mode) + "\"" + replaced = true + break + } + if !replaced { + t.Fatal("the node's configuration file records no kind of node at its root, so this helper " + + "cannot set one and every test using it would run against whatever kind the writer chose") + } + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o600); err != nil { + t.Fatalf("write the node's configuration file: %v", err) + } + return home.Root +} + +// nodeAnswers is what a node holds, read the way each delivery is read. +type nodeAnswers struct { + lookup map[string]any + decoded map[string]string +} + +// whatTheNodeAnswers starts a node in this home and reads every declared key off it. +// +// Booted twice for the reason the agreement measurement is: the first writes the files, and a flag bound +// to a key the writer sets overwrites that value before anything reads it back. +func whatTheNodeAnswers(t *testing.T, home string) nodeAnswers { + t.Helper() + var ctx *server.Context + for boot := 0; boot < 2; boot++ { + cmd := server.StartCmd(nil, home, []trace.TracerProviderOption{}) + if err := cmd.Flags().Set("home", home); err != nil { + t.Fatalf("set --home: %v", err) + } + got, err := runManager(t, configmanager.SeiConfigManager{}, cmd) + if err != nil { + t.Fatalf("boot %d was refused: %v", boot+1, err) + } + ctx = got + } + + decoded := map[string]bool{} + for _, key := range keysADecodeDelivers() { + decoded[key] = true + } + lookup := map[string]any{} + for _, key := range registry.Keys() { + if decoded[key] { + continue + } + if answer := ctx.Viper.Get(key); answer != nil { + lookup[key] = answer + } + } + return nodeAnswers{ + lookup: lookup, + decoded: configmanager.DescribeForTest(t, ctx.Config, keysADecodeDelivers()), + } +} + +// whatTheFileStates returns the keys a rendered sei.toml states, leaving out the two the file itself +// carries about its own schema and the kind of node. +func whatTheFileStates(t *testing.T, body string) map[string]any { + t.Helper() + file, err := seitoml.Parse(strings.NewReader(body)) + if err != nil { + t.Fatalf("the command rendered a file this binary cannot read: %v\n%s", err, body) + } + values, err := file.Values() + if err != nil { + t.Fatalf("read the rendered file's values: %v", err) + } + return values +} + +// runGenerateThroughRoot runs `config generate` the way an operator runs it. +// +// Through the real root command, because the command reads the start command's flags off the root to +// answer what a node runs. Built on its own it has no siblings and would refuse. +func runGenerateThroughRoot(t *testing.T, home string, extraArgs ...string) (string, error) { + t.Helper() + root, _ := NewRootCmd() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SilenceUsage = true + root.SilenceErrors = true + root.SetArgs(append([]string{"config", "generate", "--home", home}, extraArgs...)) + err := svrcmd.Execute(root, home) + return out.String(), err +} + +// TestGenerateWritesWhereABootReadsIt covers the flag that places the file. +// +// Printing is the default and a printed file is not in use, so the one thing that makes the command +// finish an operator's task is the file landing where the boot looks for it. +func TestGenerateWritesWhereABootReadsIt(t *testing.T) { + configtest.Isolate(t) + home := aNodeRunningAs(t, registry.ModeValidator) + before := whatTheNodeAnswers(t, home) + + out, err := runGenerateThroughRoot(t, home, "--mode", "validator", "--write") + if err != nil { + t.Fatalf("generate was refused: %v\n%s", err, out) + } + + path := filepath.Join(home, "config", "sei.toml") + if _, err := os.Stat(path); err != nil { + t.Fatalf("the command reported writing a file and %s is not there: %v\n%s", path, err, out) + } + after := whatTheNodeAnswers(t, home) + for key, was := range before.lookup { + if now, answers := after.lookup[key]; !answers || fmt.Sprint(was) != fmt.Sprint(now) { + t.Errorf("%s was %v and is %v after the file the command placed, so the file it writes is "+ + "not the file it prints", key, was, now) + } + } + + // Run again over the file it just wrote. An operator who repeats the command has a file they did not + // decide to replace. + second, err := runGenerateThroughRoot(t, home, "--mode", "validator", "--write") + if err == nil { + t.Errorf("the second run replaced the file the first one wrote:\n%s", second) + } +} + +// theTunedSettings are values written into a node's own files by hand, one per shape a writer has to +// render and per delivery it has to read. +// +// Chosen to differ from both writers. A value matching either one would be stated or left out for a +// reason that has nothing to do with an operator having set it, so the test would pass without covering +// what it claims. +// +// The retention and connection rows also agree with neither: the declaration and the generated file both +// state something else again, so each of these is a third value. +var theTunedSettings = []struct { + file string + section string + key string + written string +}{ + // Answered by a flag's default when no file states it, so this covers a written value outranking one. + {"app.toml", "", "pruning", `"everything"`}, + {"app.toml", "api", "max-open-connections", "4321"}, + {"app.toml", "state-store", "ss-keep-recent", "777"}, + // The keys a decode delivers, read off a struct rather than by name. + {"config.toml", "mempool", "size", "4321"}, + {"config.toml", "p2p", "max-connections", "55"}, + {"config.toml", "rpc", "max-subscription-clients", "33"}, + {"config.toml", "consensus", "create-empty-blocks-interval", `"7s"`}, +} + +// TestANodeWhoseFilesWereTunedByHandRunsWhatItRanBefore is the case the command exists for. +// +// The measurement above starts from files a boot generated, where every value is one writer or the other. +// A node worth running this on is not that node: somebody edited its files, and the values they chose +// agree with neither writer. +// +// Those values are also the ones a divergence record cannot cover, because they are not a property of +// this binary. A key an operator set to the declared value is left out and a key they set to anything +// else is stated, and which keys those are is known only by reading their files. +func TestANodeWhoseFilesWereTunedByHandRunsWhatItRanBefore(t *testing.T) { + configtest.Isolate(t) + home := aNodeRunningAs(t, registry.ModeValidator) + + // Started once so the files exist to edit. What it answers is not measured: the edits below are what + // this node runs, and they are applied before anything reads it. + whatTheNodeAnswers(t, home) + tuneByHand(t, home) + + before := whatTheNodeAnswers(t, home) + for _, setting := range theTunedSettings { + key := setting.key + if setting.section != "" { + key = setting.section + "." + setting.key + } + if _, reads := before.lookup[key]; !reads { + if _, decoded := before.decoded[key]; !decoded { + t.Fatalf("%s reads as answered by nothing after being written into %s, so the edit did "+ + "not take and this test measures a node nobody tuned", key, setting.file) + } + } + } + + out, err := runGenerateThroughRoot(t, home, "--mode", "validator") + if err != nil { + t.Fatalf("generate was refused: %v\n%s", err, out) + } + if err := os.WriteFile(filepath.Join(home, "config", "sei.toml"), []byte(out), 0o600); err != nil { + t.Fatalf("write sei.toml: %v", err) + } + + // Every edited key has to be stated. Each of them differs from the declaration, so one the file leaves + // out is a setting an operator chose and the node stops running. + stated := whatTheFileStates(t, out) + for _, setting := range theTunedSettings { + key := setting.key + if setting.section != "" { + key = setting.section + "." + setting.key + } + if _, states := stated[key]; !states { + t.Errorf("%s was written into %s by hand and the generated file does not state it, so the "+ + "node moves to the declared value and loses what an operator chose", key, setting.file) + } + } + + after := whatTheNodeAnswers(t, home) + var moved, lost []string + for key, was := range before.lookup { + now, answers := after.lookup[key] + switch { + case !answers: + lost = append(lost, key) + case fmt.Sprint(was) != fmt.Sprint(now): + moved = append(moved, fmt.Sprintf("%s was %v and is %v", key, was, now)) + } + } + for key, was := range before.decoded { + if now := after.decoded[key]; was != now { + moved = append(moved, fmt.Sprintf("%s was %q and is %q", key, was, now)) + } + } + sort.Strings(moved) + sort.Strings(lost) + for _, line := range moved { + t.Errorf("a tuned node changed across the generated file: %s", line) + } + for _, key := range lost { + t.Errorf("%s was answered before the file and is answered by nothing after it", key) + } + + if !t.Failed() { + t.Logf("%d hand-written values, %d keys a lookup answers and %d a decode delivers, all unchanged", + len(theTunedSettings), len(before.lookup), len(before.decoded)) + } +} + +// tuneByHand writes theTunedSettings into the node's own files, replacing each key's line where it sits. +// +// The line is replaced rather than the file rewritten, because a rewrite from a decoded map is not what an +// operator does and would drop every comment the file ships with, including the ones naming the keys this +// test does not touch. +func tuneByHand(t *testing.T, home string) { + t.Helper() + for _, name := range []string{"app.toml", "config.toml"} { + path := filepath.Join(home, "config", name) + body, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", name, err) + } + lines := strings.Split(string(body), "\n") + + for _, setting := range theTunedSettings { + if setting.file != name { + continue + } + replaced := false + section := "" + for i, line := range lines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "[") && strings.HasSuffix(trimmed, "]") { + section = strings.Trim(trimmed, "[]") + continue + } + if section != setting.section || !strings.HasPrefix(trimmed, setting.key+" ") { + continue + } + lines[i] = setting.key + " = " + setting.written + replaced = true + break + } + switch { + case replaced: + // A key at the file's root that the generator does not write is the flag-answered case, and + // writing it is what an operator does. Inserted at the top, because a bare key after a table + // header belongs to that table rather than to the root. + case setting.section == "": + lines = append([]string{setting.key + " = " + setting.written}, lines...) + default: + t.Fatalf("%s states no %q under %q, so this test would measure a value nobody wrote. The "+ + "key was renamed, moved section, or is no longer written by the generator", + name, setting.key, setting.section) + } + } + + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o600); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } +} + +// TestGenerateRefusesAKindTheNodeContradicts covers the file that is written, adopted, and then ignored. +// +// The delivery refuses a sei.toml whose kind disagrees with the kind the node runs as, and it refuses the +// whole file rather than the keys that differ. So a file written under the wrong kind leaves every +// declared key reading as it did, on a node whose operator has every reason to believe otherwise. The +// refusal costs a message instead. +func TestGenerateRefusesAKindTheNodeContradicts(t *testing.T) { + configtest.Isolate(t) + + t.Run("a kind the node's own file contradicts", func(t *testing.T) { + home := aNodeRunningAs(t, registry.ModeValidator) + out, err := runGenerateThroughRoot(t, home, "--mode", "full", "--write") + if err == nil { + t.Fatalf("the command wrote a file for a full node against one running as a validator, and a "+ + "boot delivers nothing from it:\n%s", out) + } + if _, err := os.Stat(filepath.Join(home, "config", "sei.toml")); !os.IsNotExist(err) { + t.Error("the refused run left a sei.toml behind, so the next boot reads a file this command " + + "declined to stand behind") + } + }) + + // The one pairing that is not a disagreement. Refusing it would leave the kind that keeps every + // version of history with no way to run this command at all, because that kind has no name in the + // node's own configuration file. + t.Run("the kind that keeps all history, whose file says it serves queries", func(t *testing.T) { + home := aNodeRunningAs(t, registry.ModeArchive) + if _, err := runGenerateThroughRoot(t, home, "--mode", "archive"); err != nil { + t.Errorf("the command refused an archive node: %v. Its own file records the query-serving "+ + "kind because that is the only name it has, and the delivery accepts the pair", err) + } + }) +} diff --git a/cmd/seid/cmd/node_agreement_test.go b/cmd/seid/cmd/node_agreement_test.go index a65ffba43d..0da3f12eef 100644 --- a/cmd/seid/cmd/node_agreement_test.go +++ b/cmd/seid/cmd/node_agreement_test.go @@ -15,47 +15,197 @@ import ( "github.com/sei-protocol/sei-chain/testutil/configtest" ) -// bootGeneratedDefaults is what each diverging key resolves to for a node that has no configuration file of -// its own and lets the boot generate one. +// divergences is what a node runs for each key where its own generated files and this binary's +// declaration disagree, per kind of node. // // A declared value is what the init command writes for a kind of node. That command is not the only thing -// in this binary that writes this file: a node started without one gets it generated by the boot instead, -// and the two do not agree. These are the keys where they differ, with what the second one produces. +// in this binary that writes these files: a node started without them gets them generated by the boot +// instead, and the two do not agree. A third value comes from somewhere else again, because a key with a +// flag of its own on the start command is answered by that flag's default whether a file mentions it or +// not, and that default is stated where the flag is declared. // -// Held as text because the two sides carry different Go types for the same key often enough that comparing -// values would be comparing shapes. What matters is which keys disagree and what a node gets instead. -var bootGeneratedDefaults = map[string]string{ - "p2p.recv-rate": "5120000", - "p2p.send-rate": "5120000", - "rpc.pprof-laddr": "localhost:6060", - "tx-index.indexer": "[kv]", +// Per kind of node, because the declaration varies on the kind and the writer does not. The boot's writer +// produces one shape for every kind, so a declaration for a kind that shape does not describe disagrees +// with it. Measuring one kind finds a quarter of the rows. +// +// Each row is a setting a sparse sei.toml moves, because a key that file leaves out takes the declared +// value. What the move does, by group: +// +// listener addresses p2p.laddr and rpc.laddr bind loopback and are declared on every interface, so a +// full or archive node adopting a sparse file reaches the network where it did +// not. rpc.pprof-laddr is the other direction: a debug listener the node opens on +// a fixed port, declared closed. +// rate ceilings p2p.recv-rate and p2p.send-rate cap what one connection may move. The declared +// value is four times what a node runs, so a sparse file raises both. +// connection policy p2p.max-connections is ten times higher declared than a seed runs, and +// p2p.allow-duplicate-ip is declared on where a seed has it off. +// service switches api.enable, grpc.enable, evm.http_enabled, evm.ws_enabled and +// state-store.ss-enable are open on the node and declared closed for a validator +// and a seed, which serve no queries. A sparse file closes them. +// retention pruning is the flag-default row, in every kind: the flag prunes and the +// declaration keeps all state history. min-retain-blocks and +// state-store.ss-keep-recent are the block and state halves of the same choice, +// and each diverges for the one kind whose rule moves it. +// indexing tx-index.indexer decides whether the node indexes transactions. The boot's +// writer produces a file for a node that serves queries, so this row is what a +// resolution for a validator states against a file written for something else. +// +// Held as text, because the two sides carry different Go types for the same key often enough that +// comparing values would be comparing shapes. What matters is which keys disagree and what a node runs +// instead. +var divergences = map[registry.Mode]map[string]string{ + registry.ModeValidator: { + "api.enable": "true", + "evm.http_enabled": "true", + "evm.ws_enabled": "true", + "grpc.enable": "true", + "p2p.recv-rate": "5120000", + "p2p.send-rate": "5120000", + "pruning": "default", + "rpc.pprof-laddr": "localhost:6060", + "state-store.ss-enable": "true", + "tx-index.indexer": "[kv]", + }, + registry.ModeFull: { + "min-retain-blocks": "0", + "p2p.laddr": "tcp://127.0.0.1:26656", + "p2p.recv-rate": "5120000", + "p2p.send-rate": "5120000", + "pruning": "default", + "rpc.laddr": "tcp://127.0.0.1:26657", + "rpc.pprof-laddr": "localhost:6060", + }, + registry.ModeSeed: { + "api.enable": "true", + "evm.http_enabled": "true", + "evm.ws_enabled": "true", + "grpc.enable": "true", + "p2p.allow-duplicate-ip": "false", + "p2p.max-connections": "100", + "p2p.recv-rate": "5120000", + "p2p.send-rate": "5120000", + "pruning": "default", + "rpc.pprof-laddr": "localhost:6060", + "state-store.ss-enable": "true", + "tx-index.indexer": "[kv]", + }, + registry.ModeArchive: { + "p2p.laddr": "tcp://127.0.0.1:26656", + "p2p.recv-rate": "5120000", + "p2p.send-rate": "5120000", + "pruning": "default", + "rpc.laddr": "tcp://127.0.0.1:26657", + "rpc.pprof-laddr": "localhost:6060", + "state-store.ss-keep-recent": "100000", + }, } -// reasoning says what a node gets, and it is why each row is measured rather than described. -var reasoning = map[string]string{ - "p2p.recv-rate": "the ceiling on what one connection may pull, four times lower than the declared " + - "value, so a node adopting a file generated by the other writer would have it raised", - "p2p.send-rate": "the same ceiling in the other direction", - "rpc.pprof-laddr": "a debug listener the declaration states is closed. The other writer opens it on a " + - "fixed port, and a flag bound to this key hides that on a node's first boot only, because the " + - "file has not been read yet for the flag's empty default to lose to", - "tx-index.indexer": "whether the node indexes transactions. The boot's writer produces a file for a " + - "node that serves queries, because the kind it defaults to is that one, so this row is what a " + - "resolution for a validator states against a file generated for something else. The pair in that " + - "file agrees with itself; what disagrees is the kind of node each side is describing", +// keysAGeneratedFileLeavesToTheirReader is every declared key a lookup delivers that a boot-generated file +// does not answer. +// +// Each of their readers starts from a default of its own and takes the source's answer only when there is +// one, so what a node runs for one of these keys is that default and the source states nothing. They are +// named rather than compared because the comparison above has nothing to read. +// +// Every one of these declarations is taken from the same default the reader falls back to, so the value a +// node runs is the declared one. That is not measured here. What reaches it is a file stating every +// declared key and a node started from it. +// +// The same in every kind of node. The writer does not vary on the kind, and which keys are declared does +// not either. +var keysAGeneratedFileLeavesToTheirReader = []string{ + "eth_replay.contract_state_checks", + "evm.enable_test_api", + "evm.max_concurrent_simulation_calls", + "evm.max_tx_pool_txs", + "evm.rpc_stats_interval", + "genesis.import-file", + "state-commit.flatkv.enable-read-write-metrics", + "state-commit.sc-snapshot-writer-limit", + "state-commit.sc-write-mode-enable-auto", + "wasm.memory_cache_size", + "wasm.simulation_gas_limit", } -// TestTheDivergencesFromAGeneratedFileAreTheRecordedOnes measures what a comment would only claim. +// TestTheDivergencesFromGeneratedFilesAreTheRecordedOnes measures what a comment would only claim. // -// The init command and the boot both generate this file and they disagree, so a declared value is what one -// of them writes and not simply what a generated file carries. Which keys those are is measured here rather -// than described, because a key that starts diverging fails and so does one that stops. +// Which keys disagree is measured rather than described, because a key that starts diverging fails and so +// does one that stops. Both deliveries, read where each is read: the keys a decode delivers off the struct +// their file is decoded into, and every other key off the source a lookup reads. // -// Driven through a real boot with no configuration file of its own and no sei.toml, so nothing is delivered -// and what the node holds is purely what the boot generated. -func TestTheDivergencesFromAGeneratedFileAreTheRecordedOnes(t *testing.T) { +// Driven through a real boot with no files of its own and no sei.toml, so nothing is delivered and what +// the node holds is purely what the boot generated. +func TestTheDivergencesFromGeneratedFilesAreTheRecordedOnes(t *testing.T) { configtest.Isolate(t) - generated := whatTheBootGenerates(t) + ctx := theBootWithNoFileOfItsOwn(t) + decoded := configmanager.DescribeForTest(t, ctx.Config, keysADecodeDelivers()) + + for _, mode := range registry.Modes() { + t.Run(string(mode), func(t *testing.T) { + recorded, records := divergences[mode] + if !records { + t.Fatalf("nothing records what a %s node diverges on, so this kind is declared and no "+ + "measurement covers it", mode) + } + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + var measured []string + for key, declared := range resolved.Values { + // A key the source does not answer reads as its reader's own default, which the source + // says nothing about. keysAGeneratedFileLeavesToTheirReader holds that set. + got, reads := whatTheNodeHolds(key, decoded, ctx) + if !reads { + continue + } + if fmt.Sprint(declared) == got { + if _, listed := recorded[key]; listed { + t.Errorf("%s no longer diverges on a %s node, both sides being %v. Take it off "+ + "the record, so the record stays the set of keys the writers state "+ + "differently", key, mode, declared) + } + continue + } + measured = append(measured, key) + want, listed := recorded[key] + switch { + case !listed: + t.Errorf("a %s node is declared to run %s as %v and one that let the boot generate "+ + "its files runs %q, and nothing records that. A sei.toml leaving this key out "+ + "moves the setting", mode, key, declared, got) + case want != got: + t.Errorf("a %s node is recorded as running %s as %q and runs %q", mode, key, want, got) + } + } + + sort.Strings(measured) + if len(measured) != len(recorded) { + t.Errorf("measured %d divergences for a %s node and %d are recorded: %v", + len(measured), mode, len(recorded), measured) + } + }) + } +} + +// TestTheKeysAGeneratedFileLeavesToTheirReaderAreTheRecordedOnes holds the set the measurement above skips. +// +// Skipping them is sound only while their reader's default is the declared value, and that holds because +// the declaration is taken from the same default. A key that stops being answered moves into this set and +// nothing else says so, and one that starts being answered leaves it and belongs in the measurement. +func TestTheKeysAGeneratedFileLeavesToTheirReaderAreTheRecordedOnes(t *testing.T) { + configtest.Isolate(t) + ctx := theBootWithNoFileOfItsOwn(t) + + decoded := make(map[string]bool) + for _, key := range keysADecodeDelivers() { + decoded[key] = true + } + recorded := make(map[string]bool, len(keysAGeneratedFileLeavesToTheirReader)) + for _, key := range keysAGeneratedFileLeavesToTheirReader { + recorded[key] = true + } resolved, err := registry.Resolve(registry.ModeValidator, registry.Sources{}) if err != nil { @@ -63,50 +213,57 @@ func TestTheDivergencesFromAGeneratedFileAreTheRecordedOnes(t *testing.T) { } var measured []string - for key, got := range generated { - declared, declares := resolved.Values[key] - if !declares { - continue - } - if fmt.Sprint(declared) == got { - if _, listed := bootGeneratedDefaults[key]; listed { - t.Errorf("%s no longer diverges, both sides being %v. Take it off the record, so the "+ - "record stays the set of keys the two generators state differently", key, declared) - } + for key := range resolved.Values { + if decoded[key] || ctx.Viper.Get(key) != nil { continue } measured = append(measured, key) - want, listed := bootGeneratedDefaults[key] - switch { - case !listed: - t.Errorf("%s is declared as %v and a node that let the boot generate its file runs %q, and "+ - "nothing records that. %s", key, declared, got, reasoning[key]) - case want != got: - t.Errorf("%s is recorded as running %q and runs %q", key, want, got) + if !recorded[key] { + t.Errorf("%s is declared and a boot-generated file does not answer it, and nothing records "+ + "that. Its reader's own default is what a node runs for it", key) + } + } + for _, key := range keysAGeneratedFileLeavesToTheirReader { + if answer := ctx.Viper.Get(key); answer != nil { + t.Errorf("%s is recorded as unanswered and a boot-generated file answers it %v, so what a "+ + "node runs for it is measurable and no measurement covers it", key, answer) } } sort.Strings(measured) - if len(measured) != len(bootGeneratedDefaults) { - t.Errorf("measured %d divergences and %d are recorded: %v", - len(measured), len(bootGeneratedDefaults), measured) + if len(measured) != len(keysAGeneratedFileLeavesToTheirReader) { + t.Errorf("measured %d unanswered keys and %d are recorded: %v", + len(measured), len(keysAGeneratedFileLeavesToTheirReader), measured) + } +} + +// whatTheNodeHolds reads one key off whichever of the two deliveries owns it, and reports whether anything +// answers it. +func whatTheNodeHolds(key string, decoded map[string]string, ctx *server.Context) (string, bool) { + if held, byADecode := decoded[key]; byADecode { + return held, true } + answer := ctx.Viper.Get(key) + if answer == nil { + return "", false + } + return fmt.Sprint(answer), true } -// whatTheBootGenerates returns what a node holds for every declared key of the decoded sections, having -// started with no configuration file of its own. -func whatTheBootGenerates(t *testing.T) map[string]string { +// theBootWithNoFileOfItsOwn starts a node with no configuration files of its own and no sei.toml, and +// returns what it holds. Nothing is delivered, so both deliveries read purely what the boot generated. +// +// Booted twice, and the second one is returned. The first writes the files, and a flag bound to a key the +// writer sets overwrites that value before anything reads it back, because the file has not been read yet. +// From the second boot the file is read and wins, so what a node runs from its second start onward is what +// the second boot holds, and a divergence only that boot shows would otherwise be invisible here. +func theBootWithNoFileOfItsOwn(t *testing.T) *server.Context { t.Helper() home := configtest.NewHome(t) if err := os.MkdirAll(filepath.Join(home.Root, "config"), 0o750); err != nil { t.Fatalf("mkdir: %v", err) } - // Booted twice, and the second one is measured. The first writes the file, and a flag bound to a key - // the writer sets overwrites that value before anything reads it back, because the file has not been - // read yet. From the second boot the file is read and wins, so what a node runs from its second start - // onward is what the second boot holds, and a divergence only that boot shows would otherwise be - // invisible here. var ctx *server.Context for boot := 0; boot < 2; boot++ { cmd := server.StartCmd(nil, home.Root, []trace.TracerProviderOption{}) @@ -122,6 +279,8 @@ func whatTheBootGenerates(t *testing.T) map[string]string { if ctx.Config == nil { t.Fatal("the boot produced no node configuration") } - - return configmanager.DescribeForTest(t, ctx.Config, keysADecodeDelivers()) + if ctx.Viper == nil { + t.Fatal("the boot produced no configuration source") + } + return ctx } diff --git a/cmd/seid/cmd/root.go b/cmd/seid/cmd/root.go index 9ca0dfef62..30378e3105 100644 --- a/cmd/seid/cmd/root.go +++ b/cmd/seid/cmd/root.go @@ -133,6 +133,7 @@ func initRootCmd( // node's sei.toml and writes nothing. configCmd := config.Cmd() configCmd.AddCommand(configmanager.CheckCmd()) + configCmd.AddCommand(configmanager.GenerateCmd()) rootCmd.AddCommand( InitCmd(app.ModuleBasics, app.DefaultNodeHome),