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

CCXT requires this.marketsLoaded = true set each new constructor, scaling problem #22530

Closed
1 task done
antifragileer opened this issue May 17, 2024 · 1 comment
Closed
1 task done
Assignees

Comments

@antifragileer
Copy link

antifragileer commented May 17, 2024

Preliminary Checks

  • I have already searched for existing issues and confirmed that this issue is not a duplicate

Is your feature request related to a problem? Please describe

The feature request is related to a scaling problem across N accounts for the public endpoints used in loadMarket, which causes rate limit problems on exchanges that rate limit by IP address rather than account for public endpoints. It is impossible to use cached versions of loadMarket between CCXT instances. This is problematic for commercial implementations using CCXT that might iterate over N trades, across N accounts, across N nodes, across N pods/containers. Using a proxy is not a viable solution as the cost to support a proxy with a rate limit of 10 requests per second per IP address create scaling challenges in the number of assets that can be watched. Proxy is too expensive at scale.

Describe the solution you'd like

The ideal solution would be for the cache of loadMarket to be managed externally from CCXT and provide an internal CCXT that allows the client to set the market data using a method that accepts Dictionary as an input parameter. That value, already typed and populated from a previous request, can be stored into CCXT and prevent the entire loadMarket routine from executing.

How the client decides to manage the cache of the Dictionary data is up to the client and not a concern of CCXT. Instead, CCXT simply needs to provide a method that allows the data to be set by the client and prevent unnecessary calls to exchange endpoints that would be required to otherwise populate this data.

For example:

export default class Exchange {
    // ... existing code ...

    async loadMarketsFromCache(cachedMarkets: Dictionary<Market>): Promise<Dictionary<Market>> {
        this.markets = cachedMarkets;
        this.markets_by_id = this.indexBy(this.markets, 'id');
        this.symbols = Object.keys(this.markets);
        this.ids = Object.keys(this.markets_by_id);
        this.marketsLoaded = true;
        return this.markets;
    }

    // ... existing code ...
}

The proposed change would provide an easy method so the client can populate the market data from a cached source and retain the internal optional requirements as though CCXT was the one to have executed the call to the exchange.

I plan on contributing the feature enhancement, unless a moderator comments on this contribution, I will move forward as I have outlined.

Describe alternatives you've considered

I considered using a proxy, but managing the rate limit issue for loadMarket post process is much more complex at scale. An easier and simpler approach would be to just allow the client to cache the object and then restore the object upon additional load requirements.

Other rate limit issues can be easily handled by using watch* methods to read values once and cache them. the loadMarket problem is distinct chicken and egg problem as the cache of loadMarket doesn't go into effect in CCXT until it makes a request to the exchange to populate the object. This does not work at scale across N CCXT instances.

@carlosmiei
Copy link
Collaborator

Hello @antifragileer, this is supported out of the box, the easiest way of doing it is to provide the markets upon instantiating the exchange, example:

const cachedMarkets = (...)
const exchange = new ccxt.coinbase({'markets': markets})

We're already doing something similar here: https://github.com/ccxt/ccxt/blob/master/ts/src/test/test.ts#L1320

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants