mentioned in: bitcoin#34749

"> Your bitcoin#34748 did not remove the struct or introduce unnecessary serialization like this does PR though. I agree the code can be improved in other ways (my suggested diff bitcoin#34749 (comment)).

Isn't your suggested-diff proving exactly the main point of this PR? The condition is inverted.

+ if (missing_from_mempool && !mempool_only && g_txospenderindex && g_txospenderindex->BlockUntilSyncedToCurrentChain()) {
+    throw JSONRPCError(RPC_MISC_ERROR, "Mempool lacks a relevant spend, and txospenderindex is unavailable.");
+ }

Here missing_from_mempool means "there is more work to do", !mempool_only means "we are not restricted only to mempool search" and g_txospenderindex && g_txospenderindex->BlockUntilSyncedToCurrentChain() means index synced.

So your suggestion throws the exception when there is more work to do, we are not restricted to the mempool search, and the index is sync. Which is the opposite we want.

I think we can agree the code is not easy to read and maintain in the current form and even in your patch-set (which I agree that is better than the current form but not enough in my opinion).

Also, I dropped my PR for this one because I found the base structure proposed by @fjahr better. It separates concerns cleanly, to the point of being very easy to describe, which makes maintenance easier. Essentially, the first part resolves what the mempool can answer, returns early if done, then falls through to the index.

Note: All of this doesn't mean we shouldn't improve the hash re-serialization you are concerned about. We can do that as well. I think it is an orthogonal improvement we can make.

Please don't take the following badly, this is merely a general thought I have had for a while.

It feels like we are sometimes applying the same review bar we have for validation or consensus code to other, less critical parts of the codebase, like indexes. And I don't think that is always the right call. Some areas would be better served by being a bit more relaxed and letting the code move forward to a better place, rather than going back and forth for changes that are unlikely to cause any real harm and can get improved in a follow-up.

I mean, areas like the indexes have not been very popular in recent years, and when it takes so much time and effort to get improvements in, it makes maintenance and writing more robust code really tedious for anyone working there (I'm saying this based on experience bitcoin#26966). At some point this just drives people away from these areas, and that is sad to see." - furszy