The important part here is removing this line:
if (swarm_to_snodes.size() == 1) return MAX_ID / 2;
because, if we end up in a case where we have only one swarm and it
*already* has that ID (e.g. create 2, which will be [MAX/2,0] then drop
0) then this returns a swarm_id that already exists, which is bad
because then we fail to insert the new swarm, a service node gets left
with an unassigned swarm id, and that then causes issues in SS because
that node thinks it is deactivated because it doesn't have a swarm id
(yet it *is* in the active nodes list, so other network members still
try to talk to it).
7293311c52 broke these tests -- they were
broken already because they were testing values under HF13 rules, so the
15k requirement wasn't kicking in. Updated them to better HF versions,
and fixed the tests to respect the HF16 requirement drop.
We add the tx secret key to the tx_extra in staking transactions so that
values can be decoded, but the tx secret key value that we have on hand
is encrypted and so we can't access it.
This moves the call that adds the secret key into the device code so
that devices can provide this. It also adds the tx version/type earlier
in the process (into `open_tx`) so that the device can know early on
that this is a stake transaction and therefore that leaking the tx
secret key is okay (and can also apply other stake-specific behaviour).
- 0 outputs is not valid post-Borromean. This was passing before because
previously the tests were using Borromean ringct construction.
- Remove and fix non-bulletproof/clsag tests which can't run anymore
(since we can't construct pre-clsag).
Passing in a true/false value is dumb, we should just call
EXPECT_TRUE/EXPECT_FALSE rather than `EXPECT_TRUE(...(true, ...` or
`EXPECT_TRUE(...(false, ...`
Passing the fee in the output array and then taking it out of the output
array is similarly stupid, so don't do that either.
The vast majority of the tests use lower_case_category.lower_case_test,
but Serialization did it differently for no reason (and even then wasn't
consistent with the test names). Fix that.
This json serialization layer was only used in the old Monero ZMQ
interface, which no longer exists, and so this is just dead code.
On top of that, it doesn't work properly for serializing CLSAG
transactions, so just delete it.
- Remove some useless epee functions, and add deprecated markers to ones
that have good replacements already.
- Don't use boost::lexical_cast when std::to_string or direct stream
output can be used just as well.
- Get rid of dumb epee "include_base_utils.h" header
Converts all use of boost::filesystem to std::filesystem.
For macos and potentially other exotic systems where std::filesystem
isn't available, we use ghc::filesystem instead (which is a drop-in
replacement for std::filesystem, unlike boost::filesystem).
This also greatly changes how we handle filenames internally by holding
them in filesystem::path objects as soon as possible (using
fs::u8path()), rather than strings, which avoids a ton of issues around
unicode filenames. As a result this lets us drop the boost::locale
dependency on Windows along with a bunch of messy Windows ifdef code,
and avoids the need for doing gross boost locale codecvt calls.
When targetting macos <10.14 macos won't allow use of anything from
C++17 that throws, such as:
- std::get on a variant
- std::visit
- std::optional::value()
- std::any_cast
This avoids all of these.
For std::get, we either replace with std::get_if (where appropriate), or
else use a `var::get` implementation of std::get added to lokimq (also
updated here). (This `var` namespace is just an `std` alias everywhere
*except* old target macos).
For std::visit, likewise lokimq adds an var::visit implementation for
old macos that we use.
std::optional::value() uses weren't useful anyway as everywhere it calls
them we've already checked that the option has a value, in which case we
can use `*opt` (which doesn't check for contents and throw).
std::any just has to be avoided as far as I can tell, but the one place
we used it is only ever a block, so I just replaced it with a `const
block*`.
This test is not very useful: it is often wrong, especially on VPSes,
and doesn't do anything at all on non-Linux. But even if it did, "OMG
SSDs ARE FASTER" is not really something lokid needs to worry about
pointing out.
Revamps how .loki LNS registrations work:
- Enable lokinet registrations beginning at HF16.
- rework renewal so that you can renew at any time and it simply adds to the end of the current
expiry. Previously there was only a window in which you could renew.
- Renewals are a new type of LNS transaction, distinct from buys and updates. (Internally it is an
update with no fields, which cannot be produced in the existing code).
- Add optional "type=" parameter to lns commands. Commands default to trying to auto-detect (i.e.
if the name ends with .loki it is lokinet), but the type allows you to be explicit *and* allows
select non-default registration lengths for lokinet buys/renewals.
- change .loki naming requirements: we now require <= 32 chars if it doesn't contain a -, and 63 if
it does. We also reserve names starting "??--" for any ?? other than xn-- (punycode), as this is
a DNS restriction. "loki.loki" and "snode.loki" are also now reserved (just in case someone
sticks .loki as a DNS search domain).
- Tweak LNS registration times to consider "a year" to be 368 days worth of blocks (to allow for
leap years and some minor block time drift).
- Overhaul how LNS registrations are displayed in the cli wallet. For example:
[wallet L6QPcD]: lns_print_name_to_owners jasonv.loki jason.loki jasonz.loki
Error: jasonv.loki not found
Name: jason.loki
Type: lokinet
Value: azfoj73snr9f3neh5c6sf7rtbaeabyxhr1m4un5aydsmsrxo964o.loki
Owner: L6QPcDVp6Fu7HwtXrXjtfvWvgBPvvMQ9FiyquMWn2BBEDsk2vydwu1A3BrK2uQcCo94G7HA5xiKvpZ4CMQva6pxW2GXkCG9
Last updated height: 46
Expiration height: 75
Encrypted value: 870e42cd172a(snip)
Error: jasonz.loki not found
- Add an RPC end-point to do simple LNS resolution; you can get the same info out of
names-to-owners, but the new lns_resolve end-point is considerably simpler for doing simple
lookups (e.g. for lokinet), and allows for a simpler SQL query + processing.
Code changes:
- Rename mapping_type::lokinet_1year to mapping_type::lokinet (see next point).
- Don't store lokinet_2y, etc. in the database, but instead always store as type=2/::lokinet. The
LNS extra data can still specify different lengths, but this now just affects the
expiration_height value that we set.
- Reworked some binding code to use std::variant's and add a bind_container() to simplify passing in
a variable list of bind parameters of different types.
- Accept both base64 and hex inputs for binary LNS parameters in the RPC interface.
- This commit adds some (incomplete) expiry adjustment code, but ignore it as it all gets replaced
with the following commit to overhaul record updating.
- Updated a bunch of test suite code, mainly related to lokinet.
- Some random C++17 niceties (string_view, variant, structured binding returns) in the related code.
- Tweaked the test suite to generate a bit fewer blocks in some cases where we just need to
confirm/unlock a transfers rather than a coinbase tx.
The replaces the LNS encryption starting in HF16 with libsodium's
XChaCha20-Poly1305 (which is recommended over the default encryption
nowadays), and replaces the encryption key with a keyed blake2b hash
rather than argon2.
This also adds a nonce because, after reading the libsodium
documentation, I'm concerned that using a fixed nonce means we are
potentially leaking the name on updates. This complicates things a bit
-- if doing external signing you need to generate an encrypted value and
sign that, rather than being able to regenerate the encrypted value.
So we have:
name_hash -- blake2b(name), same as before
enc_key -- blake2b(name, key=name_hash), instead of argon2
nonce -- randomly generated, and tacked on the end of the encryption
value (previously nonce was all-0s).
This requires changing the encryption so that you generate an encrypted
value for the update, rather than just using the update parameters, then
you provided this encrypted value when submitting an update with
external signature.
Code-related changes (aside from implementation the above): this moves
the encryption/decryption functions into the mapping_value itself, and
adds an encrypted state (so that you can encrypt/decrypt a mapping_value
in-place without needing to copy data around).
(Also note that this commit is almost certainly not "clean" -- I think
some of it leaked into the following commit, and some of that commit may
have leaked here, but this separated most of it).
Jason Rhinelander
Sean Darcy
Sean Darcy
Doyle
Sarang Noether
moneromooo-monero