FLIP 258: NFT Storage Requirement

[joshuahannan]

Adds a FLIP that proposes adding access(contract) var ownedNFTs: @{UInt64: {NonFungibleToken.NFT}} to the NFT standard Collection

PR: onflow/flow-nft#211
Discord Discussion

[joshuahannan]

After looking at the currently staged contracts on testnet, it looks like there are a good amount that still have ownedNFTs as access(all). Not sure how much of a problem this is. We would need to tell them all to re-stage their contracts

[cody-evaluate]

After looking at the currently staged contracts on testnet, it looks like there are a good amount that still have ownedNFTs as access(all). Not sure how much of a problem this is. We would need to tell them all to re-stage their contracts

fwiw, it dont think it will break anything because access(all) is a more derived/loose access level than the interface signature but Im not sure if the access(contract) on the Collection interface definition would gate access regardless of the access(all) concrete implementation or not (maybe someone could just force cast to the concrete and withdraw but im not 100% sure).

[joshuahannan]

fwiw, it dont think it will break anything because access(all) is a more derived/loose access level than the interface signature but Im not sure if the access(contract) on the Collection interface definition would gate access regardless of the access(all) concrete implementation or not (maybe someone could just force cast to the concrete and withdraw but im not 100% sure).

Why is that true? I just changed the access modifier in ExampleNFT to access(all) but kept the one in NonFungibleToken as access(contract) and it looked like it worked. It seems like an interface specifying an access level should need to be followed and having a looser access level should not be allowed.

@cody-evaluate

[joshuahannan]

I think you're right actually. So this won't be a breaking change. That is great news! I'll update the FLIP

[cody-evaluate]

fwiw, it dont think it will break anything because access(all) is a more derived/loose access level than the interface signature but Im not sure if the access(contract) on the Collection interface definition would gate access regardless of the access(all) concrete implementation or not (maybe someone could just force cast to the concrete and withdraw but im not 100% sure).

Why is that true? I just changed the access modifier in ExampleNFT to access(all) but kept the one in NonFungibleToken as access(contract) and it looked like it worked. It seems like an interface specifying an access level should need to be followed and having a looser access level should not be allowed.

@cody-evaluate

i 100% agree, im just saying that defining ownedNFTs in the concrete implementation as access(all) despite being defined as access(contract) in the interface wont break at compile time but there just might be security issues (due to force casting)

basically just saying access modifiers are covariant.

[sisyphusSmiling]

Amazing work getting this out so quickly!