-
-
Notifications
You must be signed in to change notification settings - Fork 11
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 electrum support #19
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19 +/- ##
======================================
Coverage 0.0% 0.0%
======================================
Files 9 11 +2
Lines 932 1198 +266
======================================
- Misses 932 1198 +266
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Great work. However, I dislike the idea of adding rust-bitcoin to the dependency list so much that I even did a refactoring of the electrum client to use BP libs instead of it: https://github.com/BP-WG/bp-electrum-client rust-bitcoin takes so long to compile and breaks so much with each release... I know it's behind the feature gate, but in reality, we use Can you please update the dependency to use refactored electrum-client? It's API have not changed and will should work once you get rid of all (now unneeded) conversions to rust-bitcoin structures. |
Refactored to use I set the PR as draft as IMO it makes sense to wait for a release of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, will do a more thorough review later. Since you confirm that the refactored electrum client works and does everything it is expected I will release it now
bp-electrum-client v0.11.0-beta.5 released |
updated to use the released |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 35249ab
This PR adds support for Electrum as indexer.
bp-wallet
has been modified so:electrum
feature is available, adding required dependencieselectrum
indexer has been addedany
indexer and error have been addedbp-util
has been modified so:--electrum <URL>
option can be specified--electrum
and--esplora
can be specified-e
version for esplora has been dropped