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

Enable NFT pinning flag #29017

Closed
cypt4 opened this issue Mar 13, 2023 · 3 comments · Fixed by brave/brave-core#17758
Closed

Enable NFT pinning flag #29017

cypt4 opened this issue Mar 13, 2023 · 3 comments · Fixed by brave/brave-core#17758

Comments

@cypt4
Copy link

cypt4 commented Mar 13, 2023

Figma: https://www.figma.com/file/4e7rsWzBUKv3dCdxCqB6CR/NFT-IPFS?t=afIihL0uQG3FO3gV-0
Related issues:

#19283
#26828
#28304
#28775
#28928
#28233
#28073
#28169
#28986

Related description(sec review):

Testing plan:
Enabling the feature

  1. Enable the NFT pinnig feature flag at chrome://flags
  2. Open brave wallet, add some pinnable NFTs.
    Pinnable NFTs are NFTs which have ipfs:// scheme for metadata and for image.
    For example BAYC tokens are pinnable https://opensea.io/assets/ethereum/0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d/4352
    Also metadata and image may have gateway-like url format. Such tokens are also pinnable
    https://opensea.io/assets/ethereum/0x9401518f4ebba857baa879d9f76e1cc8b31ed197/400
  3. Check onboarding flow by clicking "Learn more button" on the wallet page
    image
    Flow is described here : https://www.figma.com/file/4e7rsWzBUKv3dCdxCqB6CR/NFT-IPFS?node-id=1660%3A95153&t=CAsaMQ99tSq1M9NW-0
  4. After NFT pinning is enabled from the onboarding flow check settings page, this setting should be enabled:
    image

Pinning

  1. Check pinnable NFTs for pinning status according this design
    https://www.figma.com/file/4e7rsWzBUKv3dCdxCqB6CR/NFT-IPFS?node-id=1044%3A102005&t=CAsaMQ99tSq1M9NW-0
  2. If token is pinned, check pins manually by calling
    curl http://127.0.0.1:<api port could be viewed at chrome://ipfs-internal>/api/v0/pin/ls?type=recursive
    while running the Kubo daemon.
    Result should contain metadata CIDs and image CIDs of the pinned tokens.

Unpinning

  1. Remove a pinned NFT and wait for a while.
  2. Check pin was really removed by calling
    curl http://127.0.0.1:<api port could be viewed at chrome://ipfs-internal>/api/v0/pin/ls?type=recursive
    NOTE: Some of NFTs may share CIDs, so CID won't be unpinned until all related NFTs are removed.

Pins validation

  1. Pin several tokens and check they are actually pinned. Delete brave_ipfs folder in the browser profile folder.
  2. Change current date to 3 days forward and relaunch the browser. This should activate validation task.
  3. After several time pins should be restored, so
    curl http://127.0.0.1:<api port could be viewed at chrome://ipfs-internal>/api/v0/pin/ls?type=recursive should contain them.

Disabling the feature
12) Disabling the feature should lead to hiding pinning status or pinning-related UI on the wallet page.
13) Tokens are not unpinned unless manually cleaned up.

Cleanup
14) After feature is disabled in the settings, there should be an option to remove all pinned items. Cleanup should be verified using curl http://127.0.0.1:<api port could be viewed at chrome://ipfs-internal>/api/v0/pin/ls?type=recursive.

Local node behaviour
15) Check whether local node is automatically started after browser starts if the NFT pinning setting is enabled and not started otherwise.

Errors handling
16) Content type of NFT's image url should be "image/*". Request is made to https:///ipfs/CID and metadata field content-type or mime-type is read. You can test behavior by sniffing such requests as described here brave/brave-core#17392 (comment)
17) Pinning may fail due some reasons. It would be restarted after a time or on the next browser start. You can fail pinning by turning internet off while pinning is in progress and then test if it is continued after restart.

@cypt4 cypt4 added OS/Desktop feature/web3/ipfs feature/web3/wallet Integrating Ethereum+ wallet support labels Mar 13, 2023
@jamesmudgett jamesmudgett added the priority/P3 The next thing for us to work on. It'll ride the trains. label Mar 17, 2023
cypt4 added a commit to brave/brave-core that referenced this issue Mar 27, 2023
@yrliou
Copy link
Member

