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: add sdk.LogDeferred to report errors in defers #13619

Merged
merged 15 commits into from
Oct 26, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* [#13619](https://github.com/cosmos/cosmos-sdk/pull/13619) Add new function called LogDeferred to report errors in defers. Use the function in x/bank files.
* (tools) [#13603](https://github.com/cosmos/cosmos-sdk/pull/13603) Rename cosmovisor package name to `cosmossdk.io/tools/cosmovisor`. The new tool directory contains Cosmos SDK tools.
* (deps) [#13397](https://github.com/cosmos/cosmos-sdk/pull/13397) Bump Go version minimum requirement to `1.19`.
* [#13070](https://github.com/cosmos/cosmos-sdk/pull/13070) Migrate from `gogo/protobuf` to `cosmos/gogoproto`.
Expand Down
4 changes: 3 additions & 1 deletion internal/util.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package util

import "fmt"
import (
"fmt"
)

func CombineErrors(ret error, also error, desc string) error {
if also != nil {
Expand Down
8 changes: 8 additions & 0 deletions types/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/cosmos/cosmos-sdk/types/kv"
log "github.com/tendermint/tendermint/libs/log"
)

// SortedJSON takes any JSON and returns it sorted by keys. Also, all white-spaces
Expand Down Expand Up @@ -133,3 +134,10 @@ func ParseLengthPrefixedBytes(key []byte, startIndex int, sliceLength int) ([]by

return byteSlice, endIndex
}

// LogDeferred logs an error in a deferred function call if the returned error is non-nil.
func LogDeferred(logger log.Logger, f func() error) {
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
if err := f(); err != nil {
logger.Error(err.Error())
}
}
4 changes: 2 additions & 2 deletions x/bank/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func (k BaseKeeper) IterateAllDenomMetaData(ctx sdk.Context, cb func(types.Metad
denomMetaDataStore := prefix.NewStore(store, types.DenomMetadataPrefix)

iterator := denomMetaDataStore.Iterator(nil, nil)
defer iterator.Close()
defer sdk.LogDeferred(ctx.Logger(), func() error { return iterator.Close() })

for ; iterator.Valid(); iterator.Next() {
var metadata types.Metadata
Expand Down Expand Up @@ -527,7 +527,7 @@ func (k BaseViewKeeper) IterateTotalSupply(ctx sdk.Context, cb func(sdk.Coin) bo
supplyStore := prefix.NewStore(store, types.SupplyKey)

iterator := supplyStore.Iterator(nil, nil)
defer iterator.Close()
defer sdk.LogDeferred(ctx.Logger(), func() error { return iterator.Close() })

for ; iterator.Valid(); iterator.Next() {
var amount math.Int
Expand Down
2 changes: 1 addition & 1 deletion x/bank/keeper/send.go
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ func (k BaseSendKeeper) IterateSendEnabledEntries(ctx sdk.Context, cb func(denom
seStore := k.getSendEnabledPrefixStore(ctx)

iterator := seStore.Iterator(nil, nil)
defer iterator.Close()
defer sdk.LogDeferred(ctx.Logger(), func() error { return iterator.Close() })

for ; iterator.Valid(); iterator.Next() {
denom := string(iterator.Key())
Expand Down
2 changes: 1 addition & 1 deletion x/bank/keeper/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func (k BaseViewKeeper) IterateAccountBalances(ctx sdk.Context, addr sdk.AccAddr
accountStore := k.getAccountStore(ctx, addr)

iterator := accountStore.Iterator(nil, nil)
defer iterator.Close()
defer sdk.LogDeferred(ctx.Logger(), func() error { return iterator.Close() })

for ; iterator.Valid(); iterator.Next() {
denom := string(iterator.Key())
Expand Down
7 changes: 4 additions & 3 deletions x/bank/migrations/v2/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
v042auth "github.com/cosmos/cosmos-sdk/x/auth/migrations/v042"
v1 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v1"
"github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/tendermint/tendermint/libs/log"
)

// migrateSupply migrates the supply to be stored by denom key instead in a
Expand Down Expand Up @@ -50,15 +51,15 @@ func migrateSupply(store sdk.KVStore, cdc codec.BinaryCodec) error {

// migrateBalanceKeys migrate the balances keys to cater for variable-length
// addresses.
func migrateBalanceKeys(store sdk.KVStore) {
func migrateBalanceKeys(store sdk.KVStore, logger log.Logger) {
// old key is of format:
// prefix ("balances") || addrBytes (20 bytes) || denomBytes
// new key is of format
// prefix (0x02) || addrLen (1 byte) || addrBytes || denomBytes
oldStore := prefix.NewStore(store, v1.BalancesPrefix)

oldStoreIter := oldStore.Iterator(nil, nil)
defer oldStoreIter.Close()
defer sdk.LogDeferred(logger, func() error { return oldStoreIter.Close() })

for ; oldStoreIter.Valid(); oldStoreIter.Next() {
addr := v1.AddressFromBalancesStore(oldStoreIter.Key())
Expand All @@ -80,7 +81,7 @@ func migrateBalanceKeys(store sdk.KVStore) {
// - Prune balances & supply with zero coins (ref: https://github.com/cosmos/cosmos-sdk/pull/9229)
func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error {
store := ctx.KVStore(storeKey)
migrateBalanceKeys(store)
migrateBalanceKeys(store, ctx.Logger())

if err := pruneZeroBalances(store, cdc); err != nil {
return err
Expand Down
13 changes: 7 additions & 6 deletions x/bank/migrations/v3/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/cosmos/cosmos-sdk/types/address"
v2 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v2"
"github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/tendermint/tendermint/libs/log"
)

// MigrateStore performs in-place store migrations from v0.43 to v0.45. The
Expand All @@ -18,19 +19,19 @@ import (
// - Remove duplicate denom from denom metadata store key.
func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error {
store := ctx.KVStore(storeKey)
err := addDenomReverseIndex(store, cdc)
err := addDenomReverseIndex(store, cdc, ctx.Logger())
if err != nil {
return err
}

return migrateDenomMetadata(store)
return migrateDenomMetadata(store, ctx.Logger())
}

func addDenomReverseIndex(store sdk.KVStore, cdc codec.BinaryCodec) error {
func addDenomReverseIndex(store sdk.KVStore, cdc codec.BinaryCodec, logger log.Logger) error {
oldBalancesStore := prefix.NewStore(store, v2.BalancesPrefix)

oldBalancesIter := oldBalancesStore.Iterator(nil, nil)
defer oldBalancesIter.Close()
defer sdk.LogDeferred(logger, func() error { return oldBalancesIter.Close() })

denomPrefixStores := make(map[string]prefix.Store) // memoize prefix stores

Expand Down Expand Up @@ -72,11 +73,11 @@ func addDenomReverseIndex(store sdk.KVStore, cdc codec.BinaryCodec) error {
return nil
}

func migrateDenomMetadata(store sdk.KVStore) error {
func migrateDenomMetadata(store sdk.KVStore, logger log.Logger) error {
oldDenomMetaDataStore := prefix.NewStore(store, v2.DenomMetadataPrefix)

oldDenomMetaDataIter := oldDenomMetaDataStore.Iterator(nil, nil)
defer oldDenomMetaDataIter.Close()
defer sdk.LogDeferred(logger, func() error { return oldDenomMetaDataIter.Close() })

for ; oldDenomMetaDataIter.Valid(); oldDenomMetaDataIter.Next() {
oldKey := oldDenomMetaDataIter.Key()
Expand Down