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 #2129

Merged
merged 2 commits into from
Feb 4, 2021

Conversation

gasull
Copy link

@gasull gasull commented Jan 16, 2021

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

See Why you should never send your BCH to BTC address.

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


This change is Reviewable

Legacy addresses are the source of confusion for many BCH 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 BCH to a legacy address.  The window has a "Never show this again"
checkbox.
@cculianu
Copy link
Collaborator

This commit is a great idea. Thanks for the contribution. Will review...

@gasull
Copy link
Author

gasull commented Jan 16, 2021

I also wanted to point out that since Electron Cash and Electrum are very similar applications in look and feel, a user might have both Electron Cash and Electrum open at the same time, and copypaste a legacy address on Electron Cash when meaning to do it on Electrum. This changeset will mitigate those human errors.

@cculianu
Copy link
Collaborator

I also wanted to point out that since Electron Cash and Electrum are very similar applications in look and feel, a user might have both Electron Cash and Electrum open at the same time, and copypaste a legacy address on Electron Cash when meaning to do it on Electrum. This changeset will mitigate those human errors.

Hmm. Yeah I never thought about that.

@cculianu
Copy link
Collaborator

Ping @gasull

@gasull
Copy link
Author

gasull commented Jan 27, 2021

Ping @gasull

Sorry, I was unable to work on this lately. Getting back to it now.

@gasull
Copy link
Author

gasull commented Jan 27, 2021

@cculianu , this is ready to be code-reviewed again.

@cculianu
Copy link
Collaborator

cculianu commented Feb 4, 2021

Looks great. I like it. I reviewed. Merging. Note I want to modify the test a bit -- but I can do that myself. Just a word or two.

Great work. I like how you added a "Show details.." text.. and the text makes sense.

@cculianu cculianu merged commit aa153e2 into Electron-Cash:master Feb 4, 2021
cculianu added a commit that referenced this pull request Feb 4, 2021
Also modified the formattng a little bit.
@gasull gasull deleted the bch-warn-legacy-address branch February 6, 2021 05:18
EchterAgo pushed a commit to EchterAgo/Electron-Cash that referenced this pull request Mar 11, 2021
EchterAgo pushed a commit to EchterAgo/Electron-Cash that referenced this pull request Mar 11, 2021
EchterAgo pushed a commit to EchterAgo/Electron-Cash that referenced this pull request Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants