cmpctblock: refactor: Delete unnecessary code in `PartiallyDownloadedBlock::InitData()` #35638

This is a refactor that was split out of work related to #35558.

First, have_txn is redundant, since txn_available is a vector of shared_ptr:

https://github.com/bitcoin/bitcoin/blob/bd1771fc0cd248438e87b028b096f59ffc976d18/src/blockencodings.h#L135

resized:

https://github.com/bitcoin/bitcoin/blob/bd1771fc0cd248438e87b028b096f59ffc976d18/src/blockencodings.cpp#L70

Which means all of its members are value-initialized which for a shared_ptr means "Constructs a shared_ptr with no managed object, i.e. empty shared_ptr." and std::shared_ptr<T>::operator bool returns false for empty shared_ptr's.

In fact, the existing code already relies on this behavior:

https://github.com/bitcoin/bitcoin/blob/a8823c099618559fdf6116fe17c9afd311e88890/src/blockencodings.cpp#L161

Second,

if (a) {
    b()
} else {
    if(!a) {
        c()
    }
}

is equivalent to

if (a) {
    b()
} else {
    c()
}

so I make this change.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35638.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #35558 (p2p: Prefill compact blocks by davidgumberg)

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.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

Concept ACK

Isn't there a difference between have_txn and txn_available after a shortid duplicate resets txn_available[index]? A later match for the same index would still see have_txn[index] == true before this PR, but not after this PR. Is this still a refactor?

Isn't there a difference between have_txn and txn_available after a shortid duplicate resets txn_available[index]? A later match for the same index would still see have_txn[index] == true before this PR, but not after this PR. Is this still a refactor?

Ah, good point! So, in the case where a collision has already happened, and there is a third transaction with the same shortid, this PR would cause a node to accept the third transaction instead of continuing on with txn_available[idit->second] == false.

This PR was based on a misunderstanding so I'll close, thanks for catching.