Extract a library from the confix command-line tool. (#9012) (#9024)

(cherry picked from commit 18b5a500da)

Pull out the library functionality from scripts/confix and move it to
internal/libs/confix. Replace scripts/confix with a simple stub that has the
same command-line API, but uses the library instead.

Related:

- Move and update unit tests.
- Move scripts/confix/condiff to scripts/condiff.
- Update test data for v34, v35, and v36.
- Update reference diffs.
- Update testdata README.

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>

internal/libs/confix/confix.go

@@ -0,0 +1,155 @@
+// Package confix applies changes to a Tendermint TOML configuration file, to
+// update configurations created with an older version of Tendermint to a
+// compatible format for a newer version.
+package confix
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+
+	"github.com/creachadair/atomicfile"
+	"github.com/creachadair/tomledit"
+	"github.com/creachadair/tomledit/transform"
+	"github.com/spf13/viper"
+
+	"github.com/tendermint/tendermint/config"
+)
+
+// Upgrade reads the configuration file at configPath and applies any
+// transformations necessary to upgrade it to the current version.  If this
+// succeeds, the transformed output is written to outputPath. As a special
+// case, if outputPath == "" the output is written to stdout.
+//
+// It is safe if outputPath == inputPath. If a regular file outputPath already
+// exists, it is overwritten. In case of error, the output is not written.
+//
+// Upgrade is a convenience wrapper for calls to LoadConfig, ApplyFixes, and
+// CheckValid. If the caller requires more control over the behavior of the
+// upgrade, call those functions directly.
+func Upgrade(ctx context.Context, configPath, outputPath string) error {
+	if configPath == "" {
+		return errors.New("empty input configuration path")
+	}
+
+	doc, err := LoadConfig(configPath)
+	if err != nil {
+		return fmt.Errorf("loading config: %v", err)
+	}
+
+	if err := ApplyFixes(ctx, doc); err != nil {
+		return fmt.Errorf("updating %q: %v", configPath, err)
+	}
+
+	var buf bytes.Buffer
+	if err := tomledit.Format(&buf, doc); err != nil {
+		return fmt.Errorf("formatting config: %v", err)
+	}
+
+	// Verify that Tendermint can parse the results after our edits.
+	if err := CheckValid(buf.Bytes()); err != nil {
+		return fmt.Errorf("updated config is invalid: %v", err)
+	}
+
+	if outputPath == "" {
+		_, err = os.Stdout.Write(buf.Bytes())
+	} else {
+		err = atomicfile.WriteData(outputPath, buf.Bytes(), 0600)
+	}
+	return err
+}
+
+// ApplyFixes transforms doc and reports whether it succeeded.
+func ApplyFixes(ctx context.Context, doc *tomledit.Document) error {
+	// Check what version of Tendermint might have created this config file, as
+	// a safety check for the updates we are about to make.
+	tmVersion := GuessConfigVersion(doc)
+	if tmVersion == vUnknown {
+		return errors.New("cannot tell what Tendermint version created this config")
+	} else if tmVersion < v34 || tmVersion > v36 {
+		// TODO(creachadair): Add in rewrites for older versions.  This will
+		// require some digging to discover what the changes were.  The upgrade
+		// instructions do not give specifics.
+		return fmt.Errorf("unable to update version %s config", tmVersion)
+	}
+	return plan.Apply(ctx, doc)
+}
+
+// LoadConfig loads and parses the TOML document from path.
+func LoadConfig(path string) (*tomledit.Document, error) {
+	f, err := os.Open(path)
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	return tomledit.Parse(f)
+}
+
+const (
+	vUnknown = ""
+	v32      = "v0.32"
+	v33      = "v0.33"
+	v34      = "v0.34"
+	v35      = "v0.35"
+	v36      = "v0.36"
+)
+
+// GuessConfigVersion attempts to figure out which version of Tendermint
+// created the specified config document. It returns "" if the creating version
+// cannot be determined, otherwise a string of the form "vX.YY".
+func GuessConfigVersion(doc *tomledit.Document) string {
+	hasDisableWS := doc.First("rpc", "experimental-disable-websocket") != nil
+	hasUseLegacy := doc.First("p2p", "use-legacy") != nil // v0.35 only
+	if hasDisableWS && !hasUseLegacy {
+		return v36
+	}
+
+	hasBlockSync := transform.FindTable(doc, "blocksync") != nil // add: v0.35
+	hasStateSync := transform.FindTable(doc, "statesync") != nil // add: v0.34
+	if hasBlockSync && hasStateSync {
+		return v35
+	} else if hasStateSync {
+		return v34
+	}
+
+	hasIndexKeys := doc.First("tx_index", "index_keys") != nil // add: v0.33
+	hasIndexTags := doc.First("tx_index", "index_tags") != nil // rem: v0.33
+	if hasIndexKeys && !hasIndexTags {
+		return v33
+	}
+
+	hasFastSync := transform.FindTable(doc, "fastsync") != nil // add: v0.32
+	if hasIndexTags && hasFastSync {
+		return v32
+	}
+
+	// Something older, probably.
+	return vUnknown
+}
+
+// CheckValid checks whether the specified config appears to be a valid
+// Tendermint config file. This emulates how the node loads the config.
+func CheckValid(data []byte) error {
+	v := viper.New()
+	v.SetConfigType("toml")
+
+	if err := v.ReadConfig(bytes.NewReader(data)); err != nil {
+		return fmt.Errorf("reading config: %w", err)
+	}
+
+	var cfg config.Config
+	if err := v.Unmarshal(&cfg); err != nil {
+		return fmt.Errorf("decoding config: %w", err)
+	}
+
+	return cfg.ValidateBasic()
+}
+
+// WithLogWriter returns a child of ctx with a logger attached that sends
+// output to w. This is a convenience wrapper for transform.WithLogWriter.
+func WithLogWriter(ctx context.Context, w io.Writer) context.Context {
+	return transform.WithLogWriter(ctx, w)
+}

