From 50bd97e38cf510ff7c7a0bd57ec2672692c419be Mon Sep 17 00:00:00 2001 From: Jim Larson Date: Tue, 15 Feb 2022 19:19:38 -0800 Subject: [PATCH 1/6] feat: min and max operators on coins --- types/coin.go | 35 +++++++++++++++++++++++++ types/coin_test.go | 27 +++++++++++++++++++ x/auth/vesting/types/vesting_account.go | 16 +++-------- 3 files changed, 65 insertions(+), 13 deletions(-) diff --git a/types/coin.go b/types/coin.go index ce222e1709f8..9bbc19ed0e52 100644 --- a/types/coin.go +++ b/types/coin.go @@ -390,6 +390,41 @@ func (coins Coins) SafeSub(coinsB Coins) (Coins, bool) { return diff, diff.IsAnyNegative() } +// Max returns the maximum of each denom of its inputs. +// The inputs should be sorted. Note that the max might +// not be equal to either of its inputs. +func (coins Coins) Max(coinsB Coins) Coins { + // coins + coinsB = min(coins, coinsB) + max(coins, coinsB) + return coins.Add(coinsB...).Sub(coins.Min(coinsB)) +} + +// Min returns the minimum of each denom of its inputs. +// The inputs should be sorted. Note that the min might +// not be equal to either of its inputs. +func (coins Coins) Min(coinsB Coins) Coins { + min := make([]Coin, 0) + for indexA, indexB := 0, 0; indexA < len(coins) && indexB < len(coinsB); { + coinA, coinB := coins[indexA], coinsB[indexB] + switch strings.Compare(coinA.Denom, coinB.Denom) { + case -1: // denom missing from coinsB + indexA++ + case 0: // same denom in both + minCoin := coinA + if coinB.Amount.LT(minCoin.Amount) { + minCoin = coinB + } + if !minCoin.IsZero() { + min = append(min, minCoin) + } + indexA++ + indexB++ + case 1: // denom missing from coins + indexB++ + } + } + return NewCoins(min...) +} + // IsAllGT returns true if for every denom in coinsB, // the denom is present at a greater amount in coins. func (coins Coins) IsAllGT(coinsB Coins) bool { diff --git a/types/coin_test.go b/types/coin_test.go index 0d6b6775955b..f82b581aa691 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -657,6 +657,33 @@ func (s *coinTestSuite) TestCoins_Validate() { } } +func (s *coinTestSuite) TestMinMax() { + one := sdk.OneInt() + two := sdk.NewInt(2) + + cases := []struct { + name string + input1 sdk.Coins + input2 sdk.Coins + min sdk.Coins + max sdk.Coins + }{ + {"zero-zero", sdk.Coins{}, sdk.Coins{}, sdk.Coins{}, sdk.Coins{}}, + {"zero-one", sdk.Coins{}, sdk.Coins{{testDenom1, one}}, sdk.Coins{}, sdk.Coins{{testDenom1, one}}}, + {"two-zero", sdk.Coins{{testDenom2, two}}, sdk.Coins{}, sdk.Coins{}, sdk.Coins{{testDenom2, two}}}, + {"disjoint", sdk.Coins{{testDenom1, one}}, sdk.Coins{{testDenom2, two}}, sdk.Coins{}, sdk.Coins{{testDenom1, one}, {testDenom2, two}}}, + {"overlap", sdk.Coins{{testDenom1, one}, {testDenom2, two}}, sdk.Coins{{testDenom1, two}, {testDenom2, one}}, + sdk.Coins{{testDenom1, one}, {testDenom2, one}}, sdk.Coins{{testDenom1, two}, {testDenom2, two}}}, + } + + for _, tc := range cases { + min := tc.input1.Min(tc.input2) + max := tc.input1.Max(tc.input2) + s.Require().True(min.IsEqual(tc.min), tc.name) + s.Require().True(max.IsEqual(tc.max), tc.name) + } +} + func (s *coinTestSuite) TestCoinsGT() { one := sdk.OneInt() two := sdk.NewInt(2) diff --git a/x/auth/vesting/types/vesting_account.go b/x/auth/vesting/types/vesting_account.go index 62ff4234f9e5..1b21e882f0a9 100644 --- a/x/auth/vesting/types/vesting_account.go +++ b/x/auth/vesting/types/vesting_account.go @@ -41,20 +41,10 @@ func NewBaseVestingAccount(baseAccount *authtypes.BaseAccount, originalVesting s // // CONTRACT: Delegated vesting coins and vestingCoins must be sorted. func (bva BaseVestingAccount) LockedCoinsFromVesting(vestingCoins sdk.Coins) sdk.Coins { - lockedCoins := sdk.NewCoins() - - for _, vestingCoin := range vestingCoins { - vestingAmt := vestingCoin.Amount - delVestingAmt := bva.DelegatedVesting.AmountOf(vestingCoin.Denom) - - max := sdk.MaxInt(vestingAmt.Sub(delVestingAmt), sdk.ZeroInt()) - lockedCoin := sdk.NewCoin(vestingCoin.Denom, max) - - if !lockedCoin.IsZero() { - lockedCoins = lockedCoins.Add(lockedCoin) - } + lockedCoins := vestingCoins.Sub(vestingCoins.Min(bva.DelegatedVesting)) + if lockedCoins == nil { + return sdk.Coins{} } - return lockedCoins } From 6ebc10d1660f1b51476252c04459d1d34e2b7e21 Mon Sep 17 00:00:00 2001 From: Jim Larson Date: Thu, 17 Feb 2022 09:39:16 -0800 Subject: [PATCH 2/6] docs: add changelog note for sdk.Coins Min and Max --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8794e0c4bffc..eb6f9151ac6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -171,6 +171,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (gov) [\#10854](https://github.com/cosmos/cosmos-sdk/pull/10854) v1beta2's vote doesn't include the deprecate `option VoteOption` anymore. Instead, it only uses `WeightedVoteOption`. * (types) [\#11004](https://github.com/cosmos/cosmos-sdk/pull/11004) Added mutable versions of many of the sdk.Dec types operations. This improves performance when used by avoiding reallocating a new bigint for each operation. * (x/auth) [\#10880](https://github.com/cosmos/cosmos-sdk/pull/10880) Added a new query to the tx query service that returns a block with transactions fully decoded. +* (types) [\#11200](https://github.com/cosmos/cosmos-sdk/pull/11200) Added `Min()` and `Max()` operations on sdk.Coins. ### Bug Fixes From e1d18735286052672851047ed76e55c873f954ce Mon Sep 17 00:00:00 2001 From: Jim Larson Date: Thu, 17 Feb 2022 14:34:35 -0800 Subject: [PATCH 3/6] docs: clarify multiset semantics --- types/coin.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/types/coin.go b/types/coin.go index 9bbc19ed0e52..c7b37e3aed86 100644 --- a/types/coin.go +++ b/types/coin.go @@ -392,7 +392,8 @@ func (coins Coins) SafeSub(coinsB Coins) (Coins, bool) { // Max returns the maximum of each denom of its inputs. // The inputs should be sorted. Note that the max might -// not be equal to either of its inputs. +// not be equal to either of its inputs. Uses multiset +// semantics, so unmentioned denoms are implicitly zero. func (coins Coins) Max(coinsB Coins) Coins { // coins + coinsB = min(coins, coinsB) + max(coins, coinsB) return coins.Add(coinsB...).Sub(coins.Min(coinsB)) @@ -400,7 +401,8 @@ func (coins Coins) Max(coinsB Coins) Coins { // Min returns the minimum of each denom of its inputs. // The inputs should be sorted. Note that the min might -// not be equal to either of its inputs. +// not be equal to either of its inputs. Uses multiset +// semantics, so unmentioned denoms are implicitly zero. func (coins Coins) Min(coinsB Coins) Coins { min := make([]Coin, 0) for indexA, indexB := 0, 0; indexA < len(coins) && indexB < len(coinsB); { From 56d3a1309921914d4a0ca5978f5c6de1b8914ee8 Mon Sep 17 00:00:00 2001 From: Jim Larson Date: Thu, 17 Feb 2022 15:28:04 -0800 Subject: [PATCH 4/6] refactor: aliases for consistency with DecCoins --- types/coin.go | 11 ++++++++++- types/dec_coin.go | 18 +++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/types/coin.go b/types/coin.go index c7b37e3aed86..ca63fa32a634 100644 --- a/types/coin.go +++ b/types/coin.go @@ -395,7 +395,7 @@ func (coins Coins) SafeSub(coinsB Coins) (Coins, bool) { // not be equal to either of its inputs. Uses multiset // semantics, so unmentioned denoms are implicitly zero. func (coins Coins) Max(coinsB Coins) Coins { - // coins + coinsB = min(coins, coinsB) + max(coins, coinsB) + // coins + coinsB == min(coins, coinsB) + max(coins, coinsB) return coins.Add(coinsB...).Sub(coins.Min(coinsB)) } @@ -427,6 +427,15 @@ func (coins Coins) Min(coinsB Coins) Coins { return NewCoins(min...) } +// Intersect will return a new set of coins which coinains the minimum Coin +// for common denoms found in both `coins` and `coinsB`. Identical to Min() +// method and provided for compatibility with DecCoins. Note that the +// corresponding Union() alias for Max() will not be provided since it is +// dangerously confusable with Add(). +func (coins Coins) Intersect(coinsB Coins) Coins { + return coins.Min(coinsB) +} + // IsAllGT returns true if for every denom in coinsB, // the denom is present at a greater amount in coins. func (coins Coins) IsAllGT(coinsB Coins) bool { diff --git a/types/dec_coin.go b/types/dec_coin.go index bb32ef653a11..f66f5da1a807 100644 --- a/types/dec_coin.go +++ b/types/dec_coin.go @@ -319,7 +319,7 @@ func (coins DecCoins) SafeSub(coinsB DecCoins) (DecCoins, bool) { // Intersect will return a new set of coins which contains the minimum DecCoin // for common denoms found in both `coins` and `coinsB`. For denoms not common // to both `coins` and `coinsB` the minimum is considered to be 0, thus they -// are not added to the final set.In other words, trim any denom amount from +// are not added to the final set. In other words, trim any denom amount from // coin which exceeds that of coinB, such that (coin.Intersect(coinB)).IsLTE(coinB). func (coins DecCoins) Intersect(coinsB DecCoins) DecCoins { res := make([]DecCoin, len(coins)) @@ -333,6 +333,22 @@ func (coins DecCoins) Intersect(coinsB DecCoins) DecCoins { return removeZeroDecCoins(res) } +// Min returns the minimum of each denom of its inputs, including +// the implicitly-zero absent denoms. Identical to Intersect() and +// provided for compatibility with Coins. +func (coins DecCoins) Min(coinsB DecCoins) DecCoins { + return coins.Intersect(coinsB) +} + +// Max returns the maximum of each denom of its inputs, including +// the implicitly-zero absent denoms. Though this is the +// multiset union operation, that name is dangerously confusable +// with Add(), and will not be used. +func (coins DecCoins) Max(coinsB DecCoins) DecCoins { + // coins + coinsB == min(coins, coinsB) + max(coins, coinsB) + return coins.Add(coinsB...).Sub(coins.Min(coinsB)) +} + // GetDenomByIndex returns the Denom to make the findDup generic func (coins DecCoins) GetDenomByIndex(i int) string { return coins[i].Denom From 2059cd865fce2c48a0c4a9ca59f1d50254862524 Mon Sep 17 00:00:00 2001 From: Jim Larson Date: Fri, 18 Feb 2022 17:28:56 -0800 Subject: [PATCH 5/6] refactor: faster Max, tests for Intersect. --- types/coin.go | 66 +++++++++++++++++++++++++++++++++++++++------- types/coin_test.go | 2 ++ 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/types/coin.go b/types/coin.go index ca63fa32a634..c8b7c77db77d 100644 --- a/types/coin.go +++ b/types/coin.go @@ -390,19 +390,65 @@ func (coins Coins) SafeSub(coinsB Coins) (Coins, bool) { return diff, diff.IsAnyNegative() } -// Max returns the maximum of each denom of its inputs. -// The inputs should be sorted. Note that the max might -// not be equal to either of its inputs. Uses multiset -// semantics, so unmentioned denoms are implicitly zero. +// Max takes two valid Coins inputs and returns a valid Coins result +// where for every denom D, AmountOf(D) of the result is the maximum +// of AmountOf(D) of the inputs. Note that the result might be not +// be equal to either input. For any valid Coins a, b, and c, the +// following are always true: +// a.IsAllLTE(a.Max(b)) +// b.IsAllLTE(a.Max(b)) +// a.IsAllLTE(c) && b.IsAllLTE(c) == a.Max(b).IsAllLTE(c) +// a.Add(b...).IsEqual(a.Min(b).Add(a.Max(b)...)) +// +// E.g. +// {1A, 3B, 2C}.Max({4A, 2B, 2C} == {4A, 3B, 2C}) +// {2A, 3B}.Max({1B, 4C}) == {2A, 3B, 4C} +// {1A, 2B}.Max({}) == {1A, 2B} func (coins Coins) Max(coinsB Coins) Coins { - // coins + coinsB == min(coins, coinsB) + max(coins, coinsB) - return coins.Add(coinsB...).Sub(coins.Min(coinsB)) + max := make([]Coin, 0) + indexA, indexB := 0, 0 + for indexA < len(coins) && indexB < len(coinsB) { + coinA, coinB := coins[indexA], coinsB[indexB] + switch strings.Compare(coinA.Denom, coinB.Denom) { + case -1: // denom missing from coinsB + max = append(max, coinA) + indexA++ + case 0: // same denom in both + maxCoin := coinA + if coinB.Amount.GT(maxCoin.Amount) { + maxCoin = coinB + } + max = append(max, maxCoin) + indexA++ + indexB++ + case 1: // denom missing from coinsA + max = append(max, coinB) + indexB++ + } + } + for ; indexA < len(coins); indexA++ { + max = append(max, coins[indexA]) + } + for ; indexB < len(coinsB); indexB++ { + max = append(max, coinsB[indexB]) + } + return NewCoins(max...) } -// Min returns the minimum of each denom of its inputs. -// The inputs should be sorted. Note that the min might -// not be equal to either of its inputs. Uses multiset -// semantics, so unmentioned denoms are implicitly zero. +// Min takes two valid Coins inputs and returns a valid Coins result +// where for every denom D, AmountOf(D) of the result is the minimum +// of AmountOf(D) of the inputs. Note that the result might be not +// be equal to either input. For any valid Coins a, b, and c, the +// following are always true: +// a.Min(b).IsAllLTE(a) +// a.Min(b).IsAllLTE(b) +// c.IsAllLTE(a) && c.IsAllLTE(b) == c.IsAllLTE(a.Min(b)) +// a.Add(b...).IsEqual(a.Min(b).Add(a.Max(b)...)) +// +// E.g. +// {1A, 3B, 2C}.Min({4A, 2B, 2C} == {1A, 2B, 2C}) +// {2A, 3B}.Min({1B, 4C}) == {1B} +// {1A, 2B}.Min({3C}) == empty func (coins Coins) Min(coinsB Coins) Coins { min := make([]Coin, 0) for indexA, indexB := 0, 0; indexA < len(coins) && indexB < len(coinsB); { diff --git a/types/coin_test.go b/types/coin_test.go index f82b581aa691..11567bd3a5e4 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -679,8 +679,10 @@ func (s *coinTestSuite) TestMinMax() { for _, tc := range cases { min := tc.input1.Min(tc.input2) max := tc.input1.Max(tc.input2) + intersect := tc.input1.Intersect(tc.input2) s.Require().True(min.IsEqual(tc.min), tc.name) s.Require().True(max.IsEqual(tc.max), tc.name) + s.Require().True(intersect.IsEqual(tc.min), tc.name) } } From 9ab5dbfe4192119a1a15c7a09d9d0932cf3b19ee Mon Sep 17 00:00:00 2001 From: Jim Larson Date: Tue, 22 Feb 2022 15:21:16 -0800 Subject: [PATCH 6/6] refactor: chose a single name, removed aliases --- types/coin.go | 11 ++--------- types/coin_test.go | 2 -- types/dec_coin.go | 17 +---------------- 3 files changed, 3 insertions(+), 27 deletions(-) diff --git a/types/coin.go b/types/coin.go index c8b7c77db77d..69e77cdbdab9 100644 --- a/types/coin.go +++ b/types/coin.go @@ -449,6 +449,8 @@ func (coins Coins) Max(coinsB Coins) Coins { // {1A, 3B, 2C}.Min({4A, 2B, 2C} == {1A, 2B, 2C}) // {2A, 3B}.Min({1B, 4C}) == {1B} // {1A, 2B}.Min({3C}) == empty +// +// See also DecCoins.Intersect(). func (coins Coins) Min(coinsB Coins) Coins { min := make([]Coin, 0) for indexA, indexB := 0, 0; indexA < len(coins) && indexB < len(coinsB); { @@ -473,15 +475,6 @@ func (coins Coins) Min(coinsB Coins) Coins { return NewCoins(min...) } -// Intersect will return a new set of coins which coinains the minimum Coin -// for common denoms found in both `coins` and `coinsB`. Identical to Min() -// method and provided for compatibility with DecCoins. Note that the -// corresponding Union() alias for Max() will not be provided since it is -// dangerously confusable with Add(). -func (coins Coins) Intersect(coinsB Coins) Coins { - return coins.Min(coinsB) -} - // IsAllGT returns true if for every denom in coinsB, // the denom is present at a greater amount in coins. func (coins Coins) IsAllGT(coinsB Coins) bool { diff --git a/types/coin_test.go b/types/coin_test.go index 11567bd3a5e4..f82b581aa691 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -679,10 +679,8 @@ func (s *coinTestSuite) TestMinMax() { for _, tc := range cases { min := tc.input1.Min(tc.input2) max := tc.input1.Max(tc.input2) - intersect := tc.input1.Intersect(tc.input2) s.Require().True(min.IsEqual(tc.min), tc.name) s.Require().True(max.IsEqual(tc.max), tc.name) - s.Require().True(intersect.IsEqual(tc.min), tc.name) } } diff --git a/types/dec_coin.go b/types/dec_coin.go index f66f5da1a807..aebfcc12192c 100644 --- a/types/dec_coin.go +++ b/types/dec_coin.go @@ -321,6 +321,7 @@ func (coins DecCoins) SafeSub(coinsB DecCoins) (DecCoins, bool) { // to both `coins` and `coinsB` the minimum is considered to be 0, thus they // are not added to the final set. In other words, trim any denom amount from // coin which exceeds that of coinB, such that (coin.Intersect(coinB)).IsLTE(coinB). +// See also Coins.Min(). func (coins DecCoins) Intersect(coinsB DecCoins) DecCoins { res := make([]DecCoin, len(coins)) for i, coin := range coins { @@ -333,22 +334,6 @@ func (coins DecCoins) Intersect(coinsB DecCoins) DecCoins { return removeZeroDecCoins(res) } -// Min returns the minimum of each denom of its inputs, including -// the implicitly-zero absent denoms. Identical to Intersect() and -// provided for compatibility with Coins. -func (coins DecCoins) Min(coinsB DecCoins) DecCoins { - return coins.Intersect(coinsB) -} - -// Max returns the maximum of each denom of its inputs, including -// the implicitly-zero absent denoms. Though this is the -// multiset union operation, that name is dangerously confusable -// with Add(), and will not be used. -func (coins DecCoins) Max(coinsB DecCoins) DecCoins { - // coins + coinsB == min(coins, coinsB) + max(coins, coinsB) - return coins.Add(coinsB...).Sub(coins.Min(coinsB)) -} - // GetDenomByIndex returns the Denom to make the findDup generic func (coins DecCoins) GetDenomByIndex(i int) string { return coins[i].Denom