Skip to content

Commit

Permalink
fix(gov): fail proposal when implementation not found (#17873)
Browse files Browse the repository at this point in the history 
(cherry picked from commit 2f7bc22)

# Conflicts:
#	CHANGELOG.md
  • Loading branch information
@julienrbrt @mergify
julienrbrt authored and mergify[bot] committed Sep 26, 2023
1 parent cd9f0a6 commit 9ed374d
Show file tree
Hide file tree
Showing 3 changed files with 263 additions and 5 deletions.
122 changes: 122 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,128 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

<<<<<<< HEAD
=======
### Features

* (baseapp) [#16581](https://github.com/cosmos/cosmos-sdk/pull/16581) Implement Optimistic Execution as an experimental feature (not enabled by default).
* (client/keys) [#17639](https://github.com/cosmos/cosmos-sdk/pull/17639) Allows using and saving public keys encoded as base64
* (client) [#17513](https://github.com/cosmos/cosmos-sdk/pull/17513) Allow overwritting `client.toml`. Use `client.CreateClientConfig` in place of `client.ReadFromClientConfig` and provide a custom template and a custom config.
* (x/bank) [#17569](https://github.com/cosmos/cosmos-sdk/pull/17569) Introduce a new message type, `MsgBurn `, to burn coins.
* (server) [#17094](https://github.com/cosmos/cosmos-sdk/pull/17094) Add duration `shutdown-grace` for resource clean up (closing database handles) before exit.

### Improvements

* (client) [#17503](https://github.com/cosmos/cosmos-sdk/pull/17503) Add `client.Context{}.WithAddressCodec`, `WithValidatorAddressCodec`, `WithConsensusAddressCodec` to provide address codecs to the client context. See the [UPGRADING.md](./UPGRADING.md) for more details.
* (crypto/keyring) [#17503](https://github.com/cosmos/cosmos-sdk/pull/17503) Simplify keyring interfaces to use `[]byte` instead of `sdk.Address` for addresses.
* (all) [#16537](https://github.com/cosmos/cosmos-sdk/pull/16537) Properly propagated `fmt.Errorf` errors and using `errors.New` where appropriate.
* (rpc) [#17470](https://github.com/cosmos/cosmos-sdk/pull/17470) Avoid open 0.0.0.0 to public by default and add `listen-ip-address` argument for `testnet init-files` cmd.
* (types) [#17670](https://github.com/cosmos/cosmos-sdk/pull/17670) Use `ctx.CometInfo` in place of `ctx.VoteInfos`
* [#17733](https://github.com/cosmos/cosmos-sdk/pull/17733) Ensure `buf export` exports all proto dependencies

### 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`
* (x/staking) [#17486](https://github.com/cosmos/cosmos-sdk/pull/17486) Use collections for `RedelegationQueueKey`:
* remove from `types`: `GetRedelegationTimeKey`
* remove from `Keeper`: `RedelegationQueueIterator`
* (x/staking) [#17562](https://github.com/cosmos/cosmos-sdk/pull/17562) Use collections for `ValidatorQueue`
* remove from `types`: `GetValidatorQueueKey`, `ParseValidatorQueueKey`
* remove from `Keeper`: `ValidatorQueueIterator`
* (x/staking) [#17498](https://github.com/cosmos/cosmos-sdk/pull/17498) Use collections for `LastValidatorPower`:
* remove from `types`: `GetLastValidatorPowerKey`
* remove from `Keeper`: `LastValidatorsIterator`, `IterateLastValidators`
* (x/staking) [#17291](https://github.com/cosmos/cosmos-sdk/pull/17291) Use collections for `UnbondingDelegationByValIndex`:
* remove from `types`: `GetUBDKeyFromValIndexKey`, `GetUBDsByValIndexKey`, `GetUBDByValIndexKey`
* (x/slashing) [#17568](https://github.com/cosmos/cosmos-sdk/pull/17568) Use collections for `ValidatorMissedBlockBitmap`:
* remove from `types`: `ValidatorMissedBlockBitmapPrefixKey`, `ValidatorMissedBlockBitmapKey`
* (x/staking) [#17481](https://github.com/cosmos/cosmos-sdk/pull/17481) Use collections for `UnbondingQueue`:
* remove from `Keeper`: `UBDQueueIterator`
* remove from `types`: `GetUnbondingDelegationTimeKey`
* (x/staking) [#17123](https://github.com/cosmos/cosmos-sdk/pull/17123) Use collections for `Validators`
* (x/staking) [#17270](https://github.com/cosmos/cosmos-sdk/pull/17270) Use collections for `UnbondingDelegation`:
* remove from `types`: `GetUBDsKey`
* remove from `Keeper`: `IterateUnbondingDelegations`, `IterateDelegatorUnbondingDelegations`
* (client/keys) [#17503](https://github.com/cosmos/cosmos-sdk/pull/17503) `clientkeys.NewKeyOutput`, `MkConsKeyOutput`, `MkValKeyOutput`, `MkAccKeyOutput`, `MkAccKeysOutput` now take their corresponding address codec instead of using the global SDK config.
* (x/staking) [#17336](https://github.com/cosmos/cosmos-sdk/pull/17336) Use collections for `RedelegationByValDstIndexKey`:
* remove from `types`: `GetREDByValDstIndexKey`, `GetREDsToValDstIndexKey`
* (x/staking) [#17332](https://github.com/cosmos/cosmos-sdk/pull/17332) Use collections for `RedelegationByValSrcIndexKey`:
* remove from `types`: `GetREDKeyFromValSrcIndexKey`, `GetREDsFromValSrcIndexKey`
* (x/staking) [#17315](https://github.com/cosmos/cosmos-sdk/pull/17315) Use collections for `RedelegationKey`:
* remove from `keeper`: `GetRedelegation`
* (types) `module.BeginBlockAppModule` has been replaced by Core API `appmodule.HasBeginBlocker`.
* (types) `module.EndBlockAppModule` has been replaced by Core API `appmodule.HasEndBlocker` or `module.HasABCIEndBlock` when needing validator updates.
* (x/slashing) [#17044](https://github.com/cosmos/cosmos-sdk/pull/17044) Use collections for `AddrPubkeyRelation`:
* remove from `types`: `AddrPubkeyRelationKey`
* remove from `Keeper`: `AddPubkey`
* (x/staking) [#17260](https://github.com/cosmos/cosmos-sdk/pull/17260) Use collections for `DelegationKey`:
* remove from `types`: `GetDelegationKey`, `GetDelegationsKey`
* (x/staking) [#17288](https://github.com/cosmos/cosmos-sdk/pull/17288) Use collections for `UnbondingIndex`:
* remove from `types`: `GetUnbondingIndexKey`.
* (x/staking) [#17256](https://github.com/cosmos/cosmos-sdk/pull/17256) Use collections for `UnbondingID`.
* (x/staking) [#17260](https://github.com/cosmos/cosmos-sdk/pull/17260) Use collections for `ValidatorByConsAddr`:
* remove from `types`: `GetValidatorByConsAddrKey`
* (x/staking) [#17248](https://github.com/cosmos/cosmos-sdk/pull/17248) Use collections for `UnbondingType`.
* remove from `types`: `GetUnbondingTypeKey`.
* (client) [#17259](https://github.com/cosmos/cosmos-sdk/pull/17259) Remove deprecated `clientCtx.PrintObjectLegacy`. Use `clientCtx.PrintProto` or `clientCtx.PrintRaw` instead.
* (x/distribution) [#17115](https://github.com/cosmos/cosmos-sdk/pull/17115) Use collections for `PreviousProposer` and `ValidatorSlashEvents`:
* remove from `Keeper`: `GetPreviousProposerConsAddr`, `SetPreviousProposerConsAddr`, `GetValidatorHistoricalReferenceCount`, `GetValidatorSlashEvent`, `SetValidatorSlashEvent`.
* (x/feegrant) [#16535](https://github.com/cosmos/cosmos-sdk/pull/16535) Use collections for `FeeAllowance`, `FeeAllowanceQueue`.
* (x/staking) [#17063](https://github.com/cosmos/cosmos-sdk/pull/17063) Use collections for `HistoricalInfo`:
* remove `Keeper`: `GetHistoricalInfo`, `SetHistoricalInfo`
* (x/staking) [#17062](https://github.com/cosmos/cosmos-sdk/pull/17062) Use collections for `ValidatorUpdates`:
* remove `Keeper`: `SetValidatorUpdates`, `GetValidatorUpdates`
* (x/slashing) [#17023](https://github.com/cosmos/cosmos-sdk/pull/17023) Use collections for `ValidatorSigningInfo`:
* remove `Keeper`: `SetValidatorSigningInfo`, `GetValidatorSigningInfo`, `IterateValidatorSigningInfos`
* (x/staking) [#17026](https://github.com/cosmos/cosmos-sdk/pull/17026) Use collections for `LastTotalPower`:
* remove `Keeper`: `SetLastTotalPower`, `GetLastTotalPower`
* (x/distribution) [#16440](https://github.com/cosmos/cosmos-sdk/pull/16440) use collections for `DelegatorWithdrawAddresState`:
* remove `Keeper`: `SetDelegatorWithdrawAddr`, `DeleteDelegatorWithdrawAddr`, `IterateDelegatorWithdrawAddrs`.
* (x/distribution) [#16459](https://github.com/cosmos/cosmos-sdk/pull/16459) use collections for `ValidatorCurrentRewards` state management:
* remove `Keeper`: `IterateValidatorCurrentRewards`, `GetValidatorCurrentRewards`, `SetValidatorCurrentRewards`, `DeleteValidatorCurrentRewards`
* (x/authz) [#16509](https://github.com/cosmos/cosmos-sdk/pull/16509) `AcceptResponse` has been moved to sdk/types/authz and the `Updated` field is now of the type `sdk.Msg` instead of `authz.Authorization`.
* (x/distribution) [#16483](https://github.com/cosmos/cosmos-sdk/pull/16483) use collections for `DelegatorStartingInfo` state management:
* remove `Keeper`: `IterateDelegatorStartingInfo`, `GetDelegatorStartingInfo`, `SetDelegatorStartingInfo`, `DeleteDelegatorStartingInfo`, `HasDelegatorStartingInfo`
* (x/distribution) [#16571](https://github.com/cosmos/cosmos-sdk/pull/16571) use collections for `ValidatorAccumulatedCommission` state management:
* remove `Keeper`: `IterateValidatorAccumulatedCommission`, `GetValidatorAccumulatedCommission`, `SetValidatorAccumulatedCommission`, `DeleteValidatorAccumulatedCommission`
* (x/distribution) [#16590](https://github.com/cosmos/cosmos-sdk/pull/16590) use collections for `ValidatorOutstandingRewards` state management:
* remove `Keeper`: `IterateValidatorOutstandingRewards`, `GetValidatorOutstandingRewards`, `SetValidatorOutstandingRewards`, `DeleteValidatorOutstandingRewards`
* (x/distribution) [#16607](https://github.com/cosmos/cosmos-sdk/pull/16607) use collections for `ValidatorHistoricalRewards` state management:
* remove `Keeper`: `IterateValidatorHistoricalRewards`, `GetValidatorHistoricalRewards`, `SetValidatorHistoricalRewards`, `DeleteValidatorHistoricalRewards`, `DeleteValidatorHistoricalReward`, `DeleteAllValidatorHistoricalRewards`
* (x/slashing) [#16441](https://github.com/cosmos/cosmos-sdk/pull/16441) Params state is migrated to collections. `GetParams` has been removed.
* (types) [#16918](https://github.com/cosmos/cosmos-sdk/pull/16918) Remove `IntProto` and `DecProto`. Instead, `math.Int` and `math.LegacyDec` should be used respectively. Both types support `Marshal` and `Unmarshal` which should be used for binary marshaling.
* (client) [#17215](https://github.com/cosmos/cosmos-sdk/pull/17215) `server.StartCmd`,`server.ExportCmd`,`server.NewRollbackCmd`,`pruning.Cmd`,`genutilcli.InitCmd`,`genutilcli.GenTxCmd`,`genutilcli.CollectGenTxsCmd`,`genutilcli.AddGenesisAccountCmd`, do not take a home directory anymore. It is inferred from the root command.
* (baseapp) [#16244](https://github.com/cosmos/cosmos-sdk/pull/16244) `SetProtocolVersion` has been renamed to `SetAppVersion`. It now updates the consensus params in baseapp's `ParamStore`.
* (types) [#17348](https://github.com/cosmos/cosmos-sdk/pull/17348) Remove the `WrapServiceResult` function.
* The `*sdk.Result` returned by the msg server router will not contain the `.Data` field.
* (x/staking) [#17335](https://github.com/cosmos/cosmos-sdk/pull/17335) Remove usage of `"github.com/cosmos/cosmos-sdk/x/staking/types".Infraction_*` in favour of `"cosmossdk.io/api/cosmos/staking/v1beta1".Infraction_` in order to remove dependency between modules on staking
* (x/gov) [#17496](https://github.com/cosmos/cosmos-sdk/pull/17469) in `x/gov/types/v1beta1/vote.go` `NewVote` was removed, constructing the struct is required for this type
* (types) [#17426](https://github.com/cosmos/cosmos-sdk/pull/17426) `NewContext` does not take a `cmtproto.Header{}` any longer.
* `WithChainID` / `WithBlockHeight` / `WithBlockHeader` must be used to set values on the context
* (x/bank) [#17569](https://github.com/cosmos/cosmos-sdk/pull/17569) `BurnCoins` takes an address instead of a module name
* (x/distribution) [#17670](https://github.com/cosmos/cosmos-sdk/pull/17670) `AllocateTokens` takes `comet.VoteInfos` instead of `[]abci.VoteInfo`
* (types) [#17738](https://github.com/cosmos/cosmos-sdk/pull/17738) `WithBlockTime()` was removed & `BlockTime()` were deprecated in favor of `WithHeaderInfo()` & `HeaderInfo()`. `BlockTime` now gets data from `HeaderInfo()` instead of `BlockHeader()`.
* (client) [#17746](https://github.com/cosmos/cosmos-sdk/pull/17746) `txEncodeAmino` & `txDecodeAmino` txs via grpc and rest were removed
* `RegisterLegacyAmino` was removed from `AppModuleBasic`
* (x/staking) [#17655](https://github.com/cosmos/cosmos-sdk/pull/17655) `QueryHistoricalInfo` was adjusted to return `HistoricalRecord` and marked `Hist` as deprecated.

### CLI Breaking Changes

### State Machine Breaking

* (x/distribution) [#17115](https://github.com/cosmos/cosmos-sdk/pull/17115) Migrate `PreviousProposer` to collections.
* (x/upgrade) [#16244](https://github.com/cosmos/cosmos-sdk/pull/16244) upgrade module no longer stores the app version but gets and sets the app version stored in the `ParamStore` of baseapp.
* (x/staking) [#17655](https://github.com/cosmos/cosmos-sdk/pull/17655) `HistoricalInfo` was replaced with `HistoricalRecord`, it removes the validator set and comet header and only keep what is needed for IBC.

>>>>>>> 2f7bc22a5 (fix(gov): fail proposal when implementation not found (#17873))
## [v0.50.0-rc.1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.0-rc.1) - 2023-09-25

### Features
Expand Down
87 changes: 82 additions & 5 deletions x/gov/abci.go
View file
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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
}

Expand Down Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down Expand Up @@ -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
}
59 changes: 59 additions & 0 deletions x/gov/abci_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9ed374d

Please sign in to comment.