Channel fee penalty (WIP)

[sourabhmarathe]

Idea is to have an interface between NetworkGraph and the user's personal channel metadata. The user needs to implement a way of converting their metadata into a fee penalty so that NetworkGraph's route-finding algorithm remains tractable.

[codecov]

Codecov Report

Merging #635 into master will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## master #635 +/- ##
=======================================
Coverage 91.26% 91.26% =======================================
Files 35 35 Lines 20929 20929 =======================================
Hits 19101 19101 Misses 1828 1828 

Impacted Files	Coverage Δ
lightning/src/routing/network_graph.rs	91.61% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2087032...72f9cf2. Read the comment docs.

[sourabhmarathe]

@TheBlueMatt@ariard Can you take a look?

[ariard]

Looking more on the high-level interface, we may want to have a wrapper one like ChannelScorer to interact with both NetGraphMsgHandler and get_route. So Metadata approach and calculate_minimum_fee_penalty makes sense.

What we may want to do is the default implementation of our ChannelScorer composing score from a list of sub-components like iterating over Vec<ScorerProducer> and payment_success/failure is one of such implementation of trait ScoreProducer. A client must implement the data feeding part from PaymentSuccess/PaymentFailure reception.

That way a client can benefits from our default ScoreProducer impls and add custom ones easily (ChannelScorer::add_scorer_producer)

[ariard]

It would be great to have example in documentation, like a type Metadata being a struct BasicReliability { uptime: u32, flapiness: u32 }. Just a doc example not a default implementation.

[TheBlueMatt]

I think the biggest goal of a default channel scorer would be to ensure we don't return a just-failed route - currently if we happily return the same (lowest-fee) route over and over again and there isn't an easy way to say "I literally just tried that route and it failed". Doing more fancy stuff is cool, but just starting with a nice trait and something simple to address this failure mode would be a massive win.

[sourabhmarathe]

Idea is to have an interface between NetworkGraph and the user's personal channel metadata. The user needs to implement a way of converting their metadata into a fee penalty so that NetworkGraph's route-finding algorithm remains tractable.

[codecov]

Codecov Report

Merging #635 into master will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## master #635 +/- ##
=======================================
Coverage 91.26% 91.26% =======================================
Files 35 35 Lines 20929 20929 =======================================
Hits 19101 19101 Misses 1828 1828 

Impacted Files	Coverage Δ
lightning/src/routing/network_graph.rs	91.61% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2087032...72f9cf2. Read the comment docs.

[sourabhmarathe]

@TheBlueMatt@ariard Can you take a look?

[ariard]

Looking more on the high-level interface, we may want to have a wrapper one like ChannelScorer to interact with both NetGraphMsgHandler and get_route. So Metadata approach and calculate_minimum_fee_penalty makes sense.

What we may want to do is the default implementation of our ChannelScorer composing score from a list of sub-components like iterating over Vec<ScorerProducer> and payment_success/failure is one of such implementation of trait ScoreProducer. A client must implement the data feeding part from PaymentSuccess/PaymentFailure reception.

That way a client can benefits from our default ScoreProducer impls and add custom ones easily (ChannelScorer::add_scorer_producer)

[ariard]

It would be great to have example in documentation, like a type Metadata being a struct BasicReliability { uptime: u32, flapiness: u32 }. Just a doc example not a default implementation.

[TheBlueMatt]

I think the biggest goal of a default channel scorer would be to ensure we don't return a just-failed route - currently if we happily return the same (lowest-fee) route over and over again and there isn't an easy way to say "I literally just tried that route and it failed". Doing more fancy stuff is cool, but just starting with a nice trait and something simple to address this failure mode would be a massive win.

[sourabhmarathe]

Idea is to have an interface between NetworkGraph and the user's personal channel metadata. The user needs to implement a way of converting their metadata into a fee penalty so that NetworkGraph's route-finding algorithm remains tractable.

[codecov]

Codecov Report

Merging #635 into master will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## master #635 +/- ##
=======================================
Coverage 91.26% 91.26% =======================================
Files 35 35 Lines 20929 20929 =======================================
Hits 19101 19101 Misses 1828 1828 

Impacted Files	Coverage Δ
lightning/src/routing/network_graph.rs	91.61% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2087032...72f9cf2. Read the comment docs.

[sourabhmarathe]

@TheBlueMatt@ariard Can you take a look?

[ariard]

Looking more on the high-level interface, we may want to have a wrapper one like ChannelScorer to interact with both NetGraphMsgHandler and get_route. So Metadata approach and calculate_minimum_fee_penalty makes sense.

What we may want to do is the default implementation of our ChannelScorer composing score from a list of sub-components like iterating over Vec<ScorerProducer> and payment_success/failure is one of such implementation of trait ScoreProducer. A client must implement the data feeding part from PaymentSuccess/PaymentFailure reception.

