Skip to content
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 the crypto algos during account creation #460

Closed
tarakby opened this issue Oct 22, 2024 · 4 comments · Fixed by #461
Closed

Update the crypto algos during account creation #460

tarakby opened this issue Oct 22, 2024 · 4 comments · Fixed by #461

Comments

@tarakby
Copy link
Contributor

tarakby commented Oct 22, 2024

  1. Looking at the account creation transaction, the algorithm checks and the error messages need to be updated to avoid confusion: although Cadence supports many signature and hash algorithms, not all of them can be used to create a Flow account. The transaction check should be updated to include only the supported algorithms by a Flow account :
  • signature: ECDSA_P256 and ECDSA_secp256k1
  • hash: SHA2_256 and SHA3_256
    In the current code, any other combination would pass the sanity check but the account creation would fail later.
  1. Since the transaction creates a new account with a single key, the key must be a full-weight one (otherwise the new account is locked). Therefore it may be not useful to accept the key weight as an input. The transaction could hardcode the weight to 1000.
@tarakby
Copy link
Contributor Author

tarakby commented Oct 22, 2024

@joshuahannan I'm happy to make any eventual changes, but I wanted to discuss the issue above first.

@joshuahannan
Copy link
Member

@tarakby
Copy link
Contributor Author

tarakby commented Oct 23, 2024

How about the point (2) in the issue description, I think that would add a breaking change to the transaction. Should I avoid it?

@joshuahannan
Copy link
Member

Oh yeah, sorry for missing that. I think we could remove the weight argument. We're already introducing breaking changes. Might as well make sure everything else is as we want it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants