Review Code Changes for New Token Standards

[joshuahannan]

Context

The design of the V2 token standards has been finalized and the code has all been written. It is already being used in the emulator and preview net, so we would like to merge the feature branches in their respective repositories to the main branches so they are the first things people see when exploring the token standards and so we can tag new releases from master. We'll need some code reviews to do this though.

Description

First, hopefully you have a decent understanding of the new Cadence features that are part of the Cadence 1.0 upgrade, such as entitlements, capability controllers, and others. It'll be hard to review the code for best practices if you don't have a decent understanding of the bigger new features.

You'll want to learn about the proposals first before you look at the code. That starts with reading the FLIPs:

• NFT: FLIP 56: Non-Fungible Token Standard Version 2 flips#56
• FT: FLIP 55: Fungible Token Standard Version 2 flips#55

If you want even more context, you can read the original forum post, but a lot of the information and proposals in there are outdated and not relevant any more, so no need to read and understand the whole thing.

Additionally, the READMEs in the respective feature branches might also be useful to help reinforce some of the information about the changes.

After you've familiarized yourself with the designs of the standards, you can start looking through the code changes in the PRs:

• NFT: FEATURE: V2 NonFungibleToken Standard flow-nft#126
• FT: FEATURE: V2 FungibleToken Standard flow-ft#131

I would recommend starting with the NFT standard changes because those include the changes to ViewResolver, which the FungibleToken standard depends on.

Helpful Tips while reviewing the PRs

General:

• Consider if the READMEs are organized coherently and provide a good introduction to the standards
• Consider if the standards, transactions, and scripts use Cadence best practices properly and are well commented and easy to understand
• Check to see if the flow.json has a good structure and if it is missing anything
• No need to review the go packages and tests. We are phasing those out in favor of Cadence tests. We haven't finished transitioning all the tests to the Cadence testing framework, so some of the Go tests still remain

NFT:

• While reviewing NFT, the changes to utility/FungibleToken.cdc don't need to be considered because they are handled by the FT standard PR

FT:

• While reviewing the FT, the changes to utility/NonFungibleToken.cdc, utility/ViewResolver, and utility/MetadataViews don't need to be considered because they are handled by the other PR