Update PDS & PackNFT contracts & txs to Cadence 1.0 #47

loic1 · 2024-03-18T19:16:06Z

Changes:

Update PDS & PackNFT contracts and transactions to Cadence 1.0 / NFT Standard V2, including:
- Create new entitlements:
  - Operatable for PackNFT operations.
  - DistCreation for distribution creation.
- Update and add collection methods to enable compliance with NFT Standard v2.
Update test setup to fit with latest Cadence 1.0-based emulator, tooling, and standard contracts.

State migration from existing pre-1.0 contract is currently blocked by onflow/cadence#3182.

Capabilities migration will require more attention.

For context on Cadence 1.0:

pds/contracts/IPackNFT.cdc

loic1 · 2024-03-20T20:19:07Z

pds/contracts/IPackNFT.cdc

-        pub case Opened
-    }
+    // Enums cannot be declared anymore in interfaces in Cadence 1.0
+    // access(all) enum Status: UInt8 {


The Status enum remain needed in PackNFT contracts, but we remove it in IPackNFT because concrete type declarations are not allowed in contract interfaces anymore

good to know. We can just remove it then

loic1 · 2024-03-20T20:20:06Z

pds/contracts/PDS.cdc

+
+access(all) contract PDS{
+    /// Entitlement that grants the ability to create a distribution
+    access(all) entitlement DistCreation


New entitlement to control access for distribution creation, replacing dist creator resource interface in pre-Cadence 1.0 contract

loic1 · 2024-03-20T20:21:15Z

pds/contracts/PDS.cdc

-        access(self) let withdrawCap: Capability<&{NonFungibleToken.Provider}>
-        access(self) let operatorCap: Capability<&{IPackNFT.IOperator}>
+    access(all) resource SharedCapabilities {
+        access(self) let withdrawCap: Capability<auth(NonFungibleToken.Withdraw) &{NonFungibleToken.Provider}>


withdrawCap & operatorCap must now allow access to authorized references to the NFT Provider & PackNFT Operator, respectively.

loic1 · 2024-03-20T20:21:26Z

pds/contracts/PDS.cdc

            pre {
                cap.borrow() != nil: "Invalid capability"
            }
            self.cap = cap
        }

-        pub fun create(sharedCap: @SharedCapabilities, title: String, metadata: {String: String}) {


create is now a reserved keyword

loic1 · 2024-03-20T20:26:36Z

pds/contracts/PackNFT.cdc


-        pub fun reveal(openRequest: Bool){
+        access(NonFungibleToken.Update | NonFungibleToken.Owner) fun reveal(openRequest: Bool){


revealing and opening a pack NFT requires NonFungibleToken.Update- or NonFungibleToken.Owner-authorized reference

loic1 · 2024-03-20T20:27:49Z

pds/contracts/PackNFT.cdc

-            PackNFT.totalSupply = PackNFT.totalSupply - (1 as UInt64)
+        /// Event emitted when a NFT is destroyed (replaces Burned event before Cadence 1.0 update)
+        ///
+        access(all) event ResourceDestroyed(id: UInt64 = self.id)


With the custom destroy function being removed from Cadence 1.0:

NFT destruction should now be tracked by the special event with the reserved name ResourceDestroyed.

TotalSupply will not account for NFTs being destroyed anymore.

The corresponding PackNFT resource will have to be deployed in a separate call (e.g., in the same tx or function call where the NFT is being destroyed).

…urnCallback & comments

joshuahannan

I only reviewed for Cadence best practices. Didn't really review the business logic, transactions, or tests. Mostly looks good! Just a few small comments

pds/contracts/IPackNFT.cdc

pds/contracts/PDS.cdc

joshuahannan · 2024-04-03T15:15:14Z

pds/contracts/PackNFT.cdc

+        access(all) view fun getViews(): [Type] {
+            return []
+        }
+
+        /// Resolve this NFT's metadata views.
+        ///
+        access(all) view fun resolveView(_ view: Type): AnyStruct? {
+            return nil
+        }


Are you sure that this can't be represented by any views? I would think that at the minimum, you would at least want the NFTCollectionData and NFTCollectionDisplay views, but there might be even more that apply

The views depend on each individual PackNFT instance. The AllDay version of the PackNFT contract updated in this PR defines the views. Added a comment in the vanilla PackNFT contract to make it clear that the views should be implemented 👍

pds/contracts/PackNFT.cdc

judezhu · 2024-04-08T16:30:48Z

pds/contracts/IPackNFT.cdc


        access(contract) fun reveal(id: UInt64, nfts: [{IPackNFT.Collectible}], salt: String)
        access(contract) fun open(id: UInt64, nfts: [{IPackNFT.Collectible}])
-        init(commitHash: String, issuer: Address)
+        view init(commitHash: String, issuer: Address)


is this init method read-only? this is a bit confusing

Yes, I was confused too initially but the view keyword means the method is read-only with respect to the state outside of the method itself 👍 (for more details: https://cadence-lang.org/docs/1.0/language/functions#view-functions)

judezhu · 2024-04-08T16:44:28Z

pds/contracts/IPackNFT.cdc

-        pub case Opened
-    }
+    // Enums cannot be declared anymore in interfaces in Cadence 1.0
+    // access(all) enum Status: UInt8 {


good to know. We can just remove it then

judezhu · 2024-04-08T17:04:44Z

pds/transactions/deploy/deploy-packNFT-with-auth.cdc

                version: version,
            )
        } else {
            log("has contract")
-            owner.contracts.update__experimental(name: contractName, code: code.decodeHex())
+            owner.contracts.add(name: contractName, code: code.decodeHex())


this should be an update?

Good call, fixed it - thank you!

judezhu

Great work!

loic1 added 2 commits February 29, 2024 10:39

wip

3d559bc

update PDS & PackNFT to Cadence 1.0 (wip)

797b5a9

loic1 changed the base branch from main to stable-cadence March 18, 2024 19:16

loic1 added 5 commits March 18, 2024 13:36

wip

180c8db

update interface conformance

5e4effc

change type resource interface

e795051

bump test go version requirement

f6dff24

update assets

cc91459

loic1 commented Mar 20, 2024

View reviewed changes

pds/contracts/IPackNFT.cdc Outdated Show resolved Hide resolved

loic1 commented Mar 20, 2024

View reviewed changes

loic1 marked this pull request as ready for review March 20, 2024 20:31

loic1 requested review from a team as code owners March 20, 2024 20:31

loic1 requested review from judezhu, rrrkren and Deewai March 20, 2024 20:31

loic1 added 3 commits March 25, 2024 09:12

clean up conformances (following emulator state migration fix), add b…

befe7b2

…urnCallback & comments

updated view keyword, withdrawCap type

6cb0bd4

fix withdrawCap type

2906ec6

jacksonConrad approved these changes Apr 3, 2024

View reviewed changes

joshuahannan reviewed Apr 3, 2024

View reviewed changes

rename entitlement, fix pre-condition

e8e4a0e

judezhu reviewed Apr 8, 2024

View reviewed changes

loic1 added 2 commits April 10, 2024 10:37

actually use update for contract update

0173052

remove unnecessary borrowPackNFT

d888633

loic1 requested review from judezhu and joshuahannan April 10, 2024 17:40

judezhu approved these changes Apr 10, 2024

View reviewed changes

loic1 added 2 commits April 11, 2024 14:50

fix getLenght

43865c9

add changes to PackNFT_TopShot

f5cbcb6

joshuahannan approved these changes Apr 12, 2024

View reviewed changes

loic1 merged commit 70d1970 into stable-cadence Apr 12, 2024
1 check passed

loic1 deleted the loic/update-pds-packnft-contracts-v1 branch April 12, 2024 16:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update PDS & PackNFT contracts & txs to Cadence 1.0 #47

Update PDS & PackNFT contracts & txs to Cadence 1.0 #47

loic1 commented Mar 18, 2024 •

edited

Loading

loic1 Mar 20, 2024

judezhu Apr 8, 2024

loic1 Mar 20, 2024

loic1 Mar 20, 2024

loic1 Mar 20, 2024

loic1 Mar 20, 2024

loic1 Mar 20, 2024

joshuahannan left a comment

joshuahannan Apr 3, 2024

loic1 Apr 10, 2024

judezhu Apr 8, 2024

loic1 Apr 10, 2024

judezhu Apr 8, 2024

judezhu Apr 8, 2024

loic1 Apr 10, 2024

judezhu left a comment


		pub fun reveal(openRequest: Bool){
		access(NonFungibleToken.Update \| NonFungibleToken.Owner) fun reveal(openRequest: Bool){

Update PDS & PackNFT contracts & txs to Cadence 1.0 #47

Update PDS & PackNFT contracts & txs to Cadence 1.0 #47

Conversation

loic1 commented Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshuahannan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

judezhu left a comment

Choose a reason for hiding this comment

loic1 commented Mar 18, 2024 •

edited

Loading