Encryption manager

[molnard]

antecedent: #1122
fixes: #1121

Prerequisites

• MultiLine TextBox clips the first line after pressing enter. @danwalmsley
  

• Encrypt/Decrypt message must be implemented into NBitcoin @lontivero @NicolasDorier

• Electrum compatibility check

• Clear warning message after 7 sec

• Decrypt address browser design fix

• Review BIP proposal: https://github.com/cgilliard/BIP/blob/master/README.md

[lontivero]

@molnard the interface for Encrypt/Decript is the following:

class Pubkey
public string Encrypt(string message)

class Key
public string Decrypt(string message)

You could create two extension methods that do nothing and move forward with them because the interface will not change.

[lontivero]

@molnard the interface for Encrypt/Decript is the following:

class Pubkey
public string Encrypt(string message)

class Key
public string Decrypt(string message)

You could create two extension methods that do nothing and move forward with them because the interface will not change.

[NicolasDorier]

Encrypt has been released in the last version of NBitcoin

[molnard]

Specification

1. Can only open Encryption manager if a wallet is loaded (check point 4). Multi wallet operation is problematic because of the password and UI over complexity
2. UI structure is similar to WalletManager
3. Sign, Verify, Encrypt, Decrypt on separate tabs
4. The Tabs can be separated in the following way: Sign and Encrypt wallet dependent Verify and Decrypt wallet independent

[lontivero]

Btw, I verified the compatibility with Electrum both in NBitcoin unit test and performing a real end-to-end manual test by encrypting with Electrum and decrypting with Wasabi and in the other way too (Encrypting with Wasabi and decrypting with Electrum). It is 100% compatible. You can check the "Electrum compatibility check" checkbox.

[molnard]

Should add #1145 after merged avoiding conflict in receive tab

[molnard]

Message authenticity verified

As far as I can see. Warning messages always on the bottom. Success messages always nearby the function. I will leave this as it is now.

[lontivero]

@molnard @danwalmsley I tried to make the multiline textboxes to fill all the available space with no luck. Is it possible? Did you already try?

[molnard]

@molnard @danwalmsley I tried to make the multiline textboxes to fill all the available space with no luck. Is it possible? Did you already try?

no it is not possible. It will only grow if the text is big enough.

[molnard]

The same functionality should be added to the Receive table address list IMO. This is because users could create new addresses with labels such as "for encrypting, David, Adam" specially created by signing/encryption.

Agree, but it must not be prominent, because I suspect nobody will use it and it'll definitely add to the complexity and confuse people. In fact I just met with a OG Bitcoiner, who's was confused by how to use Wasabi, so we may want to pay even more attention to UX than we did previously.

We can simplify the right click menu by rearranging the items like this:

• Copy address
• Dequeue coin
• Encrypt manager
  • Sign message
  • Verify message
  • Encrypt message
  • Decrypt message

Do this?

[nopara73]

@molnard Let's go through this whole PR together when I'm back in Hungary.

[nopara73]

There was a new BIP proposal today by @cgilliard on the mailing list in the topic: https://github.com/cgilliard/BIP/blob/master/README.md

It'd be wise to review it for this PR.

[lontivero]

That bip describes what is already known as compact signatures (the ones used by bitcoin and by wasabi to proof inputs ownership) but with additional bits in the recId to let us know what keyhash to use in the verification. I will review it when i have my laptop in front.

Sent from my Moto G(4) using FastHub

[cgilliard]

Correct. Basically, these bits allow you to distinguish between p2pkh, p2sh segwit and bech32 segwit from the signature header.

…

On Sun, Feb 17, 2019 at 3:46 PM Lucas Ontivero ***@***.***> wrote: That bip is describes what is already known as compact signatures (the ones used by bitcoin and by wasabi to proof inputs ownership) but with additional bits in the recId to let us know what keyhash to use in the verification. I will review it when i have my laptop in front. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1127 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AG5at_5-opdtFYHH_vBq0O8wT4APVDYjks5vOb-vgaJpZM4aWeJ5> .

[luke-jr]

It's not a good idea to use/extend the old message signing scheme.

Read more here bitcoin/bitcoin#10542

[molnard]

By the way we have Electrum compatibility currently. I will take a look at BIP but any change can harm this.

[lontivero]

@molnard this new BIP will break a lot of compatibility, right. Electrum explicitly checks the recId is in the range [0, 3) so, any signature following this BIP using an address other than p2pkh will be invalid for Electrum. Moreover, signatures following this BIP will break NBitcoin key extraction because of how it is implemented.

The PR seems to be for compatibility with Ledger but we really don't know if we are going to see an standard or, for the contrary, we are going to see even more incompatibility.

My recommendation is moving forward with what we have and watch how the BIP is received and how much traction/adoption it has in the coming year.

[nopara73]

I agree with @luke-jr and I've been screaming from the start that it'll go much deeper than it seemed at the start. However I also agree with @lontivero, we put way too much work into it, so we should finish it now. I also agree with @lontivero that our primary concern should be compatibility with Electrum and other wallets.

[molnard]

This can be closed, continued: #1241