fix: export command (#3450)

Closes #3392 Opens #3472 Fixes a few bugs: 1. Previously all modules had `ExportGenesis` invoked on them even if they weren't supported by the current app version. Now we only call `ExportGenesis` for the modules that are supported by the current app version 2. The export command wasn't updated to account for the changes in #3320 which force us to mount stores after `app.New()` based on the current app version 3. The minfee module couldn't be exported b/c it didn't register a key table in `ExportGenesis` ## Testing I could export an app on app version 1 and 2. See [output](https://gist.github.com/rootulp/dfea2b5b40f7366b03706fc39321ceee)
celestiaorg · May 14, 2024 · 5c6a797 · 5c6a797
1 parent ce2c099
commit 5c6a797
Show file tree

Hide file tree

Showing 13 changed files with 237 additions and 87 deletions.
diff --git a/app/ante/min_fee_test.go b/app/ante/min_fee_test.go
@@ -147,7 +147,7 @@ func TestCheckTxFeeWithGlobalMinGasPrices(t *testing.T) {
 			require.NoError(t, err)
 
 			subspace, _ := paramsKeeper.GetSubspace(minfee.ModuleName)
-			minfee.RegisterMinFeeParamTable(subspace)
+			subspace = minfee.RegisterMinFeeParamTable(subspace)
 			subspace.Set(ctx, minfee.KeyGlobalMinGasPrice, globalminGasPriceDec)
 
 			_, _, err = ante.ValidateTxFee(ctx, tx, paramsKeeper)

diff --git a/app/app.go b/app/app.go
@@ -169,11 +169,12 @@ type App struct {
 	MsgGateKeeper *ante.MsgVersioningGateKeeper
 }
 
-// New returns a reference to an initialized celestia app.
+// New returns a reference to an uninitialized app. Callers must subsequently
+// call app.Info or app.InitChain to initialize the baseapp.
 //
-// NOTE: upgradeHeight refers specifically to the height that
-// a node will upgrade from v1 to v2. It will be deprecated in v3
-// in place for a dynamically signalling scheme
+// NOTE: upgradeHeightV2 refers specifically to the height that a node will
+// upgrade from v1 to v2. It will be deprecated in v3 in place for a dynamically
+// signalling scheme
 func New(
 	logger log.Logger,
 	db dbm.DB,
@@ -449,7 +450,7 @@ func (app *App) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.R
 	return app.mm.BeginBlock(ctx, req)
 }
 
-// EndBlocker application updates every end block
+// EndBlocker executes application updates at the end of every block.
 func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
 	res := app.mm.EndBlock(ctx, req)
 	currentVersion := app.AppVersion()
@@ -504,8 +505,11 @@ func (app *App) migrateModules(ctx sdk.Context, fromVersion, toVersion uint64) e
 	return app.mm.RunMigrations(ctx, app.configurator, fromVersion, toVersion)
 }
 
-// We wrap Info around baseapp so we can take the app version and
-// setup the multicommit store.
+// Info implements the ABCI interface. This method is a wrapper around baseapp's
+// Info command so that it can take the app version and setup the multicommit
+// store.
+//
+// Side-effect: calls baseapp.Init()
 func (app *App) Info(req abci.RequestInfo) abci.ResponseInfo {
 	if height := app.LastBlockHeight(); height > 0 {
 		ctx, err := app.CreateQueryContext(height, false)
@@ -523,16 +527,16 @@ func (app *App) Info(req abci.RequestInfo) abci.ResponseInfo {
 	resp := app.BaseApp.Info(req)
 	// mount the stores for the provided app version
 	if resp.AppVersion > 0 && !app.IsSealed() {
-		app.MountKVStores(app.versionedKeys(resp.AppVersion))
-		if err := app.LoadLatestVersion(); err != nil {
-			panic(fmt.Sprintf("loading latest version: %s", err.Error()))
-		}
+		app.mountKeysAndInit(resp.AppVersion)
 	}
 	return resp
 }
 
-// We wrap InitChain around baseapp so we can take the app version and
-// setup the multicommit store.
+// InitChain implements the ABCI interface. This method is a wrapper around
+// baseapp's InitChain so we can take the app version and setup the multicommit
+// store.
+//
+// Side-effect: calls baseapp.Init()
 func (app *App) InitChain(req abci.RequestInitChain) (res abci.ResponseInitChain) {
 	// genesis must always contain the consensus params. The validator set however is derived from the
 	// initial genesis state. The genesis must always contain a non zero app version which is the initial
@@ -546,16 +550,24 @@ func (app *App) InitChain(req abci.RequestInitChain) (res abci.ResponseInitChain
 
 	// mount the stores for the provided app version if it has not already been mounted
 	if app.AppVersion() == 0 && !app.IsSealed() {
-		app.MountKVStores(app.versionedKeys(req.ConsensusParams.Version.AppVersion))
-		if err := app.LoadLatestVersion(); err != nil {
-			panic(fmt.Sprintf("loading latest version: %s", err.Error()))
-		}
+		app.mountKeysAndInit(req.ConsensusParams.Version.AppVersion)
 	}
 
 	return app.BaseApp.InitChain(req)
 }
 
-// InitChainer application update at chain initialization
+// mountKeysAndInit mounts the keys for the provided app version and then
+// invokes baseapp.Init().
+func (app *App) mountKeysAndInit(appVersion uint64) {
+	app.MountKVStores(app.versionedKeys(appVersion))
+
+	// Invoke load latest version for it's side-effect of invoking baseapp.Init()
+	if err := app.LoadLatestVersion(); err != nil {
+		panic(fmt.Sprintf("loading latest version: %s", err.Error()))
+	}
+}
+
+// InitChainer is middleware that gets invoked part-way through the baseapp's InitChain invocation.
 func (app *App) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
 	var genesisState GenesisState
 	if err := tmjson.Unmarshal(req.AppStateBytes, &genesisState); err != nil {
@@ -749,3 +761,13 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
 
 	return paramsKeeper
 }
+
+func (app *App) InitializeAppVersion(ctx sdk.Context) {
+	appVersion := app.GetAppVersionFromParamStore(ctx)
+	if appVersion == 0 {
+		// if the param store does not have an app version set, default to v1
+		app.SetAppVersion(ctx, v1)
+	} else {
+		app.SetAppVersion(ctx, appVersion)
+	}
+}
diff --git a/app/app_test.go b/app/app_test.go
@@ -36,6 +36,9 @@ func TestNew(t *testing.T) {
 		// will panic.
 		assert.Panics(t, func() { got.StakingKeeper.SetHooks(nil) })
 	})
+	t.Run("should not have sealed the baseapp", func(t *testing.T) {
+		assert.False(t, got.IsSealed())
+	})
 }
 
 // NoopWriter is a no-op implementation of a writer.

diff --git a/app/export.go b/app/export.go
@@ -4,28 +4,34 @@ import (
 	"encoding/json"
 	"log"
 
-	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
-
 	servertypes "github.com/cosmos/cosmos-sdk/server/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types"
 	"github.com/cosmos/cosmos-sdk/x/staking"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 )
 
-// ExportAppStateAndValidators exports the state of the application for a genesis
-// file.
-func (app *App) ExportAppStateAndValidators(
-	forZeroHeight bool, jailAllowedAddrs []string,
-) (servertypes.ExportedApp, error) {
-	// as if they could withdraw from the start of the next block
-	ctx := app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()})
+// ExportAppStateAndValidators exports the state of the application for a
+// genesis file.
+func (app *App) ExportAppStateAndValidators(forZeroHeight bool, jailAllowedAddrs []string) (servertypes.ExportedApp, error) {
+	ctx, err := app.CreateQueryContext(app.LastBlockHeight(), false)
+	if err != nil {
+		return servertypes.ExportedApp{}, err
+	}
+
+	app.InitializeAppVersion(ctx)
+	if !app.IsSealed() {
+		app.mountKeysAndInit(app.AppVersion())
+	}
+
+	// Create a new context so that the commit multi-store reflects the store
+	// key mounting performed above.
+	ctx, err = app.CreateQueryContext(app.LastBlockHeight(), false)
+	if err != nil {
+		return servertypes.ExportedApp{}, err
+	}
 
-	// We export at last height + 1, because that's the height at which
-	// Tendermint will start InitChain.
-	height := app.LastBlockHeight() + 1
 	if forZeroHeight {
-		height = 0
 		app.prepForZeroHeightGenesis(ctx, jailAllowedAddrs)
 	}
 
@@ -36,12 +42,25 @@ func (app *App) ExportAppStateAndValidators(
 	}
 
 	validators, err := staking.WriteValidators(ctx, app.StakingKeeper)
+	if err != nil {
+		return servertypes.ExportedApp{}, err
+	}
+
 	return servertypes.ExportedApp{
 		AppState:        appState,
 		Validators:      validators,
-		Height:          height,
+		Height:          app.getExportHeight(forZeroHeight),
 		ConsensusParams: app.BaseApp.GetConsensusParams(ctx),
-	}, err
+	}, nil
+}
+
+func (app *App) getExportHeight(forZeroHeight bool) int64 {
+	if forZeroHeight {
+		return 0
+	}
+	// We export at last height + 1, because that's the height at which
+	// Tendermint will start InitChain.
+	return app.LastBlockHeight() + 1
 }
 
 // prepForZeroHeightGenesis preps for fresh start at zero height. Zero height

