Skip to content
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 channel_identifier usage to search for channels #1769

Merged
merged 2 commits into from
Jul 10, 2018
Merged

Remove channel_identifier usage to search for channels #1769

merged 2 commits into from
Jul 10, 2018

Conversation

andrevmatos
Copy link
Contributor

This identifier has a nonce on the blockchain, which makes it very hard
to be calculated only from the parameters given in the request.
As there should never exist more than 1 channel open for each tuple
(registry_address, token_address, own_address, partner_address), we can
just use it to search for the channels we want to interact with.
Channel_identifier is still returned in the request, for information,
but its value is ignored in the tests.
Fix #1765 , broken by the changes introduced in raiden-network/raiden-contracts#170

This identifier has a nonce on the blockchain, which makes it very hard
to be calculated only from the parameters given in the request.
As there should never exist more than 1 channel open for each tuple
(registry_address, token_address, own_address, partner_address), we can
just use it to search for the channels we want to interact with.
Channel_identifier is still returned in the request, for information,
but its value is ignored in the tests.
[ci integration]
[skip ci]
@LefterisJP
Copy link
Contributor

The PR looks fine but @andrevmatos requested in a pm to wait a bit since there is some more tests outside the api tests that are related and he wants to address

@LefterisJP LefterisJP merged commit f99468d into raiden-network:master Jul 10, 2018
@LefterisJP
Copy link
Contributor

@andrevmatos Please open another PR to fix the rest of the tests you wanted. I want the tests fixed in master if the fix is available (and since you were so kind to provide it, it now is :) )

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.

2 participants