From 988aabf6940878417ce6fd8536bc022b66ec6467 Mon Sep 17 00:00:00 2001 From: Jim Larson <32469398+JimLarson@users.noreply.github.com> Date: Tue, 22 Feb 2022 15:50:41 -0800 Subject: [PATCH] feat: min and max operators on coins (#11200) ## Description Closes: #10995 Adds `Min()` and `Max()` operations on `sdk.Coins` for per-denom minimum and maximum. Replaced an example of manual low-level construction of `Coins` with higher-level operators. Upcoming enhancements to vesting accounts make heavy use of this pattern. --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [X] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [X] added `!` to the type prefix if API or client breaking change - [X] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [X] provided a link to the relevant issue or specification - [X] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [X] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [X] added a changelog entry to `CHANGELOG.md` - [X] included comments for [documenting Go code](https://blog.golang.org/godoc) - [X] updated the relevant documentation or specification - [X] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit afbb0bd1941f7ad36e086913153af02eb6a68f5a) # Conflicts: # CHANGELOG.md --- CHANGELOG.md | 46 +++++++++++++ types/coin.go | 85 +++++++++++++++++++++++++ types/coin_test.go | 27 ++++++++ types/dec_coin.go | 3 +- x/auth/vesting/types/vesting_account.go | 16 +---- 5 files changed, 163 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f05697049af..f6eac58b232c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,52 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features * [\#11124](https://github.com/cosmos/cosmos-sdk/pull/11124) Add `GetAllVersions` to application store +<<<<<<< HEAD +======= +* (x/authz) [\#10447](https://github.com/cosmos/cosmos-sdk/pull/10447) authz `NewGrant` takes a new argument: block time, to correctly validate expire time. + + +### Client Breaking Changes + +* [\#11089](https://github.com/cosmos/cosmos-sdk/pull/11089]) interacting with the node through `grpc.Dial` requires clients to pass a codec refer to [doc](docs/run-node/interact-node.md). +* [\#9594](https://github.com/cosmos/cosmos-sdk/pull/9594) Remove legacy REST API. Please see the [REST Endpoints Migration guide](https://docs.cosmos.network/master/migrations/rest.html) to migrate to the new REST endpoints. +* [\#9995](https://github.com/cosmos/cosmos-sdk/pull/9995) Increased gas cost for creating proposals. +* [\#11029](https://github.com/cosmos/cosmos-sdk/pull/11029) The deprecated Vote Option field is removed in gov v1beta2 and nil in v1beta1. Use Options instead. +* [\#11013](https://github.com/cosmos/cosmos-sdk/pull/) The `tx gov submit-proposal` command has changed syntax to support the new Msg-based gov proposals. To access the old CLI command, please use `tx gov submit-legacy-proposal`. +* [\#11170](https://github.com/cosmos/cosmos-sdk/issues/11170) Fixes issue related to grpc-gateway of supply by ibc-denom. + +### CLI Breaking Changes + +* [\#9695](https://github.com/cosmos/cosmos-sdk/pull/9695) ` keys migrate` CLI command now takes no arguments +* [\#9246](https://github.com/cosmos/cosmos-sdk/pull/9246) Removed the CLI flag `--setup-config-only` from the `testnet` command and added the subcommand `init-files`. +* [\#9780](https://github.com/cosmos/cosmos-sdk/pull/9780) Use sigs.k8s.io for yaml, which might lead to minor YAML output changes +* [\#10625](https://github.com/cosmos/cosmos-sdk/pull/10625) Rename `--fee-account` CLI flag to `--fee-granter` +* [\#10684](https://github.com/cosmos/cosmos-sdk/pull/10684) Rename `edit-validator` command's `--moniker` flag to `--new-moniker` + +### Improvements + +* [\#11089](https://github.com/cosmos/cosmos-sdk/pull/11089]) Now cosmos-sdk consumers can upgrade gRPC to its newest versions. +* [\#10439](https://github.com/cosmos/cosmos-sdk/pull/10439) Check error for `RegisterQueryHandlerClient` in all modules `RegisterGRPCGatewayRoutes`. +* [\#9780](https://github.com/cosmos/cosmos-sdk/pull/9780) Remove gogoproto `moretags` YAML annotations and add `sigs.k8s.io/yaml` for YAML marshalling. +* (x/bank) [\#10134](https://github.com/cosmos/cosmos-sdk/pull/10134) Add `HasDenomMetadata` function to bank `Keeper` to check if a client coin denom metadata exists in state. +* (x/bank) [\#10022](https://github.com/cosmos/cosmos-sdk/pull/10022) `BankKeeper.SendCoins` now takes less execution time. +* (deps) [\#9987](https://github.com/cosmos/cosmos-sdk/pull/9987) Bump Go version minimum requirement to `1.17` +* (cli) [\#9856](https://github.com/cosmos/cosmos-sdk/pull/9856) Overwrite `--sequence` and `--account-number` flags with default flag values when used with `offline=false` in `sign-batch` command. +* (rosetta) [\#10001](https://github.com/cosmos/cosmos-sdk/issues/10001) Add documentation for rosetta-cli dockerfile and rename folder for the rosetta-ci dockerfile +* [\#9699](https://github.com/cosmos/cosmos-sdk/pull/9699) Add `:`, `.`, `-`, and `_` as allowed characters in the default denom regular expression. +* (genesis) [\#9697](https://github.com/cosmos/cosmos-sdk/pull/9697) Ensure `InitGenesis` returns with non-empty validator set. +* [\#10341](https://github.com/cosmos/cosmos-sdk/pull/10341) Move from `io/ioutil` to `io` and `os` packages. +* [\#10468](https://github.com/cosmos/cosmos-sdk/pull/10468) Allow futureOps to queue additional operations in simulations +* [\#10625](https://github.com/cosmos/cosmos-sdk/pull/10625) Add `--fee-payer` CLI flag +* (cli) [\#10683](https://github.com/cosmos/cosmos-sdk/pull/10683) In CLI, allow 1 SIGN_MODE_DIRECT signer in transactions with multiple signers. +* (deps) [\#10210](https://github.com/cosmos/cosmos-sdk/pull/10210) Bump Tendermint to [v0.35.0](https://github.com/tendermint/tendermint/releases/tag/v0.35.0). +* (deps) [\#10706](https://github.com/cosmos/cosmos-sdk/issues/10706) Bump rosetta-sdk-go to v0.7.2 and rosetta-cli to v0.7.3 +* (types/errors) [\#10779](https://github.com/cosmos/cosmos-sdk/pull/10779) Move most functionality in `types/errors` to a standalone `errors` go module, except the `RootCodespace` errors and ABCI response helpers. All functions and types that used to live in `types/errors` are now aliased so this is not a breaking change. +* (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. +>>>>>>> afbb0bd19 (feat: min and max operators on coins (#11200)) ### Bug Fixes diff --git a/types/coin.go b/types/coin.go index d4d22642d21b..af8ef0ae7cc3 100644 --- a/types/coin.go +++ b/types/coin.go @@ -390,6 +390,91 @@ func (coins Coins) SafeSub(coinsB Coins) (Coins, bool) { return diff, diff.IsAnyNegative() } +// 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 { + 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 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 +// +// 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); { + 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 240b15d11de0..cd294a899e5c 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -656,6 +656,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/types/dec_coin.go b/types/dec_coin.go index 6e400d7a0557..68fe3b9f958e 100644 --- a/types/dec_coin.go +++ b/types/dec_coin.go @@ -319,8 +319,9 @@ 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). +// See also Coins.Min(). func (coins DecCoins) Intersect(coinsB DecCoins) DecCoins { res := make([]DecCoin, len(coins)) for i, coin := range coins { diff --git a/x/auth/vesting/types/vesting_account.go b/x/auth/vesting/types/vesting_account.go index b4e35835cc83..fb1dabde85dc 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 }