Electron-cash: watching-only wallet on complex P2SH does not detect spends properly

[markblundeberg]

Example: Create a watching-only wallet for bitcoincash:pqt79ay4h05q2dk889wn28l66942mgrtgqa7jjqt3h . This is an atomic swap contract, which has a quite complex scriptSig. The second transaction in this list actually spents the earlier (third) transaction's output, but EC does not display this properly. Likewise in the Coins tab it lists a coin that was already spent.

This is a fairly deep logical mistake that looks like it may have been since fixed in spesmilo upstream. The problem is this line, which assumes that by examining a tx input you can guess the address it came from:

https://github.com/Electron-Cash/Electron-Cash/blob/06f6b1c7888b8a1d257e9489bc8a4ace308a57a9/lib/wallet.py#L712

In reality this assumption is not true. You can't confidently know the address being spent from unless you actually examine the prior transaction's scriptPubKey. It is indirectly relying on the logic in transaction.py's parse_scriptSig() but that is simply an approximated crutch and shouldn't be relied on in this way. Really, transaction input dicts should not have an 'address' field at all.

To fix this requires a logical shift in the way that transaction inputs are thought about and would require a fairly comprehensive fixing of various code bits. However, this only affects complex P2SH addresses (not multisigs) so it's not an urgent fix.

[SomberNight]

Yes, this was fixed in Electrum, mostly in https://github.com/spesmilo/electrum/pull/4405 (but I think this PR depends on other previous changes too).

[cculianu]

@SomberNight Thanks for pasting the commit. Yeah, it's a pretty comprehensive change but it makes a lot of sense. We may want to consider manually integrating very similar changes (at this point using git to merge would be difficult as our code and yours differ significantly).

[markblundeberg]

@SomberNight thanks for the link ... yes, that's about the level of comprehensiveness I was expecting to see, actually a bit more but as you say some other commits were probably involved. Makes sense that dealing with segwit would spur this kind of change. :-) I think to fix this in EC would probably also require a new wallet file version that has the 'spent_outpoints' datastructure.

This is definitely not a pressing change, but it will be an annoyance for anyone doing smart contracts (like me) in EC. I have already worked around it in my project with just a few lines of code though, no biggie.

[cculianu]

We really need to pull that from upstream. Gah. It's complex. Will do this eventually.. maybe in the next week or two. It bugs me to all hell that the wallet can't grok the balances ok when it receives weird tx's... so.

It's rare .. for now.. but could become more common in the future.