internal/libs/confix/confix_test.go

@@ -1,4 +1,4 @@
-package main_test
+package confix_test
 
 import (
 	"bytes"
@@ -9,7 +9,7 @@ import (
 	"github.com/creachadair/tomledit"
 	"github.com/google/go-cmp/cmp"
 
-	confix "github.com/tendermint/tendermint/scripts/confix"
+	"github.com/tendermint/tendermint/internal/libs/confix"
 )
 
 func mustParseConfig(t *testing.T, path string) *tomledit.Document {

internal/libs/confix/plan.go

@@ -1,4 +1,4 @@
-package main
+package confix
 
 import (
 	"context"

internal/libs/confix/testdata/README.md

@@ -41,12 +41,12 @@ The files named `diff-XX-YY.txt` were generated by using the `condiff` tool on
 the config samples for versions v0.XX and v0.YY:
 
 ```shell
-go run ./scripts/confix/condiff -desnake vXX-config vYY-config.toml > diff-XX-YY.txt
+go run ./scripts/condiff -desnake vXX-config vYY-config.toml > diff-XX-YY.txt
 ```
 
 The `baseline.txt` was computed in the same way, but using an empty starting
 file so that we capture all the settings in the target:
 
 ```shell
-go run ./scripts/confix/condiff -desnake /dev/null v26-config.toml > baseline.txt
+go run ./scripts/condiff -desnake /dev/null v26-config.toml > baseline.txt
 ```

internal/libs/confix/testdata/diff-34-35.txt

@@ -8,13 +8,11 @@
 +M blocksync.version
 -S fastsync
 -M fastsync.version
-+M mempool.ttl-duration
-+M mempool.ttl-num-blocks
-+M mempool.version
 -M mempool.wal-dir
 +M p2p.bootstrap-peers
 +M p2p.max-connections
 +M p2p.max-incoming-connection-attempts
++M p2p.max-outgoing-connections
 +M p2p.queue-type
 -M p2p.seed-mode
 +M p2p.use-legacy
@@ -28,4 +26,3 @@
 -M statesync.chunk-fetchers
 +M statesync.fetchers
 +M statesync.use-p2p
-+M tx-index.psql-conn

internal/libs/confix/testdata/v34-config.toml

@@ -272,6 +272,11 @@ dial_timeout = "3s"
 #######################################################
 [mempool]
 
+# Mempool version to use:
+#   1) "v0" - (default) FIFO mempool.
+#   2) "v1" - prioritized mempool.
+version = "v0"
+
 recheck = true
 broadcast = true
 wal_dir = ""
@@ -301,6 +306,22 @@ max_tx_bytes = 1048576
 # XXX: Unused due to https://github.com/tendermint/tendermint/issues/5796
 max_batch_bytes = 0
 
+# ttl-duration, if non-zero, defines the maximum amount of time a transaction
+# can exist for in the mempool.
+#
+# Note, if ttl-num-blocks is also defined, a transaction will be removed if it
+# has existed in the mempool at least ttl-num-blocks number of blocks or if it's
+# insertion time into the mempool is beyond ttl-duration.
+ttl-duration = "0s"
+
+# ttl-num-blocks, if non-zero, defines the maximum number of blocks a transaction
+# can exist for in the mempool.
+#
+# Note, if ttl-duration is also defined, a transaction will be removed if it
+# has existed in the mempool at least ttl-num-blocks number of blocks or if
+# it's insertion time into the mempool is beyond ttl-duration.
+ttl-num-blocks = 0
+
 #######################################################
 ###         State Sync Configuration Options        ###
 #######################################################
@@ -403,8 +424,14 @@ peer_query_maj23_sleep_duration = "2s"
 #   1) "null"
 #   2) "kv" (default) - the simplest possible indexer, backed by key-value storage (defaults to levelDB; see DBBackend).
 # 		- When "kv" is chosen "tx.height" and "tx.hash" will always be indexed.
+#   3) "psql" - the indexer services backed by PostgreSQL.
+# When "kv" or "psql" is chosen "tx.height" and "tx.hash" will always be indexed.
 indexer = "kv"
 
+# The PostgreSQL connection configuration, the connection format:
+#   postgresql://<user>:<password>@<host>:<port>/<db>?<opts>
+psql-conn = ""
+
 #######################################################
 ###       Instrumentation Configuration Options     ###
 #######################################################

internal/libs/confix/testdata/v35-config.toml

@@ -227,7 +227,9 @@ pprof-laddr = ""
 # Enable the legacy p2p layer.
 use-legacy = false
 
-# Select the p2p internal queue
+# Select the p2p internal queue.
+# Options are: "fifo", "simple-priority", "priority", and "wdrr"
+# with the default being "priority".
 queue-type = "priority"
 
 # Address to listen for incoming connections
@@ -281,6 +283,10 @@ max-num-outbound-peers = 10
 # Maximum number of connections (inbound and outbound).
 max-connections = 64
 
+# Maximum number of connections reserved for outgoing
+# connections. Must be less than max-connections
+max-outgoing-connections = 12
+
 # Rate limits the number of incoming connection attempts per IP address.
 max-incoming-connection-attempts = 100
 

internal/libs/confix/testdata/v36-config.toml

@@ -208,8 +208,10 @@ pprof-laddr = ""
 #######################################################
 [p2p]
 
-# Select the p2p internal queue
+# Select the p2p internal queue.
-queue-type = "priority"
+# Options are: "fifo", "simple-priority", and "priority",
+# with the default being "priority".
+queue-type = "simple-priority"
 
 # Address to listen for incoming connections
 laddr = "tcp://0.0.0.0:26656"
@@ -235,6 +237,10 @@ upnp = false
 # Maximum number of connections (inbound and outbound).
 max-connections = 64
 
+# Maximum number of connections reserved for outgoing
+# connections. Must be less than max-connections
+max-outgoing-connections = 12
+
 # Rate limits the number of incoming connection attempts per IP address.
 max-incoming-connection-attempts = 100
 

scripts/confix/confix.go

@@ -1,24 +1,17 @@
-// Program confix applies fixes to a Tendermint TOML configuration file to
+// Program confix applies changes to a Tendermint TOML configuration file, to
-// update a file created with an older version of Tendermint to a compatible
+// update configurations created with an older version of Tendermint to a
-// format for a newer version.
+// compatible format for a newer version.
 package main
 
 import (
-	"bytes"
 	"context"
-	"errors"
 	"flag"
 	"fmt"
 	"log"
 	"os"
 	"path/filepath"
 
-	"github.com/creachadair/atomicfile"
+	"github.com/tendermint/tendermint/internal/libs/confix"
-	"github.com/creachadair/tomledit"
-	"github.com/creachadair/tomledit/transform"
-	"github.com/spf13/viper"
-
-	"github.com/tendermint/tendermint/config"
 )
 
 func init() {
@@ -42,6 +35,7 @@ Options:
 var (
 	configPath = flag.String("config", "", "Config file path (required)")
 	outPath    = flag.String("out", "", "Output file path (default stdout)")
+	doVerbose  = flag.Bool("v", false, "Log changes to stderr")
 )
 
 func main() {
@@ -50,115 +44,11 @@ func main() {
 		log.Fatal("You must specify a non-empty -config path")
 	}
 
-	doc, err := LoadConfig(*configPath)
+	ctx := context.Background()
-	if err != nil {
+	if *doVerbose {
-		log.Fatalf("Loading config: %v", err)
+		ctx = confix.WithLogWriter(ctx, os.Stderr)
 	}
-
+	if err := confix.Upgrade(ctx, *configPath, *outPath); err != nil {
-	ctx := transform.WithLogWriter(context.Background(), os.Stderr)
+		log.Fatalf("Upgrading config: %v", err)
-	if err := ApplyFixes(ctx, doc); err != nil {
-		log.Fatalf("Updating %q: %v", *configPath, err)
-	}
-
-	var buf bytes.Buffer
-	if err := tomledit.Format(&buf, doc); err != nil {
-		log.Fatalf("Formatting config: %v", err)
-	}
-
-	// Verify that Tendermint can parse the results after our edits.
-	if err := CheckValid(buf.Bytes()); err != nil {
-		log.Fatalf("Updated config is invalid: %v", err)
-	}
-
-	if *outPath == "" {
-		os.Stdout.Write(buf.Bytes())
-	} else if err := atomicfile.WriteData(*outPath, buf.Bytes(), 0600); err != nil {
-		log.Fatalf("Writing output: %v", err)
 	}
 }
-
-// ApplyFixes transforms doc and reports whether it succeeded.
-func ApplyFixes(ctx context.Context, doc *tomledit.Document) error {
-	// Check what version of Tendermint might have created this config file, as
-	// a safety check for the updates we are about to make.
-	tmVersion := GuessConfigVersion(doc)
-	if tmVersion == vUnknown {
-		return errors.New("cannot tell what Tendermint version created this config")
-	} else if tmVersion < v34 || tmVersion > v36 {
-		// TODO(creachadair): Add in rewrites for older versions.  This will
-		// require some digging to discover what the changes were.  The upgrade
-		// instructions do not give specifics.
-		return fmt.Errorf("unable to update version %s config", tmVersion)
-	}
-	return plan.Apply(ctx, doc)
-}
-
-// LoadConfig loads and parses the TOML document from path.
-func LoadConfig(path string) (*tomledit.Document, error) {
-	f, err := os.Open(path)
-	if err != nil {
-		return nil, err
-	}
-	defer f.Close()
-	return tomledit.Parse(f)
-}
-
-const (
-	vUnknown = ""
-	v32      = "v0.32"
-	v33      = "v0.33"
-	v34      = "v0.34"
-	v35      = "v0.35"
-	v36      = "v0.36"
-)
-
-// GuessConfigVersion attempts to figure out which version of Tendermint
-// created the specified config document. It returns "" if the creating version
-// cannot be determined, otherwise a string of the form "vX.YY".
-func GuessConfigVersion(doc *tomledit.Document) string {
-	hasDisableWS := doc.First("rpc", "experimental-disable-websocket") != nil
-	hasUseLegacy := doc.First("p2p", "use-legacy") != nil // v0.35 only
-	if hasDisableWS && !hasUseLegacy {
-		return v36
-	}
-
-	hasBlockSync := transform.FindTable(doc, "blocksync") != nil // add: v0.35
-	hasStateSync := transform.FindTable(doc, "statesync") != nil // add: v0.34
-	if hasBlockSync && hasStateSync {
-		return v35
-	} else if hasStateSync {
-		return v34
-	}
-
-	hasIndexKeys := doc.First("tx_index", "index_keys") != nil // add: v0.33
-	hasIndexTags := doc.First("tx_index", "index_tags") != nil // rem: v0.33
-	if hasIndexKeys && !hasIndexTags {
-		return v33
-	}
-
-	hasFastSync := transform.FindTable(doc, "fastsync") != nil // add: v0.32
-	if hasIndexTags && hasFastSync {
-		return v32
-	}
-
-	// Something older, probably.
-	return vUnknown
-}
-
-// CheckValid checks whether the specified config appears to be a valid
-// Tendermint config file. This emulates how the node loads the config.
-func CheckValid(data []byte) error {
-	v := viper.New()
-	v.SetConfigType("toml")
-
-	if err := v.ReadConfig(bytes.NewReader(data)); err != nil {
-		return fmt.Errorf("reading config: %w", err)
-	}
-
-	var cfg config.Config
-	if err := v.Unmarshal(&cfg); err != nil {
-		return fmt.Errorf("decoding config: %w", err)
-	}
-
-	return cfg.ValidateBasic()
-}