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

Use correct interface types in linking #210

Merged
merged 1 commit into from
Mar 27, 2023
Merged

Use correct interface types in linking #210

merged 1 commit into from
Mar 27, 2023

Conversation

joshuahannan
Copy link
Contributor

The public links need to use all the types in NonFungibleToken and TopShot for these purposes so they are compatible with as many 3rd party apps as possible. The Top Shot team should make sure they're following this transaction's model for linking an account

@joshuahannan joshuahannan requested a review from jrkhan March 22, 2023 21:42
@joshuahannan joshuahannan requested a review from a team as a code owner March 22, 2023 21:42
@joshuahannan joshuahannan requested a review from jphorec March 22, 2023 21:45
@jrkhan
Copy link
Contributor

jrkhan commented Mar 23, 2023

Out of curiosity, It would seem this transaction also be used to relink account collections at all expected paths?
Would think this might address this issue? (Per @bjartek)
#204

@bjartek
Copy link

bjartek commented Mar 23, 2023

Not the way it is written right now no, but it is not hard to adapt it.

@@ -25,7 +26,7 @@ transaction(numBuckets: UInt64) {
acct.unlink(/public/MomentCollection)
}

acct.link<&{TopShot.MomentCollectionPublic}>(/public/MomentCollection, target: /storage/ShardedMomentCollection)
acct.link<&{NonFungibleToken.Receiver, NonFungibleToken.CollectionPublic, TopShot.MomentCollectionPublic}>(/public/MomentCollection, target: /storage/ShardedMomentCollection)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not MetadataViews.ResolverCollection here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the sharded collection doesn't implement that interface so we can't link it

@joshuahannan joshuahannan merged commit c37116f into master Mar 27, 2023
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.

4 participants