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

Update bdk_electrum to use electrum's merkle proof API #980

Closed
evanlinjin opened this issue May 17, 2023 · 7 comments · Fixed by #1489
Closed

Update bdk_electrum to use electrum's merkle proof API #980

evanlinjin opened this issue May 17, 2023 · 7 comments · Fixed by #1489
Assignees
Labels
api A breaking API change module-blockchain new feature New feature or request
Milestone

Comments

@evanlinjin
Copy link
Member

evanlinjin commented May 17, 2023

Describe the enhancement

As mentioned by @LLFourn in #976 (comment):

... I think electrum really intends for you to use the merkle proof API. Best solution is to do that and attach confirmation time of each block to LocalChain.

Additional context

@LagginTimes
Copy link
Contributor

After discussion with @evanlinjin, it has been decided that I will attempt to pick this up.

@evanlinjin
Copy link
Member Author

We've started a rough document here of the plan: https://hackmd.io/@bdk/electrum_spv

@tnull
Copy link
Contributor

tnull commented Jun 6, 2024

FYI, since I needed it for LDK's electrum integration, I added a utility to validate merkle proofs to rust-electrum-client a while back: bitcoindevkit/rust-electrum-client#122

Just mentioning it as they could potentially be useful.

@LLFourn
Copy link
Contributor

LLFourn commented Jun 12, 2024

Just chiming in that I think this should be a priority. It will need very careful engineering to make efficient. I think perhaps we should have CheckPoint in the BdkElectrumClient to keep track of re-orgs. This would allow us to cache merkle proofs and block headers (because we know they haven't changed). Despite this I predict the code would actually get simpler. No more weird loop and restarting in case of re-orgs.

To start off with we don't need to actually return the proofs. They can just be used to make our algorithm actually sound and robust.

@nondiremanuel nondiremanuel added this to the 1.0.0-alpha milestone Jun 25, 2024
@nondiremanuel nondiremanuel moved this from Todo to Needs Review in BDK Jun 25, 2024
@nondiremanuel nondiremanuel added the api A breaking API change label Jun 25, 2024
@notmandatory
Copy link
Member

Since this doesn't touch the wallet APIs I moved it to the beta milestone.

@notmandatory notmandatory modified the milestones: 1.0.0-alpha, 1.0.0-beta Jun 25, 2024
@LLFourn
Copy link
Contributor

LLFourn commented Jun 27, 2024

@notmandatory my suggestions on #1489 means it would touch the wallet API. Please take a look.

@notmandatory
Copy link
Member

notmandatory commented Jun 27, 2024

Moving this and #1489 back to the 1.0.0-alpha milestone.

@notmandatory notmandatory modified the milestones: 1.0.0-beta, 1.0.0-alpha Jun 27, 2024
@github-project-automation github-project-automation bot moved this from Needs Review to Done in BDK Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api A breaking API change module-blockchain new feature New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants