Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PIBD] Chain Segmenter Validation Test + Block Archive Horizon Change #3665

Merged
merged 11 commits into from
Nov 23, 2021

Conversation

yeastplume
Copy link
Member

@yeastplume yeastplume commented Nov 15, 2021

Review Version:

  • Adds an integration test that acts upon small test instances of chain data, both compacted and uncompacted. The test reads all 4 datasets from the PIBD segmenter in the order which it might happen during a live sync, then validates all of the segments. It also reconstructs the raw croaring::Bitmap from the read bitmap segments for use in output/rangeproof segment validation.
  • Adds the sample test chains (as output by the new wallet test PIBD Test Set Generation test grin-wallet#628) as binary data. Not ideal, but much better than what we have now to test this (i.e. nothing). It's anticipated this test set will be updated and used in further testing.
  • Adds an #[ignored] test that will allow users to manually run the validation test against another instance of the chain. I've run this test against a copy of the recent live chain, with segments validating successfully.
  • Removed the temporary initialisation at startup of the chain segmenter (it will need to be invoked and updated lazily once messaging is in place)
  • IMPORTANT TO REVIEW - During chain compaction on test data, the chain discards blocks that the segmenter expects to be in place. Added a manual check to adjust the horizon at which blocks are discarded if the calculated cutoff in chain::remove_historical_blocks is greater than the horizon expected by the segmenter
  • Adds a function to BitmapChunk to return an iterator over the set of integers represented by that chunk (with an offset added). Used as a helper to reconstruct a raw croaring::Bitmap for segment validation.
  • Adds functions to SegmentIdentifier to determine the number of segments that would need to be requested given the target pmmr size and the desired segment height.

---- Old Version of this ----

Snapshot of current explorations into PIBD, note this PR in its current state is more an attempt at understanding and starting to put together tests to exercise the the PIBD segmenter and segment validation. The current goal is to get an entire compressed/pruned hashset 'copied' locally via PIBD functions, and once this is in place (with sufficient unit tests exercising all of the PIBD PMMR work done to date,) we can start thinking about the actual sync process.

Thus far:

  • Introduction of some static test data into the chain crate, a small but 'real' transaction hash set with real transaction data generated via a wallet test (will commit that test separately to grin_wallet). (Yes, I'm adding static binary data to the repository for the purposes of testing PIBD/PMMR segmentation. Never ideal but much better than the nothing we currently have).
  • WIP 'megatest' that reads a test chain, copies headers into a new chain instance then attempts to read and validate segments from the original. Note this will likely evolve into series of tests exercising the segmenter and segment validation (as none exist at the moment), which will likely be what this PR ends up being.

Next up:

  • Figure out why output root validation isn't working
  • Read multiple segments from the source hashset
  • Addition of chain/txhashset functions to rebuild a new chain based on received segments (likely the focus of the next PR)

Don't forget:

  • NRD Kernels need to be considered in the validation (to be discussed)

@yeastplume yeastplume changed the title [WIP] (PIBD) tx_hashset copy test [PIBD] Chain Segmenter Validation Test + Block Archive Horizon Change Nov 17, 2021
@yeastplume
Copy link
Member Author

yeastplume commented Nov 17, 2021

Top comment updated, ready for review. This mostly adds segmenter tests against live data which should also serve as a reference for how peers would call and validate data from other peers. HOWEVER there is a change to archive thresholds that needs review + thought.

Next up would be functionality + further tests that recreate hash set data from read segments.

// TODO: This can probably be derived from the PMMR we'll eventually be building
// (check if total size is equal to total size at horizon header)
let identifier_iter =
SegmentIdentifier::traversal_iter(bitmap_pmmr_size, target_segment_height);
Copy link
Contributor

@tromp tromp Nov 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you just computed count_segments_required, you might as well check that it equals the identifier_iter size (similarly for the other MMRs) ?!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was doing that at one point but removed it (led to redundant code since count() on an iterator consumes it, I'm sure there are other ways but didn't go further). Will add something similar into future tests.

Copy link
Contributor

@tromp tromp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks reasonable. See my few in-line comments for minor issues.

@yeastplume yeastplume merged commit c8275f7 into mimblewimble:master Nov 23, 2021
@yeastplume
Copy link
Member Author

Merged, thanks for review!

@yeastplume yeastplume deleted the txhashset_copy_test branch January 20, 2022 10:01
@yeastplume yeastplume mentioned this pull request Feb 21, 2022
26 tasks
bayk added a commit to mwcproject/mwc-node that referenced this pull request Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants