From 09a2ca64d2134b0dcbb27b1a6eb17dda6087f448 Mon Sep 17 00:00:00 2001 From: Andreas Schildbach Date: Sat, 16 Jan 2016 18:23:53 +0100 Subject: [PATCH] Move Transaction.isConsistent() to Wallet.isTxConsistent(), as the wallet was the only consumer of that method. --- .../java/org/bitcoinj/core/Transaction.java | 23 ------------ .../main/java/org/bitcoinj/core/Wallet.java | 28 +++++++++++++-- .../org/bitcoinj/core/TransactionTest.java | 33 ----------------- .../java/org/bitcoinj/core/WalletTest.java | 35 +++++++++++++++++++ 4 files changed, 61 insertions(+), 58 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/Transaction.java b/core/src/main/java/org/bitcoinj/core/Transaction.java index 489ee6f28..bb8aaf599 100644 --- a/core/src/main/java/org/bitcoinj/core/Transaction.java +++ b/core/src/main/java/org/bitcoinj/core/Transaction.java @@ -270,29 +270,6 @@ public class Transaction extends ChildMessage { return inputTotal; } - /* - * If isSpent - check that all my outputs spent, otherwise check that there at least - * one unspent. - */ - boolean isConsistent(TransactionBag transactionBag, boolean isSpent) { - boolean isActuallySpent = true; - for (TransactionOutput o : outputs) { - if (o.isAvailableForSpending()) { - if (o.isMineOrWatched(transactionBag)) isActuallySpent = false; - if (o.getSpentBy() != null) { - log.error("isAvailableForSpending != spentBy"); - return false; - } - } else { - if (o.getSpentBy() == null) { - log.error("isAvailableForSpending != spentBy"); - return false; - } - } - } - return isActuallySpent == isSpent; - } - /** * Calculates the sum of the outputs that are sending coins to a key in the wallet. */ diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java index 1cbbba0ab..130c95feb 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -1505,14 +1505,14 @@ public class Wallet extends BaseTaggableObject } for (Transaction tx : unspent.values()) { - if (!tx.isConsistent(this, false)) { + if (!isTxConsistent(tx, false)) { success = false; log.error("Inconsistent unspent tx {}", tx.getHashAsString()); } } for (Transaction tx : spent.values()) { - if (!tx.isConsistent(this, true)) { + if (!isTxConsistent(tx, true)) { success = false; log.error("Inconsistent spent tx {}", tx.getHashAsString()); } @@ -1531,6 +1531,30 @@ public class Wallet extends BaseTaggableObject } } + /* + * If isSpent - check that all my outputs spent, otherwise check that there at least + * one unspent. + */ + @VisibleForTesting + boolean isTxConsistent(final Transaction tx, final boolean isSpent) { + boolean isActuallySpent = true; + for (TransactionOutput o : tx.getOutputs()) { + if (o.isAvailableForSpending()) { + if (o.isMineOrWatched(this)) isActuallySpent = false; + if (o.getSpentBy() != null) { + log.error("isAvailableForSpending != spentBy"); + return false; + } + } else { + if (o.getSpentBy() == null) { + log.error("isAvailableForSpending != spentBy"); + return false; + } + } + } + return isActuallySpent == isSpent; + } + /** Returns a wallet deserialized from the given input stream and wallet extensions. */ public static Wallet loadFromFileStream(InputStream stream, @Nullable WalletExtension... walletExtensions) throws UnreadableWalletException { Wallet wallet = new WalletProtobufSerializer().readWallet(stream, walletExtensions); diff --git a/core/src/test/java/org/bitcoinj/core/TransactionTest.java b/core/src/test/java/org/bitcoinj/core/TransactionTest.java index 2f97332ed..16597f39d 100644 --- a/core/src/test/java/org/bitcoinj/core/TransactionTest.java +++ b/core/src/test/java/org/bitcoinj/core/TransactionTest.java @@ -111,39 +111,6 @@ public class TransactionTest { tx.verify(); } - @Test - public void isConsistentReturnsFalseAsExpected() { - TransactionBag mockTB = createMock(TransactionBag.class); - - TransactionOutput to = createMock(TransactionOutput.class); - EasyMock.expect(to.isAvailableForSpending()).andReturn(true); - EasyMock.expect(to.isMineOrWatched(mockTB)).andReturn(true); - EasyMock.expect(to.getSpentBy()).andReturn(new TransactionInput(PARAMS, null, new byte[0])); - - Transaction tx = FakeTxBuilder.createFakeTxWithoutChange(PARAMS, to); - - replay(to); - - boolean isConsistent = tx.isConsistent(mockTB, false); - - assertEquals(isConsistent, false); - } - - @Test - public void isConsistentReturnsFalseAsExpected_WhenAvailableForSpendingEqualsFalse() { - TransactionOutput to = createMock(TransactionOutput.class); - EasyMock.expect(to.isAvailableForSpending()).andReturn(false); - EasyMock.expect(to.getSpentBy()).andReturn(null); - - Transaction tx = FakeTxBuilder.createFakeTxWithoutChange(PARAMS, to); - - replay(to); - - boolean isConsistent = tx.isConsistent(createMock(TransactionBag.class), false); - - assertEquals(isConsistent, false); - } - @Test public void testEstimatedLockTime_WhenParameterSignifiesBlockHeight() { int TEST_LOCK_TIME = 20; diff --git a/core/src/test/java/org/bitcoinj/core/WalletTest.java b/core/src/test/java/org/bitcoinj/core/WalletTest.java index 176c976cf..d00ffb6a6 100644 --- a/core/src/test/java/org/bitcoinj/core/WalletTest.java +++ b/core/src/test/java/org/bitcoinj/core/WalletTest.java @@ -39,6 +39,8 @@ import org.bitcoinj.utils.Fiat; import org.bitcoinj.utils.Threading; import org.bitcoinj.wallet.*; import org.bitcoinj.wallet.WalletTransaction.Pool; +import org.easymock.EasyMock; + import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.common.util.concurrent.ListenableFuture; @@ -65,6 +67,8 @@ import java.util.concurrent.atomic.AtomicReference; import static org.bitcoinj.core.Coin.*; import static org.bitcoinj.core.Utils.HEX; import static org.bitcoinj.testing.FakeTxBuilder.*; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.replay; import static com.google.common.base.Preconditions.checkNotNull; import static org.junit.Assert.*; @@ -685,6 +689,37 @@ public class WalletTest extends TestWithWallet { assertFalse(wallet.isConsistent()); } + @Test + public void isTxConsistentReturnsFalseAsExpected() { + Wallet wallet = new Wallet(params); + TransactionOutput to = createMock(TransactionOutput.class); + EasyMock.expect(to.isAvailableForSpending()).andReturn(true); + EasyMock.expect(to.isMineOrWatched(wallet)).andReturn(true); + EasyMock.expect(to.getSpentBy()).andReturn(new TransactionInput(params, null, new byte[0])); + + Transaction tx = FakeTxBuilder.createFakeTxWithoutChange(params, to); + + replay(to); + + boolean isConsistent = wallet.isTxConsistent(tx, false); + assertFalse(isConsistent); + } + + @Test + public void isTxConsistentReturnsFalseAsExpected_WhenAvailableForSpendingEqualsFalse() { + Wallet wallet = new Wallet(params); + TransactionOutput to = createMock(TransactionOutput.class); + EasyMock.expect(to.isAvailableForSpending()).andReturn(false); + EasyMock.expect(to.getSpentBy()).andReturn(null); + + Transaction tx = FakeTxBuilder.createFakeTxWithoutChange(params, to); + + replay(to); + + boolean isConsistent = wallet.isTxConsistent(tx, false); + assertFalse(isConsistent); + } + @Test public void transactions() throws Exception { // This test covers a bug in which Transaction.getValueSentFromMe was calculating incorrectly.