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

Increase network read timeout when sending transactions #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NicolasFlamel1
Copy link

A followup to my previous pull request #15 that I accidentally closed by rebasing my fork.

These changes increase the network read timeout to 5 minutes when sending transactions to avoid timing out too early when exchanging a slate with a device that takes longer than a few seconds to generate a reply.

The 5 minute timeout is now only applied when sending a receive_tx request, and all other request types will still use the existing default read timeout. This will avoid any unnecessary waiting since a check_version request always comes before a receive_tx request.

@vekamo
Copy link

vekamo commented Jan 6, 2022

Looks great 👍
I'm wondering tho if that wouldn't be better to implement a field in check_version that indicate whether or not the device could take longers to give the slate response.
Such as low_hardware: true which would mean the sender can await a response within 5, 10 min etc.
Will think about this.

@bayk
Copy link

bayk commented Sep 1, 2024

I am testing now send requests works. From what I see, timeout is need to address tor possible connection issue. The timeout value should depend on the network speed, that mostly depend on what platform tor is running. In the Windows seems it is slowest. Plus how long the network was inactive. There is 'check_tor_connection' command that ping the tor. I think we better to run it in the background when wallet is listening with tor.
Another problem that wallet might be busy and be locked during the request. That will add to waiting time as well.
I think I will implement the background ping and increase the timeout to 180 seconds, 5 minutes is too much. That should be good enough.
P.S. Also the http client is different now, there are conflicts, so I will add changes from the scratch.

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