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

feat/fix: add new Client builder method with an arbitrary timeout value & add new method for scantxoutset status #365

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

luisschwab
Copy link

This PR fixes #361 by creating a new buider method for Client, namely new_with_custom_timeout(), that adds a new timeout parameter relative to new().

This was done mainly to allow RPCs that take longer than 15 seconds as defined in the rust-jsonrpc crate.

It's up to the user to define a sensible timeout value.

let client = Client::new_with_custom_timeout(
                     "127.0.0.1",
                     Auth::UserPass("satoshi".to_string(), "satoshi".to_string()),
                     500
).unwrap();

@luisschwab luisschwab changed the title fix: add new Client builder method with an arbitrary timeout value feat/fix: add new Client builder method with an arbitrary timeout value Jul 25, 2024
@0xB10C
Copy link
Contributor

0xB10C commented Jul 26, 2024

Concept ACK

In the past I've worked around this by building a custom client (see e.g. #211 (comment)). Having the option to do this directly from rust-bitcoincore-rpc would be nice.

@luisschwab luisschwab mentioned this pull request Jul 27, 2024
@luisschwab luisschwab changed the title feat/fix: add new Client builder method with an arbitrary timeout value feat/fix: add new Client builder method with an arbitrary timeout value & add new method for scantxoutset status Aug 14, 2024
@luisschwab luisschwab changed the title feat/fix: add new Client builder method with an arbitrary timeout value & add new method for scantxoutset status feat/fix: add new Client builder method with an arbitrary timeout value & add new method for scantxoutset status Aug 14, 2024
Copy link
Contributor

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 8e8790f

Copy link
Contributor

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, sorry didn't mean to re-review this. @tcharding can you kick off the workflow so the tests run?

@tcharding
Copy link
Member

CI was all green when I read your post above but I re-ran all tests for you as well.

@apoelstra
Copy link
Member

Yeah, I kicked it a few hours ahead of you :).

@notmandatory
Copy link
Contributor

Thanks guys for kicking off the build, all checks passing.

This is a pretty straight forward change, anything else you think it needs to be ready to merge? This is required for a new feature @luisschwab is working on for BDK to allow a wallet to sync with a pruned bitcoind node.

@tcharding
Copy link
Member

I personally have completely given up on this repo, I don't know if or when @stevenroose will merge and/or release.

@notmandatory
Copy link
Contributor

@tcharding if @stevenroose is up for it maybe can add write access for a BDK dev or two? This crate is one of our upstream dependencies so we'd like to help keep it up-to-date.

@tcharding
Copy link
Member

tcharding commented Sep 3, 2024

Steven doesn't check GitHub notifications very often, you'll have more luck if you reach him on Signal or some other method. I would suggest that you would have more success if you:

  1. Just fork this repo if you really want to use it
  2. Write your own client using types from https://github.com/rust-bitcoin/rust-bitcoind-json-rpc/

(2) may require some effort but would likely be better in the long run IMO, that crate is in progress though, if you give me a list of the JSONRPC methods you guys use I can prioritise them. I'm actively hacking on it.

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.

scan_tx_out_set_blocking() is panicking as if another scan is already in progress
5 participants