From 17eed8c132cbf75ce00031ed01d9ed46337f4673 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 9 Apr 2023 01:33:28 +0200 Subject: [PATCH 1/9] Add a map coins struct to speedup bank genesis --- types/mapcoins.go | 41 ++++++++++++++++++++++++++++++++++++++++ types/mapcoins_test.go | 26 +++++++++++++++++++++++++ x/bank/keeper/genesis.go | 5 +++-- 3 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 types/mapcoins.go create mode 100644 types/mapcoins_test.go diff --git a/types/mapcoins.go b/types/mapcoins.go new file mode 100644 index 000000000000..ed8720f95c4e --- /dev/null +++ b/types/mapcoins.go @@ -0,0 +1,41 @@ +package types + +// map coins is a map representation of sdk.Coins +// intended solely for use in bulk additions. +// All serialization and iteration should be done after conversion to sdk.Coins. +type MapCoins map[string]Int + +func NewMapCoins(coins Coins) MapCoins { + m := make(MapCoins, len(coins)) + m.Add(coins...) + return m +} + +func (m MapCoins) Add(coins ...Coin) { + for _, coin := range coins { + existAmt, exists := m[coin.Denom] + if exists { + m[coin.Denom] = existAmt.Add(coin.Amount) + } else { + m[coin.Denom] = coin.Amount + } + } +} + +func (m MapCoins) ToCoins() Coins { + if len(m) == 0 { + return nil + } + coins := make(Coins, 0, len(m)) + for denom, amount := range m { + if amount.IsZero() { + continue + } + coins = append(coins, NewCoin(denom, amount)) + } + coins.Sort() + if len(coins) == 0 { + return nil + } + return coins +} diff --git a/types/mapcoins_test.go b/types/mapcoins_test.go new file mode 100644 index 000000000000..71f95840968c --- /dev/null +++ b/types/mapcoins_test.go @@ -0,0 +1,26 @@ +package types_test + +import sdk "github.com/cosmos/cosmos-sdk/types" + +func (s *coinTestSuite) TestMapCoinAdd() { + cases := []struct { + inputOne sdk.Coins + inputTwo sdk.Coins + }{ + {sdk.Coins{s.ca1, s.cm1}, sdk.Coins{s.ca1, s.cm1}}, + {sdk.Coins{s.ca0, s.cm1}, sdk.Coins{s.ca0, s.cm0}}, + {sdk.Coins{s.ca2}, sdk.Coins{s.cm0}}, + {sdk.Coins{s.ca1}, sdk.Coins{s.ca1, s.cm2}}, + {sdk.Coins{s.ca0, s.cm0}, sdk.Coins{s.ca0, s.cm0}}, + } + + for tcIndex, tc := range cases { + expected := tc.inputOne.Add(tc.inputTwo...) + m := sdk.NewMapCoins(tc.inputOne) + m.Add(tc.inputTwo...) + res := m.ToCoins() + s.Require().True(res.IsValid()) + s.Require().Equal(expected, res, "sum of coins is incorrect, tc #%d", tcIndex) + } + +} diff --git a/x/bank/keeper/genesis.go b/x/bank/keeper/genesis.go index e214570e5035..c17140cace19 100644 --- a/x/bank/keeper/genesis.go +++ b/x/bank/keeper/genesis.go @@ -17,8 +17,8 @@ func (k BaseKeeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) { for _, se := range genState.GetAllSendEnabled() { k.SetSendEnabled(ctx, se.Denom, se.Enabled) } + totalSupplyMap := sdk.NewMapCoins(sdk.Coins{}) - totalSupply := sdk.Coins{} genState.Balances = types.SanitizeGenesisBalances(genState.Balances) for _, balance := range genState.Balances { @@ -28,8 +28,9 @@ func (k BaseKeeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) { panic(fmt.Errorf("error on setting balances %w", err)) } - totalSupply = totalSupply.Add(balance.Coins...) + totalSupplyMap.Add(balance.Coins...) } + totalSupply := totalSupplyMap.ToCoins() if !genState.Supply.Empty() && !genState.Supply.Equal(totalSupply) { panic(fmt.Errorf("genesis supply is incorrect, expected %v, got %v", genState.Supply, totalSupply)) From 473dd615f4e97d22eccbfc55e3a2f88da79e8d3d Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 9 Apr 2023 01:36:57 +0200 Subject: [PATCH 2/9] Note a TODO --- types/mapcoins.go | 1 + 1 file changed, 1 insertion(+) diff --git a/types/mapcoins.go b/types/mapcoins.go index ed8720f95c4e..fed3f2544868 100644 --- a/types/mapcoins.go +++ b/types/mapcoins.go @@ -14,6 +14,7 @@ func NewMapCoins(coins Coins) MapCoins { func (m MapCoins) Add(coins ...Coin) { for _, coin := range coins { existAmt, exists := m[coin.Denom] + // TODO: Once int supports in-place arithmetic, switch this to be in-place. if exists { m[coin.Denom] = existAmt.Add(coin.Amount) } else { From 16c000608ed29a5dc52a2964d912e17af9d94fd8 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 9 Apr 2023 01:42:25 +0200 Subject: [PATCH 3/9] Add changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc3843c0a30f..5773f353ce80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -95,6 +95,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (baseapp) [#15023](https://github.com/cosmos/cosmos-sdk/pull/15023) & [#15213](https://github.com/cosmos/cosmos-sdk/pull/15213) Add `MessageRouter` interface to baseapp and pass it to authz, gov and groups instead of concrete type. * (simtestutil) [#15305](https://github.com/cosmos/cosmos-sdk/pull/15305) Add `AppStateFnWithExtendedCb` with callback function to extend rawState. * (x/consensus) [#15553](https://github.com/cosmos/cosmos-sdk/pull/15553) Migrate consensus module to use collections +* (x/bank) [#15764](https://github.com/cosmos/cosmos-sdk/pull/15764) Speedup x/bank InitGenesis + ### State Machine Breaking From 0c3c914b7c1c24c659edf93add41c279e7364fce Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 9 Apr 2023 11:37:43 +0200 Subject: [PATCH 4/9] Add benchmark --- types/coin_benchmark_test.go | 67 ++++++++++++++++++++++++++++++++++++ types/msgservice/validate.go | 3 +- 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/types/coin_benchmark_test.go b/types/coin_benchmark_test.go index 8d8abaf14fc3..e89526e7c610 100644 --- a/types/coin_benchmark_test.go +++ b/types/coin_benchmark_test.go @@ -70,3 +70,70 @@ func BenchmarkCoinsAdditionNoIntersect(b *testing.B) { b.Run(fmt.Sprintf("sizes: A_%d, B_%d", sizeA, sizeB), benchmarkingFunc(sizeA, sizeB)) } } + +func BenchmarkSumOfCoinAdds(b *testing.B) { + // This benchmark tests the performance of adding a large number of coins + // into a single coin set. + // it does numAdds additions, each addition has (numIntersectingCoins) that contian denoms + // already in the sum, and (coinsPerAdd - numIntersectingCoins) that are new denoms. + benchmarkingFunc := func(coinsPerAdd, numIntersectingCoins, numAdds int, sumFn func([]Coins) Coins) func(b *testing.B) { + return func(b *testing.B) { + b.ReportAllocs() + addCoins := make([]Coins, numAdds) + + for i := 0; i < numAdds; i++ { + intersectCoins := make([]Coin, numIntersectingCoins) + for j := 0; j < numIntersectingCoins; j++ { + intersectCoins[j] = NewCoin(coinName(j), NewInt(int64(i))) + } + addCoins[i] = intersectCoins + for j := 0; j < coinsPerAdd; j++ { + addCoins[i] = addCoins[i].Add(NewCoin(coinName(j), NewInt(int64(i)))) + } + } + + b.ResetTimer() + + for i := 0; i < b.N; i++ { + sum := Coins{} + for j := 0; j < numAdds; j++ { + sum = sum.Add(addCoins[j]...) + } + } + } + } + + MapCoinsSumFn := func(coins []Coins) Coins { + sum := MapCoins{} + for _, coin := range coins { + sum.Add(coin...) + } + return sum.ToCoins() + } + CoinsSumFn := func(coins []Coins) Coins { + sum := Coins{} + for _, coin := range coins { + sum = sum.Add(coin...) + } + return sum + } + + benchmarkSizes := [][]int{{5, 2, 1000}, {10, 1, 10000}, {10, 10, 10000}} + sumFns := []struct { + name string + fn func([]Coins) Coins + }{ + {"MapCoins", MapCoinsSumFn}, {"Coins", CoinsSumFn}, + } + for i := 0; i < len(benchmarkSizes); i++ { + for j := 0; j < 2; j++ { + coinsPerAdd := benchmarkSizes[i][0] + intersectingCoinsPerAdd := benchmarkSizes[i][1] + numAdds := benchmarkSizes[i][2] + sumFn := sumFns[j] + b.Run(fmt.Sprintf("Fn: %s, num adds: %d, coinsPerAdd: %d, non-interesecting: %d", + sumFn.name, numAdds, coinsPerAdd, intersectingCoinsPerAdd), + benchmarkingFunc(numAdds, coinsPerAdd, intersectingCoinsPerAdd, sumFn.fn)) + } + } +} diff --git a/types/msgservice/validate.go b/types/msgservice/validate.go index cbbed0ad9d8e..4264cb7e28cb 100644 --- a/types/msgservice/validate.go +++ b/types/msgservice/validate.go @@ -1,7 +1,6 @@ package msgservice import ( - "errors" "fmt" "google.golang.org/protobuf/proto" @@ -40,7 +39,7 @@ func ValidateProtoAnnotations(protoFiles *protoregistry.Files) error { return true }) - return errors.Join(serviceErrs...) + return serviceErrs[0] } // validateMsgServiceAnnotations validates that the service has the From cdbc23aa4896e01d3eb04c2f60fe9f4352da73d1 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 9 Apr 2023 11:46:20 +0200 Subject: [PATCH 5/9] nil vs empty struct behavior different than expected --- types/coin_benchmark_test.go | 4 +-- types/mapcoins.go | 4 +-- types/mapcoins_test.go | 53 ++++++++++++++++++++++++++++++------ 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/types/coin_benchmark_test.go b/types/coin_benchmark_test.go index e89526e7c610..d2e14cb46cae 100644 --- a/types/coin_benchmark_test.go +++ b/types/coin_benchmark_test.go @@ -88,7 +88,7 @@ func BenchmarkSumOfCoinAdds(b *testing.B) { } addCoins[i] = intersectCoins for j := 0; j < coinsPerAdd; j++ { - addCoins[i] = addCoins[i].Add(NewCoin(coinName(j), NewInt(int64(i)))) + addCoins[i] = addCoins[i].Add(NewCoin(coinName(i+j), NewInt(int64(i)))) } } @@ -131,7 +131,7 @@ func BenchmarkSumOfCoinAdds(b *testing.B) { intersectingCoinsPerAdd := benchmarkSizes[i][1] numAdds := benchmarkSizes[i][2] sumFn := sumFns[j] - b.Run(fmt.Sprintf("Fn: %s, num adds: %d, coinsPerAdd: %d, non-interesecting: %d", + b.Run(fmt.Sprintf("Fn: %s, num adds: %d, coinsPerAdd: %d, intersecting: %d", sumFn.name, numAdds, coinsPerAdd, intersectingCoinsPerAdd), benchmarkingFunc(numAdds, coinsPerAdd, intersectingCoinsPerAdd, sumFn.fn)) } diff --git a/types/mapcoins.go b/types/mapcoins.go index fed3f2544868..d8138dfb779c 100644 --- a/types/mapcoins.go +++ b/types/mapcoins.go @@ -25,7 +25,7 @@ func (m MapCoins) Add(coins ...Coin) { func (m MapCoins) ToCoins() Coins { if len(m) == 0 { - return nil + return Coins{} } coins := make(Coins, 0, len(m)) for denom, amount := range m { @@ -36,7 +36,7 @@ func (m MapCoins) ToCoins() Coins { } coins.Sort() if len(coins) == 0 { - return nil + return Coins{} } return coins } diff --git a/types/mapcoins_test.go b/types/mapcoins_test.go index 71f95840968c..9340a79f2ef3 100644 --- a/types/mapcoins_test.go +++ b/types/mapcoins_test.go @@ -1,26 +1,63 @@ package types_test -import sdk "github.com/cosmos/cosmos-sdk/types" +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" +) func (s *coinTestSuite) TestMapCoinAdd() { + cA0M0 := sdk.Coins{s.ca0, s.cm0} + cA0M1 := sdk.Coins{s.ca0, s.cm1} + cA1M1 := sdk.Coins{s.ca1, s.cm1} cases := []struct { + name string inputOne sdk.Coins inputTwo sdk.Coins + expected sdk.Coins + msg string }{ - {sdk.Coins{s.ca1, s.cm1}, sdk.Coins{s.ca1, s.cm1}}, - {sdk.Coins{s.ca0, s.cm1}, sdk.Coins{s.ca0, s.cm0}}, - {sdk.Coins{s.ca2}, sdk.Coins{s.cm0}}, - {sdk.Coins{s.ca1}, sdk.Coins{s.ca1, s.cm2}}, - {sdk.Coins{s.ca0, s.cm0}, sdk.Coins{s.ca0, s.cm0}}, + {"adding two empty lists", s.emptyCoins, s.emptyCoins, s.emptyCoins, "empty, non list should be returned"}, + {"empty list + set", s.emptyCoins, cA0M1, sdk.Coins{s.cm1}, "zero coins should be removed"}, + {"empty list + set", s.emptyCoins, cA1M1, cA1M1, "zero + a_non_zero = a_non_zero"}, + {"set + empty list", cA0M1, s.emptyCoins, sdk.Coins{s.cm1}, "zero coins should be removed"}, + {"set + empty list", cA1M1, s.emptyCoins, cA1M1, "a_non_zero + zero = a_non_zero"}, + { + "{1atom,1muon}+{1atom,1muon}", cA1M1, cA1M1, + sdk.Coins{s.ca2, s.cm2}, + "a + a = 2a", + }, + { + "{0atom,1muon}+{0atom,0muon}", cA0M1, cA0M0, + sdk.Coins{s.cm1}, + "zero coins should be removed", + }, + { + "{2atom}+{0muon}", + sdk.Coins{s.ca2}, + sdk.Coins{s.cm0}, + sdk.Coins{s.ca2}, + "zero coins should be removed", + }, + { + "{1atom}+{1atom,2muon}", + sdk.Coins{s.ca1}, + sdk.Coins{s.ca1, s.cm2}, + sdk.Coins{s.ca2, s.cm2}, + "should be correctly added", + }, + { + "{0atom,0muon}+{0atom,0muon}", cA0M0, cA0M0, s.emptyCoins, + "sets with zero coins should return empty set", + }, } - for tcIndex, tc := range cases { + for _, tc := range cases { expected := tc.inputOne.Add(tc.inputTwo...) m := sdk.NewMapCoins(tc.inputOne) m.Add(tc.inputTwo...) res := m.ToCoins() s.Require().True(res.IsValid()) - s.Require().Equal(expected, res, "sum of coins is incorrect, tc #%d", tcIndex) + require.Equal(s.T(), expected, res, tc.msg) } } From 0d670daea0e6afcfaba698a50fa25ef3929af3a0 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 9 Apr 2023 11:55:14 +0200 Subject: [PATCH 6/9] Benchmark update --- types/coin_benchmark_test.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/types/coin_benchmark_test.go b/types/coin_benchmark_test.go index d2e14cb46cae..5bb3f843f288 100644 --- a/types/coin_benchmark_test.go +++ b/types/coin_benchmark_test.go @@ -74,31 +74,30 @@ func BenchmarkCoinsAdditionNoIntersect(b *testing.B) { func BenchmarkSumOfCoinAdds(b *testing.B) { // This benchmark tests the performance of adding a large number of coins // into a single coin set. - // it does numAdds additions, each addition has (numIntersectingCoins) that contian denoms + // it does numAdds additions, each addition has (numIntersectingCoins) that contain denoms // already in the sum, and (coinsPerAdd - numIntersectingCoins) that are new denoms. - benchmarkingFunc := func(coinsPerAdd, numIntersectingCoins, numAdds int, sumFn func([]Coins) Coins) func(b *testing.B) { + benchmarkingFunc := func(numAdds, coinsPerAdd, numIntersectingCoins int, sumFn func([]Coins) Coins) func(b *testing.B) { return func(b *testing.B) { b.ReportAllocs() addCoins := make([]Coins, numAdds) + nonIntersectingCoins := coinsPerAdd - numIntersectingCoins for i := 0; i < numAdds; i++ { intersectCoins := make([]Coin, numIntersectingCoins) + num := NewInt(int64(i)) for j := 0; j < numIntersectingCoins; j++ { - intersectCoins[j] = NewCoin(coinName(j), NewInt(int64(i))) + intersectCoins[j] = NewCoin(coinName(j+1_000_000_000), num) } addCoins[i] = intersectCoins - for j := 0; j < coinsPerAdd; j++ { - addCoins[i] = addCoins[i].Add(NewCoin(coinName(i+j), NewInt(int64(i)))) + for j := 0; j < nonIntersectingCoins; j++ { + addCoins[i] = addCoins[i].Add(NewCoin(coinName(i*nonIntersectingCoins+j), num)) } } b.ResetTimer() for i := 0; i < b.N; i++ { - sum := Coins{} - for j := 0; j < numAdds; j++ { - sum = sum.Add(addCoins[j]...) - } + sumFn(addCoins) } } } @@ -118,7 +117,9 @@ func BenchmarkSumOfCoinAdds(b *testing.B) { return sum } - benchmarkSizes := [][]int{{5, 2, 1000}, {10, 1, 10000}, {10, 10, 10000}} + // larger benchmarks with non-overlapping coins won't terminate in reasonable timeframes with sdk.Coins + // they work fine with MapCoins + benchmarkSizes := [][]int{{5, 2, 1000}, {10, 10, 10000}} sumFns := []struct { name string fn func([]Coins) Coins From d2e8c5b24eb328477a1924a83aa2d6225b433b08 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 9 Apr 2023 12:05:59 +0200 Subject: [PATCH 7/9] Undo accidental go 1.19 local compile work --- types/mapcoins_test.go | 2 +- types/msgservice/validate.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/types/mapcoins_test.go b/types/mapcoins_test.go index 9340a79f2ef3..e89f0dba362e 100644 --- a/types/mapcoins_test.go +++ b/types/mapcoins_test.go @@ -5,7 +5,7 @@ import ( "github.com/stretchr/testify/require" ) -func (s *coinTestSuite) TestMapCoinAdd() { +func (s *coinTestSuite) TestMapCoinsAdd() { cA0M0 := sdk.Coins{s.ca0, s.cm0} cA0M1 := sdk.Coins{s.ca0, s.cm1} cA1M1 := sdk.Coins{s.ca1, s.cm1} diff --git a/types/msgservice/validate.go b/types/msgservice/validate.go index 4264cb7e28cb..cbbed0ad9d8e 100644 --- a/types/msgservice/validate.go +++ b/types/msgservice/validate.go @@ -1,6 +1,7 @@ package msgservice import ( + "errors" "fmt" "google.golang.org/protobuf/proto" @@ -39,7 +40,7 @@ func ValidateProtoAnnotations(protoFiles *protoregistry.Files) error { return true }) - return serviceErrs[0] + return errors.Join(serviceErrs...) } // validateMsgServiceAnnotations validates that the service has the From 483b8ca053d5f50445601c6f48e5be0faf789467 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Tue, 11 Apr 2023 15:57:06 +0200 Subject: [PATCH 8/9] reorder lines as desired --- types/mapcoins.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/mapcoins.go b/types/mapcoins.go index d8138dfb779c..46e9adcd1383 100644 --- a/types/mapcoins.go +++ b/types/mapcoins.go @@ -34,9 +34,9 @@ func (m MapCoins) ToCoins() Coins { } coins = append(coins, NewCoin(denom, amount)) } - coins.Sort() if len(coins) == 0 { return Coins{} } + coins.Sort() return coins } From 5a4fdc0bfd68bf83c49666e5f286039ed20229a6 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Tue, 18 Apr 2023 14:21:42 +0200 Subject: [PATCH 9/9] lint --- types/mapcoins_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/types/mapcoins_test.go b/types/mapcoins_test.go index e89f0dba362e..68fd74994c72 100644 --- a/types/mapcoins_test.go +++ b/types/mapcoins_test.go @@ -59,5 +59,4 @@ func (s *coinTestSuite) TestMapCoinsAdd() { s.Require().True(res.IsValid()) require.Equal(s.T(), expected, res, tc.msg) } - }