@evanlinjin's Initial Review

[evanlinjin]

Thanks for this work and for providing a README that explains the design and rationale.

Some initial questions:

1. What was the reason for moving away from CheckPoints? Was it to allow forward iteration and block lookups by hash?
2. Why is an explicit reindex() required? Shouldn't it be possible to implement an algorithm that updates the data correctly without needing this?

Let me try to convince you why keeping CheckPoints is a good idea:

• Shallow-cloning is cheap and thread-safe — It's literally just a smart pointer increment. We can pass the entire chain to a chain source via a shallow clone to start sync without needing to lock the BlockGraph. The non-linked-list alternative is to provide a segment of top blocks to the chain source; however, this forces the caller to decide how many blocks to include — a decision that’s error-prone (e.g., what if WW3 strikes and there’s a 100-block reorg?).
• Backward iteration is cheap — It's a linked list.
• Height-based lookup can be made efficient — We can implement a skip list.

[ValuedMammal]

Concept ack for skiplist.

[evanlinjin]

The premise of CheckPoint is so that we can share the whole block history with the chain-source cheaply. I think it is possible to implement BlockGraph without moving away from this structure internally.

Additionally, CheckPoint is public API. Chain sources use it to receive the wallet's view of the blockchain and to push updates back. If BlockGraph uses a different internal structure, we either need to do O(n) conversion at every sync boundary, or we change the API for all existing chain sources.

PR #2048 shows that we can add skiplist-style optimization to the existing Arc-based CheckPoint structure.

Is there a specific reason why we decided to move away from CheckPoint completely? If so, why couldn't BlockGraph use a similar approach for cheap cloning?