From fafee93e1b9376029afc932bf519a9c8079539d1 Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Thu, 20 Mar 2025 08:31:11 +0700 Subject: [PATCH] sonic, exchanges/okx: Make sonic opt-in by default and fix various OKX issues (#1816) * Sonic: Add sonic_on build tag Thinking about other ways to do this, but they amount to the same thing. It's messy, but I don't have another idea. * Okx: Remove redundant useAsIs on slices Slices are automatically used-as-is, so passing true for these types was unnecessary. Removing the field simplifies rationalising inverting the flag * Okx: Unify timestamp response types * Okx: Change ResetRFQMMPStatus to return a types.Time * Okx: Move withdrawData type to types * Okx: Fix AccountConfiguration slice return * Okx: Improve SendHTTPRequest documentation * Okx: Extend tests for UseAsIs non-defaults * Okx: Fix GetPublicUnderlying with sonic 1.12.9 Using **result for slices with useAsItIs causes sonic to fail. This might be addressed upstream, but it's also not clear what the unmarshal behaviour for an untyped reference to a typed reference should be in the RFC, so we could get a golang.org/encoding/json regression on this too. There's no harm in fixing this, for consistency, to match our handling for non-slice []any wrapping to just use the pointer as is. Note: As of today this requires sonic:main for this to work, because of the other bug: ``` M go.mod - github.com/bytedance/sonic v1.12.9 + github.com/bytedance/sonic v1.12.10-0.20250224121557-e30ac4f2e4fe ``` * Okx: Remove redundant slice check This code didn't work, and if I make it look at rv.Elem().Kind() it errors. Haven't dug too deeply but right now I think we just remove it. * Okx: Simplify SendHTTPRequest by removing UseAsIs Looks to be entirely derivable * Okx: Remove http check that resps must contain data GetAnnouncementTyeps failing in US because of empty response. But also any situation where there really is no data. e.g. GetCandlesticks might return no candlesticks for a period and instrument, because there aren't any. That shouldn't be an error. More saliently if you request orders, or something similar. So, since that check wasn't working before, and it's causing issues now, I'm going to remove it. * Okx: Fix TestGetAnnouncementTypes failing in US announcement-types returns empty in the US, where our github actions run. That's kinda okay. Just don't test we get any back * Sonic: Default to sonic off We've seen too many fatal panics and races with sonic, both in GCT runs and being reported in sonic, to default to it being turned on right now. Whilst we have faith sonic will get through these with time, for now the sensible thing to do for our users is make sonic opt-in. This also removes any of the conditions around 386, etc. If someone wants to run with sonic, they can. Most notably if they're trying out an experimental sonic branch that supports 386, etc. --- .github/workflows/tests.yml | 22 +-- CONTRIBUTORS | 3 +- Makefile | 6 +- README.md | 32 +++-- cmd/documentation/documentation.go | 5 + .../root_templates/root_readme.tmpl | 17 ++- encoding/json/common.go | 4 +- encoding/json/json.go | 2 +- encoding/json/sonic.go | 2 +- exchanges/okx/okx.go | 132 +++++++----------- exchanges/okx/okx_test.go | 28 ++-- exchanges/okx/okx_types.go | 13 +- exchanges/okx/okx_wrapper.go | 4 +- 13 files changed, 132 insertions(+), 138 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7813117f..ab354c2e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -4,7 +4,7 @@ env: GO_VERSION: 1.24.x jobs: backend: - name: GoCryptoTrader backend (${{ matrix.os }}, ${{ matrix.goarch }}, psql=${{ matrix.psql }}, skip_wrapper_tests=${{ matrix.skip_wrapper_tests}}, sonic=${{ matrix.sonic && matrix.goarch != '386' }}) + name: GoCryptoTrader backend (${{ matrix.os }}, ${{ matrix.goarch }}, psql=${{ matrix.psql }}, skip_wrapper_tests=${{ matrix.skip_wrapper_tests}}, sonic=${{ matrix.sonic }}) strategy: fail-fast: false matrix: @@ -13,32 +13,32 @@ jobs: goarch: amd64 psql: true skip_wrapper_tests: false - sonic: true + sonic: false - os: ubuntu-latest goarch: 386 psql: true skip_wrapper_tests: true - sonic: true # Sonic is not supported on 386 systems, this ensures our build tag condition defaults to and tests the standard encoding/json implementation + sonic: false - os: ubuntu-24.04-arm goarch: arm64 psql: false # Not supported skip_wrapper_tests: true - sonic: false # Disabled temporarily until OKX's unmarshalling is changed for sonic arm64 compatibility + sonic: false - os: macos-latest goarch: arm64 psql: true skip_wrapper_tests: true - sonic: false # Same reason as the arm64 ubuntu-latest runner + sonic: false - os: windows-latest goarch: amd64 psql: true skip_wrapper_tests: true - sonic: true + sonic: false - os: ubuntu-latest goarch: amd64 psql: true skip_wrapper_tests: false - sonic: false # Disabled intentionally to test GCT's standard encoding/json implementation + sonic: true # Only test sonic on linux/amd64 since too many races and fatals across other archs and OS runs-on: ${{ matrix.os }} @@ -96,8 +96,8 @@ jobs: - name: Set GOFLAGS run: | - if [ "${{ matrix.sonic }}" = "false" ]; then - echo "GOFLAGS=${GOFLAGS} -tags=sonic_off" >> $GITHUB_ENV + if [ "${{ matrix.sonic }}" = "true" ]; then + echo "GOFLAGS=${GOFLAGS} -tags=sonic_on" >> $GITHUB_ENV fi shell: bash @@ -119,7 +119,7 @@ jobs: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} backend-docker: - name: GoCryptoTrader backend docker (ubuntu-latest, amd64, psql=false, skip_wrapper_tests=true, sonic=true) + name: GoCryptoTrader backend docker (ubuntu-latest, amd64, psql=false, skip_wrapper_tests=true, sonic=false) runs-on: ubuntu-latest steps: @@ -180,4 +180,4 @@ jobs: cd web/ npm install npm run lint - npm run build \ No newline at end of file + npm run build diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 78590bff..70447817 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -22,6 +22,7 @@ geseq | https://github.com/geseq TaltaM | https://github.com/TaltaM cranktakular | https://github.com/cranktakular dackroyd | https://github.com/dackroyd +junnplus | https://github.com/junnplus khcchiu | https://github.com/khcchiu yangrq1018 | https://github.com/yangrq1018 woshidama323 | https://github.com/woshidama323 @@ -29,7 +30,6 @@ crackcomm | https://github.com/crackcomm mshogin | https://github.com/mshogin herenow | https://github.com/herenow tk42 | https://github.com/tk42 -soxipy | https://github.com/soxipy andreygrehov | https://github.com/andreygrehov azhang | https://github.com/azhang bretep | https://github.com/bretep @@ -54,5 +54,6 @@ m1kola | https://github.com/m1kola mattkanwisher | https://github.com/mattkanwisher merkeld | https://github.com/merkeld mKurrels | https://github.com/mKurrels +soxipy | https://github.com/soxipy starit | https://github.com/starit zeldrinn | https://github.com/zeldrinn diff --git a/Makefile b/Makefile index c7b51ecd..430f2541 100644 --- a/Makefile +++ b/Makefile @@ -79,6 +79,6 @@ check-jq: @printf "Checking if jq is installed... " @command -v jq >/dev/null 2>&1 && { printf "OK\n"; } || { printf "FAILED. Please install jq to proceed.\n"; exit 1; } -.PHONY: no_sonic -no_sonic: - go build $(LDFLAGS) -tags "sonic_off" \ No newline at end of file +.PHONY: sonic +sonic: + go build $(LDFLAGS) -tags "sonic_on" diff --git a/README.md b/README.md index 65b956d0..9180d1be 100644 --- a/README.md +++ b/README.md @@ -124,15 +124,20 @@ mkdir %AppData%\GoCryptoTrader copy config_example.json %APPDATA%\GoCryptoTrader\config.json ``` -By default, GoCryptoTrader uses the [Sonic JSON](https://github.com/bytedance/sonic) library for improved performance unless compiling for 32-bit or arm64 architectures. To disable Sonic and revert to Go's encoding/json, build with the sonic_off tag: - -```bash -go build -tags=sonic_off -``` - + Make any necessary changes to the `config.json` file. + Run the `gocryptotrader` binary file. +### Sonic JSON handling + +GoCryptoTrader can optionally use the [Sonic](https://github.com/bytedance/sonic) JSON library for improved performance, as a drop in replacement for golang.org/encoding/json. +Please see sonic [Requirements](https://github.com/bytedance/sonic/#requirement) for supported platforms. + +To enable sonic, build with the sonic_on tag: + +```bash +go build -tags=sonic_on +``` + ## Donations @@ -151,15 +156,15 @@ Binaries will be published once the codebase reaches a stable condition. |User|Contribution Amount| |--|--| -| [thrasher-](https://github.com/thrasher-) | 704 | -| [shazbert](https://github.com/shazbert) | 361 | -| [dependabot[bot]](https://github.com/apps/dependabot) | 351 | -| [gloriousCode](https://github.com/gloriousCode) | 236 | -| [gbjk](https://github.com/gbjk) | 115 | +| [thrasher-](https://github.com/thrasher-) | 708 | +| [shazbert](https://github.com/shazbert) | 362 | +| [dependabot[bot]](https://github.com/apps/dependabot) | 361 | +| [gloriousCode](https://github.com/gloriousCode) | 237 | +| [gbjk](https://github.com/gbjk) | 119 | | [dependabot-preview[bot]](https://github.com/apps/dependabot-preview) | 88 | | [xtda](https://github.com/xtda) | 47 | | [lrascao](https://github.com/lrascao) | 27 | -| [Beadko](https://github.com/Beadko) | 18 | +| [Beadko](https://github.com/Beadko) | 21 | | [Rots](https://github.com/Rots) | 15 | | [vazha](https://github.com/vazha) | 15 | | [ydm](https://github.com/ydm) | 15 | @@ -173,6 +178,7 @@ Binaries will be published once the codebase reaches a stable condition. | [TaltaM](https://github.com/TaltaM) | 6 | | [cranktakular](https://github.com/cranktakular) | 6 | | [dackroyd](https://github.com/dackroyd) | 5 | +| [junnplus](https://github.com/junnplus) | 5 | | [khcchiu](https://github.com/khcchiu) | 5 | | [yangrq1018](https://github.com/yangrq1018) | 4 | | [woshidama323](https://github.com/woshidama323) | 3 | @@ -180,7 +186,6 @@ Binaries will be published once the codebase reaches a stable condition. | [mshogin](https://github.com/mshogin) | 2 | | [herenow](https://github.com/herenow) | 2 | | [tk42](https://github.com/tk42) | 2 | -| [soxipy](https://github.com/soxipy) | 2 | | [andreygrehov](https://github.com/andreygrehov) | 2 | | [azhang](https://github.com/azhang) | 2 | | [bretep](https://github.com/bretep) | 2 | @@ -205,5 +210,6 @@ Binaries will be published once the codebase reaches a stable condition. | [mattkanwisher](https://github.com/mattkanwisher) | 1 | | [merkeld](https://github.com/merkeld) | 1 | | [mKurrels](https://github.com/mKurrels) | 1 | +| [soxipy](https://github.com/soxipy) | 2 | | [starit](https://github.com/starit) | 1 | | [zeldrinn](https://github.com/zeldrinn) | 1 | diff --git a/cmd/documentation/documentation.go b/cmd/documentation/documentation.go index 73a8a8cc..fa4543cb 100644 --- a/cmd/documentation/documentation.go +++ b/cmd/documentation/documentation.go @@ -296,6 +296,11 @@ func main() { URL: "https://github.com/mKurrels", Contributions: 1, }, + { + Login: "soxipy", + URL: "https://github.com/soxipy", + Contributions: 2, + }, { Login: "starit", URL: "https://github.com/starit", diff --git a/cmd/documentation/root_templates/root_readme.tmpl b/cmd/documentation/root_templates/root_readme.tmpl index 97948cf4..09dffe44 100644 --- a/cmd/documentation/root_templates/root_readme.tmpl +++ b/cmd/documentation/root_templates/root_readme.tmpl @@ -125,15 +125,20 @@ mkdir %AppData%\GoCryptoTrader copy config_example.json %APPDATA%\GoCryptoTrader\config.json ``` -By default, GoCryptoTrader uses the [Sonic JSON](https://github.com/bytedance/sonic) library for improved performance unless compiling for 32-bit or arm64 architectures. To disable Sonic and revert to Go's encoding/json, build with the sonic_off tag: - -```bash -go build -tags=sonic_off -``` - + Make any necessary changes to the `config.json` file. + Run the `gocryptotrader` binary file. +### Sonic JSON handling + +GoCryptoTrader can optionally use the [Sonic](https://github.com/bytedance/sonic) JSON library for improved performance, as a drop in replacement for golang.org/encoding/json. +Please see sonic [Requirements](https://github.com/bytedance/sonic/#requirement) for supported platforms. + +To enable sonic, build with the sonic_on tag: + +```bash +go build -tags=sonic_on +``` + {{template "donations" .}} ## Binaries diff --git a/encoding/json/common.go b/encoding/json/common.go index 1709de31..2abea733 100644 --- a/encoding/json/common.go +++ b/encoding/json/common.go @@ -1,6 +1,6 @@ // json is an abstraction middleware package to allow switching between json encoder/decoder implementations -// The default implementation is sonic. -// Build with `sonic_off`, '386' or 'arm64' tags to switch to golang.org/encoding/json. +// The default implementation is golang.org/encoding/json. +// Build with `sonic_on` tag to switch to using github.com/bytedance/sonic package json import "encoding/json" //nolint:depguard // Acceptable use in gct json wrapper diff --git a/encoding/json/json.go b/encoding/json/json.go index e1392642..fcdda22d 100644 --- a/encoding/json/json.go +++ b/encoding/json/json.go @@ -1,4 +1,4 @@ -//go:build sonic_off || 386 || arm64 +//go:build !sonic_on package json diff --git a/encoding/json/sonic.go b/encoding/json/sonic.go index b9503fb4..6db7b02d 100644 --- a/encoding/json/sonic.go +++ b/encoding/json/sonic.go @@ -1,4 +1,4 @@ -//go:build !sonic_off && !386 && !arm64 +//go:build sonic_on package json diff --git a/exchanges/okx/okx.go b/exchanges/okx/okx.go index 85d76268..e68c325d 100644 --- a/exchanges/okx/okx.go +++ b/exchanges/okx/okx.go @@ -8,7 +8,6 @@ import ( "net" "net/http" "net/url" - "reflect" "slices" "strconv" "strings" @@ -20,7 +19,6 @@ import ( "github.com/thrasher-corp/gocryptotrader/currency" "github.com/thrasher-corp/gocryptotrader/encoding/json" exchange "github.com/thrasher-corp/gocryptotrader/exchanges" - "github.com/thrasher-corp/gocryptotrader/exchanges/account" "github.com/thrasher-corp/gocryptotrader/exchanges/asset" "github.com/thrasher-corp/gocryptotrader/exchanges/kline" "github.com/thrasher-corp/gocryptotrader/exchanges/margin" @@ -864,10 +862,8 @@ func (ok *Okx) CancelMultipleRFQs(ctx context.Context, arg *CancelRFQRequestsPar // CancelAllRFQs cancels all active RFQs func (ok *Okx) CancelAllRFQs(ctx context.Context) (types.Time, error) { - var resp types.Time - return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, cancelAllRFQsEPL, http.MethodPost, "rfq/cancel-all-rfqs", nil, &struct { - Timestamp *types.Time `json:"ts"` - }{Timestamp: &resp}, request.AuthenticatedRequest) + resp := &tsResp{} + return resp.Timestamp, ok.SendHTTPRequest(ctx, exchange.RestSpot, cancelAllRFQsEPL, http.MethodPost, "rfq/cancel-all-rfqs", nil, resp, request.AuthenticatedRequest) } // ExecuteQuote executes a Quote. It is only used by the creator of the RFQ @@ -915,11 +911,9 @@ func (ok *Okx) SetQuoteProducts(ctx context.Context, args []SetQuoteProductParam } // ResetRFQMMPStatus reset the MMP status to be inactive -func (ok *Okx) ResetRFQMMPStatus(ctx context.Context) (time.Time, error) { - resp := &struct { - Timestamp types.Time `json:"ts"` - }{} - return resp.Timestamp.Time(), ok.SendHTTPRequest(ctx, exchange.RestSpot, resetRFQMMPEPL, http.MethodPost, "rfq/mmp-reset", nil, resp, request.AuthenticatedRequest) +func (ok *Okx) ResetRFQMMPStatus(ctx context.Context) (types.Time, error) { + resp := &tsResp{} + return resp.Timestamp, ok.SendHTTPRequest(ctx, exchange.RestSpot, resetRFQMMPEPL, http.MethodPost, "rfq/mmp-reset", nil, resp, request.AuthenticatedRequest) } // CreateQuote allows the user to Quote an RFQ that they are a counterparty to. The user MUST quote @@ -976,11 +970,8 @@ func (ok *Okx) CancelMultipleQuote(ctx context.Context, arg CancelQuotesRequestP // CancelAllRFQQuotes cancels all active quote orders func (ok *Okx) CancelAllRFQQuotes(ctx context.Context) (types.Time, error) { - var resp types.Time - return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, cancelAllQuotesEPL, http.MethodPost, "rfq/cancel-all-quotes", nil, - &struct { - Timestamp *types.Time `json:"ts"` - }{Timestamp: &resp}, request.AuthenticatedRequest) + resp := &tsResp{} + return resp.Timestamp, ok.SendHTTPRequest(ctx, exchange.RestSpot, cancelAllQuotesEPL, http.MethodPost, "rfq/cancel-all-quotes", nil, resp, request.AuthenticatedRequest) } // GetRFQs retrieves details of RFQs where the user is a counterparty, either as the creator or the recipient @@ -1313,14 +1304,11 @@ func (ok *Okx) CancelWithdrawal(ctx context.Context, withdrawalID string) (strin if withdrawalID == "" { return "", errMissingValidWithdrawalID } - type withdrawData struct { - WithdrawalID string `json:"wdId"` - } arg := &withdrawData{ WithdrawalID: withdrawalID, } - var response withdrawData - return response.WithdrawalID, ok.SendHTTPRequest(ctx, exchange.RestSpot, cancelWithdrawalEPL, http.MethodPost, "asset/cancel-withdrawal", arg, &response, request.AuthenticatedRequest) + var resp withdrawData + return resp.WithdrawalID, ok.SendHTTPRequest(ctx, exchange.RestSpot, cancelWithdrawalEPL, http.MethodPost, "asset/cancel-withdrawal", arg, &resp, request.AuthenticatedRequest) } // GetWithdrawalHistory retrieves the withdrawal records according to the currency, withdrawal status, and time range in reverse chronological order. @@ -1492,11 +1480,9 @@ func (ok *Okx) ApplyForMonthlyStatement(ctx context.Context, month string) (type if month == "" { return types.Time{}, errMonthNameRequired } - resp := &struct { - Timestamp types.Time `json:"ts"` - }{} + resp := &tsResp{} return resp.Timestamp, ok.SendHTTPRequest(ctx, exchange.RestSpot, applyForMonthlyStatementEPL, - http.MethodPost, "asset/monthly-statement", &map[string]string{"month": month}, &resp, request.AuthenticatedRequest) + http.MethodPost, "asset/monthly-statement", &map[string]string{"month": month}, resp, request.AuthenticatedRequest) } // GetMonthlyStatement retrieves monthly statements for the past year. @@ -1740,7 +1726,7 @@ func (ok *Okx) ApplyBillDetails(ctx context.Context, year, quarter string) ([]Bi return nil, errQuarterValueRequired } var resp []BillsDetailResp - return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, billHistoryArchiveEPL, http.MethodPost, "account/bills-history-archive", map[string]string{"year": year, "quarter": quarter}, &resp, request.AuthenticatedRequest, true) + return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, billHistoryArchiveEPL, http.MethodPost, "account/bills-history-archive", map[string]string{"year": year, "quarter": quarter}, &resp, request.AuthenticatedRequest) } // GetBillsHistoryArchive retrieves bill data archive @@ -1755,7 +1741,7 @@ func (ok *Okx) GetBillsHistoryArchive(ctx context.Context, year, quarter string) params.Set("year", year) params.Set("quarter", quarter) var resp []BillsArchiveInfo - return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getBillHistoryArchiveEPL, http.MethodGet, common.EncodeURLValues("account/bills-history-archive", params), nil, &resp, request.AuthenticatedRequest, true) + return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getBillHistoryArchiveEPL, http.MethodGet, common.EncodeURLValues("account/bills-history-archive", params), nil, &resp, request.AuthenticatedRequest) } // GetBillsDetail retrieves the bills of the account @@ -1805,9 +1791,9 @@ func (ok *Okx) GetBillsDetail(ctx context.Context, arg *BillsDetailQueryParamete } // GetAccountConfiguration retrieves current account configuration -func (ok *Okx) GetAccountConfiguration(ctx context.Context) ([]AccountConfigurationResponse, error) { - var resp []AccountConfigurationResponse - return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getAccountConfigurationEPL, http.MethodGet, "account/config", nil, &resp, request.AuthenticatedRequest, true) +func (ok *Okx) GetAccountConfiguration(ctx context.Context) (*AccountConfigurationResponse, error) { + var resp *AccountConfigurationResponse + return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getAccountConfigurationEPL, http.MethodGet, "account/config", nil, &resp, request.AuthenticatedRequest) } // SetPositionMode FUTURES and SWAP support both long/short mode and net mode. In net mode, users can only have positions in one direction; In long/short mode, users can hold positions in long and short directions. @@ -2652,10 +2638,8 @@ func (ok *Okx) SetRiskOffsetType(ctx context.Context, riskOffsetType string) (*R // ActivateOption activates option func (ok *Okx) ActivateOption(ctx context.Context) (types.Time, error) { - resp := &struct { - Timestamp types.Time `json:"ts"` - }{} - return resp.Timestamp, ok.SendHTTPRequest(ctx, exchange.RestSpot, activateOptionEPL, http.MethodPost, "account/activate-option", nil, &resp, request.AuthenticatedRequest) + resp := &tsResp{} + return resp.Timestamp, ok.SendHTTPRequest(ctx, exchange.RestSpot, activateOptionEPL, http.MethodPost, "account/activate-option", nil, resp, request.AuthenticatedRequest) } // SetAutoLoan only applicable to Multi-currency margin and Portfolio margin @@ -4573,7 +4557,7 @@ func (ok *Okx) GetIndexComponents(ctx context.Context, index string) (*IndexComp params := url.Values{} params.Set("index", index) var resp *IndexComponent - err := ok.SendHTTPRequest(ctx, exchange.RestSpot, getIndexComponentsEPL, http.MethodGet, common.EncodeURLValues("market/index-components", params), nil, &resp, request.UnauthenticatedRequest, true) + err := ok.SendHTTPRequest(ctx, exchange.RestSpot, getIndexComponentsEPL, http.MethodGet, common.EncodeURLValues("market/index-components", params), nil, &resp, request.UnauthenticatedRequest) if err != nil { return nil, err } @@ -5051,8 +5035,8 @@ func (ok *Okx) GetDiscountRateAndInterestFreeQuota(ctx context.Context, ccy curr // GetSystemTime retrieve API server time func (ok *Okx) GetSystemTime(ctx context.Context) (types.Time, error) { - resp := &ServerTime{} - return resp.Timestamp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getSystemTimeEPL, http.MethodGet, "public/time", nil, &resp, request.UnauthenticatedRequest) + resp := &tsResp{} + return resp.Timestamp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getSystemTimeEPL, http.MethodGet, "public/time", nil, resp, request.UnauthenticatedRequest) } // GetLiquidationOrders retrieves information on liquidation orders in the last day @@ -5182,7 +5166,7 @@ func (ok *Okx) GetPublicUnderlyings(ctx context.Context, instrumentType string) params := url.Values{} params.Set("instType", strings.ToUpper(instrumentType)) var resp []string - return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getUnderlyingEPL, http.MethodGet, common.EncodeURLValues("public/underlying", params), nil, &resp, request.UnauthenticatedRequest, false) + return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getUnderlyingEPL, http.MethodGet, common.EncodeURLValues("public/underlying", params), nil, &resp, request.UnauthenticatedRequest) } // GetInsuranceFundInformation returns insurance fund balance information @@ -5281,8 +5265,8 @@ func (ok *Okx) GetOptionsTickBands(ctx context.Context, instrumentType, instrume // GetSupportCoins retrieves the currencies supported by the trading data endpoints func (ok *Okx) GetSupportCoins(ctx context.Context) (*SupportedCoinsData, error) { - var response *SupportedCoinsData - return response, ok.SendHTTPRequest(ctx, exchange.RestSpot, getSupportCoinEPL, http.MethodGet, "rubik/stat/trading-data/support-coin", nil, &response, request.UnauthenticatedRequest, true) + var resp *SupportedCoinsData + return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getSupportCoinEPL, http.MethodGet, "rubik/stat/trading-data/support-coin", nil, &resp, request.UnauthenticatedRequest) } // GetTakerVolume retrieves the taker volume for both buyers and sellers @@ -5446,7 +5430,7 @@ func (ok *Okx) GetTakerFlow(ctx context.Context, ccy currency.Code, period kline params.Set("period", interval) } var resp *CurrencyTakerFlow - return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getTakerFlowEPL, http.MethodGet, common.EncodeURLValues("rubik/stat/option/taker-block-volume", params), nil, &resp, request.UnauthenticatedRequest, true) + return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, getTakerFlowEPL, http.MethodGet, common.EncodeURLValues("rubik/stat/option/taker-block-volume", params), nil, &resp, request.UnauthenticatedRequest) } // ********************************************************** Affiliate ********************************************************************** @@ -5507,7 +5491,7 @@ func (ok *Okx) PlaceLendingOrder(ctx context.Context, arg *LendingOrderParam) (* return nil, errLendingTermIsRequired } var resp *LendingOrderResponse - return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, placeLendingOrderEPL, http.MethodPost, "finance/fixed-loan/lending-order", arg, &resp, request.AuthenticatedRequest, false) + return resp, ok.SendHTTPRequest(ctx, exchange.RestSpot, placeLendingOrderEPL, http.MethodPost, "finance/fixed-loan/lending-order", arg, &resp, request.AuthenticatedRequest) } // AmendLendingOrder amends a lending order @@ -5851,35 +5835,21 @@ func (ok *Okx) GetFiatDepositPaymentMethods(ctx context.Context, ccy currency.Co common.EncodeURLValues("fiat/deposit-payment-methods", params), nil, &resp, request.AuthenticatedRequest) } -// SendHTTPRequest sends an authenticated http request to a desired -// path with a JSON payload (of present) -// URL arguments must be in the request path and not as url.URL values -func (ok *Okx) SendHTTPRequest(ctx context.Context, ep exchange.URL, f request.EndpointLimit, httpMethod, requestPath string, data, result any, authenticated request.AuthType, useAsItIs ...bool) (err error) { - rv := reflect.ValueOf(result) - if rv.Kind() != reflect.Pointer { - return errInvalidResponseParam - } +/* +SendHTTPRequest sends an http request, optionally with a JSON payload +URL arguments must be encoded in the request path +result must be a pointer +The response will be unmarshalled first into []any{result}, which matches most APIs, and fallback to directly into result +*/ +func (ok *Okx) SendHTTPRequest(ctx context.Context, ep exchange.URL, f request.EndpointLimit, httpMethod, requestPath string, data, result any, authenticated request.AuthType) (err error) { endpoint, err := ok.API.Endpoints.GetURL(ep) if err != nil { return err } - var respResult interface{} - switch { - case rv.Elem().Kind() == reflect.Slice && len(useAsItIs) > 0 && !useAsItIs[0]: - respResult = &[]interface{}{&result} - case rv.Elem().Kind() == reflect.Slice || - // When needed to use the result as it is. - len(useAsItIs) > 0 && useAsItIs[0]: - respResult = result - default: - respResult = &[]interface{}{result} - } - resp := struct { - Code types.Number `json:"code"` - Msg string `json:"msg"` - Data any `json:"data"` - }{ - Data: respResult, + var resp struct { + Code types.Number `json:"code"` + Msg string `json:"msg"` + Data json.RawMessage `json:"data"` } requestType := request.AuthType(request.UnauthenticatedRequest) newRequest := func() (*request.Item, error) { @@ -5899,16 +5869,13 @@ func (ok *Okx) SendHTTPRequest(ctx context.Context, ep exchange.URL, f request.E headers["x-simulated-trading"] = "1" } if authenticated == request.AuthenticatedRequest { - var creds *account.Credentials - creds, err = ok.GetCredentials(ctx) + creds, err := ok.GetCredentials(ctx) if err != nil { return nil, err } signPath := "/" + apiPath + requestPath var hmac []byte - hmac, err = crypto.GetHMAC(crypto.HashSHA256, - []byte(utcTime+httpMethod+signPath+string(payload)), - []byte(creds.Secret)) + hmac, err = crypto.GetHMAC(crypto.HashSHA256, []byte(utcTime+httpMethod+signPath+string(payload)), []byte(creds.Secret)) if err != nil { return nil, err } @@ -5928,28 +5895,29 @@ func (ok *Okx) SendHTTPRequest(ctx context.Context, ep exchange.URL, f request.E HTTPRecording: ok.HTTPRecording, }, nil } - err = ok.SendPayload(ctx, f, newRequest, requestType) - if err != nil { + if err = ok.SendPayload(ctx, f, newRequest, requestType); err != nil { if authenticated == request.AuthenticatedRequest { return fmt.Errorf("%w %w", request.ErrAuthRequestFailed, err) } return err } - if rv.Kind() == reflect.Slice { - value, okay := result.([]interface{}) - if !okay || result == nil || len(value) == 0 { - return fmt.Errorf("%w, received invalid response", common.ErrNoResponse) - } - } if err == nil && resp.Code.Int64() != 0 { if resp.Msg != "" { return fmt.Errorf("%w error code: %d message: %s", request.ErrAuthRequestFailed, resp.Code.Int64(), resp.Msg) } - err, okay := ErrorCodes[resp.Code.String()] - if okay { + if err, ok := ErrorCodes[resp.Code.String()]; ok { return err } return fmt.Errorf("%w error code: %d", request.ErrAuthRequestFailed, resp.Code.Int64()) } + + // First see if resp.Data can unmarshal into a slice of result, which is true for most APIs + if sliceErr := json.Unmarshal(resp.Data, &[]any{result}); sliceErr != nil { + // Otherwise, resp.Data should unmarshal directly into result; e.g. index-components, support-coin, and taker-block-volume + if directErr := json.Unmarshal(resp.Data, result); directErr != nil { + return fmt.Errorf("cannot unmarshal as a slice of result (error: %w) or as a reference to result (error: %w)", sliceErr, directErr) + } + } + return nil } diff --git a/exchanges/okx/okx_test.go b/exchanges/okx/okx_test.go index aa5fca0c..f319476a 100644 --- a/exchanges/okx/okx_test.go +++ b/exchanges/okx/okx_test.go @@ -227,7 +227,7 @@ func TestGetCandlesticks(t *testing.T) { _, err := ok.GetCandlesticks(contextGenerate(), "", kline.OneHour, time.Now().Add(-time.Minute*2), time.Now(), 2) require.ErrorIs(t, err, errMissingInstrumentID) - result, err := ok.GetCandlesticks(contextGenerate(), spotTP.String(), kline.OneHour, time.Now().Add(-time.Minute*2), time.Now(), 2) + result, err := ok.GetCandlesticks(contextGenerate(), spotTP.String(), kline.OneHour, time.Now().Add(-time.Hour), time.Now(), 2) require.NoError(t, err) assert.NotNil(t, result) } @@ -317,7 +317,9 @@ func TestGetIndexComponents(t *testing.T) { result, err := ok.GetIndexComponents(contextGenerate(), "ETH-USDT") require.NoError(t, err) - assert.NotNil(t, result) + require.NotNil(t, result) + assert.NotEmpty(t, result.Index, "Index should not be empty") + assert.NotEmpty(t, result.Components, "Components should not be empty") } func TestGetBlockTickers(t *testing.T) { @@ -406,14 +408,14 @@ func TestGetInstrument(t *testing.T) { }) assert.ErrorIs(t, err, errInstrumentFamilyOrUnderlyingRequired) - result, err := ok.GetInstruments(contextGenerate(), &InstrumentsFetchParams{ + resp, err := ok.GetInstruments(contextGenerate(), &InstrumentsFetchParams{ InstrumentType: instTypeFutures, Underlying: "SOL-USD", }) - assert.NoError(t, err) - assert.NotNil(t, result) + require.NoError(t, err) + assert.Empty(t, resp, "Should get back no instruments for SOL-USD futures") - result, err = ok.GetInstruments(contextGenerate(), &InstrumentsFetchParams{ + result, err := ok.GetInstruments(contextGenerate(), &InstrumentsFetchParams{ InstrumentType: instTypeSpot, }) require.NoError(t, err) @@ -524,6 +526,7 @@ func TestGetSystemTime(t *testing.T) { result, err := ok.GetSystemTime(contextGenerate()) require.NoError(t, err) assert.NotNil(t, result) + assert.False(t, result.Time().IsZero(), "GetSystemTime should not return a zero time") } func TestGetLiquidationOrders(t *testing.T) { @@ -591,7 +594,8 @@ func TestGetPublicUnderlyings(t *testing.T) { result, err := ok.GetPublicUnderlyings(contextGenerate(), instTypeFutures) require.NoError(t, err) - assert.NotNil(t, result) + require.NotNil(t, result) + assert.NotEmpty(t, result) } func TestGetInsuranceFundInformation(t *testing.T) { @@ -651,7 +655,8 @@ func TestGetSupportCoins(t *testing.T) { t.Parallel() result, err := ok.GetSupportCoins(contextGenerate()) require.NoError(t, err) - assert.NotNil(t, result) + require.NotNil(t, result) + assert.NotEmpty(t, result.Spot, "SupportedCoins Spot should not be empty") } func TestGetTakerVolume(t *testing.T) { @@ -720,7 +725,8 @@ func TestGetTakerFlow(t *testing.T) { t.Parallel() result, err := ok.GetTakerFlow(contextGenerate(), currency.BTC, kline.OneDay) require.NoError(t, err) - assert.NotNil(t, result) + require.NotNil(t, result) + assert.False(t, result.Timestamp.Time().IsZero(), "Timestamp should not be zero") } func TestPlaceOrder(t *testing.T) { @@ -6458,9 +6464,9 @@ func TestGetAnnouncements(t *testing.T) { func TestGetAnnouncementTypes(t *testing.T) { t.Parallel() - results, err := ok.GetAnnouncementTypes(contextGenerate()) + _, err := ok.GetAnnouncementTypes(contextGenerate()) require.NoError(t, err) - assert.NotEmpty(t, results) + // No tests of contents of resp because currently in US based github actions announcement-types returns empty } func TestGetDepositOrderDetail(t *testing.T) { diff --git a/exchanges/okx/okx_types.go b/exchanges/okx/okx_types.go index 5232785d..1364acc8 100644 --- a/exchanges/okx/okx_types.go +++ b/exchanges/okx/okx_types.go @@ -496,11 +496,6 @@ type DiscountRateInfoItem struct { DiscountCurrencyEquity types.Number `json:"disCcyEq"` } -// ServerTime returning the server time instance -type ServerTime struct { - Timestamp types.Time `json:"ts"` -} - // LiquidationOrderRequestParams holds information to request liquidation orders type LiquidationOrderRequestParams struct { InstrumentType string @@ -5295,3 +5290,11 @@ type MonthlyStatement struct { State string `json:"state"` Timestamp types.Time `json:"ts"` } + +type tsResp struct { + Timestamp types.Time `json:"ts"` +} + +type withdrawData struct { + WithdrawalID string `json:"wdId"` +} diff --git a/exchanges/okx/okx_wrapper.go b/exchanges/okx/okx_wrapper.go index 81053e25..e7b7b3ed 100644 --- a/exchanges/okx/okx_wrapper.go +++ b/exchanges/okx/okx_wrapper.go @@ -2346,7 +2346,7 @@ func (ok *Okx) GetCollateralMode(ctx context.Context, item asset.Item) (collater if err != nil { return 0, err } - switch cfg[0].AccountLevel { + switch cfg.AccountLevel { case "1": if item != asset.Spot { return 0, fmt.Errorf("%w %v", asset.ErrNotSupported, item) @@ -2359,7 +2359,7 @@ func (ok *Okx) GetCollateralMode(ctx context.Context, item asset.Item) (collater case "4": return collateral.PortfolioMode, nil default: - return collateral.UnknownMode, fmt.Errorf("%w %v", order.ErrCollateralInvalid, cfg[0].AccountLevel) + return collateral.UnknownMode, fmt.Errorf("%w %v", order.ErrCollateralInvalid, cfg.AccountLevel) } }