Getrawtransaction working partially without -txindex is confusing #3220

issue sipa openend this issue on November 7, 2013
  1. sipa commented at 4:26 pm on November 7, 2013: member
    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.
  2. sipa commented at 4:30 pm on November 7, 2013: member
    And a stretch goal: there should be some functionality to query a raw transaction from the wallet rather than the blockchain.
  3. jgarzik commented at 4:44 pm on November 7, 2013: contributor

    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.

  4. laanwj commented at 5:30 pm on November 7, 2013: member

    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?

  5. sipa commented at 6:15 pm on November 7, 2013: member
    @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).
  6. luke-jr commented at 6:36 pm on November 7, 2013: member
    @sipa No reason it couldn’t work for wallet transactions - the raw data is all in the wallet too, IIRC.
  7. gmaxwell commented at 6:57 pm on November 7, 2013: contributor

    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?

  8. sipa commented at 7:05 pm on November 7, 2013: member

    @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).

  9. laanwj commented at 7:48 am on November 8, 2013: member

    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.

  10. laanwj referenced this in commit 4614ae4093 on Feb 13, 2014
  11. laanwj referenced this in commit b23aaf8207 on Feb 13, 2014
  12. laanwj commented at 9:48 am on May 6, 2014: member
    gettransaction shows the raw hex of a wallet transaction since pull #3668. That’s at least one of the points in this issue.
  13. sipa commented at 9:52 am on May 6, 2014: member
    Is there really any evidence of people using getrawtransaction without -txindex? I can’t imagine how the current behaviour could be reliable for them.
  14. laanwj commented at 8:20 am on May 9, 2014: member
    @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…
  15. sipa commented at 10:27 am on May 9, 2014: member
    @laanwj Perhaps. Though if your transaction suddenly confirms between getrawmempool and getrawtransaction, you won’t get it either.
  16. laanwj commented at 10:35 am on May 9, 2014: member
    Yes, but if it confirms in that time, it may not be interesting anymore for a process monitoring the mempool.
  17. laanwj added the label Brainstorming on Jan 29, 2016
  18. laanwj removed the label Refactoring on Jan 29, 2016
  19. promag commented at 0:13 am on December 7, 2017: member
    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?
  20. Sjors commented at 3:54 pm on December 28, 2017: member

    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.

  21. sipa commented at 3:56 pm on December 28, 2017: member
    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.
  22. Sjors commented at 4:59 pm on December 28, 2017: member

    @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.

  23. sipa commented at 5:25 pm on December 28, 2017: member
    @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).
  24. promag commented at 1:41 am on February 22, 2018: member

    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.
  25. jimpo commented at 6:39 am on March 30, 2018: contributor

    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 #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”
  26. promag commented at 8:15 am on March 30, 2018: member

    Check the mempool and return if tx is found

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

  27. vengat90 commented at 8:15 am on August 8, 2018: none
    How to get receive from an address in the transaction, which command used in http://chainquery.com site?
  28. Sjors commented at 10:17 am on August 8, 2018: member

    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.

  29. sipa commented at 5:58 pm on August 8, 2018: member

    @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.

  30. sipa commented at 6:02 pm on August 8, 2018: member
    I like @jimpo’s suggested above.
  31. MarcoFalke commented at 7:37 pm on August 8, 2018: member

    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.

  32. MarcoFalke commented at 7:42 pm on August 8, 2018: member

    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:

    0NOTE: By default this function only works for mempool transactions. If the -txindex option is
    1enabled, it also works for blockchain transactions. If the block which contains the transaction
    2is known, its hash can be provided even for nodes without -txindex....
    
  33. jimpo commented at 0:06 am on August 9, 2018: contributor

    @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.

  34. MarcoFalke commented at 0:37 am on August 9, 2018: member

    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.)

  35. MarcoFalke referenced this in commit a47319dada on Jan 30, 2019
  36. MarcoFalke closed this on Feb 12, 2019

  37. Bushstar referenced this in commit dfe99c9507 on Apr 8, 2020
  38. MarcoFalke locked this on Dec 16, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-05 01:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me