search

cculianu / Fulcrum

A fast & nimble SPV Server for BCH, BTC, and LTC
Other
331 stars 77 forks source link

get_balance negative unconfirmed #105

Closed coval3nte closed 2 years ago

coval3nte commented 2 years ago

Sometimes occours that blockchain.address.get_balance method returns a negative integer triggering an Exception followed by a disconnection on electrum side.

  File "/usr/local/lib/python3.10/site-packages/electrum/interface.py", line 1003, in get_balance_for_scripthash
    assert_non_negative_integer(res['unconfirmed'])
  File "/usr/local/lib/python3.10/site-packages/electrum/interface.py", line 94, in assert_non_negative_integer
    raise RequestCorrupted(f'{val!r} should be a non-negative integer')
electrum.interface.RequestCorrupted: -1091317 should be a non-negative integer

Maybe returning zero here when there's a negative integer can be a fix?

References:

cculianu commented 2 years ago

Oh dear.

Hmm. There was a bug in Fulcrum a while back where it would screw up the balance and unconfirmed spends. I'm just curious: What version of Fulcrum in particular are you running? Or does this randomly happen on latest version? If so, is there reliable a way to reproduce this?

coval3nte commented 2 years ago

Hello, I'm using latest Fulcrum's version (v1.6.0)... Furthermore, the error may happen when I'm trying to see the balance of an address which have an unconfirmed out tx

cculianu commented 2 years ago

Hmm. Ok, I did some investigation. I am not sure that Electrum is following the same protocol as the original 1.4 by requiring unconfirmed balance to always be non-negative.

I have to dig some more but it seems to me that the Electrum programmers changed the semantics of the 'unconfirmed' entry in the dictionary. The original 1.4 ElectrumX protocol implementation definitely allowed for this value to be negative in the case where the mempool txns spend all of the unconfirmed coins + confirmed coins. To get the total balance on an address one needed to add 'confirmed' + 'unconfirmed' together and the resulting value (which would always be non-negative) would be the real final balance (assuming mempool txns confirm as-is).

I will do more digging but my initial reaction is that Electrum "altered the deal" here and didn't tell anybody....

Will let you know if that's the case.

cculianu commented 2 years ago

Ok, check this out in current master of ElectrumX:

Will keep you updated as I learn more.

I'm about 90% sure at this point that Fulcrum is not at fault here... since by definition 'unconfirmed' can be negative if it's spending confirmed coins. You're supposed to add the two together to get the real final expected spendable balance...

coval3nte commented 2 years ago

I didn't find any check apart that exception in electrum. Anyway, it's weird that this value can be negative but electrum allows only positive values as I quoted in the first post's references.

cculianu commented 2 years ago

This fails on an ElectrumX server as well.

E/i | interface.[testnet.hsmiths.com:53012] | RequestCorrupted: -45600 should be a non-negative integer
Traceback (most recent call last):
  File "/Users/calin/src/ec/electrum/electrum/network.py", line 845, in make_reliable_wrapper
    await group.spawn(iface.got_disconnected.wait())
  File "/Users/calin/Library/Python/3.8/lib/python/site-packages/aiorpcx/curio.py", line 297, in __aexit__
    await self.join()
  File "/Users/calin/src/ec/electrum/electrum/util.py", line 1266, in join
    self.completed.result()
  File "/Users/calin/src/ec/electrum/electrum/network.py", line 866, in wrapper
    return await func(self, *args, **kwargs)
  File "/Users/calin/src/ec/electrum/electrum/network.py", line 1104, in get_balance_for_scripthash
    return await self.interface.get_balance_for_scripthash(sh)
  File "/Users/calin/src/ec/electrum/electrum/interface.py", line 1023, in get_balance_for_scripthash
    assert_non_negative_integer(res['unconfirmed'])
  File "/Users/calin/src/ec/electrum/electrum/interface.py", line 93, in assert_non_negative_integer
    raise RequestCorrupted(f'{val!r} should be a non-negative integer')
electrum.interface.RequestCorrupted: -45600 should be a non-negative integer

This is a bug in electrum.

cculianu commented 2 years ago

In the above I am connected to: testnet.hsmiths.com:53012, which is an ElectrumX (not Fulcrum) server.

cculianu commented 2 years ago

FWIW, I opened an issue in Electrum, here: https://github.com/spesmilo/electrum/issues/7684