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.

Sign up for GitHub

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

E2E To verify legacy client upgrade proposal fails #4748

Merged
merged 9 commits into from
Sep 25, 2023
Merged

E2E To verify legacy client upgrade proposal fails #4748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5bedf95
chore: add test to fail legacy proposal
chatton Sep 21, 2023
c08af98
chore: adding assertion on failed gov proposal
chatton Sep 21, 2023
d61643f
Merge branch 'main' into cian/verify-client-upgrade-proposal-fails-af…
chatton Sep 21, 2023
0e43317
chore: run linter
chatton Sep 22, 2023
e4ae13f
chore: run linter
chatton Sep 22, 2023
5facf54
Merge branch 'main' into cian/verify-client-upgrade-proposal-fails-af…
chatton Sep 22, 2023
af82c18
Merge branch 'main' into cian/verify-client-upgrade-proposal-fails-af…
DimitrisJim Sep 22, 2023
65a7c3a
Merge branch 'main' into cian/verify-client-upgrade-proposal-fails-af…
chatton Sep 25, 2023
39746cd
Merge branch 'main' into cian/verify-client-upgrade-proposal-fails-af…
chatton Sep 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 36 additions & 6 deletions e2e/tests/core/02-client/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
test "github.com/strangelove-ventures/interchaintest/v8/testutil"
testifysuite "github.com/stretchr/testify/suite"

"cosmossdk.io/x/upgrade/types"
upgradetypes "cosmossdk.io/x/upgrade/types"

"github.com/cosmos/cosmos-sdk/client/grpc/cmtservice"
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
Expand Down Expand Up @@ -83,6 +83,7 @@ func (s *ClientTestSuite) TestScheduleIBCUpgrade_Succeeds() {
chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)

const planHeight = int64(75)
const legacyPlanHeight = planHeight * 2
var newChainID string

t.Run("execute proposal for MsgIBCSoftwareUpgrade", func(t *testing.T) {
Expand All @@ -105,14 +106,14 @@ func (s *ClientTestSuite) TestScheduleIBCUpgrade_Succeeds() {

scheduleUpgradeMsg, err := clienttypes.NewMsgIBCSoftwareUpgrade(
authority.String(),
types.Plan{
upgradetypes.Plan{
Name: "upgrade-client",
Height: planHeight,
},
upgradedClientState,
)
s.Require().NoError(err)
s.ExecuteGovV1Proposal(ctx, scheduleUpgradeMsg, chainA, chainAWallet)
s.ExecuteAndPassGovV1Proposal(ctx, scheduleUpgradeMsg, chainA, chainAWallet)
})

t.Run("check that IBC software upgrade has been scheduled successfully on chainA", func(t *testing.T) {
Expand All @@ -130,6 +131,35 @@ func (s *ClientTestSuite) TestScheduleIBCUpgrade_Succeeds() {
s.Require().Equal("upgrade-client", plan.Name)
s.Require().Equal(planHeight, plan.Height)
})

t.Run("ensure legacy proposal does not succeed", func(t *testing.T) {

authority, err := s.QueryModuleAccountAddress(ctx, govtypes.ModuleName, chainA)
s.Require().NoError(err)
s.Require().NotNil(authority)

clientState, err := s.QueryClientState(ctx, chainB, ibctesting.FirstClientID)
s.Require().NoError(err)

originalChainID := clientState.(*ibctm.ClientState).ChainId
revisionNumber := clienttypes.ParseChainID(originalChainID)
// increment revision number even with new chain ID to prevent loss of misbehaviour detection support
newChainID, err = clienttypes.SetRevisionNumber(originalChainID, revisionNumber+1)
s.Require().NoError(err)
s.Require().NotEqual(originalChainID, newChainID)

upgradedClientState := clientState.ZeroCustomFields().(*ibctm.ClientState)
upgradedClientState.ChainId = newChainID

legacyUpgradeProposal, err := clienttypes.NewUpgradeProposal(ibctesting.Title, ibctesting.Description, upgradetypes.Plan{
Name: "upgrade-client-legacy",
Height: legacyPlanHeight,
}, upgradedClientState)

s.Require().NoError(err)
txResp := s.ExecuteGovV1Beta1Proposal(ctx, chainA, chainAWallet, legacyUpgradeProposal)
s.AssertTxFailure(txResp, govtypes.ErrInvalidProposalContent)
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
})
}

// TestRecoverClient_Succeeds tests that a governance proposal to recover a client using a MsgRecoverClient is successful.
Expand Down Expand Up @@ -200,7 +230,7 @@ func (s *ClientTestSuite) TestRecoverClient_Succeeds() {
s.Require().NoError(err)
recoverClientMsg := clienttypes.NewMsgRecoverClient(authority.String(), subjectClientID, substituteClientID)
s.Require().NotNil(recoverClientMsg)
s.ExecuteGovV1Proposal(ctx, recoverClientMsg, chainA, chainAWallet)
s.ExecuteAndPassGovV1Proposal(ctx, recoverClientMsg, chainA, chainAWallet)
})

t.Run("check status of each client", func(t *testing.T) {
Expand Down Expand Up @@ -351,7 +381,7 @@ func (s *ClientTestSuite) TestAllowedClientsParam() {
s.Require().NotNil(authority)

msg := clienttypes.NewMsgUpdateParams(authority.String(), clienttypes.NewParams(allowedClient))
s.ExecuteGovV1Proposal(ctx, msg, chainA, chainAWallet)
s.ExecuteAndPassGovV1Proposal(ctx, msg, chainA, chainAWallet)
} else {
value, err := tmjson.Marshal([]string{allowedClient})
s.Require().NoError(err)
Expand All @@ -360,7 +390,7 @@ func (s *ClientTestSuite) TestAllowedClientsParam() {
}

proposal := paramsproposaltypes.NewParameterChangeProposal(ibctesting.Title, ibctesting.Description, changes)
s.ExecuteGovV1Beta1Proposal(ctx, chainA, chainAWallet, proposal)
s.ExecuteAndPassGovV1Beta1Proposal(ctx, chainA, chainAWallet, proposal)
}
})

Expand Down
4 changes: 2 additions & 2 deletions e2e/tests/core/03-connection/connection_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,14 @@ func (s *ConnectionTestSuite) TestMaxExpectedTimePerBlockParam() {
s.Require().NotNil(authority)

msg := connectiontypes.NewMsgUpdateParams(authority.String(), connectiontypes.NewParams(delay))
s.ExecuteGovV1Proposal(ctx, msg, chainA, chainAWallet)
s.ExecuteAndPassGovV1Proposal(ctx, msg, chainA, chainAWallet)
} else {
changes := []paramsproposaltypes.ParamChange{
paramsproposaltypes.NewParamChange(ibcexported.ModuleName, string(connectiontypes.KeyMaxExpectedTimePerBlock), fmt.Sprintf(`"%d"`, delay)),
}

proposal := paramsproposaltypes.NewParameterChangeProposal(ibctesting.Title, ibctesting.Description, changes)
s.ExecuteGovV1Beta1Proposal(ctx, chainA, chainAWallet, proposal)
s.ExecuteAndPassGovV1Beta1Proposal(ctx, chainA, chainAWallet, proposal)
}
})

