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

Implement blockchain.transaction.get verbose support #36

Open
wants to merge 2 commits into
base: new-index
Choose a base branch
from

Conversation

fujicoin
Copy link

@fujicoin fujicoin commented Jun 1, 2021

Added verbose support to blockchain.transaction.get.
It is necessary to set txindex=1 in the .conf file of coind.
Verbose support is required when used as a backend for AtomicDEX (https://atomicdex.io/).

@fujicoin fujicoin closed this Jun 2, 2021
@fujicoin fujicoin deleted the verbose_support branch June 2, 2021 01:32
@fujicoin fujicoin restored the verbose_support branch June 2, 2021 01:32
@fujicoin fujicoin reopened this Jun 2, 2021
@MrNaif2018
Copy link

@shesek Sorry for the ping, could this get merged? I can help if needed.
Due to the lack of verbose support we often get failures at our CI systems (we use electrum and blockchain.transaction.get with verbose set to True), because electrum connects to electrum.blockstream.info:50002:s or other servers running blockstream electrs.
See spesmilo/electrum#7342

@shesek
Copy link
Collaborator

shesek commented Oct 6, 2021

Hi, thanks for the contribution and apologies for the slow reply!

One of the design goals of the blockstream/electrs fork was to avoid querying the Bitcoin Core RPC in order to serve Electrum RPC requests. The original electrs relies heavily on the Bitcoin Core RPC and can require multiple calls to it to serve a single Electrum RPC request, while this fork never queries the Core RPC to serve user requests (it only queries it periodically to sync new blocks and the mempool).

Adding support for verbose=true would require replicating the Bitcoin Core RPC verbose JSON format using the data stored in rocksdb, without sending an RPC request to Core. This is definitely possible but somewhat cumbersome, which is why it didn't get implemented so far.

Also, generally speaking, I would recommend getting the raw tx and parsing it on your app's side instead of using the verbose option, to keep things more consistent. The electrum server could theoretically return a verbose json object that doesn't match the raw transaction, which could result in inconsistent state and odd behaviors in your app. fwiw the Electrum wallet itself never uses verbose.

@MrNaif2018
Copy link

I see, thanks. I found out that it is indeed not safe to rely on verbose transactions directly, so I get tx confirmations another way. But this will be efficient only with electrum protocol 1.5 when it gets implemented

@fujicoin
Copy link
Author

fujicoin commented Oct 7, 2021

Thank you for the clear answer.

aarani added a commit to aarani/geewallet-notafork that referenced this pull request Jul 14, 2022
Blockstream electrum servers don't support sending
verbose transactions, we use this functionality
for getting confirmations of a transaction, this
causes geewallet to crash.

Blockstream/electrs#36
aarani added a commit to aarani/geewallet-notafork that referenced this pull request Jul 14, 2022
Blockstream electrum servers doesn't support sending
verbose transactions, we use this functionality
for getting confirmations of a transaction, this
causes geewallet to crash.

Blockstream/electrs#36
aarani added a commit to aarani/geewallet-notafork that referenced this pull request Aug 23, 2022
Blockstream electrum servers doesn't support sending
verbose transactions, we use this functionality
for getting confirmations of a transaction, this
causes geewallet to crash.

Blockstream/electrs#36
aarani added a commit to aarani/geewallet-notafork that referenced this pull request Aug 23, 2022
Blockstream electrum servers doesn't support sending
verbose transactions, we use this functionality
for getting confirmations of a transaction, this
causes geewallet to crash.

Blockstream/electrs#36
aarani added a commit to aarani/geewallet-notafork that referenced this pull request Aug 24, 2022
Blockstream electrum servers doesn't support sending
verbose transactions, we use this functionality
for getting confirmations of a transaction, this
causes geewallet to crash.

Blockstream/electrs#36
aarani added a commit to aarani/geewallet-notafork that referenced this pull request Aug 24, 2022
Blockstream electrum servers doesn't support sending
verbose transactions, we use this functionality
for getting confirmations of a transaction, this
causes geewallet to crash.

Blockstream/electrs#36
knocte pushed a commit to nblockchain/geewallet that referenced this pull request Aug 25, 2022
Blockstream electrum servers doesn't support sending
verbose transactions, and we use this functionality
for getting confirmations of a transaction, which
causes a crash.

Blockstream/electrs#36
knocte pushed a commit to nblockchain/geewallet that referenced this pull request Aug 25, 2022
Blockstream electrum servers don't support sending
verbose transactions (see [1]), and we use this
functionality to get confirmations of a transaction;
so we prevent a crash by blacklisting them.

[1] Blockstream/electrs#36
knocte pushed a commit to nblockchain/geewallet that referenced this pull request Sep 4, 2022
Blockstream electrum servers don't support sending
verbose transactions (see [1]), and we use this
functionality to get confirmations of a transaction;
so we prevent a crash by blacklisting them.

[1] Blockstream/electrs#36
knocte pushed a commit to nblockchain/geewallet that referenced this pull request Oct 20, 2022
Blockstream electrum servers don't support sending verbose
transactions (see [1]), and we use this functionality to get
confirmations of a transaction; so we prevent a crash by
blacklisting them.

[1] Blockstream/electrs#36
@nkuba
Copy link

nkuba commented Nov 14, 2022

I see, thanks. I found out that it is indeed not safe to rely on verbose transactions directly, so I get tx confirmations another way. But this will be efficient only with electrum protocol 1.5 when it gets implemented

Hey @MrNaif2018! 👋🏻 Could you please share your workaround to get the transaction confirmations? How do you get them?

nkuba added a commit to keep-network/keep-core that referenced this pull request Nov 16, 2022
The function queries electrum for a details of the specific transaction and
converts it in a format expected by the bitcoin.Chain interface.

The simpliest solution is to use `GetTransaction` function to query electrum
as it returns the transaction details in verbose format. Unfortunatelly it's
not compatible with Esplora/Electrs implementation of ElectrumX server.
(see: Blockstream/electrs#36)

As a workaround to cover integration with Esplora/Electrs we use `GetRawTransaction`
and deserialize the transaction.
nkuba added a commit to keep-network/keep-core that referenced this pull request Nov 16, 2022
The function returns number of confirmations for the given transaction.

As Esplora/Electrs doesn't support verbose transactions details in the
response, that would contain the confirmations number we need to
workaround it. (See: Blockstream/electrs#36)

The workaround:
1. Get the raw transaction
2. Deserialize the raw transaction
3. Find transaction block height by finding it in a history of
   transactions for the output script included in the transaction.
4. Get the latest block height
5. Calculate number of confirmations by subtracting the transaction
   block height from the latest block height and adding one.
@shesek
Copy link
Collaborator

shesek commented Dec 13, 2022

@fujicoin @MrNaif2018 @nkuba What are your thoughts on Blockstream/esplora#423 (comment)?

Could you please share your workaround to get the transaction confirmations? How do you get them?

One workaround is to get the block height via blockchain.transaction.get_merkle, then subtract that from the current chain tip height (which you can get via blockchain.headers.subscribe).

@MrNaif2018
Copy link

MrNaif2018 commented Dec 13, 2022

As for the basic support at least to make it more compatible, it would be good indeed

That's how full SPV-verified transaction fetch should probably be working:
https://github.com/bitcartcc/bitcart/blob/master/daemons/btc.py#L449-L476

But before protocol upgrade it will be kind of slow

nkuba added a commit to keep-network/tbtc-v2 that referenced this pull request Jan 25, 2023
As Esplora/Electrs doesn't support verbose transactions details in the
response, that would contain the confirmations number we need to
workaround it. (See: Blockstream/electrs#36)

The workaround:
1. Get the raw transaction
2. Deserialize the raw transaction
3. Find transaction block height by finding it in a history of
   transactions for the output script included in the transaction.
4. Get the latest block height
5. Calculate number of confirmations by subtracting the transaction
   block height from the latest block height and adding one.
nkuba added a commit to keep-network/tbtc-v2 that referenced this pull request Jan 25, 2023
The simpliest solution is to request verbose format of transaction.
Unfortunatelly it's not compatible with Esplora/Electrs implementation of
ElectrumX server. (see: Blockstream/electrs#36)

As a workaround to cover integration with Esplora/Electrs we get a raw
transaction and deserialize it.
@nkuba
Copy link

nkuba commented Jan 25, 2023

One workaround is to get the block height via blockchain.transaction.get_merkle, then subtract that from the current chain tip height (which you can get via blockchain.headers.subscribe).

@shesek Please correct me if I'm wrong, but according to the Electrum Protocol blockchain.transaction.get_merkle(tx_hash, height) expects block height as an argument, so it can't be used to get the block height.

You can find a possible workaround here: keep-network/tbtc-v2@26d4f01. WDYT?

lukasz-zimnoch added a commit to keep-network/tbtc-v2 that referenced this pull request Jan 26, 2023
…ol integration (#501)

Closes: #422

In this PR we update the implementation of TypeScript Electrum
integration to support Esplora/Electrs servers.

As per Blockstream/electrs#36 verbose
transactions are not supported by Esplora/Electrs.
This affects our implementation of `getTransaction` and
`getTransactionConfirmations` functions.

For a consistent code in the client without alternative paths for
different Electrum servers implementations I decided to not use verbose
transactions at all.

###
[getTransactionConfirmations](26d4f01)

1. Get the raw transaction
2. Deserialize the raw transaction
3. Find transaction block height by finding it in a history of
   transactions for the output script included in the transaction.
4. Get the latest block height
5. Calculate number of confirmations by subtracting the transaction
   block height from the latest block height and adding one.

###
[getTransaction](a7aedd1)

We get a raw transaction and deserialize it with `bcoin`. This lets us
define a consistent type for returned transactions
(093d4ec). Before these changes, I
observed that Electrum server implementations are not consistent with
data returned in verbose JSON.

### Electrum tests

We can test electrum integration against different kinds of servers.
The most popular implementations are:
- ElectrumX
- Fulcrum
- Electrs/Esplora

We can find a list of public servers here:
https://1209k.com/bitcoin-eye/ele.php?chain=tbtc

The electrs-esplora server seems pretty unstable, so we don't want to
enable it in tests until we add retries
(#485).
@shesek
Copy link
Collaborator

shesek commented Jan 27, 2023

Please correct me if I'm wrong, but according to the Electrum Protocol blockchain.transaction.get_merkle(tx_hash, height) expects block height as an argument, so it can't be used to get the block height.

Yes you are correct -- the current Electrum protocol specification does require specifying height for get_merkle.

However, some Electrum server implementations opted to make the height argument optional regardless - including the original romanz/electrs, the esplora electrs fork (this repo), and my personal bwt server. I'm not entirely sure about other implementations. (edit: Oops, I misremembered. See next comment by @nkuba)

Also note that the next v2.0 protocol specification intends to make this behavior official:

The previously required `height` argument for `blockchain.transaction.get_merkle` is now optional.

You can find a possible workaround here: keep-network/tbtc-v2@26d4f01. WDYT?

Well obviously kind of hairy and inefficient, but as far as I know its the best you can get using just the standard Electrum server features (and assuming no verbose - which can get you the number of confirmations, but not the confirming block height like the get_history/get_merkle-based solutions can).

@nkuba
Copy link

nkuba commented Mar 7, 2023

However, some Electrum server implementations opted to make the height argument optional regardless - including the original romanz/electrs, the esplora electrs fork (this repo), and my personal bwt server. I'm not entirely sure about other implementations.

I tested it against the most popular electrum server implementations: Fulcrum, Esplora/Electrs, and ElectrumX:

## Fulcrum
echo '{"jsonrpc": "2.0", "method": "server.version", "id": 0}' | netcat 176.9.63.51 50001
# {"id":0,"jsonrpc":"2.0","result":["Fulcrum 1.8.2","1.4"]}
echo '{"jsonrpc": "2.0", "method": "blockchain.transaction.get_merkle", "params": ["bf8ec5df1cad3ab003f997e44e8e24ab4f57841587a9892de799220da10a0286"], "id": 0}' | netcat 176.9.63.51 50001
# {"id":0,"jsonrpc":"2.0","result":{"block_height":779744,"merkle":["d112134b2ae54b0cfe9e65dc0963de47c9afcec121ab656ec2b4af3f808f27fd","603009d060ebb55b48e0dc48859002a54768801abc214bf7076b89ad56bd1410","2707c3bb2464b91486d3ceef887b9565f070da752a9a0ee41eb9c740e64301bc","645dbbf3e10db1a257f5bcc6b7f0a0dd394050661932f1b07695930f24aac068","e22e7afe4feb3d74c105c348d57d86440a8de9e88479adfa7e4d2e3e405ff213","2a4fa3ecc95586cdd60b404c0f0a59aec29e96c7d015b3c3332f8069755bb79d","8729490f70fa8a394ce66dbe2c048ff3879ba2a4730fe31108d710255b2d9aab","811f9b6e15ef0be436dfc0c468fe62c34679b8824eba5421eb8055e639c71f4f","ac57fee6ee2b68d80e96d89db77a2428ba704d541902bbb087df6c42a3bfc7e8","955c0f541ce932d0d13e2316d4e990c9047cec6ecd75ecf8109b8691257ef7e3","7cf665de08a9334a24094c10524b6366ef317cdef121869ad3d0a1dff7c36340"],"pos":3}}

## Esplora/Electrs
echo '{"jsonrpc": "2.0", "method": "server.version", "id": 0}' | netcat 35.225.54.191 50001
# {"id":0,"jsonrpc":"2.0","result":["electrs-esplora 0.4.1","1.4"]}
echo '{"jsonrpc": "2.0", "method": "blockchain.transaction.get_merkle", "params": ["bf8ec5df1cad3ab003f997e44e8e24ab4f57841587a9892de799220da10a0286"], "id": 0}' | netcat 35.225.54.191 50001
# {"error":"missing height","id":0,"jsonrpc":"2.0"}

## ElectrumX
echo '{"jsonrpc": "2.0", "method": "server.version", "id": 0}' | netcat 161.97.67.106 50001
# {"jsonrpc":"2.0","result":["ElectrumX 1.16.0","1.4"],"id":0}
echo '{"jsonrpc": "2.0", "method": "blockchain.transaction.get_merkle", "params": ["bf8ec5df1cad3ab003f997e44e8e24ab4f57841587a9892de799220da10a0286"], "id": 0}' | netcat 161.97.67.106 50001
# {"jsonrpc":"2.0","error":{"code":-32602,"message":"1 argument passed to method \"blockchain.transaction.get_merkle\" but it requires 2"},"id":0}

It turned out that only Fulcrum supports get_merkle without the height parameter.

@fujicoin
Copy link
Author

fujicoin commented Mar 10, 2023

Fulcrum is cool. It is easy to modify for Bitcoin-like coins. Since it is written in C++, there is no problem with processing power. Moreover, it can be easily built with Docker. Confirmed compatibility as a backend for electrum, Trezor Suite, and AtomicDEX.

@shesek
Copy link
Collaborator

shesek commented Mar 12, 2023

@nkuba Yes you are correct. Looks like my memory failed me. Apologies for the confusion!

@tnull
Copy link

tnull commented Oct 23, 2023

Is there any resolution to this? IIUC, if I want to retrieve the hash of a block in which a transaction was included, I now would have no other chance beside to issue ~4-5 sequential requests that not only introduce a lot of overhead but also the risk to have a reorg happen at any point during their runtime, i.e., I'd have to be super-cautious to do this securely.

Verbose transactions are supported since Electrum protocol version 1.2, I was very surprised that electrs, while advertising support for 1.4, still does not support them.

junderw pushed a commit to junderw/electrs that referenced this pull request Dec 12, 2023
@stevenroose
Copy link

It seems that without verbose transactions, there is no way to get the number of confirmations of a tx nor the block hash or height it was confirmed in?

@stevenroose
Copy link

Why can't the verbose JSON be reproduced from the information in the indices? At least the information available could be returned.. It seems that the HTTP API knows most if not all of the information in the verbose API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants