LOCAL_ASSERT was only defined on Windows,
only used in the `CHECK_AND_ASSERT` macro,
and was disabled in unit tests anyways,
but only in the main translation unit,
which didn't have local asserts,
since get_set_enable_assert was inline,
so the disabling didn't even do anything.
1. Doesn't allocate a `boost::shared_ptr`
2. Doesn't use a vtable
3. Doesn't use `std::function` wrapper
4. Doesn't necessarily copy functional object
5. Doesn't need a factory function for simple declaration (yay CTAD)
6. Isolated to its own header
7. Name doesn't suck as much
One theoretically could have crafted a "long amount" (pre-v10) RingCT transaction
with non-0 padding bytes in the decoded amount which fails receiver scanning, but
passes third-party wallet proof checking.
- Make sure the server sends a complete response when the client
includes the "Connection: close" header.
- Make sure the server terminates in `m_strand` to avoid
concurrent socket closure and ops processing.
This replaces `ver_rct_non_semantics_simple_cached()` with an API that offloads
the responsibility of tracking input verification successes to the caller. The
main caller of this function in the codebase, `cryptonote::Blockchain()` instead
keeps track of the verification results for transaction in the mempool by
storing a "verification ID" in the mempool metadata table (with `txpool_tx_meta_t`).
This has several benefits, including:
* When the mempool is large (>8192 txs), we no longer experience cache misses and unnecessarily re-verify ring signatures. This greatly improves block propagation time for FCMP++ blocks under load
* For the same reason, reorg handling can be sped up by storing verification IDs of transactions popped from the chain
* Speeds up re-validating every mempool transaction on fork change (monerod revalidates the whole tx-pool on HFs #10142)
* Caches results for every single type of Monero transaction, not just latest RCT type
* Cache persists over a node restart
* Uses 512KiB less RAM (8192*2*32B)
* No additional storage or DB migration required since `txpool_tx_meta_t` already had padding allocated
* Moves more verification logic out of `cryptonote::Blockchain`
Furthermore, this opens the door to future multi-threaded block verification
speed-ups. Right now, transactions' input proof verification is limited to one
transaction at a time. However, one can imagine a scenario with verification IDs
where input proofs are optimistically multi-threaded in advance of block
processing. Then, even though ring member fetching and verification is
single-threaded inside of `cryptonote::Blockchain::check_tx_inputs()`, the
single thread can skip the CPU-intensive cryptographic code if the verification
ID allows it.
Also changes the default log category in `tx_verification_utils.cpp` from "blockchain" to "verify".
Without this commit:
1) read height from DB
2) add block to chain in separate thread
3) read chain for block id's and request them from peer
4) ERR in handle_response_chain_entry, peer's first block is the
one that was added to the chain, which has block idx=height from
step 1.
This commit reads the chain for height and highest block id's
in one go while holding the m_blockchain_lock to avoid the race.