bitcoinj/bitcoinj — pull request #257 by mackler (2014-10-26 16:54:47+00:00)
Title: Make `TransactionBroadcast` recognize network rejection of transmitted transaction

Currently, if you broadcast a transaction and it is rejected, the `TransactionBroadcast` class will not see the reject message, and you have to wait for the timeout to know there was an error. This patch changes that; the reject message is immediately recognized and the future is completed with a new type of Exception.

Two points:

First, I found it necessary to add the `PeerEventListener` that sets the exception to each connected Peer individually by looping through all connected Peers. I was under the impression that simply adding a listener to the `PeerGroup` would do that, but apparently that's incorrect. I'm thinking that either:

1. I'm missing something, and would appreciate correction; or
2. The [comment for `PeerGroup.addEventListener`](https://github.com/bitcoinj/bitcoinj/blob/master/core/src/main/java/org/bitcoinj/core/PeerGroup.java#L555) ought to be rewritten to explain that the added listener only applies to `Peers` that are connected after that method is invoked; or
3. `PeerGroup.addEventListener()` and `PeerGroup.removeEventListener()` ought to be modified so that listeners are added and removed from currently connected Peers.

Second, there are no tests with this patch. I'm not a super test-writing expert, especially when it comes to tests that interact with the outside world, such as the network. Also, I don't see any existing tests for `TransactionBroadcast`. Maybe someone could give me some guidance how this patch should be tested.

One thing in particular that concerns me is that when the listener is added or removed from the Peers it uses the `PeerGroup.getConnectedPeers()` method to get the list of Peers to loop through. There is also a `getPendingPeers()` method. Is it possible that a Peer could be connected at such a time that the listener is not added correctly? Should this patch add/remove the listener from the pending Peers as well as connected Peers?