Add 'sent' to wallet info output

[yeastplume]

It's going to become a bit difficult to track wallet send/receive issues, especially seeing as how it's difficult to see what's been sent out. Perhaps just total amount spent for now, but I think it would be desirable to have the entire send history available, which breaks down exactly how much was sent (or the user attempted to send) and where the outputs went.

[antiochp]

Maybe we can have an additional log file that the wallet can log stuff to?
That way we have an audit trail of activity from a users wallet.

[antiochp]

Agreed on the difficulty of debugging issues - the whole "confidential transactions" thing makes it kind of impossible.

[heunglee]

I am not sure if @sesam or someone else is working on this issue.
If so, please disregard my comment.

Otherwise, I propose a initial draft of requirements and some questions to clarify.
it is based off from quoting @ignopeverell's statement in #326:

1. I think the idea in Add 'sent' to wallet info output #288 was to make this accessible to wallet info, which means a separate data file, similar to wallet.dat that captures transaction-level information. It could say something like "sent X coins on Y date to Z addres, outputs A, B, C where consumed and D change was created".
2. To be easily usable by the wallet it should be some JSON data. With that the wallet could display some info but also rollback some failed transactions for example.
3. Maybe extending the format of wallet.dat would make the most sense.

Possible drawback of adding "sent" and "received" data into wallet.data

1. As time passes, the file size increases quickly
2. Increased rate of write locks
3. The current format of wallet.dat can be impacted to incorporate JSON format (Root node of "outputs" can be changed?)

So having separate file for sent and received, such as wallet_sent.dat and wallet_received.dat?
Or extending wallet.dat?

Formats:
Need to support JSON.

SENT Transaction
key_id, amount, date, destination, outputs, change, status (pending, success, failed)

RECEIVED Transaction
key_id, amount, date, sent_by(?) and ???

Impacts:
TBD

Target PR date:
TBD

[sesam]

Good. I'm not working on this.

There's a code line

  		Ok(_) => { debug!(LOGGER, "{} coins sent using strategy {} to {}", amount.to_string(), selection_strategy, dest) },

which should probably just be replaced a function that writes to say "spends.dat" and also makes some debug! or trace! log line mentioning that a transaction was logged to spends.dat. The public parts of the transaction (kernel+ signatures?) could be visible in the trace! output but preferably keep the parts hidden from bystanders / blockchain explorers should also not be in the log file, to keep it relatively safe to expose your grin.log

Note we can't use another instantiation of already in use slog-rs; it can't support multiple instances of itself due to macro conflicts and more), so some json stream (append-only) is probably the nicest, to avoid reading and rewriting the full "spends.dat" for every transaction.

Example:

{ txid: 'af35687', is: 'In-Coinbase', amount: 50.000000000, kernels: ['f8d6df'], time: (unix epoch seconds), seen_in_blocks: ['67af6e53b7f'] }
{ txid: 'bf35687', is: 'Out', amount: 10, kernels: ['a5df5df', 'b5dfa7df', 'f8d6df'], seen_in_blocks: ['8e7678fe6da'] }
{ txid: 'cf35687', is: 'In', amount: 1.2345, kernels: ['67f567ef', 'd4a6fe4d'], seen_in_blocks: ['67af6e53b7f', '9e5f4e6d3'] }

To anyone taking up implementing this: first get everything except "seen_in_blocks" added. The #easy tag on this bug applies to the first part, while to track which blocks requires more knowledge of grin, blockchains and thinking about cut-through. Write in gitter chat or the maillist to not get stuck on that.

The last low shows that we've seen a chain fork, and our incoming transaction 'cf..' is mined into (at least) two chains.

In a blockchain, you never know for sure if a transaction succeeded or failed, but what you can know is if it's been mined, and in which block hash (not chain height...:)) where it's mined. So I'm suggesting "seen_in_blocks" to be an array listing any block hashes where the relevant transaction id is known to be included.

Also, if those chains would be lost (orphaned) we probably should keep the full original transaction saved here to make it possible to resend the transactions we care about. So maybe also add "full_tx" with the original full transaction should be added later.

[sesam]

There's a "light" version of this task, that could start with a simpler pull request just adding some more fields to wallet.dat, but since wallet.dat is updated and sometimes overwritten, recovered or lost, a long term solution should have the log of transactions in an append only format.

If json streams is new to you, some links to help, here are solutions to similar problems:
serde-rs/serde#1107
http://valve.github.io/blog/2014/08/25/json-serialization-in-rust-part-1/

[heunglee]

Thank you @sesam for detailed information. It helps a lot.

Thoughts about implementation

