Skip to content

Conversation

@t-bast
Copy link
Member

@t-bast t-bast commented Jun 8, 2022

This is a PR on #2272
It contains two commits that should be reviewed separately.
Please read the commit messages, they contain more details about each commit.

This first commit doesn't contain any meaningful logic changes, it's mostly refactoring and code clean-up, it should be quite straight-forward even though it contains many line changes.

The second commit focuses on the balance estimation business logic, and I'm curious to get your thoughts. It does two things: add a didReceive (symmetric to didSend) and fix a division by zero. I spent a lot of time trying to figure out if we could do a more meaningful update in the parallel channels case of didSend, it feels like we're not doing enough, but I didn't find a better solution...

t-bast added 2 commits June 8, 2022 10:31
We refactor the balance estimates to be bundled with the graph in router data.
This lets the compiler find for us places where we forgot to update
balances, such as when pruning channels.

The changes to Validation.scala are probably easier to compare against
master than comparing them to the base PR, they make the change more
obvious compared to master.

We almost never use the `a max b` syntax in the codebase, we always use a
more explicit function call, so I updated it in BalanceEstimate for
consistency.

This commit doesn't contain meaningful business logic changes and should
be easy to review.
* Fix division by zero
* Add didReceive
* Add tests
@t-bast t-bast requested a review from thomash-acinq June 8, 2022 08:35
@thomash-acinq thomash-acinq merged commit e35092d into balance-estimate-per-side Jun 8, 2022
@t-bast t-bast deleted the balance-estimate-bast branch June 8, 2022 14:04
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.

3 participants