Skip to content

bdk_electrum: cache merkle proofs #56

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

Open
notmandatory opened this issue Nov 14, 2024 · 2 comments
Open

bdk_electrum: cache merkle proofs #56

notmandatory opened this issue Nov 14, 2024 · 2 comments
Assignees
Labels
audit Suggested as result of external code audit

Comments

@notmandatory
Copy link
Member

notmandatory commented Nov 14, 2024

"You cache transactions but not their anchor's validity, which significantly reduces the gains from caching as you need to make a request for Merkle proofs anyways."

@notmandatory notmandatory converted this from a draft issue Nov 14, 2024
@notmandatory notmandatory moved this to Discussion in BDK Wallet Nov 14, 2024
@notmandatory notmandatory added the audit Suggested as result of external code audit label Nov 14, 2024
@LagginTimes
Copy link
Contributor

I'll probably take a stab at this as well and throw it onto bitcoindevkit/bdk#1675.

@LagginTimes
Copy link
Contributor

LagginTimes commented Nov 16, 2024

I'll probably make a separate PR for this. The previous bdk_electrum rework that introduced validate_merkle_for_anchor() is reliant on transaction_get_merkle() to update our Header information in case of a reorg. Caching merkle proofs would mean that validate_merkle_for_anchor() will no longer be aware of reorgs and may insert anchors for transactions that have been reorged out.

After discussion with Evan, our idea is that we should instead:
1.) Store the Header in CheckPoint (and use this as the cache). This way we can keep our Headers consistent throughout sync/scan instead of updating Headers inside validate_merkle_for_anchor().
2.) If we pull a fresh merkle proof and the information does not match up with its Header, we simply do not create an anchor and let the sync/scan continue.

edouardparis referenced this issue in wizardsardine/liana Jan 27, 2025
c5f5284 upgrade bitcoin to 0.32 and related (Michael Mallan)

Pull request description:

  This is to upgrade bitcoin to 0.32 and related dependencies accordingly.

  Note that the bdk_electrum crate has not been upgraded to the most recent version available as the Electrum syncing there takes much longer due to the fetching and validation of Merkle proofs. There are ongoing changes in relation to that (https://github.com/bitcoindevkit/bdk/issues/1699) and so we can further upgrade this dependency once those changes have been completed.

  Non-BDK changes to the liana and lianad crates are taken from commit 20ee8b1 in draft PR #1228, which this replaces.

  (Once this PR has been tested and approved, we can release a new version of the async_hwi crate and point to that.)

ACKs for top commit:
  edouardparis:
    ACK c5f5284

Tree-SHA512: 3cb1a2060ecc4c6b9631df0eadf5692ad35495b9303edbb80a437aca60ae062a7227f20ddeb5eb722cf57dd80bfa752372a14d26b8f306844237bfdb5f41e51a
@notmandatory notmandatory transferred this issue from bitcoindevkit/bdk Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit Suggested as result of external code audit
Projects
Status: Discussion
Development

No branches or pull requests

2 participants