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

Add helper for getting claimable balance #2333

Merged
merged 1 commit into from Jun 8, 2023

Conversation

benthecarman
Copy link
Contributor

It is annoying to have to match across all the enums of Balance to just pull out the claimable_amount_satoshis value. This helper makes it easier if you just want to amount.

@codecov-commenter
Copy link

codecov-commenter commented Jun 2, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.03 🎉

Comparison is base (fb140b5) 90.34% compared to head (06c6750) 90.37%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2333      +/-   ##
==========================================
+ Coverage   90.34%   90.37%   +0.03%     
==========================================
  Files         104      104              
  Lines       53392    53782     +390     
  Branches    53392    53782     +390     
==========================================
+ Hits        48237    48607     +370     
- Misses       5155     5175      +20     
Impacted Files Coverage Δ
lightning/src/chain/channelmonitor.rs 94.73% <0.00%> (-0.16%) ⬇️

... and 23 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@TheBlueMatt
Copy link
Collaborator

Hmmmmm, I want to better understand the use-case here - are you intending to use the Balance set to tell the user what their deposit balance is? If so, we probably want to only include one of MaybePreimageClaimableHTLC and MaybeTimeoutClaimableHTLC - if we've forwarded a payment, we're only going to get one of those, but never both (choosing which would mean deciding if you want to count outbound pending payments or not). Further, in that case we should probably include the commitment tx fee on Balance::ClaimableOnChannelClose as its a bit confusing to exclude that.

Lack of a global "what is my total balance, man" method is something we've needed for a while, but I'm not sure ChannelMonitors are the right place for it, rather ChannelManager has a bit more context, specifically in-flight payments that are pending a retry but haven't been retried yet.

@benthecarman
Copy link
Contributor Author

The use case is basically to get rid of this giant match statement here: https://github.com/MutinyWallet/mutiny-node/blob/2c9436bd5eca5916d3c3812034314d97abcda124/mutiny-core/src/nodemanager.rs#L1025

We want to see the amount we are going to get back from force closes

@TheBlueMatt
Copy link
Collaborator

Mmm, then we probably want to 0 out the MaybePreimageClaimableHTLC branch? We'll want to carefully document what this is useful for and what assumptions it makes. If we dont know a preimage probably the counterparty will just claim it with the timeout.

@benthecarman
Copy link
Contributor Author

benthecarman commented Jun 7, 2023

Yeah maybe this is better as a function to the channel manager as something like get_limbo_balance that gets all the balance waiting in force closes.

Edit: doesn't seem possible because we don't have the chain monitor available :(

It is annoying to have to match across all the enums of `Balance` to
just pull out the `claimable_amount_satoshis` value. This helper makes
it easier if you just want to amount.
@benthecarman
Copy link
Contributor Author

Excluded MaybePreimageClaimableHTLC and MaybeTimeoutClaimableHTLC and updated the documentation.

@wpaulino wpaulino merged commit 99985c6 into lightningdevkit:main Jun 8, 2023
14 checks passed
@benthecarman benthecarman deleted the chan-mon-bal-helper branch June 8, 2023 01:08
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.

None yet

4 participants