Skip to content

Commit

Permalink
Merge pull request #10178 from vegaprotocol/10177-fix-overflow
Browse files Browse the repository at this point in the history
fix: validate against large order sizes that may caught internal over…
  • Loading branch information
wwestgarth authored and jeremyletang committed Nov 29, 2023
1 parent d2242e6 commit 0a8b84c
Show file tree
Hide file tree
Showing 8 changed files with 153 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### 🐛 Fixes

- [10166](https://github.com/vegaprotocol/vega/issues/10166) - Closed markets should not be subscribed to data sources when restored from a snapshot.
- [10177](https://github.com/vegaprotocol/vega/issues/10177) - Add validation that order sizes are not strangely large.

## 0.73.6

Expand Down Expand Up @@ -55,6 +56,7 @@

- [9941](https://github.com/vegaprotocol/vega/issues/9941) - Add data node mapping for `WasEligible` field in referral set.
- [9940](https://github.com/vegaprotocol/vega/issues/9940) - Truncate fee stats in quantum down to the 6 decimal.
- [10125](https://github.com/vegaprotocol/vega/issues/10125) - Wire the `JoinTeam` command in the wallet.

## 0.73.0

Expand Down
1 change: 1 addition & 0 deletions commands/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ var (
ErrMustBeLessThen366 = errors.New("must be less then 366")
ErrMustBeAtMost500 = errors.New("must be at most 500")
ErrMustBeWithinRangeGT0LT20 = errors.New("price range must be strictly greater than 0 and less than or equal to 20")
ErrSizeIsTooLarge = errors.New("size is too large")
)

type Errors map[string][]error
Expand Down
6 changes: 6 additions & 0 deletions commands/order_submission.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package commands

import (
"errors"
"math"
"math/big"

types "code.vegaprotocol.io/vega/protos/vega"
Expand Down Expand Up @@ -72,6 +73,11 @@ func checkOrderSubmission(cmd *commandspb.OrderSubmission) Errors {
errs.AddForProperty("order_submission.size", ErrMustBePositive)
}

// just make sure its not some silly big number because we do sometimes cast to int64s
if cmd.Size > math.MaxInt64/2 {
errs.AddForProperty("order_submission.size", ErrSizeIsTooLarge)
}

if cmd.TimeInForce == types.Order_TIME_IN_FORCE_GTT {
if cmd.ExpiresAt <= 0 {
errs.AddForProperty("order_submission.expires_at", ErrMustBePositive)
Expand Down
10 changes: 8 additions & 2 deletions commands/order_submission_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package commands_test

import (
"errors"
"math"
"testing"

"code.vegaprotocol.io/vega/commands"
Expand All @@ -37,7 +38,7 @@ func TestCheckOrderSubmission(t *testing.T) {
t.Run("Submitting an order with NETWORK type fails", testOrderSubmissionWithNetworkTypeFails)
t.Run("Submitting an order with undefined time in force fails", testOrderSubmissionWithUndefinedTimeInForceFails)
t.Run("Submitting an order with unspecified time in force fails", testOrderSubmissionWithUnspecifiedTimeInForceFails)
t.Run("Submitting an order with non-positive size fails", testOrderSubmissionWithNonPositiveSizeFails)
t.Run("Submitting an order with non-positive size fails", testOrderSubmissionWithInvalidSizeFails)
t.Run("Submitting an order with GTT and non-positive expiration date fails", testOrderSubmissionWithGTTAndNonPositiveExpirationDateFails)
t.Run("Submitting an order without GTT and expiration date fails", testOrderSubmissionWithoutGTTAndExpirationDateFails)
t.Run("Submitting an order with MARKET type and price fails", testOrderSubmissionWithMarketTypeAndPriceFails)
Expand Down Expand Up @@ -319,14 +320,19 @@ func testOrderSubmissionWithUndefinedTimeInForceFails(t *testing.T) {
assert.Contains(t, err.Get("order_submission.time_in_force"), commands.ErrIsNotValid)
}

func testOrderSubmissionWithNonPositiveSizeFails(t *testing.T) {
func testOrderSubmissionWithInvalidSizeFails(t *testing.T) {
// FIXME(big int) doesn't test negative numbers since it's an unsigned int
// but that will definitely be needed when moving to big int.
err := checkOrderSubmission(&commandspb.OrderSubmission{
Size: 0,
})

assert.Contains(t, err.Get("order_submission.size"), commands.ErrMustBePositive)

err = checkOrderSubmission(&commandspb.OrderSubmission{
Size: math.MaxInt64,
})
assert.Contains(t, err.Get("order_submission.size"), commands.ErrSizeIsTooLarge)
}

func testOrderSubmissionWithGTTAndNonPositiveExpirationDateFails(t *testing.T) {
Expand Down
8 changes: 8 additions & 0 deletions core/execution/future/market.go
Original file line number Diff line number Diff line change
Expand Up @@ -1939,6 +1939,10 @@ func (m *Market) submitOrder(ctx context.Context, order *types.Order) (*types.Or
return nil, nil, err
}

if err := m.position.ValidateOrder(order); err != nil {
return nil, nil, err
}

// Now that validation is handled, call the code to place the order
orderConf, orderUpdates, err := m.submitValidatedOrder(ctx, order)
if err == nil {
Expand Down Expand Up @@ -3218,6 +3222,10 @@ func (m *Market) amendOrder(
return nil, nil, err
}

if err := m.position.ValidateAmendOrder(existingOrder, amendedOrder); err != nil {
return nil, nil, err
}

// We do this first, just in case the party would also have
// change the expiry, and that would have been catched by
// the follow up checks, so we do not insert a non-existing
Expand Down
22 changes: 22 additions & 0 deletions core/positions/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,28 @@ func (e *Engine) ReloadConf(cfg Config) {
e.cfgMu.Unlock()
}

// ValidateOrder checks that the given order can be registered.
func (e *Engine) ValidateOrder(order *types.Order) error {
pos, found := e.positions[order.Party]
if !found {
pos = NewMarketPosition(order.Party)
}
return pos.ValidateOrderRegistration(order.TrueRemaining(), order.Side)
}

// ValidateAmendOrder checks that replace the given order with a new order will no cause issues with position tracking.
func (e *Engine) ValidateAmendOrder(order *types.Order, newOrder *types.Order) error {
pos, found := e.positions[order.Party]
if !found {
pos = NewMarketPosition(order.Party)
}

if order.TrueRemaining() >= newOrder.TrueRemaining() {
return nil
}
return pos.ValidateOrderRegistration(newOrder.TrueRemaining()-order.TrueRemaining(), order.Side)
}

// RegisterOrder updates the potential positions for a submitted order, as though
// the order were already accepted.
// It returns the updated position.
Expand Down
65 changes: 65 additions & 0 deletions core/positions/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package positions_test
import (
"context"
"encoding/hex"
"math"
"testing"
"time"

Expand All @@ -39,6 +40,70 @@ func TestUpdatePosition(t *testing.T) {
t.Run("Update position network trade as seller", testUpdatePositionNetworkSell)
}

func TestRegisterLargeOrder(t *testing.T) {
const (
buysize int64 = 123
sellsize int64 = 456
)
ctx := context.Background()
e := getTestEngine(t)
orderBuy := types.Order{
Party: "test_party",
Side: types.SideBuy,
Size: uint64(buysize),
Remaining: uint64(buysize),
Price: num.UintZero(),
}

assert.NoError(t, e.ValidateOrder(&orderBuy))
_ = e.RegisterOrder(ctx, &orderBuy)

// now say we're going to do another MASSIVE one
orderBuy2 := types.Order{
Party: "test_party",
Side: types.SideBuy,
Size: math.MaxInt64,
Remaining: math.MaxInt64,
Price: num.UintZero(),
}
assert.Error(t, e.ValidateOrder(&orderBuy2)) // should fail because if we registered it'll overflow
require.Panics(t, func() {
e.RegisterOrder(ctx, &orderBuy2) // should panic and not silently overflow
})
}

func TestAmendLargeOrder(t *testing.T) {
const (
buysize int64 = 123
sellsize int64 = 456
)
ctx := context.Background()
e := getTestEngine(t)
orderBuy := types.Order{
Party: "test_party",
Side: types.SideBuy,
Size: uint64(buysize),
Remaining: uint64(buysize),
Price: num.UintZero(),
}

assert.NoError(t, e.ValidateOrder(&orderBuy))
_ = e.RegisterOrder(ctx, &orderBuy)

// now say we're going to do an amend to a massive one
orderBuy2 := types.Order{
Party: "test_party",
Side: types.SideBuy,
Size: math.MaxUint64,
Remaining: math.MaxUint64,
Price: num.UintZero(),
}
assert.Error(t, e.ValidateAmendOrder(&orderBuy, &orderBuy2)) // should fail because if we registered it'll overflow
require.Panics(t, func() {
e.RegisterOrder(ctx, &orderBuy2) // should panic and not silently overflow
})
}

func TestGetOpenInterest(t *testing.T) {
engine := getTestEngine(t)
assert.Empty(t, engine.Positions())
Expand Down
41 changes: 41 additions & 0 deletions core/positions/market_position.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
package positions

import (
"errors"
"fmt"
"math"

"code.vegaprotocol.io/vega/core/types"
"code.vegaprotocol.io/vega/libs/num"
Expand Down Expand Up @@ -86,6 +88,13 @@ func (p *MarketPosition) UpdateOnOrderChange(log *logging.Logger, side types.Sid
logging.Int64("potential-buy", p.buy),
logging.Uint64("size-change", sizeChange))
}

if add && p.buy > math.MaxInt64-iSizeChange {
log.Panic("order too large to register, will overflow",
logging.Int64("potential-buy", p.buy),
logging.Uint64("size-change", sizeChange))
}

// recalculate sumproduct
if add {
p.buySumProduct.Add(p.buySumProduct, num.UintZero().Mul(price, num.NewUint(sizeChange)))
Expand All @@ -107,6 +116,13 @@ func (p *MarketPosition) UpdateOnOrderChange(log *logging.Logger, side types.Sid
logging.Int64("potential-sell", p.sell),
logging.Uint64("size-change", sizeChange))
}

if add && p.sell > math.MaxInt64-iSizeChange {
log.Panic("order too large to register, will overflow",
logging.Int64("potential-sell", p.sell),
logging.Uint64("size-change", sizeChange))
}

// recalculate sumproduct
if add {
p.sellSumProduct.Add(p.sellSumProduct, num.UintZero().Mul(price, num.NewUint(sizeChange)))
Expand Down Expand Up @@ -212,6 +228,31 @@ func (p MarketPosition) VWSell() *num.Uint {
return num.UintZero()
}

// ValidateOrder returns an error is the order is so large that the position engine does not have the precision
// to register it.
func (p MarketPosition) ValidateOrderRegistration(s uint64, side types.Side) error {
size := int64(s)
if size == 0 {
return nil
}

// check that the cast to int64 hasn't pushed it backwards
if size < 0 {
return errors.New("cannot register position without causing overflow")
}

amt := p.buy
if side == types.SideSell {
amt = p.sell
}

if size > math.MaxInt64-amt {
return errors.New("cannot register position without causing overflow")
}

return nil
}

func (p MarketPosition) OrderReducesExposure(ord *types.Order) bool {
if ord == nil || p.Size() == 0 || ord.PeggedOrder != nil {
return false
Expand Down

0 comments on commit 0a8b84c

Please sign in to comment.