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

Feature: More granular pay offer #129

Closed
wants to merge 3 commits into from

Conversation

a-mpch
Copy link
Contributor

@a-mpch a-mpch commented Jun 29, 2024

Should close #124

In this Pr:

  • We separate in two methods and also endpoints to get_invoices and to pay_invoices
  • We add an integration test (really hacky) to test getting an invoice and then paying it

Some details

  1. I serialized the invoice using Readable from LDK newest versions and then hex the bytes as decribed in this comment
  2. Another decision was to not use the interface from lnd and use the offer itself. This might be changed. Other implementations use the invoices as the parameter.

I didn't test as much I wanted but I'd like feedback first

@a-mpch a-mpch changed the title Feat/granular pay offer Feature: More granular pay offer Jun 29, 2024
@a-mpch
Copy link
Contributor Author

a-mpch commented Jun 29, 2024

@mrfelton might wanna check this 👀

Copy link

codecov bot commented Jun 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (23fe019) to head (24fe6b4).

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #129   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files           1       1           
  Lines          96      96           
======================================
  Misses         96      96           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mrfelton
Copy link
Contributor

Awesome. Any chance you could create a version that is rebased on top of #128? it would make testing a lot easier.

@a-mpch
Copy link
Contributor Author

a-mpch commented Jul 1, 2024

Awesome. Any chance you could create a version that is rebased on top of #128? it would make testing a lot easier.

I'll try ;)

a-mpch added 3 commits June 30, 2024 20:36
this commit aims to separate responsabillities when paying an offer. Also adds some new states for OfferHandler so we could track request and paying invoices separately
this commit expose both endpoints. We use a hex serialization for invoices. We should move to use a readable from ldk and then hex serialize it in the future
@a-mpch
Copy link
Contributor Author

a-mpch commented Jul 3, 2024

closing, moved to #131

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.

Feature: More granular pay offer endpoints
2 participants