orca-so / orca-sdks

Open-sourced typescript SDKs for Orca
MIT License
43 stars 10 forks source link

Remove dependency on metaplex-js sdk #61

Closed wjthieme closed 8 months ago

wjthieme commented 8 months ago

https://app.asana.com/0/0/1205157773161082/f

The dependency metaplex-foundataion/js is deprecated. This change replaces the usage of metaplex-foundataion/js with metaplex-foundataion/umi and metaplex-foundation/mpl-token-metadata with functionality staying the same.

terranmoccasin commented 8 months ago

Sweet! Thanks for looking into this @wjthieme 🙏

I'm doing some tests locally just with a script: https://gist.github.com/terranmoccasin/f994095fccaf3611c61fd44c26a3f7b9

I ran into two issues.

One was when I set the option loadImage: true for the metaplex provider.

Failed to fetch JSON metadata from 

Source: Plugin > Token Metadata

Caused By: TypeError: Only absolute URLs are supported
'

^this was thrown on the following line: https://github.com/orca-so/orca-sdks/blob/9238e0be1f0b7ac1f00f4d1ca5c4a6de18bbab43/packages/token-sdk/src/fetcher.ts#L93

Unfortunately the error is also not getting logged, and instead we are only logging the timeout.

The second issue was when I was bulk fetching >300 tokens with loadImage: false. I used the mints from https://github.com/orca-so/orca-mintlists/blob/main/assets/mintlists/orca-extended.mintlist.json.

'[AccountNotFoundError] The account of type [Metadata] was not found at the provided address [8D6RyjQ1mtco5K811WFX3tJY3bs1JkyckPVQeekQ9gpi].

Source: SDK
'
  1. Could you see if you can replicate those issues?
  2. Add better logging so we don't unintentionally silence errors like we are doing now
  3. (optional) it'd be nice to add an integration test to check for these, but we can also add later too since we've got a lot on our plates right now
wjthieme commented 8 months ago

@terranmoccasin I managed to find a way to get rid of the metaplex library altogether (since it is quite large and we only use a tiny portion (parsing onchain-metadata accounts). Wrote the logic of that myself and added a test to check that it parses correctly.

Ran the script as well and getting valid results

terranmoccasin commented 8 months ago

Actually will just merge so we can get it deployed and move the other PRs forward!

wjthieme commented 8 months ago

The big benefit is that this reduces the orca-2 bundle size by about 50% - 70% which will help with initial page loads!