RPC_TRANSACTION_ALREADY_IN_CHAIN exception race condition in sendrawtransaction #5638

I discovered a race condition in sendrawtransaction. For details: see my comment in https://github.com/bitcoin/bitcoin/commit/1d46fe3327f6645fd79b442cd72ef422418c1a50.

If you send a raw transaction which has already been broadcasted, then you don't know whether you'll receive an RPC_TRANSACTION_ALREADY_IN_CHAIN exception, since you don't know whether a new block has just been received, which includes the transaction. There are use cases where such RPC_TRANSACTION_ALREADY_IN_CHAIN occurrences do not indicate an error; however, since this type of occurrence is very rare, developers are not likely to be aware of the possibility, and silence the exception. As a result, applications can potentially experience hard-to-diagnose, very rare errors.

I think the core of the problem is that sendrawtransaction tries to combine two use cases: A: publish a transaction on the Bitcoin network B: re-transmit an already known transaction to peers RPC_TRANSACTION_ALREADY_IN_CHAIN might make sense for B, but for A (which is, IMHO, the primary use case) it is confusing.

Yes, while working on 1d46fe3 I thought about removing the RPC_TRANSACTION_ALREADY_IN_CHAIN exception, and just returning success in case the transaction is already in the chain. After all, it is successful if it is already in.

Eventually I didn't do that, to keep some compatibility with the old behavior. It's reasonably useful in some cases to know when a transaction already landed in the chain, and if not you can ignore the specific error code.

(I would argue it's misuse of an exception and returning a {'inchain':X, 'inmempool':X, 'txhash':X} structure would be more informative, but alas, that'd mean changing the interface to be even more incompatible)

If you don't want to change the behavior, then it should at least be documented, so that at least careful developers, who read the documentation, won't make mistakes here.

"help sendrawtransaction" in the 0.9.3 console window didn't show me this, and https://en.bitcoin.it/wiki/Raw_Transactions#sendrawtransaction_.3Chex_string.3E is even more brief. I discovered this because I happened to have other reasons for trying to understand the exact behavior of sendrawtransaction, so I started reading the source.

The documentation should be good enough that RPC-using developers don't need to read the source.