fix: Do not call Remove during Walk - part 2 (cosmos#19851)

CHANGELOG.md

@@ -99,6 +99,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
 * (server) [#18994](https://github.com/cosmos/cosmos-sdk/pull/18994) Update server context directly rather than a reference to a sub-object
 * (crypto) [#19691](https://github.com/cosmos/cosmos-sdk/pull/19691) Fix tx sign doesn't throw an error when incorrect Ledger is used.
 * [#19833](https://github.com/cosmos/cosmos-sdk/pull/19833) Fix some places in which we call Remove inside a Walk.
+* [#19851](https://github.com/cosmos/cosmos-sdk/pull/19851) Fix some places in which we call Remove inside a Walk (x/staking and x/gov).
 
 ### API Breaking Changes
 

x/gov/keeper/abci.go

@@ -27,35 +27,45 @@ func (k Keeper) EndBlocker(ctx context.Context) 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](k.environment.HeaderService.GetHeaderInfo(ctx).Time)
- err := k.InactiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) {
- proposal, err := k.Proposals.Get(ctx, key.K2())
+ iter, err := k.InactiveProposalsQueue.Iterate(ctx, rng)
+ if err != nil {
+ return err
+ }
+
+ inactiveProps, err := iter.KeyValues()
+ if err != nil {
+ return err
+ }
+
+ for _, prop := range inactiveProps {
+ proposal, err := k.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, k, proposal, err.Error(), false); err != nil {
- return false, err
+ return err
  }
 
  if err = k.DeleteProposal(ctx, proposal.Id); err != nil {
- return false, err
+ return err
  }
 
- return false, nil
+ continue
  }
 
- return false, err
+ return err
  }
 
  if err = k.DeleteProposal(ctx, proposal.Id); err != nil {
- return false, err
+ return err
  }
 
  params, err := k.Params.Get(ctx)
  if err != nil {
- return false, err
+ return err
  }
  if !params.BurnProposalDepositPrevote {
  err = k.RefundAndDeleteDeposits(ctx, proposal.Id) // refund deposit if proposal got removed without getting 100% of the proposal
@@ -64,7 +74,7 @@ func (k Keeper) EndBlocker(ctx context.Context) error {
  }
 
  if err != nil {
- return false, err
+ return err
  }
 
  // called when proposal become inactive
@@ -91,42 +101,49 @@ func (k Keeper) EndBlocker(ctx context.Context) error {
  "min_deposit", sdk.NewCoins(proposal.GetMinDepositFromParams(params)...).String(),
  "total_deposit", sdk.NewCoins(proposal.TotalDeposit...).String(),
  )
+ }
 
- return false, nil
- })
+ // fetch active proposals whose voting periods have ended (are passed the block time)
+ rng = collections.NewPrefixUntilPairRange[time.Time, uint64](k.environment.HeaderService.GetHeaderInfo(ctx).Time)
+
+ iter, err = k.ActiveProposalsQueue.Iterate(ctx, rng)
  if err != nil {
  return err
  }
 
- // fetch active proposals whose voting periods have ended (are passed the block time)
- rng = collections.NewPrefixUntilPairRange[time.Time, uint64](k.environment.HeaderService.GetHeaderInfo(ctx).Time)
- err = k.ActiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) {
- proposal, err := k.Proposals.Get(ctx, key.K2())
+ activeProps, err := iter.KeyValues()
+ if err != nil {
+ return err
+ }
+
+ // err = k.ActiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) {
+ for _, prop := range activeProps {
+ proposal, err := k.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, k, proposal, err.Error(), true); err != nil {
- return false, err
+ return err
  }
 
  if err = k.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)); err != nil {
- return false, err
+ return err
  }
 
- return false, nil
+ continue
  }
 
- return false, err
+ return err
  }
 
  var tagValue, logMsg string
 
  passes, burnDeposits, tallyResults, err := k.Tally(ctx, proposal)
  if err != nil {
- return false, err
+ return err
  }
 
  // Deposits are always burned if tally said so, regardless of the proposal type.
@@ -154,7 +171,7 @@ func (k Keeper) EndBlocker(ctx context.Context) error {
  }
 
  if err = k.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)); err != nil {
- return false, err
+ return err
  }
 
  switch {
@@ -210,14 +227,14 @@ func (k Keeper) EndBlocker(ctx context.Context) error {
  proposal.Expedited = false // can be removed as never read but kept for state coherence
  params, err := k.Params.Get(ctx)
  if err != nil {
- return false, err
+ return err
  }
  endTime := proposal.VotingStartTime.Add(*params.VotingPeriod)
  proposal.VotingEndTime = &endTime
 
  err = k.ActiveProposalsQueue.Set(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id), proposal.Id)
  if err != nil {
- return false, err
+ return err
  }
 
  if proposal.ProposalType == v1.ProposalType_PROPOSAL_TYPE_EXPEDITED {
@@ -237,7 +254,7 @@ func (k Keeper) EndBlocker(ctx context.Context) error {
  proposal.FinalTallyResult = &tallyResults
 
  if err = k.Proposals.Set(ctx, proposal.Id, proposal); err != nil {
- return false, err
+ return err
  }
 
  // call hook when proposal become active
@@ -264,10 +281,8 @@ func (k Keeper) EndBlocker(ctx context.Context) error {
  ); err != nil {
  logger.Error("failed to emit event", "error", err)
  }
-
- return false, nil
- })
- return err
+ }
+ return nil
 }
 
 // executes route(msg) and recovers from panic.

x/staking/keeper/validator.go

@@ -494,9 +494,24 @@ func (k Keeper) UnbondAllMatureValidators(ctx context.Context) error {
  rng := new(collections.Range[collections.Triple[uint64, time.Time, uint64]]).
  EndInclusive(collections.Join3(uint64(29), blockTime, blockHeight))
 
- return k.ValidatorQueue.Walk(ctx, rng, func(key collections.Triple[uint64, time.Time, uint64], value types.ValAddresses) (stop bool, err error) {
- return false, k.unbondMatureValidators(ctx, blockHeight, blockTime, key, value)
- })
+ // get all the values before performing any delete operations
+ iter, err := k.ValidatorQueue.Iterate(ctx, rng)
+ if err != nil {
+ return err
+ }
+
+ kvs, err := iter.KeyValues()
+ if err != nil {
+ return err
+ }
+
+ for _, kv := range kvs {
+ if err := k.unbondMatureValidators(ctx, blockHeight, blockTime, kv.Key, kv.Value); err != nil {
+ return err
+ }
+ }
+
+ return nil
 }
 
 func (k Keeper) unbondMatureValidators(