Close some TODO/FIXMEs & add more context to remaining ones (batch 1) (…

…#702)

Some TODOs which were not needed anymore were deleted. 
And also I added v4 to most of left because they could not be fixed
right now and will be resolved in v4+ release.
Other TODO/FIXME will be fixed in standalone PRs

app/app.go

@@ -179,7 +179,7 @@ var (
 		distr.AppModuleBasic{},
 		gov.NewAppModuleBasic(
 			[]govclient.ProposalHandler{
-				// TODO: Remove once IBC migrates to the new mechanism
+				// TODO(v4): Remove once IBC upgrades to the new param management mechanism. Check ibc-go/modules/core/02-client/types/params.go
 				paramsclient.ProposalHandler,
 				ibcclientclient.UpdateClientProposalHandler,
 				ibcclientclient.UpgradeProposalHandler,
@@ -531,7 +531,7 @@ func New(
 	// See: https://docs.cosmos.network/main/modules/gov#proposal-messages
 	govRouter := govv1beta1.NewRouter()
 	govRouter.AddRoute(govtypes.RouterKey, govv1beta1.ProposalHandler).
-		// TODO: Remove once IBC upgrades to the new mechanism
+		// TODO(v4): Remove once IBC upgrades to the new param management mechanism. Check ibc-go/modules/core/02-client/types/params.go
 		AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)).
 		AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper))
 
@@ -627,7 +627,7 @@ func New(
 		app.BankKeeper,
 		app.StakingKeeper,
 		distrkeeper.NewQuerier(app.DistrKeeper),
-		app.IBCKeeper.ChannelKeeper, // FIXME(v47-ibc) add the fee wrapper
+		app.IBCKeeper.ChannelKeeper,
 		app.IBCKeeper.ChannelKeeper,
 		&app.IBCKeeper.PortKeeper,
 		app.ScopedWASMKeeper,
@@ -645,7 +645,7 @@ func New(
 	// FIXME(v47-legacy): remove once we finish with full migration
 	govRouter.AddRoute(wasmtypes.RouterKey, wasmkeeper.NewWasmProposalHandler(app.WasmKeeper, wasmtypes.EnableAllProposals)) //nolint:staticcheck // we need to keep backward compatibility
 
-	// FIXME(v47-legacy): remove once we finish with full migration
+	// FIXME(v4): drop once we drop gov v1beta1 compatibility.
 	// Set legacy router for backwards compatibility with gov v1beta1
 	app.GovKeeper.SetLegacyRouter(govRouter)
 

app/upgrade/v3/upgrade.go

@@ -102,9 +102,6 @@ func New(
 
 				// wasm:
 				wasmtypes.ModuleName: wasmtypes.ParamKeyTable(), //nolint:staticcheck
-
-				// coreum:
-				// TODO(migration-away-from-x/params): Add migration of params for Coreum modules. Skipping them for now.
 			}
 
 			// https://github.com/cosmos/cosmos-sdk/pull/12363/files
@@ -113,7 +110,6 @@ func New(
 				subspace := subspace
 
 				if lo.Contains([]string{
-					// TODO(migration-away-from-x/params): Add migration of params for Coreum modules. Skipping them for now.
 					feemodeltypes.ModuleName,
 					assetfttypes.ModuleName,
 					assetnfttypes.ModuleName,
@@ -170,8 +166,7 @@ func New(
 			params.AllowedClients = append(params.AllowedClients, ibccoreexported.Localhost)
 			ibcClientKeeper.SetParams(ctx, params)
 
-			// TODO(new-gov-params): Discuss new values for the following params with the team and set here & inside genesis.v3.json.
-			// min_initial_deposit_ratio, burn_vote_quorum, burn_proposal_deposit_prevote, burn_vote_veto
+			// Set values for new params: min_initial_deposit_ratio, burn_vote_quorum, burn_proposal_deposit_prevote, burn_vote_veto
 			govParams := govKeeper.GetParams(ctx)
 			govParams.MinInitialDepositRatio = sdk.NewDec(50).Quo(sdk.NewDec(100)).String()
 			govParams.BurnVoteQuorum = false
@@ -181,8 +176,7 @@ func New(
 				return nil, err
 			}
 
-			// TODO(new-staking-params): Discuss new values for the following params with the team and set here & inside genesis.v3.json.
-			// min_commission_rate
+			// Set value for new param min_commission_rate
 			stakingParams := stakingKeeper.GetParams(ctx)
 			stakingParams.MinCommissionRate = sdk.ZeroDec()
 			err = stakingKeeper.SetParams(ctx, stakingParams)

go.mod

@@ -6,10 +6,10 @@ replace (
 	// cosmos keyring
 	github.com/99designs/keyring => github.com/cosmos/keyring v1.2.0
 	// dgrijalva/jwt-go is deprecated and doesn't receive security updates.
-	// TODO: remove it: https://github.com/cosmos/cosmos-sdk/issues/13134
+	// TODO(v4): remove it: https://github.com/cosmos/cosmos-sdk/issues/13134
 	github.com/dgrijalva/jwt-go => github.com/golang-jwt/jwt/v4 v4.4.2
 	// Fix upstream GHSA-h395-qcrw-5vmq vulnerability.
-	// TODO Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409
+	// TODO(v4) Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409
 	github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.9.0
 	// https://github.com/cosmos/cosmos-sdk/issues/14949
 	// pin the version of goleveldb to v1.0.1-0.20210819022825-2ae1ddf74ef7 required by SDK v47 upgrade guide.
@@ -31,7 +31,6 @@ require (
 	github.com/golang/protobuf v1.5.3
 	github.com/gorilla/mux v1.8.0
 	github.com/grpc-ecosystem/grpc-gateway v1.16.0
-	github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.2
 	github.com/samber/lo v1.38.1
 	github.com/spf13/cast v1.5.1
 	github.com/spf13/cobra v1.7.0
@@ -99,7 +98,6 @@ require (
 	github.com/dvsekhvalnov/jose2go v1.5.0 // indirect
 	github.com/felixge/httpsnoop v1.0.2 // indirect
 	github.com/fsnotify/fsnotify v1.6.0 // indirect
-	github.com/ghodss/yaml v1.0.0 // indirect
 	github.com/go-kit/kit v0.12.0 // indirect
 	github.com/go-kit/log v0.2.1 // indirect
 	github.com/go-logfmt/logfmt v0.6.0 // indirect

go.sum

@@ -514,7 +514,6 @@ github.com/getkin/kin-openapi v0.53.0/go.mod h1:7Yn5whZr5kJi6t+kShccXS8ae1APpYTW
 github.com/getkin/kin-openapi v0.61.0/go.mod h1:7Yn5whZr5kJi6t+kShccXS8ae1APpYTW6yheSwk8Yi4=
 github.com/getsentry/sentry-go v0.23.0 h1:dn+QRCeJv4pPt9OjVXiMcGIBIefaTJPw/h0bZWO05nE=
 github.com/getsentry/sentry-go v0.23.0/go.mod h1:lc76E2QywIyW8WuBnwl8Lc4bkmQH4+w1gwTf25trprY=
-github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk=
 github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
 github.com/gin-contrib/sse v0.1.0 h1:Y/yl/+YNO8GZSjAhjMsSuLt29uWRFHdHYUb5lYOV9qE=
 github.com/gin-contrib/sse v0.1.0/go.mod h1:RHrZQHXnP2xjPF+u1gW/2HnVO7nvIa9PG3Gm+fLHvGI=
@@ -722,8 +721,6 @@ github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgf
 github.com/grpc-ecosystem/grpc-gateway v1.9.5/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY=
 github.com/grpc-ecosystem/grpc-gateway v1.16.0 h1:gmcG1KaJ57LophUzW0Hy8NmPhnMZb4M0+kPpLofRdBo=
 github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw=
-github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.2 h1:dygLcbEBA+t/P7ck6a8AkXv6juQ4cK0RHBoh32jxhHM=
-github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.2/go.mod h1:Ap9RLCIJVtgQg1/BBgVEfypOAySvvlcpcVQkSzJCH4Y=
 github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c h1:6rhixN/i8ZofjG1Y75iExal34USq5p+wiN1tpie8IrU=
 github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c/go.mod h1:NMPJylDgVpX0MLRlPy15sqSwOFv/U1GZ2m21JhFfek0=
 github.com/gtank/merlin v0.1.1-0.20191105220539-8318aed1a79f/go.mod h1:T86dnYJhcGOh5BjZFCJWTDeTK7XW8uE+E21Cy/bIQ+s=

integration-tests/go.mod

@@ -7,10 +7,10 @@ replace (
 	github.com/99designs/keyring => github.com/cosmos/keyring v1.2.0
 	github.com/CoreumFoundation/coreum/v3 => ../
 	// dgrijalva/jwt-go is deprecated and doesn't receive security updates.
-	// TODO: remove it: https://github.com/cosmos/cosmos-sdk/issues/13134
+	// TODO(v4): remove it: https://github.com/cosmos/cosmos-sdk/issues/13134
 	github.com/dgrijalva/jwt-go => github.com/golang-jwt/jwt/v4 v4.4.2
 	// Fix upstream GHSA-h395-qcrw-5vmq vulnerability.
-	// TODO Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409
+	// TODO(v4): Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409
 	github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.9.0
 	// https://github.com/cosmos/cosmos-sdk/issues/14949
 	// pin the version of goleveldb to v1.0.1-0.20210819022825-2ae1ddf74ef7 required by SDK v47 upgrade guide.

integration-tests/modules/bank_test.go

@@ -766,8 +766,6 @@ func TestTryBankMultiSendFromMultipleAccounts(t *testing.T) {
 	requireT.ErrorIs(err, banktypes.ErrMultipleSenders)
 }
 
-// FIXME (wojtek): add test verifying that transfer fails if sender is out of balance.
-
 // TestBankCoreSend checks that core is transferred correctly between wallets.
 func TestBankCoreSend(t *testing.T) {
 	t.Parallel()
@@ -822,6 +820,22 @@ func TestBankCoreSend(t *testing.T) {
 
 	assert.Equal(t, senderInitialAmount.Sub(amountToSend).String(), balancesSender.Balance.Amount.String())
 	assert.Equal(t, recipientInitialAmount.Add(amountToSend).String(), balancesRecipient.Balance.Amount.String())
+
+	// Try to send more than remaining balance
+	msg = &banktypes.MsgSend{
+		FromAddress: sender.String(),
+		ToAddress:   recipient.String(),
+		Amount:      sdk.NewCoins(*balancesSender.Balance), // sender can't send whole balance because funds for paying fees are required.
+	}
+
+	_, err = client.BroadcastTx(
+		ctx,
+		chain.ClientContext.WithFromAddress(sender),
+		chain.TxFactory().WithGas(chain.GasLimitByMsgs(msg)),
+		msg,
+	)
+	require.Error(t, err)
+	require.ErrorContains(t, err, "insufficient funds")
 }
 
 func assertBatchAccounts(

integration-tests/modules/wasm_test.go

@@ -1855,7 +1855,7 @@ func TestWASMNonFungibleTokenInContract(t *testing.T) {
 }
 
 // TestWASMBankSendContractWithMultipleFundsAttached tests sending multiple ft funds and core token to smart contract.
-// TODO: remove this test after this task is implemented. https://app.clickup.com/t/86857vqra
+// TODO(v4): remove this test after this task is implemented. https://app.clickup.com/t/86857vqra
 func TestWASMBankSendContractWithMultipleFundsAttached(t *testing.T) {
 	t.Parallel()
 
@@ -2057,8 +2057,15 @@ func TestWASMContractInstantiationIsRejectedIfAccountExists(t *testing.T) {
 func randStringWithLength(n int) string {
 	letterRunes := []rune("abcdefghijklmnopqrstuvwxyz")
 	b := make([]rune, n)
-	for i := range b {
-		b[i] = letterRunes[rand.Intn(len(letterRunes))]
+	for {
+		for i := range b {
+			b[i] = letterRunes[rand.Intn(len(letterRunes))]
+		}
+		// Make sure string is not one of reserved subunits/symbols and if it is regenerate it.
+		if assetfttypes.ValidateSubunit(string(b)) == nil && assetfttypes.ValidateSymbol(string(b)) == nil {
+			break
+		}
 	}
+
 	return string(b)
 }

pkg/client/tx.go

@@ -312,7 +312,7 @@ func AwaitNextBlocks(
 		if res.SdkBlock != nil {
 			currentHeight = res.SdkBlock.Header.Height
 		} else {
-			// TODO: Remove this in v4 version of cored. Now it is needed because we might still use it in integration tests together with v2 cored binary.
+			// TODO(v4): Remove this in v4 version of cored. Now it is needed because we might still use it in integration tests together with v2 cored binary.
 			currentHeight = res.Block.Header.Height //nolint:staticcheck // Yes, we know that this is deprecated
 		}
 

testutil/integration/chain_ibc.go

@@ -90,7 +90,7 @@ func (c ChainContext) ExecuteTimingOutIBCTransfer(
 	if latestBlockRes.SdkBlock != nil {
 		headerTime = latestBlockRes.GetSdkBlock().GetHeader().Time
 	} else {
-		// TODO: remove this "if condition" once all the connected chains have migrated to cosmos sdk v0.47.
+		// TODO(v4): remove this "if condition" once all the connected chains have migrated to cosmos sdk v0.47.
 		// Block is deprecated in favor of SdkBlock.
 		headerTime = latestBlockRes.GetBlock().GetHeader().Time
 	}

x/asset/ft/module.go

@@ -173,12 +173,6 @@ func (AppModule) ConsensusVersion() uint64 { return 3 }
 // GenerateGenesisState creates a randomized GenState of the asset ft module.
 func (AppModule) GenerateGenesisState(_ *module.SimulationState) {}
 
-// ProposalContents doesn't return any content functions for governance proposals.
-// FIXME(v47-legacy) try to remove/replace the usage.
-func (AppModule) ProposalContents(_ module.SimulationState) []simtypes.WeightedProposalContent { //nolint:staticcheck // we need to keep backward compatibility
-	return nil
-}
-
 // RegisterStoreDecoder registers a decoder for asset ft module's types.
 func (am AppModule) RegisterStoreDecoder(_ sdk.StoreDecoderRegistry) {}
 

x/asset/nft/module.go

@@ -175,12 +175,6 @@ func (AppModule) ConsensusVersion() uint64 { return 3 }
 // GenerateGenesisState creates a randomized GenState of the assetnft module.
 func (AppModule) GenerateGenesisState(_ *module.SimulationState) {}
 
-// ProposalContents doesn't return any content functions for governance proposals.
-// FIXME(v47-legacy) try to remove/replace the usage.
-func (AppModule) ProposalContents(_ module.SimulationState) []simtypes.WeightedProposalContent { //nolint:staticcheck // we need to keep backward compatibility
-	return nil
-}
-
 // RegisterStoreDecoder registers a decoder for assetnft module's types.
 func (am AppModule) RegisterStoreDecoder(_ sdk.StoreDecoderRegistry) {}
 

x/customparams/module.go

@@ -141,12 +141,6 @@ func (AppModule) GenerateGenesisState(simState *module.SimulationState) {
 	simState.GenState[types.ModuleName] = simState.Cdc.MustMarshalJSON(types.DefaultGenesisState())
 }
 
-// ProposalContents doesn't return any content functions for governance proposals.
-// FIXME(v47-legacy) try to remove/replace the usage.
-func (AppModule) ProposalContents(_ module.SimulationState) []simtypes.WeightedProposalContent { //nolint:staticcheck // we need to keep backward compatibility
-	return nil
-}
-
 // RegisterStoreDecoder registers a decoder for supply module's types.
 func (am AppModule) RegisterStoreDecoder(_ sdk.StoreDecoderRegistry) {}
 

x/delay/module.go

@@ -129,12 +129,6 @@ func (am AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.Valid
 // GenerateGenesisState creates a randomized GenState of the delay module.
 func (AppModule) GenerateGenesisState(simState *module.SimulationState) {}
 
-// ProposalContents doesn't return any content functions for governance proposals.
-// FIXME(v47-legacy) try to remove/replace the usage.
-func (AppModule) ProposalContents(_ module.SimulationState) []simtypes.WeightedProposalContent { //nolint:staticcheck // we need to keep backward compatibility
-	return nil
-}
-
 // RegisterStoreDecoder registers a decoder for supply module's types.
 func (am AppModule) RegisterStoreDecoder(_ sdk.StoreDecoderRegistry) {}
 

x/deterministicgas/config.go

@@ -79,7 +79,7 @@ func DefaultConfig() Config {
 		MsgToMsgURL(&assetfttypes.MsgGloballyFreeze{}):      constantGasFunc(5_000),
 		MsgToMsgURL(&assetfttypes.MsgGloballyUnfreeze{}):    constantGasFunc(5_000),
 		MsgToMsgURL(&assetfttypes.MsgSetWhitelistedLimit{}): constantGasFunc(9_000),
-		// TODO: Reestimate when next token upgrade is prepared
+		// TODO(v4): Once we add a new token upgrade MsgUpgradeTokenV2 we should remove this one and re-estimate gas.
 		MsgToMsgURL(&assetfttypes.MsgUpgradeTokenV1{}): constantGasFunc(25_000),
 
 		// asset/nft

x/feemodel/module.go

@@ -187,12 +187,6 @@ func (AppModule) GenerateGenesisState(simState *module.SimulationState) {
 	simState.GenState[types.ModuleName] = simState.Cdc.MustMarshalJSON(types.DefaultGenesisState())
 }
 
-// ProposalContents doesn't return any content functions for governance proposals.
-// FIXME(v47-legacy) try to remove/replace the usage.
-func (AppModule) ProposalContents(_ module.SimulationState) []simtypes.WeightedProposalContent { //nolint:staticcheck // we need to keep backward compatibility
-	return nil
-}
-
 // RegisterStoreDecoder registers a decoder for supply module's types.
 func (am AppModule) RegisterStoreDecoder(_ sdk.StoreDecoderRegistry) {}
 

x/gov/proposal_test.go

@@ -18,7 +18,7 @@ import (
 //go:linkname revProtoTypes github.com/cosmos/gogoproto/proto.revProtoTypes
 var revProtoTypes map[reflect.Type]string
 
-// FIXME(v47-legacy): check that we need it, probably we should update it to new way of gov.
+// TODO(v4): drop together with x/gov/types/v1beta1 support.
 func TestExpectedRegisteredProposals(t *testing.T) {
 	knownProposals := map[string]struct{}{
 		// proposals we have integration tests for

x/nft/module/module.go

@@ -91,6 +91,8 @@ func (AppModuleBasic) GetTxCmd() *cobra.Command {
 type AppModule struct {
 	AppModuleBasic
 	keeper keeper.Keeper
+	// TODO(v4): To be removed together with module.
+	//   This todo comes from cosmos-sdk code and will be remove together with nft module once we drop back-compatibility.
 	// TODO accountKeeper,bankKeeper will be replaced by query service
 	accountKeeper nft.AccountKeeper
 	bankKeeper    nft.BankKeeper
@@ -150,13 +152,6 @@ func (AppModuleBasic) RegisterRESTRoutes(_ client.Context, _ *mux.Router) {}
 func (AppModule) GenerateGenesisState(simState *module.SimulationState) {
 }
 
-// ProposalContents returns all the nft content functions used to
-// simulate governance proposals.
-// FIXME(v47-legacy) try to remove/replace the usage.
-func (am AppModule) ProposalContents(simState module.SimulationState) []simtypes.WeightedProposalContent { //nolint:staticcheck // we need to keep backward compatibility
-	return nil
-}
-
 // RegisterStoreDecoder registers a decoder for nft module's types.
 func (am AppModule) RegisterStoreDecoder(sdr sdk.StoreDecoderRegistry) {
 }

x/wbank/module.go

@@ -53,11 +53,4 @@ func (am AppModule) RegisterServices(cfg module.Configurator) {
 	if err := cfg.RegisterMigration(banktypes.ModuleName, 3, m.Migrate3to4); err != nil {
 		panic(fmt.Sprintf("failed to migrate x/bank from version 3 to 4: %v", err))
 	}
-
-	// FIXME(v47-module-config): remove or replace with corresponding component
-	// Route returns the message routing key for the bank module.
-	/* func (am AppModule) Route() sdk.Route {
-		// we need to pass the wrapped keeper to the handler to use it instead of the default
-		return sdk.NewRoute(banktypes.RouterKey, bank.NewHandler(am.keeper))
-	} */
 }

x/wnft/keeper/keeper.go

@@ -28,7 +28,7 @@ func NewWrappedNFTKeeper(originalKeeper nftkeeper.Keeper, provider types.NonFung
 
 // Send overwrites Send method of the original keeper.
 // Copied from https://github.com/cosmos/cosmos-sdk/blob/a1143138716b64bc4fa0aa53c0f0fa59eb675bb7/x/nft/keeper/msg_server.go#L14
-// FIXME(v47-nft-migration): once we update the nft to sdk nft, that methods must be updated as well.
+// On each update we need to make sure it is up-to-date with original cosmos version of nft.
 func (wk Wrapper) Send(goCtx context.Context, msg *nft.MsgSend) (*nft.MsgSendResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 	sender, err := sdk.AccAddressFromBech32(msg.Sender)