Skip to content

Commit

Permalink
Merge pull request #10751 from vegaprotocol/10750
Browse files Browse the repository at this point in the history
fix: Handle cancellation of order on entering auction for party in is…
  • Loading branch information
jeremyletang authored Feb 26, 2024
2 parents 1291262 + 7ae7c34 commit 36b665c
Show file tree
Hide file tree
Showing 11 changed files with 245 additions and 42 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@
- [10725](https://github.com/vegaprotocol/vega/issues/10725) - Batch proposal votes to contain `ELS` per market.
- [10744](https://github.com/vegaprotocol/vega/issues/10744) - Prevent governance suspension of a market already governance suspended.
- [10374](https://github.com/vegaprotocol/vega/issues/10374) - Ledger entries did not return data when filtering by transfer id.
- [10750](https://github.com/vegaprotocol/vega/issues/10750) - Handle cancellation of order on entering auction for party in isolated margin mode.
- [10748](https://github.com/vegaprotocol/vega/issues/10748) - Ensure apply fees cannot fail.
- [10752](https://github.com/vegaprotocol/vega/issues/10752) - Handle amend in place correctly for failure in isolated margin check.
- [10753](https://github.com/vegaprotocol/vega/issues/10753) - Handle the case that a submitted order is `FoK` in isolated margin to not double discount it from position.
- [10136](https://github.com/vegaprotocol/vega/issues/10136) - Assure opening auction uncrossing price gets registered in the perps engine.
- [](https://github.com/vegaprotocol/vega/issues/xxx)

## 0.74.3

Expand Down
4 changes: 0 additions & 4 deletions commands/batch_market_instructions.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ func checkBatchMarketInstructions(cmd *commandspb.BatchMarketInstructions) Error
return errs.FinalAddForProperty("batch_market_instructions", ErrIsRequired)
}

if len(cmd.UpdateMarginMode) > 0 {
return errs.FinalAddForProperty("batch_market_instructions.update_margin_mode", ErrIsDisabled)
}

// there's very little to verify here, only if the batch is not empty
// all transaction verification is done when processing then.
if len(cmd.Cancellations)+
Expand Down
4 changes: 1 addition & 3 deletions commands/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,7 @@ func CheckInputData(rawInputData []byte) (*commandspb.InputData, Errors) {
case *commandspb.InputData_ApplyReferralCode:
errs.Merge(checkApplyReferralCode(cmd.ApplyReferralCode))
case *commandspb.InputData_UpdateMarginMode:
// FIXME: Disable Update margin mode for now
errs.AddForProperty("update_margin_mode", ErrIsDuplicated)
// errs.Merge(checkUpdateMarginMode(cmd.UpdateMarginMode))
errs.Merge(checkUpdateMarginMode(cmd.UpdateMarginMode))
case *commandspb.InputData_JoinTeam:
errs.Merge(checkJoinTeam(cmd.JoinTeam))
case *commandspb.InputData_UpdatePartyProfile:
Expand Down
19 changes: 19 additions & 0 deletions core/collateral/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,25 @@ func (e *Engine) TransferFeesContinuousTrading(ctx context.Context, marketID str
return e.transferFees(ctx, marketID, assetID, ft)
}

func (e *Engine) PartyCanCoverFees(asset, mktID, partyID string, amount *num.Uint) error {
generalAccount, err := e.GetPartyGeneralAccount(partyID, asset)
if err != nil {
return err
}
generalAccountBalance := generalAccount.Balance
marginAccount, _ := e.GetPartyMarginAccount(mktID, partyID, asset)

marginAccountBalance := num.UintZero()
if marginAccount != nil {
marginAccountBalance = marginAccount.Balance
}

if num.Sum(generalAccountBalance, marginAccountBalance).LT(amount) {
return fmt.Errorf("party has insufficient funds to cover fees")
}
return nil
}

func (e *Engine) transferFees(ctx context.Context, marketID string, assetID string, ft events.FeesTransfer) ([]*types.LedgerMovement, error) {
makerFee, infraFee, liquiFee, err := e.getFeesAccounts(marketID, assetID)
if err != nil {
Expand Down
36 changes: 36 additions & 0 deletions core/collateral/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package collateral_test
import (
"context"
"encoding/hex"
"fmt"
"strconv"
"testing"
"time"
Expand Down Expand Up @@ -428,6 +429,41 @@ func testFeesTransferContinuousNoTransfer(t *testing.T) {
assert.Nil(t, err)
}

func TestPartyHasSufficientBalanceForFees(t *testing.T) {
eng := getTestEngine(t)
defer eng.Finish()

party := "myparty"
// create party
eng.broker.EXPECT().Send(gomock.Any()).Times(3)
gen, err := eng.CreatePartyGeneralAccount(context.Background(), party, testMarketAsset)
require.NoError(t, err)

mar, err := eng.CreatePartyMarginAccount(context.Background(), party, testMarketID, testMarketAsset)
require.NoError(t, err)

// add funds
eng.broker.EXPECT().Send(gomock.Any()).Times(1)
err = eng.UpdateBalance(context.Background(), gen, num.NewUint(100))
assert.Nil(t, err)

// there's no margin account balance but the party has enough funds in the margin account
require.Nil(t, eng.PartyCanCoverFees(testMarketAsset, testMarketID, party, num.NewUint(50)))

// there's no margin account balance and the party has insufficient funds to cover fees in the general account
require.Error(t, fmt.Errorf("party has insufficient funds to cover fees"), eng.PartyCanCoverFees(testMarketAsset, testMarketID, party, num.NewUint(101)))

eng.broker.EXPECT().Send(gomock.Any()).Times(1)
err = eng.UpdateBalance(context.Background(), mar, num.NewUint(500))
assert.Nil(t, err)

// there's enough in the margin + general to cover the fees
require.Nil(t, eng.PartyCanCoverFees(testMarketAsset, testMarketID, party, num.NewUint(101)))

// there's not enough in the margin + general to cover the fees
require.Error(t, fmt.Errorf("party has insufficient funds to cover fees"), eng.PartyCanCoverFees(testMarketAsset, testMarketID, party, num.NewUint(601)))
}

func testReleasePartyMarginAccount(t *testing.T) {
eng := getTestEngine(t)
defer eng.Finish()
Expand Down
1 change: 1 addition & 0 deletions core/execution/common/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ type Collateral interface {
ReleaseFromHoldingAccount(ctx context.Context, transfer *types.Transfer) (*types.LedgerMovement, error)
ClearSpotMarket(ctx context.Context, mktID, quoteAsset string) ([]*types.LedgerMovement, error)
PartyHasSufficientBalance(asset, partyID string, amount *num.Uint) error
PartyCanCoverFees(asset, mktID, partyID string, amount *num.Uint) error
TransferSpot(ctx context.Context, partyID, toPartyID, asset string, quantity *num.Uint) (*types.LedgerMovement, error)
GetOrCreatePartyLiquidityFeeAccount(ctx context.Context, partyID, marketID, asset string) (*types.Account, error)
GetPartyLiquidityFeeAccount(market, partyID, asset string) (*types.Account, error)
Expand Down
14 changes: 14 additions & 0 deletions core/execution/common/mocks/mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

115 changes: 86 additions & 29 deletions core/execution/future/market.go
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,10 @@ func (m *Market) BlockEnd(ctx context.Context) {
markPriceCopy = m.markPriceCalculator.GetPrice().Clone()
}
m.liquidity.EndBlock(markPriceCopy, m.midPrice(), m.positionFactor)

if !m.matching.CheckBook() {
m.log.Panic("ontick book has orders pegged to nothing")
}
}

func (m *Market) removeAllStopOrders(
Expand Down Expand Up @@ -2299,6 +2303,11 @@ func (m *Market) submitValidatedOrder(ctx context.Context, order *types.Order) (

order.Status = types.OrderStatusActive

var aggressorFee *num.Uint
if fees != nil {
aggressorFee = fees.TotalFeesAmountPerParty()[order.Party]
}

// NB: this is the position with the trades included and the order sizes updated to remaining!!!
// NB: this is not touching the actual position from the position engine but is all done on a clone, so that
// in handle confirmation this will be done as per normal.
Expand All @@ -2313,14 +2322,23 @@ func (m *Market) submitValidatedOrder(ctx context.Context, order *types.Order) (
// If the trade would decrease the party's position, that portion will trade and margin will be released as in the Decreasing Position.
// If the order is not persistent this is the end, if it is persistent any portion of the order which
// has not traded in step 1 will move to being placed on the order book.
if len(trades) > 0 && marginMode == types.MarginModeIsolatedMargin {
if err := m.updateIsolatedMarginOnAggressor(ctx, posWithTrades, order, trades, false); err != nil {
if m.log.GetLevel() <= logging.DebugLevel {
m.log.Debug("Unable to check/add immediate trade margin for party",
logging.Order(*order), logging.Error(err))
if len(trades) > 0 {
if marginMode == types.MarginModeIsolatedMargin {
// check that the party can cover the trade AND the fees
if err := m.updateIsolatedMarginOnAggressor(ctx, posWithTrades, order, trades, false, aggressorFee); err != nil {
if m.log.GetLevel() <= logging.DebugLevel {
m.log.Debug("Unable to check/add immediate trade margin for party",
logging.Order(*order), logging.Error(err))
}
_ = m.position.UnregisterOrder(ctx, order)
return nil, nil, common.ErrMarginCheckFailed
}
} else if aggressorFee != nil {
if err := m.collateral.PartyCanCoverFees(m.settlementAsset, m.mkt.ID, order.Party, aggressorFee); err != nil {
m.log.Error("insufficient funds to cover fees", logging.Order(order), logging.Error(err))
m.unregisterAndReject(ctx, order, types.OrderErrorInsufficientFundsToPayFees)
return nil, nil, err
}
_ = m.position.UnregisterOrder(ctx, order)
return nil, nil, common.ErrMarginCheckFailed
}
}

Expand Down Expand Up @@ -2351,7 +2369,7 @@ func (m *Market) submitValidatedOrder(ctx context.Context, order *types.Order) (
// the contains the fees information
confirmation.Trades = trades

if marginMode == types.MarginModeIsolatedMargin {
if marginMode == types.MarginModeIsolatedMargin && order.Status == types.OrderStatusActive && order.TrueRemaining() > 0 {
// now we need to check if the party has sufficient funds to cover the order margin for the remaining size
// if not the remaining order is cancelled.
// if successful the required order margin are transferred to the order margin account.
Expand All @@ -2363,17 +2381,22 @@ func (m *Market) submitValidatedOrder(ctx context.Context, order *types.Order) (
_ = m.unregisterAndReject(
ctx, order, types.OrderErrorMarginCheckFailed)
m.matching.RemoveOrder(order.ID)
if len(trades) > 0 {
if err = m.applyFees(ctx, order, fees); err != nil {
m.log.Panic("failed to apply fees on order", logging.Order(order), logging.String("aggressor total fees", fees.TotalFeesAmountPerParty()[order.ID].String()), logging.Error(err))
}
// if there were trades we need to return the confirmation so the trades can be handled
// otherwise they were just removed from the book for the passive side and gone
orderUpdates := m.handleConfirmation(ctx, confirmation, nil)
return confirmation, orderUpdates, common.ErrMarginCheckFailed
}
return nil, nil, common.ErrMarginCheckFailed
}
}

if fees != nil {
err = m.applyFees(ctx, order, fees)
if err != nil {
_ = m.unregisterAndReject(
ctx, order, types.OrderErrorMarginCheckFailed)
m.matching.RemoveOrder(order.ID)
return nil, nil, common.ErrMarginCheckFailed
if err = m.applyFees(ctx, order, fees); err != nil {
m.log.Panic("failed to apply fees on order", logging.Order(order), logging.String("aggressor total fees", fees.TotalFeesAmountPerParty()[order.ID].String()), logging.Error(err))
}
}

Expand Down Expand Up @@ -2974,7 +2997,7 @@ func (m *Market) checkMarginForOrder(ctx context.Context, pos *positions.MarketP

// updateIsolatedMarginOnAggressor is called when a new or amended order is matched immediately upon submission.
// it checks that new margin requirements can be satisfied and if so transfers the margin from the general account to the margin account.
func (m *Market) updateIsolatedMarginOnAggressor(ctx context.Context, pos *positions.MarketPosition, order *types.Order, trades []*types.Trade, isAmend bool) error {
func (m *Market) updateIsolatedMarginOnAggressor(ctx context.Context, pos *positions.MarketPosition, order *types.Order, trades []*types.Trade, isAmend bool, fees *num.Uint) error {
marketObservable, mpos, increment, _, marginFactor, orders, err := m.getIsolatedMarginContext(pos, order)
if err != nil {
return err
Expand Down Expand Up @@ -3007,7 +3030,11 @@ func (m *Market) updateIsolatedMarginOnAggressor(ctx context.Context, pos *posit
}
}

risk, err := m.risk.UpdateIsolatedMarginOnAggressor(ctx, mpos, marketObservable, increment, clonedOrders, trades, marginFactor, order.Side, isAmend)
aggressorFee := num.UintZero()
if fees != nil {
aggressorFee = fees.Clone()
}
risk, err := m.risk.UpdateIsolatedMarginOnAggressor(ctx, mpos, marketObservable, increment, clonedOrders, trades, marginFactor, order.Side, isAmend, aggressorFee)
if err != nil {
return err
}
Expand All @@ -3017,6 +3044,21 @@ func (m *Market) updateIsolatedMarginOnAggressor(ctx context.Context, pos *posit
return m.transferMargins(ctx, risk, nil)
}

func (m *Market) updateIsolatedMarginOnOrderCancel(ctx context.Context, mpos *positions.MarketPosition, order *types.Order) error {
marketObservable, pos, increment, auctionPrice, marginFactor, orders, err := m.getIsolatedMarginContext(mpos, order)
if err != nil {
return err
}
risk, err := m.risk.UpdateIsolatedMarginOnOrderCancel(ctx, pos, orders, marketObservable, auctionPrice, increment, marginFactor)
if err != nil {
return err
}
if risk == nil {
return nil
}
return m.transferMargins(ctx, []events.Risk{risk}, nil)
}

func (m *Market) updateIsolatedMarginOnOrder(ctx context.Context, mpos *positions.MarketPosition, order *types.Order) error {
marketObservable, pos, increment, auctionPrice, marginFactor, orders, err := m.getIsolatedMarginContext(mpos, order)
if err != nil {
Expand Down Expand Up @@ -3307,9 +3349,11 @@ func (m *Market) cancelOrder(ctx context.Context, partyID, orderID string) (*typ
// order margin
if foundOnBook && m.getMarginMode(partyID) == types.MarginModeIsolatedMargin {
pos, _ := m.position.GetPositionByPartyID(partyID)
if err := m.updateIsolatedMarginOnOrder(ctx, pos, order); err != nil {
m.log.Panic("failed to update order margin after order cancellation", logging.Order(order), logging.String("party", pos.Party()))
}
// it might be that we place orders before an auction, then during an auction we're trying to cancel the order - if we have still other order
// they will definitely have insufficient order margin but that's ok, either they will be fine when uncrossing the auction
// or will get cancelled then, no need to punish the party and cancel them at this point. Therefore this can either release funds
// from the order account or error which we ignore.
m.updateIsolatedMarginOnOrderCancel(ctx, pos, order)
}

return &types.OrderCancellationConfirmation{Order: order}, nil
Expand Down Expand Up @@ -3384,6 +3428,7 @@ func (m *Market) AmendOrderWithIDGenerator(

conf, updatedOrders, err := m.amendOrder(ctx, orderAmendment, party)
if err != nil {
m.log.Error("failed to amend order", logging.String("marketID", orderAmendment.MarketID), logging.String("orderID", orderAmendment.OrderID), logging.Error(err))
if m.getMarginMode(party) == types.MarginModeIsolatedMargin && err == common.ErrMarginCheckFailed {
m.handleIsolatedMarginInsufficientOrderMargin(ctx, party)
}
Expand Down Expand Up @@ -3688,6 +3733,8 @@ func (m *Market) amendOrder(
if err := m.updateIsolatedMarginOnOrder(ctx, pos, amendedOrder); err == risk.ErrInsufficientFundsForMarginInGeneralAccount {
m.log.Error("party has insufficient margin to cover the order change, going to cancel all orders for the party")
_ = m.position.AmendOrder(ctx, amendedOrder, existingOrder)
// we're amending the order back in the order book so that when we unregister it, it would match what the position expects
m.orderAmendInPlace(amendedOrder, existingOrder)
return nil, nil, common.ErrMarginCheckFailed
}
}
Expand Down Expand Up @@ -3769,8 +3816,7 @@ func (m *Market) orderCancelReplace(
}
if fees != nil {
if feeErr := m.applyFees(ctx, newOrder, fees); feeErr != nil {
_ = m.position.AmendOrder(ctx, newOrder, existingOrder)
return
m.log.Panic("orderCancelReplace failed to apply fees on order", logging.Order(newOrder), logging.String("aggressor total fees", fees.TotalFeesAmountPerParty()[newOrder.ID].String()), logging.Error(feeErr))
}
}
orders = m.handleConfirmation(ctx, conf, nil)
Expand Down Expand Up @@ -3830,18 +3876,29 @@ func (m *Market) orderCancelReplace(
return nil, nil, errors.New("could not calculate fees for order")
}

var aggressorFee *num.Uint
if fees != nil {
aggressorFee = fees.TotalFeesAmountPerParty()[newOrder.Party]
}

marginMode := m.getMarginMode(newOrder.Party)
pos, _ := m.position.GetPositionByPartyID(newOrder.Party)
posWithTrades := pos
if len(trades) > 0 && marginMode == types.MarginModeIsolatedMargin {
posWithTrades = pos.UpdateInPlaceOnTrades(m.log, newOrder.Side, trades, newOrder)
// NB: this is the position with the trades included and the order sizes updated to remaining!!!
if err = m.updateIsolatedMarginOnAggressor(ctx, posWithTrades, newOrder, trades, true); err != nil {
if m.log.GetLevel() <= logging.DebugLevel {
m.log.Debug("Unable to check/add immediate trade margin for party",
logging.Order(*newOrder), logging.Error(err))
if len(trades) > 0 {
if marginMode == types.MarginModeIsolatedMargin {
posWithTrades = pos.UpdateInPlaceOnTrades(m.log, newOrder.Side, trades, newOrder)
if err = m.updateIsolatedMarginOnAggressor(ctx, posWithTrades, newOrder, trades, true, aggressorFee); err != nil {
if m.log.GetLevel() <= logging.DebugLevel {
m.log.Debug("Unable to check/add immediate trade margin for party",
logging.Order(*newOrder), logging.Error(err))
}
return nil, nil, common.ErrMarginCheckFailed
}
} else if aggressorFee != nil {
if err := m.collateral.PartyCanCoverFees(m.settlementAsset, m.mkt.ID, newOrder.Party, aggressorFee); err != nil {
m.log.Error("insufficient funds to cover fees", logging.Order(newOrder), logging.Error(err))
return nil, nil, err
}
return nil, nil, common.ErrMarginCheckFailed
}
}

Expand Down
28 changes: 28 additions & 0 deletions core/matching/orderbook.go
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,34 @@ func (b *OrderBook) CancelAllOrders(party string) ([]*types.OrderCancellationCon
return confs, err
}

func (b *OrderBook) CheckBook() bool {
if len(b.buy.levels) > 0 {
allPegged := true
for _, o := range b.buy.levels[len(b.buy.levels)-1].orders {
if o.PeggedOrder == nil || o.PeggedOrder.Reference != types.PeggedReferenceBestBid {
allPegged = false
break
}
}
if allPegged {
return false
}
}
if len(b.sell.levels) > 0 {
allPegged := true
for _, o := range b.sell.levels[len(b.sell.levels)-1].orders {
if o.PeggedOrder == nil || o.PeggedOrder.Reference != types.PeggedReferenceBestAsk {
allPegged = false
break
}
}
if allPegged {
return false
}
}
return true
}

// CancelOrder cancel an order that is active on an order book. Market and Order ID are validated, however the order must match
// the order on the book with respect to side etc. The caller will typically validate this by using a store, we should
// not trust that the external world can provide these values reliably.
Expand Down
1 change: 1 addition & 0 deletions core/risk/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ var (
ErrInsufficientFundsForOrderMargin = errors.New("insufficient funds for order margin")
ErrInsufficientFundsForMarginInGeneralAccount = errors.New("insufficient funds to cover margin in general margin")
ErrRiskFactorsNotAvailableForAsset = errors.New("risk factors not available for the specified asset")
ErrInsufficientFundsToCoverTradeFees = errors.New("insufficient funds to cover fees")
)

const RiskFactorStateVarName = "risk-factors"
Expand Down
Loading

0 comments on commit 36b665c

Please sign in to comment.