Merge branch 'master' into thane/7655-vote-extensions

CHANGELOG.md

@@ -230,6 +230,12 @@ Special thanks to external contributors on this release: @JayT106,
 - [cmd/tendermint/commands] [\#6623](https://github.com/tendermint/tendermint/pull/6623) replace `$HOME/.some/test/dir` with `t.TempDir` (@tanyabouman)
 - [statesync] \6807 Implement P2P state provider as an alternative to RPC (@cmwaters)
 
+## v0.34.19
+
+### BUG FIXES
+
+- [cli] [\#8270](https://github.com/tendermint/tendermint/issues/8270) fix reset commands (@alexanderbez).
+
 ## v0.34.18
 
 ### BREAKING CHANGES

CHANGELOG_PENDING.md

@@ -81,3 +81,4 @@ Special thanks to external contributors on this release:
 - [light] \#7640 Light Client: fix absence proof verification (@ashcherbakov)
 - [light] \#7641 Light Client: fix querying against the latest height (@ashcherbakov)
 - [cli] [#7837](https://github.com/tendermint/tendermint/pull/7837) fix app hash in state rollback. (@yihuang)
+- [cli] \#8276 scmigrate: ensure target key is correctly renamed. (@creachadair)

go.mod

@@ -72,6 +72,7 @@ require (
 	github.com/charithe/durationcheck v0.0.9 // indirect
 	github.com/chavacava/garif v0.0.0-20210405164556-e8a0a408d6af // indirect
 	github.com/containerd/continuity v0.2.1 // indirect
+	github.com/creachadair/taskgroup v0.3.2 // indirect
 	github.com/daixiang0/gci v0.3.3 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/denis-tingaikin/go-header v0.4.3 // indirect

go.sum

@@ -223,6 +223,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsr
 github.com/cpuguy83/go-md2man/v2 v2.0.1/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
 github.com/creachadair/atomicfile v0.2.4 h1:GRjpQLmz/78I4+nBQpGMFrRa9yrL157AUTrA6hnF0YU=
 github.com/creachadair/atomicfile v0.2.4/go.mod h1:BRq8Une6ckFneYXZQ+kO7p1ZZP3I2fzVzf28JxrIkBc=
+github.com/creachadair/taskgroup v0.3.2 h1:zlfutDS+5XG40AOxcHDSThxKzns8Tnr9jnr6VqkYlkM=
+github.com/creachadair/taskgroup v0.3.2/go.mod h1:wieWwecHVzsidg2CsUnFinW1faVN4+kq+TDlRJQ0Wbk=
 github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/cyphar/filepath-securejoin v0.2.2/go.mod h1:FpkQEhXnPnOthhzymB7CGsFk2G9VLXONKD9G7QGMM+4=

scripts/keymigrate/migrate.go

@@ -15,8 +15,8 @@ import (
 	"math/rand"
 	"runtime"
 	"strconv"
-	"sync"
 
+	"github.com/creachadair/taskgroup"
 	"github.com/google/orderedcode"
 	dbm "github.com/tendermint/tm-db"
 )
@@ -27,7 +27,7 @@ type (
 )
 
 func getAllLegacyKeys(db dbm.DB) ([]keyID, error) {
-	out := []keyID{}
+	var out []keyID
 
 	iter, err := db.Iterator(nil, nil)
 	if err != nil {
@@ -43,11 +43,8 @@ func getAllLegacyKeys(db dbm.DB) ([]keyID, error) {
 			continue
 		}
 
-		// there's inconsistency around tm-db's handling of
-		// key copies.
-		nk := make([]byte, len(k))
-		copy(nk, k)
-		out = append(out, nk)
+		// Make an explicit copy, since not all tm-db backends do.
+		out = append(out, []byte(string(k)))
 	}
 
 	if err = iter.Error(); err != nil {
@@ -61,17 +58,6 @@ func getAllLegacyKeys(db dbm.DB) ([]keyID, error) {
 	return out, nil
 }
 
-func makeKeyChan(keys []keyID) <-chan keyID {
-	out := make(chan keyID, len(keys))
-	defer close(out)
-
-	for _, key := range keys {
-		out <- key
-	}
-
-	return out
-}
-
 func keyIsLegacy(key keyID) bool {
 	for _, prefix := range []keyID{
 		// core "store"
@@ -111,7 +97,7 @@ func keyIsHash(key keyID) bool {
 	return len(key) == 32 && !bytes.Contains(key, []byte("/"))
 }
 
-func migarateKey(key keyID) (keyID, error) {
+func migrateKey(key keyID) (keyID, error) {
 	switch {
 	case bytes.HasPrefix(key, keyID("H:")):
 		val, err := strconv.Atoi(string(key[2:]))
@@ -349,53 +335,23 @@ func Migrate(ctx context.Context, db dbm.DB) error {
 		return err
 	}
 
-	numWorkers := runtime.NumCPU()
-	wg := &sync.WaitGroup{}
+	var errs []string
+	g, start := taskgroup.New(func(err error) error {
+		errs = append(errs, err.Error())
+		return err
+	}).Limit(runtime.NumCPU())
 
-	errs := make(chan error, numWorkers)
-
-	keyCh := makeKeyChan(keys)
-
-	// run migrations.
-	for i := 0; i < numWorkers; i++ {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			for key := range keyCh {
-				err := replaceKey(db, key, migarateKey)
-				if err != nil {
-					errs <- err
-				}
-
-				if ctx.Err() != nil {
-					return
-				}
+	for _, key := range keys {
+		key := key
+		start(func() error {
+			if err := ctx.Err(); err != nil {
+				return err
 			}
-		}()
+			return replaceKey(db, key, migrateKey)
+		})
 	}
-
-	// collect and process the errors.
-	errStrs := []string{}
-	signal := make(chan struct{})
-	go func() {
-		defer close(signal)
-		for err := range errs {
-			if err == nil {
-				continue
-			}
-			errStrs = append(errStrs, err.Error())
-		}
-	}()
-
-	// Wait for everything to be done.
-	wg.Wait()
-	close(errs)
-	<-signal
-
-	// check the error results
-	if len(errs) != 0 {
-		return fmt.Errorf("encountered errors during migration: %v", errStrs)
+	if g.Wait() != nil {
+		return fmt.Errorf("encountered errors during migration: %q", errs)
 	}
-
 	return nil
 }

scripts/keymigrate/migrate_test.go

@@ -117,7 +117,7 @@ func TestMigration(t *testing.T) {
 		})
 		t.Run("Conversion", func(t *testing.T) {
 			for kind, le := range legacyPrefixes {
-				nk, err := migarateKey(le)
+				nk, err := migrateKey(le)
 				require.NoError(t, err, kind)
 				require.False(t, keyIsLegacy(nk), kind)
 			}
@@ -159,7 +159,7 @@ func TestMigration(t *testing.T) {
 				"UserKey3":        []byte("foo/bar/baz/1.2/4"),
 			}
 			for kind, key := range table {
-				out, err := migarateKey(key)
+				out, err := migrateKey(key)
 				require.Error(t, err, kind)
 				require.Nil(t, out, kind)
 			}
@@ -177,7 +177,7 @@ func TestMigration(t *testing.T) {
 					return nil, errors.New("hi")
 				}))
 			})
-			t.Run("KeyDisapears", func(t *testing.T) {
+			t.Run("KeyDisappears", func(t *testing.T) {
 				db := dbm.NewMemDB()
 				key := keyID("hi")
 				require.NoError(t, db.Set(key, []byte("world")))
@@ -215,17 +215,6 @@ func TestMigration(t *testing.T) {
 				require.False(t, keyIsLegacy(key))
 			}
 		})
-		t.Run("ChannelConversion", func(t *testing.T) {
-			ch := makeKeyChan([]keyID{
-				makeKey(t, "abc", int64(2), int64(42)),
-				makeKey(t, int64(42)),
-			})
-			count := 0
-			for range ch {
-				count++
-			}
-			require.Equal(t, 2, count)
-		})
 		t.Run("Migrate", func(t *testing.T) {
 			_, db := getLegacyDatabase(t)
 

scripts/scmigrate/migrate.go

@@ -73,8 +73,6 @@ func sortMigrations(scData []toMigrate) {
 	})
 }
 
-func getMigrationsToDelete(in []toMigrate) []toMigrate { return in[1:] }
-
 func getAllSeenCommits(ctx context.Context, db dbm.DB) ([]toMigrate, error) {
 	scKeyPrefix := makeKeyFromPrefix(prefixSeenCommit)
 	iter, err := db.Iterator(
@@ -117,6 +115,34 @@ func getAllSeenCommits(ctx context.Context, db dbm.DB) ([]toMigrate, error) {
 	return scData, nil
 }
 
+func renameRecord(ctx context.Context, db dbm.DB, keep toMigrate) error {
+	wantKey := makeKeyFromPrefix(prefixSeenCommit)
+	if bytes.Equal(keep.key, wantKey) {
+		return nil // we already did this conversion
+	}
+
+	// This record's key has already been converted to the "new" format, we just
+	// now need to trim off the tail.
+	val, err := db.Get(keep.key)
+	if err != nil {
+		return err
+	}
+
+	batch := db.NewBatch()
+	if err := batch.Delete(keep.key); err != nil {
+		return err
+	}
+	if err := batch.Set(wantKey, val); err != nil {
+		return err
+	}
+	werr := batch.Write()
+	cerr := batch.Close()
+	if werr != nil {
+		return werr
+	}
+	return cerr
+}
+
 func deleteRecords(ctx context.Context, db dbm.DB, scData []toMigrate) error {
 	// delete all the remaining stale values in a single batch
 	batch := db.NewBatch()
@@ -141,20 +167,29 @@ func Migrate(ctx context.Context, db dbm.DB) error {
 	scData, err := getAllSeenCommits(ctx, db)
 	if err != nil {
 		return fmt.Errorf("sourcing tasks to migrate: %w", err)
+	} else if len(scData) == 0 {
+		return nil // nothing to do
 	}
 
-	// sort earliest->latest commits.
+	// Sort commits in decreasing order of height.
 	sortMigrations(scData)
 
-	// trim the one we want to save:
-	scData = getMigrationsToDelete(scData)
+	// Keep and rename the newest seen commit, delete the rest.
+	// In TM < v0.35 we kept a last-seen commit for each height; in v0.35 we
+	// retain only the latest.
+	keep, remove := scData[0], scData[1:]
 
-	if len(scData) <= 1 {
+	if err := renameRecord(ctx, db, keep); err != nil {
+		return fmt.Errorf("renaming seen commit record: %w", err)
+	}
+
+	if len(remove) == 0 {
 		return nil
 	}
 
-	// write the migration (remove )
-	if err := deleteRecords(ctx, db, scData); err != nil {
+	// Remove any older seen commits. Prior to v0.35, we kept these records for
+	// all heights, but v0.35 keeps only the latest.
+	if err := deleteRecords(ctx, db, remove); err != nil {
 		return fmt.Errorf("writing data: %w", err)
 	}
 

scripts/scmigrate/migrate_test.go

@@ -1,6 +1,7 @@
 package scmigrate
 
 import (
+	"bytes"
 	"context"
 	"math/rand"
 	"testing"
@@ -104,15 +105,6 @@ func TestMigrations(t *testing.T) {
 			assertWellOrderedMigrations(t, testData)
 		})
 	})
-	t.Run("GetMigrationsToDelete", func(t *testing.T) {
-		for i := 1; i < 100; i++ {
-			data := appendRandomMigrations([]toMigrate{}, i)
-			toMigrate := getMigrationsToDelete(data)
-			if len(data) != len(toMigrate)+1 {
-				t.Fatalf("migration prep did not save one document [original=%d migrations=%d]", len(data), len(toMigrate))
-			}
-		}
-	})
 	t.Run("InvalidMigrations", func(t *testing.T) {
 		if _, err := makeToMigrate(nil); err == nil {
 			t.Fatal("should error for nil migrations")
@@ -156,18 +148,24 @@ func TestMigrations(t *testing.T) {
 			// safe to rerun
 			t.Run(test, func(t *testing.T) {
 				if err := Migrate(ctx, db); err != nil {
-					t.Fatal(err)
+					t.Fatalf("Migration failed: %v", err)
 				}
 
 				post, err := getAllSeenCommits(ctx, db)
 				if err != nil {
-					t.Fatal(err)
+					t.Fatalf("Fetching seen commits: %v", err)
 				}
+
 				if len(post) != 1 {
-					t.Fatal("migration was not successful")
+					t.Fatalf("Wrong number of commits: got %d, wanted 1", len(post))
 				}
-				if post[0].commit.Height != latestHeight {
-					t.Fatal("migration did not save correct document")
+
+				wantKey := makeKeyFromPrefix(prefixSeenCommit)
+				if !bytes.Equal(post[0].key, wantKey) {
+					t.Errorf("Seen commit key: got %x, want %x", post[0].key, wantKey)
+				}
+				if got := post[0].commit.Height; got != latestHeight {
+					t.Fatalf("Wrong commit height after migration: got %d, wanted %d", got, latestHeight)
 				}
 			})
 		}