This purges epee::critical_region/epee::critical_section and the awful
CRITICAL_REGION_LOCAL and CRITICAL_REGION_LOCAL1 and
CRITICAL_REGION_BEGIN1 and all that crap from epee code.
This wrapper class around a mutex is just painful, macro-infested
indirection that accomplishes nothing (and, worse, forces all using code
to use a std::recursive_mutex even when a different mutex type is more
appropriate).
This commit purges it, replacing the "critical_section" mutex wrappers
with either std::mutex, std::recursive_mutex, or std::shared_mutex as
appropriate. I kept anything that looked uncertain as a
recursive_mutex, simple cases that obviously don't recurse as
std::mutex, and simple cases with reader/writing mechanics as a
shared_mutex.
Ideally all the recursive_mutexes should be eliminated because a
recursive_mutex is almost always a design flaw where someone has let the
locking code get impossibly tangled, but that requires a lot more time
to properly trace down all the ways the mutexes are used.
Other notable changes:
- There was one NIH promise/future-like class here that was used in
example one place in p2p/net_node; I replaced it with a
std::promise/future.
- moved the mutex for LMDB resizing into LMDB itself; having it in the
abstract base class is bad design, and also made it impossible to make a
moveable base class (which gets used for the fake db classes in the test
code).
common/util.h has become something of a dumping ground of random
functions. This splits them up a little by moving the filesystem bits
to common/file.h, the sha256sum functions to common/sha256sum.h, and the
(singleton) signal handler to common/signal_handler.h.
A huge amount of this is repetitive:
- `boost::get<T>(variant)` becomes `std::get<T>(variant)`
- `boost::get<T>(variant_ptr)` becomes `std::get_if<T>(variant_ptr)`
- `variant.type() == typeid(T)` becomes `std::holds_alternative<T>(variant)`
There are also some simplifications to visitors using simpler stl
visitors, or (simpler still) generic lambdas as visitors.
Also adds boost serialization serializers for std::variant and
std::optional.
* recalculate_difficulty: Batch work, avoid LMDB paging error
F Uncaught exception! Failed to get block info in recalculate difficulty: MDB_TXN_FULL: Transaction has too many dirty pages - transaction too big
* recaluculate_difficulty: Reduce batch to 10k, rewrite num_batches
Updating the checkpoint table in place, something must have been done
incorrectly or some bug, such that querying MDB_LAST on the checkpoint
table returns not the latest expected checkpoint.
Pulling out all the old checkpoints, generating a new table and
reinserting them resolves this.
Also import boost::endian namespace because the boost::endian's
throughout the file were needlessly verbose (the little_to_native calls
are already obvious).
This renames mdb_block_info_3 to mdb_block_info so that _1 and _2 are
the old versions (you can't predeclare a typedef), and use it rather
than a void pointer in the 64bit block value fetcher.
This extracts uptime proof data entirely from service node states,
instead storing (some) proof data as its own first-class object in the
code and backed by the database. We now persistently store:
- timestamp
- version
- ip & ports
- ed25519 pubkey
and update it every time a proof is received. Upon restart, we load all
the proofs from the database, which means we no longer lose last proof
received times on restart, and always have the most recently received ip
and port information (rather than only having whatever it was in the
most recently stored state, which could be ~12 blocks ago). It also
means we don't need to spam the network with proofs for up to half an
hour after a restart because we now know when we last sent (and
received) our own proof before restarting.
This separation required some restructuring: most notably changing
`state_t` to be constructed with a `service_node_list` pointer, which it
needs both directly and for BlockchainDB access. Related to this is
also eliminating the separate BlockchainDB pointer stored in
service_node_list in favour of just accessing it via m_blockchain (which
now has a new `has_db()` method to detect when it has been initialized).
Various code improvements to LMDB code (no logic changes):
- compile less code by moving the cursor macros (which are used often)
into local functions
- removes the TXN_POSTFIX_RDONLY macro because it does exactly nothing
- remove useless `inline` from various functions in C++ only has one
purpose: to tell the compiler that it's okay to have multiple
definitions (and just throw the extra away when linking). It is not
used by any modern C++ compiler to actually decide when to inline, and
so has no use in a .cpp file. (Suspect this was written by a C
programmer who doesn't fully understand C++).
- Remove the C `typedef struct blah { ... } blah;` idiom which has no
purpose in C++.
- Use inheritance on the nearly-duplicate `mdb_block_info_{1,2,3}
structs rather than repeating all the field in each one. This also
removed a completely unneeded offsetof into them that was being used
to avoid having to learn C++. (Also adds a static_assert in case
anyone ever changes some contained type and breaks things).
- Fix bi_weight comment about why it is really uin64_t
- Add a missing method `override`.
- Replace "22" buried in the code with a constant indicating what it is
for. (Oh, reader, you wanted to know what it's used for? It's the
number of different databases allowed in the LMDB)
- Move `m_cur_whatever` macros into the .cpp file, and avoid using them
in any code we've added in loki. These are incredibly useless
macros: they are basically just defined to slightly shorten a name.
I'm guessing that, at some point, these members got moved and rather
than fix the remaining code someone defined a bunch of macros to deal
with the rename. Yuck.
(Getting rid of *all* of them (i.e. replacing all `m_cur_whatever`
with `m_cursors->whatever` would be preferable, but would make future
merge conflicts more painful, so I left the macros as is for the
upstream databases but removed our service_node_data one.)
- Rename `mdb_txn_cursors` members to drop the leading `m_txc_` prefix.
When the struct is nothing more than a POD collection of values the
prefix makes no sense at all. This allows us the write
`m_cursors->service_node_data` instead of
`m_cursors->m_txc_service_node_data` (or the horrible macro, above,
which seems designed to just macro around this naming horror).
This allows fetching multiple tx heights in a single tx rather than
needing to call multiple times and incur the overhead of a transaction.
This devirtualizes the singular version and reimplements it as a simple
wrapper around the vector version.
These structures will always end up packed anyway (everything in them is
8-byte aligned and a multiple of 8 bytes), but the pack pragma also
changes their alignment which isn't good (and causes warnings). This
just static_asserts that they are padding-free instead, which is the
intention.
If the peer (whether pruned or not itself) supports sending pruned blocks
to syncing nodes, the pruned version will be sent along with the hash
of the pruned data and the block weight. The original tx hashes can be
reconstructed from the pruned txes and theur prunable data hash. Those
hashes and the block weights are hashes and checked against the set of
precompiled hashes, ensuring the data we received is the original data.
It is currently not possible to use this system when not using the set
of precompiled hashes, since block weights can not otherwise be checked
for validity.
This is off by default for now, and is enabled by --sync-pruned-blocks
Silences:
src/blockchain_db/lmdb/db_lmdb.cpp: In member function ‘virtual void cryptonote::BlockchainLMDB::open(const string&, cryptonote::network_type, int)’:
src/blockchain_db/lmdb/db_lmdb.cpp:1473:63: error: type qualifiers ignored on cast result type [-Werror=ignored-qualifiers]
1473 | if (db_version > static_cast<decltype(db_version)>(VERSION))
| ^
src/blockchain_db/lmdb/db_lmdb.cpp:1480:68: error: type qualifiers ignored on cast result type [-Werror=ignored-qualifiers]
1480 | else if (db_version < static_cast<decltype(db_version)>(VERSION))
| ^
which happens because the `decltype()` here is const, and gcc apparently
now apparently warns on a cast to a const cast result.
While here I also noticed and fixed what looks like an unaligned access
that could be a potential problem on non-x86 architectures.
Add migratory code for new alt_block_data_t.checkpointed field
Code review
Redo switch to alt chain logic to handle PoW properly
We always switch to the chain with prevailing checkpoints. If an old
chain reappears with more checkpoints it will be rebroadcasted and at
that point we will switch over then. So here we restore the old
behaviour regarding keeping alt chains around or not depending on the
scenario that caused the chain switch
