Add batch block processing result observer to block_processor

[clemahieu]

• Implements the batch block processing observer from Add the ascending bootstrap client #4158.
• Unifies notifications between block_processor observers 'processed' and 'batch_processed'.
• New observer template which puts functionality in to its own file with minimal coupling.
• This will be merge-committed to preserve functional change boundaries.

node::process_local cleanup

• Can now block without needing to directly call ::process_one. Now it will be placed in to the block_processor queue.
• Doesn't need to pass block_origin through process_one to process_live.
• Return an optional since it may not be possible to return a result if the block_processor stops.

Block processor cleanup

• Remove block_processor::wait_write workaround which was in effect trying to schedule database batches for node::process_local
• Removes nano::block_origin as unneeded when replaced with observer
• ::process_one is now fully encapsulated

block_processor::process_live cleanup

• Removes block_origin as a parameter and uses a block_processor observer instead to do block publishing.
• Removes unused parameters
• Connect takes an enable flag which can be directly pulled from the node flags.

[pwojcikdev]

Not sure about the way block origin is removed. You are trying to split data into multiple containers that naturally wants to be wrapped in a structure and move through the pipeline. It is more error prone and unnecessary, and in few places that it happens (election related code) it's a pain to work with. It's more or less the same workaround as with block arrival class, instead of tracking the source of each block (local/live/bootsrap) next to the block and then choosing broadcasting strategy based on that, we try to synchronise state across multiple components.

[clemahieu]

Not sure about the way block origin is removed. You are trying to split data into multiple containers that naturally wants to be wrapped in a structure and move through the pipeline. It is more error prone and unnecessary, and in few places that it happens (election related code) it's a pain to work with. It's more or less the same workaround as with block arrival class, instead of tracking the source of each block (local/live/bootsrap) next to the block and then choosing broadcasting strategy based on that, we try to synchronise state across multiple components.

I can see the reasoning for that and I think we'd be able to do something like that with the block_pipeline::context when merged. https://github.com/nanocurrency/nano-node/blob/6299819eaeaafcd302ca42170e6532de9272df6f/nano/node/block_pipeline/context.hpp

[pwojcikdev]

Not sure about the way block origin is removed. You are trying to split data into multiple containers that naturally wants to be wrapped in a structure and move through the pipeline. It is more error prone and unnecessary, and in few places that it happens (election related code) it's a pain to work with. It's more or less the same workaround as with block arrival class, instead of tracking the source of each block (local/live/bootsrap) next to the block and then choosing broadcasting strategy based on that, we try to synchronise state across multiple components.

I can see the reasoning for that and I think we'd be able to do something like that with the block_pipeline::context when merged. https://github.com/nanocurrency/nano-node/blob/6299819eaeaafcd302ca42170e6532de9272df6f/nano/node/block_pipeline/context.hpp

That looks like a perfect fit. The current solution is OK as a temporary measure until we get the block pipeline merged.