Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GateIO: Split Futures into USDT and CoinM #1786

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

gbjk
Copy link
Collaborator

@gbjk gbjk commented Jan 26, 2025

Rationale

GateIO has divergent URLs for handling CoinM and USDT futures, and Delivery Futures.

The changes originally in GateIO made sense prior to shazbert's asset based routing system, but it's clear now that in contexts where it's possible, we want to ensure asset splits that allow request/response routing.
Delivery Futures are actually USDT Delivery futures, and if GateIO get around to introducing CoinM Delivery Futures we'll have to split that too.

This is a pre-cursor to better websocket connection/subscription routing for this exchange (and others)

Progresses #1764

Changes

  • GateIO: Split Futures into USDT and CoinM
  • Config Versions: Allow non-continuous versions for development
  • Various refactoring

Type of change

  • New feature (non-breaking change which adds functionality)

How has this been tested

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

This allows easier development of non-stacked version upgrades.
Though the PRs still need to be merged sequentially, or renumbered right
before merging
@gbjk gbjk self-assigned this Jan 26, 2025
@gbjk gbjk added the review me This pull request is ready for review label Jan 26, 2025
@gbjk gbjk requested review from shazbert, gloriousCode and thrasher- and removed request for shazbert and gloriousCode January 26, 2025 09:46
gbjk added 3 commits January 26, 2025 16:46
* Fix CancelBatchOrders using wrong endpoint for CoinMarginedFutures
* Fix TestGetActiveOrders expecting currency.ErrCurrencyPairsEmpty
@gbjk gbjk force-pushed the feature/gateio_split_futures branch from 002f3c5 to b45be62 Compare January 26, 2025 09:47
Copy link
Collaborator

@shazbert shazbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup and some minor nits! Thanks. 🚀

Comment on lines +2253 to +2255
func (c *FuturesContract) openInterest() float64 {
return c.QuantoMultiplier.Float64() * float64(c.PositionSize) * c.IndexPrice.Float64()
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not you but this looks like notional value not open interest. Open interest can be derived from GetFutureStats this call

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I think that's outside the scope of this PR, in that it's an existing bug from #1417 which isn't made worse by this change.
It does throw shade on the abstraction work done to support this bug though 😂

@gloriousCode Could you glance and confirm that you agree this is incorrect?
I'll open the issue and address it in a separate PR, if so.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran the tests, looked at the numbers, carried the one, compared with the website and confirmed it is wrong

@gbjk gbjk requested a review from shazbert February 13, 2025 11:28
Copy link
Collaborator

@shazbert shazbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes! 👍

Copy link
Collaborator

@shazbert shazbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK. Thanks!

@shazbert shazbert added the szrc shazbert review complete label Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review me This pull request is ready for review szrc shazbert review complete
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

3 participants