Getrawtransaction working partially without -txindex is confusing

[sipa]

I think we should make getrawtransaction fail entirely if no txindex is enabled, instead of only working on not-fully-spent transactions, with a confusing error message otherwise.

[sipa]

And a stretch goal: there should be some functionality to query a raw transaction from the wallet rather than the blockchain.

[jgarzik]

More than a stretch goal, IMO, a requirement. I know of several softwares used internally that rely on 'getrawtransaction' working without txindex=1.

At a minimum, there must be a compatible replacement.

I get the confusion angle and agree -- but people are using this feature actively, even with txindex=0.

[laanwj]

I understand the confusion but I don't think removing functionality is a good response, wouldn't it be better to improve the error message to be less confusing? (maybe even mention -txindex in the message...)

Making it possible to query raw transactions from the wallet sounds like a good idea. Maybe as a new RPC command getwallettransaction to not confuse wallet and blockchain functionality?

[sipa]

@jgarzik I think you may be misinformed. getrawtransaction currently does not work for wallet transactions. It works for transactions that are either in the mempool or not entirely spent (=in the UTXO set). This may overlap with wallet transactions of course, but is not guaranteed. I think that funtionality doesn't maatch many use cases, and is inefficient. I'd rather have it work under very clear conditions.

@laanwj I'm in favor of both improving the error message and a getwallettransaction command (though gettransaction is also a wallet RPC, but one that fetches ledger-level information, not transaction-level information).

[luke-jr]

@sipa No reason it couldn't work for wallet transactions - the raw data is all in the wallet too, IIRC.

[gmaxwell]

We could just have another RPC that works on wallet transactions. I have had people pull txn that they have stuck many times using get rawtransactions, so I think we need to at least preserve the wallet behavior.

Any opinion on adding a getblockrawtransaction ? which would replace the non-determinstic behavior we have for txindex=0 now?

[sipa]

@luke-jr Sure, my main concern is not further confusing wallet operations with blockchain operations.

We could of course extend getrawtransaction with wallet functionality, and at the same time deprecate it in favor of a getwalletrawtransaction and getblockchainrawtransaction (the latter name may be confusing, as i suppose it would also do mempool fetching).

[laanwj]

If we're going to deprecate getrawtransaction let's not make it more powerful first :)

Maybe even split it into three commands:

• getwalletrawtransaction -> only active of not in nowallet mode
• getblockchainrawtransaction -> only active when txindex=1
• getmempoolrawtransaction -> always active

Edit: alternatively, make it one command, then add flags what sources to query. This makes it possible to query multiple or all sources at once if desired.

[laanwj]

gettransaction shows the raw hex of a wallet transaction since pull #3668. That's at least one of the points in this issue.

[sipa]

Is there really any evidence of people using getrawtransaction without -txindex? I can't imagine how the current behaviour could be reliable for them.

[laanwj]

@sipa One usage scenario would be together with getrawmempool, to get transaction data for mempool transactions. If getrawtransaction were to only work with -txindex we'd have to do something like #3256 to get the tx...

[sipa]

@laanwj Perhaps. Though if your transaction suddenly confirms between getrawmempool and getrawtransaction, you won't get it either.

[laanwj]

Yes, but if it confirms in that time, it may not be interesting anymore for a process monitoring the mempool.

[promag]

With the new deprecation mechanism we could mark it deprecated if -txindex is not set in 0.16 and make it available only if -txindexin 0.17?

[Sjors]

Rather than deprecating, why not make it work without -txindex, albeit slow? As I suggested in #10609 (comment) we could add since argument which would specify how far to look back. In addition there could be a timeout argument with some sane default, after which it will give up with an error.

The use case I have in mind here is e.g. a Lightning node that's looking for fairly recent transactions, really doesn't need a full transaction index and can probably live with some delay in response time.

[sipa]

I don't think that's really a good approach. If people have needs to index transactions in the chain, there should be better other projects for that.

[Sjors]

@sipa what are your thoughts on a partial index through -txindex=SOME_MIN_HEIGHT? Or a rolling index down to the prune height?

