Skip to content

Commit

Permalink
Merge PR cosmos#3555: Reintroduce Fees OR Semantics
Browse files Browse the repository at this point in the history
  • Loading branch information
alexanderbez authored and jackzampolin committed Feb 8, 2019
1 parent 685bfca commit 7bc837a
Show file tree
Hide file tree
Showing 13 changed files with 71 additions and 19 deletions.
1 change: 1 addition & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ BREAKING CHANGES

* Gaia
* [\#3457](https://github.com/cosmos/cosmos-sdk/issues/3457) Changed governance tally validatorGovInfo to use sdk.Int power instead of sdk.Dec
* Reintroduce OR semantics for tx fees

* SDK
* \#2513 Tendermint updates are adjusted by 10^-6 relative to staking tokens,
Expand Down
4 changes: 2 additions & 2 deletions cmd/gaia/cli_test/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,13 @@ func TestGaiaCLIMinimumFees(t *testing.T) {
tests.WaitForNextNBlocksTM(1, f.Port)

// Ensure tx w/ correct fees pass
txFees := fmt.Sprintf("--fees=%s,%s", sdk.NewInt64Coin(feeDenom, 2), sdk.NewInt64Coin(fee2Denom, 2))
txFees := fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(feeDenom, 2))
success, _, _ = f.TxSend(keyFoo, barAddr, sdk.NewInt64Coin(fee2Denom, 10), txFees)
require.True(f.T, success)
tests.WaitForNextNBlocksTM(1, f.Port)

// Ensure tx w/ improper fees fails
txFees = fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(feeDenom, 5))
txFees = fmt.Sprintf("--fees=%s", sdk.NewInt64Coin(feeDenom, 1))
success, _, _ = f.TxSend(keyFoo, barAddr, sdk.NewInt64Coin(fooDenom, 10), txFees)
require.False(f.T, success)

Expand Down
2 changes: 1 addition & 1 deletion docs/gaia/gaiacli.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ the transaction being included in the ledger.
Validator's have a minimum gas price (multi-denom) configuration and they use
this value when when determining if they should include the transaction in a block
during `CheckTx`, where `gasPrices >= minGasPrices`. Note, your transaction must
supply fees that match all the denominations the validator requires.
supply fees that are greater than or equal to __any__ of the denominations the validator requires.

__Note__: With such a mechanism in place, validators may start to prioritize
txs by `gasPrice` in the mempool, so providing higher fees or gas prices may yield
Expand Down
2 changes: 1 addition & 1 deletion docs/spec/auth/gas_fees.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ signature verification, as well as costs proportional to the tx size. Operators
should set minimum gas prices when starting their nodes. They must set the unit
costs of gas in each token denomination they wish to support:

`gaiad start ... --minimum-gas-prices=0.00001steak,0.05photinos`
`gaiad start ... --minimum-gas-prices=0.00001steak;0.05photinos`

