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

Add --bitcoin-rpcclienttimeout CLI parameter #2817

Closed
cdecker opened this issue Jul 18, 2019 · 4 comments · Fixed by #7095
Closed

Add --bitcoin-rpcclienttimeout CLI parameter #2817

cdecker opened this issue Jul 18, 2019 · 4 comments · Fixed by #7095
Labels

Comments

@cdecker
Copy link
Member

cdecker commented Jul 18, 2019

As discussed in #2780 we should likely expose this option through the command line so that users can adapt it to their needs.

Open questions:

  • @ZmnSCPxj proposed subtracting the --bitcoin-retry-timeout (better yet, just take the maximum of the two), to make sure we actually trigger an error at the expected time, rather than --bitcoin-retry-timeout + --bitcoin-rpcclienttimeout
@cdecker cdecker added enhancement good first issue good for onboarding labels Jul 18, 2019
@BitcoinJiuJitsu
Copy link

@s373nz is keen to have a run at this bounty after discovering on Sphinx, can we assign @cdecker?

@s373nZ
Copy link
Contributor

s373nZ commented Feb 19, 2024

@cdecker @BitcoinJiuJitsu I believe I've got some local code to define and passthrough the argument to bcli.

In terms of the open question, are you saying that --bitcoin-rpcclienttimeout should be modified to max(bitcoin_retry_timeout, bitcoin_rpcclienttimeout)? Based on my reading of #2778, that might still cause a long wait time as per the default values and your comment here suggests a lower value:

#2778 (comment)

From reviewing the conversations in #2780 and #2778, my reading is that we want the min(). Am I misunderstanding?

@s373nZ
Copy link
Contributor

s373nZ commented Feb 20, 2024

@cdecker @BitcoinJiuJitsu I took a first shot at this in #7095. The earlier question was because I initially set the parameter up with a default value of 900 to match bitcoind. I removed that and set the default to 60, where the suggestion to take the max() makes much more sense.

@cdecker
Copy link
Member Author

cdecker commented Feb 21, 2024

Very nice, thanks @s373nZ, I'll review the PR as soon as possible 👍

@cdecker cdecker linked a pull request Feb 21, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants