What is the migration path for users that were using this to query the utxo set?
I suppose gettxout and REST /rest/getutxos cover that?

Yes, use UTXO query functions to query the UTXO set.

This definitely needs mention in the release notes.
Maybe deprecate this functionality in 0.13 (put it behind an option that is disabled by default) then actually remove it in 0.14?

concept ACK

I agree that it's so incomprehensible (and not desirable) in its behavior that it's unlikely anyone was using it (or at least, using it and not silently getting screwed over by it). I do wish any of these queries that only worked with txindex had a non-txindex mechanism that took a location parameter (e.g. block height.)

I don't think we need to deprecate this behind a command line flag. (Keep in mind that there still would be no way for us to find out how many would toggle the flag)

Considering that this is not really a functionality someone can rely on and it appears unlikely someone (other than our test framework) relies on it, it is probably safe to just remove it.

I don't think we need to deprecate this behind a command line flag. (Keep in mind that there still would be no way for us to find out how many would toggle the flag)

That's true, but it would give people the opportunity to complain about it, while not having to forego the upgrade.

I don't feel strongly about it though, if you guys believe that this is a change that can be made without warning in advance I'm fine with that. Depends on how urgent this change is as well.

I do wish any of these queries that only worked with txindex had a non-txindex mechanism that took a location parameter (e.g. block height.)

One radical idea would be to expose CDiskTxPos fields on the interface here and there, and add a REST/RPC call to get a transaction from such a (blockfile_id,offset) tuple. I wonder if that would make some things easier/possible, such as external indices without having to store the block chain twice, but with fast access. Probably these 'pointers' should be obfuscated somehow to discourage direct access to the block files from outside bitcoind.

Depends on how urgent this change is as well.

The changes here are not urgent. If we choose to not adopt them quickly, I think at least the updated error message in getrawtransaction is useful.

One of my clients has something that was using this FWIW, so I'd target actual removal for v0.14

One of my clients has something that was using this FWIW, so I'd target actual removal for v0.14

That's interesting. I somehow expected that, if some functionality is available for so long, some people will come to rely on it.
What is their use case? Could they (in principle, given warning upfront to update their code) use the other UTXO query commands as replacement?

@laanwj Basically their wallet code happens to depend on it; with a fair amount of new code they could work around the problem (remember that simply saving the txs when created isn't enough due to malleability).

I don't think that's a show-stopper, but we should give users some kind of warning period. Perhaps we could make v0.13.0 appear to not have this support by default, but then re-enable it with a command line flag, that's marked as "will be removed in v0.14.0"?

Perhaps we could make v0.13.0 appear to not have this support by default, but then re-enable it with a command line flag, that's marked as "will be removed in v0.14.0"?

+1. That's exactly what I suggested too:

Maybe deprecate this functionality in 0.13 (put it behind an option that is disabled by default) then actually remove it in 0.14?

@laanwj Cool, sounds like we're in agreement. :)

@sipa Sounds good to me, thanks!

Looks like this causes a few RPC tests to fail:

wallet.py                      | False  | 9 s
rest.py                        | False  | 7 s
abandonconflict.py             | False  | 7 s

utACK c3af143, but needs the command line option (or maybe just a short-term compile flag?)

Still failing travis.

@sipa Needs rebase.

I'm ever so slightly against this change. I find it's routinely useful for me when I'm trying to debug behavior on a random node.

@morcos Perhaps that's a good reason to introduce a getmempooltransaction RPC? My reason for this (or a related) change is that getrawtransaction has very unclear conditions under which it works, and I keep seeing people say it doesn't work.

Perhaps that's a good reason to introduce a getmempooltransaction RPC

Agree with @morcos that the way toward fixing this could also be make the API clearer instead of removing functionality (that is apparently used by some people sometimes, even if only by developers for troubleshooting).

+1 for splitting up getrawtransaction into calls that look in one place only.