It doesn't seem ideal having to install some middleware app in order to use second layer stuff, nor does it seem ideal for this second layer software to worry about such a basic feature as checking if a transaction is known (including all the reorg related bookkeeping). I'm not proposing a complete overhaul, but some sort of low hanging fruit compromise would be useful.

[sipa]

@Sjors I don't think this is really needed. If you need to watch for a certain txid, you can add one of its output addresses to a watch-only wallet or so, rather than relying on full indexing of the chain (even if it's just the last few blocks).

[promag]

If we're going to deprecate getrawtransaction let's not make it more powerful first :)

@laanwj for reference getrawtransaction was improved in #10275.

If the goal is to "force" getrawtransaction to work only with -txindex then we could make getrawtransaction only available if:

1. -txindex is set; or
2. -txindex is not set but -deprecatedrpc=getrawtransaction is set.

[jimpo]

I'd be in favor of dropping the coins view cache check because it is not reliable and confusing. The remaining behavior would be:

• If block_hash argument is set, check only that block, and error if not found
• Otherwise
  • Check the mempool and return if tx is found
  • If txindex is enabled and up to date (see Build tx index in parallel with validation #11857), check it and conclusively return tx or "Not found"
  • If txindex is disabled, error with "No such mempool transaction. Use -txindex to enable blockchain transaction queries"

[promag]

Check the mempool and return if tx is found

There is getmempoolentry for this so we could also drop that check from getrawtransaction.

[vengat90]

How to get receive from an address in the transaction, which command used in http://chainquery.com site?

[Sjors]

I'd rather have this feature work partially, with clear documentation and error codes, than to make it conditional on a full txindex. That's because getrawtransaction can be quite useful to lightning nodes and perhaps even lightweight wallets, that need to occasionally fetch historical stuff (not just monitor new transactions).

I think the feature can be made complete in the longer run without txindex.

@promag wrote:

@laanwj for reference getrawtransaction was improved in #10275.

I.e. it can now take a blockhash hint.

In addition, we could build on top of #10794 to allow getrawtransaction to work even if that block is pruned, by fetching it from the network if needed.

Perhaps a future txindex light feature could use Compact Block Filter (BIP-158 style), so that we can very quickly scan all blocks, which would return a bunch of false positives that are then searched brute-force.

[sipa]

@Sjors Elsewhere I have suggested splitting the feature up into separate RPCs; for example txindexgetrawtransaction (which only works when -txindex is enabled) and mempoolgetrawtransaction (which always works, but only for unspent transactions). Support for the it-works-when-not-pruned-and-only-for-transactions-which-have-at-least-one-unspent-output-but-very-slowly feature would only remain in the deprecated getrawtransaction RPC, as I don't think there is much use for it, and its conditions are too obscure to explain.

EDIT: Actually an RPC already exists for the mempool version, called getmempoolentry.

[sipa]

I like @jimpo's suggested above.

[maflcko]

So how would removing the txout check play with races, where a tx leaves the mempool right when mempoolgetrawtransaction or getrawtransaction is called?

Also mentioned earlier #3220 (comment) by @sipa.

[maflcko]

I think the current documentation is quite clear, and there is no pressing need to change the logic right now. Also note the issue is about 5 years old and most of the discussion no longer apply to the code in master. I suggest closing it.

documentation on current master for reference:

NOTE: By default this function only works for mempool transactions. If the -txindex option is
enabled, it also works for blockchain transactions. If the block which contains the transaction
is known, its hash can be provided even for nodes without -txindex....

[jimpo]

@MarcoFalke The documentation at the end says "DEPRECATED: for now, it also works for transactions with unspent outputs.\n" I propose fully removing support for that because it is confusing, deprecated, and slow.

I'm not in favor of changing getrawtransaction to stop returning mempool txs.

[maflcko]

I'm not in favor of changing getrawtransaction to stop returning mempool txs.

If you do the step and remove the utxo lookup, you might as well remove the mempool lookup and make it explicit that the interface was changed for txes that recently moved from the mempool to the utxo set. (Tell users to use getmempoolentry, after adding a rawtx key to the returned dict.)