Fix PoolBatch index, beginHeight issues and genesis logic

.github/workflows/test.yml

@@ -90,15 +90,41 @@ jobs:
         with:
           file: ./coverage.txt
 
-  test-simulation:
+  test-sim-nondeterminism:
     runs-on: ubuntu-latest
     timeout-minutes: 15
     steps:
       - uses: actions/checkout@v2
       - uses: actions/[email protected]
         with:
-          go-version: 1.15
+          go-version: 1.16
       - name: Display go version
         run: go version
       - name: Testing simulation
         run: make test-sim-nondeterminism
+
+  test-sim-after-import:
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/[email protected]
+        with:
+          go-version: 1.16
+      - name: Display go version
+        run: go version
+      - name: Testing simulation
+        run: make test-sim-after-import
+
+  test-sim-import-export:
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/[email protected]
+        with:
+          go-version: 1.16
+      - name: Display go version
+        run: go version
+      - name: Testing simulation
+        run: make test-sim-import-export

CHANGELOG.md

@@ -37,13 +37,22 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
+## [v1.3.0](https://github.com/tendermint/liquidity/releases/tag/v1.3.0) - 2021-08-31
+
 ### State Machine Breaking
 
+* [\#433](https://github.com/tendermint/liquidity/pull/433) (sdk) Bump SDK version to [v0.43.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0).
+
 * [\#436](https://github.com/tendermint/liquidity/pull/436) Validation `MsgSwapWithinBatch` and `OfferCoinFee` ceiling
   * When calculating `OfferCoinFee`, the decimal points are rounded up.
+    - before (v1.2.x):  `MsgSwapWithinBatch.OfferCoinFee` should be `OfferCoin` * `params.SwapFeeRate/2` with Truncate or 0
+    - after (v1.3.x):  `MsgSwapWithinBatch.OfferCoinFee` should be `OfferCoin` * `params.SwapFeeRate/2` with Ceil
   * Fix reserveOfferCoinFee residual Issue due to decimal error
 
-* [\#433](https://github.com/tendermint/liquidity/pull/433) (sdk) Bump SDK version to [v0.43.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0).  
+* [\#438](https://github.com/tendermint/liquidity/pull/438) Fix PoolBatch index, beginHeight issues and genesis logic
+  * Remove `PoolBatchIndex`
+  * Fix `PoolBatch.Index` duplicated bug
+  * Fix `PoolBatch.BeginHeight` consistency issue on genesis init logic  
 
 ## [v1.2.9](https://github.com/tendermint/liquidity/releases/tag/v1.2.9) - 2021-06-26
  * Liquidity module version 1 for Gravity-DEX

Makefile

@@ -161,11 +161,11 @@ test-sim-nondeterminism:
 
 test-sim-import-export: runsim
 	@echo "Running application import/export simulation. This may take several minutes..."
-	@$(BINDIR)/runsim -Jobs=1 -SimAppPkg=$(SIMAPP) -ExitOnFail 50 5 TestAppImportExport
+	@$(BINDIR)/runsim -Jobs=4 -SimAppPkg=$(SIMAPP) -ExitOnFail 10 5 TestAppImportExport
 
 test-sim-after-import: runsim
 	@echo "Running application simulation-after-import. This may take several minutes..."
-	@$(BINDIR)/runsim -Jobs=4 -SimAppPkg=$(SIMAPP) -ExitOnFail 1 1 TestAppSimulationAfterImport
+	@$(BINDIR)/runsim -Jobs=4 -SimAppPkg=$(SIMAPP) -ExitOnFail 10 5 TestAppSimulationAfterImport
 
 .PHONY: \
 test-sim-nondeterminism \

README.md

@@ -168,11 +168,11 @@ $BINARY tx liquidity swap 1 1 50000000uusd uatom 0.019 0.003 --from validator --
 
 # An example of generating unsigned tx
 validator=$($BINARY keys show validator --keyring-backend test -a)
-$BINARY tx liquidity swap 1 1 50000000uusd uatom 0.019 0.003 --from $validator --chain-id testing --generate-only > tx_swap.json
+$BINARY tx liquidity swap 1 1 50000000uusd uatom 0.019 0.003 --from $validator --chain-id testing --generate-only &> tx_swap.json
 cat tx_swap.json
 
 # Sign the unsigned tx
-$BINARY tx sign tx_swap.json --from validator --chain-id testing --keyring-backend test -y > tx_swap_signed.json
+$BINARY tx sign tx_swap.json --from validator --chain-id testing --keyring-backend test -y &> tx_swap_signed.json
 cat tx_swap_signed.json
 
 # Encode the signed tx

app/app_test.go

@@ -0,0 +1,268 @@
+package app
+
+import (
+	"encoding/json"
+	"os"
+	"testing"
+
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/require"
+	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/libs/log"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
+	dbm "github.com/tendermint/tm-db"
+
+	"github.com/cosmos/cosmos-sdk/baseapp"
+	"github.com/cosmos/cosmos-sdk/tests/mocks"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/module"
+	"github.com/cosmos/cosmos-sdk/x/auth"
+	"github.com/cosmos/cosmos-sdk/x/auth/vesting"
+	authzmodule "github.com/cosmos/cosmos-sdk/x/authz/module"
+	"github.com/cosmos/cosmos-sdk/x/bank"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	"github.com/cosmos/cosmos-sdk/x/capability"
+	"github.com/cosmos/cosmos-sdk/x/crisis"
+	"github.com/cosmos/cosmos-sdk/x/distribution"
+	"github.com/cosmos/cosmos-sdk/x/evidence"
+	feegrantmodule "github.com/cosmos/cosmos-sdk/x/feegrant/module"
+	"github.com/cosmos/cosmos-sdk/x/genutil"
+	"github.com/cosmos/cosmos-sdk/x/gov"
+	"github.com/cosmos/cosmos-sdk/x/mint"
+	"github.com/cosmos/cosmos-sdk/x/params"
+	"github.com/cosmos/cosmos-sdk/x/slashing"
+	"github.com/cosmos/cosmos-sdk/x/staking"
+	"github.com/cosmos/cosmos-sdk/x/upgrade"
+
+	"github.com/tendermint/liquidity/x/liquidity"
+)
+
+func TestSimAppExportAndBlockedAddrs(t *testing.T) {
+	encCfg := MakeEncodingConfig()
+	db := dbm.NewMemDB()
+	app := NewLiquidityApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{})
+
+	for acc := range maccPerms {
+		require.True(
+			t,
+			app.BankKeeper.BlockedAddr(app.AccountKeeper.GetModuleAddress(acc)),
+			"ensure that blocked addresses are properly set in bank keeper",
+		)
+	}
+
+	genesisState := NewDefaultGenesisState()
+	stateBytes, err := json.MarshalIndent(genesisState, "", "  ")
+	require.NoError(t, err)
+
+	// Initialize the chain
+	app.InitChain(
+		abci.RequestInitChain{
+			Validators:    []abci.ValidatorUpdate{},
+			AppStateBytes: stateBytes,
+		},
+	)
+	app.Commit()
+
+	// Making a new app object with the db, so that initchain hasn't been called
+	app2 := NewLiquidityApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{})
+	_, err = app2.ExportAppStateAndValidators(false, []string{})
+	require.NoError(t, err, "ExportAppStateAndValidators should not have an error")
+}
+
+func TestGetMaccPerms(t *testing.T) {
+	dup := GetMaccPerms()
+	require.Equal(t, maccPerms, dup, "duplicated module account permissions differed from actual module account permissions")
+}
+
+func TestRunMigrations(t *testing.T) {
+	db := dbm.NewMemDB()
+	encCfg := MakeEncodingConfig()
+	logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout))
+	app := NewLiquidityApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{})
+
+	// Create a new baseapp and configurator for the purpose of this test.
+	bApp := baseapp.NewBaseApp(appName, logger, db, encCfg.TxConfig.TxDecoder())
+	bApp.SetCommitMultiStoreTracer(nil)
+	bApp.SetInterfaceRegistry(encCfg.InterfaceRegistry)
+	app.BaseApp = bApp
+	app.configurator = module.NewConfigurator(app.appCodec, app.MsgServiceRouter(), app.GRPCQueryRouter())
+
+	// We register all modules on the Configurator, except x/bank. x/bank will
+	// serve as the test subject on which we run the migration tests.
+	//
+	// The loop below is the same as calling `RegisterServices` on
+	// ModuleManager, except that we skip x/bank.
+	for _, module := range app.mm.Modules {
+		if module.Name() == banktypes.ModuleName {
+			continue
+		}
+
+		module.RegisterServices(app.configurator)
+	}
+
+	// Initialize the chain
+	app.InitChain(abci.RequestInitChain{})
+	app.Commit()
+
+	testCases := []struct {
+		name         string
+		moduleName   string
+		forVersion   uint64
+		expRegErr    bool // errors while registering migration
+		expRegErrMsg string
+		expRunErr    bool // errors while running migration
+		expRunErrMsg string
+		expCalled    int
+	}{
+		{
+			"cannot register migration for version 0",
+			"bank", 0,
+			true, "module migration versions should start at 1: invalid version", false, "", 0,
+		},
+		{
+			"throws error on RunMigrations if no migration registered for bank",
+			"", 1,
+			false, "", true, "no migrations found for module bank: not found", 0,
+		},
+		{
+			"can register and run migration handler for x/bank",
+			"bank", 1,
+			false, "", false, "", 1,
+		},
+		{
+			"cannot register migration handler for same module & forVersion",
+			"bank", 1,
+			true, "another migration for module bank and version 1 already exists: internal logic error", false, "", 0,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var err error
+
+			// Since it's very hard to test actual in-place store migrations in
+			// tests (due to the difficulty of maintaining multiple versions of a
+			// module), we're just testing here that the migration logic is
+			// called.
+			called := 0
+
+			if tc.moduleName != "" {
+				// Register migration for module from version `forVersion` to `forVersion+1`.
+				err = app.configurator.RegisterMigration(tc.moduleName, tc.forVersion, func(sdk.Context) error {
+					called++
+
+					return nil
+				})
+
+				if tc.expRegErr {
+					require.EqualError(t, err, tc.expRegErrMsg)
+
+					return
+				}
+			}
+			require.NoError(t, err)
+
+			// Run migrations only for bank. That's why we put the initial
+			// version for bank as 1, and for all other modules, we put as
+			// their latest ConsensusVersion.
+			_, err = app.mm.RunMigrations(
+				app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()}), app.configurator,
+				module.VersionMap{
+					"bank":         1,
+					"auth":         auth.AppModule{}.ConsensusVersion(),
+					"authz":        authzmodule.AppModule{}.ConsensusVersion(),
+					"staking":      staking.AppModule{}.ConsensusVersion(),
+					"mint":         mint.AppModule{}.ConsensusVersion(),
+					"distribution": distribution.AppModule{}.ConsensusVersion(),
+					"slashing":     slashing.AppModule{}.ConsensusVersion(),
+					"gov":          gov.AppModule{}.ConsensusVersion(),
+					"params":       params.AppModule{}.ConsensusVersion(),
+					"upgrade":      upgrade.AppModule{}.ConsensusVersion(),
+					"vesting":      vesting.AppModule{}.ConsensusVersion(),
+					"feegrant":     feegrantmodule.AppModule{}.ConsensusVersion(),
+					"evidence":     evidence.AppModule{}.ConsensusVersion(),
+					"crisis":       crisis.AppModule{}.ConsensusVersion(),
+					"genutil":      genutil.AppModule{}.ConsensusVersion(),
+					"capability":   capability.AppModule{}.ConsensusVersion(),
+					"liquidity":    liquidity.AppModule{}.ConsensusVersion(),
+				},
+			)
+			if tc.expRunErr {
+				require.EqualError(t, err, tc.expRunErrMsg)
+			} else {
+				require.NoError(t, err)
+				// Make sure bank's migration is called.
+				require.Equal(t, tc.expCalled, called)
+			}
+		})
+	}
+}
+
+func TestInitGenesisOnMigration(t *testing.T) {
+	db := dbm.NewMemDB()
+	encCfg := MakeEncodingConfig()
+	logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout))
+	app := NewLiquidityApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{})
+	ctx := app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()})
+
+	// Create a mock module. This module will serve as the new module we're
+	// adding during a migration.
+	mockCtrl := gomock.NewController(t)
+	t.Cleanup(mockCtrl.Finish)
+	mockModule := mocks.NewMockAppModule(mockCtrl)
+	mockDefaultGenesis := json.RawMessage(`{"key": "value"}`)
+	mockModule.EXPECT().DefaultGenesis(gomock.Eq(app.appCodec)).Times(1).Return(mockDefaultGenesis)
+	mockModule.EXPECT().InitGenesis(gomock.Eq(ctx), gomock.Eq(app.appCodec), gomock.Eq(mockDefaultGenesis)).Times(1).Return(nil)
+	mockModule.EXPECT().ConsensusVersion().Times(1).Return(uint64(0))
+
+	app.mm.Modules["mock"] = mockModule
+
+	// Run migrations only for "mock" module. We exclude it from
+	// the VersionMap to simulate upgrading with a new module.
+	_, err := app.mm.RunMigrations(ctx, app.configurator,
+		module.VersionMap{
+			"bank":         bank.AppModule{}.ConsensusVersion(),
+			"auth":         auth.AppModule{}.ConsensusVersion(),
+			"authz":        authzmodule.AppModule{}.ConsensusVersion(),
+			"staking":      staking.AppModule{}.ConsensusVersion(),
+			"mint":         mint.AppModule{}.ConsensusVersion(),
+			"distribution": distribution.AppModule{}.ConsensusVersion(),
+			"slashing":     slashing.AppModule{}.ConsensusVersion(),
+			"gov":          gov.AppModule{}.ConsensusVersion(),
+			"params":       params.AppModule{}.ConsensusVersion(),
+			"upgrade":      upgrade.AppModule{}.ConsensusVersion(),
+			"vesting":      vesting.AppModule{}.ConsensusVersion(),
+			"feegrant":     feegrantmodule.AppModule{}.ConsensusVersion(),
+			"evidence":     evidence.AppModule{}.ConsensusVersion(),
+			"crisis":       crisis.AppModule{}.ConsensusVersion(),
+			"genutil":      genutil.AppModule{}.ConsensusVersion(),
+			"capability":   capability.AppModule{}.ConsensusVersion(),
+			"liquidity":    liquidity.AppModule{}.ConsensusVersion(),
+		},
+	)
+	require.NoError(t, err)
+}
+
+func TestUpgradeStateOnGenesis(t *testing.T) {
+	encCfg := MakeEncodingConfig()
+	db := dbm.NewMemDB()
+	app := NewLiquidityApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{})
+	genesisState := NewDefaultGenesisState()
+	stateBytes, err := json.MarshalIndent(genesisState, "", "  ")
+	require.NoError(t, err)
+
+	// Initialize the chain
+	app.InitChain(
+		abci.RequestInitChain{
+			Validators:    []abci.ValidatorUpdate{},
+			AppStateBytes: stateBytes,
+		},
+	)
+
+	// make sure the upgrade keeper has version map in state
+	ctx := app.NewContext(false, tmproto.Header{})
+	vm := app.UpgradeKeeper.GetModuleVersionMap(ctx)
+	for v, i := range app.mm.Modules {
+		require.Equal(t, vm[v], i.ConsensusVersion())
+	}
+}

app/export.go

@@ -145,7 +145,7 @@ func (app *LiquidityApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAd
 	counter := int16(0)
 
 	for ; iter.Valid(); iter.Next() {
-		addr := sdk.ValAddress(iter.Key()[1:])
+		addr := sdk.ValAddress(stakingtypes.AddressFromValidatorsKey(iter.Key()))
 		validator, found := app.StakingKeeper.GetValidator(ctx, addr)
 		if !found {
 			panic("expected validator, not found")