That way a client can benefits from our default ScoreProducer impls and add custom ones easily (ChannelScorer::add_scorer_producer)

[ariard]

It would be great to have example in documentation, like a type Metadata being a struct BasicReliability { uptime: u32, flapiness: u32 }. Just a doc example not a default implementation.

[TheBlueMatt]

I think the biggest goal of a default channel scorer would be to ensure we don't return a just-failed route - currently if we happily return the same (lowest-fee) route over and over again and there isn't an easy way to say "I literally just tried that route and it failed". Doing more fancy stuff is cool, but just starting with a nice trait and something simple to address this failure mode would be a massive win.

[sourabhmarathe]

Idea is to have an interface between NetworkGraph and the user's personal channel metadata. The user needs to implement a way of converting their metadata into a fee penalty so that NetworkGraph's route-finding algorithm remains tractable.

[codecov]

Codecov Report

Merging #635 into master will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## master #635 +/- ##
=======================================
Coverage 91.26% 91.26% =======================================
Files 35 35 Lines 20929 20929 =======================================
Hits 19101 19101 Misses 1828 1828 

Impacted Files	Coverage Δ
lightning/src/routing/network_graph.rs	91.61% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2087032...72f9cf2. Read the comment docs.

[sourabhmarathe]

@TheBlueMatt@ariard Can you take a look?

[ariard]

Looking more on the high-level interface, we may want to have a wrapper one like ChannelScorer to interact with both NetGraphMsgHandler and get_route. So Metadata approach and calculate_minimum_fee_penalty makes sense.

What we may want to do is the default implementation of our ChannelScorer composing score from a list of sub-components like iterating over Vec<ScorerProducer> and payment_success/failure is one of such implementation of trait ScoreProducer. A client must implement the data feeding part from PaymentSuccess/PaymentFailure reception.

That way a client can benefits from our default ScoreProducer impls and add custom ones easily (ChannelScorer::add_scorer_producer)

[ariard]

It would be great to have example in documentation, like a type Metadata being a struct BasicReliability { uptime: u32, flapiness: u32 }. Just a doc example not a default implementation.

[TheBlueMatt]

I think the biggest goal of a default channel scorer would be to ensure we don't return a just-failed route - currently if we happily return the same (lowest-fee) route over and over again and there isn't an easy way to say "I literally just tried that route and it failed". Doing more fancy stuff is cool, but just starting with a nice trait and something simple to address this failure mode would be a massive win.

[sourabhmarathe]

Idea is to have an interface between NetworkGraph and the user's personal channel metadata. The user needs to implement a way of converting their metadata into a fee penalty so that NetworkGraph's route-finding algorithm remains tractable.

[codecov]

Codecov Report

Merging #635 into master will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## master #635 +/- ##
=======================================
Coverage 91.26% 91.26% =======================================
Files 35 35 Lines 20929 20929 =======================================
Hits 19101 19101 Misses 1828 1828 

Impacted Files	Coverage Δ
lightning/src/routing/network_graph.rs	91.61% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2087032...72f9cf2. Read the comment docs.

[sourabhmarathe]

@TheBlueMatt@ariard Can you take a look?

[ariard]

Looking more on the high-level interface, we may want to have a wrapper one like ChannelScorer to interact with both NetGraphMsgHandler and get_route. So Metadata approach and calculate_minimum_fee_penalty makes sense.

What we may want to do is the default implementation of our ChannelScorer composing score from a list of sub-components like iterating over Vec<ScorerProducer> and payment_success/failure is one of such implementation of trait ScoreProducer. A client must implement the data feeding part from PaymentSuccess/PaymentFailure reception.

That way a client can benefits from our default ScoreProducer impls and add custom ones easily (ChannelScorer::add_scorer_producer)

[ariard]

It would be great to have example in documentation, like a type Metadata being a struct BasicReliability { uptime: u32, flapiness: u32 }. Just a doc example not a default implementation.

[TheBlueMatt]

I think the biggest goal of a default channel scorer would be to ensure we don't return a just-failed route - currently if we happily return the same (lowest-fee) route over and over again and there isn't an easy way to say "I literally just tried that route and it failed". Doing more fancy stuff is cool, but just starting with a nice trait and something simple to address this failure mode would be a massive win.

[sourabhmarathe]

Idea is to have an interface between NetworkGraph and the user's personal channel metadata. The user needs to implement a way of converting their metadata into a fee penalty so that NetworkGraph's route-finding algorithm remains tractable.

[codecov]

Codecov Report

Merging #635 into master will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## master #635 +/- ##
=======================================
Coverage 91.26% 91.26% =======================================
Files 35 35 Lines 20929 20929 =======================================
Hits 19101 19101 Misses 1828 1828 

Impacted Files	Coverage Δ
lightning/src/routing/network_graph.rs	91.61% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2087032...72f9cf2. Read the comment docs.

