blockchain sync: reduce disk writes from 2 to 1 per tx

[jeffro256]

Summary

Pros:

• During sync, instead of performing 1 write, then 1 read, then one write for each tx in the chain, we just write once. This increases the lifespan of the disk and speeds up badly buffered / not buffered I/O. On a newer NVME and with a Ryzen 9 3900X, blockchain sync was around 3-4% faster. Differences will be more pronounced for systems bottle-necked by disk speed.
• This PR is backwards compatible to receive NOTIFY_NEW_BLOCK commands, but the code paths between handle_notify_new_block and handle_notify_new_fluffy_block are merged for less code surface and review time.

Cons:

• Complicated review

Hopefully this will move monerod towards being slightly more workable for hard drives in the future.

Design

New: cryptonote::ver_non_input_consensus()

I have created a function cryptonote::ver_non_input_consensus() in tx_verification_utils that checks all consensus rules for a group of transactions besides the checks in Blockchain::check_tx_inputs(). For Blockchain::handle_block_to_main_chain, this is the condition that txs must satisfy before being attempted to be checked for inputs and added to blocks. This function is the most important component that MUST be correct or otherwise chain splits / inflation could occur. To audit the correctness of this function, start at the function cryptonote::core::handle_incoming_txs() in the old code and step through of the rules checked until the end of the function cryptonote::tx_memory_pool::add_tx(). cryptonote::ver_non_input_consensus() should cover all of those rules.

Modified: core::handle_incoming_tx[s]()

Before, cryptonote::core::handle_incoming_txs() was responsible for parsing all txs (inside blocks and for pool), checking their semantics, passing those txs to the mempool, and notifying ZMQ. Now, cryptonote::core::handle_incoming_txs() is deleted and there is only cryptonote::core::handle_incoming_tx(). cryptonote::core::handle_incoming_tx() is now basically just a wrapper around tx_memory_pool::add_tx(), additionally triggering ZMQ events, and is only called for new transaction notifications from the protocol handler (not block downloads).

Modified: tx_memory_pool::add_tx()

All of the consensus checks besides Blockchain::check_tx_inputs() inside of add_tx() were removed and replaced with a call to cryptonote::ver_non_input_consensus(). The relay checks remain the same.

Modified: Blockchain::add_block()

add_block() now takes a structure called a "pool supplement" which is simply a map of TXIDs to their corresponding cryptonote::transaction and transaction blob. When handle_block_to_main_chain attempts to take transactions from the transaction pool to add a new block, if that fails, then it falls back on taking txs from the pool supplement. The pool supplement has all the non-input consensus rules checked after the PoW check is done. If the block ends up getting handled in Blockchain::handle_alternative_block, then the pool supplement transactions are added to the tx_memory_pool after their respective alt PoW checks.

Modified: t_cryptonote_protocol_handler::handle_notify_new_fluffy_block()

The main difference with this function now is that we construct a pool supplement and pass that to core::handle_incoming_block() instead of calling core::handle_incoming_txs() to add everything to the mempool first.

Modified: t_cryptonote_protocol_handler::try_add_next_blocks()

The changes are very similar to the changes made to handle_notify_new_fluffy_block.

Modified: t_cryptonote_protocol_handler::handle_notify_new_block()

Before, this function has separate handling logic, but now we just convert the NOTIFY_NEW_BLOCK request into a NOTIFY_NEW_FLUFFY_BLOCK request and call handle_notify_new_block with it. This saves us having to make the same changes to both code paths.

[jeffro256]

I'm thinking about having core::handle_incoming_txs basically do nothing except pass the tx to tx_memory_pool::add_tx, which then passes the transaction through the verify_pool_supplement tests. This would make the code so much more robust against future discrepancy between changes to the pool rules and the verify_pool_supplement rules, but it would require some more refactoring.

[0xFFFC0000]

New performance results: the performance problem of pop_blocks specifically related to this PR has been fixed in the new push. The only remaining part is we still have a little bit of performance drop for sync operation. I am attaching the file in case anyone wants to check it.

results-2500blocks-5iter-v2.txt

[jeffro256]

I think I've found a reason why the sync time of this PR looks slower than the sync time of master that test script: between the call to pop_blocks and flush_txpool, which is several seconds in some cases, the master node can use the popped txs already inside the mempool to skip most the checks (especially Blockchain::check_tx_inputs) before validating a block which gives it a significant boost. This state won't happen organically during normal sync, so this test script doesn't quite capture the normal behavior during sync when you didn't already have the txs in the mempool.

To fix the script, instead of doing:

1. pop_blocks
2. flush_txpool
3. Wait for sync

You could do:

1. Start monerod offline
2. pop_blocks
3. flush_txpool
4. stop_daemon
5. Start monerod online
6. Wait for sync

This does have the downside of including the start-up time as the sync time, and the choice of peers on new instance may affect the speed at which it syncs, but you could minimize these effects by increasing the number of popped blocks.

[jeffro256]

Okay @vtnerd the last commits should hopefully handle ZMQ tx notifications better. We only notify A) when an incoming relayed transaction is new and added to the pool, B) a tx from a pool supplement was used to add a block, or C) an alt block contained a new tx and it was added to the pool.

[selsta]

I applied this pull request locally and comments 5 commit is missing... not sure what's going on

[jeffro256]

Rebased to fix conflicts on CORE_RPC_VERSION_MINOR definition