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

bdk_electrum: cache merkle proofs #1699

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

bdk_electrum: cache merkle proofs #1699

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

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 added this to BDK Nov 14, 2024
@notmandatory notmandatory converted this from a draft issue Nov 14, 2024
@notmandatory notmandatory moved this to Discussion in BDK Nov 14, 2024
@notmandatory notmandatory added audit Suggested as result of external code audit module-blockchain labels Nov 14, 2024
@LagginTimes
Copy link
Contributor

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

@LagginTimes
Copy link
Contributor

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 will 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.

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 module-blockchain
Projects
Status: Discussion
Development

No branches or pull requests

2 participants