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.
On devnet we don't have a storage server, and so storage ports are left
uninitialized and effectively become random ports on the network. This
initializes them to 0, and avoids comparing SS ports for devnet for the
duplicate ip/ports warning.
Jason Rhinelander
Sean Darcy