Fix inconsistencies related to best chain order in RFC and state impl (#1267)

Prior to this PR we realized that the RFC had been drafted with the assumption that chains would be ordered from best to worst in `NonFinalizedState`. This assumption was incorrect, since `BTreeSet` only ever orders values in ascending order. This discrepancy was noticed and fixed in the code, but there were still some inconsistencies that needed to be cleaned up.

This PR updates all the incorrect or confusing comments about chain ordering in the RFC and code.
This commit is contained in:
Jane Lusby 2020-11-09 15:53:16 -08:00 committed by GitHub
parent efef2a2bd7
commit 34f50d7ebb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 14 additions and 12 deletions

View File

@ -124,11 +124,12 @@ pub enum Request {
``` ```
`zebra-state` breaks down its requests into two categories and provides `zebra-state` breaks down its requests into two categories and provides
different guarantees for each category: requests that modify the state, and requests that different guarantees for each category: requests that modify the state, and
do not. Requests that update the state are guaranteed to run sequentially and requests that do not. Requests that update the state are guaranteed to run
will never race against each other. Requests that read state are done sequentially and will never race against each other. Requests that read state
asynchronously and are guaranteed to read at least the state present at the are done asynchronously and are guaranteed to read at least the state present
time the request was processed by the service, or a later state present at the time the request future is executed. The state service avoids at the time the request was processed by the service, or a later state
present at the time the request future is executed. The state service avoids
race conditions between the read state and the written state by doing all race conditions between the read state and the written state by doing all
contextual verification internally. contextual verification internally.
@ -231,7 +232,7 @@ time the request was processed, or a later state.
At a high level, the in-memory data structures store a collection of chains, At a high level, the in-memory data structures store a collection of chains,
each rooted at the highest finalized block. Each chain consists of a map from each rooted at the highest finalized block. Each chain consists of a map from
heights to blocks. Chains are stored using an ordered map from cumulative work to heights to blocks. Chains are stored using an ordered map from cumulative work to
chains, so that the map ordering is the ordering of best to worst chains. chains, so that the map ordering is the ordering of worst to best chains.
### The `Chain` type ### The `Chain` type
[chain-type]: #chain-type [chain-type]: #chain-type
@ -343,10 +344,10 @@ Remove the highest height block of the non-finalized portion of a chain.
#### `Ord` #### `Ord`
The `Chain` type implements `Ord` for reorganizing chains. First chains The `Chain` type implements `Ord` for reorganizing chains. First chains are
are compared by their `partial_cumulative_work`. Ties are then broken by compared by their `partial_cumulative_work`. Ties are then broken by
comparing `block::Hash`es of the tips of each chain. (This tie-breaker comparing `block::Hash`es of the tips of each chain. (This tie-breaker means
means that all `Chain`s in the `ChainSet` must have at least one block.) that all `Chain`s in the `NonFinalizedState` must have at least one block.)
**Note**: Unlike `zcashd`, Zebra does not use block arrival times as a **Note**: Unlike `zcashd`, Zebra does not use block arrival times as a
tie-breaker for the best tip. Since Zebra downloads blocks in parallel, tie-breaker for the best tip. Since Zebra downloads blocks in parallel,
@ -617,7 +618,7 @@ Zcash structures are encoded using `ZcashSerialize`/`ZcashDeserialize`.
- The `hash_by_height` and `height_by_hash` trees provide a bijection between - The `hash_by_height` and `height_by_hash` trees provide a bijection between
block heights and block hashes. (Since the Sled state only stores finalized block heights and block hashes. (Since the Sled state only stores finalized
state, they are actually a bijection). state, they are actually a bijection).
- The `block_by_height` tree provides a bijection between block heights and block - The `block_by_height` tree provides a bijection between block heights and block
data. There is no corresponding `height_by_block` tree: instead, hash the block, data. There is no corresponding `height_by_block` tree: instead, hash the block,
and use `height_by_hash`. (Since the Sled state only stores finalized state, and use `height_by_hash`. (Since the Sled state only stores finalized state,
@ -668,7 +669,7 @@ check that `block`'s parent hash is `null` (all zeroes) and its height is `0`.
(Due to a [bug in zcashd](https://github.com/ZcashFoundation/zebra/issues/559), (Due to a [bug in zcashd](https://github.com/ZcashFoundation/zebra/issues/559),
genesis block anchors and transactions are ignored during validation.) genesis block anchors and transactions are ignored during validation.)
4. Update the `sprout_anchors` and `sapling_anchors` trees with the Sprout and 4. Update the `sprout_anchors` and `sapling_anchors` trees with the Sprout and
Sapling anchors. Sapling anchors.

View File

@ -93,6 +93,7 @@ impl NonFinalizedState {
pub fn any_chain_contains(&self, hash: &block::Hash) -> bool { pub fn any_chain_contains(&self, hash: &block::Hash) -> bool {
self.chain_set self.chain_set
.iter() .iter()
.rev()
.any(|chain| chain.height_by_hash.contains_key(hash)) .any(|chain| chain.height_by_hash.contains_key(hash))
} }