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

Reason for payment is now list of fake reasons instead of random string ID to obfuscate fact its from the Bisq #4249

Closed
wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented May 9, 2020

Solves #2869

image


For now, the list of reasons is very limited, and I need to find some more (like hundreds more) reasons. But from the code perspective it's working.

I did some research in the #2869 issue, and I doubt that Venmo is usable.

public class ReasonsForPayment {

private static final String[] REASONS = {
"Miss you",
Copy link
Author

Choose a reason for hiding this comment

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

Should we translate this via resources/i18n?

@cd2357
Copy link
Contributor

cd2357 commented May 10, 2020

NACK.

Interesting concept, but I think it adds complexity and might actually make it easier to match such a payment description to Bisq.

The random string used now is used a lot in other places as well (some merchants use it as Order ID, or Purchase ID, to match it with orders in their own payment systems).

Also, it is sometimes a random alpha-numeric string, sometimes only numeric. Which obfuscates it even more.

So I'd say it's good the way it is now.

@ghost
Copy link
Author

ghost commented May 10, 2020

True, I agree with your points. The only thing I like to solve is how to have issues ready for implementation, that were accepted and are actually wanted before anyone picks them up and spend time on them.

I'll open this on Keybase chat as well.

@wiz
Copy link
Member

wiz commented May 10, 2020

NACK, mediators need the trade ID in the payment details field to resolve disputes

@dmos62
Copy link
Contributor

dmos62 commented May 14, 2020

NACK, small dictionary size means that this is private by obscurity (that's bad). If we wanted to increase dictionary size, we would have to use some kind of dynamic generation, but then how to make it resilient to heuristics? Plus what @wiz said about needing trade ID in the payments field. @petrhejna sorry for the bad experience. You happened upon an issue whose solution is easy to implement (most likely), but hard to find. @sqrrm I think there's a concensus that this PR should be closed.

@sqrrm
Copy link
Member

sqrrm commented May 15, 2020

Closing as not accepted.

@sqrrm sqrrm closed this May 15, 2020
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