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).
It's slow and unnecessary and depends on no keys in the hardware wallet
(c is public info in CLSAG). If the wallet was to provide a changed c
value then verification would simply fail.
The hard wallet debug code had no way to enable it, and if you did
manually add the define, didn't compile. It was also nasty, gross,
disgusting code that someone slopped into the file.
This fixes it, adds a cmake option for it, and significantly cleans it
up--just because code is for debugging doesn't mean it should be nasty
and broken.
- Sending one 32-byte key at a time is noticeably slower than sending in
larger chunks.
- Sending in 256-byte chunks was broken because the size field is only
254 bytes. Since we are actually sending these for Keccak, it makes
some sense to chunk it at 136 bytes (i.e. keccak block size).
- Change how multipart works to send as parts 1->2->...->0. Previously
0xff (rather than 0) marked the last chunk.
- Allow multi-part chunks to wrap by wrapping the part after 255 to 1
(skipping 0 since that now means "last").
- Use multi-part chunk scheme for CLSAG in addition to prefix hashing.
Function signatures (especially in headers) should be readable!
Also removes useless "const" on pass-by-value parameters from headers,
and pass the bool argument by value instead of by const lvalue
reference.
A few times I've connected my test Ledger but forgotten to open the app,
and then had connection failures without any indicating of what went
wrong (just "No device found"). This adds "(Is the device running with
the wallet app opened?)" to that error message as that is likely often
the culprit.
- 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 is dead code as the previous if is always entered for CLSAG. The
body of this is applied later in the function, so this looks like code
that got moved but the original didn't get deleted?
The blockchain doesn't accept MLSAG txes anymore (since HF16 + 10
blocks), so there is no need to keep the generation code around.
Also renames mlsag_prehash to clsag_prehash since that is where it is
primarily used now.
For no good reason the hardware wallet creation didn't allow you to
specify a restore height from the cli wallet for no good reason. This
commit lets it use the same prompt, and improves the prompt somewhat:
- you can now specify `curr` to use the current blockchain height (and
this is the default for the hardware wallet).
- added a note about the purpose of the restore height (i.e. that
transactions won't be seen if before the restore height).
- put example values in the description.
- show the default in the prompt.
- Adds "create_wallet" rpc arguments to allow creating a hardware wallet
- Adds a "create-hwdev" option to both rpc and cli wallets that creates
a "[wallet].hwdev.txt" file containing an optional comment to flag the
wallet as a hardware wallet. Although this data is present in the
wallet file itself, that isn't available without a password description
and the the GUI wallet needs to know this *before* it opens the wallet
to properly display a wallet list with some indicators for hardware
wallets. The comment also lets the user record some optional comment
such as "Jason's Ledger Nano S" to serve as a reminder in the GUI wallet
list.
- Fixes some unicode EN DASHes that got added to documentation comments
in place of the intended hyphens.
Tx prefix communication was missing some needed information on the tx
type, and was a little inefficient. This redoes the protocol to send
the tx type info and then the entire prefix (rather than starting from a
few bytes in). It also changes how we number requests and signal the
final piece of a multi-piece transmission.
Loki-side updates for Ledger Nano S support:
- Add a get-network command (and bump protocol version) so that we can
verify that the Ledger is set to the correct network type (i.e. mainnet
or testnet). Previously there was no check at all, so you could have a
testnet wallet on desktop using mainnet keys on the Ledger. Now they
get checked and an error occurs on mismatch.
- Reset required version to 0.9.0
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.
