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

Stealth invoices #210

Merged
merged 3 commits into from
Aug 12, 2022
Merged

Stealth invoices #210

merged 3 commits into from
Aug 12, 2022

Conversation

ShatteredBunny
Copy link
Contributor

Fixes #168

The profile has now a flag whether stealth invoices should be used. The default is off.

When an order is created, a UUID is generated as a reference. This reference should never be public and only visible for the participants of the trade. If stealth invoices are enabled, only the reference is included into the invoice description.

Copy link
Collaborator

@Reckless-Satoshi Reckless-Satoshi left a comment

Choose a reason for hiding this comment

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

This is a superb work! tested and works flawlessly.

I left a few minor comments in case you want to polish the few things I've detected. Most are really not needed, just in case you feel they add to your work.

I am very excited to see you got around so quickly to run the stack and contribute both to back and frontend. Welcome to robodevs and thank you!!

Please submit an invoice in this thread for 120K Sats from a proxy wallet (not your own node_id) and a long expiration time (+2 days).

api/models.py Outdated
@@ -297,6 +297,10 @@ class ExpiryReasons(models.IntegerChoices):
NESINV = 4, "Neither escrow locked or invoice submitted"

# order info
reference = models.CharField(max_length=36,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be cleaner if we use a UUIDField instead of a CharField. It will be filled with a UUID as soon as the object is created (more info on UUIDFields https://www.geeksforgeeks.org/uuidfield-django-models/ )

import uuid
  
reference = models.UUIDField(
         default = uuid.uuid4,
         editable = False)

Would save a few lines also in views.py

api/views.py Outdated
@@ -30,6 +30,7 @@
from django.utils import timezone
from django.conf import settings
from decouple import config
from uuid import uuid4
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not needed if we use a UUIDField

api/views.py Outdated
@@ -141,6 +142,7 @@ def post(self, request):
escrow_duration=escrow_duration,
bond_size=bond_size,
bondless_taker=bondless_taker,
reference=uuid4()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

@@ -740,10 +742,13 @@ def post(self, request, format=None):
user.profile.is_referred = True
user.profile.referred_by = queryset[0]

user.profile.wants_stealth = False
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe good idea to leave here. But I believe it is not needed since user.profile.wants_stealth is already False by default

<Grid item>
<FormControlLabel
labelPlacement="start"
label={`${t("Use stealth invoices")}`}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The translation key "use stealth invoices":"{same but in another language}" is now available. Would be ideal to add a new key to en.json so future translations include it. It can be added at the end of "Bottom Bar", around here:
https://github.com/Reckless-Satoshi/robosats/blob/efd89756e41820cd0a1cc26105d23a7300094f84/frontend/static/locales/en.json#L229-L230

api/views.py Outdated
user.profile.save()

context["public_key"] = user.profile.public_key
context["encrypted_private_key"] = user.profile.encrypted_private_key
context["wants_stealth"] = False
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good, but if we remove the line above it would make sense to change this one to

context["wants_stealth"] = user.profile.wants_stealth

@ShatteredBunny
Copy link
Contributor Author

Thanks for going through it, I didn't knew about uuid fields and missed the localization file
I also added a tooltip explaining what stealth invoices do, may not be obvious

This invoice is only valid one day, I don't know if there is a custodial wallet that lets you set the time... let me know if it was too short
lnbc1200u1p30dz79pp5kh93tv9uq7m5v346kkxtqhc8fqmh2ms7sh6wtn4yndrzayu6pj5qdqu2askcmr9wssx7e3q2dshgmmndp5scqzpgxqyz5vqsp5mwl30w504agyktpkft2ujy3l0m50d6hnwqy8vue4984z0zd322jq9qyyssqsjgxrw0dpr06rp7ev0d6xdlzke060dcyeslcqd083fvz9nz79pf9mejazfnmfnyjk4jthx304k0p84nszjcjq50edt3qhqg273l4wegp4w3g3x

@Reckless-Satoshi
Copy link
Collaborator

Looks great and the Tooltip will indeed be very handy !

lnbc1200u1p30dz79pp5kh93tv9uq7m5v346kkxtqhc8fqmh2ms7sh6wtn4yndrzayu6pj5qdqu2askcmr9wssx7e3q2dshgmmndp5scqzpgxqyz5vqsp5mwl30w504agyktpkft2ujy3l0m50d6hnwqy8vue4984z0zd322jq9qyyssqsjgxrw0dpr06rp7ev0d6xdlzke060dcyeslcqd083fvz9nz79pf9mejazfnmfnyjk4jthx304k0p84nszjcjq50edt3qhqg273l4wegp4w3g3x

1ff79dbae08332ecd811f750b4f00dd007145bbf7ad80274da12981ee57a35b7

Merging!

@Reckless-Satoshi Reckless-Satoshi merged commit eff58dc into RoboSats:main Aug 12, 2022
@ShatteredBunny ShatteredBunny deleted the stealth-invoices branch August 12, 2022 19:22
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.

Add 'stealth invoices': an On/Off toggle for lightning invoice description
2 participants