Replace WalletEventListener.onDeadTransaction with a generic confidence changed callback, this simplifies the case of listening for all confidence changes in all wallet transactions and gives a single place to save the wallet from.

src/com/google/bitcoin/core/AbstractWalletEventListener.java

@@ -76,18 +76,18 @@ public abstract class AbstractWalletEventListener implements WalletEventListener
     }
 
     /**
-     * This is called on a Peer thread when a transaction becomes <i>dead</i>. A dead transaction is one that has
+     * Called on a Peer thread when a transaction changes its confidence level. You can also attach event listeners to
-     * been overridden by a double spend from the network and so will never confirm no matter how long you wait.<p>
+     * the individual transactions, if you don't care about all of them. Usually you would save the wallet to disk after
+     * receiving this callback.<p>
      *
-     * A dead transaction can occur if somebody is attacking the network, or by accident if keys are being shared.
+     * You should pay attention to this callback in case a transaction becomes <i>dead</i>, that is, somebody
-     * You can use this event handler to inform the user of the situation. A dead spend will show up in the BitCoin
+     * successfully executed a double spend against you. This is a (very!) rare situation but the user should be
-     * C++ client of the recipient as 0/unconfirmed forever, so if it was used to purchase something,
+     * notified that money they thought they had, was taken away from them.<p>
-     * the user needs to know their goods will never arrive.
      *
-     * @param deadTx        The transaction that is newly dead.
+     * @param wallet
-     * @param replacementTx The transaction that killed it.
+     * @param tx
      */
