For some obnoxious reason this code passes a bunch of what should be
constructor arguments as template arguments, which makes them compile
time constants and thus induces an impossible condition here. Guard it
behind an `if constexpr` to silence the warning.
- Replaces the wallet RPC classes with ones like the core RPC server
(with serialization code moved into a new .cpp file).
- Restricted commands are now carried through the RPC serialization
types (by inheriting from RESTRICTED instead of RPC_COMMAND) and
restrictions are handled in one place rather than being handled in
each of the 49 restricted endpoints. This differs a little from how
the core http server works (which has a PUBLIC base class) because
for the wallet rpc server unrestricted really doesn't mean "public",
it means something closer to view-only.
- GET_TRANSFERS_CSV is now restricted (it looks like an oversight that
it wasn't before since GET_TRANSFERS is restricted)
- GET_ADDRESS_BOOK_ENTRY is now restricted. Since restricted mode is
meant to provide something like view-only access, it doesn't make
much sense that address book entries were available.
- Use uWebSockets to provide the wallet RPC server HTTP functionality.
This version is quite a bit simpler than the core RPC version since it
doesn't support multithreaded (parallel) requests, and so we don't
have to worry about queuing jobs.
- Converted all the numeric wallet rpc error codes defines to constexprs
- Changed how endpoints get called; previous this was called:
bool on_some_endpoint(const wallet_rpc::COMMAND_RPC_SOME_ENDPOINT::request& req, wallet_rpc::COMMAND_RPC_SOME_ENDPOINT::response& res, epee::json_rpc::error& er, const connection_context *ctx = NULL)
This PR changes it similarly to how core_rpc_server's endpoints work:
wallet_rpc::SOME_ENDPOINT invoke(wallet_rpc::COMMAND_RPC_SOME_ENDPOINT::request&& req);
That is:
- the response is now returned
- the request is provided by mutable rvalue reference
- the error object is gone (just throw instead)
- the connection_context is gone (it was not used at all by any wallet
rpc endpoint).
- Consolidated most of the (identical) exception handling to the RPC
method invocation point rather than needing to repeat it in each
individual endpoint. This means each endpoint's `invoke` method can
now just throw (or not catch) exceptions. Some try/catches are still
there because they are converting one type of exception into another,
but the generic ones that return a generic error are gone.
- Removed all remaining epee http code.
- DRYed out some wallet rpc code.
In short: epee's http client is garbage, standard violating, and
unreliable.
This completely removes the epee http client support and replaces it
with cpr, a curl-based C++ wrapper. rpc/http_client.h wraps cpr for RPC
requests specifically, but it is also usable directly.
This replacement has a number of advantages:
- requests are considerably more reliable. The epee http client code
assumes that a connection will be kept alive forever, and returns a
failure if a connection is ever closed. This results in some very
annoying things: for example, preparing a transaction and then waiting
a long tim before confirming it will usually result in an error
communication with the daemon. This is just terribly behaviour: the
right thing to do on a connection failure is to resubmit the request.
- epee's http client is broken in lots of other ways: for example, it
tries throwing SSL at the port to see if it is HTTPS, but this is
protocol violating and just breaks (with a several second timeout) on
anything that *isn't* epee http server (for example, when lokid is
behind a proxying server).
- even when it isn't doing the above, the client breaks in other ways:
for example, there is a comment (replaced in this PR) in the Trezor PR
code that forces a connection close after every request because epee's
http client doesn't do proper keep-alive request handling.
- it seems noticeably faster to me in practical use in this PR; both
simple requests (for example, when running `lokid status`) and
wallet<->daemon connections are faster, probably because of crappy
code in epee. (I think this is also related to the throw-ssl-at-it
junk above: the epee client always generates an ssl certificate during
static initialization because it might need one at some point).
- significantly reduces the amount of code we have to maintain.
- removes all the epee ssl option code: curl can handle all of that just
fine.
- removes the epee socks proxy code; curl can handle that just fine.
(And can do more: it also supports using HTTP/HTTPS proxies).
- When a cli wallet connection fails we know show why it failed (which
now is an error message from curl), which could have all sorts of
reasons like hostname resolution failure, bad ssl certificate, etc.
Previously you just got a useless generic error that tells you
nothing.
Other related changes in this PR:
- Drops the check-for-update and download-update code. To the best of
my knowledge these have never been supported in loki-core and so it
didn't seem worth the trouble to convert them to use cpr for the
requests.
- Cleaned up node_rpc_proxy return values: there was an inconsistent mix
of ways to return errors and how the returned strings were handled.
Instead this cleans it up to return a pair<bool, val>, which (with
C++17) can be transparently captured as:
auto [success, val] = node.whatever(req);
This drops the failure message string, but it was almost always set to
something fairly useless (if we want to resurrect it we could easily
change the first element to be a custom type with a bool operator for
success, and a `.error` attribute containing some error string, but
for the most part the current code wasn't doing much useful with the
failure string).
- changed local detection (for automatic trusted daemon determination)
to just look for localhost, and to not try to resolve anything.
Trusting non-public IPs does not work well (e.g. with lokinet where
all .loki addresses resolve to a local IP).
- ssl fingerprint option is removed; this isn't supported by curl
(because it is essentially just duplicating what a custom cainfo
bundle does)
- --daemon-ssl-allow-chained is removed; it wasn't a useful option (if
you don't want chaining, don't specify a cainfo chain).
- --daemon-address is now a URL instead of just host:port. (If you omit
the protocol, http:// is prepended).
- --daemon-host and --daemon-port are now deprecated and produce a
warning (in simplewallet) if used; the replacement is to use
--daemon-address.
- --daemon-ssl is deprecated; specify --daemon-address=https://whatever
instead.
- the above three are now hidden from --help
- reordered the wallet connection options to make more logical sense.
This replaces the NIH epee http server which does not work all that well
with an external C++ library called uWebSockets. Fundamentally this
gives the following advantages:
- Much less code to maintain
- Just one thread for handling HTTP connections versus epee's pool of
threads
- Uses existing LokiMQ job server and existing thread pool for handling
the actual tasks; they are processed/scheduled in the same "rpc" or
"admin" queues as lokimq rpc calls. One notable benefit is that "admin"
rpc commands get their own queue (and thus cannot be delayed by long rpc
commands). Currently the lokimq threads and the http rpc thread pool
and the p2p thread pool and the job queue thread pool and the dns lookup
thread pool and... are *all* different thread pools; this is a step
towards consolidating them.
- Very little mutex contention (which has been a major problem with epee
RPC in the past): there is one mutex (inside uWebSockets) for putting
responses back into the thread managing the connection; everything
internally gets handled through (lock-free) lokimq inproc sockets.
- Faster RPC performance on average, and much better worst case
performance. Epee's http interface seems to have some race condition
that ocassionally stalls a request (even a very simple one) for a dozen
or more seconds for no good reason.
- Long polling gets redone here to no longer need threads; instead we
just store the request and respond when the thread pool, or else in a
timer (that runs once/second) for timing out long polls.
---
The basic idea of how this works from a high level:
We launch a single thread to handle HTTP RPC requests and response data.
This uWebSockets thread is essentially running an event loop: it never
actually handles any logic; it only serves to shuttle data that arrives
in a request to some other thread, and then, at some later point, to
send some reply back to that waiting connection. Everything is
asynchronous and non-blocking here: the basic uWebSockets event loop
just operates as things arrive, passes it off immediately, and goes back
to waiting for the next thing to arrive.
The basic flow is like this:
0. uWS thread -- listens on localhost:22023
1. uWS thread -- incoming request on localhost:22023
2. uWS thread -- fires callback, which injects the task into the LokiMQ job queue
3. LMQ main loop -- schedules it as an RPC job
4. LMQ rpc thread -- Some LokiMQ thread runs it, gets the result
5. LMQ rpc thread -- Result gets queued up for the uWS thread
6. uWS thread -- takes the request and starts sending it
(asynchronously) back to the requestor.
In more detail:
uWebSockets has registered has registered handlers for non-jsonrpc
requests (legacy JSON or binary). If the port is restricted then admin
commands get mapped to a "Access denied" response handler, otherwise
public commands (and admin commands on an unrestricted port) go to the
rpc command handler.
POST requests to /json_rpc have their own handler; this is a little
different than the above because it has to parse the request before it
can determine whether it is allowed or not, but once this is done it
continues roughly the same as legacy/binary requests.
uWebSockets then listens on the given IP/port for new incoming requests,
and starts listening for requests in a thread (we own this thread).
When a request arrives, it fires the event handler for that request.
(This may happen multiple times, if the client is sending a bunch of
data in a POST request). Once we have the full request, we then queue
the job in LokiMQ, putting it in the "rpc" or "admin" command
categories. (The one practical different here is that "admin" is
configured to be allowed to start up its own thread if all other threads
are busy, while "rpc" commands are prioritized along with everything
else.) LokiMQ then schedules this, along with native LokiMQ "rpc." or
"admin." requests.
When a LMQ worker thread becomes available, the RPC command gets called
in it and runs. Whatever output it produces (or error message, if it
throws) then gets wrapped up in jsonrpc boilerplate (if necessary), and
delivered to the uWebSockets thread to be sent in reply to that request.
uWebSockets picks up the data and sends whatever it can without
blocking, then buffers whatever it couldn't send to be sent again in a
later event loop iteration once the requestor can accept more data.
(This part is outside lokid; we only have to give uWS the data and let
it worry about delivery).
---
PR specifics:
Things removed from this PR:
1. ssl settings; with this PR the HTTP RPC interface is plain-text. The
previous default generated a self-signed certificate for the server on
startup and then the client accepted any certificate. This is actually
*worse* than unencrypted because it is entirely MITM-readable and yet
might make people think that their RPC communication is encrypted, and
setting up actual certificates is difficult enough that I think most
people don't bother.
uWebSockets *does* support HTTPS, and we could glue the existing options
into it, but I'm not convinced it's worthwhile: it works much better to
put HTTPS in a front-end proxy holding the certificate that proxies
requests to the backend (which can then listen in restricted mode on
some localhost port). One reason this is better is that it is much
easier to reload and/or restart such a front-end server, while
certificate updates with lokid require a full restart. Another reason
is that you get an error page instead of a timeout if something is wrong
with the backend. Finally we also save having to generate a temporary
certificate on *every* lokid invocation.
2. HTTP Digest authentication. Digest authentication is obsolete (and
was already obsolete when it got added to Monero). HTTP-Digest was
originally an attempt to provide a password authentication mechanism
that does not leak the password in transit, but still required that the
server know the password. It only has marginal value against replay
attacks, and is made entirely obsolete by sending traffic over HTTPS
instead. No client out there supports Digest but *not* Basic auth, and
so given the limited usefulness it seems pointless to support more than
Basic auth for HTTP RPC login.
What's worse is that epee's HTTP Digest authentication is a terrible
implementation: it uses boost::spirit -- a recursive descent parser
meant for building complex language grammars -- just to parse a single
HTTP header for Digest auth. This is a big load of crap that should
never have been accepted upstream, and that we should get rid of (even
if we wanted to support Digest auth it takes less than 100 lines of code
to do it when *not* using a recursive descent parser).
Currently all the variables in here need to be added into every
compilation unit; C++17 inline constexpr avoids that by throwing away
the duplicates at link time, plus lets the compiler optimize things
away.
Also eliminates a side effect in `get_config()` that mutated the
variable (so that one `get_config()` would end up changing the data of a
previous `get_config()`). Instead `get_config()` is now always constant
and doesn't care about the hard fork version; instead there is a
convenience method to get the governance wallet_address which takes the
hf version.
This started out with a few string_view simplifications, but unwinding
the string_view calls resulted in quite a few changes.
- use constexpr string_view instead of macros for constants
- use constexpr ints for other macros instead of constants (to be
consistent with the string_view constexprs); also eliminated a couple of
unused variables.
- convert amount parsing to string_view, and rewrite it with comments to
be far less confusing. (Previously it was remove a "." from a string,
chopping 0s off the end then putting them back on which was hard to
follow).
- fixed some not-quite-right amount parsing unit tests
- replace a bunch of string code (including lots of code allocating new
strings with .substr on a string) with std::string_view.
- avoid using C functions for strings
- convert epee http uri conversion methods to string_view (and fix one
typoed func name "conver" -> "convert")
- Add a `tools::hex_to_type` that converts hex directly into a (simple)
type, with hex and length verification for the type, and use it to
eliminate a bunch of intermediate std::string conversions.
- Convert a bunch of error-prone raw pointer string appends into
more type-safe `tools::view_guts(val)` calls. (In particular view_guts
always gets the sizeof() correct, while the individual call could easily
put the wrong type in the `sizeof()`).
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).
- Adds a tools::trim that operates on a string_view to replace boost
string trim usage.
- Replaces some boost::split's with tools::split's
- updates vercmp to operate on string_views instead of c strings
The assignment side effect in this call wasn't happening in non-debug
builds (since `assert` is a macro). Removed the side effect, fixed the
code to build with the tx extra DRY change, and added the result check
separately in the assert.
- pessimizing move in wallet2 prevents copy ellision
- various for loops were creating copies (clang-10 now warns about
this). Mostly this is because they had the type wrong when looping
through a map: the iterator type of a `map<K, V>` is `pair<const K,
V>` not `pair<K, V>`. Replaced them with C++17:
for (const auto& [key, val] : var)
which is so much nicer.
- cryptonote::core did not have a virtual destructor, but had virtual
methods (causing both a warning, and likely a crash if we ever have
something inheriting from it held in a unique_ptr<core>).
- core() constructor still had explicit even though it lost the single
argument.
- test code class had a `final` destructor but wasn't marked final. (It
also has a billion superfluous `virtual` declarations but I left them
in place because it's just test code).
Add a multi-address lookup mode to the unbound lookup code and remove
the gross code in net_node.inl that creates one thread per address.
(And worse, tries to use boost thread interruption which cannot work at
all since any such interruption is voluntary and won't do anything to a
thread that is stalled waiting for a synchronous libunbound lookup).
The replacement has unbound create and manage a single thread for async
lookups then waits up to the given timeout for results.
Currently this code isn't actually used because our seed node DNS list
is empty, but this lets us eliminate a boost::thread dependency while
not removing the feature (also includes a test case).
byte_slice was an abomination that used more than 400 lines of code to
implement what can be implemented in about 20 lines of code with a
std::shared_ptr<std::string> and a std::string_view and a couple
convenience methods.
Much of the levin noise generation using it used it gratuitously: they
always allocate a new string, but then return that wrapped it in a
byte_slice abomination because... well, there's no reason at all except
apparently that the byte_slice author wanted to push byte_slice into
places it didn't belong at all (even if you accept the overbuilt
monstrosity). Those methods now take a string_view and return a string,
which is what they should have done in the first place.
Replaces reintroduced boost::chronos and posix_times with std::chrono.
Also rewrote the time printing parts of the performance_tests code to do
it better use std::chrono types instead of uint64_ts.
REGISTER_CALLBACK and REGISTER_CALLBACK_METHOD do *exactly the same
thing*, and both take a different and unnecessary argument. DRY it out
and make it use a lambda instead of boost::bind.
This removes some boost thread interruption code, but that code is
highly unlikely to have done anything: you cannot preemptively interrupt
a thread, and pretty much anywhere a thread would get stuck is in I/O,
where boost's (voluntary) thread interruption cannot follow.
It also removes thread attributes because they have really limited
usefulness. There *was* a reason to introduce them many years ago (when
musl libc ran into a stack limit problem with libunbound) but musl
increased the default stack size long since.
Some of the invalid bulletproof tests were relying on the broken
behaviour of tx serialization failure still producing a tx hash based on
a partially filled serialized blob. The code now checks for
serialization failures, and throws when trying to get a tx hash from a
tx that fails to serialize. (This isn't new behaviour: it's just that
some serialization failures didn't actually signal a failure properly).
Fix up the tests to use a random tx hash if such a serialization
exception occurs, so that we can still add the invalid txes to blocks
for the purposes of testing block/tx failures.
loki_tx_builder fails for this test: it creates unserializable txes
because `vin` is too large. This was previously "passing" the test for
completely wrong reasons: the test produces transactions that fail to
serialize (triggering a "invalid mg_ss2 size: have 2, expected 3"
failure) while the point of the test was to produce serializable, but
invalid, transactions.
The failure wasn't being detected because the `tx_to_blob(t)` call in
`cryptonote_format_utils.cpp` would return a *partially filled*
serialization blob in case of serialization failure, which was
incredibly broken. `calculate_transaction_hash()` then was perfectly
happy to proceed with hashing this broken blob.
`tx_to_blob(t)` now returns an empty string on such a serialization
failure, and this made the test start failing because
`calculate_transaction_hash()` now fails.
This improves `calculate_transaction_hash()` a little more to add
earlier and more direct detection of such a failure, and fixes the test
by reverting it to the original Monero tx builder test code, which
creates the intended serializable-but-invalid transactions.
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.
The tx extra getting/setting API is a little non-DRY, so this dries it
out with some small changes:
- Added get_field_from_tx_extra that takes a specific tx_extra lvalue
reference and fetches it. This eliminates a bunch of nearly duplicate
code that calls parse - find - return.
- Add three new functions to add tagged data:
- add_tagged_data_to_tx_extra is a raw function that takes a
tag and string_view and appends it to tx_extra
- add_tx_extra<T>(tx_extra, val) lets you copy `val` directly as a
`T`. `val` does *not* have to be the same as `T` (`T` is not
deduced) but rather just has to have the same size. This makes it
very easy to add simple tx_extra values such as `tx_extra_pub_key`
from just a pub key without needing to wrap it first:
add_tx_extra<tx_extra_pub_key>(tx.extra, regular_old_pubkey);
- add_tx_extra<T>(tx, val) - very light wrapper around the above that
takes a transaction lvalue directly (rather than the extra vector)
for slight convenience.
These additions allow removing several now-unneeded methods and simplify
some code around the tx extra handling.
This commit also converts the TX_EXTRA_* macros into constexprs.
Add number parsing and basic string splitting to common/string_util.h
and use it (replacing some regexes and boost string utilities).
Includes unit tests.
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.
The serialization interface here had a lot going wrong with it. This
overhauls it drastically and makes it a lot nicer to deal with.
(This commit is in two parts: this first one updating the base
serialization code, the subsequent one updating various places using
it. There's a third part, depending on how you want to count,
converting boost::variant to std::variant which relies on the
serialization changes made here).
- everything is now in the `serialization` namespace instead of having
some things there and other things in the root namespace.
- serialization failures now throw exceptions with reasons for the
failure rather than needing to snake a bool back through the call stack
(without any message).
- the `template <bool W, template <bool> class Archive>` monstrosity is
gone. Instead an Archive class has a Archive::is_serializer or
Archive::is_deserializer constexpr bool that can be checked (there was
something sort of similar before, but required messing around with
boost::mpl crap in both the generic and specific serialization code).
- the serialization code is significantly more flexible: instead of
having to slam everything into a class itself, you can also serialize
using with a free function in the same namespace as the class.
- serialization macros are still provided, but now considered
deprecated, replaced with (ADL callable) function names that don't hide
the action from the caller. So:
FIELD(a);
VARINT_FIELD(b);
FIELD_N("c", some_c);
VARINT_FIELD_N("d", some_d);
FIELDS(x); // (this is like the above, but doesn't write a tag)
ENUM_FIELD(e, e < myenum::_count);
FIELD(f);
if (!W && f != 42) return false;
becomes (all of this is documented in serialization.h):
field(ar, "a", a);
field_varint(ar, "b", b);
field(ar, "c", some_c);
field_varint(ar, "d", some_d);
value(x);
// enums just get passed to field_varint. It takes an optional
// lambda to verify on deserialization:
field_varint(ar, "e", f, [&] { return e < myenum::_count; });
// But the verifier isn't limited to enums:
field(ar, "f", f, [&] { return f == 42; });
and all of this works without needing to `serialization::` qualify the
beginning. In the rare case where you have a conflicting function
defined (e.g. a local `field()`) you can qualify to disambiguate.
- The messy eof hacks where you call `serialize_noeof` is cleaned up.
You now call `serialization::serialize(ar, val)` to deserialize an
*entire* value (which requires that the whole strem is consumed), and
`serialization::value(ar, val)` to append a serialization.
- Container serialization is significantly simplified; the various
serialization/vector.h (and similar) are now extremely thin wrappers
around the generic main container serialization code.
- INSERT_INTO_JSON_OBJECT, GET_FROM_JSON_OBJECT, and
OBJECT_HAS_MEMBER_OR_THROW macros are gone, replaced with nearly
identical yet more flexible (for both the caller and the compiler)
relatively simple templated functions.
- Drastically simplified the ability to serialize to/from a string via
new `serialization::binary_string_archiver` and
`serialization::binary_string_unarchiver` serializers. The former takes
no arguments; the latter takes a string_view. This means you can
serialize to binary using:
serialization::binary_string_archiver ar;
serialization::serialize(ar, myvalue);
std::string serialized = ar.str();
and can deserialize using:
MyType myvalue;
serialization::binary_string_unarchiver ar{serialized};
serialization::serialize(ar, myvalue);
(though really this interface is for slightly more complicated cases
than these; see the next point)
- the existing dump_binary() and parse_binary() are tweaked a bit: both
now throw, and dump_binary() returns the result string instead of taking
it as an output parameter. (parse_binary() is now void, rather than
returning a bool, because there are many places where in-place
deserialization is desirable).
- make one_shot_read_buffer internal to binary_string_unarchiver, and
use it there. The interfaces here means we no longer need to rely on
the seeking behaviour, so the serialization issues on mac shouldn't
happen now.
- begin_array()/begin_object() now use an RAII interface to make
serializing arrays much easier. Where previously we had a lot of code
that did something like this:
ar.begin_array();
for (auto it = whatever.begin(); it != whatever.end(); it++)
{
FIELDS(val);
if (*(it+1) != whatever.end())
{
ar.delimit_array();
}
}
ar.end_array();
Now an array serialization looks like this:
auto arr = ar.begin_array();
for (auto& val : whatever)
value(arr.element(), val);
- serialized non-varint integer values weren't endian safe, now they
are.
- variant serialization converted to use std::variant instead of
boost::variant and significantly cleaned up using C++17 features.
- varint (no "a", i.e. variable length integers) was not easy to follow
and badly documented (the given examples in the description were flat
out wrong). Rewrote it, with substantially better documentation and
unit tests, and taking advantage of C++14 `0b` and `'` in integer
literals to make it far easier to follow.
Changes all boost mutexes, locks, and condition_variables to their stl
equivalents.
Changes all lock_guard/unique_lock/shared_lock to not specify the mutex
type (C++17), e.g.
std::lock_guard foo{mutex};
instead of
std::lock_guard<oh::um::what::mutex> foo{mutex};
Also changes some related boost::thread calls to std::thread, and some
related boost chrono calls to stl chrono.
boost::thread isn't changed here to std::thread because some of the
instances rely on some boost thread extensions.
- replace the gross cmake shell script tools being used to create the data file with
cmake code and a single configure_file
- change checkpoint data to string_view
- tools::sha256sum() had two different overloads that did very different
things, yet presents two overloads: one that takes a pointer+size, the
other that takes a string lvalue ref, and yet the string lvalue
reference is *entirely* different (it reads a file). Append _str and
_file suffixes to make it less dangerous.
Switch loki dev branch to C++17 compilation, and update the code with
various C++17 niceties.
- stop including the (deprecated) lokimq/string_view.h header and
instead switch everything to use std::string_view and `""sv` instead of
`""_sv`.
- std::string_view is much nicer than epee::span, so updated various
loki-specific code to use it instead.
- made epee "portable storage" serialization accept a std::string_view
instead of const lvalue std::string so that we can avoid copying.
- switched from mapbox::variant to std::variant
- use `auto [a, b] = whatever()` instead of `T1 a; T2 b; std::tie(a, b)
= whatever()` in a couple places (in the wallet code).
- switch to std::lock(...) instead of boost::lock(...) for simultaneous
lock acquisition. boost::lock() won't compile in C++17 mode when given
locks of different types.
- removed various pre-C++17 workarounds, e.g. for fold expressions,
unused argument attributes, and byte-spannable object detection.
- class template deduction means lock types no longer have to specify
the mutex, so `std::unique_lock<std::mutex> lock{mutex}` can become
`std::unique_lock lock{mutex}`. This will make switching any mutex
types (e.g. from boost to std mutexes) far easier as you just have to
update the type in the header and everything should work. This also
makes the tools::unique_lock and tools::shared_lock methods redundant
(which were a sort of poor-mans-pre-C++17 way to eliminate the
redundancy) so they are now gone and replaced with direct unique_lock or
shared_lock constructions.
- Redid the LNS validation using a string_view; instead of using raw
char pointers the code now uses a string view and chops off parts of the
view as it validates. So, for instance, it starts with "abcd.loki",
validates the ".loki" and chops the view to "abcd", then validates the
first character and chops to "bcd", validates the last and chops to
"bc", then can just check everything remaining for is-valid-middle-char.
- LNS validation gained a couple minor validation checks in the process:
- slightly tightened the requirement on lokinet addresses to require
that the last character of the mapped address is 'y' or 'o' (the
last base32z char holds only one significant bit).
- In parse_owner_to_generic_owner made sure that the owner value has
the correct size (otherwise we could up end not filling or
overfilling the pubkey buffer).
- Replaced base32z/base64/hex conversions with lokimq's versions which
have a nicer interface, are better optimized, and don't depend on epee.
This adds a static dependency script for libraries like boost, unbound,
etc. to cmake, invokable with:
cmake .. -DBUILD_STATIC_DEPS=ON
which downloads and builds static versions of all our required
dependencies (boost, unbound, openssl, ncurses, etc.). It also implies
-DSTATIC=ON to build other vendored deps (like miniupnpc, lokimq) as
static as well.
Unlike the contrib/depends system, this is easier to maintain (one
script using nicer cmake with functions instead of raw Makefile
spaghetti code), and isn't concerned with reproducible builds -- this
doesn't rebuild the compiler, for instance. It also works with the
existing build system so that it is simply another way to invoke the
cmake build scripts but doesn't require any external tooling.
This works on Linux, Mac, and Windows.
Some random comments on this commit (for preserving history):
- Don't use target_link_libraries on imported targets. Newer cmake is
fine with it, but Bionic's cmake doesn't like it but seems okay with
setting the properties directly.
- This rebuilds libzmq and libsodium, even though there is some
provision already within loki-core to do so: however, the existing
embedded libzmq fails with the static deps because it uses libzmq's
cmake build script, which relies on pkg-config to find libsodium which
ends up finding the system one (or not finding any), rather than the one
we build with DownloadLibSodium. Since both libsodium and libzmq are
faily simple builds it seemed easiest to just add them to the cmake
static build rather than trying to shoehorn the current code into the
static build script.
- Half of the protobuf build system ignores CC/CXX just because Google,
and there's no documentation anywhere except for a random closed bug
report about needing to set these other variables (CC_FOR_BUILD,
CXX_FOR_BUILD) instead, but you need to. Thanks Google.
- The boost build is set to output very little because even the minimum
-d1 output level spams ~15k lines of output just for the headers it
installs.
The same struct `lazy_init` was being used in two different files linked
into the same binary, causing test failures depending on which one got
kept in the final binary. (Enabling LTO builds noticed the violation,
warned about it, and caused the spent_outputs.not_found test to fail).
