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

Implements an internal txindex so we don't have to run bitcoind with txindex=1 #132

Merged
merged 3 commits into from
Nov 7, 2022

Conversation

sr-gi
Copy link
Member

@sr-gi sr-gi commented Oct 6, 2022

Generalizes the Watcher's LocatorCache into a TxIndex that can be used both by the Watcher and the Responder. The former does use it in the same way as before (as a LocatorCache to keep track of the Locators of the last n blocks). As for the latter, it is used to implement a pruned transaction index (a Txid:BlockHash map).

By adding this TxIndex in the Responder we don't have to run bitcoind with txindex=1 anymore and, therefore, we are able to reduce the initial storage requirements significantly: from ~420 GB on disk at the time of writing to a few dozen KB in memory.

With this new approach queries to past transactions will be performed internally instead of against bitcoind in most cases. If a transaction cannot be found in our index nor in mempool it'll mean that it was confirmed long ago (confirmation count > our TxIndex length). The Responder index is set to be IRREVOCABLY_RESOLVED blocks long, meaning that if we don't find a transaction on it we don't need to care anyway.

@sr-gi sr-gi changed the title Implements an internal txindex so we can get rid of bitcoind's txindex Implements an internal txindex so don't have to run bitcoind with txindex=1 Oct 6, 2022
@sr-gi sr-gi changed the title Implements an internal txindex so don't have to run bitcoind with txindex=1 Implements an internal txindex so we don't have to run bitcoind with txindex=1 Oct 7, 2022
@sr-gi sr-gi force-pushed the 130-txindex branch 2 times, most recently from fc04b8e to 58ede34 Compare October 14, 2022 14:58
@sr-gi sr-gi added feature request New feature or request Seeking Code Review review me pls labels Oct 14, 2022
@sr-gi sr-gi marked this pull request as ready for review October 14, 2022 15:15
Copy link
Contributor

@orbitalturtle orbitalturtle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question and a couple of small things, but otherwise looks good to me. 🔥

I'll give this PR a test run sometime this week too

}

/// Fixes the index by removing disconnected data.
fn fix(&mut self, header: &BlockHeader) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if this function name could be a bit more descriptive, like "remove_block" or something

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about remove_disconnected_block?

teos/src/tx_index.rs Outdated Show resolved Hide resolved
teos/src/tx_index.rs Outdated Show resolved Hide resolved
teos/src/carrier.rs Show resolved Hide resolved
Copy link
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nits and questions, look good otherwise.

teos/src/tx_index.rs Outdated Show resolved Hide resolved
teos/src/tx_index.rs Outdated Show resolved Hide resolved
teos/src/tx_index.rs Outdated Show resolved Hide resolved
teos/src/tx_index.rs Outdated Show resolved Hide resolved
teos/src/tx_index.rs Outdated Show resolved Hide resolved
teos/src/tx_index.rs Outdated Show resolved Hide resolved
teos/src/tx_index.rs Outdated Show resolved Hide resolved
teos/src/test_utils.rs Outdated Show resolved Hide resolved
teos/src/responder.rs Outdated Show resolved Hide resolved
@sr-gi
Copy link
Member Author

sr-gi commented Nov 2, 2022

@mariocynicys added fixes from your review comments.

@orbitalturtle did you have the chance to check the changes from your suggestions?

@sr-gi
Copy link
Member Author

sr-gi commented Nov 4, 2022

@orbitalturtle @mariocynicys I will rebase the changes and merge this if you're happy with it

@mariocynicys
Copy link
Collaborator

@mariocynicys added fixes from your review comments.

Sorry I missed this. Let me quickly review this.

Copy link
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@orbitalturtle
Copy link
Contributor

@sr-gi I can take a second look this evening!

Copy link
Contributor

@orbitalturtle orbitalturtle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, yay pruned node compatibility! 🔥

teos/src/carrier.rs Show resolved Hide resolved
In order to fix talaia-labs#130 we need to implement our own txindex. Turns out this is almost identical
to our `LocatorCache`, so we can generalize it and use it for both purposes.
@sr-gi
Copy link
Member Author

sr-gi commented Nov 7, 2022

Rebased and squashed

@sr-gi sr-gi added this to the 0.2 milestone Nov 7, 2022
@sr-gi sr-gi merged commit eb40ac0 into talaia-labs:master Nov 7, 2022
@sr-gi sr-gi removed the Seeking Code Review review me pls label Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants