feat(chain): add get and range methods to CheckPoint

[evanlinjin]

Partially fixes #1354

Description

These methods allow us to query for checkpoints contained within the linked list by height and height range. This is useful to determine checkpoints to fetch for chain sources without having to refer back to the LocalChain.

Currently this is not implemented efficiently, but in the future, we will change CheckPoint to use a skip list structure.

Notes to the reviewers

Please refer to the conversation in #1354 for more details. In summary, both TxGraph::missing_heights and tx_graph::ChangeSet::missing_heights_from are problematic in their own ways. Additionally, it's a better API for chain sources if we only need to lock our receiving structures twice (once to obtain initial state and once for applying the update). Instead of relying on methods such as EsploraExt::update_local_chain to get relevant checkpoints, we can use these query methods instead. This allows up to get rid of ::missing_heights-esc methods and remove the need to lock receiving structures in the middle of the sync.

Changelog notice

• Added get and range methods to CheckPoint (and in turn, LocalChain). This simulates an API where we have implemented a skip list of checkpoints (to implement in the future). This is a better API because we can query for any height or height range with just a checkpoint tip instead of relying on a separate checkpoint index (which needs to live in LocalChain).
• Changed LocalChain to have a faster Eq implementation. We now maintain an xor value of all checkpoint block hashes. We compare this xor value to determine whether two chains are equal.
• Added PartialEq implementation for CheckPoint and local_chain::Update.

Checklists

All Submissions:

• I've signed all my commits
• I followed the contribution guidelines
• I ran cargo fmt and cargo clippy before committing

New Features:

• I've added tests for the new feature
• I've added docs for the new feature

[LLFourn]

I'm slightly concerned we're introducing these methods without using them anywhere. Especially query_multi which I don't think needs to exist yet. I suggest cutting query_multi.

[LLFourn]

ACK aa84ecf

[notmandatory]

ACK aa84ecf

Appreciate the focused scope for this series of chain PRs, much easier to review. I have one small test code suggestion but otherwise looks good to me.

[evanlinjin]

Ahhh I probably still need to add a few tests.

[ValuedMammal]

Overall looks good.

get sounds like a better name than query. My other suggestion would have been to call it find, since that is how it's implemented.

Interesting that LocalChain now delegates virtually everything to the checkpoint. Curious to see if the structure of LocalChain becomes more relevant when skip list is involved.

[evanlinjin]

@LLFourn CheckPoint::range is also used here.

[LLFourn]

Thanks. I missed that one and had forgot about the API of this function. On a side note this is interesting because we are anchoring a tx not to the block that it's in. So even if we made a rule that esplora/electrum etc must tell us about the block that the tx is in we'd still need the concept of anchoring if we want to keep this API.

[LLFourn]

ACK cdbc131

[evanlinjin]

I've addressed all review comments and removed the From<LocalChain> impl for BTreeMap<u32, BlockHash> as it wasn't being used.

[jirijakes]

The PR helped me better understand some internals. Still learning.

Only have one question and one minor suggestion (not too important).

ACK 6d78f0e

[jirijakes]

Not too important, but it took me a while to understand what changeset is returned when and what is when assigned to self.tip and when it is left untouched.

Perhaps?

Suggested change

	self.tip = match remove_from.map(|cp| cp.prev()) {
	// The checkpoint below the earliest checkpoint to remove will be the new tip.
	Some(Some(new_tip)) => new_tip,
	// If there is no checkpoint below the earliest checkpoint to remove, it means the
	// "earliest checkpoint to remove" is the genesis block. We disallow removing the
	// genesis block.
	Some(None) => return Err(MissingGenesisError),
	// If there is nothing to remove, we return an empty changeset.
	None => return Ok(ChangeSet::default()),
	};
	Ok(changeset)
	match remove_from.map(|cp| cp.prev()) {
	// The checkpoint below the earliest checkpoint to remove will be the new tip.
	Some(Some(new_tip)) => {
	self.tip = new_tip;
	Ok(changeset)
	}
	// If there is no checkpoint below the earliest checkpoint to remove, it means the
	// "earliest checkpoint to remove" is the genesis block. We disallow removing the
	// genesis block.
	Some(None) => Err(MissingGenesisError),
	// If there is nothing to remove, we return an empty changeset.
	None => Ok(ChangeSet::default()),
	}

[evanlinjin]

Your suggestion is definitely better, but I'll ignore it now because I want this merged and rebasing is annoying 😅

[LLFourn]

ACK 53942cc