[Wallet][RPC] Export basecoin transactions to CSV file #904

eugene-so · 2021-03-23T15:17:12Z

New Feature

Add RPC exporttransactions to export to a CSV file, similar to the existing listtransactions RPC command

Solution

Added a well tested external CSV library to the project that will make future CSV functionality trivial in the future
Added the rpc exporttransactions command supporting an output filename; and filtering by account, date range, and/or basic piped separated search string

exporttransactions (dummy filename start end categories include_watchonly)

Exports transactions between 'start' date and 'end' date matching 'categories' to a CSV file.
Note: To export from a specified "account", restart veild with -depreciatedrpc=accounts and
use this RPC with an "account" argument

Arguments:
1. "dummy"      (string, optional) If set, should be "*" for backwards compatibility.
2. "filename"   (string, optional) The filename with path (either absolute or relative to veild) [default=<datadir>/export/transactions.csv].
3. "start"      (string, optional) The start date in the format YYYY-MM-DD [default=beginning of ISO8601 time].
4. "end"        (string, optional) The end date in the format YYYY-MM-DD
 [default=present time].5. "filter"     (string, optional) A pipe(|) separated transaction filter [default=no filter]
   Allowable filter words TBD
6. include_watchonly (bool, optional, default=false) Include transactions to watch-only addresses (see 'importaddress')

Bounty Payment Address

sv1qqp6aptgvgp9t9h8sgzkqmu6cgq2e20l9x6fsl5ask7t3ygy2jagftcpq0x5z0h522ca5h06qq3hx33pke00r7gjt3j24n896gf55y68ptrmjqqqqd8lz3

Unit Testing Results

Compare exported CSV with output of existing list transactions command. Should be identical except export CSV shows also sent basecoin transactions which were missing in listtransactions.

Known issues

Only basecoin transactions at this time.
Pulls 'send' transactions from multi-destination transactions that you are a receiver.

Rock-N-Troll

utACK
-hardcoding dates in ExportTransactions 2021-0201:00:00 and such seems like a code smell to me
-log print statement to print # extended private masterkey also seems unneccessary and risky?

eugene-so · 2021-03-27T08:08:29Z

utACK
-hardcoding dates in ExportTransactions 2021-0201:00:00 and such seems like a code smell to me
-log print statement to print # extended private masterkey also seems unneccessary and risky?

Sure, I can default the start time to absolute zero instead then.
Where do you see the log print statement of the private master key?

Rock-N-Troll · 2021-03-27T13:34:54Z

utACK
-hardcoding dates in ExportTransactions 2021-0201:00:00 and such seems like a code smell to me
-log print statement to print # extended private masterkey also seems unneccessary and risky?

Sure, I can default the start time to absolute zero instead then.
Where do you see the log print statement of the private master key?

5722 in wallet.cpp -> LogPrintf("# extended private masterkey: %s\n\n", EncodeExtKey(masterKey));

but it looks like that code is removed

seanPhill · 2021-03-27T13:46:48Z

