Uh oh!
There was an error while loading. Please reload this page.
Swap Vec<&RouteHop> parameters for slices - #1728
Merged
jkczyz merged 2 commits intoSep 19, 2022
Merged
Conversation
Codecov ReportBase: 90.73% // Head: 90.71% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@## main #1728 +/- ##
==========================================
- Coverage 90.73% 90.71% -0.02%
==========================================
Files 86 86 Lines 46675 46679 +4 Branches 46675 46679 +4 ==========================================
- Hits 42350 42345 -5 - Misses 4325 4334 +9
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
jkczyz
previously approved these changes
Sep 19, 2022
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
tnull
self-requested a review
September 19, 2022 06:56
In c353c3e, new scorer-updating methods were added to the `Router` object, however they were passed as a `Vec` of references. We use the list-of-references pattern to make bindings simpler (by not requiring allocations per entry), however there's no reason prefer to passing a `Vec` over a slice, given the `Vec` doesn't hold ownership of the objects anyway.
In c353c3e an accessor method was added which returns an `Option<&u64>`. While this allows Rust to return an 8-byte object, returning a reference to something pointer-sized is a somewhat strange API. Instead, we opt for a straight `Option<u64>`, which is sadly somewhat larger on the stack, but is simpler and already supported in the bindings generation.
TheBlueMattforce-pushed
the
2022-09-slices-not-vecs
branch
from
September 19, 2022 09:23
6d6b9cc to
85eb1fdCompareTheBlueMatt
commented
Sep 19, 2022
CollaboratorAuthor
Dropped the excess ampersands and went ahead and squashed since its super trivial: $ git diff-tree -U1 6d6b9cc4 85eb1fde
diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs
index 5af6cb766..3b4fe7092 100644
--- a/lightning-invoice/src/payment.rs+++ b/lightning-invoice/src/payment.rs@@ -1847,3 +1847,3 @@ mod tests {
fn notify_payment_path_failed(&self, path: &[&RouteHop], short_channel_id: u64) {
- self.scorer.lock().payment_path_failed(&path, short_channel_id);+ self.scorer.lock().payment_path_failed(path, short_channel_id);
}
@@ -1851,3 +1851,3 @@ mod tests {
fn notify_payment_path_successful(&self, path: &[&RouteHop]) {
- self.scorer.lock().payment_path_successful(&path);+ self.scorer.lock().payment_path_successful(path);
}
@@ -1855,3 +1855,3 @@ mod tests {
fn notify_payment_probe_successful(&self, path: &[&RouteHop]) {
- self.scorer.lock().probe_successful(&path);+ self.scorer.lock().probe_successful(path);
}
@@ -1859,3 +1859,3 @@ mod tests {
fn notify_payment_probe_failed(&self, path: &[&RouteHop], short_channel_id: u64) {
- self.scorer.lock().probe_failed(&path, short_channel_id);+ self.scorer.lock().probe_failed(path, short_channel_id);
}
diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs
index 8c96edbb0..5faecbfab 100644
--- a/lightning-invoice/src/utils.rs+++ b/lightning-invoice/src/utils.rs@@ -486,3 +486,3 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref> Router for DefaultR
fn notify_payment_path_failed(&self, path: &[&RouteHop], short_channel_id: u64) {
- self.scorer.lock().payment_path_failed(&path, short_channel_id);+ self.scorer.lock().payment_path_failed(path, short_channel_id);
}
@@ -490,3 +490,3 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref> Router for DefaultR
fn notify_payment_path_successful(&self, path: &[&RouteHop]) {
- self.scorer.lock().payment_path_successful(&path);+ self.scorer.lock().payment_path_successful(path);
}
@@ -494,3 +494,3 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref> Router for DefaultR
fn notify_payment_probe_successful(&self, path: &[&RouteHop]) {
- self.scorer.lock().probe_successful(&path);+ self.scorer.lock().probe_successful(path);
}
@@ -498,3 +498,3 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref> Router for DefaultR
fn notify_payment_probe_failed(&self, path: &[&RouteHop], short_channel_id: u64) {
- self.scorer.lock().probe_failed(&path, short_channel_id);+ self.scorer.lock().probe_failed(path, short_channel_id);
}
|
tnull
approved these changes
Sep 19, 2022
jkczyz
approved these changes
Sep 19, 2022
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 freeto 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.
In c353c3e, new scorer-updating methods were added to the
Routerobject, however they were passed as aVecof references. We use the list-of-references pattern to make bindings simpler (by not requiring allocations per entry), however there's no reason prefer to passing aVecover a slice, given theVecdoesn't hold ownership of the objects anyway.We also add a second commit which replaces an
Option<&u64>with anOption<u64>.