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

Improve xmr proof service #6217

Merged
merged 14 commits into from
Jun 1, 2022

Conversation

chimp1984
Copy link
Contributor

Fixes some issues brought up by kayabanerve

@chimp1984 chimp1984 marked this pull request as ready for review May 26, 2022 21:31
@ripcurlx
Copy link
Contributor

@jmacxx Could you please review this PR? Thanks!

@ghost
Copy link

ghost commented May 30, 2022

@ripcurlx yes, I am in process of reviewing/testing the code. Will let you know ASAP.

This popup text could probably use some revision from yourself / @m52go / @w0000000t to make it more understandable. It is shown at startup (only users with XMR accounts).

image

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK
I tested mainly the subaddress generation - works fine for me.

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK - based on #6217 (review)

@ripcurlx ripcurlx added this to the v1.9.2 milestone Jun 1, 2022
@ripcurlx ripcurlx merged commit a62655c into bisq-network:master Jun 1, 2022
@w0000000t
Copy link
Contributor

@ripcurlx yes, I am in process of reviewing/testing the code. Will let you know ASAP.

This popup text could probably use some revision from yourself / @m52go / @w0000000t to make it more understandable. It is shown at startup (only users with XMR accounts).

image

I thought I had replied already, sorry for the delay! I probably left the browser tab hanging and it went lost.
What I wrote the last time was to make the popup text shorter and more effective, like so:

WARNING
After changing data directory you must also change XMR wallet and address for security reasons [1]

@ghost
Copy link

ghost commented Jun 1, 2022

@w0000000t thanks that looks good to me. How about this revision:

Notice for XMR users
If changing data directory you must also change XMR wallet and address for security reasons [1]
Read about why it's a good idea to switch to a new data directory every so often. [2]

[1] https://www.getmonero.org/2018/09/25/a-post-mortum-of-the-burning-bug.html
[2] https://bisq.wiki/Switching_to_a_new_data_directory

@w0000000t
Copy link
Contributor

The amount of text increased, but likewise for information density, I am cool with it

@ghost ghost mentioned this pull request Jun 5, 2022
@w0000000t
Copy link
Contributor

I am currently testing this, and it didn't show on startup for me, or at least I cannot be 100% sure it didn't show when starting 1.9.3 testing instance with existing data directory with existing XMR accounts as I did close the popups quite shortly thereafter 😅
But I used the arbitrator instance, which had no existing XMR accounts, to create an XMR account and restart it, and this popup didn't show.

@ghost
Copy link

ghost commented Jun 24, 2022

The popup moved to Account menu: #6236 (comment)

@chimp1984 chimp1984 deleted the improve-xmr-proof-service branch October 20, 2022 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants