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

fix: Make MulDecTruncate() return nil DecCoins when multiplier is 0 #10687

Merged
merged 13 commits into from
Jan 12, 2022
Merged

fix: Make MulDecTruncate() return nil DecCoins when multiplier is 0 #10687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
78eb351
Fix behavior of MulDecTruncate()
evancholmberg Dec 5, 2021
573c8f4
Update test to expect panic when multiplier d is 0
evancholmberg Dec 6, 2021
2b5cc6e
Update CHANGELOG.md
evancholmberg Dec 7, 2021
c94d1ff
Merge branch 'master' into evancholmberg/9790-wrong-muldectruncate-be…
evancholmberg Dec 7, 2021
c01dfb4
Update CHANGELOG.md
evancholmberg Dec 7, 2021
bfe6cfc
Merge branch 'master' into evancholmberg/9790-wrong-muldectruncate-be…
atheeshp Dec 8, 2021
ff9a948
Merge branch 'master' into evancholmberg/9790-wrong-muldectruncate-be…
alexanderbez Dec 9, 2021
a31d3ee
Return `nil `DecCoins` when `d` is zero
evancholmberg Dec 16, 2021
031f4b6
Merge branch 'cosmos:master' into evancholmberg/9790-wrong-muldectrun…
evancholmberg Dec 16, 2021
b98b4fa
Merge branch 'master' into evancholmberg/9790-wrong-muldectruncate-be…
mergify[bot] Dec 17, 2021
85afa8c
Merge branch 'master' into evancholmberg/9790-wrong-muldectruncate-be…
evancholmberg Jan 11, 2022
62f3061
Update CHANGELOG.md
evancholmberg Jan 11, 2022
90e9cbc
Merge branch 'master' into evancholmberg/9790-wrong-muldectruncate-be…
amaury1093 Jan 12, 2022
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 @@ -189,6 +189,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [\#10593](https://github.com/cosmos/cosmos-sdk/pull/10593) Update swagger-ui to v4.1.0 to fix xss vulnerability.
* [\#10674](https://github.com/cosmos/cosmos-sdk/pull/10674) Fix issue with `Error.Wrap` and `Error.Wrapf` usage with `errors.Is`.
* [\#10897](https://github.com/cosmos/cosmos-sdk/pull/10897) Fix: set a non-zero value on gas overflow.
* [#9790](https://github.com/cosmos/cosmos-sdk/pull/10687) Fix behavior of `DecCoins.MulDecTruncate`.

### State Machine Breaking

Expand Down
7 changes: 5 additions & 2 deletions types/dec_coin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,12 +373,15 @@ func (coins DecCoins) MulDec(d Dec) DecCoins {
}

// MulDecTruncate multiplies all the decimal coins by a decimal, truncating. It
// panics if d is zero.
// returns nil DecCoins if d is zero.
//
// CONTRACT: No zero coins will be returned.
func (coins DecCoins) MulDecTruncate(d Dec) DecCoins {
var res DecCoins
if d.IsZero() {
return DecCoins{}
}

var res DecCoins
for _, coin := range coins {
product := DecCoin{
Denom: coin.Denom,
Expand Down
4 changes: 1 addition & 3 deletions types/dec_coin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,12 +880,10 @@ func (s *decCoinTestSuite) TestDecCoins_MulDecTruncate() {
}{
{"No Coins", sdk.DecCoins{}, sdk.NewDec(1), sdk.DecCoins(nil), false},

// TODO - Fix test - Function comment documentation for MulDecTruncate says if multiplier d is zero, it should panic.
// However, that is not the observed behaviour. Currently nil is returned.
{"Multiple coins - zero multiplier", sdk.DecCoins{
sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(10, 3)},
sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(30, 2)},
}, sdk.NewDec(0), sdk.DecCoins(nil), false},
}, sdk.NewDec(0), sdk.DecCoins{}, false},

{"Multiple coins - positive multiplier", sdk.DecCoins{
sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(15, 1)},
Expand Down