Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verbose transactions are currently unsupported with verbose flag of blockchain.transaction.get #7342

Closed
MrNaif2018 opened this issue Jun 9, 2021 · 5 comments
Labels
topic-network 🕸 related to logic in network.py (etc)

Comments

@MrNaif2018
Copy link
Contributor

Electrum version: 4.0.3 (exact commit)

In our APIs we provide a get_transaction RPC call, which works a bit different than usual gettransaction command

raw = await self.session.send_request('blockchain.transaction.get', [tx_hash], timeout=timeout)

We do the same, but with verbose parameter turned on.

Here is how it is implemented in our API

It works perfectly in most cases and returns an additional field: confirmations in addition to usual transaction JSON output.
But in some cases, more frequently on testnet, and especially often in our CI runs, we get the following error:
aiorpcx.jsonrpc.ProtocolError: (-32600, 'ill-formed response error object: verbose transactions are currently unsupported')

CI Run logs
The failing test
Note: the SDK being tested is just calling that get_transaction method on daemon directly, it's still electrum

Are those servers with unsupported verbose transactions just running some old version of ElectrumX? Is there a way to filter them out from the list of servers to connect to, or is there a better way to get confirmations for an arbitrary transaction (not always wallet-related)?

@SomberNight
Copy link
Member

SomberNight commented Jun 27, 2021

verbose transactions are currently unsupported

No idea which server implementation this might be coming from.

Are those servers with unsupported verbose transactions just running some old version of ElectrumX?

I don't think it is ElectrumX.

Is there a way to filter them out from the list of servers to connect to

No, the protocol does not really support optional features / feature negotiation.
There is only protocol version negotiation; and servers are supposed to support all features defined for the negotiated protocol version.

or is there a better way to get confirmations for an arbitrary transaction (not always wallet-related)?

With current protocol (1.4), you would need to

  • deserialise the tx,
  • request the history for one of its output addresses (which will include the txid and the block height it is mined at),
  • if you want SPV guarantees, get merkle proof for txid at block height

With protocol 1.5 (spesmilo/electrumx#90), you will be able to just use blockchain.transaction.get_merkle(txid)

@MrNaif2018
Copy link
Contributor Author

From my tests looks like the server lacking verbose support is Blockstream electrs. It can be reproduced by setting server to electrum.blockstream.info:50002:s. It happens on our CI systems when this server gets randomly picked, or other servers running electrs by blockstream. I will try to push adding verbose support to it forward, and with protocol 1.5 (or maybe earlier) will implement proper SPV checking of transactions

@MrNaif2018
Copy link
Contributor Author

With current protocol (1.4), you would need to

deserialise the tx,
request the history for one of its output addresses (which will include the txid and the block height it is mined at),
if you want SPV guarantees, get merkle proof for txid at block height

@SomberNight, I have tried implementing the approach you mentioned, that's what I've got:

async def verify_transaction(self, tx_hash, tx_height):
    merkle = await self.network.get_merkle_for_transaction(tx_hash, tx_height)
    tx_height = merkle.get("block_height")
    pos = merkle.get("pos")
    merkle_branch = merkle.get("merkle")
    async with self.network.bhi_lock:
        header = self.network.blockchain().read_header(tx_height)
    self.electrum.verifier.verify_tx_is_in_block(tx_hash, merkle_branch, pos, header, tx_height)


async def get_transaction(self, tx_hash):
    result = await self.network.get_transaction(tx_hash)
    tx = self.electrum.transaction.Transaction(result)
    tx.deserialize()
    result_formatted = tx.to_json()
    address = None
    for output in result_formatted["outputs"]:
        if output["address"] is not None:
            address = output["address"]
            break
    if address is None:
        raise Exception("Invalid transaction: output address is None")
    scripthash = self.electrum.bitcoin.address_to_scripthash(address)
    history = await self.network.get_history_for_scripthash(scripthash)
    tx_height = None
    for tx_info in history:
        if tx_info["tx_hash"] == tx_hash:
            tx_height = tx_info["height"]
            break
    if tx_height is None:
        raise Exception("Invalid transaction: not included in address histories")
    current_height = self.network.get_local_height()
    confirmations = 0 if tx_height == 0 else current_height - tx_height + 1
    result_formatted.update({"confirmations": confirmations})
    await self.verify_transaction(tx_hash, tx_height)
    return result_formatted

Is that secure now? The only thing that concerns me is that it is fetching history for the whole address. Is that a current limitation in 1.4 protocol? But at least that exception is no longer here. Would you accept a PR changing gettransaction or adding new command to receive confirmations for a transaction in an SPV safe way? Something like mentioned here: #5660 (comment)

@SomberNight
Copy link
Member

Yes, something like that should work.

The only thing that concerns me is that it is fetching history for the whole address. Is that a current limitation in 1.4 protocol?

Yes, there is no neat way to do this with the current protocol. :/
It can get quite expensive to download the full history of a busy address...

Would you accept a PR changing gettransaction or adding new command to receive confirmations for a transaction in an SPV safe way?

I think we should wait for protocol 1.5 first. After that, yes, we could add a new method that takes a txid, and returns an SPV verified number of confirmations.

@shesek
Copy link

shesek commented Oct 6, 2021

From my tests looks like the server lacking verbose support is Blockstream electrs.

Yes, that is correct, the Blockstream electrs fork does not support the verbose option. See Blockstream/electrs#36 for more information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-network 🕸 related to logic in network.py (etc)
Projects
None yet
Development

No branches or pull requests

3 participants