[GUI] Atomic swaps appear incorrectly in the wallet view.

[ben-abraham]

Swaps performed between two parties do not correctly show in the QT wallet.

From the wallets' perspective, swaps can take on 4 forms (really 6 but keeping it simple for now) The picture above is taken from the perspective of wallet 'B'

1. A Creates Buy Order, B executes (currently shows like the bottom 4 transactions above)
2. A Creates Sell Order, B executes (currently shows like the second-to-bottom set of 3 transactions above)
3. B Creates Buy Order, A executes (currently shows like the second-to-top single transaction above)
4. B Creates Sell Order, A executes (currently shows like the top-most transaction above)

For reference, the full transactions can be viewed here (NOTE: #1 is constructed funny, this is a client bug):

1. 06c69a693eb44409cdbd4777e3abcdf27210d4546fc72d16a81ddc47126a0d3d
2. c55e448e842e5056db56f039089d653474917eccbeb695a3c454c9d3448b15bc
3. 6bef75a5aa9f4cf236c3768fd6f02d7ee40b8812f8583f45aed1243b09b8fcee
4. e7e7a71d713332eb885de73ea426341b9706d9c2132fe92181d941742514d42c

What I'd expect to see (single/two transactions doesn't matter, NOTE: some tx's above use RVNTEST, i'm just simplifying here):

1. -1 SWAPTEST, +1 RVN
2. +1 SWAPTEST, -1 RVN
3. -1 SWAPTEST, +1 RVN
4. +1 SWAPTEST, -1 RVN

Partially what's happening is that wallet.cpp:1673 CWalletTx::GetAmounts is making a couple mistakes in it's calculation.

if (assetoutput.type == TX_TRANSFER_ASSET)
  assetsSent.emplace_back(assetoutput);

One issue is the above block, which will attribute any transfer outputs as "sent" (to counterparty) regardless of the address it is sent to. Fixing that code by checking fIsMine resolves transaction #2 above and partially fixes #1.

The other problems are a bit larger, namely that we don't have full data on the vins of the transaction, so for example with transaction #3, We can see that the output received is 1RVN, but we don't know the values/sizes of the actual UTXOs at that point-in-time to be able to determine what I provided for the exchange. The UTXO's are by definition spent when looking at these transactions in the UI (unless you just submitted them). Simply enumerating the outputs is unreliable because the counterparty could supply any number of additional inputs/outputs in a combined transaction when we have created the order.

We could add a special check for if a/multiple vouts are signed with SINGLE|ANYONECANPAY to at least quickly determine if we should only care about specific vin/vout pair(s), however this still has the same problem right? In that we don't know what the vin side of a specific vin/vout pair was providing for the trade. Would we be able to look up the previous tx since it would always be one of ours? (assuming we are looking at a signed-by-ourselves vin/vout pair)

Just spitballing ideas at this point :) want to open the discussion since it's more complex than initially expected.

[HyperPeek]

I guess 2. and 3. should be RVNTEST asset swaps? As you pointed out this looks tricky to solve, although it should clearly be possible, as from the chain the swap can clearly be identified within the single tx.

[ben-abraham]

I guess 2. and 3. should be RVNTEST asset swaps?

Correct, I switched assets in the middle of testing on accident.

As you pointed out this looks tricky to solve, although it should clearly be possible, as from the chain the swap can clearly be identified within the single tx.

Best explanation I can come up with is that the nodes at the time of transactions have the whole UTXO set in memory, and are thus able to easily identify all the currently-unspent UTXO values in that trade. I'm thinking the method of looking at specific vin/vout pairs is going to be the main way to handle this, but that requires searching and decoding old transactions from myself, which may get a bit expensive (although this code only runs once per tx per app launch best I can tell)

[ben-abraham]

Alright, after working through everything, that process did end up working out it seems. I am now able to get atomic swap transactions to show up in the transaction list like this:

The main detection is done by noticing SINGLE|ANYONECANPAY and determining on which side of the transaction we are on from there. If a swap is detected the old ui row-add logic is bypassed and a swap row is created instead.

For now I simply truncate the asset decimals to 2 since it ends up a bit cleaner in the transaction views, but we should be able to query asset metadata if we want to format to native decimals.