自从我写了《NBA顶级投篮》的智能合约后，一年来我学到了什么？

你好！我叫Josh。我叫Josh，我在Dapper Labs为Flow区块链编写智能合约。

如果你是新来的，欢迎!这是我关于Cadence的双周博客，Cadence是Flow用于智能合约的新的最先进的语言。我建议在阅读本篇之前先阅读我的第一篇关于初学者材料的文章，因为我将假设读者已经对Cadence有了基本了解。

NBA Top Shot

如果你对了解Flow或Cadence有任何兴趣，你可能听说过NBA Top Shot，这是Dapper Labs官方授权的数字收藏品体验。Top Shot是一个大型项目，有许多活动部件，但时刻所有权和价值转移的记录由Flow区块链上的几个智能合约处理，我负责编写。(在Flow和Top Shot团队的许多其他人的宝贵帮助下！）。

NBA Top Shot - Pacers fans, @PaversGonnaPave的简介

查看他们收集的所有146个时刻

www.nbatopshot.com

是的，这是我的Top Shot系列，是的，我也是印第安纳步行者队的球迷。从Top Shot智能合约中的一些评论中，你可能会猜到这一点。

我在2019年9月开始为Dapper Labs工作，虽然我有多年的Solidity和Ethereum经验，但我对Cadence还是一个完全陌生的人。我在12月开始编写Top Shot合同，并在1月完成了初稿，之后我们花了几个月时间进行调整和测试，然后在6月推出。

自从我编写智能合约以来，一年多过去了，我作为一个软件开发人员，特别是Cadence开发人员的技能有了很大的提高。虽然我为我们所建立的东西感到自豪，并且对Top Shot智能合约的安全性和可靠性非常有信心，但事后看来，为了有更一致的设计、可读性和可用性，我有很多事情会做得不同，有点像一些NBA球队对他们的球衣选择的感觉。

与某些球队的球衣选择相比，NBA Top Shot可以说是非常成功的，这也激发了该领域的许多新团队想要在Flow上建立类似的体验。其中许多项目只是简单地复制和粘贴Top Shot的智能合约代码，只是改变一些名称，而没有花太多心思。

我完全理解这样做的冲动，因为这需要智能合约开发者付出最少的努力，但我希望更多的新人能从之前的项目决定中学习，这样他们就能对技术水平做出有意义的改进，而不是被次优的做事方式所困。

让我们来看看TopShot.cdc

核心的射门合同

如果你还没有，我建议你去看看Top Shot智能合约回购。

dapperlabs/nba-smart-contracts

这个资源库包含实现NBA Top Shot核心功能的智能合约和交易...

github.com

‍

那里有很多合同的文件，可以帮助你适应，但我在这里会做一个简单的概述。

TopShot.cdc是一个NFT合约（实现了NonFungibleToken接口），定义了Top Shot Moment NFT的分类、创建和所有权。

每一个Top Shot Moment NFT代表了NBA赛季中的一场比赛。比赛被分组，通常有一些首要的主题，如稀有性或比赛的类型。

一套剧目可以有一个或多个，同一剧目可以存在于多套剧目中，但剧目和一套剧目的组合，也就是所谓的版本，是独一无二的，也是对单个时刻的分类所在。

同一个版本可以铸造多个 "时刻"，每个 "时刻 "都有一个序列号，表明它是在哪个版本铸造的。

有趣的轶事。Top Shot曾经有一个完全不同的架构，我们有NFT模具来铸造新的时刻，但这是另一个故事。好时光。

正如你所看到的，主要结构是相当简单的。你将剧目添加到集上，然后从这些组合中铸币。它对我们和用户来说都很好，但我想对它进行三个主要改进，我认为其他正在制作类似经验的人应该考虑。

这些建议适用于原始版本的Top Shot合同。有些改进已经被Top Shot团队实施并包含 在升级中，但有些改进无法用传统方法升级，所以它们仍然存在。(留下的不是漏洞，只是非理想的设计）我将在更新中为每一个已经修复的问题加上一个FIXED 标志。

第一个改进。让字典和数组字段私有化

已修复

这是我看到Cadence开发者经常犯的一个错误。在Cadence中，公共字段意味着它们可以公开阅读，但不能公开写入。这只适用于字段本身，但如果该字段是一个字典或数组，该字典或数组的成员仍然可以被分配到。更多信息请参见我们的最佳实践文档。

反模式的Cadence

这是一个有主见的问题清单，如果在Cadence打算用于生产的代码中发现这些问题，就可以进行改进。A...

docs.onflow.org

‍

任何Cadence开发者都应该默认将所有字典和数组字段设为私有，我指的是access(contract)或access(self)。他们还应该为这些定义明确的getter和setter函数，使其非常清楚地表明开发者想要允许哪种类型的访问。

在Top Shot合同的情况下，这意味着这些字段应该成为访问（合同）。

If they are public, this means that the admin would have the ability to modify the retired status or number minted for specific moment plays, which is not an ability that the admin should have. The Top Shot team is fixing these issues in our contract promptly.

Second Improvement: More thoughtful metadata

The Play struct currently has two fields:

Metadata for Top Shot NFTs is currently simply a String to String mapping, so for example, a mapping in the metadata field might be "FullName": "Domantas Sabonis", or "Golden State Warriors": “Blew a 3–1 lead in the 2016 NBA Finals".

Each Moment has a bunch of fields. This is a decent way to manage it, but it doesn’t allow for much wiggle room. If we ever want to add more complex metadata to Top Shot moments, working with this construct will be difficult.

If I had to redo this, I would have thought more deeply about what potential metadata my project might need to store. It could be a json data structure, an image, or many other things. Every project needs to think deeply about what makes their metadata unique and figure out a way to include it in the smart contract code.

See this issue in the NFT standard repo for a discussion about on-chain metadata if you’d like to contribute!

Third Improvement: Unified Set resource and Set metadata struct

Partially Fixed: We added a unified set metadata struct, but could not unify the set resource

Currently, Sets in Top Shot are represented by two different data structures:

• A SetData struct, which records the id, name, and series of the set.
• A Set resource, which records other information about the set, including plays that are in it, editions, and retired statuses. It also acts as a authorization resource for the admin to create editions, mint moments, retire plays, and more.

We originally thought it might be useful to have them separate to keep static metadata about a set separate from the dynamic data that the admin controls, but in practice, this isn’t really that useful and just makes the code a little bit harder to understand and harder to query.

How could we have improved it?

If I could do it over again, I would have put all the SetData fields in the Set resource like this:

This way, we have a single unified Set object that the admin uses to manage set data. Then, I would have also removed the setDatas field from the smart contract, because those are stored in the Set resource now, so the field isn’t necessary.

I also would change theSetData struct definition to have all of the same fields as the Set resource. You might be wondering, “If you removed the SetData field, why does the SetData struct still need to exist?”

In the original version of the Top Shot contract, when someone wants to query information about a Set or Play in Top Shot, they have to call individual getter functions for each piece of data. This is very cumbersome and I have discovered in my time since writing the contract that it is almost always better to group related data together for queries.

Since SetData no longer needs to be the source of truth for important Set metadata, it can be used for a better purpose, an easily queryable source of all the current information about a Set. Now, if someone wants a piece or all of the information about a Set, they can simply find all the information in one place by instantiating a new SetData struct for the set they want to know about, like this!

They could parse it however they want within the script, or within the application code that sent the script. It gives the developer a lot more flexibility than the sufficient, but restrictive getters that the Top Shot contract currently provides.

Fourth Improvement: Perform state changing operations in admin resources, not in public structs

FIXED

In the original version of the Top Shot contract, when a new play or set is created, the ID tracker for the play or the set would be incremented to make sure that the next play or set created would have a unique ID. It would also emit an event indicating what the new play or set is. Seems fine on the surface, but the problem arises from where these operations happen.

These operations were performed in the initializer for the Play and Set structs:

At the time, Cadence did not, and still doesn’t, support private struct definitions. This means that anyone could create an instance of this Play struct or the SetData struct whenever they want. Every initialization would increment the id counter and emit an event, even though these sets and plays that were created were not real sets or plays. Eventually, the field would reach the limit for UInt32 and overflow, which could prevent new plays or sets from being created. This would be quite expensive to exploit, but is still a serious vulnerability that should always be avoided.

This references an important security aspect of smart contract development that should always be considered, regardless of the contract you are working on. If you have any operations that change important state in the contract, you should always default those operations to being hidden in an admin resource that restricts that functionality to only those who should be able to do it. There are obviously exceptions to this, but they should be carefully considered before committing to.

Fifth Improvement: Borrow references to resources instead of loading them from storage

FIXED

The Top Shot contract stores an important resource in a contract field, the Set resource. There are certain functions, including the queryable set metadata functionality that I described above, that need to get information about one of these sets that is stored in the contract. The solution that we originally used for accessing these sets was to load the whole set from storage, read its fields, and then put it back where it came from.

This works fine, but it is pretty inefficient. It takes a lot of computational resources to load an object from storage and save it again.

This coding pattern also caused a problem with the TopShotShardedCollection.cdc smart contract because when you load a resource from storage, its owner field is set to nil . If you try to access its owner field expecting it to be a certain value (like when you are emitting a Deposit event, for example), it will be nil , even if you plan to put it right back into storage right after withdrawing it like we did in the sharded collection contract.

The correct solution for this pattern is to borrow a reference to the resource instead. Borrowing a reference only uses only line of code instead of two and is a much more efficient operation:

‍

Conclusion

I hope these suggestions have been useful for some of you! I would like to further reiterate to those of you who want to build NFT projects on Flow to think deeply about what makes your NFTs unique and try to reflect that with unique features in your NFT smart contract. A simple copy and paste of the Top Shot contract will not set you apart from the rest of the pack.

If you have any comments about improvements you’d make to the Top Shot contract, or just want to talk smack about the Pacers, please share here or in our Discord! There may be some I am missing and we might even be able to include them in a later version.

If you have any questions, the entire Flow team, Top Shot team, and community is here to support you! Please do not hesitate to reach out via our Discord server, the Flow Forum, or via an issue in the Top Shot Github repo.

Are there any other topics or interesting projects that you know would useful to newcomers or that you would like me to write a blog post about? Feel free to comment with your ideas and I might include them in a future post!

Flow Discord: https://discord.gg/flow

Flow Forum: https://forum.onflow.org

Flow Github: https://github.com/onflow/flow

See you next week! 👋