Miner transaction aggregation can effectively burn coins

[DaMatrix]

While experimenting with sending transactions on testnet, my miner node has caused me to irreversibly lose a few thousand testnet coins.

Here were the steps I took leading up to the loss of the coins:

1. Started user node
2. Generated new address 79df731ca45193797c07797be6a5b94dc471e025f9b529ba60a6e2f3f95b0e33 using GET /payment_address
3. Using POST /make_payment, sent the entire wallet balance (3570679049403 tokens, copied from available_total_tokens) to 79df731ca45193797c07797be6a5b94dc471e025f9b529ba60a6e2f3f95b0e33: https://explorer.aiblock.dev/transaction/g871ec2973f09e7a66812b90139e5158
   (this transaction is very big, the explorer takes like 30 seconds to load the page)
4. Using POST /make_payment, sent 1234567 tokens to 79df731ca45193797c07797be6a5b94dc471e025f9b529ba60a6e2f3f95b0e33: https://explorer.aiblock.dev/transaction/gf1df6a0ad9c10a39eeacb45e3db3b88
   This redeemed the output from the first transaction. As not all the input tokens were sent, the wallet automatically generated a new address 55bbca2481816632e39d0a1a86869821262844326f56c993e8d422693ff46c75 to receive the remaining 3570677814836 tokens
5. Stopped user node
6. Started combined user/miner node with ADDRESS_AGGREGATION_LIMIT unset (causing it to default to 999), and using the same wallet database as the user node
7. As the wallet contains over 1300 addresses, it promptly tried to run address aggregation. The miner submitted multiple empty transactions over the course of a few minutes.
   At some point during this process, the address 55bbca2481816632e39d0a1a86869821262844326f56c993e8d422693ff46c75 got removed from the wallet database, along with the corresponding keypair. However, the 3570677814836 tokens never left the address, and are now completely inaccessible as I don't know what the private key was.
8. After a few minutes, I noticed the balance had dropped significantly and restarted the node to see if that would make it show up again (as this has helped a few times in the past), then stopped it again when the balance didn't show up and I realized that my node was sending empty transactions to the mempool.

I don't yet understand the codebase well enough to understand what exactly could have caused this, but I can tell that it's probably related to destroy_spent_transactions_and_keys() incorrectly deciding that the 2nd output of gf1df6a0ad9c10a39eeacb45e3db3b88 had been spent, and that the key could therefore be pruned.

Something that stands out to me while looking at the transaction creation code is that there are quite a few places where the database is updated in a non-atomic manner: for example, fetch_tx_ins_and_tx_outs_merge_input_addrs() calls consume_inputs_for_payment(), which erases the outputs from the database fund store before the transaction is actually fully built. If something fails later in the process, or if the mempool node rejects the transaction for whatever reason, the wallet will end up in an inconsistent state where the outputs are removed from the database but not actually spent. (The latter case is mentioned in an existing TODO: https://github.com/AIBlockOfficial/Network/blob/fa7accd/src/miner.rs#L1452-L1453 )

I have backups of the wallet state from immediately before transaction gf1df6a0ad9c10a39eeacb45e3db3b88 was made and from shortly after I realized the keys for 55bbca2481816632e39d0a1a86869821262844326f56c993e8d422693ff46c75 had been deleted, can upload those if they'd be of any help.