yrliou commented Mar 29, 2023

Security & privacy review done in https://github.com/brave/security/issues/1182

@kjozwiak
Copy link
Member

kjozwiak commented Apr 6, 2023

The above requires 1.51.80 or higher for 1.51.x verification 👍

@srirambv
Copy link
Contributor

Verification passed on

Brave 1.51.79 Chromium: 112.0.5615.49 (Official Build) beta (64-bit)
Revision bd2a7bcb881c11e8cfe3078709382934e3916914-refs/branch-heads/5615@{#936}
OS Linux
Verified steps from issue description
Enabling the feature - Passed
  • Verified brave://flags/#enable-nft-pinning is set to Default
  • Verified adding an NFT to visible asset list, automatically shows the NFT pinning banner
  • Verified clicking on Learn more on the banner loads interstitial page brave://wallet/crypto/local-ipfs-node
  • Verified clicking on Check which NFT's of mine can be pinned loads second interstitial page brave://wallet/crypto/inspect-nfts
  • Verified clicking on Keep my NFTs always online starts IPFS on localhost and start pinning the available NFT's
  • Verified when Keep my NFTs always online is selected, setting Automatically pin NFT's is enabled, default is disabled
Pinning - Passed
  • Verified able to pin NFTs
  • Verified when pinning is in process, shows message banner message about being pinned in local node
  • Verified when successfully pinned, show message about number of NFTs that are pinned
  • Additional verification notes for banner messages and pinning data recursively
Unpinning - Passed
  • Verified after removing an NFT and running curl http://127.0.0.1:<api port could be viewed at chrome://ipfs-internal>/api/v0/pin/ls?type=recursive, shows {"Keys":{}} message
    image
Pins Validation - Passed
  • Verified deleting brave_ifps folder and moving date ahead and restarting the browser restores brave_ipfs folder again
  • Verified resetting device date back to current date after restart starts to pin the NFT's again.
  • Running curl http://127.0.0.1:<api port could be viewed at chrome://ipfs-internal>/api/v0/pin/ls?type=recursive continues to show the pinned NFT details
Disable feature - Passed
  • Verified disabling Automatically pin NFTs setting and reloading the NFT tab shows the onboarding banner message
  • Verified manually deleting the NFT's from visible asset list also removes the onboarding banner message
Cleanup - Passed
  • Verified disabling Automatically pin NFTs setting, a new setting for Clear pinned NFTs is shown
  • Verified clicking that clears the NFTs pinned state and shows onboarding banner message
  • Encountered #29553 & #29554
Local node behaviour - Passed
  • Verified when user click on Get started with IPFS from interstitial page, IPFS local node is installed and started.
Error handling - Passed
  • Verified if NFT's content-type is not of the format image/*, NFT pinning fails. Verified as part of brave-browser#28775 (comment)
  • Verified due to network issues NFT pinning fails, it starts the pinning process again when browser is restarted

Verification passed on

Brave 1.51.79 Chromium: 112.0.5615.49 (Official Build) beta (64-bit)
Revision bd2a7bcb881c11e8cfe3078709382934e3916914-refs/branch-heads/5615@{#936}
OS Windows 11 Version 22H2 (Build 22621.1344)
Verified steps from issue description
Enabling the feature - Passed
  • Verified brave://flags/#enable-nft-pinning is set to Default
  • Verified adding an NFT to visible asset list, automatically shows the NFT pinning banner
  • Verified clicking on Learn more on the banner loads interstitial page brave://wallet/crypto/local-ipfs-node
  • Verified clicking on Check which NFT's of mine can be pinned loads second interstitial page brave://wallet/crypto/inspect-nfts
  • Verified clicking on Keep my NFTs always online starts IPFS on localhost and start pinning the available NFT's
  • Verified when Keep my NFTs always online is selected, setting Automatically pin NFT's is enabled, default is disabled
Pinning - Passed
  • Verified able to pin NFTs
  • Verified when pinning is in process, shows message banner message about being pinned in local node
  • Verified when successfully pinned, show message about number of NFTs that are pinned
  • Additional verification notes for banner messages and pinning data recursively
Unpinning - Passed
  • Verified after removing an NFT and running curl http://127.0.0.1:<api port could be viewed at chrome://ipfs-internal>/api/v0/pin/ls?type=recursive, shows {"Keys":{}} message
    WindowsTerminal_OQhWi4wCC9
Pins Validation - Passed
  • Verified deleting brave_ifps folder and moving date ahead and restarting the browser restores brave_ipfs folder again
  • Verified resetting device date back to current date after restart starts to pin the NFT's again.
  • Running curl http://127.0.0.1:<api port could be viewed at chrome://ipfs-internal>/api/v0/pin/ls?type=recursive continues to show the pinned NFT details
Disable feature - Passed
  • Verified disabling Automatically pin NFTs setting and reloading the NFT tab shows the onboarding banner message
  • Verified manually deleting the NFT's from visible asset list also removes the onboarding banner message
Cleanup - Passed
  • Verified disabling Automatically pin NFTs setting, a new setting for Clear pinned NFTs is shown
  • Verified clicking that clears the NFTs pinned state and shows onboarding banner message
  • Encountered #29553 & #29554
Local node behaviour - Passed
  • Verified when user click on Get started with IPFS from interstitial page, IPFS local node is installed and started.
Error handling - Passed
  • Verified if NFT's content-type is not of the format image/*, NFT pinning fails. Verified as part of brave-browser#28775 (comment)
  • Verified due to network issues NFT pinning fails, it starts the pinning process again when browser is restarted

Verification passed on

Brave 1.51.79 Chromium: 112.0.5615.49 (Official Build) beta (arm64)
Revision bd2a7bcb881c11e8cfe3078709382934e3916914-refs/branch-heads/5615@{#936}
OS macOS Version 13.0 (Build 22A380)
Verified steps from issue description
Enabling the feature - Passed
  • Verified brave://flags/#enable-nft-pinning is set to Default
  • Verified adding an NFT to visible asset list, automatically shows the NFT pinning banner
  • Verified clicking on Learn more on the banner loads interstitial page brave://wallet/crypto/local-ipfs-node
  • Verified clicking on Check which NFT's of mine can be pinned loads second interstitial page brave://wallet/crypto/inspect-nfts
  • Verified clicking on Keep my NFTs always online starts IPFS on localhost and start pinning the available NFT's
  • Verified when Keep my NFTs always online is selected, setting Automatically pin NFT's is enabled, default is disabled
Pinning - Passed
  • Verified able to pin NFTs
  • Verified when pinning is in process, shows message banner message about being pinned in local node
  • Verified when successfully pinned, show message about number of NFTs that are pinned
  • Additional verification notes for banner messages and pinning data recursively
Unpinning - Passed
  • Verified after removing an NFT and running curl http://127.0.0.1:<api port could be viewed at chrome://ipfs-internal>/api/v0/pin/ls?type=recursive, shows {"Keys":{}} message
    image
Pins Validation - Passed
  • Verified deleting brave_ifps folder and moving date ahead and restarting the browser restores brave_ipfs folder again
  • Verified resetting device date back to current date after restart starts to pin the NFT's again.
  • Running curl http://127.0.0.1:<api port could be viewed at chrome://ipfs-internal>/api/v0/pin/ls?type=recursive continues to show the pinned NFT details
Disable feature - Passed
  • Verified disabling Automatically pin NFTs setting and reloading the NFT tab shows the onboarding banner message
  • Verified manually deleting the NFT's from visible asset list also removes the onboarding banner message
Cleanup - Passed
  • Verified disabling Automatically pin NFTs setting, a new setting for Clear pinned NFTs is shown
  • Verified clicking that clears the NFTs pinned state and shows onboarding banner message
  • Encountered #29553 & #29554
Local node behaviour - Passed
  • Verified when user click on Get started with IPFS from interstitial page, IPFS local node is installed and started.
Error handling - Passed
  • Verified if NFT's content-type is not of the format image/*, NFT pinning fails. Verified as part of brave-browser#28775 (comment)
  • Verified due to network issues NFT pinning fails, it starts the pinning process again when browser is restarted

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.

7 participants
@jamesmudgett @kjozwiak @yrliou @srirambv @brave-builds @cypt4 and others