Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 12 commits into from
Nov 7, 2023
8 changes: 4 additions & 4 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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,
Expand All @@ -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)

Expand Down
10 changes: 2 additions & 8 deletions app/upgrade/v3/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
6 changes: 2 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
4 changes: 2 additions & 2 deletions integration-tests/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 16 additions & 2 deletions integration-tests/modules/bank_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/modules/wasm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1751,7 +1751,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()

Expand Down
2 changes: 1 addition & 1 deletion pkg/client/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion testutil/integration/chain_ibc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
12 changes: 0 additions & 12 deletions tools/tools.go

This file was deleted.

6 changes: 0 additions & 6 deletions x/asset/ft/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}

Expand Down
6 changes: 0 additions & 6 deletions x/asset/nft/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}

Expand Down
6 changes: 0 additions & 6 deletions x/customparams/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}

Expand Down
6 changes: 0 additions & 6 deletions x/delay/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}

Expand Down
2 changes: 1 addition & 1 deletion x/deterministicgas/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 0 additions & 6 deletions x/feemodel/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}

Expand Down
2 changes: 1 addition & 1 deletion x/gov/proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 2 additions & 7 deletions x/nft/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
}
Expand Down
7 changes: 0 additions & 7 deletions x/wbank/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
} */
}
2 changes: 1 addition & 1 deletion x/wnft/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading