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

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

Merged
merged 22 commits into from
Feb 15, 2019
Merged

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

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
36c67dc
reconcile DecCoin API with Coin API
Feb 11, 2019
2d4acf5
add NewInt64DecCoin
Feb 11, 2019
6ed2d63
finish reconciliation of dec coins and update types tests
Feb 12, 2019
3ad2bd7
update deccoin API and add validateDenom
Feb 12, 2019
57f0881
update further usage of NewInt64DecCoin
Feb 12, 2019
e512bbd
add a pending log
Feb 12, 2019
92410a7
Merge branch 'develop' into bez/3311-coins-api-recon
Feb 12, 2019
b7715e4
update denom lengths in coins tests
Feb 12, 2019
4531a69
Merge branch 'develop' into bez/3311-coins-api-recon
Feb 12, 2019
d4e07ba
fix unit tests
Feb 12, 2019
997a4ca
update coin tests to use global denom vars
Feb 12, 2019
e21243e
fix distribution unit tests (negative amount)
Feb 12, 2019
a2eb468
remove SameDenomAs
Feb 14, 2019
7ea51ae
Remove redundant code in IsAnyNegative
Feb 14, 2019
1755d0f
cleanup IsEqual
Feb 14, 2019
950b600
Rename IsPositive to IsAllPositive
Feb 14, 2019
b80a494
Merge branch 'develop' into bez/3311-coins-api-recon
Feb 14, 2019
a910027
fix pending
Feb 14, 2019
3a4aae7
minor cleanup
Feb 14, 2019
4b6ec59
fix types tests
Feb 14, 2019
621b21b
panic on diff denom during logic coin methods
Feb 14, 2019
82c2cc1
update i/o message tests to not include messages that panic
Feb 14, 2019
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,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.

* Tendermint

Expand Down
2 changes: 1 addition & 1 deletion baseapp/baseapp_test.go
View file
Open in desktop
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
View file
Open in desktop
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)
}
20 changes: 14 additions & 6 deletions types/coin.go
View file
Open in desktop
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 Down Expand Up @@ -354,9 +353,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 Down Expand Up @@ -403,6 +401,7 @@ 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 {
Expand Down Expand Up @@ -485,6 +484,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