search

reach-sh / humble-sdk

A Javascript library for interacting with the HumbleSwap DEx
https://app.humble.sh
10 stars 1 forks source link

Cache assets once fetched #90

Closed guanzo closed 2 years ago

guanzo commented 2 years ago

Pool fetching performance could be improved if assets were cached once fetched.

https://github.com/reach-sh/humble-sdk/blob/822121a642f3c24c0138b5156b2bfa2b90c4415c/src/participants/PoolAnnouncer.ts#L87-L90

The other Algo dApp sdk's i've used just cache them forever. There are mutable asset parameters but they aren't usually used afaik.

MrJackdaw commented 2 years ago

Hello @guanzo; apologies for the belated response.

The SDK is primarily a "portal" to the blockchain smart contracts. It does not cache data because we would either have to make assumptions about where the SDK is being used, or dump hedonistic amounts of data in memory.

  1. Making assumptions means e.g. using browser- (or server-)specific state or cache management libraries. This would restrict all SDK users to those platforms (e.g. browser or server), which didn't seem right
    • A(n unconsidered) option would be to integrate a state management mechanism that users could subscribe to. However, this introduces a new way of getting data from the SDK, and two state managers where you could have only one or none.
  2. Dumping it all in memory means we potentially store things that some end-users don't care about. It would also be a blackbox, since you couldn't see or influence what it stores.

We updated our docs yesterday with suggested steps for fetching and storing data. It is how we implement things on the HumbleSwap UI. See the section titled Optimized Pool Fetching near the bottom of the page.

Please let me know if this helps!

guanzo commented 2 years ago

  1. This is overthinking it imo. It could just be a simple object where the key is the asset Id and the value is the asset data. Works anywhere and no 3rd party libs needed.

    • If you meant the user could opt into caching like with an init option: The "getting data" interface would remain the same, i.e. you would still call fetchLiquidityPool(), it's just that the assets could come from cache instead of network.
    • If you mean adding a new subscribe function, then yeah that's not good.

  2. Sure I just don't consider this a big deal, it's 35KB of memory for all assets. I wouldn't call that hedonistic. You don't need to see or influence it really, it's a cache of immutable assets. Whether it comes from network or cache is immaterial.

We updated our docs yesterday

I'll check out the docs thanks.

The impetus for this issue is I'm trying to optimize my bot which makes a ton of swap quotes. It wouldn't be so bad if the reach sdk used algod to get asset info, but it uses public indexers which is pretty slow. Thanks for the response.

EDIT: The new includeTokens option will suffice, thanks! Can close the issue

MrJackdaw commented 2 years ago

😬 Apolgies: I should have mentioned that update as well. Glad you found a resolution!