Expand Down
4 changes: 2 additions & 2 deletions e2e/tests/interchain_accounts/gov_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (s *InterchainAccountsGovTestSuite) TestInterchainAccountsGovIntegration()
t.Run("execute proposal for MsgRegisterInterchainAccount", func(t *testing.T) {
version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, govModuleAddress.String(), version)
s.ExecuteGovV1Proposal(ctx, msgRegisterAccount, chainA, controllerAccount)
s.ExecuteAndPassGovV1Proposal(ctx, msgRegisterAccount, chainA, controllerAccount)
})

t.Run("start relayer", func(t *testing.T) {
Expand Down Expand Up @@ -102,7 +102,7 @@ func (s *InterchainAccountsGovTestSuite) TestInterchainAccountsGovIntegration()
}

msgSendTx := controllertypes.NewMsgSendTx(govModuleAddress.String(), ibctesting.FirstConnectionID, uint64(time.Hour.Nanoseconds()), packetData)
s.ExecuteGovV1Proposal(ctx, msgSendTx, chainA, controllerAccount)
s.ExecuteAndPassGovV1Proposal(ctx, msgSendTx, chainA, controllerAccount)
})

t.Run("verify tokens transferred", func(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions e2e/tests/interchain_accounts/params_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,14 @@ func (s *InterchainAccountsParamsTestSuite) TestControllerEnabledParam() {
Signer: authority.String(),
Params: controllertypes.NewParams(false),
}
s.ExecuteGovV1Proposal(ctx, &msg, chainA, controllerAccount)
s.ExecuteAndPassGovV1Proposal(ctx, &msg, chainA, controllerAccount)
} else {
changes := []paramsproposaltypes.ParamChange{
paramsproposaltypes.NewParamChange(controllertypes.StoreKey, string(controllertypes.KeyControllerEnabled), "false"),
}

proposal := paramsproposaltypes.NewParameterChangeProposal(ibctesting.Title, ibctesting.Description, changes)
s.ExecuteGovV1Beta1Proposal(ctx, chainA, controllerAccount, proposal)
s.ExecuteAndPassGovV1Beta1Proposal(ctx, chainA, controllerAccount, proposal)
}
})

