scmigrate: ensure target key is correctly renamed (backport #8276) (#8280)

Prior to v0.35, the keys for seen-commit records included the applicable
height.  In v0.35 and beyond, we only keep the record for the latest height,
and its key does not include the height.

Update the seen-commit migration to ensure that the record we retain after
migration is correctly renamed to omit the height from its key.

Update the test cases to check for this condition after migrating.

(cherry picked from commit f3858e52de)

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)