doc: Assert that assumed-valid blocks are not fully valid in CheckBlockIndex()

[maflcko]

It does not make sense for a block to be fully-valid and assumed-valid at the same time.

Check it in CheckBlockIndex() and fix the tests that violate this assumption.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	fjahr, ryanofsky
Concept ACK	delta1, epiccurious

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #29370 (assumeutxo: Get rid of faked nTx and nChainTx values by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

[delta1]

Concept ACK fa027e0

[epiccurious]

Concept ACK fa027e0.

[fjahr]

Seems reasonable, code review ACK fa027e0

Not sure why this is marked as a "doc" change though.

[delta1]

@fjahr i think drahtbot added that label because the commit starts with “doc:”

[fjahr]

@fjahr i think drahtbot added that label because the commit starts with “doc:”

Yes, my question is why the "doc" is there in the commit desciption

[ryanofsky]

Code review ACK fa027e0. This change makes sense and is an improvement over the status quo. I think it could be considered it a "doc" change since it is just adding an assert, which is a form of documentation, and not changing actual code. Not sure if that is how "doc" has been used other places, though.

This PR is a little redundant with #29370, which drops the BLOCK_ASSUMED_VALID flag entirely, so there is no way it could conflict with BLOCK_VALID_SCRIPTS. But it makes sense to check for conflicting flags as long as both flags exist.

[maflcko]

This PR is a little redundant with #29370, which drops the BLOCK_ASSUMED_VALID flag entirely, so there is no way it could conflict with BLOCK_VALID_SCRIPTS. But it makes sense to check for conflicting flags as long as both flags exist.

Yeah, makes sense. I forgot to close this pull when the other was opened.