Bug fix: don't pointlessly download a transaction we already have because we're broadcasting it.

This is a regression caused by the TxConfidenceTable rewrite: previously it pinned entire transactions and that was used to notice that a broadcast was happening. Now it only pins confidence objects. So instead we use the confidence.source property: if it's SELF then we assume we have it somewhere in the address space and don't bother downloading it when a peer announces it. If it's UNKNOWN then PeerGroup.broadcastTransaction will set it to SELF.
This commit is contained in:
Mike Hearn 2015-04-07 15:37:22 +02:00
parent 325e7e170b
commit c426e34646
3 changed files with 11 additions and 2 deletions

View file

@ -1073,6 +1073,9 @@ public class Peer extends PeerSocketHandler {
if (conf.numBroadcastPeers() > 1) { if (conf.numBroadcastPeers() > 1) {
// Some other peer already announced this so don't download. // Some other peer already announced this so don't download.
it.remove(); it.remove();
} else if (conf.getSource().equals(TransactionConfidence.Source.SELF)) {
// We created this transaction ourselves, so don't download.
it.remove();
} else { } else {
log.debug("{}: getdata on tx {}", getAddress(), item.hash); log.debug("{}: getdata on tx {}", getAddress(), item.hash);
getdata.addItem(item); getdata.addItem(item);

View file

@ -1795,7 +1795,11 @@ public class PeerGroup implements TransactionBroadcaster {
* which is calculated by watching the transaction propagate across the network and be announced by peers.</p> * which is calculated by watching the transaction propagate across the network and be announced by peers.</p>
*/ */
public TransactionBroadcast broadcastTransaction(final Transaction tx, final int minConnections) { public TransactionBroadcast broadcastTransaction(final Transaction tx, final int minConnections) {
// TODO: Context being owned by BlockChain isn't right w.r.t future intentions so it shouldn't really be optional here. // If we don't have a record of where this tx came from already, set it to be ourselves so Peer doesn't end up
// redownloading it from the network redundantly.
if (tx.getConfidence().getSource().equals(TransactionConfidence.Source.UNKNOWN)) {
tx.getConfidence().setSource(TransactionConfidence.Source.SELF);
}
final TransactionBroadcast broadcast = new TransactionBroadcast(this, tx); final TransactionBroadcast broadcast = new TransactionBroadcast(this, tx);
broadcast.setMinConnections(minConnections); broadcast.setMinConnections(minConnections);
// Send the TX to the wallet once we have a successful broadcast. // Send the TX to the wallet once we have a successful broadcast.

View file

@ -64,6 +64,7 @@ public class TransactionBroadcastTest extends TestWithPeerGroup {
public void fourPeers() throws Exception { public void fourPeers() throws Exception {
InboundMessageQueuer[] channels = { connectPeer(1), connectPeer(2), connectPeer(3), connectPeer(4) }; InboundMessageQueuer[] channels = { connectPeer(1), connectPeer(2), connectPeer(3), connectPeer(4) };
Transaction tx = new Transaction(params); Transaction tx = new Transaction(params);
tx.getConfidence().setSource(TransactionConfidence.Source.SELF);
TransactionBroadcast broadcast = new TransactionBroadcast(peerGroup, tx); TransactionBroadcast broadcast = new TransactionBroadcast(peerGroup, tx);
final AtomicDouble lastProgress = new AtomicDouble(); final AtomicDouble lastProgress = new AtomicDouble();
broadcast.setProgressCallback(new TransactionBroadcast.ProgressCallback() { broadcast.setProgressCallback(new TransactionBroadcast.ProgressCallback() {
@ -92,10 +93,11 @@ public class TransactionBroadcastTest extends TestWithPeerGroup {
assertFalse(future.isDone()); assertFalse(future.isDone());
assertEquals(0.0, lastProgress.get(), 0.0); assertEquals(0.0, lastProgress.get(), 0.0);
inbound(channels[1], InventoryMessage.with(tx)); inbound(channels[1], InventoryMessage.with(tx));
pingAndWait(channels[1]);
future.get(); future.get();
Threading.waitForUserCode(); Threading.waitForUserCode();
assertEquals(1.0, lastProgress.get(), 0.0); assertEquals(1.0, lastProgress.get(), 0.0);
// There is no response from the Peer as it has nothing to do.
assertNull(outbound(channels[1]));
} }
@Test @Test