When adding transactions to mempool or gossipping transactions, validators check
if the transaction's gas prices, which are determined by the provided fees, meet
Expand Down
4 changes: 2 additions & 2 deletions server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const (
// BaseConfig defines the server's basic configuration
type BaseConfig struct {
// The minimum gas prices a validator is willing to accept for processing a
// transaction. A transaction's fees must meet the minimum of each denomination
// specified in this config (e.g. 0.01photino,0.0001stake).
// transaction. A transaction's fees must meet the minimum of any denomination
// specified in this config (e.g. 0.01photino;0.0001stake).
MinGasPrices string `mapstructure:"minimum-gas-prices"`
}

Expand Down
4 changes: 2 additions & 2 deletions server/config/toml.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ const defaultConfigTemplate = `# This is a TOML config file.
##### main base config options #####
# The minimum gas prices a validator is willing to accept for processing a
# transaction. A transaction's fees must meet the minimum of each denomination
# specified in this config (e.g. 0.01photino,0.0001stake).
# transaction. A transaction's fees must meet the minimum of any denomination
# specified in this config (e.g. 0.01photino;0.0001stake).
minimum-gas-prices = "{{ .BaseConfig.MinGasPrices }}"
`

Expand Down
2 changes: 1 addition & 1 deletion server/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func StartCmd(ctx *Context, appCreator AppCreator) *cobra.Command {
cmd.Flags().String(flagPruning, "syncable", "Pruning strategy: syncable, nothing, everything")
cmd.Flags().String(
FlagMinGasPrices, "",
"Minimum gas prices to accept for transactions; All fees in a tx must meet this minimum (e.g. 0.01photino,0.0001stake)",
"Minimum gas prices to accept for transactions; Any fee in a tx must meet this minimum (e.g. 0.01photino;0.0001stake)",
)

// add support for all Tendermint-specific command line options
Expand Down
20 changes: 20 additions & 0 deletions types/coin.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,26 @@ func (coins Coins) IsAllLTE(coinsB Coins) bool {
return coinsB.IsAllGTE(coins)
}

// IsAnyGTE returns true iff coins contains at least one denom that is present
// at a greater or equal amount in coinsB; it returns false otherwise.
//
// NOTE: IsAnyGTE operates under the invariant that both coin sets are sorted
// by denominations and there exists no zero coins.
func (coins Coins) IsAnyGTE(coinsB Coins) bool {
if len(coinsB) == 0 {
return false
}

for _, coin := range coins {
amt := coinsB.AmountOf(coin.Denom)
if coin.Amount.GTE(amt) {
return true
}
}

return false
}

// IsZero returns true if there are no coins or all coins are zero.
func (coins Coins) IsZero() bool {
for _, coin := range coins {
Expand Down
19 changes: 19 additions & 0 deletions types/coin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,3 +508,22 @@ func TestAmountOf(t *testing.T) {

assert.Panics(t, func() { cases[0].coins.AmountOf("Invalid") })
}

func TestCoinsIsAnyGTE(t *testing.T) {
one := NewInt(1)
two := NewInt(2)

assert.False(t, Coins{}.IsAnyGTE(Coins{}))
assert.False(t, Coins{{"a", one}}.IsAnyGTE(Coins{}))
assert.False(t, Coins{}.IsAnyGTE(Coins{{"a", one}}))
assert.False(t, Coins{{"a", one}}.IsAnyGTE(Coins{{"a", two}}))
assert.True(t, Coins{{"a", one}, {"b", two}}.IsAnyGTE(Coins{{"a", two}, {"b", one}}))
assert.True(t, Coins{{"a", one}}.IsAnyGTE(Coins{{"a", one}}))
assert.True(t, Coins{{"a", two}}.IsAnyGTE(Coins{{"a", one}}))
assert.True(t, Coins{{"a", one}}.IsAnyGTE(Coins{{"a", one}, {"b", two}}))
assert.True(t, Coins{{"b", two}}.IsAnyGTE(Coins{{"a", one}, {"b", two}}))
assert.False(t, Coins{{"b", one}}.IsAnyGTE(Coins{{"a", one}, {"b", two}}))
assert.True(t, Coins{{"a", one}, {"b", two}}.IsAnyGTE(Coins{{"a", one}, {"b", one}}))
assert.True(t, Coins{{"a", one}, {"b", one}}.IsAnyGTE(Coins{{"a", one}, {"b", two}}))
assert.True(t, Coins{{"x", one}, {"y", one}}.IsAnyGTE(Coins{{"b", one}, {"c", one}, {"y", one}, {"z", one}}))
}
4 changes: 3 additions & 1 deletion types/dec_coin.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package types

import (
"fmt"
"regexp"
"sort"
"strings"

Expand Down Expand Up @@ -375,7 +376,8 @@ func ParseDecCoins(coinsStr string) (coins DecCoins, err error) {
return nil, nil
}

coinStrs := strings.Split(coinsStr, ",")
splitRe := regexp.MustCompile(",|;")
coinStrs := splitRe.Split(coinsStr, -1)
for _, coinStr := range coinStrs {
coin, err := ParseDecCoin(coinStr)
if err != nil {
Expand Down
8 changes: 8 additions & 0 deletions types/int.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ func equal(i *big.Int, i2 *big.Int) bool { return i.Cmp(i2) == 0 }

func gt(i *big.Int, i2 *big.Int) bool { return i.Cmp(i2) == 1 }

func gte(i *big.Int, i2 *big.Int) bool { return i.Cmp(i2) >= 0 }

func lt(i *big.Int, i2 *big.Int) bool { return i.Cmp(i2) == -1 }

func lte(i *big.Int, i2 *big.Int) bool { return i.Cmp(i2) <= 0 }
Expand Down Expand Up @@ -188,6 +190,12 @@ func (i Int) GT(i2 Int) bool {
return gt(i.i, i2.i)
}

// GTE returns true if receiver Int is greater than or equal to the parameter
// Int.
func (i Int) GTE(i2 Int) bool {
return gte(i.i, i2.i)
}

// LT returns true if first Int is lesser than second
func (i Int) LT(i2 Int) bool {
return lt(i.i, i2.i)
Expand Down
2 changes: 1 addition & 1 deletion x/auth/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ func EnsureSufficientMempoolFees(ctx sdk.Context, stdFee StdFee) sdk.Result {
requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt())
}

if !stdFee.Amount.IsAllGTE(requiredFees) {
if !stdFee.Amount.IsAnyGTE(requiredFees) {
return sdk.ErrInsufficientFee(
fmt.Sprintf(
"insufficient fees; got: %q required: %q", stdFee.Amount, requiredFees,
Expand Down
18 changes: 10 additions & 8 deletions x/auth/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -710,23 +710,25 @@ func TestEnsureSufficientMempoolFees(t *testing.T) {
input := setupTestInput()
ctx := input.ctx.WithMinGasPrices(
sdk.DecCoins{
sdk.NewDecCoinFromDec("photino", sdk.NewDecWithPrec(1000000, sdk.Precision)), // 0.0001photino
sdk.NewDecCoinFromDec("stake", sdk.NewDecWithPrec(10000, sdk.Precision)), // 0.000001stake
sdk.NewDecCoinFromDec("photino", sdk.NewDecWithPrec(50000000000000, sdk.Precision)), // 0.0001photino
sdk.NewDecCoinFromDec("stake", sdk.NewDecWithPrec(10000000000000, sdk.Precision)), // 0.000001stake
},
)

testCases := []struct {
input StdFee
expectedOK bool
}{
{NewStdFee(200000, sdk.Coins{sdk.NewInt64Coin("photino", 5)}), false},
{NewStdFee(200000, sdk.Coins{sdk.NewInt64Coin("stake", 1)}), false},
{NewStdFee(200000, sdk.Coins{sdk.NewInt64Coin("photino", 20)}), false},
{NewStdFee(200000, sdk.Coins{sdk.NewInt64Coin("stake", 2)}), true},
{NewStdFee(200000, sdk.Coins{sdk.NewInt64Coin("photino", 10)}), true},
{
NewStdFee(
200000,
sdk.Coins{
sdk.NewInt64Coin("photino", 20),
sdk.NewInt64Coin("stake", 1),
sdk.NewInt64Coin("photino", 10),
sdk.NewInt64Coin("stake", 2),
},
),
true,
Expand All @@ -735,9 +737,9 @@ func TestEnsureSufficientMempoolFees(t *testing.T) {
NewStdFee(
200000,
sdk.Coins{
sdk.NewInt64Coin("atom", 2),
sdk.NewInt64Coin("photino", 20),
sdk.NewInt64Coin("stake", 1),
sdk.NewInt64Coin("atom", 5),
sdk.NewInt64Coin("photino", 10),
sdk.NewInt64Coin("stake", 2),
},
),
true,
Expand Down

0 comments on commit 7bc837a

Please sign in to comment.