As @sesam pointed out, wallet.dat is frequently updated, sometimes restored or lost. (The send tx's seems not need to be restored unless audit of history of sends is required. Am I right?) So it would be better to have a separate file, sends.dat or wallet_tx.dat?

As @sasam suggested, the implementation can be placed in the part of result handling. When I walked through the source codes of wallet, I thought the right place for implementation was in issue_send_tx of wallet/sender.rs where outputs are stored in wallet.dat. That part seems provide advantage to extend features later on.

Any suggestion and recommendation appreciated.
Wish a wonderful Christmas!

[ignopeverell]

I think I'd agree about having a separate file. Maybe wallet.stats? And merry Christmas as well!

[heunglee]

Just record thought of what factor to consider for later. In the future, the transaction status and history can be important for thin wallets. Also restoring tx history as well? Users need to know them like thin client of byteball.

[yeastplume]

I would definitely leave wallet.dat alone and do this separately. Ideally we should always be able to reconstruct all of the relevant parts of wallet.dat from the seed+password and the blockchain.

I'd probably just start with a simple log, as you suggest above, that at least records transactions received and sent, as well as whether they were successful (as least, so far as the receiver has reported, they could be lying). Once that's in place, I'd possibly look at adding a couple of routines that

1. parse the contents of the log and display entries in a human readable format with summary
2. Compare the contents of the log against the blockchain and verify/clean up the state of the log entries (inasmuch as is possible), particularly comparing destroyed inputs from 'completed' sent transactions and ensuring they're not there, etc. (in the simple case... there are a few other considerations around this we can get into later). Ideally you want to try to reconcile this with the total amounts reported in wallet.dat, and report when you're unable to do so.

I possibly wouldn't delete anything from the log here (keep it append only), but mark transactions /outputs off as erroneous based on blockchain info during the parsing/verification.

Also, note that the transaction workflow in sender/receiver.rs is about to change significantly due to the introduction of aggsig transactions, which will split the transaction construction into several phases. I'd try and keep the logic for all of this as separate as possible, and keep the interface as simple as possible, as in ensure the transaction log just takes simple transactions (which contains all of the inputs/outputs) as input without the need for any other context.

[heunglee]

The current implementation is to write json object of each transfer (sent or received) into wallet.stats file. The summary of transfers follow the wallet data summary as shown in the attached. Each json object contains inputs and outputs in addition to the attributes shown in wallet info.

Please review the screenshots and suggest what to add or modify. Once the review and change will be completed, I will do PR.

[heunglee]

I do not know how to get the sender's wallet address because it is not passed to process received transaction. So it is set to localhost for now. Advised on the sender's wallet address? Please note that the transfer record is written into file only when the transaction is successful in this implementation.

If needed, I can implement API endpoints to view the wallet data and stats, and to send coins in the web browser of the local host.

[sesam]

Looks nice!

wallet nice output

I don't know how this timestamp is defined and verified, or not, but that seems important. Most importantly, will the timestamp be exactly the same for sender and receiver?
Depending on what's more true, possible timestamp headlines: "First seen" or "Senders time (no trust)" or "Senders time (partly trusted)".

json format

• Wallet address should probably always have the port number listed even if it's the default port, since the default settings can be changed in code or grin.toml, to ensure it stays trace:able.
• maybe rename timestamp_* to more closely explain where the time comes from.
• if timestamp_sec is controlled by the sender (and so not trustable) I'd like to also have a "first_seen_sec" that should match exactly what I'd see in debug log outputs.

[heunglee]

Thank you @sesam.

Re: timestamp

The content of each transfer does not include timestamp that can be recored.

A sample of transfer content:
{
"amount": 9000000000,
"blind_sum": "0a6cb4c07ec99c15b241692bee3572e0eec4823c9302721555df7ba0128bba94",
"tx": "00000000007a12000000000000002e..."
}
For that matter, I decided to generate the timestamp automatically when recording the transfer into wallet.stats file. (Almost around the time the wallet processes receiving and sending coins.) So the best description for timestamp is "sent at" for sending or "received at" for receiving. Common timestamp headline for sending and receiving can be "Sent or Received at."

timestamp_sec is the value of time::TimeSpec.sec and timestamp_rfc822 is the human readable format of timestamp_sec. "timestamp_sec" can be changed to "created_at." Also timestamp_rfc822 can be removed not to be confused.

Thank you

[heunglee]

Re: Destination ("wallet address" in table headline)
When receiving grin coins, the destination is the receiving node which is the localhost. So I just set to localhost without port information. However, I was not sure what the proper description would be for it.

For history, the wallet address for receiving would not be a matter because it is my wallet on full node.
Maybe not?

[heunglee]

For users not to be confused, made some changes as follows:

[sesam]

Great job. I like how it's hard to misunderstand.