Fix wrong CT / RingCT amounts in transaction history when wallet locked

[us77ipis]

Problem

When receiving a CT transaction while the wallet is locked (or unlocked for staking only), the amount displayed in the transaction history is wrong, and keeps wrong until a rescanringctwallet rpc is called and the wallet is restarted.

Root Cause

• More concretely the wrong amount showed is -1 satoshi per output in the transaction, which comes from the default value in the following line: https://github.com/Veil-Project/veil/blob/7c8bee132ea6305b29a613679b7591a27d8edbd8/src/veil/ringct/outputrecord.h#L43
• Furthermore, for the case when the transaction contained a change output, there was a bug in the way the GUI calculates the transaction amount, which resulted in that default -1 satoshi also being added for the change output (which isn't owned by the receiving wallet), thus the final amount was in this case always displayed as 1 satoshi less than the actual transaction was.
• For incoming RingCT transactions the ORF_OWNED flag was not set correctly, leading to RingCT transactions being showed as "(n/a)" in the transaction history, if they arrived while the wallet was locked.

Solution

• When a CT transaction arrives while the wallet is locked (or unlocked for staking only):
  • Instead of a wrong amount show "Hidden" in the transaction history entry.
  • In the tooltip of the transaction row show the message "Unlock wallet to see hidden amount".
  • In the "Incoming transaction" GUI notification also show "Hidden" instead of a wrong amount.
• Fix rescanringctwallet to properly update corresponding entries in the transaction history GUI.
• Keep track of locked transactions and when the wallet is then unlocked, automatically do a partial rescanringctwallet which only rescans those tracked locked transactions, and thus should be fast also for wallets having a very long transaction history.

Bounty Payment Address

sv1qqpjsrc60t60jhaywj5krmwla52ska70twc7wun6qnee65guxhvtxegpqwhuxypra4jn3pq86s24ryltcw6g2ss4573hyqac9u4g23m9mvxpyqqqwny49k

Testing Results

• Tested by sending basecoin/CT/RingCT from one testnet wallet to CT/RingCT of another testnet wallet, where the received amount showed up first as "Hidden" and once the wallet was unlocked, it showed the correct amount.
• Tested unlocking of the wallet while the transactions are still unconfirmed.
• Tested restarting the wallet between transaction arrival and unlocking of the wallet (state consistency across wallet restarts).

Fixes #922, #668

[us77ipis]

The current changes of the PR already work and solve the problem. However, it is still marked as draft because currently when the wallet is unlocked it does the full rescanringctwallet in the UI thread, which I don't think is a good idea. We could do one of the following things (or both?) to optimize this:

• Do rescanringctwallet in another thread, when the wallet is unlocked.
• Don't do a full rescanringctwallet, but instead keep track of "locked" transactions and then only specifically rescan those when the wallet is unlocked.

[us77ipis]

Btw. what will the bounty be for solving this soon three year old issue with as far as I can read in #922 quite high impact for Veil users :wink: ?

[us77ipis]

Just noticed it still doesn't work if the user unlocks the wallet while the transaction is still unconfirmed.

[seanPhill]

I've got this PR running on Testnet, and firstly ran the released standard released version, to compare, but this time, unlocking has so far taken 2.5 hours, and is continuing to show the macOS beach ball of waiting, and no progress in the debug log. :( Admittedly, this wallet is normally slow (because the wallet.dat file is moderately large, and it has done a huge number of stakes and spends), but not like this. I have been able to rescanringctwallet with no problem, but with PR1001 it's just not completing. This is programmed to only do a single rescanringctwallet upon unlock, right? There was probably at least thirty or forty CT (from incoming zerocoin spends) incoming transactions that had not got a value except for "0.00000001" (as displayed on the released version), and "0.00000000" when opened in PR1001. The beachball of waiting is invisible in the screenshot, but it is still there.

[us77ipis]

That sounds like a locking issue. I think I forgot to lock also cs_main. @seanPhill Please try again with the changes I just pushed.

[us77ipis]

There was probably at least thirty or forty CT (from incoming zerocoin spends) incoming transactions that had not got a value except for "0.00000001" (as displayed on the released version), and "0.00000000" when opened in PR1001.

That's the expected behavior for transactions that had already arrived before. For the "Hidden" text to be displayed the transaction has to have income through the wallet that has the changes of this PR. However, once you unlock the wallet (and it doesn't hang up) also the old "0.00000000" values should be replaced by the correct ones.

[seanPhill]

Tested a second time with the changes at the point in time of my thumbs up above. Fully worked as expected. This wallet took just a few minutes, but a smaller wallet would have been quicker.

[us77ipis]

This PR is now ready for extensive testing and review.

[us77ipis]

I don't know how the translation stuff currently works. Since I added the new strings "Hidden" and "Unlock wallet to see hidden amount", should some translations be added? If yes, how exactly is that handled?

[seanPhill]

I just had an issue with this function tonight. [Don't know if this is related to the requested changes.]

While it has normally worked fine for me, my testnet wallet had been locked (and / or closed) and receiving a lot of transactions for about twenty hours, and maybe I unlocked it a bit soon after starting it up, but when I did, the transactions this time did not correct themselves. I was still getting the mouseover message about needing to unlock to see the correct amounts. I waited a few minutes, and tried locking the wallet again and unlocking it again, and it still did not correct the transactions list. ... I then manually did rescanringctwallet, and it promptly fixed all the amounts.

This is a wallet with over 55,000 transaction rows in the course of twenty days from fresh seed. The issue was with the 62 received CT amounts over the course of 20 hours, while the wallet was either locked or turned off.

[us77ipis]

[Don't know if this is related to the requested changes.]

I already pushed the requested changes (and marked them as solved), don't know why it still shows "Changes requested".

I then manually did rescanringctwallet, and it promptly fixed all the amounts.

If full rescanringctwallet worked, then that is an issue with the optimization, thus for whatever reason those transactions didn't get tracked as locked transactions.

[seanPhill]

I've retested with the wallet again, with a few days of transactions to reveal. No problem. With a day's worth of transactions synced while locked, and still more than a day of transactions left to sync, when unlocked, it revealed the amounts and continued syncing, no problem. Closed the wallet, and restarted with more than a day of transactions to sync, and then syncing completely while locked, when unlocked, it worked as per normal, the transactions were revealed over no more than three seconds, and everything was fine.

That failure a few days ago I will have to consider an anomaly that might not ever happen to anyone without such a large wallet as my test wallet. Could not be reproduced, and the solution was very simple anyway (rescanringctwallet).

[seanPhill]

@Zannick can you confirm that your requested changes have been made? ... -> code review. Everything should be ready.

[WetOne]

utACK 6333d875984d0cecaceefa6d2f7a830755274a44

[seanPhill]

This PR solves the issue that caused an exchange in early 2019 to reverse their decision to allow stealth withdrawals. I hope that this PR can be included in the next release, which will enable exchanges to enable stealth withdrawals. @codeofalltrades