Concept ACK

It would be nice to implement 81638f5 differently, without adding the <chain.h> dependency to all index headers. This dependency provides access to node internal types that indexes running on other processes (in the future) will not know about.

It seems we all implemented the same "index X requires block data" and "index Y requires block undo data" in slightly different ways. I did it in #26966 some time ago, and ryanofsky also did it in #24230.

My preference would be to follow #24230 and use the custom options class. It is more flexible than introducing a new method to override on all/most classes every time a new index customization is added.

It would be nice to implement 81638f5 differently, without adding the <chain.h> dependency to all index headers. This dependency provides access to node internal types that indexes running on other processes (in the future) will not know about.

Should not be too complicated to get rid of the dependency. I will draft something, maybe I'll just give @maflcko s suggestion a try.

maflcko's suggestion wouldn't remove the dependency. Unless you place the new enum somewhere else?. Which I don't think it worth it.

It seems we all implemented the same "index X requires block data" and "index Y requires block undo data" in slightly different ways. I did it in #26966 some time ago, and ryanofsky also did it in #24230.
My preference would be to follow #24230 and use the custom options class. It is more flexible than introducing a new method to override on all/most classes every time a new index customization is added.

I'm wouldn't call having helper functions returning bools for each case more flexible than what we have here. At least this is what I saw in #24230 and I think you mean:

virtual bool RequiresBlockUndoData() const { return false; }

It may be more readable in the short term but if we build more complicated stuff beyond just checking undo data I think this will be complicated. And we already use the mask everywhere so why not build on this? I like being able to grep for the blockstatus enum values too in order to see where block data is checked in various ways, that would be also lost.

If we agree that the final goal of the indexes is to run in isolation, in a standalone process, and be the first consumers of the kernel library, then linking them to chain internal fields like the CBlockIndex status mask isn't the way to go. These objects will live only within the kernel process.

Also, I don't think the grepping argument is valid in most circumstances. It can be used anywhere to break layer distinctions. For example, what if the GUI needs to know if a certain block is available on disk in the future. Would you add the status masks enum dependency to a widget class? Doing so would break the current structure: views -> model/interfaces -> kernel (this is also done this way to allow the GUI to run in a standalone process as well).

The RequiresBlockUndoData() implementation is from my PR, and I'm not fan of it anymore (I don't dislike it, just prefer a different approach).
I think #24230 approach is better as it introduces a new base class method to override called CustomOptions() that returns a struct containing index's specific information. I think that building upon this would be cleaner and more flexible, as we would only need to add fields to a struct instead of changing the base class interface with each new option. - would probably change the struct name, which is currently called NotifyOptions -.

But the index still needs to have an understanding of what block data is and what it needs in order to decide if it can handle what the the kernel gives it. So I don't think it can be avoided that the index has knowledge of the block status so this would need to be shared. That doesn't mean that they can't run isolation, but the kernel needs to share this knowledge with them. We can only avoid this if we forget about the concept suggested here and let the index ask for data from the kernel until it hits something unexpected and fails or we basically reimplement the block status as a list of bools in the index which will be much harder to reason about and maintain. I don't see how an options object prevents that.

We both agree that indexes need to share their sync data requirements in some way. Perhaps we are not understanding each other because your rationale is based on the current sync mechanism, while I am considering a future version that is no longer at the index base class.

An index running in a standalone process would only sync through kernel signals. It will register with the kernel, providing its last known block hash and the data and events it wants to receive and listen to. Then, it will only react to them. It will no longer request data directly from the kernel as it does currently.

The kernel, running in a different process, will emit the signals required to sync the registered index, just as it does for other listeners like the wallet (no difference between them). It will provide the BlockInfo struct, which may or may not contain specific data, such as block undo data during connection/disconnection and other information if requested during registration.

This is why using an options struct instead of creating a method for each index sync requirement is more flexible to me. The index will provide this struct during registration to the kernel running in a different process and then forget about it. Then, if any event arrives without the requested data, the index will abort its execution.

Moreover, even if you prefer the multi-overridden-methods approach instead of the options struct (which is how I implemented this in #26966), I don't think accessing the uint32_t block index status bit flags field helps much in terms of reasoning about the code or maintenance. People working on upper layers like the indexes, the wallet, or the GUI should focus on receiving the data they expect and should not have to worry/learn about how the block verification status is mapped in memory.

As a quick sanity check, I rebuilt -coinstatsindex and checked gettxoutsetinfo still gives me the same stats at block 840,000.