From 34f50d7ebbe53b2aa176452f3756646e913e7972 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Mon, 9 Nov 2020 15:53:16 -0800 Subject: [PATCH] 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. --- book/src/dev/rfcs/0005-state-updates.md | 25 ++++++++++--------- .../memory_state/non_finalized_state.rs | 1 + 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/book/src/dev/rfcs/0005-state-updates.md b/book/src/dev/rfcs/0005-state-updates.md index e2842f11671..0192f0b52dc 100644 --- a/book/src/dev/rfcs/0005-state-updates.md +++ b/book/src/dev/rfcs/0005-state-updates.md @@ -124,11 +124,12 @@ pub enum Request { ``` `zebra-state` breaks down its requests into two categories and provides -different guarantees for each category: requests that modify the state, and requests that -do not. Requests that update the state are guaranteed to run sequentially and -will never race against each other. Requests that read state are done -asynchronously and are guaranteed to read at least the state present 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 +different guarantees for each category: requests that modify the state, and +requests that do not. Requests that update the state are guaranteed to run +sequentially and will never race against each other. Requests that read state +are done asynchronously and are guaranteed to read at least the state present +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 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, 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 -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 [chain-type]: #chain-type @@ -343,10 +344,10 @@ Remove the highest height block of the non-finalized portion of a chain. #### `Ord` -The `Chain` type implements `Ord` for reorganizing chains. First chains -are compared by their `partial_cumulative_work`. Ties are then broken by -comparing `block::Hash`es of the tips of each chain. (This tie-breaker -means that all `Chain`s in the `ChainSet` must have at least one block.) +The `Chain` type implements `Ord` for reorganizing chains. First chains are +compared by their `partial_cumulative_work`. Ties are then broken by +comparing `block::Hash`es of the tips of each chain. (This tie-breaker means +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 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 block heights and block hashes. (Since the Sled state only stores finalized state, they are actually a bijection). - + - 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, 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), genesis block anchors and transactions are ignored during validation.) - + 4. Update the `sprout_anchors` and `sapling_anchors` trees with the Sprout and Sapling anchors. diff --git a/zebra-state/src/service/memory_state/non_finalized_state.rs b/zebra-state/src/service/memory_state/non_finalized_state.rs index 5b6965f06c6..0536e239758 100644 --- a/zebra-state/src/service/memory_state/non_finalized_state.rs +++ b/zebra-state/src/service/memory_state/non_finalized_state.rs @@ -93,6 +93,7 @@ impl NonFinalizedState { pub fn any_chain_contains(&self, hash: &block::Hash) -> bool { self.chain_set .iter() + .rev() .any(|chain| chain.height_by_hash.contains_key(hash)) }