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

R4R: Reconcile DecCoin/s API with Coin/s API #3607

Merged
merged 22 commits into from
Feb 15, 2019
Merged
Show file tree
Hide file tree
Changes from 21 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
2 changes: 2 additions & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ IMPROVEMENTS
* Gaia

* SDK
* [\#3311] Reconcile the `DecCoin/s` API with the `Coin/s` API.
* [\#3614] Add coin denom length checks to the coins constructors.
* \#3621 remove many inter-module dependancies
* [\#3601] JSON-stringify the ABCI log response which includes the log and message
index.
Expand Down
2 changes: 1 addition & 1 deletion baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ func TestBaseAppOptionSeal(t *testing.T) {
}

func TestSetMinGasPrices(t *testing.T) {
minGasPrices := sdk.DecCoins{sdk.NewDecCoin("stake", 5000)}
minGasPrices := sdk.DecCoins{sdk.NewInt64DecCoin("stake", 5000)}
app := newBaseApp(t.Name(), SetMinGasPrices(minGasPrices.String()))
require.Equal(t, minGasPrices, app.minGasPrices)
}
Expand Down
2 changes: 1 addition & 1 deletion server/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ func TestDefaultConfig(t *testing.T) {

func TestSetMinimumFees(t *testing.T) {
cfg := DefaultConfig()
cfg.SetMinGasPrices(sdk.DecCoins{sdk.NewDecCoin("foo", 5)})
cfg.SetMinGasPrices(sdk.DecCoins{sdk.NewInt64DecCoin("foo", 5)})
require.Equal(t, "5.000000000000000000foo", cfg.MinGasPrices)
}
59 changes: 35 additions & 24 deletions types/coin.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,11 @@ type Coin struct {
// NewCoin returns a new coin with a denomination and amount. It will panic if
// the amount is negative.
func NewCoin(denom string, amount Int) Coin {
validateDenom(denom)

if amount.LT(ZeroInt()) {
panic(fmt.Sprintf("negative coin amount: %v\n", amount))
}
if strings.ToLower(denom) != denom {
panic(fmt.Sprintf("denom cannot contain upper case characters: %s\n", denom))
}

return Coin{
Denom: denom,
Expand All @@ -51,11 +50,6 @@ func (coin Coin) String() string {
return fmt.Sprintf("%v%v", coin.Amount, coin.Denom)
}

// SameDenomAs returns true if the two coins are the same denom
func (coin Coin) SameDenomAs(other Coin) bool {
return (coin.Denom == other.Denom)
}

// IsZero returns if this represents no money
func (coin Coin) IsZero() bool {
return coin.Amount.IsZero()
Expand All @@ -64,24 +58,36 @@ func (coin Coin) IsZero() bool {
// 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 {
return coin.SameDenomAs(other) && (!coin.Amount.LT(other.Amount))
if coin.Denom != other.Denom {
panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, other.Denom))
}

return !coin.Amount.LT(other.Amount)
}

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

return coin.Amount.LT(other.Amount)
}

// IsEqual returns true if the two sets of Coins have the same value
func (coin Coin) IsEqual(other Coin) bool {
return coin.SameDenomAs(other) && (coin.Amount.Equal(other.Amount))
if coin.Denom != other.Denom {
panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, other.Denom))
}

return coin.Amount.Equal(other.Amount)
}

// Adds amounts of two coins with same denom. If the coins differ in denom then
// it panics.
func (coin Coin) Plus(coinB Coin) Coin {
if !coin.SameDenomAs(coinB) {
if coin.Denom != coinB.Denom {
panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, coinB.Denom))
}

Expand All @@ -91,7 +97,7 @@ func (coin Coin) Plus(coinB Coin) Coin {
// Subtracts amounts of two coins with same denom. If the coins differ in denom
// then it panics.
func (coin Coin) Minus(coinB Coin) Coin {
if !coin.SameDenomAs(coinB) {
if coin.Denom != coinB.Denom {
panic(fmt.Sprintf("invalid coin denominations; %s, %s", coin.Denom, coinB.Denom))
}

Expand Down Expand Up @@ -273,7 +279,7 @@ func (coins Coins) IsAllGT(coinsB Coins) bool {
return false
}

return diff.IsPositive()
return diff.IsAllPositive()
}

// IsAllGTE returns true iff for every denom in coins, the denom is present at
Expand Down Expand Up @@ -339,7 +345,7 @@ func (coins Coins) IsEqual(coinsB Coins) bool {
coinsB = coinsB.Sort()

for i := 0; i < len(coins); i++ {
if coins[i].Denom != coinsB[i].Denom || !coins[i].Amount.Equal(coinsB[i].Amount) {
if !coins[i].IsEqual(coinsB[i]) {
return false
}
}
Expand All @@ -354,9 +360,8 @@ func (coins Coins) Empty() bool {

// Returns the amount of a denom from coins
func (coins Coins) AmountOf(denom string) Int {
if strings.ToLower(denom) != denom {
panic(fmt.Sprintf("denom cannot contain upper case characters: %s\n", denom))
}
validateDenom(denom)

switch len(coins) {
case 0:
return ZeroInt()
Expand All @@ -382,11 +387,11 @@ func (coins Coins) AmountOf(denom string) Int {
}
}

// IsPositive returns true if there is at least one coin and all currencies
// IsAllPositive returns true if there is at least one coin and all currencies
// have a positive value.
//
// TODO: Remove once unsigned integers are used.
func (coins Coins) IsPositive() bool {
func (coins Coins) IsAllPositive() bool {
if len(coins) == 0 {
return false
}
Expand All @@ -403,12 +408,9 @@ func (coins Coins) IsPositive() bool {
// IsAnyNegative returns true if there is at least one coin whose amount
// is negative; returns false otherwise. It returns false if the coin set
// is empty too.
//
// TODO: Remove once unsigned integers are used.
func (coins Coins) IsAnyNegative() bool {
if len(coins) == 0 {
return false
}

for _, coin := range coins {
if coin.IsNegative() {
return true
Expand Down Expand Up @@ -485,6 +487,15 @@ var (
reDecCoin = regexp.MustCompile(fmt.Sprintf(`^(%s)%s(%s)$`, reDecAmt, reSpc, reDnm))
)

func validateDenom(denom string) {
if len(denom) < 3 || len(denom) > 16 {
Copy link
Contributor

Choose a reason for hiding this comment

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

look like magic numbers to me

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm how so? This is just for validation, we can pick any consistent length range.

panic(fmt.Sprintf("invalid denom length: %s", denom))
}
if strings.ToLower(denom) != denom {
panic(fmt.Sprintf("denom cannot contain upper case characters: %s", denom))
}
}

// ParseCoin parses a cli input for one coin type, returning errors if invalid.
// This returns an error on an empty string as well.
func ParseCoin(coinStr string) (coin Coin, err error) {
Expand Down
Loading