CoopSettle: blockchain, API and message integration#7264
Merged
istankovic merged 9 commits intoAug 20, 2021
Conversation
ezdac
force-pushed
the
feature/coop-settle-contract
branch
from
d2217a1 to
e98f705
Compare
istankovic
reviewed
Aug 9, 2021
istankovic
reviewed
Aug 9, 2021
istankovic
reviewed
Aug 9, 2021
netcriptus
suggested changes
Aug 9, 2021
ulope
reviewed
Aug 9, 2021
Collaborator
ulope
left a comment
ulope
left a comment
There was a problem hiding this comment.
As requested left some comments.
netcriptus
force-pushed
the
feature/coop-settle-contract
branch
2 times, most recently
from
af2e729 to
79fd518
Compare
istankovic
force-pushed
the
feature/coop-settle-contract
branch
from
79fd518 to
9a925d4
Compare
netcriptus
force-pushed
the
feature/coop-settle-contract
branch
from
72a03a4 to
85b81c1
Compare
netcriptus
approved these changes
Aug 11, 2021
ulope
requested changes
Aug 11, 2021
Collaborator
ulope
left a comment
ulope
left a comment
There was a problem hiding this comment.
Some comments below.
There are a lot of test failures.
Many are due to ActionChannelCoopSettle missing in https://github.com/ezdac/raiden/blob/7acbd02ee045da241d418de056a4f79f188d6f3c/raiden/transfer/node.py#L727
But fixing that leads to
Traceback (most recent call last):
File "/Users/ulo/Dropbox/devel/brainbot/raiden/raiden/tests/utils/detect_failure.py", line 60, in wrapper
result.get()
File "src/gevent/event.py", line 329, in gevent._gevent_cevent.AsyncResult.get
File "src/gevent/event.py", line 359, in gevent._gevent_cevent.AsyncResult.get
File "src/gevent/event.py", line 347, in gevent._gevent_cevent.AsyncResult.get
File "src/gevent/event.py", line 327, in gevent._gevent_cevent.AsyncResult._raise_exception
File "/Users/ulo/Envs/raiden/lib/python3.9/site-packages/gevent/_compat.py", line 65, in reraise
raise value.with_traceback(tb)
File "src/gevent/greenlet.py", line 906, in gevent._gevent_cgreenlet.Greenlet.run
File "/Users/ulo/Dropbox/devel/brainbot/raiden/raiden/tests/integration/long_running/test_settlement.py", line 135, in test_settle_is_automatically_called
RaidenAPI(app1).channel_close(registry_address, token_address, app0.address)
File "/Users/ulo/Dropbox/devel/brainbot/raiden/raiden/api/python.py", line 831, in channel_close
self.channel_batch_close(
File "/Users/ulo/Dropbox/devel/brainbot/raiden/raiden/api/python.py", line 874, in channel_batch_close
non_settled_channels = self._batch_coop_settle(channels_to_close, retry_timeout)
File "/Users/ulo/Dropbox/devel/brainbot/raiden/raiden/api/python.py", line 927, in _batch_coop_settle
for channel_state in set(channels_to_settle) - set(unsuccessful_channels):
TypeError: unhashable type: 'NettingChannelState'
89f9671 to
fa0ae90
Compare
istankovic
force-pushed
the
feature/coop-settle-contract
branch
11 times, most recently
from
85355b0 to
011e9e3
Compare
istankovic
force-pushed
the
feature/coop-settle-contract
branch
from
011e9e3 to
6144c15
Compare
ulope
approved these changes
Aug 20, 2021
| "Waiting on channel", | ||
| node=to_checksum_address(self.raiden.address), | ||
| partner_address=to_checksum_address(self.partner_address), | ||
| condition=condition, |
Collaborator
There was a problem hiding this comment.
I still think this will need some more work, for example optimizing the Conditions __str__ / __repr__ output (or maybe even adding a human readable description to them).
But this doesn't need to happen in this PR.
Before, the ChannelSettled event did not emit the transferred-amounts for both participants of the settle-event from the blockchain. As of raiden-network/raiden-contracts#1126 the amounts are also emitted directly, and therefore do not have to be polled from the blockchain again. This also eliminates the need to pass the `proxy_manager` and the `current_confirmed_head` down to the `blockchainevent_to_state_change`. The event handler only processes blockchain events up to the currently confirmed block, so reorgs are only an issue when querying the blockchain specifically. Additionally, the ChannelSettled event now contains both participant's addresses ( raiden-network/raiden-contracts#1494 ), since otherwise the amounts and lockroots arguments could not be associated with the channel-participants. This is reflected in the decoding of the blockchain event.
The waiting module showed a lot of code duplication that could be
abstracted.
Now there is a custom interface `ChannelStateCondition` where
custom conditions can be implemented, logically combined
("and" and "or") and handed to `ChannelStateWaiter` instances.
The existing `wait_*` functions have been re-implemented using
the new mechanism internally, but observe the same interface as before
for compatibility reasons. The functions could be factored out
at a later point.
The waiting is extensively used in the `raiden.api.python` module
as well as in tests.
This implements the handling of the `ContractSendChannelCoopSettle` that gets delegated to the proxies, which handle the contract call on the blockchain.
In order to determine whether an on-chain coop-settle was successful, we have to listen to ChannelSettled events and differentiate them from the events emitted in the settlement-lifecycle. Similarly, expired Withdraws that are associated with a CoopSettle should declare the whole CoopSettle expired on the state.
In order to communicate a coop-settle with a partner, the coop_settle field has to be encodable and decodable on the message-schema level - this is now implemented.
The off-band coop-settle protocol is an optimization for two partner nodes to agree on the parameters for a channel-settle. Therefore it will be transparently initiated as soon as a user wants to close a channel. If the partner is offline or the withdraws related to the coop-settle expire for whatever reason, the on-chain settlement lifecycle is used.
istankovic
force-pushed
the
feature/coop-settle-contract
branch
from
6144c15 to
55e305d
Compare
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.
Description
Fixes: #1514
Depends on: #7233
Depends on: #7251This PR includes the commits from the current state of #7233 (up until ec30b5f), since it builds on top of it.
If something has to be changed in the aforementioned PR, this also has to be reflected here (i.e. rebased)!
This PR implements the e2e- integration for the API, the messages and the blockchain, so that the full coop-settle lifecycle
can be initiated and completed from a user.
What's still missing in this PR:
waitingmechanism for a feedback on the success of an initiated coop-settle works as expected (see c16440e and e98f705)raiden-contracts: due to Make tests and smoketest work with latest contracts #7203 current/to-be-implemented tests will fail because we don't test against the newest changes in the contracts that include the coop-settle in the first place as well as modifications to the ChannelSettled event