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

Fix for the pruning proof rebuild issue (issue #444) #449

Merged

Conversation

michaelsutton
Copy link
Contributor

@michaelsutton michaelsutton commented Apr 10, 2024

The following describes the pruning bug and the fix assuming sufficient prior knowledge.

Some notation:

  • High levels: block levels at which there are less than 2000 blocks since genesis (so genesis is part of that proof level). Currently this is approx around level 39 and above.
  • Low levels: block levels close to current difficulty and hence it is likely that all 2000 proof blocks are within the last day before the corresponding pruning point.
  • Intermediate levels: block levels which are not that rare but are still unlikely to have more than 2000 blocks all within a single day. Hence their level proof likely has blocks both before and after the previous pruning point.

The bug is mostly in the case of intermediate levels. It requires that such a block is mined in the anticone of the pruning point (so the bug is likely to occur ~once in a few days, and requires that someone synced a new node exactly at that day).

The logic missing: we did not update relations per level for the pruning point anticone (aka "trusted" blocks).

Scenario: a "lucky" intermediate-level block R happened to be in the pruning point anticone. A future block B on that level is likely to have R as its only parent (for that level). Since level relations are not updated for R, B computes Ghostdag for that level over ORIGIN -- essentially starting from blue score 0 instead of building over the previous 2000 blocks. When building the new proof, this segment of blocks (S = B + its future) is topologically disconnected from block N which is the block at depth 1000 of the next level (which is likely below the current new pruning point). So N becomes root, and the traversal starting from it never reaches S. Leading to a possible prune of blocks in S (some may be preserved via other levels)

Result: following the prune, the proof rebuild fails bcs of blocks missing from S. This is only a symptom of the bug, the real problem is that the level proof is literally incomplete.

Fix: update relations per level also for trusted blocks

@michaelsutton michaelsutton changed the title Possible fix for the pruning proof rebuild issue (issue #444) Fix for the pruning proof rebuild issue (issue #444) Apr 10, 2024
Copy link
Collaborator

@tiram88 tiram88 left a comment

Choose a reason for hiding this comment

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

I reviewed the last 3 commits.

My knowledge of the pruning system is not deep enough to bring a valuable review on this part of the PR.

notify/src/address/tracker.rs Outdated Show resolved Hide resolved
kaspad/src/args.rs Show resolved Hide resolved
consensus/src/processes/mass.rs Show resolved Hide resolved
coderofstuff
coderofstuff previously approved these changes Apr 10, 2024
Copy link
Collaborator

@coderofstuff coderofstuff left a comment

Choose a reason for hiding this comment

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

The fix is mainly the change in header_processor. There are 3 other sections unrelated to the pruning proof bug. Approving for these sections

  • pruning proof bug fix
  • kip9 change
  • enable-mainnet-mining change

Please address Tiram's comments for the UTXO tracking section.

@michaelsutton michaelsutton merged commit 1eebd4d into kaspanet:master Apr 10, 2024
6 checks passed
smartgoo pushed a commit to smartgoo/rusty-kaspa that referenced this pull request Jun 18, 2024
…t#449)

* add a strict assertion which should catch the pruning bug before actual data is pruned

* possible fix: add `block_at_depth_2m` as an additional traversal root

* rollback: rollback the previous fix since it's not the root cause

* add additional dbg info to assertion

* bug fix: write level relations for trusted blocks (blocks in the pruning point anticone of a newly synced node)

* enable mainnet mining by default

* simplify kip 9 beta condition + more mass tests

* set default tracked addresses to 1M

* fix tracker prealloc property + adds compile time assertion for upper bound
D-Stacks pushed a commit to D-Stacks/rusty-kaspa that referenced this pull request Jul 12, 2024
…t#449)

* add a strict assertion which should catch the pruning bug before actual data is pruned

* possible fix: add `block_at_depth_2m` as an additional traversal root

* rollback: rollback the previous fix since it's not the root cause

* add additional dbg info to assertion

* bug fix: write level relations for trusted blocks (blocks in the pruning point anticone of a newly synced node)

* enable mainnet mining by default

* simplify kip 9 beta condition + more mass tests

* set default tracked addresses to 1M

* fix tracker prealloc property + adds compile time assertion for upper bound
D-Stacks pushed a commit to D-Stacks/rusty-kaspa that referenced this pull request Jul 12, 2024
…t#449)

* add a strict assertion which should catch the pruning bug before actual data is pruned

* possible fix: add `block_at_depth_2m` as an additional traversal root

* rollback: rollback the previous fix since it's not the root cause

* add additional dbg info to assertion

* bug fix: write level relations for trusted blocks (blocks in the pruning point anticone of a newly synced node)

* enable mainnet mining by default

* simplify kip 9 beta condition + more mass tests

* set default tracked addresses to 1M

* fix tracker prealloc property + adds compile time assertion for upper bound
D-Stacks pushed a commit to D-Stacks/rusty-kaspa that referenced this pull request Jul 17, 2024
…t#449)

* add a strict assertion which should catch the pruning bug before actual data is pruned

* possible fix: add `block_at_depth_2m` as an additional traversal root

* rollback: rollback the previous fix since it's not the root cause

* add additional dbg info to assertion

* bug fix: write level relations for trusted blocks (blocks in the pruning point anticone of a newly synced node)

* enable mainnet mining by default

* simplify kip 9 beta condition + more mass tests

* set default tracked addresses to 1M

* fix tracker prealloc property + adds compile time assertion for upper bound
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.

4 participants