-
Notifications
You must be signed in to change notification settings - Fork 153
Support partial mempool synchronization #168
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
base: new-index
Are you sure you want to change the base?
Conversation
Prior to this change, the local electrs mempool was only updated with new transactions once we were able to get a complete snapshot of every transaction in bitcoind's mempool, which could require multiple rounds of attempts if any transactions are replaced (or otherwise evicted) between querying for the mempool txids and querying for the transactions themselves. PR romanz#89 made each round more efficient, but obtaining a complete snapshot can still potentially take many rounds the more RBF is used, with each round taking longer the larger the mempool gets (just listing the mempool txids to find the delta becomes costly). With this change, the local mempool is instead updated with whatever transactions we are able to get, without waiting for a fully consistent snapshot - which should improve performance and reduce latency. Making partial updates requires some extra care on the electrs side to ensure mempool consistency (i.e., no double-spends or missing input txos), since we can no longer fully rely on bitcoind validating consistency for us. Specifically: - If any transactions were RBF'd and gone by the time we queried for them, we explicitly check for transactions that depend on the missing transactions (or their descendants) and remove them too. - Mempool evictions must be processed first before any additions, even if the replacing transactions aren't available. This ensures double-spend conflicts are not possible.
c7285eb
to
3ff2156
Compare
This could definitely use some tests, but testing the edge-cases here is pretty tricky (in particular, the case where we manage to get the child tx before the parent is RBF'd, but then miss the parent). Still thinking on ways to approach it. |
tack |
let new_txids = bitcoind_txids | ||
.iter() | ||
.filter(|&txid| !indexed_txids.contains(txid)) | ||
.collect::<Vec<_>>(); |
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.
nit: shouldn't this be another difference() ?
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.
utACK 3ff2156
Prior to this change, the local electrs mempool was only updated with new transactions once we were able to get a complete snapshot of every transaction in bitcoind's mempool, which could require multiple rounds of attempts if any transactions are replaced (or otherwise evicted) between querying for the mempool txids and querying for the transactions themselves.
PR #89 made each round more efficient, but obtaining a complete snapshot can still potentially take many rounds the more RBF is used, with each round taking longer the larger the mempool gets (just listing the mempool txids to find the delta becomes costly).
With this change, the local mempool is instead updated with whatever transactions we are able to get, without waiting for a fully consistent snapshot - which should improve performance and reduce latency.
Making partial updates requires some extra care on the electrs side to ensure mempool consistency (i.e., no double-spends or missing input txos), since we can no longer fully rely on bitcoind validating consistency for us. Specifically:
If any mempool transactions became unavailable (replaced) by the time we queried for them, we explicitly check for transactions that depend on the missing transactions (or their descendants) and remove them too.
Mempool evictions are processed first before any additions, even if the replacing transactions aren't available. This ensures double-spend conflicts are not possible. (technically this was already the case, but now it became more critical)