x/stake: Limit the size of rationals from user input

CHANGELOG.md

@@ -16,7 +16,7 @@ BREAKING CHANGES
 * [stake] remove Tick and add EndBlocker
 * [stake] introduce concept of unbonding for delegations and validators
   * `gaiacli stake unbond` replaced with `gaiacli stake begin-unbonding`
-  * introduced: 
+  * introduced:
     * `gaiacli stake complete-unbonding`
     * `gaiacli stake begin-redelegation`
     * `gaiacli stake complete-redelegation`
@@ -38,7 +38,7 @@ FEATURES
 * [gaiad] unsafe_reset_all now resets addrbook.json
 * [democoin] add x/oracle, x/assoc
 
-FIXES 
+FIXES
 * [gaia] Added self delegation for validators in the genesis creation
 * [lcd] tests now don't depend on raw json text
 * [stake] error strings lower case
@@ -51,6 +51,7 @@ FIXES
 * \#1343 - fixed unnecessary parallelism in CI
 * \#1353 - CLI: Show pool shares fractions in human-readable format
 * \#1258 - printing big.rat's can no longer overflow int64
+* \#887  - limit the size of rationals that can be passed in from user input
 
 IMPROVEMENTS
 * bank module uses go-wire codec instead of 'encoding/json'

types/int.go

@@ -214,6 +214,11 @@ func (i Int) Neg() (res Int) {
 	return Int{neg(i.i)}
 }
 
