Simplify Migration Structs (#1036)

* fail on invalid migration config

* add checks for height and migration

* fixed comment

* better error messages

* simplified migration structs

* migration-package versioning

* fix struct tag

* fixed migration sql bug

* simplified genesis state validation

* added flags

* remove --migration flag

cmd/kwil-admin/cmds/migration/genesis.go

@@ -12,6 +12,7 @@ import (
 	"github.com/kwilteam/kwil-db/cmd/kwil-admin/cmds/common"
 	"github.com/kwilteam/kwil-db/common/chain"
 	"github.com/kwilteam/kwil-db/core/types"
+	"github.com/kwilteam/kwil-db/internal/migrations"
 	"github.com/kwilteam/kwil-db/internal/statesync"
 )
 
@@ -49,11 +50,18 @@ func genesisStateCmd() *cobra.Command {
 				return display.PrintErr(cmd, err)
 			}
 
+			// this check should change in every version:
+			// For backwards compatibility, we should be able to unmarshal structs from previous versions.
+			// Since v0.9 is our first time supporting migration, we only need to check for v0.9.
+			if metadata.Version != migrations.MigrationVersion {
+				return display.PrintErr(cmd, fmt.Errorf("genesis state download is incompatible. Received version: %d, supported versions: [%d]", metadata.Version, migrations.MigrationVersion))
+			}
+
 			// If there is no active migration or if the migration has not started yet, return the migration state
 			// indicating that there is no genesis state to download.
 			if metadata.MigrationState.Status == types.NoActiveMigration ||
 				metadata.MigrationState.Status == types.MigrationNotStarted ||
-				metadata.GenesisConfig == nil || metadata.SnapshotMetadata == nil {
+				metadata.GenesisInfo == nil || metadata.SnapshotMetadata == nil {
 				return display.PrintCmd(cmd, &MigrationState{
 					Info: metadata.MigrationState,
 				})
@@ -69,18 +77,33 @@ func genesisStateCmd() *cobra.Command {
 				return display.PrintErr(cmd, err)
 			}
 
-			// retrieve the genesis config
-			var genCfg chain.GenesisConfig
-			if err = json.Unmarshal(metadata.GenesisConfig, &genCfg); err != nil {
-				return display.PrintErr(cmd, err)
-			}
-
 			// retrieve the snapshot metadata
 			var snapshotMetadata statesync.Snapshot
 			if err = json.Unmarshal(metadata.SnapshotMetadata, &snapshotMetadata); err != nil {
 				return display.PrintErr(cmd, err)
 			}
 
+			genCfg := chain.GenesisConfig{
+				DataAppHash:   metadata.GenesisInfo.AppHash,
+				InitialHeight: metadata.MigrationState.StartHeight,
+				ConsensusParams: &chain.ConsensusParams{
+					BaseConsensusParams: chain.BaseConsensusParams{
+						Migration: chain.MigrationParams{
+							StartHeight: metadata.MigrationState.StartHeight,
+							EndHeight:   metadata.MigrationState.EndHeight,
+						},
+					},
+				},
+			}
+
+			for _, nv := range metadata.GenesisInfo.Validators {
+				genCfg.Validators = append(genCfg.Validators, &chain.GenesisValidator{
+					Name:   nv.Name,
+					PubKey: nv.PubKey,
+					Power:  nv.Power,
+				})
+			}
+
 			// Print the genesis state to the genesis.json file
 			genesisFile := filepath.Join(expandedDir, genesisFileName)
 			err = genCfg.SaveAs(genesisFile)

cmd/kwil-admin/cmds/migration/list.go

@@ -59,7 +59,6 @@ func (m *MigrationsList) MarshalText() ([]byte, error) {
 		msg.WriteString(fmt.Sprintf("%s:\n", migration.ID))
 		msg.WriteString(fmt.Sprintf("\tactivationPeriod: %d\n", migration.ActivationPeriod))
 		msg.WriteString(fmt.Sprintf("\tmigrationDuration: %d\n", migration.Duration))
-		msg.WriteString(fmt.Sprintf("\tchainID: %s\n", migration.ChainID))
 		msg.WriteString(fmt.Sprintf("\ttimestamp: %s\n", migration.Timestamp))
 	}
 	return msg.Bytes(), nil

cmd/kwil-admin/cmds/migration/propose.go

@@ -13,17 +13,20 @@ import (
 )
 
 var (
-	proposeLong = "A Validator operator can submit a migration proposal using the `propose` subcommand. The migration proposal includes the new `chain-id`, `activation-period` and `duration`. This action will generate a migration resolution for the other validators to vote on. If a super-majority of validators approve the migration proposal, the migration will commence after the specified activation-period blocks from approval and will continue for the duration defined by duration blocks."
+	proposeLong = `A validator operator can submit a migration proposal using the ` + "`" + `propose` + "`" + ` subcommand.
+	
+The migration proposal includes the ` + "`" + `activation-period` + "`" + ` and ` + "`" + `duration` + "`" + `. This will generate a migration resolution
+for the other validators to vote on. If a super-majority of validators approve the migration proposal, the migration will
+commence after the specified activation-period blocks from approval and will continue for the duration defined by duration blocks.`
 
-	proposeExample = `# Submit a migration proposal to migrate to a new chain "kwil-chain-new" with activation period 1000 and migration duration of 14400 blocks.
-kwil-admin migrate propose --activation-period 1000 --duration 14400 --chain-id kwil-chain-new
+	proposeExample = `# Submit a migration proposal to migrate to a new chain with activation period 1000 and migration duration of 14400 blocks.
+kwil-admin migrate propose --activation-period 1000 --duration 14400
 (or)
-kwil-admin migrate propose -a 1000 -d 14400 -c kwil-chain-new`
+kwil-admin migrate propose -a 1000 -d 14400`
 )
 
 func proposeCmd() *cobra.Command {
 	var activationPeriod, migrationDuration uint64
-	var chainID string
 
 	cmd := &cobra.Command{
 		Use:     "propose",
@@ -42,14 +45,9 @@ func proposeCmd() *cobra.Command {
 				return display.PrintErr(cmd, errors.New("start-height and migration duration must be greater than 0"))
 			}
 
-			if chainID == "" {
-				return display.PrintErr(cmd, errors.New("chain-id configuration is not set"))
-			}
-
 			proposal := migrations.MigrationDeclaration{
 				ActivationPeriod: activationPeriod,
 				Duration:         migrationDuration,
-				ChainID:          chainID,
 				Timestamp:        time.Now().String(),
 			}
 			proposalBts, err := proposal.MarshalBinary()
@@ -70,6 +68,5 @@ func proposeCmd() *cobra.Command {
 
 	cmd.Flags().Uint64VarP(&activationPeriod, "activation-period", "a", 0, "The number of blocks before the migration is activated since the approval of the proposal.")
 	cmd.Flags().Uint64VarP(&migrationDuration, "duration", "d", 0, "The duration of the migration.")
-	cmd.Flags().StringVarP(&chainID, "chain-id", "c", "", "The chain ID of the migration.")
 	return cmd
 }

cmd/kwil-admin/cmds/setup/genesis.go

@@ -21,9 +21,6 @@ var (
 
 This command creates a new genesis file with optionally specified modifications.
 
-If the ` + "`" + `--migration` + "`" + ` flag is set, an incomplete genesis file is generated that can be used in a zero
-downtime migration. If generating a migration genesis file, validators and initial state cannot be set.
-
 Validators, balance allocations, and forks should have the format "name:key:power", "address:balance",
 and "name:height" respectively.`
 
@@ -34,16 +31,13 @@ kwil-admin setup genesis
 kwil-admin setup genesis --out /path/to/directory --chain-id mychainid --validator my_validator:890fe7ae9cb1fa6177555d5651e1b8451b4a9c64021c876236c700bc2690ff1d:1
 
 # Create a new genesis.json with the specified allocation
-kwil-admin setup genesis --alloc 0x7f5f4552091a69125d5dfcb7b8c2659029395bdf:100
-
-# Create a new genesis.json file to be used in a network migration
-kwil-admin setup genesis --migration --out /path/to/directory --chain-id mychainid`
+kwil-admin setup genesis --alloc 0x7f5f4552091a69125d5dfcb7b8c2659029395bdf:100`
 )
 
 func genesisCmd() *cobra.Command {
 	var validators, allocs, forks []string
 	var chainID, output, genesisState string
-	var migration, withGasCosts bool
+	var withGasCosts bool
 	var maxBytesPerBlock, joinExpiry, voteExpiry, maxVotesPerBlock int64
 	cmd := &cobra.Command{
 		Use:     "genesis",
@@ -69,10 +63,6 @@ func genesisCmd() *cobra.Command {
 
 			genesisInfo := chain.DefaultGenesisConfig()
 			if cmd.Flags().Changed(validatorsFlag) {
-				if migration {
-					return makeErr(errors.New("cannot set validators when generating a migration genesis file"))
-				}
-
 				for _, v := range validators {
 					parts := strings.Split(v, ":")
 					if len(parts) != 3 {
@@ -134,17 +124,9 @@ func genesisCmd() *cobra.Command {
 				}
 			}
 			if cmd.Flags().Changed(chainIDFlag) {
-				if migration {
-					return makeErr(errors.New("cannot set chain ID when generating a migration genesis file"))
-				}
-
 				genesisInfo.ChainID = chainID
 			}
 			if cmd.Flags().Changed(genesisStateFlag) {
-				if migration {
-					return makeErr(errors.New("cannot set genesis state when generating a migration genesis file"))
-				}
-
 				apphash, err := appHashFromSnapshotFile(genesisState)
 				if err != nil {
 					return makeErr(err)
@@ -166,12 +148,6 @@ func genesisCmd() *cobra.Command {
 				genesisInfo.ConsensusParams.Votes.MaxVotesPerTx = maxVotesPerBlock
 			}
 
-			if migration {
-				genesisInfo.Validators = nil
-				genesisInfo.Alloc = nil
-				genesisInfo.ForkHeights = nil
-			}
-
 			existingFile, err := os.Stat(out)
 			if err == nil && existingFile.IsDir() {
 				return makeErr(fmt.Errorf("a directory already exists at %s, please remove it first", out))
@@ -198,7 +174,6 @@ func genesisCmd() *cobra.Command {
 	cmd.Flags().Int64Var(&joinExpiry, joinExpiryFlag, 0, "Number of blocks before a join proposal expires")
 	cmd.Flags().Int64Var(&voteExpiry, voteExpiryFlag, 0, "Number of blocks before a vote proposal expires")
 	cmd.Flags().Int64Var(&maxVotesPerBlock, maxVotesPerTxFlag, 0, "Maximum number of votes per validator transaction (each validator has 1 validator tx per block)")
-	cmd.Flags().BoolVar(&migration, migrationFlag, false, "Generate an incomplete genesis file for zero downtime migration")
 	cmd.Flags().StringVar(&genesisState, genesisStateFlag, "", "Path to a genesis state file")
 
 	return cmd

cmd/kwild/server/migration.go

@@ -21,6 +21,7 @@ import (
 	"github.com/kwilteam/kwil-db/core/log"
 	"github.com/kwilteam/kwil-db/core/types"
 	"github.com/kwilteam/kwil-db/internal/abci/cometbft"
+	"github.com/kwilteam/kwil-db/internal/migrations"
 	"github.com/kwilteam/kwil-db/internal/statesync"
 )
 
@@ -60,14 +61,33 @@ func PrepareForMigration(ctx context.Context, kwildCfg *commonCfg.KwildConfig, g
 	logger.Info("Entering migration mode", log.String("migrate_from", kwildCfg.MigrationConfig.MigrateFrom))
 
 	snapshotFileName := config.GenesisStateFileName(kwildCfg.RootDir)
-	// check if genesis hash is set in the genesis config
-	if genesisCfg.DataAppHash != nil &&
-		genesisCfg.ConsensusParams.Migration.StartHeight != 0 &&
-		genesisCfg.ConsensusParams.Migration.EndHeight != 0 &&
-		validateGenesisState(snapshotFileName, genesisCfg.DataAppHash) {
-		// genesis state already downloaded. No need to poll for genesis state
+
+	// if the genesis state is already downloaded, then no need to poll for genesis state
+	_, err := os.Stat(snapshotFileName)
+	if err == nil {
 		logger.Info("Genesis state already downloaded", log.String("genesis snapshot", snapshotFileName))
+
+		if err := validateGenesisState(snapshotFileName, genesisCfg.DataAppHash); err != nil {
+			return nil, nil, err
+		}
+
 		return kwildCfg, genesisCfg, nil
+	} else if !os.IsNotExist(err) {
+		return nil, nil, fmt.Errorf("failed to check genesis state file: %w", err)
+	}
+
+	// if we reach here, then we still need to download the genesis state
+	// Therefore, the genesis app hash, initial height, and migration info
+	// should not already be set in the genesis config.
+	if len(genesisCfg.DataAppHash) != 0 {
+		return nil, nil, errors.New("migration genesis config should not have app hash set")
+	}
+	if genesisCfg.InitialHeight != 0 && genesisCfg.InitialHeight != 1 {
+		// we are forcing users to adopt the height provided by the old chain
+		return nil, nil, errors.New("migration genesis config should not have initial height set")
+	}
+	if genesisCfg.ConsensusParams.Migration.IsMigration() {
+		return nil, nil, errors.New("migration genesis config should not have migration info set")
 	}
 
 	// old chain client
@@ -124,21 +144,23 @@ func (m *migrationClient) downloadGenesisState(ctx context.Context) error {
 		return err
 	}
 
+	// this check should change in every version:
+	// For backwards compatibility, we should be able to unmarshal structs from previous versions.
+	// Since v0.9 is our first time supporting migration, we only need to check for v0.9.
+	if metadata.Version != migrations.MigrationVersion {
+		return fmt.Errorf("genesis state download is incompatible. Received version: %d, supported versions: [%d]", metadata.Version, migrations.MigrationVersion)
+	}
+
 	// Check if the genesis state is ready
 	if metadata.MigrationState.Status == types.NoActiveMigration || metadata.MigrationState.Status == types.MigrationNotStarted {
 		return fmt.Errorf("status %s", metadata.MigrationState.Status.String())
 	}
 
 	// Genesis state should ready
-	if metadata.SnapshotMetadata == nil || metadata.GenesisConfig == nil {
+	if metadata.SnapshotMetadata == nil || metadata.GenesisInfo == nil {
 		return errors.New("genesis state not available")
 	}
 
-	var genCfg chain.GenesisConfig
-	if err := json.Unmarshal(metadata.GenesisConfig, &genCfg); err != nil {
-		return fmt.Errorf("failed to unmarshal genesis config: %w", err)
-	}
-
 	// Save the genesis state
 	var snapshotMetadata statesync.Snapshot
 	if err := json.Unmarshal(metadata.SnapshotMetadata, &snapshotMetadata); err != nil {
@@ -169,11 +191,27 @@ func (m *migrationClient) downloadGenesisState(ctx context.Context) error {
 	}
 
 	// Update the genesis config
-	m.genesisCfg.DataAppHash = genCfg.DataAppHash
-	m.genesisCfg.Validators = genCfg.Validators
-	m.genesisCfg.ConsensusParams.Migration = genCfg.ConsensusParams.Migration
+	m.genesisCfg.DataAppHash = metadata.GenesisInfo.AppHash
+	m.genesisCfg.ConsensusParams.Migration = chain.MigrationParams{
+		StartHeight: metadata.MigrationState.StartHeight,
+		EndHeight:   metadata.MigrationState.EndHeight,
+	}
 	m.genesisCfg.InitialHeight = metadata.MigrationState.StartHeight
 
+	// if validators are not set in the genesis config, then set them.
+	// Otherwise, ignore the validators from the old chain.
+	if len(m.genesisCfg.Validators) == 0 {
+		for _, v := range metadata.GenesisInfo.Validators {
+			m.genesisCfg.Validators = append(m.genesisCfg.Validators, &chain.GenesisValidator{
+				Name:   v.Name,
+				PubKey: v.PubKey,
+				Power:  v.Power,
+			})
+		}
+	} else {
+		m.logger.Warn("Validators already set in the genesis config. Ignoring the validators from the old chain")
+	}
+
 	// persist the genesis config
 	if err := m.genesisCfg.SaveAs(filepath.Join(m.kwildCfg.RootDir, cometbft.GenesisJSONName)); err != nil {
 		return fmt.Errorf("failed to save genesis config: %w", err)
@@ -191,30 +229,34 @@ func (m *migrationClient) downloadGenesisState(ctx context.Context) error {
 	return nil
 }
 
-func validateGenesisState(filename string, appHash []byte) bool {
-	// check if the genesis state file exists
-	if _, err := os.Stat(filename); os.IsNotExist(err) {
-		return false
-	}
+// validateGenesisState validates the genesis state file against the app hash.
+// It is the caller's responsibility to check if the file exists.
+func validateGenesisState(filename string, appHash []byte) error {
+	// we don't need to check if the file exists since the caller should have already checked it
 
 	genesisStateFile, err := os.Open(filename)
 	if err != nil {
-		return false
+		return fmt.Errorf("failed to open genesis state file: %w", err)
 	}
 
 	// gzip reader and hash reader
 	gzipReader, err := gzip.NewReader(genesisStateFile)
 	if err != nil {
-		failBuild(err, "failed to create gzip reader")
+		return fmt.Errorf("failed to create gzip reader: %w", err)
 	}
 	defer gzipReader.Close()
 
 	hasher := sha256.New()
 	_, err = io.Copy(hasher, gzipReader)
 	if err != nil {
-		return false
+		return fmt.Errorf("failed to hash genesis state file: %w", err)
 	}
 
 	hash := hasher.Sum(nil)
-	return appHash != nil && len(hash) == len(appHash) && bytes.Equal(hash, appHash)
+
+	if !bytes.Equal(hash, appHash) {
+		return errors.New("app hash does not match the genesis state")
+	}
+
+	return nil
 }

common/chain/chaincfg.go

@@ -135,6 +135,11 @@ type MigrationParams struct {
 	EndHeight int64 `json:"end_height,omitempty"`
 }
 
+// IsMigration returns true if the migration parameters are set.
+func (m *MigrationParams) IsMigration() bool {
+	return m.StartHeight != 0 && m.EndHeight != 0
+}
+
 func defaultConsensusParams() *ConsensusParams {
 	return &ConsensusParams{
 		BaseConsensusParams: BaseConsensusParams{