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

Create LocalChain with genesis block hardwired for Network #1079

Closed
Tracked by #74
notmandatory opened this issue Aug 16, 2023 · 5 comments · Fixed by #1178
Closed
Tracked by #74

Create LocalChain with genesis block hardwired for Network #1079

notmandatory opened this issue Aug 16, 2023 · 5 comments · Fixed by #1178
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@notmandatory
Copy link
Member

notmandatory commented Aug 16, 2023

Describe the enhancement

LocalChain will no longer implement Default. Instead, it will require a genesis hash to construct. This ensures that we will always have at least one checkpoint in LocalChain (the genesis block).

ChainOracle::get_chain_tip will no longer need to return an Option as we guarantee that we at least have the genesis block.

pub trait ChainOracle {
    /* other trait methods and types */

    /// Get the best chain's chain tip.
    fn get_chain_tip(&self) -> Result<BlockId, Self::Error>;
}

Implementation details

LocalChain will need to ensure that the genesis checkpoint cannot be replaced while updating. I think it would also make sense to persist the genesis checkpoint, this way recovery from persistence will fail with the wrong genesis hash.

Changes to chain-source crates

Any method that takes in an option of a checkpoint should just take in the checkpoint (no Option).

For the bdk_bitcoind_rpc crate, instead of emitting (u32, Block), we want to emit (CheckPoint, Block). This is because for the first emission (given a start height that is >1), we can no longer rely on the block header to construct the chain update (since we now need to connect to the genesis block!). Additionally, if there is no previous checkpoint (Emitter::last_cp), the first emission's checkpoint should also include the genesis block.

Use case

This fixes #1107 without requiring methods of TxGraph and IndexedTxGraph to take in an Option of BlockId as the static block.

The additional benefit of this is that if the caller accidentally updates LocalChain with dat from the wrong network, the update can never connect (since the genesis block cannot be replaced).

Additional context

Suggested by @evanlinjin in bitcoindevkit/.github#72.

Comment on PR#1116: #1116 (comment)

@notmandatory notmandatory added the enhancement New feature or request label Aug 16, 2023
@notmandatory notmandatory added this to the 1.0.0-alpha.3 milestone Aug 16, 2023
@danielabrozzoni danielabrozzoni self-assigned this Aug 18, 2023
@danielabrozzoni
Copy link
Member

I'm taking a shot at this one :)

@LLFourn
Copy link
Contributor

LLFourn commented Aug 21, 2023

What is the benefit of this and what is the planned API?

@notmandatory
Copy link
Member Author

I think the idea is that you can create a new LocalChain prepopulated with the appropriate genesis BlockId and this would prevent blocks from the wrong chain from being added to it. Is that what you had in mind @evanlinjin ?

@LLFourn
Copy link
Contributor

LLFourn commented Aug 22, 2023

So this would be an optional kind of constructor like LocalChain::new_mainnet?

@evanlinjin
Copy link
Member

The rationale is described here: #1116 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants