Changed data type of account key index to uin32 #3465
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Cadence Benchstat comparisonThis branch with compared with the base branch onflow:master commit cc99e72 Collapsed results for better readability
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
janezpodhostnik
commented
Jul 12, 2024
|
I had a look at the referenced issue and it sort of makes sense in flow-go. However, do we want to enforce limits of Flow in Cadence? It would be nice to keep Cadence agnostic of particular chain-limits. Even without the proposed changes, it should be possible to check/reject invalid indices. |
does this mean you would prefer cadence still having a |
|
Maybe it's good to discuss this a bit more synchronously, maybe as part of the next implementation sync? https://www.notion.so/flowfoundation/Cadence-Implementation-Sync-Notes-Agenda-cf6890eb8a9f4692a8c08611856a5af5?pvs=4#e78f967d7c814ea8b0178e83d39b2e4b. Can have it earlier, don't need to wait for a week. Can also have a separate sync about it
On one end we have the key management APIs and types (e.g. The question is where we define limits and how we enforce them. For example, if we enforce flow-go's (new?) limit to max |
properly convert Cadence Int to Go uint32 by checking range, do not truncate
|
@janezpodhostnik As discussed, I fixed the Cadence @SupunS PTAL 🙏 |
turbolent
approved these changes
Jul 16, 2024
SupunS
approved these changes
Jul 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Work towards onflow/flow-go#6204
Description
Changed data type of account key index to uin32.
There are still a few casts in the code, that I did not know how to get rid of.
masterbranchFiles changedin the Github PR explorer