feat: clean code #789

sc0Vu · 2025-12-11T15:53:53Z

changes:

futures: remove simplejson usage
add demo environment, can switch by UseDemo
fix stuck process when call WsUserDataServeSignature
rename interface{} to any

0x0001 · 2025-12-15T06:44:52Z

check.sh


+# There is a bug if we define the same json tag in the struct.
+# Disable structtag temporarily.
+# see: https://github.com/golang/go/issues/40102


it's not a bug, is a feature

Duplicate json tags, making the intent of the code unclear.

think this

func TestSameJsonTag(t *testing.T) { data := []byte(`{ "i":"1" }`) type V struct { Ii int `json:"i"` S string `json:"i"` } var v V if err := json.Unmarshal(data, &v); err != nil { t.Fatal(err) } if v.Ii != 0 { t.Errorf("i expected %d, got %d", 0, v.Ii) } if v.S != "1" { t.Errorf("i expected %s, got %s", "1", v.S) } }

0x0001 · 2025-12-15T07:02:45Z

Cannot remove simplejson
Because the Go language's JSON parser causes bug regression.
Like this

func TestCaseInsensitiveJsonTag(t *testing.T) {
	// before data
	data := []byte(`{
	"n":"1"
	}`)

	type V struct {
		Name string `json:"n"`
	}

	var v V
	if err := json.Unmarshal(data, &v); err != nil {
		t.Fatal(err)
	}
	if v.Name != "1" {
		t.Errorf("Name expected %s, got %s", "1", v.Name)
	}

	// after data
	data = []byte(`{
	"n":"1",
	"N":"2"
	}`)
	var v2 V
	if err := json.Unmarshal(data, &v2); err != nil {
		t.Fatal(err)
	}
	if v2.Name != "1" {
		t.Errorf("Name V2 expected %s, got %s", "1", v2.Name)
	}
}

--- FAIL: TestCaseInsensitiveJsonTag (0.00s)
json_test.go:135: Name V2 expected 1, got 2

sc0Vu · 2025-12-15T14:35:47Z

@0x0001 The case sensitive tag would work if both n and N are defined. For example,

type V struct {
	Name string `json:"n"`
	Name2 string `json:"N"`
}

This issue may occur if a required key is missing in our code and the key is case-sensitive. However, I believe it should work correctly with the current JSON encoding.Or is there anything I might be missing?

0x0001 · 2025-12-15T15:23:17Z

@0x0001 The case sensitive tag would work if both n and N are defined. For example,
type V struct {
	Name string `json:"n"`
	Name2 string `json:"N"`
}
This issue may occur if a required key is missing in our code and the key is case-sensitive. However, I believe it should work correctly with the current JSON encoding.Or is there anything I might be missing?

Yes, but we've had cases where binance updated the response and added new fields.

0x0001 · 2025-12-15T15:31:20Z

maybe we can use json/v2 replace encoding/json

github.com/go-json-experiment/json

Since you are doing this work, I think we can replace all interface{} with any. What do you think?

sc0Vu · 2025-12-16T15:12:31Z

@0x0001 I’m not sure it’s a good idea to use the v2 JSON given the case-sensitivity issue. If the Binance data were correct, we wouldn’t be facing this problem.

I've replaced interface{} to any.

Copilot

Pull request overview

This pull request implements code cleanup and several enhancements to the Binance Go SDK. The changes focus on modernizing the codebase, adding demo environment support, fixing a process deadlock issue, and removing the simplejson dependency.

Changes:

Replaced interface{} with any throughout the codebase for better readability (Go 1.18+)
Added demo environment support with new UseDemo flag and corresponding endpoints across all API modules
Fixed a stuck process issue in WsUserDataServeSignature by adding a goroutine to properly handle connection closure
Removed simplejson dependency in futures and portfolio packages, using standard JSON unmarshaling with proper struct tags

Reviewed changes

