romanz / electrs

An efficient re-implementation of Electrum Server in Rust
MIT License
1.09k stars 404 forks source link

Bug: Electrs shows misleading error message when blockchain.transaction.get does not find the transaction #936

Open cjackie opened 1 year ago

cjackie commented 1 year ago

Describe the bug Electrs shows misleading and confusing error message when getting a non-existing transaction using "blockchain.transaction.get"

Electrs version 0.9.11.

To Reproduce Steps to reproduce the behavior:

  1. Configure and start electrs
  2. Go to the contrib folder of the repo i.e. do "cd \<path to electrs>/contrib"
  3. Use ./get_tx.py to get a transaction with valid-looking transaction but does not exist in the Bitcoin network e.g. "dd8424a665e367d190b487a9bfb945420be78ff219a68ff81799c55c10fc6650"
  4. See error in the response. The error is "message is No such mempool transaction. Use -txindex or provide a block hash to enable blockchain transaction queries. Use gettransaction for wallet transactions".

Expected behavior The error indicating that the transaction is not found, instead showing the Bitcoin error message that suggests "txindex" is needed, or blockhash is provided. Setting txindex=1 in Bitcoin core is not required to use electrs (as advertised in README.md), and the blockhash should have been provided in https://github.com/romanz/electrs/blob/d0b69f0dc43f2d8ceefc8422f5231f15d064b699/src/electrum.rs#L370-L373

Configuration Standard config

Electrum client N/A

Additional context

cjackie commented 1 year ago

Here is the fix:

change

      if verbose {
          let blockhash = self
              .tracker
              .lookup_transaction(&self.daemon, txid)?
              .map(|(blockhash, _tx)| blockhash);
          return self.daemon.get_transaction_info(&txid, blockhash);
      }

To

        if verbose {
            return match self.tracker.lookup_transaction(&self.daemon, txid)?.map(|(blockhash, _tx)| blockhash) {
                None => bail!("Transaction is not found."),
                Some(blockhash) => self.daemon.get_transaction_info(&txid, blockhash)
            }
        }

Then tighten the second parameter of daemon.get_transaction_info, making it non-optional

Eunovo commented 1 year ago

I'm working on a PR for this. The error, "No such mempool transaction. Use -txindex or provide a block hash to enable blockchain transaction queries. Use gettransaction for wallet transactions." comes from bitcoind. The original bitcoind error returns a code: -5 along with this message. I want to parse the resulting error json and map the code: -5 error to something like "Transaction not found" for the transaction get operation.

antonilol commented 1 year ago

"No such mempool transaction. Use -txindex or provide a block hash to enable blockchain transaction queries. Use gettransaction for wallet transactions."

didn't bitcoind just only search the mempool because you dont have txindex enabled?

getting (getrawtransaction) a transaction from the blockchain by txid without txindex turned on is not supported by bitcoind

Eunovo commented 1 year ago

getting (getrawtransaction) a transaction from the blockchain by txid without txindex turned on is not supported by bitcoind

I tried this, you get a similar error but without the -txindex message.

cjackie commented 1 year ago

"No such mempool transaction. Use -txindex or provide a block hash to enable blockchain transaction queries. Use gettransaction for wallet transactions."

didn't bitcoind just only search the mempool because you dont have txindex enabled?

getting (getrawtransaction) a transaction from the blockchain by txid without txindex turned on is not supported by bitcoind

"No such mempool transaction. Use -txindex or provide a block hash to enable blockchain transaction queries. Use gettransaction for wallet transactions."

didn't bitcoind just only search the mempool because you dont have txindex enabled?

getting (getrawtransaction) a transaction from the blockchain by txid without txindex turned on is not supported by bitcoind

As mentioned in the description, setting txindex in bitcoind isn't required to use Electrs. The error message is coming straight from bitcoind which is confusing to Electrs users. A better error message would be "Transaction is not found."