-
Notifications
You must be signed in to change notification settings - Fork 8
don't check zap request pubkey #444
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
Conversation
dangeross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we going to run into issues if we send a zap receipt event signed by the wrong nostr pubkey (the one in the LNURL info response)?
@dangeross I'm not sure. I guess that depends on the client, I haven't gone too deep into that. But you're right it's not very nice. What I could do is persist the nostr pubkey with the payment hash on the server side. Then the server side can handle only invoices where the nostr pubkey is its own. The client will sync the nostr pubkey along with the other metadata. It will decide to publish the event if the pubkey is its own as well. How does that sound? |
Sounds like it could work. I was wondering if it's enough to persist the nostr pubkey with the payment hash, then the server validates the persisted pubkey for the zap is not it's own before going ahead and publishing the event. I guess you don't even have to persist the pubkey, only a bool if it's the server pubkey or not |
Yes sounds like that would work. I'll cook that up tomorrow. |
| lnurl_models::nostr::create_zap_receipt( | ||
| &zap.zap_request, | ||
| invoice, | ||
| preimage, | ||
| signing_keys, | ||
| ) | ||
| .map_err(|e| { | ||
| error!("failed to recreate zap receipt: {}", e); | ||
| ( | ||
| StatusCode::INTERNAL_SERVER_ERROR, | ||
| Json(json!({"error": "internal server error"})), | ||
| ) | ||
| })? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the server creates/signs the zap receipt instead of accepting the client one, does the client's zap receipt get updated? I'm assuming at the next sync it pulls it from the server, but just checking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a good point. I decided to return the used zap receipt from the server, and use that for storage in the client.
21f2aa2 to
53dbff9
Compare
danielgranhao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just some nits
dangeross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
53dbff9 to
63eb657
Compare
The pubkey from the lnurl response is actually not in the zap request:
Example request:
Our nostr pubkey:
0af73fd4383afc8f3001b1c650aac01f111fec7bed027fb415181ba66cdec53eSo just create a zap receipt on the client side when it's not there yet.