diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f8d34dac5..41779baa9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (x/gov) [#24044](https://github.com/cosmos/cosmos-sdk/pull/24044) Fix some places in which we call Remove inside a Walk (x/gov). * (baseapp) [#24042](https://github.com/cosmos/cosmos-sdk/pull/24042) Fixed a data race inside BaseApp.getContext, found by end-to-end (e2e) tests. * (client/server) [#24059](https://github.com/cosmos/cosmos-sdk/pull/24059) Consistently set viper prefix in client and server. It defaults for the binary name for both client and server. * (client/keys) [#24041](https://github.com/cosmos/cosmos-sdk/pull/24041) `keys delete` won't terminate when a key is not found, but will log the error. diff --git a/x/gov/abci.go b/x/gov/abci.go index 56ab8641bd..870983e3e9 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -24,35 +24,45 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { // delete dead proposals from store and returns theirs deposits. // A proposal is dead when it's inactive and didn't get enough deposit on time to get into voting phase. rng := collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.BlockTime()) - err := keeper.InactiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) { - proposal, err := keeper.Proposals.Get(ctx, key.K2()) + iter, err := keeper.InactiveProposalsQueue.Iterate(ctx, rng) + if err != nil { + return err + } + + inactiveProps, err := iter.KeyValues() + if err != nil { + return err + } + + for _, prop := range inactiveProps { + proposal, err := keeper.Proposals.Get(ctx, prop.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() + proposal.Id = prop.Key.K2() if err := failUnsupportedProposal(logger, ctx, keeper, proposal, err.Error(), false); err != nil { - return false, err + return err } if err = keeper.DeleteProposal(ctx, proposal.Id); err != nil { - return false, err + return err } - return false, nil + return nil } - return false, err + return err } if err = keeper.DeleteProposal(ctx, proposal.Id); err != nil { - return false, err + return err } params, err := keeper.Params.Get(ctx) if err != nil { - return false, err + return err } if !params.BurnProposalDepositPrevote { err = keeper.RefundAndDeleteDeposits(ctx, proposal.Id) // refund deposit if proposal got removed without getting 100% of the proposal @@ -61,7 +71,7 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { } if err != nil { - return false, err + return err } // called when proposal become inactive @@ -89,42 +99,47 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { "min_deposit", sdk.NewCoins(proposal.GetMinDepositFromParams(params)...).String(), "total_deposit", sdk.NewCoins(proposal.TotalDeposit...).String(), ) - - return false, nil - }) - if err != nil { - return err } // fetch active proposals whose voting periods have ended (are passed the block time) rng = collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.BlockTime()) - err = keeper.ActiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) { - proposal, err := keeper.Proposals.Get(ctx, key.K2()) + iter, err = keeper.ActiveProposalsQueue.Iterate(ctx, rng) + if err != nil { + return err + } + + activeProps, err := iter.KeyValues() + if err != nil { + return err + } + + for _, prop := range activeProps { + proposal, err := keeper.Proposals.Get(ctx, prop.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() + proposal.Id = prop.Key.K2() if err := failUnsupportedProposal(logger, ctx, keeper, proposal, err.Error(), true); err != nil { - return false, err + return err } if err = keeper.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)); err != nil { - return false, err + return err } - return false, nil + return nil } - return false, err + return err } var tagValue, logMsg string passes, burnDeposits, tallyResults, err := keeper.Tally(ctx, proposal) if err != nil { - return false, err + return err } // If an expedited proposal fails, we do not want to update @@ -138,12 +153,12 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { err = keeper.RefundAndDeleteDeposits(ctx, proposal.Id) } if err != nil { - return false, err + return err } } if err = keeper.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)); err != nil { - return false, err + return err } switch { @@ -207,14 +222,14 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { proposal.Expedited = false params, err := keeper.Params.Get(ctx) if err != nil { - return false, err + return err } endTime := proposal.VotingStartTime.Add(*params.VotingPeriod) proposal.VotingEndTime = &endTime err = keeper.ActiveProposalsQueue.Set(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id), proposal.Id) if err != nil { - return false, err + return err } tagValue = types.AttributeValueExpeditedProposalRejected @@ -230,7 +245,7 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { err = keeper.SetProposal(ctx, proposal) if err != nil { - return false, err + return err } // when proposal become active @@ -259,11 +274,6 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { sdk.NewAttribute(types.AttributeKeyProposalLog, logMsg), ), ) - - return false, nil - }) - if err != nil { - return err } return nil } diff --git a/x/gov/client/cli/prompt_test.go b/x/gov/client/cli/prompt_test.go index 673c9da183..64d61728cb 100644 --- a/x/gov/client/cli/prompt_test.go +++ b/x/gov/client/cli/prompt_test.go @@ -47,7 +47,8 @@ func TestPromptIntegerOverflow(t *testing.T) { fin, fw := readline.NewFillableStdin(os.Stdin) readline.Stdin = fin - _, _ = fw.Write([]byte(overflowStr + "\n")) + _, err := fw.Write([]byte(overflowStr + "\n")) + require.NoError(t, err) v, err := cli.Prompt(st{}, "") assert.Equal(t, st{}, v, "expected a value of zero") @@ -77,7 +78,8 @@ func TestPromptParseInteger(t *testing.T) { fin, fw := readline.NewFillableStdin(os.Stdin) readline.Stdin = fin - _, _ = fw.Write([]byte(tc.in + "\n")) + _, err := fw.Write([]byte(tc.in + "\n")) + assert.NoError(t, err) v, err := cli.Prompt(st{}, "") assert.Nil(t, err, "expected a nil error") diff --git a/x/gov/simulation/operations_test.go b/x/gov/simulation/operations_test.go index c6dfb7a66d..9ee9539d68 100644 --- a/x/gov/simulation/operations_test.go +++ b/x/gov/simulation/operations_test.go @@ -183,7 +183,6 @@ func TestSimulateMsgSubmitLegacyProposal(t *testing.T) { Hash: app.LastCommitID().Hash, }) require.NoError(t, err) - // execute operation op := simulation.SimulateMsgSubmitLegacyProposal(suite.TxConfig, suite.AccountKeeper, suite.BankKeeper, suite.GovKeeper, MockWeightedProposals{3}.ContentSimulatorFn()) operationMsg, _, err := op(r, app.BaseApp, ctx, accounts, "")