cryptoadvance / specter-desktop

A desktop GUI for Bitcoin Core optimised to work with hardware wallets
MIT License
793 stars 236 forks source link

Extension-generation needs adjustements #1957

Open k9ert opened 1 year ago

k9ert commented 1 year ago

The extension-generation process is currently assuming you need encrypted userdata. That should get adjusted like this:

Mind the different templating syntax. Those are "metatemplates" as they are creating templates which themself are later templates.

You can test it via using --tmpl-fs-source and checkout the dummy-project somewhere. Also i've created a small section about https://docs.specter.solutions/desktop/extensions/intro/#virtualenv-management

k9ert commented 1 year ago

One other thing here would be that the templates are becoming more dependent on the version of specter controlling the generation-process. It would be cool if we could get another cli-switch which would specify the version of specter, simply assuming that the dummy-repo has tags with that version (and maybe falling back to master if it doesn't). This is already established in parts: https://github.com/cryptoadvance/specter-desktop/blob/master/src/cryptoadvance/specter/services/extension_gen.py#L41 but need to be parametrized and maybe the fallback-solution need to be implemented.

Aditi-Singh16 commented 1 year ago

Hi @k9ert, I have a confusion... You said that "The extension-generation process is currently assuming you need encrypted userdata." could you please explain what you meant by this?

Like what data is encrypted and why encrypted data? Also when I created the extension, I didn't realize this... could you point out where is the indication of this assumption?

k9ert commented 1 year ago

It's here: https://github.com/cryptoadvance/specter-desktop/blob/a45daba4f9a39af26d75ffb21d70e955c5a49f2a/src/cryptoadvance/specter/services/service.py#L41 And it's not even true in the meantime. Because that flag is inherited, the current standard behaviour is, that the data is not encrypted. So making that optional and then setting the value somewhere here in the generation-process would be cool. For that, you would ask about that in the cli_ext.py and create an attribute in the extension_gen.py, adding it in the Environment (line 70) which will be relevant if the service.py (see link above) is rendered in line 107.

It depends on whether you want to have encryption on how sensitive your data is. Labeling or something like that might not be that critical. In the Swan extension (where Keith invented the encryption), we obtained a token to swan which needed to kept secure as it's similiar to a password. So we encrypted the token with the password of the user. If the token is decrypted, it's only kept in memory. This has the disadvantage that if specter boots up anew, it doesn't have access to any of the encrypted data until the user is typing in his password. If the functionality of the extension is dependent on that, this might also be a major UIUX fail that's why one should not encrypt the data until it's really necessary. You can also use both at the same time but that's currently not even documented and far being an option in the generation-process.