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.

Sign up for GitHub

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

Jump to bottom

feat(types): Implement .IsGT for types.Coin #19281

Merged
merged 8 commits into from
Jan 30, 2024
Merged

feat(types): Implement .IsGT for types.Coin #19281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2d27e38
implementing coin.IsGT(coin)
spoo-bar Jan 26, 2024
9e545ee
adding coin.IsGT(coin) test
spoo-bar Jan 26, 2024
77fadc4
Update coin_test.go
spoo-bar Jan 26, 2024
b3bffd5
Merge branch 'cosmos:main' into spoorthi/add-coins-IsGT
spoo-bar Jan 29, 2024
8133715
Update CHANGELOG.md
spoo-bar Jan 29, 2024
757f837
Merge branch 'spoorthi/add-coins-IsGT' of https://github.com/spoo-bar…
spoo-bar Jan 29, 2024
d8b35d1
Merge branch 'main' into spoorthi/add-coins-IsGT
spoo-bar Jan 30, 2024
036b6ad
Merge branch 'main' into spoorthi/add-coins-IsGT
spoo-bar Jan 30, 2024
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i

### Features

* (types) [#19281](https://github.com/cosmos/cosmos-sdk/pull/19281) Added a new method, `IsGT`, for `types.Coin`. This method is used to check if a `types.Coin` is greater than another `types.Coin`.
* (client) [#18557](https://github.com/cosmos/cosmos-sdk/pull/18557) Add `--qrcode` flag to `keys show` command to support displaying keys address QR code.
* (client) [#18101](https://github.com/cosmos/cosmos-sdk/pull/18101) Add a `keyring-default-keyname` in `client.toml` for specifying a default key name, and skip the need to use the `--from` flag when signing transactions.
* (tests) [#17868](https://github.com/cosmos/cosmos-sdk/pull/17868) Added helper method `SubmitTestTx` in testutil to broadcast test txns to test e2e tests.
Expand Down
10 changes: 10 additions & 0 deletions types/coin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@ func (coin Coin) IsZero() bool {
return coin.Amount.IsZero()
}

// IsGT returns true if they are the same type and the receiver is
// a greater value
func (coin Coin) IsGT(other Coin) bool {
if coin.Denom != other.Denom {
panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, other.Denom))
}

return coin.Amount.GT(other.Amount)
}
Comment on lines +71 to +79
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation of IsGT method correctly compares two Coin instances to determine if the receiver coin's amount is greater than the other's, ensuring they have the same denomination. However, using panic for handling different denominations is not ideal for comparison operations, as it forces the caller to handle potential panics, which could be avoided with error handling.

Consider returning an error alongside the boolean result to handle denomination mismatches more gracefully, allowing the caller to decide how to handle such cases without the risk of a panic.

- func (coin Coin) IsGT(other Coin) bool {
+ func (coin Coin) IsGT(other Coin) (bool, error) {
  if coin.Denom != other.Denom {
-   panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, other.Denom))
+   return false, fmt.Errorf("invalid coin denominations; %s, %s", coin.Denom, other.Denom)
  }
- return coin.Amount.GT(other.Amount)
+ return coin.Amount.GT(other.Amount), nil
}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
// IsGT returns true if they are the same type and the receiver is
// a greater value
func (coin Coin) IsGT(other Coin) bool {
if coin.Denom != other.Denom {
panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, other.Denom))
}
return coin.Amount.GT(other.Amount)
}
// IsGT returns true if they are the same type and the receiver is
// a greater value
func (coin Coin) IsGT(other Coin) (bool, error) {
if coin.Denom != other.Denom {
return false, fmt.Errorf("invalid coin denominations; %s, %s", coin.Denom, other.Denom)
}
return coin.Amount.GT(other.Amount), nil
}


// IsGTE returns true if they are the same type and the receiver is
// an equal or greater value
func (coin Coin) IsGTE(other Coin) bool {
Expand Down
24 changes: 24 additions & 0 deletions types/coin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,30 @@ func (s *coinTestSuite) TestQuoIntCoins() {
}
}

func (s *coinTestSuite) TestIsGTCoin() {
cases := []struct {
inputOne sdk.Coin
inputTwo sdk.Coin
expected bool
panics bool
}{
{sdk.NewInt64Coin(testDenom1, 2), sdk.NewInt64Coin(testDenom1, 1), true, false},
{sdk.NewInt64Coin(testDenom1, 1), sdk.NewInt64Coin(testDenom1, 1), false, false},
{sdk.NewInt64Coin(testDenom1, 1), sdk.NewInt64Coin(testDenom1, 2), false, false},
{sdk.NewInt64Coin(testDenom1, 1), sdk.NewInt64Coin(testDenom2, 1), false, true},
}

for tcIndex, tc := range cases {
tc := tc
if tc.panics {
s.Require().Panics(func() { tc.inputOne.IsGT(tc.inputTwo) })
} else {
res := tc.inputOne.IsGT(tc.inputTwo)
s.Require().Equal(tc.expected, res, "coin GT relation is incorrect, tc #%d", tcIndex)
}
}
}

func (s *coinTestSuite) TestIsGTECoin() {
cases := []struct {
inputOne sdk.Coin
Expand Down
Loading