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

feat: move events to transactions service #510

Merged
merged 21 commits into from
Aug 23, 2024

Conversation

rolznz
Copy link
Contributor

@rolznz rolznz commented Aug 19, 2024

Closes #350
Fixes #515
Fixes #516
Fixes #322
Fixes #422

This PR enable notifications for self payments, and keysend self-payments.

ln clients that do not support notifications will not receive events currently because the app connections won't have the permission.

Notable changes

  • standard events are now "payment_sent", "payment_received", "payment_failed" (payment_succeeded is now payment_sent)
  • lnclient events are prefixed with "lnclient_" and only consumed by the transactions service
  • internal keysend payments are now possible
  • NIP-47 notifications are fired for internal payments
  • NIP-47 notifications are fired for lnclients that do not support notifications (using fallback method of checking unsettled transactions) - but they are currently filtered out due to the apps not having permission to fire them.
  • No longer necessary to order event consumers as they are unlinked (NIP-47 notifier now uses transaction service events)
  • New DB column on transactions: failure_reason

TODOs

  • only have one payment_sent and one payment_failed event, but triggered from transaction service. - the lnclient ones should be renamed to be clearly different.
  • NIP-47 notifier should consume these events.
  • internal keysends
  • reduce duplication in self payment interception code
  • Fix offset paging bug
  • Fix payment permission denied event not firing
  • Fix error: payment already marked as sent
  • Unit Tests:
    • paging
    • Send payment should check for existing successful payment before sending, otherwise wrong error code is possible (e.g. budget exceeded)
    • Send nwc permission denied event on payment
    • marking transaction as settled cannot be done twice (to avoid duplicate notification)
      • sent
      • received
    • marking payment as failed cannot be done twice (to avoid duplicate notification)
    • transaction service - firing events
      • payment sent
      • payment received
    • keysend self payments
    • notifications
      • now work for e.g. cashu client (using check unsettled transaction code)
      • internal keysends
  • Manual tests
    • internal keysend payment with boostagram
    • events correctly fired
      • payment_sent
      • payment_received
      • payment_failed
      • alby oauth client - review data
    • nip-47 notifications
      • for other wallet types e.g. cashu/phoenix - ❌ does not work unless we always mark that we support notifications - which might cause issues with apps because then they would not poll
      • keysend self-payment
      • wallet self-payment

@rolznz rolznz marked this pull request as ready for review August 20, 2024 13:09
@rolznz rolznz changed the title feat: move events to transactions service (WIP) feat: move events to transactions service Aug 20, 2024
@rolznz rolznz merged commit 5f1b0de into master Aug 23, 2024
8 checks passed
@rolznz rolznz deleted the feat/transactions-service-events branch August 23, 2024 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant