-
Notifications
You must be signed in to change notification settings - Fork 105
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
Update NBA contracts & txs to Cadence 1.0 #248
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only had a chance to look through FastBreakV1
, but leaving initial comments (some nits)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great start! I only reviewed the contracts and left comments on them. You'll need to find someone on the studio team to review the rest of the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Followed up on a few of my comments that were never addressed and left a few more. The changes will require the contracts to be re-staged on testnet after you address them. Let me know before you want to re-stage so I can verify that everything has been addressed
contracts/FastBreakV1.cdc
Outdated
access(all) event Withdraw(id: UInt64, from: Address?) | ||
|
||
pub event Deposit(id: UInt64, to: Address?) | ||
access(all) event Deposit(id: UInt64, to: Address?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd recommend removing these because the default NonFungibleToken
events are much better
contracts/TopShot.cdc
Outdated
@@ -1191,7 +1224,7 @@ pub contract TopShot: NonFungibleToken { | |||
|
|||
// lock takes a token id and a duration in seconds and locks | |||
// the moment for that duration | |||
pub fun lock(id: UInt64, duration: UFix64) { | |||
access(NonFungibleToken.Update | NonFungibleToken.Owner) fun lock(id: UInt64, duration: UFix64) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you include this event emission here?
* cadence 1 upgrade * fix tests * token updated
Updated FastbreakV1 for Staging
* Bump golang.org/x/net from 0.19.0 to 0.23.0 in /lib/go/test Bumps [golang.org/x/net](https://github.com/golang/net) from 0.19.0 to 0.23.0. - [Commits](golang/net@v0.19.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * generated --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Taylor Petrychyn <taylor.petrychyn@dapperlabs.com>
Upgrade flow go sdk
* Fix tests * ci * Update go version
…nftUpdated event call, as it was already being called
Added events validation
NBA-2865: Fixed event unit tests
Fixed issues on events decoding
Add new destroy event name
…n-script Fixed check collection script
Fixed MarketPlace Contracts
* save cap on storage * fix var / entitlements * add comments and V1 Sale handling to create and start sale collection (#266) --------- Co-authored-by: Joshua Hannan <hannanjoshua19@gmail.com>
No description provided.