solana-labs / explorer

Explorer for Solana clusters
https://explorer.solana.com
MIT License
217 stars 280 forks source link

Use UTL API directly for address token info #298

Closed mcintyre94 closed 11 months ago

mcintyre94 commented 11 months ago

The problem here was not with our request coalescer, the getMultipleAccounts requests are coming from the UTL SDK. Specifically on the transaction page where we use <Address pubkey={pubkey} ... fetchTokenLabelInfo. We do this on addresses that may be, but probably are not, token mints, ie every address in a transaction.

Previously this check came from the baked in legacy token list. When the token list was removed it was moved to the UTL SDK. The UTL SDK first calls the UTL API to see if the address has known token info. If it does not then it makes an on-chain call using getMultipleAccounts to fetch the token info from the Metaplex metadata program if it exists.

I don't think this fallback is appropriate for our use case of fetchTokenLabelInfo on addresses. Most addresses displayed are not tokens, and we display many addresses simultaneously. This is leading to many unnecessary getMultipleAccounts calls, and rate limiting.

Instead for this use case, we should only use the UTL API. This may mean that brand new tokens can be displayed in eg the token details page, but won't be correctly labelled in addresses. But this is a better tradeoff than making an on-chain request for every address that we render that may be a token.

Note that we already use the same public API for search (https://github.com/solana-labs/explorer/blob/master/app/utils/token-search.ts), where the SDK also can't provide an on-chain fallback.

vercel[bot] commented 11 months ago

@mcintyre94 is attempting to deploy a commit to the Solana Labs Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] commented 11 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
explorer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 25, 2023 3:12pm
steveluscher commented 11 months ago

Something like this:

const oldFetch = globalThis.fetch.bind(globalThis);
const accountsToFetchAtEndOfRunloop = new Set();
const resultPromises = new Map();
globalThis.fetch = (...args) => {
    const [resource, options] = args;
    const body = JSON.parse(options.body);
    if (
        body.jsonrpc === '2.0' &&
        (body.method === 'getMultipleAccounts' || body.method === 'getAccountInfo')
    ) {
        let resolveResult;
        const resultPromise = new Promise(resolve => {
            resolveResult = resolve;
        });
        const address = body.params[0]; // Different for gMA of course.
        resultPromises.set(
            address,
            [
                ...(resultPromises.get(address) ?? []),
                resultPromise,
            ],
        );
        if (accountsToFetchAtEndOfRunloop.size === 0) {
            queueMicrotask(() => {
                const accounts = [...accountsToFetchAtEndOfRunloop];
                accountsToFetchAtEndOfRunloop.clear();
                const results = await oldFetch(resource, {
                    ...options,
                    body: JSON.stringify({
                      jsonrpc: '2.0',
                      method: 'getMultipleAccounts',
                      params: [accounts],
                    }),
                });
                const promises = {...resultPromises};
                resultPromises.clear();
                accounts.forEach((address, ii) => {
                    promises[address].forEach(resolve => {
                        resolve(results[ii]);
                    });
                });
            })
        }
        accountsToFetchAtEndOfRunloop.add(address);
        return await resultPromise;
    } else {
        return await oldFetch(...args);
    }
};

Obviously way more to the code than that (multiple caches based on the params passed to the call, data validation, error handling, maximum batch size before splitting into a separate gMA call, etc.) but something like that.

mcintyre94 commented 11 months ago

Cool that looks like a way better approach! I'm going to land this as-is for now, but leave the multiple accounts issue open. We should definitely replace our multiple accounts fetcher with that custom fetch solution.