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

cache NFT metadata responses #29532

Closed
josheleonard opened this issue Apr 5, 2023 · 2 comments · Fixed by brave/brave-core#18572
Closed

cache NFT metadata responses #29532

josheleonard opened this issue Apr 5, 2023 · 2 comments · Fixed by brave/brave-core#18572

Comments

@josheleonard
Copy link

Description

Store valid NFT metadata responses within the api cache to speedup subsequent requests for NFT metadata

Steps to Reproduce

  1. Create a Brave Wallet
  2. Add an NFT to portfolio
  3. View the NFT page
  4. Navigate to a different page
  5. return to the NFT page

Actual result:

The NFT meta data was not cached, preventing speedup of the second page load

Expected result:

The NFT metadata is cached, allowing for speedup on the second page load

Reproduces how often:

Brave version (brave://version info)

Version/Channel Information:

  • Can you reproduce this issue with the current release?
  • Can you reproduce this issue with the beta channel?
  • Can you reproduce this issue with the nightly channel?

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields?
  • Does the issue resolve itself when disabling Brave Rewards?
  • Is the issue reproducible on the latest version of Chrome?

Miscellaneous Information:

@cypt4
Copy link

cypt4 commented Apr 6, 2023

We can change policy in the APIRequestHelper for some requests. At the moment it bypasses cache. I think for erc721 tokens we could at least cache getting of metadata url. Also we could enable cache when accessing nft gateway for the metadata content.

  request->load_flags = net::LOAD_BYPASS_CACHE | net::LOAD_DISABLE_CACHE |
                        net::LOAD_DO_NOT_SAVE_COOKIES;

@jamesmudgett jamesmudgett moved this to Backlog in Web3 Apr 14, 2023
@jamesmudgett jamesmudgett added the priority/P4 Planned work. We expect to get to it "soon". label Apr 14, 2023
@nvonpentz nvonpentz self-assigned this May 22, 2023
@nvonpentz nvonpentz moved this from Backlog to In Progress in Web3 May 22, 2023
@nvonpentz nvonpentz added QA/No and removed QA/Yes labels May 22, 2023
@nvonpentz
Copy link
Member

Removed QA/Yes because this isn't easily verifiable by QA team.

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

Successfully merging a pull request may close this issue.

6 participants