FEATURE: Cadence 1.0 Core Contracts Changes #319
Conversation
joshuahannan
force-pushed
the
stable-cadence
branch
from
7479ccf
to
18e5b36
Compare
* update to view functions for stable cadence * remove unreachable code * update templates to Stable Cadence * update to Stable Cadence preview 4 * update for stable cadence * fix parse error --------- Co-authored-by: Josh Hannan <[email protected]> Co-authored-by: Bastian Müller <[email protected]>
joshuahannan
force-pushed
the
stable-cadence
branch
from
0e0a35d
to
253f985
Compare
* update deps, add entitlement for locked account creator * make ci
joshuahannan
requested review from
jordanschalm,
janezpodhostnik and
sisyphusSmiling
and removed request for
bluesign
|
I think we're basically ready to merge this to master, so I added a few people who might be able to give it a review! The changes are fairly straightforward, just lots of boilerplate updates to be compatible with the new token standards and Cadence 1.0. Let me know if you'd like to chat with me first about the changes before you review. |
|
@sisyphusSmiling The changes were big enough that I just created a PR with them for you to review. Can you take a look? |
* better use of revertible random with correct types * improve random distribution
| ?? panic("No moves pending list in account storage") | ||
|
|
||
| // Iterate through all delegators and unstake their tokens | ||
| for delegator in nodeRecord.delegators.keys { | ||
| // since their node has unstaked | ||
| let keys = nodeRecord.delegators.keys |
There was a problem hiding this comment.
Seems like this iteration and the logic within it is repeated a couple times. Could this be made into a helper or maybe an iterator like forEachDelegator? Not pressing at all, maybe something to think about as a change to include later.
There was a problem hiding this comment.
Good idea! I created an issue for later: #431
| signer.load<{UInt64: UInt64}>(from: /storage/executionEffortWeights) | ||
| signer.save(newWeights, to: /storage/executionEffortWeights) | ||
| prepare(signer: auth(Storage) &Account) { | ||
| signer.storage.load<{UInt64: UInt64}>(from: /storage/executionEffortWeights) |
There was a problem hiding this comment.
Applies to all transactions where raw values are saved in storage - do we need any sort of input validation for these? I'd imagine the values are sensitive and probably live in storage due to contract update limitations, but adding validation in the committing transaction would at least feel a bit safer.
Getting out of scope a bit, but if we know what these values will remain and don't think more will be added in the future, we could move them to some kind of config resource or contract and add setters so the input validation is done at the contract level and we could emit events when the values are updated.
There was a problem hiding this comment.
yeah, I'm not sure why they weren't added to the FlowServiceAccount.Administrator resource in the first place.
I created an issue: #432
|
@jordanschalm Would you be able to give a review of just the contracts here? No need to look at all the tests and transactions since that is mostly just boilerplate. Want to get one more approval on this before we merge it. |
| // Additionally, these events will eventually be removed from this contract completely | ||
| // in favor of the FungibleToken events |
There was a problem hiding this comment.
Is there any timeline for making that change? I seem to remember we were considering doing this as part of Cadence 1.0 / Crescendo?
There was a problem hiding this comment.
I need to double check with Vishal about that, but we had decided that we didn't want to remove it as part of Crescendo because there were partners who it would be a breaking change for. I still would prefer to remove it since there are other breaking changes. I'll ask again. I created an issue to track it: #434
|
Thank you so much for the review @jordanschalm! I'll work on getting all the Cadence dependencies updated for this and the other contract repos soon and then we'll be able to merge this! |
|
Finally merged to master! Thank you everyone for all the reviews and help on this! 🥳 🎉 |
Includes changes related to Cadence 1.0
Many functions have been changed to
viewfor new view function requirementsUpdates casting to use correct
assyntaxchanges
pubtoaccess(all)Adds entitlements
Uses the new fungible token standard and
BurnercontractRemoves custom destructors