[PIBD] PMMR Desegmenter Structure (Pt. 1) #3667
Conversation
…s, validating each segment as it goes
…ncompacted sample data. Also contains method of running test againt live chain data
…for fully unpruned trees atm)
yeastplume
changed the title
yeastplume
changed the title
chain/src/chain.rs
Outdated
| @@ -863,6 +865,43 @@ impl Chain { | |||
| )) | |||
| } | |||
|
|
|||
| /// instantiate desegmenter (in same lazy fashion as segmenter, though this should be be | |||
| // is in flight and we cross a horizon boundary, but needs more thinking) | ||
| let desegmenter = self.init_desegmenter(archive_header)?; | ||
| let mut cache = self.pibd_desegmenter.write(); | ||
| *cache = Some(desegmenter.clone()); |
There was a problem hiding this comment.
Shouldn't these last 2 lines of code be the responsibility of the init_desegmenter method?
That would make more sense to me...
chain/src/txhashset/desegmenter.rs
Outdated
| fn calc_bitmap_mmr_sizes(&mut self) { | ||
| // Number of leaves (BitmapChunks) | ||
| self.bitmap_mmr_leaf_count = | ||
| (pmmr::n_leaves(self.archive_header.output_mmr_size) as f64 / 1024f64).ceil() as u64; |
There was a problem hiding this comment.
Don't use floating point!
(n+1023) / 1024 computes rounded up division with plain integers.
core/src/core/pmmr/segment.rs
Outdated
| @@ -87,7 +87,8 @@ impl SegmentIdentifier { | |||
| /// Returns number of segments required that would needed in order to read a | |||
| /// pmmr of size `target_mmr_size` in segments of height `segment_height` | |||
| pub fn count_segments_required(target_mmr_size: u64, segment_height: u8) -> usize { | |||
| pmmr::n_leaves(target_mmr_size) as usize / (1 << segment_height as usize) | |||
| (pmmr::n_leaves(target_mmr_size) as f64 / (1 << segment_height as usize) as f64).ceil() | |||
There was a problem hiding this comment.
No floating point please! See other comment.
| /// TODO: Still experimenting, this expects chunks received to be in order | ||
| pub fn add_bitmap_segment( | ||
| &mut self, | ||
| segment: Segment<BitmapChunk>, |
There was a problem hiding this comment.
why feed in a whole segment when you only use its leaf_data component?
| /// TODO: Not complete | ||
| pub fn apply_output_segment( | ||
| &mut self, | ||
| segment: Segment<OutputIdentifier>, |
There was a problem hiding this comment.
why feed in a whole segment when you only use its leaf_data component?
|
|
||
| /// Apply a rangeproof segment to the output PMMR. must be called in order | ||
| /// TODO: Not complete | ||
| pub fn apply_rangeproof_segment(&mut self, segment: Segment<RangeProof>) -> Result<(), Error> { |
There was a problem hiding this comment.
why feed in a whole segment when you only use one component?
| fn test_pibd_copy_real() { | ||
| util::init_test_logger(); | ||
| // if testing against a real chain, insert location here | ||
| let src_root_dir = format!("/Users/yeastplume/Projects/grin_project/server/chain_data"); |
There was a problem hiding this comment.
That path is not gonna work on my machine:-(
There was a problem hiding this comment.
it's not meant to, it's meant for running a manual test against a live chain instance (so while testing you modify the path).. I'll make this more clear in comments
| @@ -87,7 +87,7 @@ fn test_pibd_chain_validation_impl(is_test_chain: bool, src_root_dir: &str) { | |||
| println!("BITMAP PMMR NUM_LEAVES: {}", bitmap_mmr_num_leaves); | |||
|
|
|||
| // And total size of the bitmap PMMR | |||
| let bitmap_pmmr_size = pmmr::peaks(bitmap_mmr_num_leaves + 1) | |||
| let bitmap_pmmr_size = pmmr::peaks(bitmap_mmr_num_leaves) | |||
There was a problem hiding this comment.
I assume this is because the former was written before my 1-based -> 0-based conversion?
|
Thank you, will address bits before merging this. It's perhaps a bit early for review and there are still a lot of things here that are going to change, but I'd just like to get everything I have to date merged before continuing with this. |
|
Addressed a few issues, will keep the rest in consideration during future work |
This is the first part of several anticipated PRs to enable and test the reconstruction of the txhashset from PMMR segments. Note that none of the code introduced here is called from anywhere but the main test
test_pibd_copy. Merging this a bit earlier than I'd planned, but breaking up the work like this keeps the review process a bit more manageable, and the included test is needed to help track down a potential serious performance issue in header sync 😄Included in this PR:
desegmentertype containing functions to validate and append PMMR segments.BitmapAccumulatorto return the accumulator as a raw bitmap for use in desegmenter validationcount_segments_requiredlast_postommr_sizein Segment to be consistent with changes Fixmmr part2 #3666Note the current copy tests do work, however they haven't been tested or verified against actual pruned data.