codebase: Consolidate orders functionality and other minor fixes #1957

thrasher- · 2025-06-26T01:37:06Z

PR Description

Introduces order status and type unmarshalling and rolls it out across the codebase where possible and some other minor fixes along the way.

Depends on #1948

Type of change

New feature (non-breaking change which adds functionality)

How has this been tested

go test ./... -race
golangci-lint run

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation and regenerated documentation via the documentation tool
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally and on Github Actions with my changes
Any dependent changes have been merged and published in downstream modules

Copilot

Pull Request Overview

This PR standardizes timestamp parsing by introducing a new DateTime type alongside the existing Time wrapper, and applies these types across the codebase to replace raw numeric/unparse logic. It also streamlines exchange wrappers and WebSocket handlers to use strongly typed fields for order sides, statuses, and times, eliminating repetitive manual parsing.

Added DateTime type with JSON unmarshal support and tests.
Replaced raw int64/float64 timestamps with types.Time/types.DateTime in exchange models.
Simplified wrapper and WS code by using direct .Time() and typed order.Side/order.Type/order.Status fields.

Reviewed Changes

Copilot reviewed 121 out of 121 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
types/time.go	Added `DateTime` type and UnmarshalJSON implementation
types/time_test.go	New tests for `DateTime` unmarshalling
exchanges/yobit/yobit_wrapper.go	Switched raw timestamps to `types.Time` and removed manual parsing
exchanges/yobit/yobit_types.go	Updated model fields to use `types.Time` and `order.Side`
exchanges/order/orders.go	Extended `StringToOrderSide/Type/Status` and JSON hooks

Comments suppressed due to low confidence (2)

types/time.go:86

The UnmarshalJSON method uses fmt.Errorf but the fmt package is not imported. Add import "fmt" at the top.

func (d *DateTime) UnmarshalJSON(data []byte) error {

exchanges/order/orders.go:1086

The code calls reflect.TypeFor but the reflect package is not imported. Add import "reflect" to this file.

		return &json.UnmarshalTypeError{Value: string(data), Type: reflect.TypeFor[Side](), Offset: 1}

shazbert

Sexy clean. Here have some nits.

shazbert · 2025-06-29T04:39:01Z

exchanges/binance/binance_types.go

+	CommissionAsset                   currency.Code `json:"N"`
+	TransactionTime                   types.Time    `json:"T"`
+	TradeID                           int64         `json:"t"`
+	Ignored                           int64         `json:"I"` // Must be ignored explicitly, otherwise it overwrites 'i'.


int64 -> json.RawMessage

shazbert · 2025-06-29T04:39:49Z

exchanges/binance/binance_types.go

+	Ignored                           int64         `json:"I"` // Must be ignored explicitly, otherwise it overwrites 'i'.
+	IsOnOrderBook                     bool          `json:"w"`
+	IsMaker                           bool          `json:"m"`
+	Ignored2                          bool          `json:"M"` // See the comment for "I".


bool -> json.RawMessage

shazbert · 2025-06-29T04:46:08Z

exchanges/binanceus/binanceus_types.go

+	CommissionAsset                   currency.Code `json:"N"`
+	TransactionTime                   types.Time    `json:"T"`
+	TradeID                           int64         `json:"t"`
+	Ignored                           int64         `json:"I"` // Must be ignored explicitly, otherwise it overwrites 'i'.


type to json.RawMessage

shazbert · 2025-06-29T04:46:13Z

exchanges/binanceus/binanceus_types.go

+	Ignored                           int64         `json:"I"` // Must be ignored explicitly, otherwise it overwrites 'i'.
+	IsOnOrderBook                     bool          `json:"w"`
+	IsMaker                           bool          `json:"m"`
+	Ignored2                          bool          `json:"M"` // See the comment for "I".


type to json.RawMessage

exchanges/bitmex/bitmex_types.go

exchanges/deribit/deribit_websocket.go

shazbert · 2025-06-29T05:20:01Z

exchanges/deribit/deribit_websocket.go

+	case len(channels) == 5:
+		// on the “5-item” feed we always treat every push as a snapshot
+		return d.Websocket.Orderbook.LoadSnapshot(book)
+


rm spaces between cases

shazbert · 2025-06-29T05:21:02Z

exchanges/deribit/deribit_websocket.go

 		})
