From 5ae00d4e20efbf39a69a4c56851e2efbca511153 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 3 Apr 2013 00:12:24 +0100 Subject: [PATCH] Simplifications to the wallet code. Wallet: Remove/deprecate NOT_IN_BEST_CHAIN as a confidence type. TxConfidence: Rename NOT_SEEN_IN_CHAIN -> PENDING which is more precise and consistent. PeerGroup: Fix definition of "mined" --- core/src/bitcoin.proto | 4 +- .../com/google/bitcoin/core/PeerGroup.java | 2 +- .../com/google/bitcoin/core/Transaction.java | 10 +-- .../bitcoin/core/TransactionConfidence.java | 25 +++----- .../java/com/google/bitcoin/core/Wallet.java | 61 ++++++++----------- .../store/WalletProtobufSerializer.java | 16 +++-- .../google/bitcoin/core/ChainSplitTest.java | 22 ++++--- .../com/google/bitcoin/core/WalletTest.java | 8 +-- 8 files changed, 67 insertions(+), 81 deletions(-) diff --git a/core/src/bitcoin.proto b/core/src/bitcoin.proto index c0afed23..0ce4bb69 100644 --- a/core/src/bitcoin.proto +++ b/core/src/bitcoin.proto @@ -107,8 +107,8 @@ message TransactionConfidence { enum Type { UNKNOWN = 0; BUILDING = 1; // In best chain. If and only if appeared_at_height is present. - NOT_SEEN_IN_CHAIN = 2; // Pending inclusion in best chain. - NOT_IN_BEST_CHAIN = 3; // In non-best chain, pending inclusion in best chain. + PENDING = 2; // Unconfirmed and sitting in the networks memory pools, waiting to be included in the chain. + NOT_IN_BEST_CHAIN = 3; // Deprecated: equivalent to PENDING. DEAD = 4; // Either if overriding_transaction is present or transaction is dead coinbase } diff --git a/core/src/main/java/com/google/bitcoin/core/PeerGroup.java b/core/src/main/java/com/google/bitcoin/core/PeerGroup.java index c3c5c6c7..03fa2dfe 100644 --- a/core/src/main/java/com/google/bitcoin/core/PeerGroup.java +++ b/core/src/main/java/com/google/bitcoin/core/PeerGroup.java @@ -1153,7 +1153,7 @@ public class PeerGroup extends AbstractIdleService { // Thread safe - this can run in parallel. final TransactionConfidence conf = tx.getConfidence(); int numSeenPeers = conf.numBroadcastPeers(); - boolean mined = conf.getConfidenceType() != TransactionConfidence.ConfidenceType.NOT_SEEN_IN_CHAIN; + boolean mined = tx.getAppearsInHashes() != null; log.info("broadcastTransaction: TX {} seen by {} peers{}", new Object[]{pinnedTx.getHashAsString(), numSeenPeers, mined ? " and mined" : ""}); if (!(numSeenPeers >= minConnections || mined)) diff --git a/core/src/main/java/com/google/bitcoin/core/Transaction.java b/core/src/main/java/com/google/bitcoin/core/Transaction.java index 115d49b9..eb9867e2 100644 --- a/core/src/main/java/com/google/bitcoin/core/Transaction.java +++ b/core/src/main/java/com/google/bitcoin/core/Transaction.java @@ -237,7 +237,7 @@ public class Transaction extends ChildMessage implements Serializable { * @return true if this transaction hasn't been seen in any block yet. */ public boolean isPending() { - return getConfidence().getConfidenceType() == TransactionConfidence.ConfidenceType.NOT_SEEN_IN_CHAIN; + return getConfidence().getConfidenceType() == TransactionConfidence.ConfidenceType.PENDING; } /** @@ -289,14 +289,6 @@ public class Transaction extends ChildMessage implements Serializable { appearsInHashes.add(blockHash); } - /** Called by the wallet once a re-org means we don't appear in the best chain anymore. */ - void notifyNotOnBestChain() { - TransactionConfidence transactionConfidence = getConfidence(); - transactionConfidence.setConfidenceType(TransactionConfidence.ConfidenceType.NOT_IN_BEST_CHAIN); - transactionConfidence.setDepthInBlocks(0); - transactionConfidence.setWorkDone(BigInteger.ZERO); - } - /** * Calculates the sum of the inputs that are spending coins with keys in the wallet. This requires the * transactions sending coins to those keys to be in the wallet. This method will not attempt to download the diff --git a/core/src/main/java/com/google/bitcoin/core/TransactionConfidence.java b/core/src/main/java/com/google/bitcoin/core/TransactionConfidence.java index 331b6c83..5545ca24 100644 --- a/core/src/main/java/com/google/bitcoin/core/TransactionConfidence.java +++ b/core/src/main/java/com/google/bitcoin/core/TransactionConfidence.java @@ -76,21 +76,14 @@ public class TransactionConfidence implements Serializable { BUILDING(1), /** - * If NOT_SEEN_IN_CHAIN, then the transaction is pending and should be included shortly, as long as it is being + * If PENDING, then the transaction is unconfirmed and should be included shortly, as long as it is being * announced and is considered valid by the network. A pending transaction will be announced if the containing * wallet has been attached to a live {@link PeerGroup} using {@link PeerGroup#addWallet(Wallet)}. * You can estimate how likely the transaction is to be included by connecting to a bunch of nodes then measuring * how many announce it, using {@link com.google.bitcoin.core.TransactionConfidence#numBroadcastPeers()}. * Or if you saw it from a trusted peer, you can assume it's valid and will get mined sooner or later as well. */ - NOT_SEEN_IN_CHAIN(2), - - /** - * If NOT_IN_BEST_CHAIN, then the transaction has been included in a block, but that block is on a fork. A - * transaction can change from BUILDING to NOT_IN_BEST_CHAIN and vice versa if a reorganization takes place, - * due to a split in the consensus. - */ - NOT_IN_BEST_CHAIN(3), + PENDING(2), /** * If DEAD, then it means the transaction won't confirm unless there is another re-org, @@ -118,8 +111,7 @@ public class TransactionConfidence implements Serializable { switch (value) { case 0: return UNKNOWN; case 1: return BUILDING; - case 2: return NOT_SEEN_IN_CHAIN; - case 3: return NOT_IN_BEST_CHAIN; + case 2: return PENDING; case 4: return DEAD; default: return null; } @@ -238,7 +230,7 @@ public class TransactionConfidence implements Serializable { /** * Called by a {@link Peer} when a transaction is pending and announced by a peer. The more peers announce the * transaction, the more peers have validated it (assuming your internet connection is not being intercepted). - * If confidence is currently unknown, sets it to {@link ConfidenceType#NOT_SEEN_IN_CHAIN}. Listeners will be + * If confidence is currently unknown, sets it to {@link ConfidenceType#PENDING}. Listeners will be * invoked in this case. * * @param address IP address of the peer, used as a proxy for identity. @@ -248,7 +240,7 @@ public class TransactionConfidence implements Serializable { return; // Duplicate. synchronized (this) { if (getConfidenceType() == ConfidenceType.UNKNOWN) { - this.confidenceType = ConfidenceType.NOT_SEEN_IN_CHAIN; + this.confidenceType = ConfidenceType.PENDING; } } runListeners(); @@ -292,11 +284,8 @@ public class TransactionConfidence implements Serializable { case DEAD: builder.append("Dead: overridden by double spend and will not confirm."); break; - case NOT_IN_BEST_CHAIN: - builder.append("Seen in side chain but not best chain."); - break; - case NOT_SEEN_IN_CHAIN: - builder.append("Not seen in chain."); + case PENDING: + builder.append("Pending/unconfirmed."); break; case BUILDING: builder.append(String.format("Appeared in best chain at height %d, depth %d, work done %s.", diff --git a/core/src/main/java/com/google/bitcoin/core/Wallet.java b/core/src/main/java/com/google/bitcoin/core/Wallet.java index 529cbb47..d69319ad 100644 --- a/core/src/main/java/com/google/bitcoin/core/Wallet.java +++ b/core/src/main/java/com/google/bitcoin/core/Wallet.java @@ -17,6 +17,8 @@ package com.google.bitcoin.core; import com.google.bitcoin.crypto.KeyCrypterScrypt; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; import org.bitcoinj.wallet.Protos.Wallet.EncryptionType; import org.spongycastle.crypto.params.KeyParameter; @@ -277,19 +279,10 @@ public class Wallet implements Serializable, BlockChainListener { // Only pick chain-included transactions, or transactions that are ours and pending. TransactionConfidence confidence = tx.getConfidence(); ConfidenceType type = confidence.getConfidenceType(); - boolean pending = type.equals(ConfidenceType.NOT_SEEN_IN_CHAIN) || - type.equals(ConfidenceType.NOT_IN_BEST_CHAIN); - boolean confirmed = type.equals(ConfidenceType.BUILDING); - if (!confirmed) { - // If the transaction is still pending ... - if (!pending) return false; - // And it was created by us ... - if (!confidence.getSource().equals(TransactionConfidence.Source.SELF)) return false; - // And it's been seen by the network and propagated ... - if (confidence.numBroadcastPeers() <= 1) return false; - // Then it's OK to select. - } - return true; + if (type.equals(ConfidenceType.BUILDING)) return true; + return type.equals(ConfidenceType.PENDING) && + confidence.getSource().equals(TransactionConfidence.Source.SELF) && + confidence.numBroadcastPeers() > 1; } } @@ -921,7 +914,7 @@ public class Wallet implements Serializable, BlockChainListener { // have been missed out. ConfidenceType currentConfidence = tx.getConfidence().getConfidenceType(); if (currentConfidence == ConfidenceType.UNKNOWN) { - tx.getConfidence().setConfidenceType(ConfidenceType.NOT_SEEN_IN_CHAIN); + tx.getConfidence().setConfidenceType(ConfidenceType.PENDING); // Manually invoke the wallet tx confidence listener here as we didn't yet commit therefore the // txConfidenceListener wasn't added. invokeOnTransactionConfidenceChanged(tx); @@ -1983,7 +1976,7 @@ public class Wallet implements Serializable, BlockChainListener { return false; } checkState(selection.gathered.size() > 0); - req.tx.getConfidence().setConfidenceType(ConfidenceType.NOT_SEEN_IN_CHAIN); + req.tx.getConfidence().setConfidenceType(ConfidenceType.PENDING); BigInteger change = selection.valueGathered.subtract(value); if (change.compareTo(BigInteger.ZERO) > 0) { // The value of the inputs is greater than what we want to send. Just like in real life then, @@ -2318,10 +2311,9 @@ public class Wallet implements Serializable, BlockChainListener { /** *

Don't call this directly. It's not intended for API users.

* - *

Called by the {@link BlockChain} when the best chain (representing total work done) has changed. In this case, - * we need to go through our transactions and find out if any have become invalid. It's possible for our balance - * to go down in this case: money we thought we had can suddenly vanish if the rest of the network agrees it - * should be so.

+ *

Called by the {@link BlockChain} when the best chain (representing total work done) has changed. This can + * cause the number of confirmations of a transaction to go higher, lower, drop to zero and can even result in + * a transaction going dead (will never confirm) due to a double spend.

* *

The oldBlocks/newBlocks lists are ordered height-wise from top first to bottom last.

*/ @@ -2330,7 +2322,7 @@ public class Wallet implements Serializable, BlockChainListener { try { // This runs on any peer thread with the block chain synchronized. // - // The reorganize functionality of the wallet is tested in ChainSplitTests. + // The reorganize functionality of the wallet is tested in ChainSplitTest. // // For each transaction we track which blocks they appeared in. Once a re-org takes place we have to find all // transactions in the old branch, all transactions in the new branch and find the difference of those sets. @@ -2371,9 +2363,16 @@ public class Wallet implements Serializable, BlockChainListener { } } + // Map block hash to transactions that appear in it. + Multimap blockTxMap = ArrayListMultimap.create(); + for (Transaction tx : all.values()) { Collection appearsIn = tx.getAppearsInHashes(); checkNotNull(appearsIn); + + for (Sha256Hash block : appearsIn) + blockTxMap.put(block, tx); + // If the set of blocks this transaction appears in is disjoint with one of the chain segments it means // the transaction was never incorporated by a miner into that side of the chain. boolean inOldSection = !Collections.disjoint(appearsIn, oldBlockHashes); @@ -2435,24 +2434,15 @@ public class Wallet implements Serializable, BlockChainListener { spent.clear(); inactive.clear(); for (Transaction tx : commonChainTransactions.values()) { - int unspentOutputs = 0; - for (TransactionOutput output : tx.getOutputs()) { - if (output.isAvailableForSpending() && output.isMine(this)) unspentOutputs++; - } - if (unspentOutputs > 0) { - log.info(" TX {} ->unspent", tx.getHashAsString()); - unspent.put(tx.getHash(), tx); - } else { - log.info(" TX {} ->spent", tx.getHashAsString()); + if (tx.isEveryOwnedOutputSpent(this)) spent.put(tx.getHash(), tx); - } + else + unspent.put(tx.getHash(), tx); } // Inform all transactions that exist only in the old chain that they have moved, so they can update confidence // and timestamps. Transactions will be told they're on the new best chain when the blocks are replayed. for (Transaction tx : onlyOldChainTransactions.values()) { - tx.notifyNotOnBestChain(); - // Kill any coinbase transactions that are only in the old chain. // These transactions are no longer valid. if (tx.isCoinBase()) { @@ -2624,12 +2614,15 @@ public class Wallet implements Serializable, BlockChainListener { // If all inputs do not appear in this wallet move to inactive. if (noSuchTx == numInputs) { - log.info(" ->inactive", tx.getHashAsString() + ", confidence = " + tx.getConfidence().getConfidenceType().name()); + tx.getConfidence().setConfidenceType(ConfidenceType.PENDING); + log.info(" ->inactive", tx.getHashAsString() + ", confidence = PENDING"); inactive.put(tx.getHash(), tx); dead.remove(tx.getHash()); + } else if (success == numInputs - noSuchTx) { // All inputs are either valid for spending or don't come from us. Miners are trying to reinclude it. - log.info(" ->pending", tx.getHashAsString() + ", confidence = " + tx.getConfidence().getConfidenceType().name()); + tx.getConfidence().setConfidenceType(ConfidenceType.PENDING); + log.info(" ->pending", tx.getHashAsString() + ", confidence = PENDING"); pending.put(tx.getHash(), tx); dead.remove(tx.getHash()); } diff --git a/core/src/main/java/com/google/bitcoin/store/WalletProtobufSerializer.java b/core/src/main/java/com/google/bitcoin/store/WalletProtobufSerializer.java index c32ce9ec..a2b8b0fa 100644 --- a/core/src/main/java/com/google/bitcoin/store/WalletProtobufSerializer.java +++ b/core/src/main/java/com/google/bitcoin/store/WalletProtobufSerializer.java @@ -26,11 +26,9 @@ import java.util.*; import org.bitcoinj.wallet.Protos; import org.bitcoinj.wallet.Protos.Wallet.EncryptionType; -import org.bitcoinj.wallet.Protos.Key.Type; import com.google.bitcoin.crypto.EncryptedPrivateKey; import com.google.bitcoin.crypto.KeyCrypter; -import com.google.bitcoin.crypto.KeyCrypterException; import com.google.bitcoin.crypto.KeyCrypterScrypt; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -489,8 +487,18 @@ public class WalletProtobufSerializer { log.warn("Unknown confidence type for tx {}", tx.getHashAsString()); return; } - ConfidenceType confidenceType = - TransactionConfidence.ConfidenceType.valueOf(confidenceProto.getType().getNumber()); + ConfidenceType confidenceType; + switch (confidenceProto.getType()) { + case BUILDING: confidenceType = ConfidenceType.BUILDING; break; + case DEAD: confidenceType = ConfidenceType.DEAD; break; + // These two are equivalent (must be able to read old wallets). + case NOT_IN_BEST_CHAIN: confidenceType = ConfidenceType.PENDING; break; + case NOT_SEEN_IN_CHAIN: confidenceType = ConfidenceType.PENDING; break; + case UNKNOWN: + // Fall through. + default: + confidenceType = ConfidenceType.UNKNOWN; break; + } confidence.setConfidenceType(confidenceType); if (confidenceProto.hasAppearedAtHeight()) { if (confidence.getConfidenceType() != ConfidenceType.BUILDING) { diff --git a/core/src/test/java/com/google/bitcoin/core/ChainSplitTest.java b/core/src/test/java/com/google/bitcoin/core/ChainSplitTest.java index 4826f158..98f80c3f 100644 --- a/core/src/test/java/com/google/bitcoin/core/ChainSplitTest.java +++ b/core/src/test/java/com/google/bitcoin/core/ChainSplitTest.java @@ -25,6 +25,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.math.BigInteger; +import java.net.InetAddress; import java.util.ArrayList; import static org.junit.Assert.*; @@ -146,24 +147,27 @@ public class ChainSplitTest { Address dest = new ECKey().toAddress(unitTestParams); Transaction spend = wallet.createSend(dest, Utils.toNanoCoins(10, 0)); wallet.commitTx(spend); - // Waiting for confirmation ... + // Waiting for confirmation ... make it eligible for selection. assertEquals(BigInteger.ZERO, wallet.getBalance()); + spend.getConfidence().markBroadcastBy(new PeerAddress(InetAddress.getByAddress(new byte[]{1, 2, 3, 4}))); + spend.getConfidence().markBroadcastBy(new PeerAddress(InetAddress.getByAddress(new byte[]{5,6,7,8}))); + assertEquals(ConfidenceType.PENDING, spend.getConfidence().getConfidenceType()); + assertEquals(Utils.toNanoCoins(40, 0), wallet.getBalance()); Block b2 = b1.createNextBlock(someOtherGuy); b2.addTransaction(spend); b2.solve(); chain.add(b2); - assertEquals(Utils.toNanoCoins(40, 0), wallet.getBalance()); + // We have 40 coins in change. + assertEquals(ConfidenceType.BUILDING, spend.getConfidence().getConfidenceType()); // genesis -> b1 (receive coins) -> b2 (spend coins) // \-> b3 -> b4 Block b3 = b1.createNextBlock(someOtherGuy); Block b4 = b3.createNextBlock(someOtherGuy); chain.add(b3); chain.add(b4); - // b4 causes a re-org that should make our spend go inactive. Because the inputs are already spent our - // available balance drops to zero again. - assertEquals(BigInteger.ZERO, wallet.getBalance(Wallet.BalanceType.AVAILABLE)); - // We estimate that it'll make it back into the block chain (we know we won't double spend). - // assertEquals(Utils.toNanoCoins(40, 0), wallet.getBalance(Wallet.BalanceType.ESTIMATED)); + // b4 causes a re-org that should make our spend go pending again. + assertEquals(Utils.toNanoCoins(40, 0), wallet.getBalance()); + assertEquals(ConfidenceType.PENDING, spend.getConfidence().getConfidenceType()); } @Test @@ -416,8 +420,8 @@ public class ChainSplitTest { assertEquals(4, txns.get(0).getConfidence().getDepthInBlocks()); assertEquals(work1.add(work4).add(work5).add(work6), txns.get(0).getConfidence().getWorkDone()); - // Transaction 1 (in block b2) is now on a side chain. - assertEquals(TransactionConfidence.ConfidenceType.NOT_IN_BEST_CHAIN, txns.get(1).getConfidence().getConfidenceType()); + // Transaction 1 (in block b2) is now on a side chain, so it goes pending (not see in chain). + assertEquals(ConfidenceType.PENDING, txns.get(1).getConfidence().getConfidenceType()); try { txns.get(1).getConfidence().getAppearedAtChainHeight(); fail(); diff --git a/core/src/test/java/com/google/bitcoin/core/WalletTest.java b/core/src/test/java/com/google/bitcoin/core/WalletTest.java index 9e56cc7f..1323e493 100644 --- a/core/src/test/java/com/google/bitcoin/core/WalletTest.java +++ b/core/src/test/java/com/google/bitcoin/core/WalletTest.java @@ -190,7 +190,7 @@ public class WalletTest extends TestWithWallet { private void basicSanityChecks(Wallet wallet, Transaction t, Address fromAddress, Address destination) throws ScriptException { assertEquals("Wrong number of tx inputs", 1, t.getInputs().size()); assertEquals(fromAddress, t.getInputs().get(0).getScriptSig().getFromAddress(params)); - assertEquals(t.getConfidence().getConfidenceType(), TransactionConfidence.ConfidenceType.NOT_SEEN_IN_CHAIN); + assertEquals(t.getConfidence().getConfidenceType(), TransactionConfidence.ConfidenceType.PENDING); assertEquals("Wrong number of tx outputs",2, t.getOutputs().size()); assertEquals(destination, t.getOutputs().get(0).getScriptPubKey().getToAddress(params)); assertEquals(wallet.getChangeAddress(), t.getOutputs().get(1).getScriptPubKey().getToAddress(params)); @@ -263,7 +263,7 @@ public class WalletTest extends TestWithWallet { assertTrue(complete); assertEquals(1, t2.getInputs().size()); assertEquals(myAddress, t2.getInputs().get(0).getScriptSig().getFromAddress(params)); - assertEquals(t2.getConfidence().getConfidenceType(), TransactionConfidence.ConfidenceType.NOT_SEEN_IN_CHAIN); + assertEquals(t2.getConfidence().getConfidenceType(), TransactionConfidence.ConfidenceType.PENDING); // We have NOT proven that the signature is correct! wallet.commitTx(t2); @@ -577,7 +577,7 @@ public class WalletTest extends TestWithWallet { TestUtils.DoubleSpends doubleSpends = TestUtils.createFakeDoubleSpendTxns(params, myAddress); // t1 spends to our wallet. t2 double spends somewhere else. wallet.receivePending(doubleSpends.t1, null); - assertEquals(TransactionConfidence.ConfidenceType.NOT_SEEN_IN_CHAIN, + assertEquals(TransactionConfidence.ConfidenceType.PENDING, doubleSpends.t1.getConfidence().getConfidenceType()); sendMoneyToWallet(doubleSpends.t2, AbstractBlockChain.NewBlockType.BEST_CHAIN); assertEquals(TransactionConfidence.ConfidenceType.DEAD, @@ -632,7 +632,7 @@ public class WalletTest extends TestWithWallet { flags[1] = true; } }); - assertEquals(TransactionConfidence.ConfidenceType.NOT_SEEN_IN_CHAIN, + assertEquals(TransactionConfidence.ConfidenceType.PENDING, notifiedTx[0].getConfidence().getConfidenceType()); final Transaction t1Copy = new Transaction(params, t1.bitcoinSerialize()); sendMoneyToWallet(t1Copy, AbstractBlockChain.NewBlockType.BEST_CHAIN);