Skip to content

Fix two issues with ResetBlockFailureFlags, add tests #3965

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.

Sign up for GitHub

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

Jump to bottom
Merged
PastaPastaPasta merged 4 commits into from
Feb 1, 2021
Merged

Fix two issues with ResetBlockFailureFlags, add tests #3965

PastaPastaPasta merged 4 commits into from
Feb 1, 2021

Conversation

@UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Jan 23, 2021

Without fe38ff9 new test crashes with errors like

Assertion failure:
  assertion: (pindex->nStatus & BLOCK_FAILED_MASK) == 0
  file: validation.cpp, line: 5013
  function: CheckBlockIndex
   0#: (0x10E526E65) stacktraces.cpp:651        - __assert_rtn
   1#: (0x10E2BBBD0) validation.cpp:5013        - CheckBlockIndex
   2#: (0x10E2BA6DC) microsec_time_clock.hpp:70 - ActivateBestChain
   3#: (0x10E2BBF34) validation.cpp             - ???
   4#: (0x10E181B2A) memory:4521                - reconsiderblock
   5#: (0x10E21B72B) server.cpp:626             - execute
   6#: (0x10DE65A0D) httprpc.cpp:197            - HTTPReq_JSONRPC
   7#: (0x10DE73178) functional:1799            - operator()
   8#: (0x10DE746BD) memory:2651                - Run
   9#: (0x10DE74C8A) memory:2649                - __thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(WorkQueue<HTTPClosure> *), WorkQueue<HTTPClosure> *> >
  10#: (0x7FFF6DDC82EB) <unknown-file>             - ???
  11#: (0x7FFF6DDCB249) <unknown-file>             - ???
  12#: (0x7FFF6DDC740D) <unknown-file>             - ???
Assertion failed: ((pindex->nStatus & BLOCK_FAILED_MASK) == 0), function CheckBlockIndex, file validation.cpp, line 5013.

sipa and others added 3 commits January 23, 2021 07:25
@sipa @UdjinM6
Merge bitcoin#13199: Bugfix: ensure consistency of m_failed_blocks af…
01dc1fd 
…ter reconsiderblock

11fa6bb Bugfix: ensure consistency of m_failed_blocks after reconsiderblock (Suhas Daftuar)

Pull request description:

  This was introduced in 015a525 and could cause a node to crash (due to assertion failure) when using the `reconsiderblock` rpc.

Tree-SHA512: 820dcd761bf983e36f5d0f16777ed75c833daaf62a6b3a4dbd17f6caaf9287223e3a202d06540ac62f8ba72926b73b0873bb76c6273ddcb19d9408f4c1cd325e
@UdjinM6
bugfix: Mark all nearest BLOCK_FAILED_CHILD descendants (if any) as B…
fe38ff9 
…LOCK_FAILED_VALID while removing the invalidity flag from all ancestors in ResetBlockFailureFlags

Fixes `Assertion failed: ((pindex->nStatus & BLOCK_FAILED_MASK) == 0), function CheckBlockIndex`
@UdjinM6
tests: Make sure ResetBlockFailureFlags does the job correctly
8de520f
@UdjinM6 UdjinM6 added this to the 17 milestone Jan 23, 2021
@UdjinM6 UdjinM6 marked this pull request as ready for review January 23, 2021 14:18
@UdjinM6 UdjinM6 mentioned this pull request Jan 23, 2021
Merged
xdustinface
xdustinface reviewed Jan 25, 2021
View reviewed changes
xdustinface
xdustinface approved these changes Jan 25, 2021
View reviewed changes
Copy link

@xdustinface xdustinface left a comment

Choose a reason for hiding this comment

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

utACK

PastaPastaPasta
PastaPastaPasta approved these changes Feb 1, 2021
View reviewed changes
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK

@PastaPastaPasta PastaPastaPasta merged commit 7188004 into dashpay:develop Feb 1, 2021
@UdjinM6 UdjinM6 deleted the fix_ResetBlockFailureFlags branch July 1, 2021 21:56
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Mar 27, 2022
@UdjinM6 @sipa @gades
Fix two issues with ResetBlockFailureFlags, add tests (dashpay#3965)
5cae10f 
* Merge bitcoin#13199: Bugfix: ensure consistency of m_failed_blocks after reconsiderblock

11fa6bb Bugfix: ensure consistency of m_failed_blocks after reconsiderblock (Suhas Daftuar)

Pull request description:

  This was introduced in 015a525 and could cause a node to crash (due to assertion failure) when using the `reconsiderblock` rpc.

Tree-SHA512: 820dcd761bf983e36f5d0f16777ed75c833daaf62a6b3a4dbd17f6caaf9287223e3a202d06540ac62f8ba72926b73b0873bb76c6273ddcb19d9408f4c1cd325e

* bugfix: Mark all nearest BLOCK_FAILED_CHILD descendants (if any) as BLOCK_FAILED_VALID while removing the invalidity flag from all ancestors in ResetBlockFailureFlags

Fixes `Assertion failed: ((pindex->nStatus & BLOCK_FAILED_MASK) == 0), function CheckBlockIndex`

* tests: Make sure ResetBlockFailureFlags does the job correctly

* Wait for the expected block height, check the final chain tip hash

Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PastaPastaPasta PastaPastaPasta PastaPastaPasta approved these changes

@xdustinface xdustinface xdustinface approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

17

Development

Successfully merging this pull request may close these issues.

4 participants

@UdjinM6 @PastaPastaPasta @xdustinface @sipa