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

Make the coinjoin input warning less scary #3728

Merged
merged 2 commits into from Jul 1, 2021

Conversation

PastaPastaPasta
Copy link
Member

I'm not sure how much of a fan I am of the warning text I propose, I'm open to suggestions, however, I think this kind of a modification is quite needed.

Milestone should probably be 17 b/c translation changes, however, it'd probably be fine to include in 16

Scary Warning
more scary
Less Scary Warning
less scary

@kxcd
Copy link

kxcd commented Sep 23, 2020

Thanks for making this change, but where did you guys learn english? Can we consider may mildly harm your privacy to simply may reduce your privacy ? Isn't that what we are really trying to convey here?

@PastaPastaPasta
Copy link
Member Author

Thanks for making this change, but where did you guys learn english? Can we consider may mildly harm your privacy to simply may reduce your privacy ? Isn't that what we are really trying to convey here?

"can harm your privacy" was the previous verbage, I have no problem using "reduce" but I think it's valuable to keep "mildly" present in this case, but would like other opinions

@troydash
Copy link

In my opinion that entire confirmation screen needs to be re-done. It's a wall of text. End users don't need to know how many bytes the transaction is, the fee rate per byte, five times displaying the amount to be sent measured in four different denominations, a verbose explanation of why the fee might be high, and a vague even more verbose warning about too many inputs making the transaction more vulnerable in some nebulous way.

That being said, restricting the feedback only to the warning about the inputs in this PR, I would favor "reduce" without the "mildly" or "significantly" qualifiers. Whether the effect is to be considered mild or significant, may be different depending on the user and their priorities.

@troydash
Copy link

troydash commented Sep 23, 2020

Would it be possible to collapse some of the information into a "Learn more" type link? For example,

"Warning: This transaction will use 10 inputs. Using PrivateSend with 10 or more inputs may reduce your privacy and is not recommended for users who require the highest level of privacy. [Learn more]

Where the Learn More takes you to an expanded text or an additional popup which explains that the more inputs there are the more vulnerable it is to this or that, as well as instruction(s) for how to avoid/fix this situation or other best practices for users who want the highest level of privacy.

"Most users can ignore this warning" -- how does a user know if they are part of most users? Everyone who is already going out of their way to mix and use PS would say that they want a "high" level of privacy, but this warning message is really geared towards people who want even more than an average PS user does.

With the way it is now, there is too much text there for most users, and not enough info for the users who need to maximize their privacy.

@qwizzie
Copy link

qwizzie commented Sep 23, 2020

I would also like a 'learn more' link that provide users with clear instructions on how to proceed further, now that they are in the situation that their input amounts actually exceed 10 inputs. When should users act on this alert and when should users ignore this alert ? If they are suppose to act on it, then what exactly should they do ? Should they bundle input amounts through coin control ? Do something else ? That all remains unclear to me. Within the help section of the wallet we also have PrivateSend information that can be viewed, maybe the 'learn more' link can point to additional information there ? The alert and additional information should also be added to our Dash Docs. At this point i think troydash recommended alert warning text (including the 'learn more' link) is easier to read and understand, then the proposed Less Scary Warning text.

@strophy
Copy link
Collaborator

strophy commented Sep 23, 2020

+1 on @kxcd recommendation to use normal English, "mildly harm" is really weird 😅 I think linking to the documentation for more details is a good idea as well, when the docs are forked in the coming week for the 0.16 release I will add a section to the PS docs. You can set a "Learn more" link from the Dash Core warning dialog to here: https://docs.dash.org/en/stable/wallets/dashcore/privatesend-instantsend.html#inputs

Content will appear at that location around the same time as release.

@PastaPastaPasta
Copy link
Member Author

PastaPastaPasta commented Sep 24, 2020

@xdustinface @UdjinM6 Linked to some PS docs could be good (but we'd need to ensure that address is static and available going into the future) However, I'm not sure how I'd integrate a link here, so if either of you could provide some thoughts that'd be cool (as in the code to do a link to learn more)

@xdustinface
Copy link

@troydash Agree, this dialog needs to be re-done! Put it on my list 👍

@PastaPastaPasta see ba00dac which does the following
image

@strophy
Copy link
Collaborator

strophy commented Oct 7, 2020

Added https://docs.dash.org/en/stable/wallets/dashcore/privatesend-instantsend.html#inputs please review the language and instructions.

@qwizzie
Copy link

qwizzie commented Oct 8, 2020

Added https://docs.dash.org/en/stable/wallets/dashcore/privatesend-instantsend.html#inputs please review the language and instructions.

Language

I would change 'increasing the number of inputs in a transactions' to 'increasing the number of inputs in a transaction'
I would change 'denominations more effeciently' to 'denominations more efficiently'

Instructions

'You can reduce the number of inputs in a PrivateSend transaction by defining the transaction amount in such a way that it consumes your available input denominations more effeciently, or by mixing more Dash to ensure enough appropriately denominated inputs are available. Use the Coin Control function in the PrivateSend tab to view and select or deselect mixed UTXOs currently available for use as transaction inputs.'

Feedback

For those users that have slightly more input amounts then the alert warns about, i can see this being helpful.
But for those users that have done a lot of mixing, used a lot of rounds and ended up with a lot of input amounts (input amounts running up in the hundreds), the instruction seems a bit incomplete.

The only way i know how to significantly reduce the number of input amounts for those users, is to select and send a lot of those input amounts to a new receiving address in a 'to yourself' transaction (through PrivateSend). I am not sure how this affects the privacy though, as this seems more of a work around. Also people mentioned they mixed it again after the 'to yourself' transaction, which seems like a good way of ending up in a never ending circle (as mixing creates more input amounts, possibly triggering the warning again).

@strophy
Copy link
Collaborator

strophy commented Oct 8, 2020

Thanks, typos are fixed.

I'm not sure if it is still necessary/recommended to PrivateSend "to yourself" - I think some of the changes to how Dash manages denominations in 0.16 mean this might no longer be necessary? In that case this scary message should appear less frequently anyway, and we wouldn't need to document the "send to yourself" method at all. @PastaPastaPasta would know more

Signed-off-by: pasta <pasta@dashboost.org>
@PastaPastaPasta PastaPastaPasta modified the milestones: 18, 17.1 Jun 25, 2021
@PastaPastaPasta PastaPastaPasta changed the title Make the privatesend input warning less scary Make the coinjoin input warning less scary Jun 25, 2021
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 merged commit fd96da3 into dashpay:develop Jul 1, 2021
@PastaPastaPasta PastaPastaPasta deleted the adjust-ps-inputs-warning branch September 2, 2021 19:58
gades pushed a commit to cosanta/cosanta-core that referenced this pull request May 8, 2022
* Link to documentation

Signed-off-by: pasta <pasta@dashboost.org>

* Use coinjoin link instead of privatesend
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.

None yet

7 participants