Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 42 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
dddd529
move constitution to collections
May 11, 2023
3d8306b
move params to collections
May 11, 2023
e559d41
chore: CHANGELOG.md
May 11, 2023
41d8e73
address PR reviews
May 11, 2023
6bb70e6
be consistent
May 12, 2023
c503920
more test fixes
May 12, 2023
9bf4d8c
Merge branch 'main' into tip/gov/coll-params-constitution
testinginprod May 12, 2023
0cd34ed
use assert and not require
May 12, 2023
8fc50d7
Merge remote-tracking branch 'origin/tip/gov/coll-params-constitution…
May 12, 2023
915618b
fix prefixes numbers
May 12, 2023
d291e99
move deposit to collections1
May 12, 2023
6932323
move another part of deposits to use collections
May 12, 2023
0792749
Merge branch 'main' into tip/gov/coll-deposit
May 12, 2023
a9cc7ba
add panic in genesis
May 12, 2023
f29f12e
remove unused func
May 12, 2023
4e53600
more cleanups + format
May 12, 2023
99066f3
remove improt
May 12, 2023
b23e38d
change the collections API to error on walking
May 13, 2023
6af4aa9
chore: CHANGELOG.md
May 13, 2023
5191de7
lint bank
May 13, 2023
4f9bea7
remove unused error
May 13, 2023
3ee570e
remove unused errors
May 13, 2023
64bf12f
fix some minor thing
May 13, 2023
39c3c4a
Merge branch 'main' into tip/gov/coll-deposit
testinginprod May 13, 2023
a4fe45c
more minor things
May 13, 2023
66b7866
remove another method yet again
May 13, 2023
730116e
fmt
May 13, 2023
3a04628
Merge branch 'main' into tip/gov/coll-deposit
testinginprod May 15, 2023
44906b0
chore: changelog
May 15, 2023
abbe962
Merge remote-tracking branch 'origin/tip/gov/coll-deposit' into tip/g…
May 15, 2023
a69ed21
supress lint
May 15, 2023
9aa0be4
fix lint maybe
May 15, 2023
21efcb5
fix lint 2
May 15, 2023
c295c8f
Merge branch 'main' into tip/gov/coll-deposit
testinginprod May 15, 2023
7f8b066
go mod tidy all
May 15, 2023
667c4e3
Merge remote-tracking branch 'origin/tip/gov/coll-deposit' into tip/g…
May 15, 2023
2830f8a
Merge branch 'main' into tip/gov/coll-deposit
testinginprod May 15, 2023
72dd01b
remove dependency on another error pkg
May 15, 2023
d400342
Merge remote-tracking branch 'origin/tip/gov/coll-deposit' into tip/g…
May 15, 2023
98195ea
tidy again
May 15, 2023
f5eb34c
try fix confix
May 15, 2023
b60262f
Merge branch 'main' into tip/gov/coll-deposit
testinginprod May 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions collections/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
2 changes: 1 addition & 1 deletion collections/indexed_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
4 changes: 2 additions & 2 deletions collections/indexes/multi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
}
Expand Down
4 changes: 2 additions & 2 deletions collections/indexes/reverse_pair.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
}
Expand Down
2 changes: 1 addition & 1 deletion collections/indexes/unique.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
16 changes: 13 additions & 3 deletions collections/iter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 uint64, 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 uint64, 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
}
4 changes: 2 additions & 2 deletions collections/keyset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
Expand Down
8 changes: 6 additions & 2 deletions collections/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added the possibility to make walkFunc error, this plays well with gov and generally is better to allow for a straightforward way to error during iterations and propagate errors back.

iter, err := m.Iterate(ctx, ranger)
if err != nil {
return err
Expand All @@ -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
}
}
Expand Down
16 changes: 12 additions & 4 deletions x/bank/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package keeper

import (
"context"
"cosmossdk.io/collections"
"errors"
"fmt"

"cosmossdk.io/core/store"
Expand Down Expand Up @@ -256,9 +258,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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adjusts to match the collections API breaking change.

return cb(metadata), nil
})
if err != nil && !errors.Is(err, collections.ErrInvalidIterator) {
panic(err)
}
}

// SetDenomMetaData sets the denominations metadata
Expand Down Expand Up @@ -474,7 +479,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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adjusts to match the collections API breaking change.

return cb(sdk.NewCoin(s, m)), nil
})
if err != nil && !errors.Is(err, collections.ErrInvalidIterator) {
panic(err)
}
}
8 changes: 7 additions & 1 deletion x/bank/keeper/send.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package keeper
import (
"context"
"fmt"
"github.com/pkg/errors"
testinginprod marked this conversation as resolved.
Show resolved Hide resolved

"cosmossdk.io/collections"
"cosmossdk.io/core/store"
Expand Down Expand Up @@ -376,7 +377,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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adjusts to match the collections API breaking change.

_ = 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 && errors.Is(err, collections.ErrInvalidIterator) {
panic(err)
}
}

// GetAllSendEnabledEntries gets all the SendEnabled entries that are stored.
Expand Down
10 changes: 5 additions & 5 deletions x/bank/keeper/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ 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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adjusts to match the collections API breaking change.

return cb(sdk.NewCoin(key.K2(), value)), nil
})
if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { // TODO(tip): is this the correct strategy
panic(err)
Expand All @@ -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)
}
}
Expand Down
5 changes: 4 additions & 1 deletion x/gov/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}

Expand Down
Loading