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.

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

refactor: remove usage of global basedenom #18268

Merged
merged 8 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/mint) [#18283](https://github.com/cosmos/cosmos-sdk/pull/18283) Mint module was moved to its own go.mod `cosmossdk.io/x/mint`
* (x/consensus) [#18041](https://github.com/cosmos/cosmos-sdk/pull/18041) `ToProtoConsensusParams()` returns an error
* (x/slashing) [#18115](https://github.com/cosmos/cosmos-sdk/pull/18115) `NewValidatorSigningInfo` takes strings instead of `sdk.AccAddress`
* (types) [#18268](https://github.com/cosmos/cosmos-sdk/pull/18268) Remove global setting of basedenom. Use the staking module parameter instead

### CLI Breaking Changes

Expand Down
42 changes: 41 additions & 1 deletion types/coin.go
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,8 @@ func ParseCoinNormalized(coinStr string) (coin Coin, err error) {
return Coin{}, err
}

coin, _ = NormalizeDecCoin(decCoin).TruncateDecimal()
coin, _ = normalizeDecCoin(decCoin).TruncateDecimal()
odeke-em marked this conversation as resolved.
Show resolved Hide resolved
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved

return coin, nil
}

Expand All @@ -900,3 +901,42 @@ func ParseCoinsNormalized(coinStr string) (Coins, error) {
}
return NormalizeCoins(coins), nil
}

// ----------------------------------------------------------------------------
// Functions used for testing purposes.

// normalizeDecCoin try to convert a decimal coin to the smallest unit registered,
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
// returns original one if failed.
func normalizeDecCoin(coin DecCoin) DecCoin {
newCoin, err := convertDecCoin(coin)
if err != nil {
return coin
}
return newCoin
}
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 30, 2023

Choose a reason for hiding this comment

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

The function normalizeDecCoin is marked as being used for testing purposes, but it's being used in production code. If it's meant to be used in production code, the comment should be updated to reflect this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@tac0turtle kindly check out this comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like humans are talking. I'll be quiet for now. If you need me again, just tag @coderabbitai in a new comment.


// NormalizeCoins normalize and truncate a list of decimal coins
func NormalizeCoins(coins []DecCoin) Coins {
if coins == nil {
return nil
}
result := make([]Coin, 0, len(coins))

for _, coin := range coins {
newCoin, _ := normalizeDecCoin(coin).TruncateDecimal()
result = append(result, newCoin)
}
odeke-em marked this conversation as resolved.
Show resolved Hide resolved
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved

return result
}

// ConvertDecCoin attempts to convert a decimal coin to a given denomination. If the given
// denomination is invalid or if neither denomination is registered, an error
// is returned.
func convertDecCoin(coin DecCoin) (DecCoin, error) {
if err := ValidateDenom(coin.Denom); err != nil {
return DecCoin{}, err
}

return NewDecCoinFromDec(coin.Denom, coin.Amount), nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The function convertDecCoin doesn't seem to be doing any conversion. It's just creating a new DecCoin with the same denomination and amount as the input coin. If there's no conversion happening, consider renaming the function to better reflect its purpose.

160 changes: 0 additions & 160 deletions types/denom.go

This file was deleted.

Loading
Loading