Add method to count total fees in a Route #999#1063
Merged
TheBlueMatt merged 1 commit intolightningdevkit:mainfrom Sep 21, 2021
Merged
Add method to count total fees in a Route #999#1063TheBlueMatt merged 1 commit intolightningdevkit:mainfrom
TheBlueMatt merged 1 commit intolightningdevkit:mainfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1063 +/- ##
==========================================
- Coverage 90.82% 90.74% -0.08%
==========================================
Files 65 65
Lines 32890 33948 +1058
==========================================
+ Hits 29872 30806 +934
- Misses 3018 3142 +124
Continue to review full report at Codecov.
|
TheBlueMatt
reviewed
Aug 27, 2021
…devkit#999 * Added `get_total_fees` method to route, to calculate all the fees paid accross each path. * Added `get_total_amount` method to route, to calculate the total of actual amounts paid in each path.
galderz
force-pushed
the
t_total_fee_999
branch
from
03aaa9d to
03bb808
Compare
Contributor
Author
|
Reworked unit tests and added an extra |
TheBlueMatt
approved these changes
Sep 21, 2021
valentinewallace
approved these changes
Sep 21, 2021
Contributor
valentinewallace
left a comment
valentinewallace
left a comment
There was a problem hiding this comment.
Just nits, LGTM
Comment on lines
+76
to
+77
| /// This doesn't include any extra payment made to the recipient, | ||
| /// which can happen in excess of the amount passed to `get_route`'s `final_value_msat`. |
Contributor
There was a problem hiding this comment.
nit: lines should be 100chars each
| .flat_map(|path| path.split_last().unwrap().1) | ||
| .map(|hop| &hop.fee_msat) | ||
| .sum(); | ||
| } |
Contributor
There was a problem hiding this comment.
nit: newline below
| .map(|hop| &hop.fee_msat) | ||
| .sum(); | ||
| } | ||
| /// Returns the total amount paid on this Route, excluding the fees. |
Contributor
There was a problem hiding this comment.
nit: ticks around Route and above
| .sum(); | ||
| } | ||
| /// Returns the total amount paid on this Route, excluding the fees. | ||
| pub fn get_total_amount(&self) -> u64 { |
Contributor
There was a problem hiding this comment.
Idk if it's worth noting for these methods that they'll panic for 0-length routes?
| RouteHop { | ||
| pubkey: PublicKey::from_slice(&hex::decode("02eec7245d6b7d2ccb30380bfbe2a3648cd7a942653f5aa340edcea1f283686619").unwrap()[..]).unwrap(), | ||
| channel_features: ChannelFeatures::empty(), node_features: NodeFeatures::empty(), | ||
| short_channel_id: 0, fee_msat: 100, cltv_expiry_delta: 0 // Test vectors are garbage and not generateble from a RouteHop, we fill in payloads manually |
Contributor
There was a problem hiding this comment.
nit here and below: s/generateble/generatable (i think these comments can all be deleted though 😛)
Collaborator
|
Gonna land this, will handle val's comments in a new PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #999. Add a member function for Route that returns the total amount of fees paid across all paths.
Further work will be carried out following @valentinewallace's work in #1053, so that the same can be calculated post payment. This implies taking into account which payments were really confirmed and discard what failed or was retried.