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

Warn if a legacy addres is used #75

Closed
wants to merge 5 commits into from

Conversation

gasull
Copy link

@gasull gasull commented Feb 6, 2021

Legacy addresses are the source of confusion for many BCHA users.

See:

https://blog.cex.io/news/why-you-should-never-send-bch-to-btc-address-21346

This changeset creates a popup warning window when the user tries to
send BCHA to a legacy address. The window has a "Never show this again"
checkbox.

This is a backport of Electron-Cash#2129.


This change is Reviewable

EchterAgo and others added 5 commits January 31, 2021 08:52
Both include good bug fixes. One user hit the issue where
`libusb_get_device_list` returned `LIBUSB_ERROR_OTHER` which is fixed
by this commit included in libusb 1.0.23:

libusb/libusb@fd95d44

closes Electron-Cash#2132

Backport note:
 - python libusb1 was already updated for  ElectrumABC
Seed generation: use BIP39 by default for better compatibility:
* Most (all?) hardware wallets use BIP39 seeds in combination with BIP43-style derivation paths (like m / purpose' / coin_type' / account' / change / address_index)
* Electrum uses BIP43 derivation paths for all hardware wallets, AND for BIP39 seeds imported in software wallets
* Electrum seeds (not BIP39) use seed versioning instead of purpose derivation path.
* Software wallets with Electrum seeds use "m / change / address_index" derivation paths

User interface improvements:
* 2FA config is removed from initial setup as it confuses some users
* Instead, 2FA can be activated from the option menu (clicking on the Satochip logo on the lower right corner

Other minor corrections:
* PIN can have a maximum length of 16, not 64

Backport notes:
 - some minor changes were necessery to account for ElectrumABC PR Electron-Cash#57
 - untested because I do not own a satochip card
During wallet creation, the following error sometimes pops to the user:
"Failed to create a client for this device. Make sure it is in a correct state".

This error does not always show, and even when it does, it does not stop the wallet creation.

This issue is generated in the method 'has_usable_connection_with_device()'.
The reason is that sometimes the card is not ready when the wizard tries to connect to it.
Retrying after a short delay is enough to solve the issue.
Legacy addresses are the source of confusion for many BCHA users.

See:

https://blog.cex.io/news/why-you-should-never-send-bch-to-btc-address-21346

This changeset creates a popup warning window when the user tries to
send BCHA to a legacy address.  The window has a "Never show this again"
checkbox.
@PiRK
Copy link
Collaborator

PiRK commented Feb 12, 2021

Thanks. For some reason I did not get a notification and just saw your PR right now. Will review soon.

@PiRK
Copy link
Collaborator

PiRK commented Feb 17, 2021

Closed in favor of #89

I cherry-picked your commit and added a few minor changes, to avoid hardcoding the project name "Electrum ABC" or the currency unit name. I want to be able to change these in a single place.

@PiRK PiRK closed this Feb 17, 2021
@gasull gasull deleted the abc-warn-legacy-address branch February 19, 2021 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants