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

Cadence 1.0 migration #1

Merged
merged 14 commits into from
Dec 15, 2024
Merged

Cadence 1.0 migration #1

merged 14 commits into from
Dec 15, 2024

Conversation

lealobanov
Copy link
Collaborator

No description provided.

@lealobanov
Copy link
Collaborator Author

@franklywatson this repo should be good to go now, apart from the import resolution of the NFT resource defined in TopShot : https://github.com/dapperlabs/nba-smart-contracts/blob/25e36b953e0153e66fa5f520bab960f3e3aede76/contracts/TopShot.cdc#L657

error: cannot create resource type outside of containing contract: `TopShot.NFT`
   --> f8d6e0586b0a20c7.Recipe:180:50
    |
180 |             let newMoment: @TopShot.NFT <- create TopShot.NFT(
181 |                 serialNumber: numInPlay + 1,
182 |                 playID: playID,
183 |                 setID: self.setID,
184 |                 subeditionID: 0
185 |             )
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

One option would be to duplicate the NFT struct inside the Recipe contract; my only concern with it is that the NFT struct is quite large (several hundred lines), most of which isn't directly relevant to the recipe logic

lealobanov and others added 3 commits December 9, 2024 17:02
@franklywatson
Copy link
Contributor

@lealobanov yeah I'm not thrilled about pulling in the NFT contract code. What this tells me is that example is wrong. We should pivot this and other examples trying to access private internal contract structures into proper examples that use the TopShot contract of today.

For actions like 'Create a Topshot Set' there should only really be a transaction, nothing more.

@lealobanov
Copy link
Collaborator Author

@franklywatson Agree with above, I've removed the redundant TS code duplication in Recipe.cdc and just left the transaction which runs against the imported TS contract. Will raise a small PR on the main frontend repo to make the embedded contract code snippet optional, so we can support recipes that are tx only. Thanks!

@lealobanov lealobanov merged commit 8c0cf57 into main Dec 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants