We can't call cryptonote::add_tx_secret_key_to_tx_extra from `device`
code because that isn't necessarily available in `device` (though for
some odd reason this only actually showed up on the i386 build).
This amends the call to just get the secret key, leaving the actual job
of adding it to tx.extra to the caller (which is a cleaner way to do it
anyway).
For some reason we aren't keeping the old chain as an alt chain anymore,
but that shouldn't be a problem: fix the test as well as make some
improvements in the tests it does.
What we are passing here is invalid, and so raises an exception, but the
test structure here doesn't have a nice way to catch that, so just
disable the test.
This adds a variable hack into loki-core that lets us disable the
transaction hard fork requirement so that the test suite can still
generate transactions under older tx rules even though the transactions
will be modern CLSAG txes.
These are sort of bastardized txes that can never occur on the proper
mainnet, but let us keep tests that apply to v2/v3 transactions even
though we can't actually generate proper v2/v3 transactions anymore.
A few tests got removed here because they are testing for old invalid
bulletproof formats that don't matter anymore because they will never be
accepted on the current chain anyway.
Moving it does what you'd expect: moves the lambda, copies the current
cancel status, and cancels the old one.
The implicit move constructor could malfunction: the old one wouldn't
necessarily end up cancelled.
There's no reason we need intermediate blocks, so make it just generate
v7@0 (for genesis), v14s to make funds, and the the target. (Or just
v7@0 + target for <v14 hard fork versions).
- updates tests to work properly with current HF
- makes loki_generate_sequential_hard_fork_table jump 7->14....->15->16
rather than intermediate 8-9-10-11-12-13 blocks. (The 14 sequence is
the generate block rewards before 15 lowers and 16 eliminates mining
rewards).
- remove test relying on the old 30-day expiry; that only worked on old
HFs, but also required old (pre-v4) txes which don't work anymore, so I
just removed the test.
When we need to fill a block we are currently generating a ton of
transactions, but that is fairly slow: much faster to generate a small
number of huge txes.
Exceptions (e.g. because you denied the tx on the Ledger) were printing
and being immediately (mostly) overwritten by the wallet prompt. This
fixes them to be returned as proper errors (and thus bright red,
consistent with other returned simplewallet errors).
Core tests were breaking because of the removal of pre-CLSAG
transaction generation support. This fixes it by allowing and using
CLSAG transactions before HF16 (which is safe to do now that we are well
past the hard fork).
Moves a bunch of inline methods out into a new cryptonote_basic.cpp
compilation unit. (Given how widely cryptonote_basic.h gets included it
seems desirable to have as little code needing compilation as possible).
Being able to pass the hash to the Ledger might be abusable (e.g. if it
passed a different hash, with a different secret key to try to sign
something else using the device's secret keys).
There is no reason at all to sign a *different* message in every stake
unlock; signatures already have their own nonce.
Having something that serves no purpose is worse than not having it
(because it leads to questions about why such a thing is there), so this
commit removes it by always using 0 as a nonce and comments about it.
Removing this from the broadcast tx would require a new tx extra field
so that isn't worth doing for now (but can be done in the future if we
change the tx extra structure for unlocks).
This also simplifies the nonce-to-hash code and fixes an endian bug in
it.
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).
Changing crypto random functions broke the test code horribly (because
it depends on a deterministic random order), but it has no nice way to
reproduce it!
This restructures it a bit and significantly improves it, also updating
it for the new generated crypto values, and adding support for setting a
`REGEN=1` environment variable to generate a new test file.
We use generate_ring_signature and check_ring_signature somewhat
inappropriately to sign and check a signature of a single key image.
While it works for that, the full ring signature algorithm adds quite a
bit of complexity that we don't need (and simply doesn't run) for the
key image proof included in stake transactions and exported key images
from the wallet.
This splits it up, makes the key image interface considerably simpler,
and adds annotation comments through it (and also adds comments into the
"main" signature code).
This is a necessary step to getting stake transactions and key image
exports working with Ledger, without implementing the full ring
signature (because that is quite involved, and not needed for most of
these cases).
Also remove unused gen/check_ring_signatures interfaces: The raw pointer
code is never called, except through the vector version and one place in
the test suite, so just remove it and make the vector version the main
implementation.
- A static function inside an anonymous namespace is pointless.
- Add comments above the sheer nastiness that is overloading `&` to
avoid having to reinterpret_cast a char to an unsigned char. This
horror needs to die.
- Replace nasty C99 flexible array members reinterpret_cast'ed into a
value and then stuffed into a shared_ptr with a proper struct.
- Don't predeclare a bunch of crap long before the crap is used; this is
C++, not C.
This code is nasty:
- every time through the loop requires a mutex acquire and release
rather than holding the mutex around the loop.
- the comment about the limit is confusing af: eventually I figured out
that the "limit" equals 15L, and that this code is basically trying to
generate a value in (0, 15L) then mod L the number to get an unbiased
value in (0, L).
- However, the code is broken in two ways:
- it can return 0 because (nL % L == 0) for integer n > 0
- the while condition is broken af so that the while loop can *never*
repeat, and thus the entire function is just always returning
(random % L) which thus has the slight bias.
This commit fixes all these issues by moving the mutex outside the loop
and borrowing libsodium's approach for the generation: generate a random
value, mask off the 3 most significant bits, then repeat until this
yields a value in (0, L), which happens slightly more than 1/2 the time.
This is a bit slower than the 15L approach, but much simpler and
generating random scalars is not a performance bottleneck.
Fixes lots of crappy C++ code. I strongly get the impression from these
changes that whoever wrote this code was a C programmer with very little
C++ experience. Sadly no one in the upstream Monero PR review tried to
help or seemed to care about the code quality.
- Get rid of superfluous `this->` throughout the ledger code.
- DRY: abstract away sending sequences of bytes, replacing:
memmove(buffer, this->buffer_send+offset, 32);
offset += 32
with:
send_bytes(buffer, 32, offset);
- DRY: abstract sending/receiving u32
- DRY: abstract receiving bytes/u32
- properly prefix memcpy/memmove with std::
- use std::string_view and std::string for setting/retrieving name
- rename `this->controle_device` to `debug_device`
- replace `f(void)` -> `f()` (on C++ methods, FFS!)
- DRY: replace set-length-then-exchange dance with a function
- DRY: merge nearly-identical exchange() and exchange_wait_for_input()
- remove never-used ok/mask arguments from exchange()
- Remove ASSERT_SW macro used only in one place
- Replace dumb ASSERT_X macro that was just an alias for another macro
- remove ASSERT_T0 macro that isn't used anywhere
don't send 32 null bytes for no reason in INS_GEN_TXOUT_KEYS when there
is no additional txkey (this doesn't even match the case when there
is one since we send it encrypted, requiring 64 bytes).