Copilot reviewed 51 out of 51 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
v2/websocket_service.go	Added demo endpoints and fixed stuck process with proper connection cleanup goroutine
v2/client.go	Added UseDemo flag and demo API endpoint with selection logic
v2/request.go	Replaced interface{} with any in params and method signatures
v2/futures/websocket_service.go	Removed simplejson dependency, added JSON tags to structs, added demo endpoint support
v2/futures/client.go	Added demo API endpoint and selection logic
v2/futures/exchange_info_service.go	Replaced interface{} with any in struct field types
v2/delivery/websocket_service.go	Added demo endpoint support and replaced interface{} with any
v2/delivery/client.go	Added demo API endpoint and selection logic
v2/delivery/exchange_info_service.go	Replaced interface{} with any in struct field types
v2/options/websocket_service.go	Added demo endpoint constants (empty) and replaced interface{} with any
v2/options/exchange_info_service.go	Replaced interface{} with any in struct field types
v2/portfolio/websocket_service.go	Removed simplejson dependency and added JSON tag
v2/common/websocket/types.go	Replaced interface{} with any in request parameters
examples/config.go	Added UseDemo configuration with default value
examples/watcher_futures.go	New example demonstrating futures user data stream with demo environment
check.sh	Added go vet structtag=false flag to work around JSON tag issue

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-19T10:07:37Z

v2/options/websocket_service.go

 	if UseTestnet {
 		return baseWsTestnetUrl
 	}
+	if UseDemo {
+		return baseWsDemoUrl
+	}
 	return baseWsMainUrl


The endpoint selection logic has a potential issue: if both UseTestnet and UseDemo are set to true, UseTestnet takes precedence and UseDemo is ignored. However, since the demo URLs are empty strings, returning an empty string when UseDemo is true will cause connection failures. Consider adding validation to prevent UseDemo from being used when the endpoint is not configured.

Copilot · 2026-01-19T10:07:38Z

v2/websocket_service.go

 	if UseTestnet {
 		return BaseWsTestnetURL
 	}
+	if UseDemo {
+		return BaseWsDemoURL
+	}
 	return BaseWsMainURL


The endpoint selection logic has a potential issue: if both UseTestnet and UseDemo are set to true, UseTestnet takes precedence and UseDemo is ignored. Consider adding validation to ensure only one environment flag is active at a time, or document this priority explicitly.

Copilot · 2026-01-19T10:07:38Z

v2/websocket_service.go

 	if UseTestnet {
 		return BaseCombinedTestnetURL
 	}
+	if UseDemo {
+		return BaseCombinedDemoURL
+	}
 	return BaseCombinedMainURL


The endpoint selection logic has a potential issue: if both UseTestnet and UseDemo are set to true, UseTestnet takes precedence and UseDemo is ignored. Consider adding validation to ensure only one environment flag is active at a time, or document this priority explicitly.

Copilot · 2026-01-19T10:07:38Z

v2/websocket_service.go

 	if UseTestnet {
 		return BaseWsApiTestnetURL
 	}
+	if UseDemo {
+		return BaseWsApiDemoURL
+	}
 	return BaseWsApiMainURL


The endpoint selection logic has a potential issue: if both UseTestnet and UseDemo are set to true, UseTestnet takes precedence and UseDemo is ignored. Consider adding validation to ensure only one environment flag is active at a time, or document this priority explicitly.

Copilot · 2026-01-19T10:07:39Z

examples/config.go

 	APIKey:     getEnvOrDefault("BINANCE_API_KEY", ""),
 	SecretKey:  getEnvOrDefault("BINANCE_SECRET_KEY", ""),
 	UseTestnet: getEnvOrDefault("BINANCE_USE_TESTNET", "true") == "true",
+	UseDemo:    getEnvOrDefault("BINANCE_USE_DEMO", "true") == "true",


The default value for UseDemo is set to "true", which means the demo environment will be used by default when BINANCE_USE_DEMO is not set. This is likely unintended as most users would expect the production environment by default. Consider changing the default to "false".

Suggested change

UseDemo: getEnvOrDefault("BINANCE_USE_DEMO", "true") == "true",

UseDemo: getEnvOrDefault("BINANCE_USE_DEMO", "false") == "true",

Copilot · 2026-01-19T10:07:40Z

v2/futures/websocket_service.go

 	if UseTestnet {
 		return BaseCombinedTestnetURL
 	}
+	if UseDemo {
+		return BaseCombinedDemoURL
+	}
 	return BaseCombinedMainURL


