Reinstate usage of some SDK functionality unique to our fork. (#1930)

* [1760]: Use ReadPageRequestWithPageKeyDecoded everywhere we used to.

* [1760]: Clean up uses of cosmossdk.io/simapp

* [1760]: Handle some of the count-authz TODOs, but I forgot to update the grant command, so there's a couple things that are still needed in the SDK.

* [1760]: Switch to a local version of the SDK that has the count authz cmd fix.

* [1760]: Fix the rest of the count-authz TODOs.

* [1760]: Use GenerateOrBroadcastTxCLIAsGovProp now that it's back.

* [1760]: Put the locked coins stuff back in.

* [1760]: Put back uses of InputOutputCoins and InputOutputCoinsProv.

* [1760]: Get rid of the test_amino build tag and the make targets for it. Remove the check-test stuff since we've also got build-tests which is easier for me to remember.

* [1760]: Untab the make build comment about removing the delay so that it's actually a comment rather than a command that's printed.

* [1760]: Refactor the make build-tests target to behave just use the same flow as make test, and also give it the same tags.

* [1760]: Update a TODO comment that is now half done.

* [1760]: Use a BaseKeeper for the bank keeper instead of the Keeper interface since that interface doesn't have InputOutputCoinsProv.

* [1760]: Fix the exchange mock bank keeper to have InputOutputCoinsProv instead of the old one.

* [1760]: Use a concrete bank BaseKeeper in the app instead of a reference to one because there's some deep down-stream wiring stuff that assumes it's concrete.

* [1760]: Update the hold tests that check spendable balances. The actual test is commented out right now until the quarantine PR merges that has the needed query helper.

* [1760]: Bump the sdk to v0.50.5-pio-3 (from v0.50.5-pio-2).

* [1760]: Allow defining the TEST_PACKAGES used in the make test target.

* [1760]: Add the migration of bank params to the umber upgrade.

* [1760]: Add changelog entry.

* [1760]: Uncomment the hold cli unit test that gets spendable balances so that once the quarantine PR is merged, this will be ready too.

* [1760]: Do a string comparison of the coins in TestHoldsNotInFromSpendable.

CHANGELOG.md

@@ -61,6 +61,9 @@ Ref: https://keepachangelog.com/en/1.0.0/
   *  Attribute module param migration [#1927](https://github.com/provenance-io/provenance/pull/1927)
   *  Marker module param migration [#1934](https://github.com/provenance-io/provenance/pull/1934)
   *  Metadata module param migration [#1932](https://github.com/provenance-io/provenance/pull/1932)
+* Restore the hold module [#1930](https://github.com/provenance-io/provenance/pull/1930).
+* Restore gov-prop cli commands and fix next key decoding [#1930](https://github.com/provenance-io/provenance/pull/1930).
+* Switch to InputOutputCoinsProv for exchange transfers [#1930](https://github.com/provenance-io/provenance/pull/1930).
 
 ### Dependencies
 

Makefile

@@ -132,7 +132,7 @@ install: go.sum
 	CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CFLAGS="$(CGO_CFLAGS)" $(GO) install $(BUILD_FLAGS) ./cmd/provenanced
 
 build: validate-go-version go.sum
-	# TODO[1760]: Remove this delay once we're stable again.
+# TODO[1760]: Remove this delay once we're stable again.
 	@if [ -z "${ACK_50}" ]; then printf '\033[93mWARNING:\033[0m This branch is currently unstable and should not be built for use.\n         To bypass this 10 second delay: ACK_50=1 make build\n'; sleep 10; fi
 	mkdir -p $(BUILDDIR)
 	CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CFLAGS="$(CGO_CFLAGS)" $(GO) build -o $(BUILDDIR)/ $(BUILD_FLAGS) ./cmd/provenanced
@@ -330,50 +330,36 @@ PACKAGES               := $(shell $(GO) list ./... 2>/dev/null || true)
 PACKAGES_NOSIMULATION  := $(filter-out %/simulation%,$(PACKAGES))
 PACKAGES_SIMULATION    := $(filter     %/simulation%,$(PACKAGES))
 
-TEST_PACKAGES=./...
-TEST_TARGETS := test-unit test-unit-amino test-unit-proto test-ledger-mock test-race test-ledger test-race
+TEST_PACKAGES ?= ./...
+TEST_TARGETS := test-unit test-unit-proto test-ledger-mock test-race test-ledger build-tests
 
 # Test runs-specific rules. To add a new test target, just add
 # a new rule, customise TAGS, ARGS and/or TEST_PACKAGES ad libitum, and
 # append the new rule to the TEST_TARGETS list.
 test-unit: TAGS+=cgo ledger test_ledger_mock norace
-test-unit-amino: TAGS+=ledger test_ledger_mock test_amino norace
+build-tests: TAGS+=cgo ledger test_ledger_mock norace
+build-tests: ARGS+=-run='ZYX_NOPE_NOPE_XYZ'
 test-ledger: TAGS+=cgo ledger norace
 test-ledger-mock: TAGS+=ledger test_ledger_mock norace
 test-race: ARGS+=-race
 test-race: TAGS+=cgo ledger test_ledger_mock
 test-race: TEST_PACKAGES=$(PACKAGES_NOSIMULATION)
 $(TEST_TARGETS): run-tests
 
-# check-* compiles and collects tests without running them
-# note: go test -c doesn't support multiple packages yet (https://github.com/golang/go/issues/15513)
-CHECK_TEST_TARGETS := check-test-unit check-test-unit-amino
-check-test-unit: TAGS+=cgo ledger test_ledger_mock norace
-check-test-unit-amino: TAGS+=ledger test_ledger_mock test_amino norace
-$(CHECK_TEST_TARGETS): ARGS+=-run=none
-$(CHECK_TEST_TARGETS): run-tests
-
 run-tests: go.sum
 ifneq (,$(shell which tparse 2>/dev/null))
 	$(GO) test -mod=readonly -json $(ARGS) -tags='$(TAGS)' $(TEST_PACKAGES) | tparse
 else
 	$(GO) test -mod=readonly $(ARGS) -tags='$(TAGS)' $(TEST_PACKAGES)
 endif
 
-build-tests: go.sum
-ifneq (,$(shell which tparse 2>/dev/null))
-	$(GO) test -mod=readonly -json $(ARGS) -tags='$(TAGS)' -run='ZYX_NOPE_NOPE_XYZ' $(TEST_PACKAGES) | tparse
-else
-	$(GO) test -mod=readonly $(ARGS) -tags='$(TAGS)' -run='ZYX_NOPE_NOPE_XYZ' $(TEST_PACKAGES)
-endif
-
 test-cover:
 	export VERSION=$(VERSION); bash -x contrib/test_cover.sh
 
 benchmark:
 	$(GO) test -mod=readonly -bench=. $(PACKAGES_NOSIMULATION)
 
-.PHONY: test test-all test-unit test-race test-cover benchmark run-tests build-tests $(TEST_TARGETS)
+.PHONY: test test-all test-cover benchmark run-tests build-tests $(TEST_TARGETS)
 
 ##############################
 # Test Network Targets

app/app.go

@@ -8,34 +8,31 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/CosmWasm/wasmd/x/wasm"
-	wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper"
-	wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types"
 	"github.com/gorilla/mux"
 	"github.com/rakyll/statik/fs"
 	"github.com/spf13/cast"
 	"github.com/spf13/viper"
 
+	"github.com/CosmWasm/wasmd/x/wasm"
+	wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper"
+	wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types"
+
 	abci "github.com/cometbft/cometbft/abci/types"
 	cmtos "github.com/cometbft/cometbft/libs/os"
 
 	"cosmossdk.io/log"
 	sdkmath "cosmossdk.io/math"
-	"cosmossdk.io/x/tx/signing"
-
 	storetypes "cosmossdk.io/store/types"
 	"cosmossdk.io/x/evidence"
 	evidencekeeper "cosmossdk.io/x/evidence/keeper"
 	evidencetypes "cosmossdk.io/x/evidence/types"
 	"cosmossdk.io/x/feegrant"
 	feegrantkeeper "cosmossdk.io/x/feegrant/keeper"
 	feegrantmodule "cosmossdk.io/x/feegrant/module"
+	"cosmossdk.io/x/tx/signing"
 	"cosmossdk.io/x/upgrade"
 	upgradekeeper "cosmossdk.io/x/upgrade/keeper"
 	upgradetypes "cosmossdk.io/x/upgrade/types"
-	"github.com/cosmos/cosmos-sdk/codec/address"
-	"github.com/cosmos/cosmos-sdk/std"
-	"github.com/cosmos/gogoproto/proto"
 
 	icq "github.com/cosmos/ibc-apps/modules/async-icq/v8"
 	icqkeeper "github.com/cosmos/ibc-apps/modules/async-icq/v8/keeper"
@@ -47,11 +44,13 @@ import (
 	"github.com/cosmos/cosmos-sdk/client/grpc/cmtservice"
 	nodeservice "github.com/cosmos/cosmos-sdk/client/grpc/node"
 	"github.com/cosmos/cosmos-sdk/codec"
+	"github.com/cosmos/cosmos-sdk/codec/address"
 	"github.com/cosmos/cosmos-sdk/codec/types"
 	"github.com/cosmos/cosmos-sdk/runtime"
 	"github.com/cosmos/cosmos-sdk/server/api"
 	serverconfig "github.com/cosmos/cosmos-sdk/server/config"
 	servertypes "github.com/cosmos/cosmos-sdk/server/types"
+	"github.com/cosmos/cosmos-sdk/std"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/module"
 	sigtypes "github.com/cosmos/cosmos-sdk/types/tx/signing"
@@ -107,11 +106,12 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/staking"
 	stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
+	"github.com/cosmos/gogoproto/proto"
+
 	"github.com/cosmos/ibc-go/modules/capability"
 	capabilitykeeper "github.com/cosmos/ibc-go/modules/capability/keeper"
 	capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
 	ica "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts"
-
 	icahost "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host"
 	icahostkeeper "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/keeper"
 	icahosttypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types"
@@ -248,7 +248,7 @@ type App struct {
 
 	// keepers
 	AccountKeeper         authkeeper.AccountKeeper
-	BankKeeper            bankkeeper.Keeper
+	BankKeeper            bankkeeper.BaseKeeper
 	CapabilityKeeper      *capabilitykeeper.Keeper
 	StakingKeeper         *stakingkeeper.Keeper
 	SlashingKeeper        slashingkeeper.Keeper

app/app_test.go

@@ -11,7 +11,6 @@ import (
 
 	"cosmossdk.io/log"
 	sdkmath "cosmossdk.io/math"
-	sdksim "cosmossdk.io/simapp"
 
 	dbm "github.com/cosmos/cosmos-db"
 	sdktypes "github.com/cosmos/cosmos-sdk/codec/types"
@@ -159,7 +158,7 @@ func TestExportAppStateAndValidators(t *testing.T) {
 	exported, err := app.ExportAppStateAndValidators(false, nil, nil)
 	require.NoError(t, err, "ExportAppStateAndValidators")
 
-	var genState sdksim.GenesisState
+	var genState map[string]json.RawMessage
 	err = json.Unmarshal(exported.AppState, &genState)
 	require.NoError(t, err, "unmarshalling exported app state")
 

app/params/proto.go

@@ -1,10 +1,8 @@
-//go:build !test_amino
-// +build !test_amino
-
 package params
 
 import (
 	"cosmossdk.io/x/tx/signing"
+
 	"github.com/cosmos/cosmos-sdk/codec"
 	amino "github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/codec/address"
@@ -19,8 +17,7 @@ import (
 // App user shouldn't create new codecs - use the app.AppCodec instead.
 //
 // TODO[1760]: Update all the simulation stuff that uses this to instead get what's needed from the SimState.
-// That will involve passing the SimSate into many places where we used to provide a codec.
-// Then delete this file and amino.go, and clean up stuff in the Makefile that uses the test_amino tag.
+// That will involve passing the SimSate into many places where we used to provide a codec. Then, delete this file.
 //
 // Deprecated: Either get this from the app (app.GetEncodingConfig()) or use MakeTestEncodingConfig (from the app package),
 // or get what's needed from the SimSate.

app/sim_test.go

@@ -19,7 +19,6 @@ import (
 	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
 
 	"cosmossdk.io/log"
-	sdksim "cosmossdk.io/simapp"
 	"cosmossdk.io/store"
 	storetypes "cosmossdk.io/store/types"
 	evidencetypes "cosmossdk.io/x/evidence/types"
@@ -71,7 +70,7 @@ type StoreKeysPrefixes struct {
 	Prefixes [][]byte
 }
 
-// ProvAppStateFn wraps the sdksim.AppStateFn and sets the ICA GenesisState if isn't yet defined in the appState.
+// ProvAppStateFn wraps the simtypes.AppStateFn and sets the ICA GenesisState if isn't yet defined in the appState.
 func ProvAppStateFn(cdc codec.JSONCodec, simManager *module.SimulationManager, genesisState map[string]json.RawMessage) simtypes.AppStateFn {
 	return func(r *rand.Rand, accs []simtypes.Account, config simtypes.Config) (json.RawMessage, []simtypes.Account, string, time.Time) {
 		appState, simAccs, chainID, genesisTimestamp := simtestutil.AppStateFn(cdc, simManager, genesisState)(r, accs, config)
@@ -201,7 +200,7 @@ func TestSimple(t *testing.T) {
 // /usr/local/go/bin/go test -benchmem -run=^$ github.com/provenance-io/provenance -bench ^BenchmarkFullAppSimulation$ -Commit=true -cpuprofile cpu.out
 func TestAppImportExport(t *testing.T) {
 	// uncomment to run in ide without flags.
-	//sdksim.FlagEnabledValue = true
+	//simcli.FlagEnabledValue = true
 
 	config, db, dir, logger, skip, err := setupSimulation("leveldb-app-sim", "Simulation")
 	if skip {
@@ -257,7 +256,7 @@ func TestAppImportExport(t *testing.T) {
 
 	newApp := New(log.NewNopLogger(), newDB, nil, true, map[int64]bool{}, home, simcli.FlagPeriodValue, simtestutil.EmptyAppOptions{}, fauxMerkleModeOpt)
 
-	var genesisState sdksim.GenesisState
+	var genesisState map[string]json.RawMessage
 	err = json.Unmarshal(exported.AppState, &genesisState)
 	require.NoError(t, err)
 
@@ -402,9 +401,9 @@ func TestAppSimulationAfterImport(t *testing.T) {
 // and doesn't depend on the application.
 func TestAppStateDeterminism(t *testing.T) {
 	// uncomment these to run in ide without flags.
-	//sdksim.FlagEnabledValue = true
-	//sdksim.FlagBlockSizeValue = 100
-	//sdksim.FlagNumBlocksValue = 50
+	//simcli.FlagEnabledValue = true
+	//simcli.FlagBlockSizeValue = 100
+	//simcli.FlagNumBlocksValue = 50
 
 	if !simcli.FlagEnabledValue {
 		t.Skip("skipping application simulation")

app/upgrades.go

@@ -6,9 +6,11 @@ import (
 
 	storetypes "cosmossdk.io/store/types"
 	upgradetypes "cosmossdk.io/x/upgrade/types"
+
 	"github.com/cosmos/cosmos-sdk/baseapp"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/module"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	crisistypes "github.com/cosmos/cosmos-sdk/x/crisis/types"
 	paramstypes "github.com/cosmos/cosmos-sdk/x/params/types"
 	"github.com/cosmos/ibc-go/v8/modules/core/exported"
@@ -49,7 +51,7 @@ var upgrades = map[string]appUpgrade{
 		Handler: func(ctx sdk.Context, app *App, vm module.VersionMap) (module.VersionMap, error) {
 			var err error
 
-			if err := pruneIBCExpiredConsensusStates(ctx, app); err != nil {
+			if err = pruneIBCExpiredConsensusStates(ctx, app); err != nil {
 				return nil, err
 			}
 
@@ -58,6 +60,11 @@ var upgrades = map[string]appUpgrade{
 				return nil, err
 			}
 
+			err = migrateBankParams(ctx, app)
+			if err != nil {
+				return nil, err
+			}
+
 			migrateAttributeParams(ctx, app)
 			migrateMarkerParams(ctx, app)
 			migrateMetadataOSLocatorParams(ctx, app)
@@ -83,7 +90,7 @@ var upgrades = map[string]appUpgrade{
 		Handler: func(ctx sdk.Context, app *App, vm module.VersionMap) (module.VersionMap, error) {
 			var err error
 
-			if err := pruneIBCExpiredConsensusStates(ctx, app); err != nil {
+			if err = pruneIBCExpiredConsensusStates(ctx, app); err != nil {
 				return nil, err
 			}
 
@@ -92,6 +99,11 @@ var upgrades = map[string]appUpgrade{
 				return nil, err
 			}
 
+			err = migrateBankParams(ctx, app)
+			if err != nil {
+				return nil, err
+			}
+
 			migrateAttributeParams(ctx, app)
 			migrateMarkerParams(ctx, app)
 			migrateMetadataOSLocatorParams(ctx, app)
@@ -262,7 +274,7 @@ func pruneIBCExpiredConsensusStates(ctx sdk.Context, app *App) error {
 	ctx.Logger().Info("Pruning expired consensus states for IBC.")
 	_, err := ibctmmigrations.PruneExpiredConsensusStates(ctx, app.appCodec, app.IBCKeeper.ClientKeeper)
 	if err != nil {
-		ctx.Logger().Error(fmt.Sprintf("unable to prune expired consensus states, error: %s.", err))
+		ctx.Logger().Error(fmt.Sprintf("Unable to prune expired consensus states, error: %s.", err))
 		return err
 	}
 	ctx.Logger().Info("Done pruning expired consensus states for IBC.")
@@ -287,13 +299,34 @@ func migrateBaseappParams(ctx sdk.Context, app *App) error {
 	legacyBaseAppSubspace := app.ParamsKeeper.Subspace(baseapp.Paramspace).WithKeyTable(paramstypes.ConsensusParamsKeyTable())
 	err := baseapp.MigrateParams(ctx, legacyBaseAppSubspace, app.ConsensusParamsKeeper.ParamsStore)
 	if err != nil {
-		ctx.Logger().Error(fmt.Sprintf("unable to migrate legacy params to ConsensusParamsKeeper, error: %s.", err))
+		ctx.Logger().Error(fmt.Sprintf("Unable to migrate legacy params to ConsensusParamsKeeper, error: %s.", err))
 		return err
 	}
 	ctx.Logger().Info("Done migrating legacy params.")
 	return nil
 }
 
+// migrateBankParams migrates the bank params from the params module to the bank module's state.
+// The SDK has this as part of their bank v4 migration, but we're already on v4, so that one
+// won't run on its own. This is the only part of that migration that we still need to have
+// done, and this brings us in-line with format the bank state on v4.
+// TODO: delete with the umber handlers.
+func migrateBankParams(ctx sdk.Context, app *App) (err error) {
+	ctx.Logger().Info("Migrating bank params.")
+	defer func() {
+		if err != nil {
+			ctx.Logger().Error(fmt.Sprintf("Unable to migrate bank params, error: %s.", err))
+		}
+		ctx.Logger().Info("Done migrating bank params.")
+	}()
+
+	bankParamsSpace, ok := app.ParamsKeeper.GetSubspace(banktypes.ModuleName)
+	if !ok {
+		return fmt.Errorf("params subspace not found: %q", banktypes.ModuleName)
+	}
+	return app.BankKeeper.MigrateParamsProv(ctx, bankParamsSpace)
+}
+
 // migrateAttributeParams migrates to new Attribute Params store
 // TODO: Remove with the umber handlers.
 func migrateAttributeParams(ctx sdk.Context, app *App) {

app/upgrades_test.go

@@ -379,6 +379,8 @@ func (s *UpgradeTestSuite) TestUmberRC1() {
 		"INF Done pruning expired consensus states for IBC.",
 		"INF Migrating legacy params.",
 		"INF Done migrating legacy params.",
+		"INF Migrating bank params.",
+		"INF Done migrating bank params.",
 		"INF Migrating attribute params.",
 		"INF Done migrating attribute params.",
 		"INF Migrating marker params.",
@@ -402,6 +404,8 @@ func (s *UpgradeTestSuite) TestUmber() {
 		"INF Done pruning expired consensus states for IBC.",
 		"INF Migrating legacy params.",
 		"INF Done migrating legacy params.",
+		"INF Migrating bank params.",
+		"INF Done migrating bank params.",
 		"INF Migrating attribute params.",
 		"INF Done migrating attribute params.",
 		"INF Migrating marker params.",