Wallet storage 'shuffled_coins' data structure is buggy and will produce incorrect results

[cculianu]

See:

https://github.com/clifordsymack/Electron-Cash/blob/cd586516a8640c17f66549b0063afe5124c13cea/lib/wallet.py#L731

And also:

https://github.com/clifordsymack/Electron-Cash/blob/cd586516a8640c17f66549b0063afe5124c13cea/lib/coinchooser.py#L172

It is the absolutely wrong approach to store the 'shuffled' UTXOs in the wallet as a list of coins to choose from.

UTXOs are not permanent things, but rather transient entities.

This code is missing:

• UTXO-removal code for a shuffled coin.
  • What happens if you spend that coin on your iOS app wallet, then you synch. up later with your desktop wallet (same wallet)? This shuffled coin list now contains invalid outputs -- which the coinchooser will use because the coinchooser is broken now due to the second link above.
• Validation of the shuffled-coin list versus the actual extant UTXOs -- this needs to be done when switching chains.
• My reading of the code leads me to believe even just spending a shuffled coin will result in it not being removed from the 'shuffled_coin' list and the next time around the coinchooser will attempt to double-spend it again and again and again....

This is a high priority bug.

[markblundeberg]

We definitely need to test the spending mechanisms during the next shuffle test. Creation of shuffled coins is only half of what gives you privacy -- the spending has to be done correctly too, or else all benefits are lost.

[cculianu]

Well, ideally yes. That's the goal, agreed.

However -- right now the code is in such a state that even it just functioning as a wallet is not something you can expect of it.

[cculianu]

Ok, I pushed a commit that undoes the damage discussed above. Will re-code the wallet logic (actually it can all be done from the send screen) to actually spend shuffled tx's as appropriate.