+// String converts int to string
+func (i Int) String() string {
+	return i.i.String()
+}
+
 // MarshalAmino defines custom encoding scheme
 func (i Int) MarshalAmino() (string, error) {
 	if i.i == nil { // Necessary since default Uint initialization has i.i as nil

types/rational.go

@@ -38,8 +38,8 @@ func NewRat(Numerator int64, Denominator ...int64) Rat {
 }
 
 // create a rational from decimal string or integer string
-func NewRatFromDecimal(decimalStr string) (f Rat, err Error) {
-
+// precision is the number of values after the decimal point which should be read
+func NewRatFromDecimal(decimalStr string, prec int) (f Rat, err Error) {
 	// first extract any negative symbol
 	neg := false
 	if string(decimalStr[0]) == "-" {
@@ -61,6 +61,9 @@ func NewRatFromDecimal(decimalStr string) (f Rat, err Error) {
 		if len(str[0]) == 0 || len(str[1]) == 0 {
 			return f, ErrUnknownRequest("not a decimal string")
 		}
+		if len(str[1]) > prec {
+			str[1] = str[1][:prec]
+		}
 		numStr = str[0] + str[1]
 		len := int64(len(str[1]))
 		denom = new(big.Int).Exp(big.NewInt(10), big.NewInt(len), nil).Int64()
@@ -70,7 +73,17 @@ func NewRatFromDecimal(decimalStr string) (f Rat, err Error) {
 
 	num, errConv := strconv.Atoi(numStr)
 	if errConv != nil {
-		return f, ErrUnknownRequest(errConv.Error())
+		// resort to big int, don't make this default option for efficiency
+		numBig, success := new(big.Int).SetString(numStr, 10)
+		if success != true {
+			return f, ErrUnknownRequest("not a decimal string")
+		}
+
+		if neg {
+			numBig.Neg(numBig)
+		}
+
+		return NewRatFromBigInt(numBig, big.NewInt(denom)), nil
 	}
 
 	if neg {
@@ -105,9 +118,9 @@ func NewRatFromInt(num Int, denom ...Int) Rat {
 }
 
 //nolint
-func (r Rat) Num() int64          { return r.Rat.Num().Int64() }   // Num - return the numerator
-func (r Rat) Denom() int64        { return r.Rat.Denom().Int64() } // Denom  - return the denominator
-func (r Rat) IsZero() bool        { return r.Num() == 0 }          // IsZero - Is the Rat equal to zero
+func (r Rat) Num() Int            { return Int{r.Rat.Num()} }   // Num - return the numerator
+func (r Rat) Denom() Int          { return Int{r.Rat.Denom()} } // Denom  - return the denominator
+func (r Rat) IsZero() bool        { return r.Num().IsZero() }   // IsZero - Is the Rat equal to zero
 func (r Rat) Equal(r2 Rat) bool   { return (r.Rat).Cmp(r2.Rat) == 0 }
 func (r Rat) GT(r2 Rat) bool      { return (r.Rat).Cmp(r2.Rat) == 1 }             // greater than
 func (r Rat) GTE(r2 Rat) bool     { return !r.LT(r2) }                            // greater than or equal

types/rational_test.go

@@ -21,6 +21,8 @@ func TestNew(t *testing.T) {
 }
 
 func TestNewFromDecimal(t *testing.T) {
+	largeBigInt, success := new(big.Int).SetString("3109736052979742687701388262607869", 10)
+	require.True(t, success)
 	tests := []struct {
 		decimalStr string
 		expErr     bool
@@ -31,7 +33,10 @@ func TestNewFromDecimal(t *testing.T) {
 		{"1.1", false, NewRat(11, 10)},
 		{"0.75", false, NewRat(3, 4)},
 		{"0.8", false, NewRat(4, 5)},
-		{"0.11111", false, NewRat(11111, 100000)},
+		{"0.11111", false, NewRat(1111, 10000)},
+		{"628240629832763.5738930323617075341", false, NewRat(3141203149163817869, 5000)},
+		{"621947210595948537540277652521.5738930323617075341",
+			false, NewRatFromBigInt(largeBigInt, big.NewInt(5000))},
 		{".", true, Rat{}},
 		{".0", true, Rat{}},
 		{"1.", true, Rat{}},
@@ -41,17 +46,16 @@ func TestNewFromDecimal(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-
-		res, err := NewRatFromDecimal(tc.decimalStr)
+		res, err := NewRatFromDecimal(tc.decimalStr, 4)
 		if tc.expErr {
 			assert.NotNil(t, err, tc.decimalStr)
 		} else {
-			assert.Nil(t, err)
-			assert.True(t, res.Equal(tc.exp))
+			require.Nil(t, err)
+			require.True(t, res.Equal(tc.exp))
 		}
 
 		// negative tc
-		res, err = NewRatFromDecimal("-" + tc.decimalStr)
+		res, err = NewRatFromDecimal("-"+tc.decimalStr, 4)
 		if tc.expErr {
 			assert.NotNil(t, err, tc.decimalStr)
 		} else {
@@ -133,7 +137,7 @@ func TestArithmetic(t *testing.T) {
 		assert.True(t, tc.resAdd.Equal(tc.r1.Add(tc.r2)), "r1 %v, r2 %v", tc.r1.Rat, tc.r2.Rat)
 		assert.True(t, tc.resSub.Equal(tc.r1.Sub(tc.r2)), "r1 %v, r2 %v", tc.r1.Rat, tc.r2.Rat)
 
-		if tc.r2.Num() == 0 { // panic for divide by zero
+		if tc.r2.Num().IsZero() { // panic for divide by zero
 			assert.Panics(t, func() { tc.r1.Quo(tc.r2) })
 		} else {
 			assert.True(t, tc.resDiv.Equal(tc.r1.Quo(tc.r2)), "r1 %v, r2 %v", tc.r1.Rat, tc.r2.Rat)

x/stake/client/cli/tx.go

@@ -12,6 +12,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/wire"
 	authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli"
 	"github.com/cosmos/cosmos-sdk/x/stake"
+	"github.com/cosmos/cosmos-sdk/x/stake/types"
 )
 
 // create create validator command
@@ -216,7 +217,7 @@ func getShares(storeName string, cdc *wire.Codec, sharesAmountStr, sharesPercent
 	case sharesAmountStr == "" && sharesPercentStr == "":
 		return sharesAmount, errors.Errorf("can either specify the amount OR the percent of the shares, not both")
 	case sharesAmountStr != "":
-		sharesAmount, err = sdk.NewRatFromDecimal(sharesAmountStr)
+		sharesAmount, err = sdk.NewRatFromDecimal(sharesAmountStr, types.MaxBondDenominatorPrecision)
 		if err != nil {
 			return sharesAmount, err
 		}
@@ -225,7 +226,7 @@ func getShares(storeName string, cdc *wire.Codec, sharesAmountStr, sharesPercent
 		}
 	case sharesPercentStr != "":
 		var sharesPercent sdk.Rat
-		sharesPercent, err = sdk.NewRatFromDecimal(sharesPercentStr)
+		sharesPercent, err = sdk.NewRatFromDecimal(sharesPercentStr, types.MaxBondDenominatorPrecision)
 		if err != nil {
 			return sharesAmount, err
 		}

x/stake/client/rest/tx.go

@@ -15,6 +15,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/wire"
 	"github.com/cosmos/cosmos-sdk/x/stake"
+	"github.com/cosmos/cosmos-sdk/x/stake/types"
 )
 
 func registerTxRoutes(ctx context.CoreContext, r *mux.Router, cdc *wire.Codec, kb keys.Keybase) {
@@ -146,7 +147,7 @@ func editDelegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, ctx conte
 				w.Write([]byte(fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())))
 				return
 			}
-			shares, err := sdk.NewRatFromDecimal(msg.SharesAmount)
+			shares, err := sdk.NewRatFromDecimal(msg.SharesAmount, types.MaxBondDenominatorPrecision)
 			if err != nil {
 				w.WriteHeader(http.StatusInternalServerError)
 				w.Write([]byte(fmt.Sprintf("Couldn't decode shares amount. Error: %s", err.Error())))
@@ -211,7 +212,7 @@ func editDelegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, ctx conte
 				w.Write([]byte(fmt.Sprintf("Couldn't decode validator. Error: %s", err.Error())))
 				return
 			}
-			shares, err := sdk.NewRatFromDecimal(msg.SharesAmount)
+			shares, err := sdk.NewRatFromDecimal(msg.SharesAmount, types.MaxBondDenominatorPrecision)
 			if err != nil {
 				w.WriteHeader(http.StatusInternalServerError)
 				w.Write([]byte(fmt.Sprintf("Couldn't decode shares amount. Error: %s", err.Error())))

x/stake/types/errors.go

@@ -79,6 +79,12 @@ func ErrNotEnoughDelegationShares(codespace sdk.CodespaceType, shares string) sd
 func ErrBadSharesAmount(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "shares must be > 0")
 }
+func ErrBadSharesPrecision(codespace sdk.CodespaceType) sdk.Error {
+	return sdk.NewError(codespace, CodeInvalidDelegation,
+		fmt.Sprintf("shares denominator must be < %s, try reducing the number of decimal points",
+			maximumBondingRationalDenominator.String()),
+	)
+}
 func ErrBadSharesPercent(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "shares percent must be >0 and <=1")
 }

x/stake/types/msg.go

@@ -1,18 +1,27 @@
 package types
 
 import (
+	"math"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	crypto "github.com/tendermint/go-crypto"
 )
 
 // name to idetify transaction types
 const MsgType = "stake"
 
-//Verify interface at compile time
+// Maximum amount of decimal points in the decimal representation of rationals
+// used in MsgBeginUnbonding / MsgBeginRedelegate
+const MaxBondDenominatorPrecision = 8
+
+// Verify interface at compile time
 var _, _, _ sdk.Msg = &MsgCreateValidator{}, &MsgEditValidator{}, &MsgDelegate{}
 var _, _ sdk.Msg = &MsgBeginUnbonding{}, &MsgCompleteUnbonding{}
 var _, _ sdk.Msg = &MsgBeginRedelegate{}, &MsgCompleteRedelegate{}
 
+// Initialize Int for the denominator
+var maximumBondingRationalDenominator sdk.Int = sdk.NewInt(int64(math.Pow10(MaxBondDenominatorPrecision)))
+
 //______________________________________________________________________
 
 // MsgCreateValidator - struct for unbonding transactions
@@ -234,6 +243,9 @@ func (msg MsgBeginRedelegate) ValidateBasic() sdk.Error {
 	if msg.SharesAmount.LTE(sdk.ZeroRat()) {
 		return ErrBadSharesAmount(DefaultCodespace)
 	}
+	if msg.SharesAmount.Denom().GT(maximumBondingRationalDenominator) {
+		return ErrBadSharesPrecision(DefaultCodespace)
+	}
 	return nil
 }
 
@@ -340,6 +352,9 @@ func (msg MsgBeginUnbonding) ValidateBasic() sdk.Error {
 	if msg.SharesAmount.LTE(sdk.ZeroRat()) {
 		return ErrBadSharesAmount(DefaultCodespace)
 	}
+	if msg.SharesAmount.Denom().GT(maximumBondingRationalDenominator) {
+		return ErrBadSharesPrecision(DefaultCodespace)
+	}
 	return nil
 }