diff --git a/app/module/manager.go b/app/module/manager.go
@@ -179,11 +179,17 @@ func (m *Manager) InitGenesis(ctx sdk.Context, cdc codec.JSONCodec, genesisData
 	}
 }
 
-// ExportGenesis performs export genesis functionality for modules
+// ExportGenesis performs export genesis functionality for the modules supported
+// in a particular version.
 func (m *Manager) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec, version uint64) map[string]json.RawMessage {
 	genesisData := make(map[string]json.RawMessage)
 	modules := m.versionedModules[version]
-	for _, moduleName := range m.OrderExportGenesis {
+	moduleNamesForVersion := m.ModuleNames(version)
+	moduleNamesToExport := filter(m.OrderExportGenesis, func(moduleName string) bool {
+		// filter out modules that are not supported by this version
+		return slices.Contains(moduleNamesForVersion, moduleName)
+	})
+	for _, moduleName := range moduleNamesToExport {
 		genesisData[moduleName] = modules[moduleName].ExportGenesis(ctx, cdc)
 	}
 

diff --git a/app/module/manager_test.go b/app/module/manager_test.go
@@ -146,34 +146,71 @@ func TestManager_InitGenesis(t *testing.T) {
 }
 
 func TestManager_ExportGenesis(t *testing.T) {
-	mockCtrl := gomock.NewController(t)
-	t.Cleanup(mockCtrl.Finish)
-
-	mockAppModule1 := mocks.NewMockAppModule(mockCtrl)
-	mockAppModule2 := mocks.NewMockAppModule(mockCtrl)
-	mockAppModule1.EXPECT().Name().Times(2).Return("module1")
-	mockAppModule1.EXPECT().ConsensusVersion().Times(1).Return(uint64(1))
-	mockAppModule2.EXPECT().Name().Times(2).Return("module2")
-	mockAppModule2.EXPECT().ConsensusVersion().Times(1).Return(uint64(1))
-	mm, err := module.NewManager([]module.VersionedModule{
-		{Module: mockAppModule1, FromVersion: 1, ToVersion: 1},
-		{Module: mockAppModule2, FromVersion: 1, ToVersion: 1},
+	t.Run("export genesis with two modules at version 1", func(t *testing.T) {
+		mockCtrl := gomock.NewController(t)
+		t.Cleanup(mockCtrl.Finish)
+
+		mockAppModule1 := mocks.NewMockAppModule(mockCtrl)
+		mockAppModule2 := mocks.NewMockAppModule(mockCtrl)
+		mockAppModule1.EXPECT().Name().Times(2).Return("module1")
+		mockAppModule1.EXPECT().ConsensusVersion().Times(1).Return(uint64(1))
+		mockAppModule2.EXPECT().Name().Times(2).Return("module2")
+		mockAppModule2.EXPECT().ConsensusVersion().Times(1).Return(uint64(1))
+		mm, err := module.NewManager([]module.VersionedModule{
+			{Module: mockAppModule1, FromVersion: 1, ToVersion: 1},
+			{Module: mockAppModule2, FromVersion: 1, ToVersion: 1},
+		})
+		require.NoError(t, err)
+		require.NotNil(t, mm)
+		require.Equal(t, 2, len(mm.ModuleNames(1)))
+
+		ctx := sdk.Context{}
+		interfaceRegistry := types.NewInterfaceRegistry()
+		cdc := codec.NewProtoCodec(interfaceRegistry)
+		mockAppModule1.EXPECT().ExportGenesis(gomock.Eq(ctx), gomock.Eq(cdc)).Times(1).Return(json.RawMessage(`{"key1": "value1"}`))
+		mockAppModule2.EXPECT().ExportGenesis(gomock.Eq(ctx), gomock.Eq(cdc)).Times(1).Return(json.RawMessage(`{"key2": "value2"}`))
+
+		want := map[string]json.RawMessage{
+			"module1": json.RawMessage(`{"key1": "value1"}`),
+			"module2": json.RawMessage(`{"key2": "value2"}`),
+		}
+		require.Equal(t, want, mm.ExportGenesis(ctx, cdc, 1))
+	})
+	t.Run("export genesis with one modules at version 1, one modules at version 2", func(t *testing.T) {
+		mockCtrl := gomock.NewController(t)
+		t.Cleanup(mockCtrl.Finish)
+
+		mockAppModule1 := mocks.NewMockAppModule(mockCtrl)
+		mockAppModule2 := mocks.NewMockAppModule(mockCtrl)
+		mockAppModule1.EXPECT().Name().Times(2).Return("module1")
+		mockAppModule1.EXPECT().ConsensusVersion().Times(2).Return(uint64(1))
+		mockAppModule2.EXPECT().Name().Times(2).Return("module2")
+		mockAppModule2.EXPECT().ConsensusVersion().Times(2).Return(uint64(1))
+		mm, err := module.NewManager([]module.VersionedModule{
+			{Module: mockAppModule1, FromVersion: 1, ToVersion: 1},
+			{Module: mockAppModule2, FromVersion: 2, ToVersion: 2},
+		})
+		require.NoError(t, err)
+		require.NotNil(t, mm)
+		require.Equal(t, 1, len(mm.ModuleNames(1)))
+		require.Equal(t, 1, len(mm.ModuleNames(2)))
+
+		ctx := sdk.Context{}
+		interfaceRegistry := types.NewInterfaceRegistry()
+		cdc := codec.NewProtoCodec(interfaceRegistry)
+		mockAppModule1.EXPECT().ExportGenesis(gomock.Eq(ctx), gomock.Eq(cdc)).Times(1).Return(json.RawMessage(`{"key1": "value1"}`))
+		mockAppModule2.EXPECT().ExportGenesis(gomock.Eq(ctx), gomock.Eq(cdc)).Times(1).Return(json.RawMessage(`{"key2": "value2"}`))
+
+		want := map[string]json.RawMessage{
+			"module1": json.RawMessage(`{"key1": "value1"}`),
+		}
+		assert.Equal(t, want, mm.ExportGenesis(ctx, cdc, 1))
+
+		want2 := map[string]json.RawMessage{
+			"module2": json.RawMessage(`{"key2": "value2"}`),
+		}
+		assert.Equal(t, want2, mm.ExportGenesis(ctx, cdc, 2))
 	})
-	require.NoError(t, err)
-	require.NotNil(t, mm)
-	require.Equal(t, 2, len(mm.ModuleNames(1)))
-
-	ctx := sdk.Context{}
-	interfaceRegistry := types.NewInterfaceRegistry()
-	cdc := codec.NewProtoCodec(interfaceRegistry)
-	mockAppModule1.EXPECT().ExportGenesis(gomock.Eq(ctx), gomock.Eq(cdc)).Times(1).Return(json.RawMessage(`{"key1": "value1"}`))
-	mockAppModule2.EXPECT().ExportGenesis(gomock.Eq(ctx), gomock.Eq(cdc)).Times(1).Return(json.RawMessage(`{"key2": "value2"}`))
-
-	want := map[string]json.RawMessage{
-		"module1": json.RawMessage(`{"key1": "value1"}`),
-		"module2": json.RawMessage(`{"key2": "value2"}`),
-	}
-	require.Equal(t, want, mm.ExportGenesis(ctx, cdc, 1))
 }
 
 func TestManager_BeginBlock(t *testing.T) {

diff --git a/app/test/export_test.go b/app/test/export_test.go
@@ -0,0 +1,56 @@
+package app_test
+
+import (
+	"testing"
+
+	"github.com/celestiaorg/celestia-app/v2/app"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	abci "github.com/tendermint/tendermint/abci/types"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
+	tmversion "github.com/tendermint/tendermint/proto/tendermint/version"
+)
+
+func TestExportAppStateAndValidators(t *testing.T) {
+	t.Run("should return exported app for version 1", func(t *testing.T) {
+		forZeroHeight := true
+		jailAllowedAddrs := []string{}
+		testApp, _ := SetupTestAppWithUpgradeHeight(t, 3)
+
+		exported, err := testApp.ExportAppStateAndValidators(forZeroHeight, jailAllowedAddrs)
+		require.NoError(t, err)
+		assert.NotNil(t, exported)
+		assert.Equal(t, uint64(1), exported.ConsensusParams.Version.AppVersion)
+	})
+	t.Run("should return exported app for version 2", func(t *testing.T) {
+		forZeroHeight := false
+		jailAllowedAddrs := []string{}
+
+		testApp, _ := SetupTestAppWithUpgradeHeight(t, 3)
+		upgradeToV2(t, testApp)
+
+		exported, err := testApp.ExportAppStateAndValidators(forZeroHeight, jailAllowedAddrs)
+		require.NoError(t, err)
+		assert.NotNil(t, exported)
+		// TODO: the following assertion is commented out because the exported app does not populate consensus params.version
+		// assert.Equal(t, uint64(2), exported.ConsensusParams.Version.AppVersion)
+	})
+}
+
+func upgradeToV2(t *testing.T, testApp *app.App) {
+	testApp.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{
+		Height:  2,
+		Version: tmversion.Consensus{App: 1},
+	}})
+	// Upgrade from v1 -> v2
+	testApp.EndBlock(abci.RequestEndBlock{Height: 2})
+	testApp.Commit()
+	require.EqualValues(t, 2, testApp.AppVersion())
+	testApp.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{
+		Height:  3,
+		Version: tmversion.Consensus{App: 2},
+	}})
+	testApp.EndBlock(abci.RequestEndBlock{Height: 3})
+	testApp.Commit()
+	require.EqualValues(t, 3, testApp.LastBlockHeight())
+}