Hi Eugene, would you like me to send you a used testnet wallet? 75MB of wallet.dat. (I've had much larger, but this should suffice.) It's also got some millions of RingCT if I remember correctly. I'm moving and cannot maintain this wallet at present.

eugene-so · 2021-03-28T15:41:30Z

Hi Eugene, would you like me to send you a used testnet wallet? 75MB of wallet.dat. (I've had much larger, but this should suffice.) It's also got some millions of RingCT if I remember correctly. I'm moving and cannot maintain this wallet at present.

Sure, I received a test wallet.dat already, but another one will only help my testing.

eugene-so · 2021-03-28T16:00:33Z

@Rock-N-Troll,
I have successfully exported the first csv file and currently export the following fields, similar to the listtransactions option:

typedef enum
{
TRANSACTION_CSV_FIELD_WATCHONLY,
TRANSACTION_CSV_FIELD_ACCOUNT,
TRANSACTION_CSV_FIELD_ADDRESS,
TRANSACTION_CSV_FIELD_CATEGORY,
TRANSACTION_CSV_FIELD_AMOUNT,
TRANSACTION_CSV_FIELD_LABEL,
TRANSACTION_CSV_FIELD_VOUT,
TRANSACTION_CSV_FIELD_FEE,
TRANSACTION_CSV_FIELD_CONFIRMATION,
TRANSACTION_CSV_FIELD_GENERATED,
TRANSACTION_CSV_FIELD_BLOCKHASH,
TRANSACTION_CSV_FIELD_BLOCKINDEX,
TRANSACTION_CSV_FIELD_BLOCKTIME,
TRANSACTION_CSV_FIELD_TRUSTED,
TRANSACTION_CSV_FIELD_WALLETCONFLICTS,
TRANSACTION_CSV_FIELD_TXID,
TRANSACTION_CSV_FIELD_TIME,
TRANSACTION_CSV_FIELD_TIMERECEIVED,
TRANSACTION_CSV_FIELD_COMPUTETIME,
TRANSACTION_CSV_FIELD_BIP125_REPLACEABLE,
TRANSACTION_CSV_FIELD_ABANDONED,
TRANSACTION_CSV_FIELD_COUNT
} TRANSACTION_CSV_FIELDS;

Currently, everything is exported in the same format as listtransactions. Would it be helpful if the time related fields are converted to human readable format? Do you see any fields that should be added or removed (ie. unnecessary)?

eugene-so · 2021-04-02T03:44:35Z

I have verified this against the listtransactions feature using two test wallets. All the exported data looks good as far as I can tell. I also tested the data range and pipe separated filter features It all works as I expect.

eugene-so · 2021-04-02T04:37:28Z

Do you guys know why the OSX build failed? I don't believe I touched anything related to that error.

Rock-N-Troll · 2021-04-03T04:45:09Z

Do you guys know why the OSX build failed? I don't believe I touched anything related to that error.

Currently there is an issue with the build process in Github it seems. CaveSpectre is investigating.

eugene-so · 2021-04-10T16:31:13Z

Problem

Add feature to export wallet transactions to CSV file, similar to the existing listtransactions command

Root Cause

New feature

Solution

Added a well tested external CSV library to the project that will make future CSV functionality trivial in the future
Added the rpc exporttransactions command supporting an output filename; and filtering by account, date range, and/or basic piped separated search string

Issue

#518

Bounty Payment Address

sv1qqp6aptgvgp9t9h8sgzkqmu6cgq2e20l9x6fsl5ask7t3ygy2jagftcpq0x5z0h522ca5h06qq3hx33pke00r7gjt3j24n896gf55y68ptrmjqqqqd8lz3

Unit Testing Results

Compare exported CSV with output of existing list transactions command. Should be identical except export CSV shows also sent basecoin transactions which were missing in listtransactions.

codeofalltrades · 2021-04-16T03:26:13Z

Testing on window 10 I noticed the file is overwritten if the filename is not changed on future exports.

eugene-so · 2021-04-17T00:26:00Z

Testing on window 10 I noticed the file is overwritten if the filename is not changed on future exports.

What should it do rather than overwrite?

codeofalltrades · 2021-04-17T03:25:43Z

Testing on window 10 I noticed the file is overwritten if the filename is not changed on future exports.

What should it do rather than overwrite?
I think it makes sense. I don't see a way to ask to override. I was leaving a note for myself.

src/wallet/rpcwallet.cpp

codeofalltrades · 2021-04-20T02:51:55Z

Sorry I should have been more clear. We will want to include all transaction types. It might be best to split each additional tx type into separate PR. Let's consider this one the base export function with basecoin tx type.

eugene-so · 2021-04-20T03:32:26Z

Yes that sounds good. Exporting more details about the other transaction types will require a deeper dive and we probably want to improve the listtransactions function while we're at it so a separate or makes sense.

…

________________________________ From: Code of All Trades ***@***.***> Sent: Monday, April 19, 2021 8:52:09 PM To: Veil-Project/veil ***@***.***> Cc: eugene-so ***@***.***>; Assign ***@***.***> Subject: Re: [Veil-Project/veil] [Wallet][RPC] csv wallet transactions export (#904) Sorry I should have been more clear. We will want to include all transaction types. It might be best to split each additional tx type into separate PR. Let's consider this one the base export function with basecoin tx type. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub<#904 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AQ7EU4K66VEC2LKFJVLXOZTTJTT5TANCNFSM4ZVMNJ3Q>.

eugene-so · 2021-05-21T04:55:40Z

What is the status of this review?

codeofalltrades

Please squash to 1 commit. Otherwise good to go for basecoin transactions.
ACK 1c9f6f8

eugene-so · 2021-05-25T14:49:26Z

Please squash to 1 commit. Otherwise good to go for basecoin transactions.
ACK 1c9f6f8

Squashed

codeofalltrades · 2021-05-26T05:30:20Z

@CaveSpectre11 ready for final review.

CaveSpectre11

Note that there's some other issues that need to be sorted out. For example, listtransactions don't show send transactions that I was actually the receiver on:

bv1q7cmcjcazyxger9y9llnqpuhj33pmrn9hl0ny24	receive	42000
bv1qmwyp37wm7nfyhrmf3wdkugquwsl8qrq2jku4he	send	1023
bv1qjg9nzaufgurar3muy7nuqkdk4987egm8cwtrh4	send	3066
bv1qn6nl6djp7fw5vqdmf4zdyqazy9fxc8gmkmwv6c	send	3623
bv1ql7kjjxvq8sd0l23c62kdydehp8a65vjljs2kkd	send	100000
bv1q93799xecdl50tcnh99epx5kwww42rxs4umt75x	send	30000
19cumt48xkv8s3uegtwqr0c59uapfjvscv5y9z3	send	4794309.99

I'm not quite sure how listtransactions is preventing those "sends", but they are other outputs in the same transaction this wallet received the 42000.

But that can be fixed at a later date.

For now, only the text in the exporttransactions help needs to be addressed.

CaveSpectre11 · 2021-05-30T14:44:32Z

src/wallet/rpcwallet.cpp

+    if (!IsDeprecatedRPCEnabled("accounts")) {
+        help_text = "exporttransactions (dummy filename start end categories include_watchonly)\n"
+            "\nExports transactions between 'start' date and 'end' date matching 'categories' to a CSV file.\n"
+            "Note: To export from a specified the \"account\", use this RPC with an \"account\" argument and restart\n"
+            "veild with -deprecatedrpc=accounts\n"


NIT. It is best to isolate the differences rather than duplicate a lot of code that would make changes needed to be done in multiple places.

e.g.
help_text = "exporttransactions (" + IsDeprecatedRPCEnabled("accounts") ? "\"account\"" : "dummy" + "..."

if (!IsDepreciatedRPCEnabled("accounts")) { help_text = help_text + "Note: To export from..." } help_text = help_text + "<common stuff here>"

src/wallet/rpcwallet.cpp

CaveSpectre11

7e49d8e - needs to have the help text for exporttransactions tweaked (see above comment)

…to CSV format If additional export information is required in the future, we should revisit CWalletTx::GetAmounts() to see if send/receive info can be better extracted from the wallet.

CaveSpectre11

Noticed the help tag had 'categories' and text had 'filter'. Also took care of the indenting nit (and another one seen)

ACK d5b00c3

codeofalltrades requested review from codeofalltrades and CaveSpectre11 March 26, 2021 04:18

codeofalltrades assigned eugene-so Mar 26, 2021

codeofalltrades added Component: RPC Related to the console commands themselves. Component: Wallet Relating to keystore, tx creation, and balance tracking Tag: TransactionRecords The display of transaction information labels Mar 26, 2021

Rock-N-Troll reviewed Mar 27, 2021

View reviewed changes

codeofalltrades changed the title ~~Feature/518 csv wallet transactions export~~ [WIP][Wallet][RPC]Feature/518 csv wallet transactions export Mar 27, 2021

eugene-so force-pushed the feature/518_csvWalletTransactionsExport branch 4 times, most recently from c235394 to 03a6966 Compare April 10, 2021 16:16

codeofalltrades changed the title ~~[WIP][Wallet][RPC]Feature/518 csv wallet transactions export~~ [Wallet][RPC]Feature/518 csv wallet transactions export Apr 11, 2021

CaveSpectre11 changed the title ~~[Wallet][RPC]Feature/518 csv wallet transactions export~~ [Wallet][RPC] csv wallet transactions export Apr 11, 2021

codeofalltrades added the Tag: Waiting For Code Review Waiting for code review from a core developer label Apr 12, 2021

codeofalltrades requested changes Apr 18, 2021

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

eugene-so requested a review from codeofalltrades April 19, 2021 07:44

CaveSpectre11 mentioned this pull request May 2, 2021

Initial push into C++17 branch #929

Closed

codeofalltrades changed the title ~~[Wallet][RPC] csv wallet transactions export~~ [Wallet][RPC] Export basecoin transactions to CSV file May 22, 2021

codeofalltrades approved these changes May 22, 2021

View reviewed changes

eugene-so force-pushed the feature/518_csvWalletTransactionsExport branch from 1c9f6f8 to 7e49d8e Compare May 25, 2021 14:47

CaveSpectre11 approved these changes May 30, 2021

View reviewed changes

CaveSpectre11 self-requested a review May 30, 2021 16:13

CaveSpectre11 requested changes May 30, 2021

View reviewed changes

eugene-so force-pushed the feature/518_csvWalletTransactionsExport branch from 7e49d8e to e11af21 Compare May 30, 2021 17:43

Create exporttransactions feature that exports basecoin transactions …

d5b00c3

…to CSV format If additional export information is required in the future, we should revisit CWalletTx::GetAmounts() to see if send/receive info can be better extracted from the wallet.

CaveSpectre11 force-pushed the feature/518_csvWalletTransactionsExport branch from e11af21 to d5b00c3 Compare May 30, 2021 19:31

CaveSpectre11 approved these changes May 30, 2021

View reviewed changes

CaveSpectre11 added Code Review: Passed and removed Tag: Waiting For Code Review Waiting for code review from a core developer labels May 30, 2021

codeofalltrades merged commit b899ec6 into Veil-Project:master Jun 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Wallet][RPC] Export basecoin transactions to CSV file #904

[Wallet][RPC] Export basecoin transactions to CSV file #904

eugene-so commented Mar 23, 2021 •

edited by CaveSpectre11

Loading

Rock-N-Troll left a comment •

edited

Loading

eugene-so commented Mar 27, 2021

Rock-N-Troll commented Mar 27, 2021 •

edited

Loading

seanPhill commented Mar 27, 2021

eugene-so commented Mar 28, 2021

eugene-so commented Mar 28, 2021

eugene-so commented Apr 2, 2021

eugene-so commented Apr 2, 2021

Rock-N-Troll commented Apr 3, 2021

eugene-so commented Apr 10, 2021 •

edited by CaveSpectre11

Loading

codeofalltrades commented Apr 16, 2021

eugene-so commented Apr 17, 2021

codeofalltrades commented Apr 17, 2021

codeofalltrades commented Apr 20, 2021

eugene-so commented Apr 20, 2021 via email

eugene-so commented May 21, 2021

codeofalltrades left a comment

eugene-so commented May 25, 2021

codeofalltrades commented May 26, 2021

CaveSpectre11 left a comment •

edited

Loading

CaveSpectre11 May 30, 2021

CaveSpectre11 left a comment

CaveSpectre11 left a comment

[Wallet][RPC] Export basecoin transactions to CSV file #904

[Wallet][RPC] Export basecoin transactions to CSV file #904

Conversation

eugene-so commented Mar 23, 2021 • edited by CaveSpectre11 Loading

New Feature

Solution

Bounty Payment Address

Unit Testing Results

Known issues

Rock-N-Troll left a comment • edited Loading

Choose a reason for hiding this comment

eugene-so commented Mar 27, 2021

Rock-N-Troll commented Mar 27, 2021 • edited Loading

seanPhill commented Mar 27, 2021

eugene-so commented Mar 28, 2021

eugene-so commented Mar 28, 2021

eugene-so commented Apr 2, 2021

eugene-so commented Apr 2, 2021

Rock-N-Troll commented Apr 3, 2021

eugene-so commented Apr 10, 2021 • edited by CaveSpectre11 Loading

Problem

Root Cause

Solution

Issue

Bounty Payment Address

Unit Testing Results

codeofalltrades commented Apr 16, 2021

eugene-so commented Apr 17, 2021

codeofalltrades commented Apr 17, 2021

codeofalltrades commented Apr 20, 2021

eugene-so commented Apr 20, 2021 via email

eugene-so commented May 21, 2021

codeofalltrades left a comment

Choose a reason for hiding this comment

eugene-so commented May 25, 2021

codeofalltrades commented May 26, 2021

CaveSpectre11 left a comment • edited Loading

Choose a reason for hiding this comment

CaveSpectre11 May 30, 2021

Choose a reason for hiding this comment

CaveSpectre11 left a comment

Choose a reason for hiding this comment

CaveSpectre11 left a comment

Choose a reason for hiding this comment

eugene-so commented Mar 23, 2021 •

edited by CaveSpectre11

Loading

Rock-N-Troll left a comment •

edited

Loading

Rock-N-Troll commented Mar 27, 2021 •

edited

Loading

eugene-so commented Apr 10, 2021 •

edited by CaveSpectre11

Loading

CaveSpectre11 left a comment •

edited

Loading