[sourabhmarathe]

@TheBlueMatt@ariard Can you take a look?

[ariard]

Looking more on the high-level interface, we may want to have a wrapper one like ChannelScorer to interact with both NetGraphMsgHandler and get_route. So Metadata approach and calculate_minimum_fee_penalty makes sense.

What we may want to do is the default implementation of our ChannelScorer composing score from a list of sub-components like iterating over Vec<ScorerProducer> and payment_success/failure is one of such implementation of trait ScoreProducer. A client must implement the data feeding part from PaymentSuccess/PaymentFailure reception.

That way a client can benefits from our default ScoreProducer impls and add custom ones easily (ChannelScorer::add_scorer_producer)

[ariard]

It would be great to have example in documentation, like a type Metadata being a struct BasicReliability { uptime: u32, flapiness: u32 }. Just a doc example not a default implementation.

[TheBlueMatt]

I think the biggest goal of a default channel scorer would be to ensure we don't return a just-failed route - currently if we happily return the same (lowest-fee) route over and over again and there isn't an easy way to say "I literally just tried that route and it failed". Doing more fancy stuff is cool, but just starting with a nice trait and something simple to address this failure mode would be a massive win.

[sourabhmarathe]

Idea is to have an interface between NetworkGraph and the user's personal channel metadata. The user needs to implement a way of converting their metadata into a fee penalty so that NetworkGraph's route-finding algorithm remains tractable.

[codecov]

Codecov Report

Merging #635 into master will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## master #635 +/- ##
=======================================
Coverage 91.26% 91.26% =======================================
Files 35 35 Lines 20929 20929 =======================================
Hits 19101 19101 Misses 1828 1828 

Impacted Files	Coverage Δ
lightning/src/routing/network_graph.rs	91.61% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2087032...72f9cf2. Read the comment docs.

[sourabhmarathe]

@TheBlueMatt@ariard Can you take a look?

[ariard]

Looking more on the high-level interface, we may want to have a wrapper one like ChannelScorer to interact with both NetGraphMsgHandler and get_route. So Metadata approach and calculate_minimum_fee_penalty makes sense.

What we may want to do is the default implementation of our ChannelScorer composing score from a list of sub-components like iterating over Vec<ScorerProducer> and payment_success/failure is one of such implementation of trait ScoreProducer. A client must implement the data feeding part from PaymentSuccess/PaymentFailure reception.

That way a client can benefits from our default ScoreProducer impls and add custom ones easily (ChannelScorer::add_scorer_producer)

[ariard]

It would be great to have example in documentation, like a type Metadata being a struct BasicReliability { uptime: u32, flapiness: u32 }. Just a doc example not a default implementation.

[TheBlueMatt]

I think the biggest goal of a default channel scorer would be to ensure we don't return a just-failed route - currently if we happily return the same (lowest-fee) route over and over again and there isn't an easy way to say "I literally just tried that route and it failed". Doing more fancy stuff is cool, but just starting with a nice trait and something simple to address this failure mode would be a massive win.

[sourabhmarathe]

Idea is to have an interface between NetworkGraph and the user's personal channel metadata. The user needs to implement a way of converting their metadata into a fee penalty so that NetworkGraph's route-finding algorithm remains tractable.

[codecov]

Codecov Report

Merging #635 into master will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## master #635 +/- ##
=======================================
Coverage 91.26% 91.26% =======================================
Files 35 35 Lines 20929 20929 =======================================
Hits 19101 19101 Misses 1828 1828 

Impacted Files	Coverage Δ
lightning/src/routing/network_graph.rs	91.61% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2087032...72f9cf2. Read the comment docs.

[sourabhmarathe]

@TheBlueMatt@ariard Can you take a look?

[ariard]

Looking more on the high-level interface, we may want to have a wrapper one like ChannelScorer to interact with both NetGraphMsgHandler and get_route. So Metadata approach and calculate_minimum_fee_penalty makes sense.

What we may want to do is the default implementation of our ChannelScorer composing score from a list of sub-components like iterating over Vec<ScorerProducer> and payment_success/failure is one of such implementation of trait ScoreProducer. A client must implement the data feeding part from PaymentSuccess/PaymentFailure reception.

That way a client can benefits from our default ScoreProducer impls and add custom ones easily (ChannelScorer::add_scorer_producer)

[ariard]

It would be great to have example in documentation, like a type Metadata being a struct BasicReliability { uptime: u32, flapiness: u32 }. Just a doc example not a default implementation.

[TheBlueMatt]

I think the biggest goal of a default channel scorer would be to ensure we don't return a just-failed route - currently if we happily return the same (lowest-fee) route over and over again and there isn't an easy way to say "I literally just tried that route and it failed". Doing more fancy stuff is cool, but just starting with a nice trait and something simple to address this failure mode would be a massive win.