We figure out, when parsing a blockchain-confirmed transaction, what the
change is but never properly set that in the new
confirmed_transfer_details entry that we create from the
unconfirmed_transfer_details, and the latter only has the *actual*
change but doesn't know about outputs we sent to ourself.
This commit fixes it by updating the change value when such a case
occurs, which fixes a wrong output & running total bug in csv exports.
Received inputs were being handled incorrectly when importing key
images: only the *first* payment received was being removed, but it
can easily happen that we have multiple payments to ourselves in the
same wallet (for example, if we send a fixed amount to ourselves then
almost always send two non-zero amounts to ourself: the amount + the
change).
This would result in (n-1) "in" outputs for the transaction still
showing up in the output, along with the "out" transaction (which
contains the overall net transfer amount). For example:
- Alice sends 100 + 12.3 change to herself.
- Bob has Alice's view keys, opens a view-only wallet, sees 100 and 12.3
as two separate inputs from the same transaction.
- Alice provides a key image export to Bob to verify the account
- Bob imports.
Bob's export would now show:
in 12.3
out 0
(The inputs of the same transaction are apparently ordered randomly, so
it could also be 'in 100'/'out 0')
and the running total would be 12.3 too high (or 100 too high if the 100
got left instead of the 12.3).
This fixes it by removing *all* matching payments when we import key
images instead of just the first one.
Cleans up some messy code.
Reformats the CSV output to be a little cleaner (in both code and
result), using fmt:
- change second "amount" field to "sent_amount"
- don't unnecessarily quote the subaddr indices field (only quote when
more than one need to be listed as comma-separated).
- change "checkpoint" field output to "yes"/"no" rather than
"checkpointed"/"no".
- Adds spacing between fields which makes it a little easier to read and
should still parse identically.
- Date format also changes somewhat (because fmt is now formatting it)
and includes " UTC" to clarify.
Miscellanous crap that I couldn't help but cleanup while working in
wallet2 to solve the export csv issues:
- To celebrate the 10 year anniversary of god inventing `auto` we
replace some obtuse code that was trying to make itself look bigger by
typing out massive obvious types.
- Remove the single remaining tab in wallet2.cpp to get wallet2.cpp from
being 99.9993% tab-free too 100.0000000% tab free.
- Fix THROW_WALLET_EXCEPTION_IF calls that checked for not-found *after*
another check that already dereferenced the iterator.
- Make `get_attribute` interface less stupid by returning an optional
string instead of a bool with an out string argument.
- Simplify various std::holds_alternative + var::get combinations with a
single std::get_if.
- Optimize and DRY hex conversion of payment id in make_transfer_view.
The existing code was duplicated 4 times but also was inefficiently
performing extra string allocations via multiple substr calls.
- Loki -> Oxen in a wallet error message.
- Initialize confirmed_transfer_details members inline rather than using
an obtuse constructor.
- Apply some C++17 `if` initializer syntatic sugar.
- Don't use `0 != pointer` to test for null
- Don't use a linear scan of a std::vector to do what an unordered_set
is meant to to.
- Replace NULL with nullptr
- eliminate boost::format from wallet2
- eliminate boost::lexical_cast from wallet2
wallet2 expects to get back the UINT64_MAX value here for a
fully-reserved service node: if it does, it recognizes the node is full
and goes to look for reserved contribution spots.
We don't do anything differently for a nullopt versus an empty vector,
so just return an empty vector for both not-at-the-hf and no-payments
cases.
(This also fixes a segfault and/or chainsync bug in the previous commit
because the optional is dereferenced without checking when on hf19).
Fix edge case where the block producer has garbage in the db: the code
that clears it happens when we *accept* the block, but we can end up
here before then, when we produce the block, so just return empty in
such a case.
