multi_index: fix compilation failure with boost >= 1.91 #35175

This effectively reverts a3cb309e7c31853f272bffaa65fb6ab0a7cc4083 from PR #30194.

That PR reduced the multi_index type signatures as recommended upstream, but this is no longer supported as of boost 1.91 because it is no longer necessary. 1.91 drops support for the pre-c++11 work-arounds that bloated the type signatures to begin with.

The upstream BOOST_MULTI_INDEX_ENABLE_MPL_SUPPORT define is meant to provide compatibility with removed features, but it does not work for this case. Using indexed_by directly when defining the multi_index (as opposed to inheriting from it) works with all versions, and avoids the use of the back-compat define.

This is a slight regression when building against boost < 1.91 because the bloated type signatures are reintroduced in that case, but it's not significant enough to go to the trouble of introducing version detection and ifdefs.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--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.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

Ping @fanquake @hebasto @maflcko

I cherry-picked #35147 on top of this one temporarily to make sure that all is good after the bump to boost 1.91. I'll remove that commit if/when CI is happy.

All good with bumped boost: https://github.com/bitcoin/bitcoin/actions/runs/25070811505/

I've popped off that commit so the boost bump can happen separately.

Concept ACK.

This is a slight regression when building against boost < 1.91 because the bloated type signatures are reintroduced in that case...

To clarify, this only affects compile times, right?

What about an alternative approach: https://github.com/hebasto/bitcoin/commit/335cc907b39a86dc9784b21468fdad292a2e7926?

Concept ACK.

This is a slight regression when building against boost < 1.91 because the bloated type signatures are reintroduced in that case...

To clarify, this only affects compile times, right?

I doubt it really affects compile times much, it's mostly just cosmetic, as the types are much easier to deal with for compiler warnings or debugger output.

Take mapTx's destructor for an example:

boost::multi_index::multi_index_container<CTxMemPoolEntry, CTxMemPool::CTxMemPoolEntry_Indices, std::allocator<CTxMemPoolEntry> >::~multi_index_container()

vs.

boost::multi_index::multi_index_container<CTxMemPoolEntry, boost::multi_index::indexed_by<boost::multi_index::hashed_unique<mempoolentry_txid, SaltedTxidHasher, mpl_::na, mpl_::na>, boost::multi_index::hashed_unique<boost::multi_index::tag<index_by_wtxid, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, mempoolentry_wtxid, SaltedWtxidHasher, mpl_::na>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<entry_time, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByEntryTime>, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, std::allocator<CTxMemPoolEntry> >::~multi_index_container()

Size-wise, the bloated types cause an unstripped bitcoind to be 594,600 bytes larger. Stripped, they're no different. So the release binaries are the same either way.

What about an alternative approach: hebasto@335cc90?

This is the same as what's here, just with aliases as syntactic sugar. It still produces the bloated types when using <1.91. We wouldn't use those aliases anywhere else, so I don't see any use in creating them. I'm not opposed if there's a preference for it, though.

Edit: with boost 1.91, it looks like: boost::multi_index::multi_index_container<CTxMemPoolEntry, boost::multi_index::indexed_by<boost::multi_index::hashed_unique<mempoolentry_txid, SaltedTxidHasher, void, void>, boost::multi_index::hashed_unique<boost::multi_index::tag<index_by_wtxid>, mempoole ntry_wtxid, SaltedWtxidHasher, void>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<entry_time>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByEntryTime> >, std::allocator<CTxMemPoolEntry> >::~multi_index_container().

Size-wise, the bloated types cause an unstripped bitcoind to be 594,600 bytes larger. Stripped, they're no different. So the release binaries are the same either way.

So the following bumping Boost to 1.91 in depends should also be backported to all supported release branches.

ACK 0bc9d354dfd8074d1c36a891a69b6585a8775c65.

So the following bumping Boost to 1.91 in depends should also be backported to all supported release branches.

Why's that? We can just ship the slightly larger debug symbols.

review ACK 0bc9d354dfd8074d1c36a891a69b6585a8775c65 🎶

<details><summary>Show signature</summary>

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 0bc9d354dfd8074d1c36a891a69b6585a8775c65 🎶
hpCEc+K7VumJwa/or1iDc5ztW73i+VaVF3CE/2iG452HREFjkWchTE6DvGbujbS1zka4+PUXIodZvvu5V9pGBw==

</details>

ACK 0bc9d354dfd8074d1c36a891a69b6585a8775c65

Backported to 31.x in #35046.

Size-wise, the bloated types cause an unstripped bitcoind to be 594,600 bytes larger. Stripped, they're no different. So the release binaries are the same either way.

So the following bumping Boost to 1.91 in depends should also be backported to all supported release branches.

Ah, sorry, the way I described that was confusing. I meant that when stripped, there's no difference between the two approaches. So it's irrelevant for release binaries.

Thinking about this though, I think we actually want to avoid bumping boost for any backports, because (starting with v1.91), that would mean backporting a boost::signals implementation that was never seen/tested in master.

Backported to 30.x in #35083.

Backported to 29.x in #34855.

Backported to 28.x in #35214.