diff --git a/application/20240411-nft-storage.md b/application/20240411-nft-storage.md new file mode 100644 index 00000000..c9fcd43a --- /dev/null +++ b/application/20240411-nft-storage.md @@ -0,0 +1,119 @@ +--- +status: Implemented +flip: 258 +authors: Joshua Hannan (joshua.hannan@flowfoundation.org) + Cody Bouche (cody@evaluate.market) +sponsor: Joshua Hannan (joshua.hannan@flowfoundation.org) +updated: 2024-03-11 +--- + +# Non-Fungible Token Storage Requirement + +## Objective + +Enforce that NFT projects store their NFTs in their `Collection` +in an `access(contract)` dictionary field and provide a default function +for iterating over the NFT IDs in the collection. + +## Motivation + +The original NFT standard enforced that `Collection`s had a field for storing NFTs: +```cadence +pub var ownedNFTs: @{UInt64: NonFungibleToken.NFT} +``` + +In the V2 NFT Standard, it was decided that we wanted to give projects more flexibility +for how they store their NFTs, so this requirement was removed. +We didn't realize at the time that this requirement actually had benefits like +allowing more sophisticated and standardized scripts that interacted with collections, +like iterating through IDs in a collection without having +to load all of them into memory at once. + +## Design Proposal + +[The conversation that triggered this](https://discord.com/channels/613813861610684416/621847426201944074/1227401046926692443) +is in Discord. +A [pull request with the suggested code changes](https://github.com/onflow/flow-nft/pull/211) +is in the flow-nft github repository. + + +This FLIP proposes adding the `ownedNFTs` field back to the standard: + +```cadence +access(contract) var ownedNFTs: @{UInt64: {NonFungibleToken.NFT}} +``` + +It also proposes adding a function to `NonFungibleToken.Collection`, `forEachID()`, +that allows iterating through the IDs in a collection without having +to load all of them into memory first: +```cadence +/// Allows a given function to iterate through the list +/// of owned NFT IDs in a collection without first +/// having to load the entire list into memory +access(all) fun forEachID(_ f: fun (UInt64): Bool): Void { + self.ownedNFTs.forEachKey(f) +} +``` + +The function specification provides a default implementation so that projects +don't have to implement the function themselves. + +## User Benefit + +With a standard specification for how to store NFTs, developers +can have more confidence in understanding any given NFT project's storage. + +Without this storage specification, there would be no way to get all the IDs +in a large collection without hitting the computation limit. + +This also allows the standard to define more sophisticated functionality that +interacts with stored NFTs, like we have done with adding the `forEachID()` function. +In the future, there may be other functionality that is enabled +by having standardized storage. + +## Drawbacks + +This could make things a little more awkward for developers who do want to define +a more sophisticated system for storing NFTs +because they will still have to have the `ownedNFTs` field, +but it doesn't prevent them at all because they can simply ignore the field requirement +and do whatever else they want. + +## Alternatives Considered + +1. Keep the standard the same: + * This would make some projects like evaluate not be able to function properly who rely on the iterator feature for their app. +2. Only introduce the `forEachID()` function with a default implementation but without the storage requirement + * This would not potentially be a breaking change but would leave it up + to the projects to implement the function, which is not reliable. + We are far enough from the upgrade that this change should be acceptable. +3. Introduce a Cadence feature that allows getting slices from the IDs array + * There isn't time to do this before the Cadence 1.0 upgrade. + +## Performance Implications + +The `forEachID()` also has a limit (200k) on how many IDs it can access, +but that is much larger than almost any collection on Flow will ever be. + +## Dependencies + + +### Engineering Impact + +* Build and test time will stay the same. they are relatively small changes. + +### Best Practices + + +### Tutorials and Examples + +* Check out the Cadence 1.0 migration guide for instructions on how to update contracts to the new standards: + * https://cadence-lang.org/docs/cadence_migration_guide/ + +### Compatibility + +* FCL, emulator, and other such tools should not be affected. + +### User Impact + +* The upgrade will go out on the networks at the same time as Cadence 1.0 (Crescendo) if approved. \ No newline at end of file