-
Notifications
You must be signed in to change notification settings - Fork 18
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
Remove handling of ChannelNewDeposit event #496
Conversation
Handling the event turned out to be a source of issues. Besides, it doesn't provide any advantages over just using the capacity updates from the nodes. This commit removes all internal handling of the deposit event. Fixes raiden-network#494
Codecov Report
@@ Coverage Diff @@
## master #496 +/- ##
==========================================
+ Coverage 90.71% 91.04% +0.32%
==========================================
Files 35 35
Lines 2252 2211 -41
Branches 291 282 -9
==========================================
- Hits 2043 2013 -30
+ Misses 155 149 -6
+ Partials 54 49 -5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, this is more removed code than I expected!
@@ -112,7 +94,7 @@ def test_pfs_rejects_capacity_update_with_wrong_chain_id( | |||
): | |||
setup_channel(pathfinding_service_web3_mock) | |||
|
|||
message = get_updatepfs_message( | |||
message = get_capacity_update_message( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good that you changed that, too! I already though we got all the updatepfs occurrences last time...
tests/pathfinding/test_graphs.py
Outdated
@@ -99,7 +99,7 @@ def test_routing_simple( | |||
view01: ChannelView = token_network_model.G[addresses[0]][addresses[1]]["view"] | |||
view10: ChannelView = token_network_model.G[addresses[1]][addresses[0]]["view"] | |||
|
|||
assert view01.deposit == 100 | |||
# assert view01.deposit == 100 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not remove that line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Classic, I was sure I would remember to remove this once the tests pass :-/
Handling the event turned out to be a source of issues. Besides, it
doesn't provide any advantages over just using the capacity updates from
the nodes.
This commit removes all internal handling of the deposit event.
Fixes #494