+
+	case len(channels) == 5:


merge with first case and only allocate orderbook.Book on LoadSnapshot call.

shazbert · 2025-06-29T05:23:20Z

exchanges/deribit/deribit_websocket.go

+	}
+	asks := buildLevels(ob.Asks)
+	bids := buildLevels(ob.Bids)
+	if len(asks) == 0 && len(bids) == 0 {


This happening a lot? If you can check that the LastUpdateID is sequential even when no action has occurred on the books between the update frequency window we can actually load zero values in now.

exchanges/deribit/deribit_wrapper.go

codecov · 2025-07-02T06:44:15Z

Codecov Report

❌ Patch coverage is 41.02142% with 358 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.94%. Comparing base (d5b2cf1) to head (0fb38ca).

Files with missing lines	Patch %	Lines
exchanges/kucoin/kucoin_wrapper.go	5.26%	36 Missing ⚠️
exchanges/btcmarkets/btcmarkets_wrapper.go	3.03%	32 Missing ⚠️
exchanges/okx/okx_wrapper.go	4.00%	24 Missing ⚠️
exchanges/deribit/deribit_websocket.go	58.92%	16 Missing and 7 partials ⚠️
exchanges/deribit/deribit_wrapper.go	17.85%	23 Missing ⚠️
exchanges/poloniex/poloniex_wrapper.go	0.00%	18 Missing ⚠️
exchanges/binance/binance_wrapper.go	39.28%	17 Missing ⚠️
exchanges/gateio/gateio_wrapper.go	29.16%	17 Missing ⚠️
exchanges/bitmex/bitmex_wrapper.go	11.11%	16 Missing ⚠️
exchanges/btse/btse_wrapper.go	6.66%	14 Missing ⚠️
... and 27 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1957      +/-   ##
==========================================
+ Coverage   39.70%   39.94%   +0.24%     
==========================================
  Files         434      434              
  Lines      171748   170185    -1563     
==========================================
- Hits        68185    67979     -206     
+ Misses      96417    95166    -1251     
+ Partials     7146     7040     -106

Files with missing lines	Coverage Δ
exchanges/binance/binance_types.go	`100.00% <ø> (ø)`
exchanges/binance/binance_ufutures.go	`30.44% <100.00%> (-0.15%)`	⬇️
exchanges/binance/cfutures_types.go	`100.00% <ø> (ø)`
exchanges/binanceus/binanceus_types.go	`100.00% <ø> (ø)`
exchanges/bitfinex/bitfinex_types.go	`90.90% <ø> (ø)`
exchanges/bitflyer/bitflyer_types.go	`50.00% <ø> (ø)`
exchanges/bitflyer/bitflyer_wrapper.go	`42.64% <100.00%> (+0.26%)`	⬆️
exchanges/bithumb/bithumb_types.go	`100.00% <ø> (ø)`
exchanges/bitstamp/bitstamp_wrapper.go	`56.88% <100.00%> (+0.18%)`	⬆️
exchanges/btcmarkets/btcmarkets_types.go	`100.00% <ø> (ø)`
... and 48 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

gbjk

Nice cleanup! 🎉

A few 🚧 changes, 🐒 nitpicks and 🦍 groomings.

exchanges/binance/binance_types.go

exchanges/binance/binance_wrapper.go

exchanges/binanceus/binanceus_types.go

exchanges/bitfinex/bitfinex_wrapper.go

exchanges/bitmex/bitmex_types.go

exchanges/huobi/huobi_websocket.go

exchanges/kraken/kraken_websocket.go

exchanges/order/orders.go

samuael

The PR looks grea. just added minor reviews for now

samuael · 2025-07-21T20:31:04Z

exchanges/bithumb/bithumb_types.go

@@ -61,7 +62,7 @@ type TransactionHistory struct {
 	Data   []struct {
 		ContNumber      int64          `json:"cont_no,string"`
 		TransactionDate types.DateTime `json:"transaction_date"`
-		Type            string         `json:"type"`
+		Side            order.Side     `json:"type"`


Is this really a side or type?

Yep this and other exchanges have their order side as "type" https://apidocs.bithumb.com/v1.2.0/reference/%EA%B1%B0%EB%9E%98-%EC%A3%BC%EB%AC%B8%EB%82%B4%EC%97%AD-%EC%A1%B0%ED%9A%8C

samuael · 2025-07-21T20:50:43Z

exchanges/btse/btse_wrapper.go

-				"%s GetOrderInfo unable to parse currency pair: %s\n",
-				e.Name,
-				err)
-		}
 		od.Exchange = e.Name


can't we instantiate this like ```
od = order.Detail{
Exchange: e.Name,
...
}

samuael · 2025-07-21T21:01:56Z

exchanges/exmo/exmo_types.go

-	Amount   float64    `json:"amount"`
+	TradeID  int64         `json:"trade_id"`
+	Date     types.Time    `json:"date"`
+	Side     order.Side    `json:"type"`


side or type??

Side, despite them using type as well 😆

json { "BTC_USD": [ { "order_id": "14", "client_id": "100500", "created": "1435517311", "type": "buy", "pair": "BTC_USD", "price": "100", "quantity": "1", "amount": "100" } ] }

samuael · 2025-07-21T21:02:36Z

exchanges/exmo/exmo_types.go

+		Timestamp types.Time    `json:"dt"`
+		Type      string        `json:"type"`
+		Currency  currency.Code `json:"curr"`
+		Status    string        `json:"status"`


Is this not order.Status?

Nope this one is for wallet transactions

exchanges/binance/binance_types.go

gloriousCode

What a slog! 😴
Tried to focus on areas where custom functions were used versus straight type replacement occurred.

gloriousCode · 2025-08-19T03:17:57Z

exchanges/btcmarkets/btcmarkets_wrapper.go

-		resp.Status = order.Cancelled
-	case orderPartiallyCancelled:
-		resp.Status = order.PartiallyCancelled
-	case orderFailed:


This and a few other strings here are unused as a result of this change and should be removed

gloriousCode · 2025-08-19T03:51:03Z

exchanges/gemini/gemini_types.go

@@ -161,7 +162,7 @@ type TradeHistory struct {
 	Amount          float64    `json:"amount,string"`
 	Timestamp       types.Time `json:"timestamp"`
 	TimestampMS     types.Time `json:"timestampms"`
-	Type            string     `json:"type"`
+	Side            order.Side `json:"side"`


Are you sure?

gloriousCode · 2025-08-19T03:52:01Z

exchanges/gemini/gemini_types.go

@@ -140,7 +141,7 @@ type Order struct {
 	Exchange          string     `json:"exchange"`
 	Price             float64    `json:"price,string"`
 	AvgExecutionPrice float64    `json:"avg_execution_price,string"`
-	Side              string     `json:"side"`
+	Side              order.Side `json:"side"`
 	Type              string     `json:"type"`


Not a type?

gloriousCode · 2025-08-19T03:57:26Z

exchanges/gemini/gemini_types.go

@@ -344,7 +345,7 @@ type wsTrade struct {
 	Timestamp types.Time `json:"timestamp"`
 	Price     float64    `json:"price,string"`
 	Quantity  float64    `json:"quantity,string"`
-	Side      string     `json:"side"`
+	Side      order.Side `json:"side"`


Not sure I see this type even present in Gemini api docs

gloriousCode · 2025-08-19T03:58:37Z

exchanges/gemini/gemini_websocket.go

-
-func stringToOrderType(oType string) (order.Type, error) {
-	switch oType {
-	case "exchange limit", "auction-only limit", "indication-of-interest limit":


Auction only limit support dropped by this PR

It is covered here: https://github.com/thrasher-/gocryptotrader/blob/cleanup_v3/exchanges/order/orders.go#L1102

Has the API changed?AUCTION-ONLY LIMIT YOU MAY BE HOLDING DOWN SHIFT TO YELL INTO THE IDE

Ah I see now, it needs a -

gloriousCode · 2025-08-19T04:09:38Z

exchanges/okx/okx_wrapper.go

-	}
-	orderType, tif, err := orderTypeFromString(orderDetail.OrderType)
+
+	_, tif, err := orderTypeFromString(orderDetail.OrderType.String())


Could have changed param to order.Type and calledString() from the func once, instead of 4 times. FAR OUT 😄
Function could be improved further: GetTIFFromOrderType seems better

gloriousCode · 2025-08-19T04:41:49Z

exchanges/huobi/huobi_websocket.go

+func setOrderTypeAndSide(rawOrderType string, origSide order.Side, d *order.Detail) error {
+	var err error
+	if origSide == order.UnknownSide {
+		d.Side, err = stringToOrderSide(rawOrderType)


Looks like a typo, therefore you should add a test for this function

thrasher- requested review from gbjk, shazbert, samuael and Copilot June 26, 2025 01:37

thrasher- self-assigned this Jun 26, 2025

thrasher- added the nomerge requires dependency This pull request is dependent on another, so it can't be merged until the dependent one is merged label Jun 26, 2025

Copilot AI reviewed Jun 26, 2025

View reviewed changes

shazbert requested changes Jun 29, 2025

View reviewed changes

thrasher- added the reconstructing Based on PR feedback, this is currently being reworked and is not to be merged label Jul 1, 2025

thrasher- force-pushed the cleanup_v3 branch from 753ea35 to 537c323 Compare July 2, 2025 06:39

thrasher- removed the nomerge requires dependency This pull request is dependent on another, so it can't be merged until the dependent one is merged label Jul 2, 2025

gbjk requested changes Jul 3, 2025

View reviewed changes

thrasher- added the rebase/merge of master required label Jul 17, 2025

thrasher- force-pushed the cleanup_v3 branch from 537c323 to 938ec6a Compare July 17, 2025 03:29

thrasher- removed the rebase/merge of master required label Jul 17, 2025

samuael reviewed Jul 21, 2025

View reviewed changes

thrasher- force-pushed the cleanup_v3 branch from 938ec6a to 8a21572 Compare August 7, 2025 05:11

gbjk approved these changes Aug 7, 2025

View reviewed changes

thrasher- added review me This pull request is ready for review and removed reconstructing Based on PR feedback, this is currently being reworked and is not to be merged labels Aug 10, 2025

thrasher- linked an issue Aug 10, 2025 that may be closed by this pull request

Bithumb: Add zero filter for orderbook amount #1947

Closed

gloriousCode requested changes Aug 15, 2025

View reviewed changes

exchanges/binance/binance_types.go Show resolved Hide resolved

thrasher- mentioned this pull request Aug 18, 2025

build/linters: Bump Go to v1.25 and golangci-lint to v2.4.0 #2005

Merged

11 tasks

gloriousCode requested changes Aug 19, 2025

View reviewed changes

thrasher- requested a review from cranktakular August 19, 2025 06:07

shazbert added the rebase/merge of master required label Aug 24, 2025

thrasher- added reconstructing Based on PR feedback, this is currently being reworked and is not to be merged and removed review me This pull request is ready for review labels Aug 25, 2025

thrasher- force-pushed the cleanup_v3 branch from 8a21572 to 60ff2a0 Compare August 25, 2025 05:02

thrasher- removed the rebase/merge of master required label Aug 25, 2025

thrasher- added 2 commits August 26, 2025 13:38

codebase: Consolidate orders functionality and other minor fixes

8b1ade5

refactor: Fix nitterinos

0fb38ca

thrasher- force-pushed the cleanup_v3 branch from 60ff2a0 to 0fb38ca Compare August 26, 2025 03:43

codebase: Consolidate orders functionality and other minor fixes #1957

Are you sure you want to change the base?

codebase: Consolidate orders functionality and other minor fixes #1957

Uh oh!

Conversation

thrasher- commented Jun 26, 2025

PR Description

Type of change

How has this been tested

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

shazbert left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gbjk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

samuael left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gloriousCode left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jul 2, 2025 •

edited

Loading

gloriousCode Aug 20, 2025 •

edited

Loading

gloriousCode Aug 19, 2025 •

edited

Loading