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

Adds Batch client connection and async RPC calls to the bitcoind client #807

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

Conversation

Vib-UX
Copy link

@Vib-UX Vib-UX commented Aug 8, 2022

PR serves in relation to fixing issue 5041

With this PR we enable batch client connection and async RPC calls to the bitcoind client providing testcoverages for the same. This helps us in batching the required requests needed for speeding up operations by reducing round trips.

Speedups:

btcwallet-testCoverageSpeedUp

Pull Request Checklist

Testing

  • Your PR passes all CI checks.
  • Tests covering the positive and negative (error paths) are included.
  • Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

📝 Please see our Contribution Guidelines for further guidance.

Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Thank you for the PR, this will speed up chain rescans considerably!

Great work so far, looks pretty good. My feedback mostly consists of style feedback, the code seems to work as advertised 🎉

chain/bitcoind_client.go Show resolved Hide resolved
chain/bitcoind_client.go Outdated Show resolved Hide resolved
chain/bitcoind_client.go Outdated Show resolved Hide resolved
chain/bitcoind_conn.go Outdated Show resolved Hide resolved
chain/bitcoind_conn.go Outdated Show resolved Hide resolved
chain/bitcoind_client_test.go Show resolved Hide resolved
chain/bitcoind_client_test.go Outdated Show resolved Hide resolved
chain/utils_test.go Outdated Show resolved Hide resolved
chain/bitcoind_client_test.go Outdated Show resolved Hide resolved
chain/bitcoind_client_test.go Outdated Show resolved Hide resolved
@Vib-UX Vib-UX force-pushed the BatchClient branch 2 times, most recently from 759043b to 7f29aa8 Compare August 21, 2022 16:08
@Vib-UX
Copy link
Author

Vib-UX commented Aug 21, 2022

Thank you for the PR, this will speed up chain rescans considerably!

Great work so far, looks pretty good. My feedback mostly consists of style feedback, the code seems to work as advertised 🎉

Thanks for the insightful review @guggero, I have updated the commits accordingly.

@Vib-UX Vib-UX requested a review from guggero August 21, 2022 17:29
@guggero
Copy link
Collaborator

guggero commented Aug 29, 2022

@Vib-UX Just FYI: I'm waiting with my next round of review since some of the changes requested in the lnd PR will probably require some changes here as well.

Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

PR seems to be abandoned, submitting empty review to remove it from my "reviews requested" list.

@Vib-UX
Copy link
Author

Vib-UX commented Nov 17, 2022

PR seems to be abandoned, submitting empty review to remove it from my "reviews requested" list.

Recently solved a few test-coverage issues while implementing generic functions, and will be pushing updates by this weekend, had a chat with @bhandras as well regarding this.

This commit adds a batch client connection and async RPC
calls to the bitcoind client which will enable batching multiple
requests speeding up operations by reducing round trips.
This will leverage the updated batch client to fetch multiple blocks in
one go which can speed up rescan and filter blocks.
Both RescanBlocksBatched() and FilterBlocksBatched() leverages GetBlocksBatch()
which works with the updated batch client for speed ups.
Testcoverage to ensure that GetBlocksBatch() which leverages batchAPI
works as expected with enhanced performance.
Testcoverage to ensure that RescanBlocksBatched() which leverages
updated batchAPI for scanning the chain works as expected with enhanced
performance.
Testcoverage to ensure that FilterBlocksBatched() which leverages
batchAPI for scanning the chain works as expected with enhanced
performance.
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