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: stuck after restart when min filtered block is previous block of a check point #162

Merged

Conversation

yangby-cryptape
Copy link
Collaborator

@yangby-cryptape yangby-cryptape commented Dec 2, 2023

Changes

  • In the following code:

    pub(crate) fn update_min_filtered_block_number(&self, min_filtered_block_number: BlockNumber) {
    let should_cached_check_point_index =
    self.calc_best_check_point_index_not_greater_than(min_filtered_block_number);

    min_filtered_block_number is the last filtered block which has been synchronized.

    should_cached_check_point_index is the index of the check point, which contains the next filtered block which will be synchronized.

    So, the it should be fixed with following patch:

      pub(crate) fn update_min_filtered_block_number(&self, min_filtered_block_number: BlockNumber) { 
          let should_cached_check_point_index = 
    -         self.calc_best_check_point_index_not_greater_than(min_filtered_block_number); 
    +         self.calc_best_check_point_index_not_greater_than(min_filtered_block_number + 1); 
  • See the following code:

    fn calc_best_check_point_index_not_greater_than(&self, number: BlockNumber) -> u32 {
    (number / self.check_point_interval) as u32
    }

    At start, the check point index and the block number of it is 0, 0.

    The following check point index and the block number of it are 1, 2000, 2, 4000 and son on.

    When the client want to synchronizes the block 2000,the last check point should be 0.

    When the client want to synchronizes the block 2001 to 4000,the last check point should be 1.

    So, when client synchronizes the filtered block number, the check point should be:

    -         (number / self.check_point_interval) as u32 
    +         (number.saturating_sub(1) / self.check_point_interval) as u32
  • Changed some naming to make the names are more appropriate.

@yangby-cryptape yangby-cryptape force-pushed the bugfix/stuct-after-restart branch from 4c58852 to e7499f3 Compare December 2, 2023 02:06
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (38aa9b2) 71.05% compared to head (e7499f3) 71.09%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #162      +/-   ##
===========================================
+ Coverage    71.05%   71.09%   +0.03%     
===========================================
  Files           25       25              
  Lines         6420     6428       +8     
===========================================
+ Hits          4562     4570       +8     
  Misses        1858     1858              
Flag Coverage Δ
unittests 71.09% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yangby-cryptape yangby-cryptape marked this pull request as ready for review December 2, 2023 06:08
@yangby-cryptape yangby-cryptape requested a review from quake December 2, 2023 06:08
@quake quake merged commit d25eabb into nervosnetwork:develop Dec 2, 2023
6 checks passed
@yangby-cryptape yangby-cryptape deleted the bugfix/stuct-after-restart branch December 2, 2023 06:47
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.

3 participants