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

Not all flow.json syntaxes are supported for account keys #313

Closed
jribbink opened this issue May 5, 2023 · 1 comment · Fixed by #466
Closed

Not all flow.json syntaxes are supported for account keys #313

jribbink opened this issue May 5, 2023 · 1 comment · Fixed by #466
Assignees
Labels
bug Something isn't working

Comments

@jribbink
Copy link
Contributor

jribbink commented May 5, 2023

Instructions

Issue To Be Solved

Currently, the Flow CLI supports 4 different ways for specifying account keys in the flow.json.

Unfortunately, the VSCode extension only supports one of these - the simple format post-flow-cli-v0.22. Because vscode-cadence relies on a signature match in order to verify emulator liveliness, this presents a problem to developers attempting to use any of the 3 other formats.

(Optional): Suggest A Solution

I think parsing flow.json is inherently a bit dangerous regarding compatibility and should be avoided as much as possible. While we could trivially support the "key" and "keys" variations of accounts in flow.json, this doesn't account for advanced account keys at all. We could obviously support all signing algorithms, etc. required for advanced account keys as of now, but implementing this is a lot of work & has a high risk of becoming outdated/incompatible yet again.

Considering that we only use this functionality to test emulator liveliness & everything else done regarding account keys is handled by the language server/golang side I think it would make more sense to do the following:

  1. Wait for emulator to start using port/hostname like we do now
  2. Send a dummy transaction using language server command in order to verify that we have the correct emulator.

This would produce exactly the same result as we have right now and leave the flow.json logic to the edge/Golang side.

(Optional): Context

Took me far too long to use the extension in kitty-items. After discovering why, I realized that we potentially don't support a number of existing flow.json files and this should probably be looked at (especially considering kitty-items is more or less the quintessential dapp example).

@jribbink jribbink added feedback feature A new user feature or a new package API labels May 5, 2023
@jribbink jribbink added bug Something isn't working and removed feedback feature A new user feature or a new package API labels May 6, 2023
@sideninja
Copy link
Member

@jribbink This is a good point about supporting schema changes. I would say tho that kitty-items is no longer a good example to test anything against as it hasn't been updated in too long. The "keys" has been deprecated since. But I agree we could change the check to something different, like sending an empty transaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants