sickpig / bch-rpc-explorer

Database-free, self-hosted Bitcoin Cash explorer, via RPC. Built with Node.js, express, bootstrap-v4.
MIT License
57 stars 128 forks source link

Negotiate higher version against electrum server to show balance on token addresses #190

Closed imaginaryusername closed 1 year ago

imaginaryusername commented 1 year ago

Per the fulcrum get_balance method, BCH balance on "token" addresses will only be shown if version is >1.5. This has led to results such as https://chipnet.imaginary.cash/address/bchtest:pzn85ff863dnceq0gt5sd88e00afchyur5jasutc0s?sort=desc&limit=10&offset=10 where BCH balances on token'd UTXOs are excluded, and the overall balance is shown as zero.

I don't know where it is, but since token support is added the explorer should be able to report a version of 1.5.0 and make Fulcrum report BCH balance on tokens again.

sickpig commented 1 year ago

actually we are using @dagurval rostrum as electrumX protocol implementation.

having said that I think that the version of rostrum we are using (8.1.0) support the token_filter optional parameter for the get_balance so it seems like just a matter of adapting the /address/ view.

will work on that

sickpig commented 1 year ago

@imaginaryusername

Little heads up

So I set up py script that establishes a connection to your fulcrum instance @ chipnet.imaginary.cash:50001, negotiates protocol version to 1.5 and then issue a get_balance for this address bchtest:pzn85ff863dnceq0gt5sd88e00afchyur5jasutc0s

if ”include_tokens” optional param is passed to get_balance the result we have is:

{
  "id": 0,
  "jsonrpc": "2.0",
  "result": {
    "confirmed": 16000,
    "unconfirmed": 0
  }
}

if ”exclude_tokens” optional param is passed to get_balance the result we have is:

{
  "id": 0,
  "jsonrpc": "2.0",
  "result": {
    "confirmed": 0,
    "unconfirmed": 0
  }
}
imaginaryusername commented 1 year ago

The docs call it token_filter at Fulcrum too but yes that does look right to me.

sickpig commented 1 year ago

The docs call it token_filter at Fulcrum too but yes that does look right to me.

rostrum calls the filter parameter the same, namelytoken_filter, exclude_tokens is actually one of the 3 possible values for that param, see:

https://bitcoinunlimited.gitlab.io/rostrum/protocol/methods/#blockchainaddressget_balance

I'm going to open a PR to fix the problem you described in the OP, namely negotiate version 1.5 for electrum protocol (only if using Fulcrum, 1.4 is enough for Rostrum).

Having said that I think that Fulcrum 1.9.1 behaviour is not coherent with the timing of network upgrades activation, I mean any clients speaking protocol 1.4 should not be aware of token existence, so what's the point of excluding UTXOs from the computation of address amount in case they are "attached" to a cashtoken?

maybe something to forward to @cculianu

imaginaryusername commented 1 year ago

This was a result of two goals:

  1. The desire to not have 1.4 clients spend tokens accidentally. They already can't do that for the most part since cashtokens utxo signing method is changed.
  2. The desire to not have 1.4 clients see "unspendable" UTXOs they cannot understand, both in code and in meatspace

Removing visibility of token-attached sats to 1.4 clients satisfy both of these goals. If we forward the sats to them anyway, upon receipt there'll be some unspendable and unexplained sats which can cause UX problems.

sickpig commented 1 year ago

This was a result of two goals:

  1. The desire to not have 1.4 clients spend tokens accidentally. They already can't do that for the most part since cashtokens utxo signing method is changed.
  2. The desire to not have 1.4 clients see "unspendable" UTXOs they cannot understand, both in code and in meatspace

Removing visibility of token-attached sats to 1.4 clients satisfy both of these goals. If we forward the sats to them anyway, upon receipt there'll be some unspendable and unexplained sats which can cause UX problems.

mmmm I have to say that I found the above arguments weird.

Just for the sake of clarity, let me explain my point of view on this matter.

Suppose for a moment we erroneously send UTXOs that represents cashtokens to a client that speaks 1.4 protocol only.

Your point is: since it is probably impossible (not certain) that they won't be able to spend those UTXOs due to signature scheme changes why bother showing the UTXOs among the total amount.

My point is:

just my 2 cents.