validation: Use witness maleation flag for non-segwit blocks

[fjahr]

Depends on #29524

This is an alternative to #29539 since the race condition documented there should be fixed by #29524. The race condition was discussed here. If the race is not possible we can cache this check for non-segwit blocks (= no expected witness) as well.

[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
Concept ACK	davidgumberg

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

Conflicts

No conflicts as of last run.

[fjahr]

Rebased since #29524 was merged

[davidgumberg]

Concept ACK

Nits that seem to be close to the scope of this PR:

I think this comment is incorrect:

/*
 * Note: If the witness commitment is expected (i.e. `expect_witness_commitment
 * = true`), then the block is required to have at least one transaction and the
 * first transaction needs to have at least one input. */

Isn't it only if the witness commitment is expected and present (i.e. commitpos != NO_WITNESS_COMMITMENT ) that:

assert(!block.vtx.empty() && !block.vtx[0]->vin.empty());

---

The comment in src/primitives/block.h is incorrect:

-     mutable bool m_checked_witness_commitment{false}; // CheckWitnessCommitment()
+     mutable bool m_checked_witness_commitment{false}; // CheckWitnessMalleation()

---

Also, maybe the flag should be renamed to m_checked_witness_malleation so that it's scope is less ambiguous

[maflcko]

-0. Not sure if this is worth it? While the race should have been fixed, are there any benefits that warrant the review?