refactor(gov)!: use collections for deposit state management (#16127)

Co-authored-by: unknown unknown <unknown@unknown>

CHANGELOG.md

@@ -196,6 +196,9 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (types/math) [#16040](https://github.com/cosmos/cosmos-sdk/pull/16040) Remove unused aliases in math.go
 * (x/gov) [#16106](https://github.com/cosmos/cosmos-sdk/pull/16106) Remove gRPC query methods from Keeper
 * (x/gov) [#16118](https://github.com/cosmos/cosmos-sdk/pull/16118/) Use collections for constituion and params state management.
+* (x/gov) [](https://github.com/cosmos/cosmos-sdk/pull/16127) Use collections for deposit management:
+  - The following methods are removed from the gov keeper: `GetDeposit`, `GetAllDeposits`, `IterateAllDeposits`.
+  - The following functions are removed from the gov types: `DepositKey`, `DepositsKey`.
 
 ### Client Breaking Changes
 

collections/CHANGELOG.md

@@ -35,6 +35,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * [#16074](https://github.com/cosmos/cosmos-sdk/pull/16074) – makes the generic Collection interface public, still highly unstable.
 
+### API Breaking
+
+* [#16127](https://github.com/cosmos/cosmos-sdk/pull/16127) – In the `Walk` method the call back function being passed is allowed to error.
+
 ## [v0.1.0](https://github.com/cosmos/cosmos-sdk/releases/tag/collections%2Fv0.1.0)
 
 Collections `v0.1.0` is released! Check out the [docs](https://docs.cosmos.network/main/packages/collections) to know how to use the APIs. 

collections/indexed_map.go

@@ -93,7 +93,7 @@ func (m *IndexedMap[PrimaryKey, Value, Idx]) Remove(ctx context.Context, pk Prim
 }
 
 // Walk applies the same semantics as Map.Walk.
-func (m *IndexedMap[PrimaryKey, Value, Idx]) Walk(ctx context.Context, ranger Ranger[PrimaryKey], walkFunc func(key PrimaryKey, value Value) bool) error {
+func (m *IndexedMap[PrimaryKey, Value, Idx]) Walk(ctx context.Context, ranger Ranger[PrimaryKey], walkFunc func(key PrimaryKey, value Value) (stop bool, err error)) error {
 	return m.m.Walk(ctx, ranger, walkFunc)
 }
 

collections/indexes/multi.go

@@ -82,9 +82,9 @@ func (m *Multi[ReferenceKey, PrimaryKey, Value]) Iterate(ctx context.Context, ra
 func (m *Multi[ReferenceKey, PrimaryKey, Value]) Walk(
 	ctx context.Context,
 	ranger collections.Ranger[collections.Pair[ReferenceKey, PrimaryKey]],
-	walkFunc func(indexingKey ReferenceKey, indexedKey PrimaryKey) bool,
+	walkFunc func(indexingKey ReferenceKey, indexedKey PrimaryKey) (stop bool, err error),
 ) error {
-	return m.refKeys.Walk(ctx, ranger, func(key collections.Pair[ReferenceKey, PrimaryKey]) bool {
+	return m.refKeys.Walk(ctx, ranger, func(key collections.Pair[ReferenceKey, PrimaryKey]) (bool, error) {
 		return walkFunc(key.K1(), key.K2())
 	})
 }

collections/indexes/reverse_pair.go

@@ -67,9 +67,9 @@ func (i *ReversePair[K1, K2, Value]) Unreference(ctx context.Context, pk collect
 func (i *ReversePair[K1, K2, Value]) Walk(
 	ctx context.Context,
 	ranger collections.Ranger[collections.Pair[K2, K1]],
-	walkFunc func(indexingKey K2, indexedKey K1) bool,
+	walkFunc func(indexingKey K2, indexedKey K1) (stop bool, err error),
 ) error {
-	return i.refKeys.Walk(ctx, ranger, func(key collections.Pair[K2, K1]) bool {
+	return i.refKeys.Walk(ctx, ranger, func(key collections.Pair[K2, K1]) (bool, error) {
 		return walkFunc(key.K1(), key.K2())
 	})
 }

collections/indexes/unique.go

@@ -90,7 +90,7 @@ func (i *Unique[ReferenceKey, PrimaryKey, Value]) Iterate(ctx context.Context, r
 func (i *Unique[ReferenceKey, PrimaryKey, Value]) Walk(
 	ctx context.Context,
 	ranger collections.Ranger[ReferenceKey],
-	walkFunc func(indexingKey ReferenceKey, indexedKey PrimaryKey) bool,
+	walkFunc func(indexingKey ReferenceKey, indexedKey PrimaryKey) (stop bool, err error),
 ) error {
 	return i.refKeys.Walk(ctx, ranger, walkFunc)
 }

collections/iter_test.go

@@ -175,14 +175,24 @@ func TestWalk(t *testing.T) {
 	}
 
 	u := uint64(0)
-	err = m.Walk(ctx, nil, func(key, value uint64) bool {
+	err = m.Walk(ctx, nil, func(key, value uint64) (bool, error) {
 		if key == 5 {
-			return true
+			return true, nil
 		}
 		require.Equal(t, u, key)
 		require.Equal(t, u, value)
 		u++
-		return false
+		return false, nil
 	})
 	require.NoError(t, err)
+
+	sentinelErr := fmt.Errorf("sentinel error")
+	err = m.Walk(ctx, nil, func(key, value uint64) (stop bool, err error) {
+		require.LessOrEqual(t, key, uint64(3)) // asserts that after the number three we stop
+		if key == 3 {
+			return false, sentinelErr
+		}
+		return false, nil
+	})
+	require.ErrorIs(t, err, sentinelErr) // asserts correct error propagation
 }

collections/keyset.go

@@ -53,8 +53,8 @@ func (k KeySet[K]) IterateRaw(ctx context.Context, start, end []byte, order Orde
 
 // Walk provides the same functionality as Map.Walk, but callbacks the walk
 // function only with the key.
-func (k KeySet[K]) Walk(ctx context.Context, ranger Ranger[K], walkFunc func(key K) bool) error {
-	return (Map[K, NoValue])(k).Walk(ctx, ranger, func(key K, value NoValue) bool { return walkFunc(key) })
+func (k KeySet[K]) Walk(ctx context.Context, ranger Ranger[K], walkFunc func(key K) (stop bool, err error)) error {
+	return (Map[K, NoValue])(k).Walk(ctx, ranger, func(key K, value NoValue) (bool, error) { return walkFunc(key) })
 }
 
 func (k KeySet[K]) KeyCodec() codec.KeyCodec[K]           { return (Map[K, NoValue])(k).KeyCodec() }

collections/map.go

@@ -127,7 +127,7 @@ func (m Map[K, V]) Iterate(ctx context.Context, ranger Ranger[K]) (Iterator[K, V
 // walk function with the decoded key and value. If the callback function
 // returns true then the walking is stopped.
 // A nil ranger equals to walking over the entire key and value set.
-func (m Map[K, V]) Walk(ctx context.Context, ranger Ranger[K], walkFunc func(K, V) bool) error {
+func (m Map[K, V]) Walk(ctx context.Context, ranger Ranger[K], walkFunc func(key K, value V) (stop bool, err error)) error {
 	iter, err := m.Iterate(ctx, ranger)
 	if err != nil {
 		return err
@@ -139,7 +139,11 @@ func (m Map[K, V]) Walk(ctx context.Context, ranger Ranger[K], walkFunc func(K,
 		if err != nil {
 			return err
 		}
-		if walkFunc(kv.Key, kv.Value) {
+		stop, err := walkFunc(kv.Key, kv.Value)
+		if err != nil {
+			return err
+		}
+		if stop {
 			return nil
 		}
 	}

tools/confix/go.mod

@@ -155,8 +155,6 @@ require (
 	sigs.k8s.io/yaml v1.3.0 // indirect
 )
 
-replace cosmossdk.io/collections => ../../collections
-
 // Fix upstream GHSA-h395-qcrw-5vmq and GHSA-3vp4-m3rf-835h vulnerabilities.
 // TODO Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409
 // TODO investigate if we can outright delete this dependency, otherwise go install won't work :(

tools/confix/go.sum

@@ -37,6 +37,8 @@ cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9
 cloud.google.com/go/storage v1.14.0/go.mod h1:GrKmX003DSIwi9o29oFT7YDnHYwZoctc3fOKtUw0Xmo=
 cosmossdk.io/api v0.4.1 h1:0ikaYM6GyxTYYcfBiyR8YnLCfhNnhKpEFnaSepCTmqg=
 cosmossdk.io/api v0.4.1/go.mod h1:jR7k5ok90LxW2lFUXvd8Vpo/dr4PpiyVegxdm7b1ZdE=
+cosmossdk.io/collections v0.1.0 h1:nzJGeiq32KnZroSrhB6rPifw4I85Cgmzw/YAmr4luv8=
+cosmossdk.io/collections v0.1.0/go.mod h1:xbauc0YsbUF8qKMVeBZl0pFCunxBIhKN/WlxpZ3lBuo=
 cosmossdk.io/core v0.7.0 h1:GFss3qt2P9p23Cz24NnqLkslzb8n+B75A24x1JgJJp0=
 cosmossdk.io/core v0.7.0/go.mod h1:36hP0ZH/8ipsjzfcp0yKU4bqQXUGhS0/m1krWFCtwCc=
 cosmossdk.io/depinject v1.0.0-alpha.3 h1:6evFIgj//Y3w09bqOUOzEpFj5tsxBqdc5CfkO7z+zfw=

x/bank/keeper/keeper.go

@@ -2,8 +2,11 @@ package keeper
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
+	"cosmossdk.io/collections"
+
 	"cosmossdk.io/core/store"
 	"cosmossdk.io/log"
 	"cosmossdk.io/math"
@@ -256,9 +259,12 @@ func (k BaseKeeper) GetAllDenomMetaData(ctx context.Context) []types.Metadata {
 // provides the metadata to a callback. If true is returned from the
 // callback, iteration is halted.
 func (k BaseKeeper) IterateAllDenomMetaData(ctx context.Context, cb func(types.Metadata) bool) {
-	_ = k.BaseViewKeeper.DenomMetadata.Walk(ctx, nil, func(_ string, metadata types.Metadata) bool {
-		return cb(metadata)
+	err := k.BaseViewKeeper.DenomMetadata.Walk(ctx, nil, func(_ string, metadata types.Metadata) (stop bool, err error) {
+		return cb(metadata), nil
 	})
+	if err != nil && !errors.Is(err, collections.ErrInvalidIterator) {
+		panic(err)
+	}
 }
 
 // SetDenomMetaData sets the denominations metadata
@@ -474,7 +480,10 @@ func (k BaseKeeper) trackUndelegation(ctx context.Context, addr sdk.AccAddress,
 // with the balance of each coin.
 // The iteration stops if the callback returns true.
 func (k BaseViewKeeper) IterateTotalSupply(ctx context.Context, cb func(sdk.Coin) bool) {
-	_ = k.Supply.Walk(ctx, nil, func(s string, m math.Int) bool {
-		return cb(sdk.NewCoin(s, m))
+	err := k.Supply.Walk(ctx, nil, func(s string, m math.Int) (bool, error) {
+		return cb(sdk.NewCoin(s, m)), nil
 	})
+	if err != nil && !errors.Is(err, collections.ErrInvalidIterator) {
+		panic(err)
+	}
 }

x/bank/keeper/send.go

@@ -376,7 +376,12 @@ func (k BaseSendKeeper) DeleteSendEnabled(ctx context.Context, denoms ...string)
 
 // IterateSendEnabledEntries iterates over all the SendEnabled entries.
 func (k BaseSendKeeper) IterateSendEnabledEntries(ctx context.Context, cb func(denom string, sendEnabled bool) bool) {
-	_ = k.SendEnabled.Walk(ctx, nil, cb)
+	err := k.SendEnabled.Walk(ctx, nil, func(key string, value bool) (stop bool, err error) {
+		return cb(key, value), nil
+	})
+	if err != nil && !errorsmod.IsOf(err, collections.ErrInvalidIterator) {
+		panic(err)
+	}
 }
 
 // GetAllSendEnabledEntries gets all the SendEnabled entries that are stored.

x/bank/keeper/view.go

@@ -157,10 +157,10 @@ func (k BaseViewKeeper) GetBalance(ctx context.Context, addr sdk.AccAddress, den
 // provides the token balance to a callback. If true is returned from the
 // callback, iteration is halted.
 func (k BaseViewKeeper) IterateAccountBalances(ctx context.Context, addr sdk.AccAddress, cb func(sdk.Coin) bool) {
-	err := k.Balances.Walk(ctx, collections.NewPrefixedPairRange[sdk.AccAddress, string](addr), func(key collections.Pair[sdk.AccAddress, string], value math.Int) bool {
-		return cb(sdk.NewCoin(key.K2(), value))
+	err := k.Balances.Walk(ctx, collections.NewPrefixedPairRange[sdk.AccAddress, string](addr), func(key collections.Pair[sdk.AccAddress, string], value math.Int) (stop bool, err error) {
+		return cb(sdk.NewCoin(key.K2(), value)), nil
 	})
-	if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { // TODO(tip): is this the correct strategy
+	if err != nil && !errors.Is(err, collections.ErrInvalidIterator) {
 		panic(err)
 	}
 }
@@ -169,10 +169,10 @@ func (k BaseViewKeeper) IterateAccountBalances(ctx context.Context, addr sdk.Acc
 // denominations that are provided to a callback. If true is returned from the
 // callback, iteration is halted.
 func (k BaseViewKeeper) IterateAllBalances(ctx context.Context, cb func(sdk.AccAddress, sdk.Coin) bool) {
-	err := k.Balances.Walk(ctx, nil, func(key collections.Pair[sdk.AccAddress, string], value math.Int) bool {
-		return cb(key.K1(), sdk.NewCoin(key.K2(), value))
+	err := k.Balances.Walk(ctx, nil, func(key collections.Pair[sdk.AccAddress, string], value math.Int) (stop bool, err error) {
+		return cb(key.K1(), sdk.NewCoin(key.K2(), value)), nil
 	})
-	if err != nil {
+	if err != nil && !errors.Is(err, collections.ErrInvalidIterator) {
 		panic(err)
 	}
 }

x/gov/genesis.go

@@ -34,7 +34,10 @@ func InitGenesis(ctx sdk.Context, ak types.AccountKeeper, bk types.BankKeeper, k
 
 	var totalDeposits sdk.Coins
 	for _, deposit := range data.Deposits {
-		k.SetDeposit(ctx, *deposit)
+		err := k.SetDeposit(ctx, *deposit)
+		if err != nil {
+			panic(err)
+		}
 		totalDeposits = totalDeposits.Add(deposit.Amount...)
 	}