Coloured coin GUI test

[Naviabheeman]

For each type of token this test does:

• Issue → check IssueTokenDialog table shows type/balance, OverviewPage balance , transaction record = TokenIssue
• Send to own address -> check balance, transaction = SendToAddress
• Burn -> check balance, transaction = TokenBurn
• Reissue token -> check balance, transaction = TokenIssue
• Verify that then token is removed from OverviewPage combo and token table when the balance is 0

Receive / payment request (using the surviving REISSUABLE token)

• Switch ReceiveCoinsDialog to token mode, select the token, enter label/amount/message
• Click Receive → verify the payment info popup shows correct URI, token colorId, amount, label, message
• Verify entry added to requested-payments table
• Click Clear → inputs reset, token combo disabled
• Remove the entry → row count decreases

[azuchi]

This loop is unnecessary since we have already iterated through all colored outputs in the tokenCredit loop (lines 193-196).

[azuchi]

In transactionrecord.cpp, removing the ISMINE_NO check on line 197 changes the code path for send transactions:

Before: creditAmt = 20 (change only), netAmt = -80 → falls into else if (debitAmt > 0) branch → tokenAmount = -80

After: creditAmt = 100 (all outputs), netAmt = 0 → falls into netAmt == 0 branch → tokenAmount = +80

The sign of tokenAmount flips from negative to positive for sends. Is this intentional? If so, it might be worth adding a test assertion to verify the expected tokenAmount value (e.g., checking that a send of 30 tokens shows tokenAmount = 30).

[Naviabheeman]

This was added to identify the token issue and token burn transactions properly. Whether or not the output belongs to us, we include it in the total credit calculation. Then token issue is tokenCredit > 0 and tokenDebit =0 and burn is tokenCredit < tokenDebit. However I hadn't noticed that it flipped the amount in the case of token transfers. I'll check.

[azuchi]

I have a few remaining items:

Outstanding Items

1. Redundant loop (unaddressed from previous review)
The explicit loop at transactionrecord.cpp:208-211:

  for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) {                                                                                                                                                                     
      if (!wtx.txout_color_id[i].empty())                                                                                                                                                                                      
          allColorHexes.insert(wtx.txout_color_id[i]);                                                                                                                                                                         
  }

is redundant. Since the ISMINE_NO check was removed at line 197, tokenCredit now contains every colored output already, so the immediately preceding for (const auto& e : tokenCredit) allColorHexes.insert(e.first) covers it. Please remove this loop.

2. Add tokenAmount value assertion in tests

The sign-flip fix is not covered by the test suite. checkLatestTx only verifies the record type (e.g., SendToAddress), not the tokenAmount value. A regression that re-flips the sign or zeroes out the amount would pass silently.

Please extend the helper, e.g.:

checkLatestTx(TransactionRecord::SendToAddress, cid, /*expectedAmount=*/-30);

and assert the expected signed value for at least the issue / send / burn cases.

3. Add an external-recipient transfer test

All current tests use getColoredAddress(cid) (self-transfer). The code path that the sign fix actually corrects — sending to a non-wallet recipient — is not exercised. Please add at least one case sending to an address owned by a separate CWallet instance and verify the sender side records tokenAmount = - and type SendToAddress.

4. (Optional) Clarify isIssue to match the stated intent

In your earlier reply you described the intent as "issue = tokenCredit > 0 && tokenDebit == 0". The current expression goes through netAmt:

bool isIssue = (netAmt > 0 && fAllFromMe && !isBurn); 

Rewriting it to mirror the stated invariant directly would make the logic easier to read and harder to break in future edits:

bool isIssue = (debitAmt == 0 && colorOutputTotal > 0 && fAllFromMe);

The !isBurn guard becomes unnecessary because debitAmt == 0 already excludes burns.

Items 1–3 are blocking; item 4 is a nice-to-have. Once items 1–3 are addressed, this is good to merge.