-    public void onDeadTransaction(Wallet wallet, Transaction deadTx, Transaction replacementTx) {
+    public void onTransactionConfidenceChanged(Wallet wallet, Transaction tx) {
         onChange();
     }
 

src/com/google/bitcoin/core/Wallet.java

@@ -339,9 +339,10 @@ public class Wallet implements Serializable {
         // the Peer before we got to this point, but in some cases (unit tests, other sources of transactions) it may
         // have been missed out.
         TransactionConfidence.ConfidenceType currentConfidence = tx.getConfidence().getConfidenceType();
-        assert currentConfidence == TransactionConfidence.ConfidenceType.UNKNOWN ||
+        if (currentConfidence == TransactionConfidence.ConfidenceType.UNKNOWN) {
-               currentConfidence == TransactionConfidence.ConfidenceType.NOT_SEEN_IN_CHAIN : currentConfidence;
             tx.getConfidence().setConfidenceType(TransactionConfidence.ConfidenceType.NOT_SEEN_IN_CHAIN);
+            invokeOnTransactionConfidenceChanged(tx);
+        }
 
         BigInteger balance = getBalance();
 
@@ -472,17 +473,14 @@ public class Wallet implements Serializable {
                 pending.put(tx.getHash(), tx);
             }
         } else {
-            // Mark the tx as appearing in this block so we can find it later after a re-org. If this IS a re-org taking
-            // place, this call will let the Transaction update its confidence and timestamp data to reflect the new
-            // best chain, as long as the new best chain blocks are passed to receive() last.
-            if (block != null)
-                tx.setBlockAppearance(block, bestChain);
             // This TX didn't originate with us. It could be sending us coins and also spending our own coins if keys
             // are being shared between different wallets.
             if (sideChain) {
                 log.info("  ->inactive");
                 inactive.put(tx.getHash(), tx);
             } else if (bestChain) {
+                // This can trigger tx confidence listeners to be run in the case of double spends. We may need to
+                // delay the execution of the listeners until the bottom to avoid the wallet mutating during updates.
                 processTxFromBestChain(tx);
             }
         }
@@ -497,6 +495,7 @@ public class Wallet implements Serializable {
         // transaction update its confidence and timestamp bookkeeping data.
         if (block != null) {
             tx.setBlockAppearance(block, bestChain);
+            invokeOnTransactionConfidenceChanged(tx);
         }
 
         // Inform anyone interested that we have received or sent coins but only if:
@@ -508,7 +507,8 @@ public class Wallet implements Serializable {
         //    listeners.
         //
         // TODO: Decide whether to run the event listeners, if a tx confidence listener already modified the wallet.
-        if (!reorg && bestChain && wtx == null) {
+        boolean wasPending = wtx != null;
+        if (!reorg && bestChain && !wasPending) {
             BigInteger newBalance = getBalance();
             if (valueSentToMe.compareTo(BigInteger.ZERO) > 0) {
                 invokeOnCoinsReceived(tx, prevBalance, newBalance);
@@ -556,7 +556,7 @@ public class Wallet implements Serializable {
             dead.put(doubleSpend.getHash(), doubleSpend);
             // Inform the event listeners of the newly dead tx.
             doubleSpend.getConfidence().setOverridingTransaction(tx);
-            invokeOnDeadTransaction(doubleSpend, tx);
+            invokeOnTransactionConfidenceChanged(doubleSpend);
         }
     }
 
@@ -610,13 +610,9 @@ public class Wallet implements Serializable {
                         dead.put(connected.getHash(), connected);
                         // Now forcibly change the connection.
                         input.connect(unspent, true);
-                        // Inform the event listeners of the newly dead tx.
+                        // Inform the [tx] event listeners of the newly dead tx. This sets confidence type also.
                         connected.getConfidence().setOverridingTransaction(tx);
-                        for (WalletEventListener listener : eventListeners) {
+                        invokeOnTransactionConfidenceChanged(connected);
-                            synchronized (listener) {
-                                listener.onDeadTransaction(this, connected, tx);
-                            }
-                        }
                     }
                 } else {
                     // A pending transaction that tried to double spend our coins - we log and ignore it, because either
@@ -1426,15 +1422,13 @@ public class Wallet implements Serializable {
 
         log.info("post-reorg balance is {}", Utils.bitcoinValueToFriendlyString(getBalance()));
 
-        // Inform event listeners that a re-org took place.
+        // Inform event listeners that a re-org took place. They should save the wallet at this point.
-        for (WalletEventListener l : eventListeners) {
+        EventListenerInvoker.invoke(eventListeners, new EventListenerInvoker<WalletEventListener>() {
-            // Synchronize on the event listener as well. This allows a single listener to handle events from
+            @Override
-            // multiple wallets without needing to worry about being thread safe.
+            public void invoke(WalletEventListener listener) {
-            synchronized (l) {
+                listener.onReorganize(Wallet.this);
-                l.onReorganize(this);
             }
-        }
+        });
-        
         assert isConsistent();
     }
 
@@ -1464,7 +1458,9 @@ public class Wallet implements Serializable {
                 Transaction replacement = doubleSpent.getSpentBy().getParentTransaction();
                 dead.put(tx.getHash(), tx);
                 pending.remove(tx.getHash());
-                invokeOnDeadTransaction(tx, replacement);
+                // This updates the tx confidence type automatically.
+                tx.getConfidence().setOverridingTransaction(replacement);
+                invokeOnTransactionConfidenceChanged(tx);
                 break;
             }
         }
@@ -1481,19 +1477,16 @@ public class Wallet implements Serializable {
         }
     }
 
-    private void invokeOnDeadTransaction(Transaction tx, Transaction replacement) {
+    private void invokeOnTransactionConfidenceChanged(final Transaction tx) {
-        for (int i = 0; i < eventListeners.size(); i++) {
+        EventListenerInvoker.invoke(eventListeners, new EventListenerInvoker<WalletEventListener>() {
-            WalletEventListener listener = eventListeners.get(i);
+            @Override
-            synchronized (listener) {
+            public void invoke(WalletEventListener listener) {
-                listener.onDeadTransaction(this, tx, replacement);
+                listener.onTransactionConfidenceChanged(Wallet.this, tx);
-            }
-            if (eventListeners.get(i) != listener) {
-                // Listener removed itself.
-                i--;
-            }
             }
+        });
     }
 
+
     /**
      * Returns an immutable view of the transactions currently waiting for network confirmations.
      */

src/com/google/bitcoin/core/WalletEventListener.java

@@ -75,17 +75,28 @@ public interface WalletEventListener {
      */
     void onReorganize(Wallet wallet);
 
+    // TODO: Flesh out the docs below some more to clarify what happens during re-orgs and other edge cases.
     /**
-     * This is called on a Peer thread when a transaction becomes <i>dead</i>. A dead transaction is one that has
+     * Called on a Peer thread when a transaction changes its confidence level. You can also attach event listeners to
-     * been overridden by a double spend from the network and so will never confirm no matter how long you wait.<p>
+     * the individual transactions, if you don't care about all of them. Usually you would save the wallet to disk after
-     * <p/>
+     * receiving this callback.<p>
-     * A dead transaction can occur if somebody is attacking the network, or by accident if keys are being shared.
-     * You can use this event handler to inform the user of the situation. A dead spend will show up in the BitCoin
-     * C++ client of the recipient as 0/unconfirmed forever, so if it was used to purchase something,
-     * the user needs to know their goods will never arrive.
      *
-     * @param deadTx        The transaction that is newly dead.
+     * You should pay attention to this callback in case a transaction becomes <i>dead</i>, that is, a transaction you
-     * @param replacementTx The transaction that killed it.
+     * believed to be active (send or receive) becomes overridden by the network. This can happen if<p>
+     *
+     * <ol>
+     *     <li>You are sharing keys between wallets and accidentally create/broadcast a double spend.</li>
+     *     <li>Somebody is attacking the network and reversing transactions, ie, the user is a victim of fraud.</li>
+     *     <li>A bug: for example you create a transaction, broadcast it but fail to commit it. The {@link Wallet}
+     *     will then re-use the same outputs when creating the next spend.</li>
+     * </ol><p>
+     *
+     * To find if the transaction is dead, you can use <tt>tx.getConfidence().getConfidenceType() ==
+     * TransactionConfidence.ConfidenceType.OVERRIDDEN_BY_DOUBLE_SPEND</tt>. If it is, you should notify the user
+     * in some way so they know the thing they bought may not arrive/the thing they sold should not be dispatched.
+     *
+     * @param wallet
+     * @param tx
      */
-    void onDeadTransaction(Wallet wallet, Transaction deadTx, Transaction replacementTx);
+    void onTransactionConfidenceChanged(Wallet wallet, Transaction tx);
 }

tests/com/google/bitcoin/core/ChainSplitTests.java

@@ -187,7 +187,9 @@ public class ChainSplitTests {
         final boolean[] eventCalled = new boolean[1];
         wallet.addEventListener(new AbstractWalletEventListener() {
             @Override
-            public void onDeadTransaction(Wallet wallet, Transaction deadTx, Transaction replacementTx) {
+            public void onTransactionConfidenceChanged(Wallet wallet, Transaction tx) {
+                super.onTransactionConfidenceChanged(wallet, tx);
+                if (tx.getConfidence().getConfidenceType() == TransactionConfidence.ConfidenceType.OVERRIDDEN_BY_DOUBLE_SPEND)
                     eventCalled[0] = true;
             }
         });
@@ -227,9 +229,10 @@ public class ChainSplitTests {
         final Transaction[] eventReplacement = new Transaction[1];
         wallet.addEventListener(new AbstractWalletEventListener() {
             @Override
-            public void onDeadTransaction(Wallet wallet, Transaction deadTx, Transaction replacementTx) {
+            public void onTransactionConfidenceChanged(Wallet wallet, Transaction tx) {
-                eventDead[0] = deadTx;
+                super.onTransactionConfidenceChanged(wallet, tx);
-                eventReplacement[0] = replacementTx;
+                eventDead[0] = tx;
+                eventReplacement[0] = tx.getConfidence().getOverridingTransaction();
             }
         });
 

tests/com/google/bitcoin/core/WalletTest.java

@@ -310,9 +310,13 @@ public class WalletTest {
         final Transaction[] eventReplacement = new Transaction[1];
         wallet.addEventListener(new AbstractWalletEventListener() {
             @Override
-            public void onDeadTransaction(Wallet wallet, Transaction deadTx, Transaction replacementTx) {
+            public void onTransactionConfidenceChanged(Wallet wallet, Transaction tx) {
-                eventDead[0] = deadTx;
+                super.onTransactionConfidenceChanged(wallet, tx);
-                eventReplacement[0] = replacementTx;
+                if (tx.getConfidence().getConfidenceType() ==
+                        TransactionConfidence.ConfidenceType.OVERRIDDEN_BY_DOUBLE_SPEND) {
+                    eventDead[0] = tx;
+                    eventReplacement[0] = tx.getConfidence().getOverridingTransaction();
+                }
             }
         });
 
@@ -474,9 +478,14 @@ public class WalletTest {
                 called[0] = tx;
             }
 
-            public void onDeadTransaction(Wallet wallet, Transaction deadTx, Transaction replacementTx) {
+            @Override
-                called[0] = deadTx;
+            public void onTransactionConfidenceChanged(Wallet wallet, Transaction tx) {
-                called[1] = replacementTx;
+                super.onTransactionConfidenceChanged(wallet, tx);
+                if (tx.getConfidence().getConfidenceType() == 
+                        TransactionConfidence.ConfidenceType.OVERRIDDEN_BY_DOUBLE_SPEND) {
+                    called[0] = tx;
+                    called[1] = tx.getConfidence().getOverridingTransaction();
+                }
             }
         });