Resolve some race conditions.

It was possible that during saving of a wallet a network thread would update the confidence metrics due to broadcast announcements. This change makes TransactionConfidence use a COW list so the broadcast peer set can be iterated over safely. Resolves issue 277.

core/src/main/java/com/google/bitcoin/core/PeerGroup.java

@@ -996,7 +996,7 @@ public class PeerGroup extends AbstractIdleService {
                 tx.getConfidence().addEventListener(new TransactionConfidence.Listener() {
                     public void onConfidenceChanged(Transaction tx) {
                         // This will run in a peer thread.
-                        final int numSeenPeers = tx.getConfidence().getBroadcastBy().size();
+                        final int numSeenPeers = tx.getConfidence().numBroadcastPeers();
                         boolean done = false;
                         log.info("broadcastTransaction: TX {} seen by {} peers", pinnedTx.getHashAsString(),
                                 numSeenPeers);

core/src/main/java/com/google/bitcoin/core/TransactionConfidence.java

@@ -22,8 +22,8 @@ import com.google.common.base.Preconditions;
 import java.io.Serializable;
 import java.math.BigInteger;
 import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.Set;
+import java.util.ListIterator;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
  * <p>A TransactionConfidence object tracks data you can use to make a confidence decision about a transaction.
@@ -61,7 +61,7 @@ public class TransactionConfidence implements Serializable {
      * IP address as an approximation. It's obviously vulnerable to being gamed if we allow arbitrary people to connect
      * to us, so only peers we explicitly connected to should go here.
      */
-    private Set<PeerAddress> broadcastBy;
+    private CopyOnWriteArrayList<PeerAddress> broadcastBy;
     /** The Transaction that this confidence object is associated with. */
     private Transaction transaction;
     // Lazily created listeners array.
@@ -180,7 +180,7 @@ public class TransactionConfidence implements Serializable {
 
     public TransactionConfidence(Transaction tx) {
         // Assume a default number of peers for our set.
-        broadcastBy = new HashSet<PeerAddress>(10);
+        broadcastBy = new CopyOnWriteArrayList<PeerAddress>();
         transaction = tx;
     }
 
@@ -234,8 +234,9 @@ public class TransactionConfidence implements Serializable {
      * @param address IP address of the peer, used as a proxy for identity.
      */
     public void markBroadcastBy(PeerAddress address) {
+        if (!broadcastBy.addIfAbsent(address))
+            return;  // Duplicate.
         synchronized (this) {
-            broadcastBy.add(address);
             if (getConfidenceType() == ConfidenceType.UNKNOWN) {
                 this.confidenceType = ConfidenceType.NOT_SEEN_IN_CHAIN;
             }
@@ -246,15 +247,20 @@ public class TransactionConfidence implements Serializable {
     /**
      * Returns how many peers have been passed to {@link TransactionConfidence#markBroadcastBy}.
      */
-    public synchronized int numBroadcastPeers() {
+    public int numBroadcastPeers() {
         return broadcastBy.size();
     }
 
     /**
      * Returns a synchronized set of {@link PeerAddress}es that announced the transaction.
      */
-    public synchronized Set<PeerAddress> getBroadcastBy() {
-        return broadcastBy;
+    public ListIterator<PeerAddress> getBroadcastBy() {
+        return broadcastBy.listIterator();
+    }
+
+    /** Returns true if the given address has been seen via markBroadcastBy() */
+    public boolean wasBroadcastBy(PeerAddress address) {
+        return broadcastBy.contains(address);
     }
 
     @Override

core/src/main/java/com/google/bitcoin/store/WalletProtobufSerializer.java

@@ -31,10 +31,7 @@ import java.io.OutputStream;
 import java.math.BigInteger;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
-import java.util.Collection;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.*;
 
 /**
  * Serialize and de-serialize a wallet to a byte stream containing a
@@ -203,19 +200,22 @@ public class WalletProtobufSerializer {
     private static void writeConfidence(Protos.Transaction.Builder txBuilder,
                                         TransactionConfidence confidence,
                                         Protos.TransactionConfidence.Builder confidenceBuilder) {
-        confidenceBuilder.setType(Protos.TransactionConfidence.Type.valueOf(confidence.getConfidenceType().getValue()));
-        if (confidence.getConfidenceType() == ConfidenceType.BUILDING) {
-            confidenceBuilder.setAppearedAtHeight(confidence.getAppearedAtChainHeight());
-            confidenceBuilder.setDepth(confidence.getDepthInBlocks());
-            if (confidence.getWorkDone() != null) {
-                confidenceBuilder.setWorkDone(confidence.getWorkDone().longValue());
+        synchronized (confidence) {
+            confidenceBuilder.setType(Protos.TransactionConfidence.Type.valueOf(confidence.getConfidenceType().getValue()));
+            if (confidence.getConfidenceType() == ConfidenceType.BUILDING) {
+                confidenceBuilder.setAppearedAtHeight(confidence.getAppearedAtChainHeight());
+                confidenceBuilder.setDepth(confidence.getDepthInBlocks());
+                if (confidence.getWorkDone() != null) {
+                    confidenceBuilder.setWorkDone(confidence.getWorkDone().longValue());
+                }
+            }
+            if (confidence.getConfidenceType() == ConfidenceType.DEAD) {
+                Sha256Hash overridingHash = confidence.getOverridingTransaction().getHash();
+                confidenceBuilder.setOverridingTransaction(hashToByteString(overridingHash));
             }
         }
-        if (confidence.getConfidenceType() == ConfidenceType.DEAD) {
-            Sha256Hash overridingHash = confidence.getOverridingTransaction().getHash();
-            confidenceBuilder.setOverridingTransaction(hashToByteString(overridingHash));
-        }
-        for (PeerAddress address : confidence.getBroadcastBy()) {
+        for (ListIterator<PeerAddress> it = confidence.getBroadcastBy(); it.hasNext();) {
+            PeerAddress address = it.next();
             Protos.PeerAddress proto = Protos.PeerAddress.newBuilder()
                     .setIpAddress(ByteString.copyFrom(address.getAddr().getAddress()))
                     .setPort(address.getPort())

core/src/test/java/com/google/bitcoin/core/PeerGroupTest.java

@@ -268,8 +268,8 @@ public class PeerGroupTest extends TestWithNetworkConnections {
         inbound(p1, tx);   // returns nothing after a queue drain.
         // Two peers saw this tx hash.
         assertEquals(2, tx.getConfidence().numBroadcastPeers());
-        assertTrue(tx.getConfidence().getBroadcastBy().contains(peerOf(p1).getAddress()));
-        assertTrue(tx.getConfidence().getBroadcastBy().contains(peerOf(p2).getAddress()));
+        assertTrue(tx.getConfidence().wasBroadcastBy(peerOf(p1).getAddress()));
+        assertTrue(tx.getConfidence().wasBroadcastBy(peerOf(p2).getAddress()));
 
         tx.getConfidence().addEventListener(new TransactionConfidence.Listener() {
             public void onConfidenceChanged(Transaction tx) {
@@ -280,7 +280,7 @@ public class PeerGroupTest extends TestWithNetworkConnections {
         inbound(p3, inv);
         assertEquals(tx, event[1]);
         assertEquals(3, tx.getConfidence().numBroadcastPeers());
-        assertTrue(tx.getConfidence().getBroadcastBy().contains(peerOf(p3).getAddress()));
+        assertTrue(tx.getConfidence().wasBroadcastBy(peerOf(p3).getAddress()));
     }
 
     @Test