diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs index 07177f11c..b5a068516 100644 --- a/zebra-state/src/constants.rs +++ b/zebra-state/src/constants.rs @@ -53,13 +53,13 @@ pub(crate) const DATABASE_FORMAT_MINOR_VERSION: u64 = 2; /// The database format patch version, incremented each time the on-disk database format has a /// significant format compatibility fix. -pub(crate) const DATABASE_FORMAT_PATCH_VERSION: u64 = 1; +pub(crate) const DATABASE_FORMAT_PATCH_VERSION: u64 = 2; /// Returns the highest database version that modifies the subtree index format. /// /// This version is used by tests to wait for the subtree upgrade to finish. pub fn latest_version_for_adding_subtrees() -> Version { - Version::parse("25.2.1").expect("Hardcoded version string should be valid.") + Version::parse("25.2.2").expect("Hardcoded version string should be valid.") } /// The name of the file containing the minor and patch database versions. diff --git a/zebra-state/src/service/finalized_state/disk_db.rs b/zebra-state/src/service/finalized_state/disk_db.rs index d001c87d1..3772fb7a7 100644 --- a/zebra-state/src/service/finalized_state/disk_db.rs +++ b/zebra-state/src/service/finalized_state/disk_db.rs @@ -258,6 +258,7 @@ impl PartialEq for DiskDb { self.ephemeral, other.ephemeral, "database with same path but different ephemeral configs", ); + return true; } @@ -431,6 +432,46 @@ impl DiskDb { /// /// Holding this iterator open might delay block commit transactions. pub fn zs_range_iter(&self, cf: &C, range: R) -> impl Iterator + '_ + where + C: rocksdb::AsColumnFamilyRef, + K: IntoDisk + FromDisk, + V: FromDisk, + R: RangeBounds, + { + self.zs_range_iter_with_direction(cf, range, false) + } + + /// Returns a reverse iterator over the items in `cf` in `range`. + /// + /// Holding this iterator open might delay block commit transactions. + /// + /// This code is copied from `zs_range_iter()`, but with the mode reversed. + pub fn zs_reverse_range_iter( + &self, + cf: &C, + range: R, + ) -> impl Iterator + '_ + where + C: rocksdb::AsColumnFamilyRef, + K: IntoDisk + FromDisk, + V: FromDisk, + R: RangeBounds, + { + self.zs_range_iter_with_direction(cf, range, true) + } + + /// Returns an iterator over the items in `cf` in `range`. + /// + /// RocksDB iterators are ordered by increasing key bytes by default. + /// Otherwise, if `reverse` is `true`, the iterator is ordered by decreasing key bytes. + /// + /// Holding this iterator open might delay block commit transactions. + fn zs_range_iter_with_direction( + &self, + cf: &C, + range: R, + reverse: bool, + ) -> impl Iterator + '_ where C: rocksdb::AsColumnFamilyRef, K: IntoDisk + FromDisk, @@ -451,40 +492,67 @@ impl DiskDb { let start_bound = map_to_vec(range.start_bound()); let end_bound = map_to_vec(range.end_bound()); - let range = (start_bound.clone(), end_bound); + let range = (start_bound, end_bound); - let start_bound_vec = - if let Included(ref start_bound) | Excluded(ref start_bound) = start_bound { - start_bound.clone() - } else { - // Actually unused - Vec::new() - }; - - let start_mode = if matches!(start_bound, Unbounded) { - // Unbounded iterators start at the first item - rocksdb::IteratorMode::Start - } else { - rocksdb::IteratorMode::From(start_bound_vec.as_slice(), rocksdb::Direction::Forward) - }; + let mode = Self::zs_iter_mode(&range, reverse); // Reading multiple items from iterators has caused database hangs, // in previous RocksDB versions self.db - .iterator_cf(cf, start_mode) + .iterator_cf(cf, mode) .map(|result| result.expect("unexpected database failure")) .map(|(key, value)| (key.to_vec(), value)) - // Skip excluded start bound and empty ranges. The `start_mode` already skips keys - // before the start bound. + // Skip excluded "from" bound and empty ranges. The `mode` already skips keys + // strictly before the "from" bound. .skip_while({ let range = range.clone(); move |(key, _value)| !range.contains(key) }) - // Take until the excluded end bound is reached, or we're after the included end bound. + // Take until the excluded "to" bound is reached, + // or we're after the included "to" bound. .take_while(move |(key, _value)| range.contains(key)) .map(|(key, value)| (K::from_bytes(key), V::from_bytes(value))) } + /// Returns the RocksDB iterator "from" mode for `range`. + /// + /// RocksDB iterators are ordered by increasing key bytes by default. + /// Otherwise, if `reverse` is `true`, the iterator is ordered by decreasing key bytes. + fn zs_iter_mode(range: &R, reverse: bool) -> rocksdb::IteratorMode + where + R: RangeBounds>, + { + use std::ops::Bound::*; + + let from_bound = if reverse { + range.end_bound() + } else { + range.start_bound() + }; + + match from_bound { + Unbounded => { + if reverse { + // Reversed unbounded iterators start from the last item + rocksdb::IteratorMode::End + } else { + // Unbounded iterators start from the first item + rocksdb::IteratorMode::Start + } + } + + Included(bound) | Excluded(bound) => { + let direction = if reverse { + rocksdb::Direction::Reverse + } else { + rocksdb::Direction::Forward + }; + + rocksdb::IteratorMode::From(bound.as_slice(), direction) + } + } + } + /// The ideal open file limit for Zebra const IDEAL_OPEN_FILE_LIMIT: u64 = 1024; diff --git a/zebra-state/src/service/finalized_state/disk_format/upgrade/add_subtrees.rs b/zebra-state/src/service/finalized_state/disk_format/upgrade/add_subtrees.rs index fc5af751d..fafff57f4 100644 --- a/zebra-state/src/service/finalized_state/disk_format/upgrade/add_subtrees.rs +++ b/zebra-state/src/service/finalized_state/disk_format/upgrade/add_subtrees.rs @@ -21,6 +21,9 @@ use crate::service::finalized_state::{ /// Runs disk format upgrade for adding Sapling and Orchard note commitment subtrees to database. /// +/// Trees are added to the database in reverse height order, so that wallets can sync correctly +/// while the upgrade is running. +/// /// Returns `Ok` if the upgrade completed, and `Err` if it was cancelled. #[allow(clippy::unwrap_in_result)] #[instrument(skip(upgrade_db, cancel_receiver))] @@ -38,13 +41,21 @@ pub fn run( // block can't complete multiple level 16 subtrees (or complete an entire subtree by itself). // Currently, with 2MB blocks and v4/v5 sapling and orchard output sizes, the subtree index can // increase by at most 1 every ~20 blocks. + // + // # Compatibility + // + // Because wallets search backwards from the chain tip, subtrees need to be added to the + // database in reverse height order. (Tip first, genesis last.) + // + // Otherwise, wallets that sync during the upgrade will be missing some notes. // Generate a list of sapling subtree inputs: previous and current trees, and their end heights. let subtrees = upgrade_db - .sapling_tree_by_height_range(..=initial_tip_height) + .sapling_tree_by_reversed_height_range(..=initial_tip_height) // We need both the tree and its previous tree for each shielded block. .tuple_windows() - .map(|((prev_end_height, prev_tree), (end_height, tree))| { + // Because the iterator is reversed, the larger tree is first. + .map(|((end_height, tree), (prev_end_height, prev_tree))| { (prev_end_height, prev_tree, end_height, tree) }) // Find new subtrees. @@ -65,10 +76,11 @@ pub fn run( // Generate a list of orchard subtree inputs: previous and current trees, and their end heights. let subtrees = upgrade_db - .orchard_tree_by_height_range(..=initial_tip_height) + .orchard_tree_by_reversed_height_range(..=initial_tip_height) // We need both the tree and its previous tree for each shielded block. .tuple_windows() - .map(|((prev_end_height, prev_tree), (end_height, tree))| { + // Because the iterator is reversed, the larger tree is first. + .map(|((end_height, tree), (prev_end_height, prev_tree))| { (prev_end_height, prev_tree, end_height, tree) }) // Find new subtrees. diff --git a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs index 5b510d901..b74ed6d2c 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/shielded.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/shielded.rs @@ -198,6 +198,19 @@ impl ZebraDb { self.db.zs_range_iter(&sapling_trees, range) } + /// Returns the Sapling note commitment trees in the reversed range, in decreasing height order. + #[allow(clippy::unwrap_in_result)] + pub fn sapling_tree_by_reversed_height_range( + &self, + range: R, + ) -> impl Iterator)> + '_ + where + R: std::ops::RangeBounds, + { + let sapling_trees = self.db.cf_handle("sapling_note_commitment_tree").unwrap(); + self.db.zs_reverse_range_iter(&sapling_trees, range) + } + /// Returns the Sapling note commitment subtree at this `index`. /// /// # Correctness @@ -331,6 +344,19 @@ impl ZebraDb { self.db.zs_range_iter(&orchard_trees, range) } + /// Returns the Orchard note commitment trees in the reversed range, in decreasing height order. + #[allow(clippy::unwrap_in_result)] + pub fn orchard_tree_by_reversed_height_range( + &self, + range: R, + ) -> impl Iterator)> + '_ + where + R: std::ops::RangeBounds, + { + let orchard_trees = self.db.cf_handle("orchard_note_commitment_tree").unwrap(); + self.db.zs_reverse_range_iter(&orchard_trees, range) + } + /// Returns the Orchard note commitment subtree at this `index`. /// /// # Correctness