Sadie/AllDay Secure Cadence upgrade #10
Conversation
| @@ -244,7 +244,9 @@ func metadataDict(dict map[string]string) cadence.Dictionary { | |||
| pairs := []cadence.KeyValuePair{} | |||
|
|
|||
| for key, value := range dict { | |||
| pairs = append(pairs, cadence.KeyValuePair{Key: cadence.NewString(key), Value: cadence.NewString(value)}) | |||
| k, _ := cadence.NewString(key) | |||
There was a problem hiding this comment.
These changes aren't related to the secure Cadence work but rather an older version of the SDK that we hadn't updated in a while
| self.id = series.id | ||
| self.name = series.name | ||
| self.active = series.active | ||
| if let series = &AllDay.seriesByID[id] as &AllDay.Series? { |
There was a problem hiding this comment.
Talked to Bastian about best practise syntax for this. Slack thread here for context: https://axiomzen.slack.com/archives/CTGAW0TM2/p1653430599342029
| self.ownedNFTs[id] != nil: "Cannot borrow NFT, no such id" | ||
| } | ||
|
|
||
| return (&self.ownedNFTs[id] as &NonFungibleToken.NFT?)! |
There was a problem hiding this comment.
Used the force unwrap here because we have already nil checked in the precondition and I figured it would be easier than changing the function signature to limit what we need to change on our transactions
There was a problem hiding this comment.
(Open to other suggestions here though!)
There was a problem hiding this comment.
yeah, I think in an ideal world, we would return an optional, but that would require changing the standard, which would be a breaking change unfortunately
sadief
changed the title
sadief
requested review from
benjaminkvm,
EricLin2004,
fkenji,
judezhu,
srieeni and
joshuahannan
| self.ownedNFTs[id] != nil: "Cannot borrow NFT, no such id" | ||
| } | ||
|
|
||
| return (&self.ownedNFTs[id] as &NonFungibleToken.NFT?)! |
There was a problem hiding this comment.
yeah, I think in an ideal world, we would return an optional, but that would require changing the standard, which would be a breaking change unfortunately
|
Is this ready to be merged? |
|
@joshuahannan yes - merging now - the contract is upgraded now on testnet |
Updating the AllDay.cdc contract to be compatible with the Secure Cadence upgrade. Details on upgrade here: https://forum.onflow.org/t/breaking-changes-coming-with-secure-cadence-release/3052/14
Main things updated in this contract were for the following breaking changes:
Taking a reference to an optional value now produces an optionally-typed reference