Skip to content

Commit

Permalink
small fixes; took into account comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@insumity
insumity committed Sep 5, 2024
1 parent 3476a45 commit 3bccedb
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 91 deletions.
133 changes: 54 additions & 79 deletions app/upgrades/v20/upgrades.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ package v20
import (
"context"
"fmt"
"github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
"time"

authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
providerkeeper "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper"
providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types"

Expand All @@ -16,7 +14,8 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
govtypesv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"

"github.com/cosmos/gaia/v20/app/keepers"
Expand Down Expand Up @@ -145,8 +144,8 @@ func InitializeLastProviderConsensusValidatorSet(

// MigrateICSProposals migrates ICS legacy proposals
func MigrateICSProposals(ctx sdk.Context, msgServer providertypes.MsgServer, providerKeeper providerkeeper.Keeper, govKeeper govkeeper.Keeper) error {
proposals := []govtypes.Proposal{}
err := govKeeper.Proposals.Walk(ctx, nil, func(key uint64, proposal govtypes.Proposal) (stop bool, err error) {
proposals := []govtypesv1.Proposal{}
err := govKeeper.Proposals.Walk(ctx, nil, func(key uint64, proposal govtypesv1.Proposal) (stop bool, err error) {
proposals = append(proposals, proposal)
return false, nil // go through the entire collection
})
Expand All @@ -167,7 +166,7 @@ func MigrateICSProposals(ctx sdk.Context, msgServer providertypes.MsgServer, pro
return nil
}

func ConsumerAdditionProposalToMsgConsumerAddition(proposal providertypes.ConsumerAdditionProposal, authority string) providertypes.MsgConsumerAddition {
func ConsumerAdditionProposalToMsgConsumerAddition(proposal providertypes.ConsumerAdditionProposal) providertypes.MsgConsumerAddition {
return providertypes.MsgConsumerAddition{
ChainId: proposal.ChainId,
InitialHeight: proposal.InitialHeight,
Expand All @@ -186,21 +185,21 @@ func ConsumerAdditionProposalToMsgConsumerAddition(proposal providertypes.Consum
ValidatorSetCap: proposal.ValidatorSetCap,
Allowlist: proposal.Allowlist,
Denylist: proposal.Denylist,
Authority: authority,
Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
MinStake: proposal.MinStake,
AllowInactiveVals: proposal.AllowInactiveVals,
}
}

func ConsumerRemovalProposalToMsgConsumerRemoval(proposal providertypes.ConsumerRemovalProposal, authority string) providertypes.MsgConsumerRemoval {
func ConsumerRemovalProposalToMsgConsumerRemoval(proposal providertypes.ConsumerRemovalProposal) providertypes.MsgConsumerRemoval {
return providertypes.MsgConsumerRemoval{
ChainId: proposal.ChainId,
StopTime: proposal.StopTime,
Authority: authority,
Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
}
}

func ConsumerModificationProposalToMsgConsumerModification(proposal providertypes.ConsumerModificationProposal, authority string) providertypes.MsgConsumerModification {
func ConsumerModificationProposalToMsgConsumerModification(proposal providertypes.ConsumerModificationProposal) providertypes.MsgConsumerModification {
return providertypes.MsgConsumerModification{
Title: proposal.Title,
Description: proposal.Description,
Expand All @@ -210,7 +209,7 @@ func ConsumerModificationProposalToMsgConsumerModification(proposal providertype
ValidatorSetCap: proposal.ValidatorSetCap,
Allowlist: proposal.Allowlist,
Denylist: proposal.Denylist,
Authority: authority,
Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
MinStake: proposal.MinStake,
AllowInactiveVals: proposal.AllowInactiveVals,
}
Expand All @@ -229,11 +228,11 @@ func MigrateICSProposal(
msgServer providertypes.MsgServer,
providerKeeper providerkeeper.Keeper,
govKeeper govkeeper.Keeper,
proposal govtypes.Proposal) error {
proposal govtypesv1.Proposal) error {
// ignore proposals that were rejected or failed
if proposal.Status != govtypes.StatusDepositPeriod &&
proposal.Status != govtypes.StatusVotingPeriod &&
proposal.Status != govtypes.StatusPassed {
if proposal.Status != govtypesv1.StatusDepositPeriod &&
proposal.Status != govtypesv1.StatusVotingPeriod &&
proposal.Status != govtypesv1.StatusPassed {
return nil
}

Expand Down Expand Up @@ -278,17 +277,6 @@ func MigrateICSProposal(
if err != nil {
return err
}
case *providertypes.MsgChangeRewardDenoms:
err := MigrateMsgChangeRewardDenoms(
ctx,
govKeeper,
proposal.Id,
*msg,
index,
)
if err != nil {
return err
}
}
}

Expand All @@ -300,12 +288,12 @@ func MigrateICSLegacyProposal(
msgServer providertypes.MsgServer,
providerKeeper providerkeeper.Keeper,
govKeeper govkeeper.Keeper,
proposal govtypes.Proposal,
proposal govtypesv1.Proposal,
) error {
// ignore proposals that were rejected or failed
if proposal.Status != govtypes.StatusDepositPeriod &&
proposal.Status != govtypes.StatusVotingPeriod &&
proposal.Status != govtypes.StatusPassed {
if proposal.Status != govtypesv1.StatusDepositPeriod &&
proposal.Status != govtypesv1.StatusVotingPeriod &&
proposal.Status != govtypesv1.StatusPassed {
return nil
}

Expand All @@ -318,11 +306,11 @@ func MigrateICSLegacyProposal(
msg := messages[0]

// ignore non-legacy proposals
sdkLegacyMsg, isLegacyProposal := msg.GetCachedValue().(*govtypes.MsgExecLegacyContent)
sdkLegacyMsg, isLegacyProposal := msg.GetCachedValue().(*govtypesv1.MsgExecLegacyContent)
if !isLegacyProposal {
return nil
}
content, err := govtypes.LegacyContentFromMessage(sdkLegacyMsg)
content, err := govtypesv1.LegacyContentFromMessage(sdkLegacyMsg)
if err != nil {
return err
}
Expand All @@ -334,7 +322,7 @@ func MigrateICSLegacyProposal(
providerKeeper,
govKeeper,
proposal.Id,
ConsumerAdditionProposalToMsgConsumerAddition(*msg, "authority"),
ConsumerAdditionProposalToMsgConsumerAddition(*msg),
0)

case *providertypes.ConsumerRemovalProposal:
Expand All @@ -344,7 +332,7 @@ func MigrateICSLegacyProposal(
providerKeeper,
govKeeper,
proposal.Id,
ConsumerRemovalProposalToMsgConsumerRemoval(*msg, "authority"),
ConsumerRemovalProposalToMsgConsumerRemoval(*msg),
0,
)

Expand All @@ -354,7 +342,7 @@ func MigrateICSLegacyProposal(
providerKeeper,
govKeeper,
proposal.Id,
ConsumerModificationProposalToMsgConsumerModification(*msg, "authority"),
ConsumerModificationProposalToMsgConsumerModification(*msg),
0,
)

Expand Down Expand Up @@ -384,8 +372,8 @@ func MigrateMsgConsumerAddition(
if err != nil {
return err
}
if proposal.Status == govtypes.StatusPassed {
// proposal that passed
if proposal.Status == govtypesv1.StatusPassed {
// MsgConsumerAddition that passed
for _, consumerId := range providerKeeper.GetAllActiveConsumerIds(ctx) {

Check failure on line 377 in app/upgrades/v20/upgrades.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

var-naming: range var consumerId should be consumerID (revive)

Check failure on line 377 in app/upgrades/v20/upgrades.go

View workflow job for this annotation

GitHub Actions / Analyze 

var-naming: range var consumerId should be consumerID (revive)
chainId, err := providerKeeper.GetConsumerChainId(ctx, consumerId)

Check failure on line 378 in app/upgrades/v20/upgrades.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

var-naming: var chainId should be chainID (revive)

Check failure on line 378 in app/upgrades/v20/upgrades.go

View workflow job for this annotation

GitHub Actions / Analyze 

var-naming: var chainId should be chainID (revive)
if err != nil {
Expand All @@ -409,11 +397,13 @@ func MigrateMsgConsumerAddition(
// chain to be launched at msg.SpawnTime.

// create a new consumer chain with all the parameters
metadata := CreateConsumerMetadata(fmt.Sprintf("Chain with chain id %s", msg.ChainId), "TBA")
metadata := providertypes.ConsumerMetadata{
Name: msg.ChainId,
Description: "TBA",
Metadata: "TBA",
}

initParams, err := CreateConsumerInitializationParameters(msg.InitialHeight, msg.GenesisHash, msg.BinaryHash,
msg.SpawnTime, msg.UnbondingPeriod, msg.CcvTimeoutPeriod, msg.TransferTimeoutPeriod, msg.ConsumerRedistributionFraction,
msg.BlocksPerDistributionTransmission, msg.HistoricalEntries, msg.DistributionTransmissionChannel)
initParams, err := CreateConsumerInitializationParameters(msg)
if err != nil {
// invalid init params -- ignore proposal
ctx.Logger().Error(
Expand Down Expand Up @@ -473,11 +463,13 @@ func MigrateMsgConsumerAddition(
// Otherwise, create a new consumer chain (MsgCreateConsumer), and
// replace the proposal's content with a MsgUpdateConsumer

metadata := CreateConsumerMetadata(fmt.Sprintf("Chain with chain id %s", msg.ChainId), "TBA")
metadata := providertypes.ConsumerMetadata{
Name: msg.ChainId,
Description: "TBA",
Metadata: "TBA",
}

initParams, err := CreateConsumerInitializationParameters(msg.InitialHeight, msg.GenesisHash, msg.BinaryHash,
msg.SpawnTime, msg.UnbondingPeriod, msg.CcvTimeoutPeriod, msg.TransferTimeoutPeriod, msg.ConsumerRedistributionFraction,
msg.BlocksPerDistributionTransmission, msg.HistoricalEntries, msg.DistributionTransmissionChannel)
initParams, err := CreateConsumerInitializationParameters(msg)
if err != nil {
// invalid init params -- delete proposal
if err := govKeeper.DeleteProposal(ctx, proposal.Id); err != nil {
Expand Down Expand Up @@ -553,36 +545,19 @@ func MigrateMsgConsumerAddition(
return nil
}

func CreateConsumerMetadata(title string, description string) providertypes.ConsumerMetadata {
metadata := providertypes.ConsumerMetadata{
Name: title,
Description: description,
Metadata: "TBA",
}
err := providertypes.ValidateConsumerMetadata(metadata)
if err != nil {
metadata.Name = providertypes.TruncateString(metadata.Name, providertypes.MaxNameLength)
metadata.Description = providertypes.TruncateString(metadata.Description, providertypes.MaxDescriptionLength)
}
return metadata
}

func CreateConsumerInitializationParameters(
initialHeight types.Height, genesisHash []byte, binaryHash []byte, spawnTime time.Time, unbondingPeriod time.Duration,
ccvTimeoutPeriod time.Duration, transferTimeoutPeriod time.Duration, consumerRedistributionFraction string,
blocksPerDistributionTransmission int64, historicalEntries int64, distributionTransmissionChannel string) (providertypes.ConsumerInitializationParameters, error) {
func CreateConsumerInitializationParameters(msgConsumerAddition providertypes.MsgConsumerAddition) (providertypes.ConsumerInitializationParameters, error) {
initParams := providertypes.ConsumerInitializationParameters{
InitialHeight: initialHeight,
GenesisHash: genesisHash,
BinaryHash: binaryHash,
SpawnTime: spawnTime,
UnbondingPeriod: unbondingPeriod,
CcvTimeoutPeriod: ccvTimeoutPeriod,
TransferTimeoutPeriod: transferTimeoutPeriod,
ConsumerRedistributionFraction: consumerRedistributionFraction,
BlocksPerDistributionTransmission: blocksPerDistributionTransmission,
HistoricalEntries: historicalEntries,
DistributionTransmissionChannel: distributionTransmissionChannel,
InitialHeight: msgConsumerAddition.InitialHeight,
GenesisHash: msgConsumerAddition.GenesisHash,
BinaryHash: msgConsumerAddition.BinaryHash,
SpawnTime: msgConsumerAddition.SpawnTime,
UnbondingPeriod: msgConsumerAddition.UnbondingPeriod,
CcvTimeoutPeriod: msgConsumerAddition.CcvTimeoutPeriod,
TransferTimeoutPeriod: msgConsumerAddition.TransferTimeoutPeriod,
ConsumerRedistributionFraction: msgConsumerAddition.ConsumerRedistributionFraction,
BlocksPerDistributionTransmission: msgConsumerAddition.BlocksPerDistributionTransmission,
HistoricalEntries: msgConsumerAddition.HistoricalEntries,
DistributionTransmissionChannel: msgConsumerAddition.DistributionTransmissionChannel,
}
err := providertypes.ValidateInitializationParameters(initParams)
return initParams, err
Expand Down Expand Up @@ -638,7 +613,7 @@ func MigrateMsgConsumerRemoval(
proposal.Id, msg.ChainId,
),
)
if proposal.Status != govtypes.StatusPassed {
if proposal.Status != govtypesv1.StatusPassed {
// if the proposal didn't pass yet, then just remove it
if err := govKeeper.DeleteProposal(ctx, proposal.Id); err != nil {
return err
Expand All @@ -658,7 +633,7 @@ func MigrateMsgConsumerRemoval(
Signer: govKeeper.GetAuthority(),
}

if proposal.Status == govtypes.StatusPassed {
if proposal.Status == govtypesv1.StatusPassed {
// ConsumerRemovalProposal that passed -- it was added to the
// list of pending consumer removal proposals, which was deleted during
// the migration of the provider module
Expand Down Expand Up @@ -713,7 +688,7 @@ func MigrateMsgConsumerModification(
if err != nil {
return err
}
if proposal.Status == govtypes.StatusPassed {
if proposal.Status == govtypesv1.StatusPassed {
// proposal that passed -- it was already handled in
// a previous block since these proposals are handled immediately
ctx.Logger().Info(
Expand Down Expand Up @@ -802,7 +777,7 @@ func MigrateMsgChangeRewardDenoms(
if err != nil {
return err
}
if proposal.Status == govtypes.StatusPassed {
if proposal.Status == govtypesv1.StatusPassed {
// proposal that passed -- it was already handled in
// a previous block since these proposals are handled immediately
ctx.Logger().Info(
Expand Down
18 changes: 6 additions & 12 deletions app/upgrades/v20/upgrades_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package v20_test

import (
"encoding/base64"
"fmt"
tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
Expand Down Expand Up @@ -113,7 +114,8 @@ func TestMigrateMsgConsumerAdditionWithNotPassedProposalAndValidParams(t *testin
require.Equal(t, uint64(1), consumerId)
consumerMetadata, err := providerKeeper.GetConsumerMetadata(ctx, "0")
require.NoError(t, err)
require.Contains(t, "Chain with chain id a ChainId", consumerMetadata.Name)
fmt.Println(consumerMetadata)
require.Equal(t, msgConsumerAddition.ChainId, consumerMetadata.Name)

proposal, err = govKeeper.Proposals.Get(ctx, 0)
require.NoError(t, err)
Expand All @@ -122,11 +124,7 @@ func TestMigrateMsgConsumerAdditionWithNotPassedProposalAndValidParams(t *testin
require.Equal(t, messages[0].Value, proposal.Messages[0].Value)

// verify that the proposal's second message now contains a `MsgUpdateConsumer` message
initParams, err := v20.CreateConsumerInitializationParameters(
msgConsumerAddition.InitialHeight, msgConsumerAddition.GenesisHash, msgConsumerAddition.BinaryHash,
msgConsumerAddition.SpawnTime, msgConsumerAddition.UnbondingPeriod, msgConsumerAddition.CcvTimeoutPeriod, msgConsumerAddition.TransferTimeoutPeriod,
msgConsumerAddition.ConsumerRedistributionFraction, msgConsumerAddition.BlocksPerDistributionTransmission, msgConsumerAddition.HistoricalEntries,
msgConsumerAddition.DistributionTransmissionChannel)
initParams, err := v20.CreateConsumerInitializationParameters(msgConsumerAddition)
require.NoError(t, err)

powerShapingParams, err := v20.CreatePowerShapingParameters(msgConsumerAddition.Top_N, msgConsumerAddition.ValidatorsPowerCap,
Expand Down Expand Up @@ -177,7 +175,7 @@ func TestMigrateMsgConsumerAdditionWithPassedProposal(t *testing.T) {
require.Equal(t, uint64(1), consumerId)
consumerMetadata, err := providerKeeper.GetConsumerMetadata(ctx, "0")
require.NoError(t, err)
require.Contains(t, "Chain with chain id a ChainId", consumerMetadata.Name)
require.Equal(t, msgConsumerAddition.ChainId, consumerMetadata.Name)

proposal, err = govKeeper.Proposals.Get(ctx, 0)
require.NoError(t, err)
Expand All @@ -186,11 +184,7 @@ func TestMigrateMsgConsumerAdditionWithPassedProposal(t *testing.T) {
require.Equal(t, messages[0].Value, proposal.Messages[0].Value)

// verify that the proposal's second message now contains a `MsgUpdateConsumer` message
initParams, err := v20.CreateConsumerInitializationParameters(
msgConsumerAddition.InitialHeight, msgConsumerAddition.GenesisHash, msgConsumerAddition.BinaryHash,
msgConsumerAddition.SpawnTime, msgConsumerAddition.UnbondingPeriod, msgConsumerAddition.CcvTimeoutPeriod, msgConsumerAddition.TransferTimeoutPeriod,
msgConsumerAddition.ConsumerRedistributionFraction, msgConsumerAddition.BlocksPerDistributionTransmission, msgConsumerAddition.HistoricalEntries,
msgConsumerAddition.DistributionTransmissionChannel)
initParams, err := v20.CreateConsumerInitializationParameters(msgConsumerAddition)
require.NoError(t, err)

powerShapingParams, err := v20.CreatePowerShapingParameters(msgConsumerAddition.Top_N, msgConsumerAddition.ValidatorsPowerCap,
Expand Down

0 comments on commit 3bccedb

Please sign in to comment.