The ChainstateManager never uses the snapshot chainstate in anything unrelated to tests; should we just take it out? #24870

While working around on #24760, trying to move things around and centralize BlockIndex loading from DB for testing, I stumbled upon m_snapshot_chainstate, which seems to never be used at any stage in the code at all. I would like to advocate for removing it (which I'm happy to give a shot) because it just complicates things. I guess it's a relic from something in the past (?).

Or... maybe I'm missing what it's used for?

From my analysis, here's how m_snapshot_chainstate can get assigned (excluding tests):

• With ChainstateManager::InitializeChainstate(), but that's only called in LoadChainstate() (which is called in init.cpp), which keeps the second parameter std::nullopt, the default, which means it never gets assigned.
• With ChainstateManager::ActivateSnapshot(), which is never called anywhere

Maybe I'm missing something... I'm not sure. But if it's not used anywhere, I would like to advocate for removing it to make ChainState management monolithic and simple.

And just between us :-) , while I was working with #24760, I saw that with the pointers that switch between states and I was frustrated that I left the whole thing for a week, until I came back now and I'm happy to see that only one ChainState is being used. Simple is better.

Let me know whether you guys support such a move. I think this is doable in one day I can put a PR for it today or tomorrow.

See #15605

Three years... wow.

This complicates things by an order of magnitude for moving away from excessive memory dependence. From the looks of it, we have two (multiple?) CChain objects that should go to DB with different prefixes defined at construction.

I'll close this issue since it's pointless to discuss removing stuff if there's no consensus it's not needed.