search

ExchangeUnion / raiden

Raiden Network
https://developer.raiden.network
MIT License
0 stars 0 forks source link

Resolver retry loop #13

Closed offerm closed 5 years ago

offerm commented 5 years ago

Fixes: #

Description

Please, describe what this PR does in detail:

PR review check list

Quality check list that cannot be automatically verified.

offerm commented 5 years ago

@erkarl @sangaman - Still WIP. But if you like to have a look, go for it.

offerm commented 5 years ago

@guys, this is ready as far as I'm concerned.

sangaman commented 5 years ago

Something to think about going forward would be if we can get raiden to resume retries if raiden is stopped and restarted while a resolve request is in the loop. For now this is a good solution to prevent fund loss when xud goes down but raiden stays up.

offerm commented 5 years ago

Something to think about going forward would be if we can get raiden to resume retries if raiden is stopped and restarted while a resolve request is in the loop. For now this is a good solution to prevent fund loss when xud goes down but raiden stays up.

Good point. Actually, Raiden should retry the resolver upon restart. However. this is not working as it should and I think I know why. Will open a separate issue for it. thanks

offerm commented 5 years ago

I disagree that these should be considered as failures.

On Wed, Oct 2, 2019 at 5:28 PM Daniel McNally notifications@github.com wrote:

@sangaman commented on this pull request.

In raiden/network/resolver/client.py https://github.com/ExchangeUnion/raiden/pull/13#discussion_r330582357:

  • if secret_request_event.expiration < current_state.block_number:
  • return False
  • response = None
  • try:

  • before calling resolver, update block height

  • request["chain_height"] = chain_state.block_number
  • response = requests.post(raiden.config["resolver_endpoint"], json=request)
  • except requests.exceptions.RequestException:
  • pass
  • if response is not None:
  • if response.status_code == HTTPStatus.OK:
  • break
  • if response.status_code == HTTPStatus.NOT_FOUND:

Oh I was just saying that raiden should treat it as a failure and return False like it does on the next line. I do see it was already merged into raiden's develop branch, let me see if I can propose those changes there.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ExchangeUnion/raiden/pull/13?email_source=notifications&email_token=ACKDI4TKOSVZQKQYOII3SV3QMSVYTA5CNFSM4I3DRFR2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCGUOOJI#discussion_r330582357, or mute the thread https://github.com/notifications/unsubscribe-auth/ACKDI4WSXRDJ2GCYCU6O6OLQMSVYTANCNFSM4I3DRFRQ .

sangaman commented 5 years ago

I disagree that these should be considered as failures.

I'm not sure what you mean, the previous code treated anything besides 200 as a failure so this would just be sticking to the logic from before the retry loop. What should it be treated as instead? AFAIK all http status codes >=400 typically indicate a failed request.

Currently if this receives a status code besides 200 or 404 it will just continue the loop. I'm going to merge this though since it was already merged into the raiden develop branch, and will open a separate PR.