Expand Down Expand Up @@ -131,14 +131,14 @@ func (s *InterchainAccountsParamsTestSuite) TestHostEnabledParam() {
Signer: authority.String(),
Params: hosttypes.NewParams(false, []string{hosttypes.AllowAllHostMsgs}),
}
s.ExecuteGovV1Proposal(ctx, &msg, chainB, chainBUser)
s.ExecuteAndPassGovV1Proposal(ctx, &msg, chainB, chainBUser)
} else {
changes := []paramsproposaltypes.ParamChange{
paramsproposaltypes.NewParamChange(hosttypes.StoreKey, string(hosttypes.KeyHostEnabled), "false"),
}

proposal := paramsproposaltypes.NewParameterChangeProposal(ibctesting.Title, ibctesting.Description, changes)
s.ExecuteGovV1Beta1Proposal(ctx, chainB, chainBUser, proposal)
s.ExecuteAndPassGovV1Beta1Proposal(ctx, chainB, chainBUser, proposal)
}
})

Expand Down
8 changes: 4 additions & 4 deletions e2e/tests/transfer/base_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,14 +285,14 @@ func (s *TransferTestSuite) TestSendEnabledParam() {
t.Run("change send enabled parameter to disabled", func(t *testing.T) {
if isSelfManagingParams {
msg := transfertypes.NewMsgUpdateParams(govModuleAddress.String(), transfertypes.NewParams(false, true))
s.ExecuteGovV1Proposal(ctx, msg, chainA, chainAWallet)
s.ExecuteAndPassGovV1Proposal(ctx, msg, chainA, chainAWallet)
} else {
changes := []paramsproposaltypes.ParamChange{
paramsproposaltypes.NewParamChange(transfertypes.StoreKey, string(transfertypes.KeySendEnabled), "false"),
}

proposal := paramsproposaltypes.NewParameterChangeProposal(ibctesting.Title, ibctesting.Description, changes)
s.ExecuteGovV1Beta1Proposal(ctx, chainA, chainAWallet, proposal)
s.ExecuteAndPassGovV1Beta1Proposal(ctx, chainA, chainAWallet, proposal)
}
})

Expand Down Expand Up @@ -376,14 +376,14 @@ func (s *TransferTestSuite) TestReceiveEnabledParam() {
t.Run("change receive enabled parameter to disabled ", func(t *testing.T) {
if isSelfManagingParams {
msg := transfertypes.NewMsgUpdateParams(govModuleAddress.String(), transfertypes.NewParams(false, false))
s.ExecuteGovV1Proposal(ctx, msg, chainA, chainAWallet)
s.ExecuteAndPassGovV1Proposal(ctx, msg, chainA, chainAWallet)
} else {
changes := []paramsproposaltypes.ParamChange{
paramsproposaltypes.NewParamChange(transfertypes.StoreKey, string(transfertypes.KeyReceiveEnabled), "false"),
}

proposal := paramsproposaltypes.NewParameterChangeProposal(ibctesting.Title, ibctesting.Description, changes)
s.ExecuteGovV1Beta1Proposal(ctx, chainA, chainAWallet, proposal)
s.ExecuteAndPassGovV1Beta1Proposal(ctx, chainA, chainAWallet, proposal)
}
})

Expand Down
4 changes: 2 additions & 2 deletions e2e/tests/upgrades/upgrade_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (s *UpgradeTestSuite) UpgradeChain(ctx context.Context, chain *cosmos.Cosmo
}

upgradeProposal := upgradetypes.NewSoftwareUpgradeProposal(fmt.Sprintf("upgrade from %s to %s", currentVersion, upgradeVersion), "upgrade chain E2E test", plan)
s.ExecuteGovV1Beta1Proposal(ctx, chain, wallet, upgradeProposal)
s.ExecuteAndPassGovV1Beta1Proposal(ctx, chain, wallet, upgradeProposal)

height, err := chain.Height(ctx)
s.Require().NoError(err, "error fetching height before upgrade")
Expand Down Expand Up @@ -561,7 +561,7 @@ func (s *UpgradeTestSuite) TestV7ToV8ChainUpgrade() {
s.Require().NotNil(authority)

msg := clienttypes.NewMsgUpdateParams(authority.String(), clienttypes.NewParams(exported.Tendermint, "some-client"))
s.ExecuteGovV1Proposal(ctx, msg, chainB, chainBWallet)
s.ExecuteAndPassGovV1Proposal(ctx, msg, chainB, chainBWallet)
})

t.Run("query params", func(t *testing.T) {
Expand Down
1 change: 0 additions & 1 deletion e2e/testsuite/grpc_query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,6 @@ func (s *E2ETestSuite) GetValidatorSetByHeight(ctx context.Context, chain ibc.Ch
// QueryModuleAccountAddress returns the sdk.AccAddress of a given module name.
func (s *E2ETestSuite) QueryModuleAccountAddress(ctx context.Context, moduleName string, chain *cosmos.CosmosChain) (sdk.AccAddress, error) {
authClient := s.GetChainGRCPClients(chain).AuthQueryClient

resp, err := authClient.ModuleAccountByName(ctx, &authtypes.QueryModuleAccountByNameRequest{
Name: moduleName,
})
Expand Down
27 changes: 16 additions & 11 deletions e2e/testsuite/tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,9 @@ If this is a compatibility test, ensure that the fields are being sanitized in t
return errorMsg
}

// ExecuteGovV1Proposal submits a v1 governance proposal using the provided user and message and uses all validators
// ExecuteAndPassGovV1Proposal submits a v1 governance proposal using the provided user and message and uses all validators
// to vote yes on the proposal. It ensures the proposal successfully passes.
func (s *E2ETestSuite) ExecuteGovV1Proposal(ctx context.Context, msg sdk.Msg, chain *cosmos.CosmosChain, user ibc.Wallet) {
func (s *E2ETestSuite) ExecuteAndPassGovV1Proposal(ctx context.Context, msg sdk.Msg, chain *cosmos.CosmosChain, user ibc.Wallet) {
sender, err := sdk.AccAddressFromBech32(user.FormattedAddress())
s.Require().NoError(err)

Expand Down Expand Up @@ -170,21 +170,15 @@ func (s *E2ETestSuite) ExecuteGovV1Proposal(ctx context.Context, msg sdk.Msg, ch
s.Require().Equal(govtypesv1.StatusPassed, proposal.Status)
}

// ExecuteGovV1Beta1Proposal submits the given v1beta1 governance proposal using the provided user and uses all validators to vote yes on the proposal.
// ExecuteAndPassGovV1Beta1Proposal submits the given v1beta1 governance proposal using the provided user and uses all validators to vote yes on the proposal.
// It ensures the proposal successfully passes.
func (s *E2ETestSuite) ExecuteGovV1Beta1Proposal(ctx context.Context, chain *cosmos.CosmosChain, user ibc.Wallet, content govtypesv1beta1.Content) {
func (s *E2ETestSuite) ExecuteAndPassGovV1Beta1Proposal(ctx context.Context, chain *cosmos.CosmosChain, user ibc.Wallet, content govtypesv1beta1.Content) {
proposalID := s.proposalIDs[chain.Config().ChainID]
defer func() {
s.proposalIDs[chain.Config().ChainID] = proposalID + 1
}()

sender, err := sdk.AccAddressFromBech32(user.FormattedAddress())
s.Require().NoError(err)

msgSubmitProposal, err := govtypesv1beta1.NewMsgSubmitProposal(content, sdk.NewCoins(sdk.NewCoin(chain.Config().Denom, govtypesv1beta1.DefaultMinDepositTokens)), sender)
s.Require().NoError(err)

txResp := s.BroadcastMessages(ctx, chain, user, msgSubmitProposal)
txResp := s.ExecuteGovV1Beta1Proposal(ctx, chain, user, content)
s.AssertTxSuccess(txResp)

// TODO: replace with parsed proposal ID from MsgSubmitProposalResponse
Expand All @@ -209,6 +203,17 @@ func (s *E2ETestSuite) ExecuteGovV1Beta1Proposal(ctx context.Context, chain *cos
s.Require().Equal(govtypesv1beta1.StatusPassed, proposal.Status)
}

// ExecuteGovV1Beta1Proposal submits a v1beta1 governance proposal using the provided content.
func (s *E2ETestSuite) ExecuteGovV1Beta1Proposal(ctx context.Context, chain *cosmos.CosmosChain, user ibc.Wallet, content govtypesv1beta1.Content) sdk.TxResponse {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

food for thought nit: executing governance proposals to me kinda assumes they passed as I'm imaging the proposal being executed. This is just submitting the proposal, nothing is being executed, but I don't feel strongly about this. Just mentioning it since it came to mind

sender, err := sdk.AccAddressFromBech32(user.FormattedAddress())
s.Require().NoError(err)

msgSubmitProposal, err := govtypesv1beta1.NewMsgSubmitProposal(content, sdk.NewCoins(sdk.NewCoin(chain.Config().Denom, govtypesv1beta1.DefaultMinDepositTokens)), sender)
s.Require().NoError(err)

return s.BroadcastMessages(ctx, chain, user, msgSubmitProposal)
}

// Transfer broadcasts a MsgTransfer message.
func (s *E2ETestSuite) Transfer(ctx context.Context, chain *cosmos.CosmosChain, user ibc.Wallet,
portID, channelID string, token sdk.Coin, sender, receiver string, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, memo string,
Expand Down
Loading