Skip to content

Commit 2c43a57

Browse files
SpicyLemonSpicyLemon
authored
perf(x/bank): Improve performance of GetAllBalances and GetAccountsBalances. (#24660)
1 parent 30eaefc commit 2c43a57

File tree

3 files changed

+212
-10
lines changed

3 files changed

+212
-10
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
4747
* (types) [#24668](https://github.com/cosmos/cosmos-sdk/pull/24668) Scope the global config to a particular binary so that multiple SDK binaries can be properly run on the same machine.
4848
* (baseapp) [#24655](https://github.com/cosmos/cosmos-sdk/pull/24655) Add mutex locks for `state` and make `lastCommitInfo` atomic to prevent race conditions between `Commit` and `CreateQueryContext`.
4949
* (proto) [#24161](https://github.com/cosmos/cosmos-sdk/pull/24161) Remove unnecessary annotations from `x/staking` authz proto.
50+
* (x/bank) [#24660](https://github.com/cosmos/cosmos-sdk/pull/24660) Improve performance of the `GetAllBalances` and `GetAccountsBalances` keeper methods.
5051

5152
### Bug Fixes
5253

‎x/bank/keeper/keeper_test.go‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package keeper_test
22

33
import (
4+
"bytes"
45
"context"
56
"crypto/sha256"
67
"encoding/hex"
78
"errors"
89
"fmt"
10+
"slices"
911
"strings"
1012
"testing"
1113
"time"
@@ -2474,6 +2476,207 @@ func (suite *KeeperTestSuite) TestGetAllSendEnabledEntries() {
24742476
})
24752477
}
24762478

2479+
func (suite *KeeperTestSuite) TestGetAllBalances() {
2480+
addr0 := sdk.AccAddress("addr_0______________")
2481+
addr1 := sdk.AccAddress("addr_1______________")
2482+
addr2 := sdk.AccAddress("addr_2______________")
2483+
addr5 := sdk.AccAddress("addr_5______________")
2484+
addr20 := sdk.AccAddress("addr_20_____________")
2485+
2486+
bal0 := sdk.Coins(nil)
2487+
bal1 := sdk.NewCoins(sdk.NewInt64Coin("one", 111))
2488+
bal2 := sdk.NewCoins(sdk.NewInt64Coin("one", 222), sdk.NewInt64Coin("two", 234))
2489+
bal5 := sdk.NewCoins(
2490+
sdk.NewInt64Coin("one", 37), sdk.NewInt64Coin("two", 3), sdk.NewInt64Coin("three", 399),
2491+
sdk.NewInt64Coin("four", 3456), sdk.NewInt64Coin("five", 321),
2492+
)
2493+
bal20 := sdk.NewCoins(
2494+
sdk.NewInt64Coin("one", 4987), sdk.NewInt64Coin("two", 444), sdk.NewInt64Coin("three", 41),
2495+
sdk.NewInt64Coin("four", 442), sdk.NewInt64Coin("five", 443), sdk.NewInt64Coin("six", 46622),
2496+
sdk.NewInt64Coin("seven", 487), sdk.NewInt64Coin("eight", 4), sdk.NewInt64Coin("nine", 4991),
2497+
sdk.NewInt64Coin("ten", 40000000), sdk.NewInt64Coin("eleven", 4545), sdk.NewInt64Coin("twelve", 41212),
2498+
sdk.NewInt64Coin("thirteen", 41333), sdk.NewInt64Coin("fourteen", 401414), sdk.NewInt64Coin("fifteen", 47),
2499+
sdk.NewInt64Coin("sixteen", 400016), sdk.NewInt64Coin("seventeen", 404), sdk.NewInt64Coin("eighteen", 4454),
2500+
sdk.NewInt64Coin("ninteen", 41298), sdk.NewInt64Coin("twenty", 456789),
2501+
)
2502+
2503+
bals := []struct {
2504+
Addr sdk.AccAddress
2505+
Coins sdk.Coins
2506+
}{
2507+
{Addr: addr0, Coins: bal0},
2508+
{Addr: addr1, Coins: bal1},
2509+
{Addr: addr2, Coins: bal2},
2510+
{Addr: addr5, Coins: bal5},
2511+
{Addr: addr20, Coins: bal20},
2512+
}
2513+
2514+
for _, bal := range bals {
2515+
if bal.Coins.IsZero() {
2516+
continue
2517+
}
2518+
2519+
suite.mockMintCoins(minterAcc)
2520+
suite.Require().NoError(suite.bankKeeper.MintCoins(suite.ctx, authtypes.Minter, bal.Coins),
2521+
"minting %s for %s", bal.Coins, string(bal.Addr))
2522+
suite.mockSendCoinsFromModuleToAccount(minterAcc, bal.Addr)
2523+
suite.Require().NoError(
2524+
suite.bankKeeper.SendCoinsFromModuleToAccount(suite.ctx, authtypes.Minter, bal.Addr, bal.Coins),
2525+
"sending freshly minted %s to %s", bal.Coins, string(bal.Addr),
2526+
)
2527+
}
2528+
2529+
tests := []struct {
2530+
name string
2531+
addr sdk.AccAddress
2532+
exp sdk.Coins
2533+
}{
2534+
{name: "nil addr", addr: nil, exp: nil},
2535+
{name: "empty addr", addr: sdk.AccAddress{}, exp: nil},
2536+
{name: "addr without anything", addr: addr0, exp: bal0},
2537+
{name: "addr with one denom", addr: addr1, exp: bal1},
2538+
{name: "addr with two denom", addr: addr2, exp: bal2},
2539+
{name: "addr with five denom", addr: addr5, exp: bal5},
2540+
{name: "addr with twenty denom", addr: addr20, exp: bal20},
2541+
}
2542+
2543+
count := 100
2544+
for _, tc := range tests {
2545+
suite.Run(tc.name, func() {
2546+
var act sdk.Coins
2547+
testFunc := func() {
2548+
act = suite.bankKeeper.GetAllBalances(suite.ctx, tc.addr)
2549+
}
2550+
for i := 1; i <= count; i++ {
2551+
suite.Require().NotPanics(testFunc, "[%d/%d]: GetAllBalances", i, count)
2552+
if !suite.Assert().Equal(tc.exp.String(), act.String(), "[%d/%d]: GetAllBalances result", i, count) && i == 1 {
2553+
// If it fails on the first one, stop now since that probably means they'll all fail.
2554+
// If it's not the first, keep going so it's easier to see that it's not deterministic.
2555+
break
2556+
}
2557+
actSorted := copyAndSortCoins(act)
2558+
suite.Assert().Equal(actSorted, act,
2559+
"[%d/%d]: GetAllBalances result is not sorted.\nexpected: %s\nactual : %s",
2560+
i, count, actSorted, act)
2561+
}
2562+
})
2563+
}
2564+
}
2565+
2566+
func (suite *KeeperTestSuite) TestGetAccountsBalances() {
2567+
addrNames := make(map[string]string)
2568+
balsToStrings := func(bals []banktypes.Balance) []string {
2569+
if bals == nil {
2570+
return nil
2571+
}
2572+
rv := make([]string, len(bals))
2573+
for i, bal := range bals {
2574+
rv[i] = fmt.Sprintf("%s (%s) = %s", bal.Address, addrNames[bal.Address], bal.Coins)
2575+
}
2576+
return rv
2577+
}
2578+
2579+
addrA := sdk.AccAddress("addr_a______________")
2580+
addrB := sdk.AccAddress("addr_b______________")
2581+
addrC := sdk.AccAddress("addr_c______________")
2582+
addrD := sdk.AccAddress("addr_d______________")
2583+
addrE := sdk.AccAddress("addr_e______________")
2584+
addrF := sdk.AccAddress("addr_f______________")
2585+
addrG := sdk.AccAddress("addr_g______________")
2586+
2587+
balA := sdk.NewCoins(sdk.NewInt64Coin("one", 111))
2588+
balB := sdk.NewCoins(
2589+
sdk.NewInt64Coin("one", 4987), sdk.NewInt64Coin("two", 444), sdk.NewInt64Coin("three", 41),
2590+
sdk.NewInt64Coin("four", 442), sdk.NewInt64Coin("five", 443), sdk.NewInt64Coin("six", 46622),
2591+
sdk.NewInt64Coin("seven", 487), sdk.NewInt64Coin("eight", 4), sdk.NewInt64Coin("nine", 4991),
2592+
sdk.NewInt64Coin("ten", 40000000), sdk.NewInt64Coin("eleven", 4545), sdk.NewInt64Coin("twelve", 41212),
2593+
sdk.NewInt64Coin("thirteen", 41333), sdk.NewInt64Coin("fourteen", 401414), sdk.NewInt64Coin("fifteen", 47),
2594+
sdk.NewInt64Coin("sixteen", 400016), sdk.NewInt64Coin("seventeen", 404), sdk.NewInt64Coin("eighteen", 4454),
2595+
sdk.NewInt64Coin("ninteen", 41298), sdk.NewInt64Coin("twenty", 456789),
2596+
)
2597+
balC := sdk.NewCoins(sdk.NewInt64Coin("one", 222), sdk.NewInt64Coin("two", 234))
2598+
balD := sdk.Coins(nil)
2599+
balE := sdk.NewCoins(sdk.NewInt64Coin("banana", 99), sdk.NewInt64Coin("six", 789))
2600+
balF := sdk.NewCoins(
2601+
sdk.NewInt64Coin("one", 37), sdk.NewInt64Coin("two", 3), sdk.NewInt64Coin("three", 399),
2602+
sdk.NewInt64Coin("four", 3456), sdk.NewInt64Coin("five", 321),
2603+
)
2604+
balG := sdk.NewCoins(sdk.NewInt64Coin("one", 543))
2605+
2606+
type addrCoins struct {
2607+
Addr sdk.AccAddress
2608+
Coins sdk.Coins
2609+
}
2610+
2611+
bals := []addrCoins{
2612+
{Addr: addrA, Coins: balA},
2613+
{Addr: addrB, Coins: balB},
2614+
{Addr: addrC, Coins: balC},
2615+
{Addr: addrD, Coins: balD},
2616+
{Addr: addrE, Coins: balE},
2617+
{Addr: addrF, Coins: balF},
2618+
{Addr: addrG, Coins: balG},
2619+
}
2620+
slices.SortFunc(bals, func(a, b addrCoins) int {
2621+
return bytes.Compare(a.Addr, b.Addr)
2622+
})
2623+
2624+
expBals := make([]banktypes.Balance, 0, len(bals))
2625+
2626+
for _, bal := range bals {
2627+
addrNames[bal.Addr.String()] = string(bal.Addr)
2628+
if bal.Coins.IsZero() {
2629+
continue
2630+
}
2631+
2632+
suite.mockMintCoins(minterAcc)
2633+
suite.Require().NoError(suite.bankKeeper.MintCoins(suite.ctx, authtypes.Minter, bal.Coins),
2634+
"minting %s for %s", bal.Coins, string(bal.Addr))
2635+
suite.mockSendCoinsFromModuleToAccount(minterAcc, bal.Addr)
2636+
suite.Require().NoError(
2637+
suite.bankKeeper.SendCoinsFromModuleToAccount(suite.ctx, authtypes.Minter, bal.Addr, bal.Coins),
2638+
"sending freshly minted %s to %s", bal.Coins, string(bal.Addr),
2639+
)
2640+
2641+
expBals = append(expBals, banktypes.Balance{Address: bal.Addr.String(), Coins: bal.Coins})
2642+
}
2643+
2644+
expStrs := balsToStrings(expBals)
2645+
suite.Require().NotEmpty(expStrs, "The expected balance strings should not be empty after setup.")
2646+
2647+
var actBals []banktypes.Balance
2648+
testFunc := func() {
2649+
actBals = suite.bankKeeper.GetAccountsBalances(suite.ctx)
2650+
}
2651+
2652+
count := 1000
2653+
for i := 1; i <= count; i++ {
2654+
suite.Require().NotPanics(testFunc, "[%d/%d]: GetAccountsBalances", i, count)
2655+
actStrs := balsToStrings(actBals)
2656+
if !suite.Assert().Equal(expStrs, actStrs, "[%d/%d]: GetAccountsBalances result", i, count) && i == 1 {
2657+
// If it fails on the first one, stop now since that probably means it'll always fail.
2658+
// If it's not the first, keep going so it's easier to see that it's not deterministic.
2659+
break
2660+
}
2661+
for b, actBal := range actBals {
2662+
actBalSorted := copyAndSortCoins(actBal.Coins)
2663+
suite.Assert().Equal(actBalSorted, actBal.Coins,
2664+
"[%d/%d]: Balance[%d] is not sorted.\nexpected: %s\nactual : %s",
2665+
i, count, b, actBalSorted, actBal.Coins)
2666+
}
2667+
}
2668+
}
2669+
2670+
// copyAndSortCoins returns a copy of the provided coins slice and sorts it.
2671+
func copyAndSortCoins(coins sdk.Coins) sdk.Coins {
2672+
if coins == nil {
2673+
return nil
2674+
}
2675+
rv := make(sdk.Coins, len(coins))
2676+
copy(rv, coins)
2677+
return rv.Sort()
2678+
}
2679+
24772680
type mockSubspace struct {
24782681
ps banktypes.Params
24792682
}

‎x/bank/keeper/view.go‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -106,33 +106,31 @@ func (k BaseViewKeeper) Logger() log.Logger {
106106
func (k BaseViewKeeper) GetAllBalances(ctx context.Context, addr sdk.AccAddress) sdk.Coins {
107107
balances := sdk.NewCoins()
108108
k.IterateAccountBalances(ctx, addr, func(balance sdk.Coin) bool {
109-
balances = balances.Add(balance)
109+
balances = append(balances, balance)
110110
return false
111111
})
112112

113-
return balances.Sort()
113+
return balances
114114
}
115115

116116
// GetAccountsBalances returns all the accounts balances from the store.
117117
func (k BaseViewKeeper) GetAccountsBalances(ctx context.Context) []types.Balance {
118118
balances := make([]types.Balance, 0)
119-
mapAddressToBalancesIdx := make(map[string]int)
120119

121120
k.IterateAllBalances(ctx, func(addr sdk.AccAddress, balance sdk.Coin) bool {
122-
idx, ok := mapAddressToBalancesIdx[addr.String()]
123-
if ok {
124-
// address is already on the set of accounts balances
125-
balances[idx].Coins = balances[idx].Coins.Add(balance)
126-
balances[idx].Coins.Sort()
121+
addrStr := addr.String()
122+
if len(balances) > 0 && balances[len(balances)-1].Address == addrStr {
123+
// Same address as last entry = add the coin to it.
124+
balances[len(balances)-1].Coins = append(balances[len(balances)-1].Coins, balance)
127125
return false
128126
}
129127

128+
// New address = new entry.
130129
accountBalance := types.Balance{
131-
Address: addr.String(),
130+
Address: addrStr,
132131
Coins: sdk.NewCoins(balance),
133132
}
134133
balances = append(balances, accountBalance)
135-
mapAddressToBalancesIdx[addr.String()] = len(balances) - 1
136134
return false
137135
})
138136

0 commit comments

Comments
 (0)