mirror of
https://github.com/d0zingcat/gocryptotrader.git
synced 2026-05-29 15:10:37 +00:00
gctrpc/order manager: Add ModifyOrder endpoint (#724)
* gctcli: modifyorder stubs * gctcli: add ModifyOrderRequest and ModifyOrderResponse in rpc.proto * gctcli: regenerate rpc.pb.go after the addition of ModifyOrder structs * gctrpc: add ModifyOrder() and regenerate dependent files * gctcli: modifyorder command now uses newly generated ModifyOrder() RPC * exchanges/order/orders.go: use time.Time.Equal() instead of == * gctrpc: update ModifyOrderRequest and ModifyResponse and regenerate gRPC * gctcli/commands: rework modifyorder * engine: implement RPCServer.ModifyOrder * engine: commit an initial version OrderManager.Modify(), still does not update state of managed orders * engine: OrderManager.Modify now updates the inner state of managed orders, but introduces race conditions, needs fixes * engine/order_manager.go: comply with golangci-lint * gctcli: fix getOrderCommand.ArgsUsage * gctcli: fix getModifyOrderCommand args and ArgsUsage * engine: OrderManager.Modify() now correctly updates price of modified order * engine: RPCServer.ModifyOrder now uses checkParams() as advised * exchanges: (1) IBotExchange.ModifyOrder now returns a Modify struct, (2) all exchanges are updated to comply with that change * exchanges/order: Detail.UpdateOrderFromModify also updates the ID * engine/order_manager: add store.modifyExisting() and use it in OrderManager.Modify to update (on success) the state of the modified order * exchanges: Bitfinex.ModifyOrder() now returns the ID in case of an error * engine: OrdetManager.Modify() now emits an order event * exchanges/bithumb: proper order.payment_currency key * engine/order_manager: populate more Modify fields as they are needed by (some) exchanges, add comments * engine: test OrderManager.Modify() * engine: test store.modifyExisting() * engine: write a docstring for store.modifyExisting * engine: OrderManager.Modify() now also sets Modify.Price and Modify.Amount in case of zero values * engine: TestOrderManager_Modify() now verify the effects of price and/or amount set to 0 * engine: OrderManger.Modify() now uses the commsManager to let observers know of errors * engine: TestOrderManager_Modify() uses t.Fatal() * engine: TestOrderManager_Modify() and TestStore_modifyOrder() supply t.Error() with proper messages * exchanges/order_manager_test: fix a golangci-lint complaint * engine/order_manager: fix an error comparison bug and simplify * gctcli/commands: check if either price or amount is set, otherwise we would waste an API call
This commit is contained in:
Yordan Miladinov
GitHub
@@ -284,6 +284,77 @@ func (m *OrderManager) validate(newOrder *order.Submit) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Modify depends on the order.Modify.ID and order.Modify.Exchange fields to uniquely
|
||||
// identify an order to modify.
|
||||
func (m *OrderManager) Modify(mod *order.Modify) (*order.ModifyResponse, error) {
|
||||
if m == nil {
|
||||
return nil, fmt.Errorf("order manager %w", ErrNilSubsystem)
|
||||
}
|
||||
if atomic.LoadInt32(&m.started) == 0 {
|
||||
return nil, fmt.Errorf("order manager %w", ErrSubSystemNotStarted)
|
||||
}
|
||||
|
||||
// Fetch details from locally managed order store.
|
||||
det, err := m.orderStore.getByExchangeAndID(mod.Exchange, mod.ID)
|
||||
if det == nil || err != nil {
|
||||
return nil, fmt.Errorf("order does not exist: %w", err)
|
||||
}
|
||||
|
||||
// Populate additional Modify fields as some of them are required by various
|
||||
// exchange implementations.
|
||||
mod.Pair = det.Pair // Used by Bithumb.
|
||||
mod.Side = det.Side // Used by Bithumb.
|
||||
mod.PostOnly = det.PostOnly // Used by Poloniex.
|
||||
mod.ImmediateOrCancel = det.ImmediateOrCancel // Used by Poloniex.
|
||||
|
||||
// Following is just a precaution to not modify orders by mistake if exchange
|
||||
// implementations do not check fields of the Modify struct for zero values.
|
||||
if mod.Amount == 0 {
|
||||
mod.Amount = det.Amount
|
||||
}
|
||||
if mod.Price == 0 {
|
||||
mod.Price = det.Price
|
||||
}
|
||||
|
||||
// Get exchange instance and submit order modification request.
|
||||
exch := m.orderStore.exchangeManager.GetExchangeByName(mod.Exchange)
|
||||
if exch == nil {
|
||||
return nil, ErrExchangeNotFound
|
||||
}
|
||||
res, err := exch.ModifyOrder(mod)
|
||||
if err != nil {
|
||||
message := fmt.Sprintf(
|
||||
"Order manager: Exchange %s order ID=%v: failed to modify",
|
||||
mod.Exchange,
|
||||
mod.ID,
|
||||
)
|
||||
m.orderStore.commsManager.PushEvent(base.Event{
|
||||
Type: "order",
|
||||
Message: message,
|
||||
})
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// If modification is successful, apply changes to local order store.
|
||||
//
|
||||
// XXX: This comes with a race condition, because [request -> changes] are not
|
||||
// atomic.
|
||||
err = m.orderStore.modifyExisting(mod.ID, &res)
|
||||
|
||||
// Notify observers.
|
||||
var message string
|
||||
if err != nil {
|
||||
message = "Order manager: Exchange %s order ID=%v: modified on exchange, but failed to modify locally"
|
||||
} else {
|
||||
message = "Order manager: Exchange %s order ID=%v: modified successfully"
|
||||
}
|
||||
m.orderStore.commsManager.PushEvent(base.Event{
|
||||
Type: "order",
|
||||
Message: fmt.Sprintf(message, mod.Exchange, res.ID),
|
||||
})
|
||||
return &order.ModifyResponse{OrderID: res.ID}, err
|
||||
}
|
||||
|
||||
// Submit will take in an order struct, send it to the exchange and
|
||||
// populate it in the OrderManager if successful
|
||||
func (m *OrderManager) Submit(newOrder *order.Submit) (*OrderSubmitResponse, error) {
|
||||
@@ -653,6 +724,26 @@ func (s *store) updateExisting(od *order.Detail) error {
|
||||
return ErrOrderNotFound
|
||||
}
|
||||
|
||||
// modifyExisting depends on mod.Exchange and given ID to uniquely identify an order and
|
||||
// modify it.
|
||||
func (s *store) modifyExisting(id string, mod *order.Modify) error {
|
||||
s.m.Lock()
|
||||
defer s.m.Unlock()
|
||||
r, ok := s.Orders[strings.ToLower(mod.Exchange)]
|
||||
if !ok {
|
||||
return ErrExchangeNotFound
|
||||
}
|
||||
for x := range r {
|
||||
if r[x].ID == id {
|
||||
r[x].UpdateOrderFromModify(mod)
|
||||
return nil
|
||||
}
|
||||
}
|
||||
return ErrOrderNotFound
|
||||
}
|
||||
|
||||
// upsert (1) checks if such an exchange exists in the exchangeManager, (2) checks if
|
||||
// order exists and updates/creates it.
|
||||
func (s *store) upsert(od *order.Detail) error {
|
||||
lName := strings.ToLower(od.Exchange)
|
||||
exch := s.exchangeManager.GetExchangeByName(lName)
|
||||
|
||||
@@ -40,6 +40,12 @@ func (f omfExchange) GetOrderInfo(orderID string, pair currency.Pair, assetType
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (f omfExchange) ModifyOrder(action *order.Modify) (order.Modify, error) {
|
||||
ans := *action
|
||||
ans.ID = "modified_order_id"
|
||||
return ans, nil
|
||||
}
|
||||
|
||||
func TestSetupOrderManager(t *testing.T) {
|
||||
_, err := SetupOrderManager(nil, nil, nil, false)
|
||||
if !errors.Is(err, errNilExchangeManager) {
|
||||
@@ -333,6 +339,55 @@ func TestExists(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestStore_modifyOrder(t *testing.T) {
|
||||
m := OrdersSetup(t)
|
||||
pair := currency.Pair{
|
||||
Base: currency.NewCode("XXXXX"),
|
||||
Quote: currency.NewCode("YYYYY"),
|
||||
}
|
||||
err := m.orderStore.add(&order.Detail{
|
||||
Exchange: testExchange,
|
||||
AssetType: asset.Spot,
|
||||
Pair: pair,
|
||||
ID: "fake_order_id",
|
||||
|
||||
Price: 8,
|
||||
Amount: 128,
|
||||
})
|
||||
if err != nil {
|
||||
t.Error(err)
|
||||
}
|
||||
|
||||
err = m.orderStore.modifyExisting("fake_order_id", &order.Modify{
|
||||
Exchange: testExchange,
|
||||
|
||||
ID: "another_fake_order_id",
|
||||
Price: 16,
|
||||
Amount: 256,
|
||||
})
|
||||
if err != nil {
|
||||
t.Error(err)
|
||||
}
|
||||
|
||||
_, err = m.orderStore.getByExchangeAndID(testExchange, "fake_order_id")
|
||||
if err == nil {
|
||||
// Expected error, such an order should not exist anymore in the store.
|
||||
t.Error("Expected error")
|
||||
}
|
||||
|
||||
det, err := m.orderStore.getByExchangeAndID(testExchange, "another_fake_order_id")
|
||||
if det == nil || err != nil {
|
||||
t.Fatal("Failed to fetch order details")
|
||||
}
|
||||
if det.ID != "another_fake_order_id" || det.Price != 16 || det.Amount != 256 {
|
||||
t.Errorf(
|
||||
"have (%s,%f,%f), want (%s,%f,%f)",
|
||||
det.ID, det.Price, det.Amount,
|
||||
"another_fake_order_id", 16., 256.,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCancelOrder(t *testing.T) {
|
||||
m := OrdersSetup(t)
|
||||
|
||||
@@ -555,6 +610,96 @@ func TestSubmit(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestOrderManager_Modify(t *testing.T) {
|
||||
pair := currency.Pair{
|
||||
Base: currency.NewCode("XXXXX"),
|
||||
Quote: currency.NewCode("YYYYY"),
|
||||
}
|
||||
f := func(mod order.Modify, expectError bool, price, amount float64) {
|
||||
t.Helper()
|
||||
|
||||
m := OrdersSetup(t)
|
||||
err := m.orderStore.add(&order.Detail{
|
||||
Exchange: testExchange,
|
||||
AssetType: asset.Spot,
|
||||
Pair: pair,
|
||||
ID: "fake_order_id",
|
||||
//
|
||||
Price: 8,
|
||||
Amount: 128,
|
||||
})
|
||||
if err != nil {
|
||||
t.Error(err)
|
||||
}
|
||||
|
||||
resp, err := m.Modify(&mod)
|
||||
if expectError {
|
||||
if err == nil {
|
||||
t.Fatal("Expected error")
|
||||
}
|
||||
return
|
||||
} else if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if resp.OrderID != "modified_order_id" {
|
||||
t.Errorf("have \"%s\", want \"modified_order_id\"", resp.OrderID)
|
||||
}
|
||||
|
||||
det, err := m.orderStore.getByExchangeAndID(testExchange, resp.OrderID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if det.ID != resp.OrderID || det.Price != price || det.Amount != amount {
|
||||
t.Errorf(
|
||||
"have (%s,%f,%f), want (%s,%f,%f)",
|
||||
det.ID, det.Price, det.Amount,
|
||||
resp.OrderID, price, amount,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
model := order.Modify{
|
||||
// These fields identify the order.
|
||||
Exchange: testExchange,
|
||||
AssetType: asset.Spot,
|
||||
Pair: pair,
|
||||
ID: "fake_order_id",
|
||||
// These fields modify the order.
|
||||
Price: 0,
|
||||
Amount: 0,
|
||||
}
|
||||
|
||||
// [1] Test if nonexistent order returns an error.
|
||||
one := model
|
||||
one.ID = "nonexistent_order_id"
|
||||
f(one, true, 0, 0)
|
||||
|
||||
// [2] Test if price of 0 is ignored.
|
||||
two := model
|
||||
two.Price = 0
|
||||
two.Amount = 256
|
||||
f(two, false, 8, 256)
|
||||
|
||||
// [3] Test if amount of 0 is ignored.
|
||||
three := model
|
||||
three.Price = 16
|
||||
three.Amount = 0
|
||||
f(three, false, 16, 128)
|
||||
|
||||
// [4] Test if both fields work together.
|
||||
four := model
|
||||
four.Price = 16
|
||||
four.Amount = 256
|
||||
f(four, false, 16, 256)
|
||||
|
||||
// [5] Test if both fields missing modifies anything but the ID.
|
||||
five := model
|
||||
five.Price = 0
|
||||
five.Amount = 0
|
||||
f(five, false, 8, 128)
|
||||
}
|
||||
|
||||
func TestProcessOrders(t *testing.T) {
|
||||
m := OrdersSetup(t)
|
||||
m.processOrders()
|
||||
|
||||
@@ -1361,6 +1361,40 @@ func (s *RPCServer) CancelAllOrders(_ context.Context, r *gctrpc.CancelAllOrders
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (s *RPCServer) ModifyOrder(_ context.Context, r *gctrpc.ModifyOrderRequest) (*gctrpc.ModifyOrderResponse, error) {
|
||||
assetType, err := asset.New(r.Asset)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
pair := currency.Pair{
|
||||
Delimiter: r.Pair.Delimiter,
|
||||
Base: currency.NewCode(r.Pair.Base),
|
||||
Quote: currency.NewCode(r.Pair.Quote),
|
||||
}
|
||||
exch := s.GetExchangeByName(r.Exchange)
|
||||
err = checkParams(r.Exchange, exch, assetType, pair)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
mod := order.Modify{
|
||||
Exchange: r.Exchange,
|
||||
AssetType: assetType,
|
||||
Pair: pair,
|
||||
ID: r.OrderId,
|
||||
|
||||
Amount: r.Amount,
|
||||
Price: r.Price,
|
||||
}
|
||||
resp, err := s.OrderManager.Modify(&mod)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &gctrpc.ModifyOrderResponse{
|
||||
ModifiedOrderId: resp.OrderID,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// GetEvents returns the stored events list
|
||||
func (s *RPCServer) GetEvents(_ context.Context, _ *gctrpc.GetEventsRequest) (*gctrpc.GetEventsResponse, error) {
|
||||
return &gctrpc.GetEventsResponse{}, common.ErrNotYetImplemented
|
||||
|
||||
Reference in New Issue
Block a user