The endpoint selection logic has a potential issue: if both UseTestnet and UseDemo are set to true, UseTestnet takes precedence and UseDemo is ignored. Consider adding validation to ensure only one environment flag is active at a time, or document this priority explicitly.

Copilot · 2026-01-19T10:07:40Z

v2/futures/client.go

 	if UseTestnet {
 		return BaseApiTestnetUrl
 	}
+	if UseDemo {
+		return BaseApiDemoURL
+	}
 	return BaseApiMainUrl


The endpoint selection logic has a potential issue: if both UseTestnet and UseDemo are set to true, UseTestnet takes precedence and UseDemo is ignored. Consider adding validation to ensure only one environment flag is active at a time, or document this priority explicitly.

Copilot · 2026-01-19T10:07:41Z

v2/delivery/client.go

 	if UseTestnet {
 		return BaseApiTestnetUrl
 	}
+	if UseDemo {
+		return BaseApiDemoURL
+	}
 	return BaseApiMainUrl


The endpoint selection logic has a potential issue: if both UseTestnet and UseDemo are set to true, UseTestnet takes precedence and UseDemo is ignored. Consider adding validation to ensure only one environment flag is active at a time, or document this priority explicitly.

Copilot · 2026-01-19T10:07:41Z

v2/delivery/websocket_service.go

 	if UseTestnet {
 		return BaseWsTestnetUrl
 	}
+	if UseDemo {
+		return BaseWsDemoURL
+	}
 	return BaseWsMainUrl


The endpoint selection logic has a potential issue: if both UseTestnet and UseDemo are set to true, UseTestnet takes precedence and UseDemo is ignored. Consider adding validation to ensure only one environment flag is active at a time, or document this priority explicitly.

Copilot · 2026-01-19T10:07:41Z

v2/client.go

 	if UseTestnet {
 		return BaseAPITestnetURL
 	}
+	if UseDemo {
+		return BaseAPIDemoURL
+	}
 	return BaseAPIMainURL


The endpoint selection logic has a potential issue: if both UseTestnet and UseDemo are set to true, UseTestnet takes precedence and UseDemo is ignored. Consider adding validation to ensure only one environment flag is active at a time, or document this priority explicitly.

futures: remove simplejson usage

3dc048c

sc0Vu requested a review from carlosmiei December 11, 2025 15:54

sc0Vu added 8 commits December 13, 2025 03:18

Add WatchFuturesUserDataStream example

20bf306

Update config

ca710f5

Update

710492e

Add demo for client

a01efdd

Fix idle connection in WsUserDataServeSignature

39fc1fa

Add UseDemo for delivery

3a86069

Add UseDemo for options

406d9e5

Disable structtag in vet

32690f2

sc0Vu force-pushed the clean-code-algo-order branch from 1de513b to 32690f2 Compare December 12, 2025 22:00

0x0001 reviewed Dec 15, 2025

View reviewed changes

Update portofolio ws

447e045

sc0Vu added 3 commits December 15, 2025 22:37

Update

7a3c43e

Update

4314ba1

Merge branch 'master' into clean-code-algo-order

48e11d3

sc0Vu added 2 commits December 16, 2025 23:01

Replace interface{} with any

de2923f

Update

9fb448d

sc0Vu marked this pull request as ready for review December 17, 2025 07:31

carlosmiei requested a review from Copilot January 19, 2026 10:02

Copilot started reviewing on behalf of carlosmiei January 19, 2026 10:03 View session

Copilot AI reviewed Jan 19, 2026

View reviewed changes

carlosmiei merged commit 36a1f70 into ccxt:master Jan 19, 2026
8 checks passed

	UseDemo: getEnvOrDefault("BINANCE_USE_DEMO", "true") == "true",
	UseDemo: getEnvOrDefault("BINANCE_USE_DEMO", "false") == "true",

feat: clean code #789

feat: clean code #789

Uh oh!

Conversation

sc0Vu commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0x0001 Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

0x0001 commented Dec 15, 2025

Uh oh!

sc0Vu commented Dec 15, 2025

Uh oh!

0x0001 commented Dec 15, 2025

Uh oh!

0x0001 commented Dec 15, 2025

Uh oh!

sc0Vu commented Dec 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sc0Vu commented Dec 11, 2025 •

edited

Loading