certusone
/
cosmos-sdk
mirror of https://github.com/certusone/cosmos-sdk.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge PR #1415: x/stake: Limit the size of rationals from user input 

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

This commit sets the maximum number of decimal points that can be
passed in from messages. This is enforced on the validate basic of
MsgBeginUnbonding and MsgBeginRedelegation. The cli has been
updated to truncate the user input to the specified precision. This
also updates types/rational to return big ints for Num() and Den().

Closes #887

* Switch NewFromDecimal to error instead of truncating
This commit is contained in:
Dev Ojha 2018-06-29 13:30:12 -07:00 committed by Christopher Goes
parent 0d28eda146
commit 47e4682d9f
8 changed files with 71 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -67,6 +67,7 @@ FIXES
* \#1367 - set ChainID in InitChain
* \#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'

4
types/int.go
View File

@ -227,6 +227,10 @@ func (i Int) Neg() (res Int) {
return Int{neg(i.i)}
}
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

29
types/rational.go
View File

@ -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 {
return f, ErrUnknownRequest("string has too many decimals")
}
numStr = str[0] + str[1]
len := int64(len(str[1]))
denom = new(big.Int).Exp(big.NewInt(10), big.NewInt(len), nil).Int64()
@ -69,8 +72,20 @@ func NewRatFromDecimal(decimalStr string) (f Rat, err Error) {
}
num, errConv := strconv.Atoi(numStr)
if errConv != nil {
return f, ErrUnknownRequest(errConv.Error())
if errConv != nil && strings.HasSuffix(errConv.Error(), "value out of range") {
// 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
} else if errConv != nil {
return f, ErrUnknownRequest("not a decimal string")
}
if neg {
@ -105,9 +120,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

25
types/rational_test.go
View File

@ -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,13 @@ 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", true, NewRat(1111, 10000)},
{"628240629832763.5738930323617075341", true, NewRat(3141203149163817869, 5000)},
{"621947210595948537540277652521.5738930323617075341",
true, NewRatFromBigInt(largeBigInt, big.NewInt(5000))},
{"628240629832763.5738", false, NewRat(3141203149163817869, 5000)},
{"621947210595948537540277652521.5738",
false, NewRatFromBigInt(largeBigInt, big.NewInt(5000))},
{".", true, Rat{}},
{".0", true, Rat{}},
{"1.", true, Rat{}},
@ -41,22 +49,21 @@ 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, tc.decimalStr)
require.True(t, res.Equal(tc.exp), tc.decimalStr)
}
// negative tc
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.Mul(NewRat(-1))))
assert.Nil(t, err, tc.decimalStr)
assert.True(t, res.Equal(tc.exp.Mul(NewRat(-1))), tc.decimalStr)
}
}
}
@ -133,7 +140,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)

5
x/stake/client/cli/tx.go
View File

@ -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
@ -219,7 +220,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
}
@ -228,7 +229,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
}

5
x/stake/client/rest/tx.go
View File

@ -14,6 +14,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) {
@ -145,7 +146,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())))
@ -210,7 +211,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())))

6
x/stake/types/errors.go
View File

@ -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")
}

17
x/stake/types/msg.go
View File

@ -1,6 +1,8 @@
package types
import (
"math"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/tendermint/tendermint/crypto"
)
@ -8,11 +10,18 @@ import (
// 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
}