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.

Sign up for GitHub

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

Jump to bottom

Mints: New melt flow #622

Merged
merged 39 commits into from
Sep 24, 2024
Merged

Mints: New melt flow #622

merged 39 commits into from
Sep 24, 2024

Conversation

callebtc
Copy link
Collaborator

No description provided.

lollerfirst and others added 26 commits September 11, 2024 15:34
@lollerfirst
PaymentResult
38e7e22
@lollerfirst
ledger: rely on PaymentResult instead of paid flag. Double check for …
26eb30c 
…payments marked pending.
@lollerfirst
Merge remote-tracking branch 'origin/main' into payment-result
0cba761
@lollerfirst
None is PENDING
012178c
@lollerfirst
make format
5b818b8
@lollerfirst
reflected changes API tests where PaymentStatus is used + reflected…
22e8be4 
… changes in lnbits
@lollerfirst
reflect changes in blink backend and tests
82ff6c5
@lollerfirst
fix lnbits get_payment_status
0aa3c97
@callebtc
remove paid flag
12b4185
@callebtc
fix mypy
e1f1590
@callebtc
remove more paid flags
2695929
@callebtc
fix strike mypy
904eb6d
@callebtc
green
6faa0a1
@callebtc
shorten all state checks
fbba4c8
@callebtc
fix
4bf0ca2
@callebtc
fix some tests
e3d5706
@callebtc
gimme ✅
19bb7bf
@callebtc
fix............
55c3fc9
@callebtc
fix lnbits
71e680c
@callebtc
fix error
ebb18d0
@callebtc
lightning refactor
31791f8
@callebtc
add more regtest tests
b9e456f
@callebtc
add tests for pending state and failure
a6b788e
@callebtc
shorten checks
7edb6fd
@callebtc
use match case for startup check - and remember modified checking_id …
3d75d22 
…from pay_invoice
@callebtc
fix strike pending return
2634c41
@codecov Codecov
Copy link

codecov bot commented Sep 21, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 65.64706% with 146 lines in your changes missing coverage. Please review.

Project coverage is 63.83%. Comparing base (8675745) to head (6f53664).
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
cashu/lightning/strike.py 61.33% 29 Missing ⚠️
cashu/mint/ledger.py 60.27% 29 Missing ⚠️
cashu/wallet/lightning/lightning.py 22.22% 14 Missing ⚠️
cashu/lightning/lnbits.py 47.82% 12 Missing ⚠️
cashu/lightning/lndrest.py 50.00% 11 Missing ⚠️
cashu/wallet/cli/cli.py 0.00% 10 Missing ⚠️
cashu/lightning/lnd_grpc/lnd_grpc.py 57.89% 8 Missing ⚠️
cashu/lightning/blink.py 80.64% 6 Missing ⚠️
cashu/lightning/base.py 86.48% 5 Missing ⚠️
cashu/lightning/corelightningrest.py 82.60% 4 Missing ⚠️
... and 7 more
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #622      +/-   ##
==========================================
- Coverage   69.02%   63.83%   -5.19%     
==========================================
  Files          92       98       +6     
  Lines        8674    10555    +1881     
==========================================
+ Hits         5987     6738     +751     
- Misses       2687     3817    +1130
Flag Coverage Δ
63.83% <65.64%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

lollerfirst
lollerfirst reviewed Sep 24, 2024
View reviewed changes
cashu/mint/ledger.py
Comment on lines +954 to +958
case _:
logger.error(
f"Payment state is {status.result} and payment was {payment.result}. Proofs for melt quote {melt_quote.quote} are stuck as PENDING."
)
return PostMeltQuoteResponse.from_melt_quote(melt_quote)
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this is accounting for case SETTLED and case PENDING, even though you probably only thought of PENDING. (what if a backend returns UNKNOWN but then returns SETTLED at the second time of asking?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The second check is only for making sure the payment in the first step actually FAILED | UNKNOWN. If the first FAILED | UNKNOWN but the second one is not FAILED | UNKNOWN, the transaction should remain stuck and thus the melt quote should remain pending.

@callebtc callebtc merged commit d8d3037 into main Sep 24, 2024
18 checks passed
@callebtc callebtc deleted the new-melt-flow branch September 24, 2024 12:55
callebtc added a commit that referenced this pull request Oct 4, 2024
@callebtc
readd paid flag removed in #622 before
acd3a88
callebtc added a commit that referenced this pull request Oct 4, 2024
@callebtc
Fix: ledger requires deprecated paid flags in db + wallet disable b…
7fdca3b 
…ase64 keysets by default (#634)

* wallet: deprecate base64 keysets by default

* readd deprecated paid to melt quotes as well

* readd paid flag removed in #622 before
@callebtc callebtc changed the title WIP: New melt flow Mints: New melt flow Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lollerfirst lollerfirst lollerfirst left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@callebtc @lollerfirst