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

pd: index cometbft block data #3451

Closed
Tracked by #1804
erwanor opened this issue Nov 29, 2023 · 3 comments · Fixed by #3473
Closed
Tracked by #1804

pd: index cometbft block data #3451

erwanor opened this issue Nov 29, 2023 · 3 comments · Fixed by #3473
Assignees

Comments

@erwanor
Copy link
Member

erwanor commented Nov 29, 2023

In Penumbra, chain upgrades are performed via "genesis migrations". The chain state is exported, and node operators produce an upgraded genesis file that specifies the new application hash and starting height for the post-upgrade chain.

Unfortunately, this process requires cometbft to delete all of the block data preceding the upgrade height. This is problematic because Penumbra clients require access to historical block data in order to synchronize their view service, or query transactions.

In particular, Penumbra clients need to:

  • query transactions by block height ("fetch all txs that are present in block $h$")
  • query transactions by tx hash ("fetch the transaction data corresponding to tx_hash")

To solve this, we need to index block data directly into the application, and make the TendermintProxy service route queries to this new local block storage.

In order to do this, we should:

  1. Figure out a storage strategy and schema for the full block data

The general schema of our indices should allow for retrieval by block height (frequent) and retrieval by tx hash (sparser):

  • block_height -> Vec<block_data>
  • tx_hash -> (block_height, position_in_block)

In pseudo-code:

fn get_tx(tx_hash) -> Tx {
    let (height, position) = lookup_index(tx_hash);
    let tx = get_block(height)[position];
    tx
}
  1. Pull transactions handled in ProcessProposal and store them in pd block db
  2. Modify the TendermintProxy service to route get_tx and get_block RPCs to the local block store

This is part of #1804, and blocking for #3408

@github-project-automation github-project-automation bot moved this to 🗄️ Backlog in Penumbra Nov 29, 2023
@erwanor erwanor moved this from 🗄️ Backlog to 📝 Todo in Penumbra Nov 29, 2023
@erwanor erwanor moved this to Testnet 64: Titan in Testnets Nov 29, 2023
@erwanor
Copy link
Member Author

erwanor commented Dec 4, 2023

Re: storage. For now we can simply store the block data in a prefixed substore, this means that we will need to:

  1. Add cometbft_data to the list of substore prefixes in the application crate (rg SUBSTORE_PREFIXES)
  2. When processing ProcessProposal message, peek the block data to get the raw transactions (see ProcessProposal::txs)
  3. Store the transactions by height in nonverifiable storage at key cometbft_data/block/{height:020}
  4. Add a get_transactions_by_height to the TendermintProxy service

@hdevalence
Copy link
Member

Two points:

  1. We shouldn't use the TendermintProxy service for this, since we're not proxying the requests to Tendermint/Comet. Stepping back a bit and questioning the requirement: what Penumbra clients need is a way to get the set of transactions in the block at some height, so that they can scan them locally. The only reason they're using the TendermintProxy RPC is for convenience -- there was already a way to get the literal block, so that seemed easier. But given that the block data isn't retained through upgrades, it turns out not to be. Instead of spending effort on intercepting requests etc., we should just define an RPC that has the behavior we want.

  2. I don't think it's necessary to store the transactions individually, i.e., multiple k/v pairs, we can store one big proto blob at each height, using the response message type we define as part of (1).

@erwanor
Copy link
Member Author

erwanor commented Dec 4, 2023

Re 2: agreed, that's what I was suggesting

@zbuc zbuc self-assigned this Dec 5, 2023
@zbuc zbuc moved this from 📝 Todo to 🏗 In progress in Penumbra Dec 5, 2023
@github-project-automation github-project-automation bot moved this from Testnet 64: Titan to Testnet 63: Rhea (Web Wallet) in Testnets Dec 5, 2023
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Penumbra Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Status: Testnet 63: Rhea (Web Wallet)
Development

Successfully merging a pull request may close this issue.

3 participants