LaconicNetwork/cosmos-sdk
9
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

fix: Do not call Remove during Walk (#24044)

Browse Source 
Co-authored-by: Alex | Interchain Labs <alex@interchainlabs.io>
Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com>
This commit is contained in:
Đông Liều 2025-03-24 22:18:59 +07:00 committed by GitHub
parent e9fffcc4a7
commit 9b2e91fbf9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 48 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -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.

76
x/gov/abci.go
View File

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

6
x/gov/client/cli/prompt_test.go
View File

@ -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")

1
x/gov/simulation/operations_test.go
View File

@ -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, "")