diff --git a/CHANGELOG.md b/CHANGELOG.md index 0184dfcaec76..6aad09aac81b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,8 +57,12 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (x/gov) [#17873](https://github.com/cosmos/cosmos-sdk/pull/17873) Fail any inactive and active proposals whose messages cannot be decoded. + ### API Breaking Changes +* (app) [#17838](https://github.com/cosmos/cosmos-sdk/pull/17838) Params module was removed from simapp and all imports of the params module removed throughout the repo. + * The Cosmos SDK has migrated aay from using params, if you're app still uses it, then you can leave it plugged into your app * (x/staking) [#17778](https://github.com/cosmos/cosmos-sdk/pull/17778) Use collections for `Params` * remove from `Keeper`: `GetParams`, `SetParams` * (types/simulation) [#17737](https://github.com/cosmos/cosmos-sdk/pull/17737) Remove unused parameter from `RandomFees` diff --git a/x/gov/abci.go b/x/gov/abci.go index 8fbfb5c75613..f43370348fd7 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -1,10 +1,12 @@ package gov import ( + "errors" "fmt" "time" "cosmossdk.io/collections" + "cosmossdk.io/log" "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/telemetry" @@ -25,10 +27,26 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { err := keeper.InactiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) { proposal, err := keeper.Proposals.Get(ctx, key.K2()) if err != nil { + // if the proposal has an encoding error, this means it cannot be processed by x/gov + // this could be due to some types missing their registration + // instead of returning an error (i.e, halting the chain), we fail the proposal + if errors.Is(err, collections.ErrEncoding) { + proposal.Id = key.K2() + if err := failUnsupportedProposal(logger, ctx, keeper, proposal, err.Error(), false); err != nil { + return false, err + } + + if err = keeper.DeleteProposal(ctx, proposal.Id); err != nil { + return false, err + } + + return false, nil + } + return false, err } - err = keeper.DeleteProposal(ctx, proposal.Id) - if err != nil { + + if err = keeper.DeleteProposal(ctx, proposal.Id); err != nil { return false, err } @@ -77,6 +95,22 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { err = keeper.ActiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) { proposal, err := keeper.Proposals.Get(ctx, key.K2()) if err != nil { + // if the proposal has an encoding error, this means it cannot be processed by x/gov + // this could be due to some types missing their registration + // instead of returning an error (i.e, halting the chain), we fail the proposal + if errors.Is(err, collections.ErrEncoding) { + proposal.Id = key.K2() + if err := failUnsupportedProposal(logger, ctx, keeper, proposal, err.Error(), true); err != nil { + return false, err + } + + if err = keeper.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)); err != nil { + return false, err + } + + return false, nil + } + return false, err } @@ -97,14 +131,12 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { } else { err = keeper.RefundAndDeleteDeposits(ctx, proposal.Id) } - if err != nil { return false, err } } - err = keeper.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)) - if err != nil { + if err = keeper.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)); err != nil { return false, err } @@ -235,3 +267,48 @@ func safeExecuteHandler(ctx sdk.Context, msg sdk.Msg, handler baseapp.MsgService res, err = handler(ctx, msg) return } + +// failUnsupportedProposal fails a proposal that cannot be processed by gov +func failUnsupportedProposal( + logger log.Logger, + ctx sdk.Context, + keeper *keeper.Keeper, + proposal v1.Proposal, + errMsg string, + active bool, +) error { + proposal.Status = v1.StatusFailed + proposal.FailedReason = fmt.Sprintf("proposal failed because it cannot be processed by gov: %s", errMsg) + proposal.Messages = nil // clear out the messages + + if err := keeper.SetProposal(ctx, proposal); err != nil { + return err + } + + if err := keeper.RefundAndDeleteDeposits(ctx, proposal.Id); err != nil { + return err + } + + eventType := types.EventTypeInactiveProposal + if active { + eventType = types.EventTypeActiveProposal + } + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + eventType, + sdk.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposal.Id)), + sdk.NewAttribute(types.AttributeKeyProposalResult, types.AttributeValueProposalFailed), + ), + ) + + logger.Info( + "proposal failed to decode; deleted", + "proposal", proposal.Id, + "expedited", proposal.Expedited, + "title", proposal.Title, + "results", errMsg, + ) + + return nil +} diff --git a/x/gov/abci_test.go b/x/gov/abci_test.go index 915efd2333f7..1565e94a2b8f 100644 --- a/x/gov/abci_test.go +++ b/x/gov/abci_test.go @@ -22,6 +22,65 @@ import ( stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) +func TestUnregisteredProposal_InactiveProposalFails(t *testing.T) { + suite := createTestSuite(t) + ctx := suite.App.BaseApp.NewContext(false) + addrs := simtestutil.AddTestAddrs(suite.BankKeeper, suite.StakingKeeper, ctx, 10, valTokens) + + // manually set proposal in store + startTime, endTime := time.Now().Add(-4*time.Hour), ctx.BlockHeader().Time + proposal, err := v1.NewProposal([]sdk.Msg{ + &v1.Proposal{}, // invalid proposal message + }, 1, startTime, startTime, "", "Unsupported proposal", "Unsupported proposal", addrs[0], false) + require.NoError(t, err) + + err = suite.GovKeeper.SetProposal(ctx, proposal) + require.NoError(t, err) + + // manually set proposal in inactive proposal queue + err = suite.GovKeeper.InactiveProposalsQueue.Set(ctx, collections.Join(endTime, proposal.Id), proposal.Id) + require.NoError(t, err) + + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper) + + err = gov.EndBlocker(ctx, suite.GovKeeper) + require.NoError(t, err) + + _, err = suite.GovKeeper.Proposals.Get(ctx, proposal.Id) + require.Error(t, err, collections.ErrNotFound) +} + +func TestUnregisteredProposal_ActiveProposalFails(t *testing.T) { + suite := createTestSuite(t) + ctx := suite.App.BaseApp.NewContext(false) + addrs := simtestutil.AddTestAddrs(suite.BankKeeper, suite.StakingKeeper, ctx, 10, valTokens) + + // manually set proposal in store + startTime, endTime := time.Now().Add(-4*time.Hour), ctx.BlockHeader().Time + proposal, err := v1.NewProposal([]sdk.Msg{ + &v1.Proposal{}, // invalid proposal message + }, 1, startTime, startTime, "", "Unsupported proposal", "Unsupported proposal", addrs[0], false) + require.NoError(t, err) + proposal.Status = v1.StatusVotingPeriod + proposal.VotingEndTime = &endTime + + err = suite.GovKeeper.SetProposal(ctx, proposal) + require.NoError(t, err) + + // manually set proposal in active proposal queue + err = suite.GovKeeper.ActiveProposalsQueue.Set(ctx, collections.Join(endTime, proposal.Id), proposal.Id) + require.NoError(t, err) + + checkActiveProposalsQueue(t, ctx, suite.GovKeeper) + + err = gov.EndBlocker(ctx, suite.GovKeeper) + require.NoError(t, err) + + p, err := suite.GovKeeper.Proposals.Get(ctx, proposal.Id) + require.NoError(t, err) + require.Equal(t, v1.StatusFailed, p.Status) +} + func TestTickExpiredDepositPeriod(t *testing.T) { suite := createTestSuite(t) app := suite.App