3
0
mirror of https://github.com/Qortal/altcoinj.git synced 2025-01-31 23:32:16 +00:00

Lots of bug fixes to double spend handling.

Fix some corruptions that could occur during a Finney attack. Resolves issue 182.
This commit is contained in:
Mike Hearn 2013-03-21 16:14:16 +01:00
parent 76e539e8e7
commit 56285da06c
2 changed files with 84 additions and 23 deletions

View File

@ -757,6 +757,7 @@ public class Wallet implements Serializable, BlockChainListener {
} }
} }
if (!success) log.error(toString());
return success; return success;
} finally { } finally {
lock.unlock(); lock.unlock();
@ -961,7 +962,7 @@ public class Wallet implements Serializable, BlockChainListener {
try { try {
return tx.getValueSentFromMe(this).compareTo(BigInteger.ZERO) > 0 || return tx.getValueSentFromMe(this).compareTo(BigInteger.ZERO) > 0 ||
tx.getValueSentToMe(this).compareTo(BigInteger.ZERO) > 0 || tx.getValueSentToMe(this).compareTo(BigInteger.ZERO) > 0 ||
findDoubleSpendAgainstPending(tx) != null; checkForDoubleSpendAgainstPending(tx, false);
} finally { } finally {
lock.unlock(); lock.unlock();
} }
@ -971,7 +972,7 @@ public class Wallet implements Serializable, BlockChainListener {
* Checks if "tx" is spending any inputs of pending transactions. Not a general check, but it can work even if * Checks if "tx" is spending any inputs of pending transactions. Not a general check, but it can work even if
* the double spent inputs are not ours. Returns the pending tx that was double spent or null if none found. * the double spent inputs are not ours. Returns the pending tx that was double spent or null if none found.
*/ */
private TransactionInput findDoubleSpendAgainstPending(Transaction tx) { private boolean checkForDoubleSpendAgainstPending(Transaction tx, boolean takeAction) {
checkState(lock.isLocked()); checkState(lock.isLocked());
// Compile a set of outpoints that are spent by tx. // Compile a set of outpoints that are spent by tx.
HashSet<TransactionOutPoint> outpoints = new HashSet<TransactionOutPoint>(); HashSet<TransactionOutPoint> outpoints = new HashSet<TransactionOutPoint>();
@ -981,13 +982,24 @@ public class Wallet implements Serializable, BlockChainListener {
// Now for each pending transaction, see if it shares any outpoints with this tx. // Now for each pending transaction, see if it shares any outpoints with this tx.
for (Transaction p : pending.values()) { for (Transaction p : pending.values()) {
for (TransactionInput input : p.getInputs()) { for (TransactionInput input : p.getInputs()) {
if (outpoints.contains(input.getOutpoint())) { // This relies on the fact that TransactionOutPoint equality is defined at the protocol not object
// level - outpoints from two different inputs that point to the same output compare the same.
TransactionOutPoint outpoint = input.getOutpoint();
if (outpoints.contains(outpoint)) {
// It does, it's a double spend against the pending pool, which makes it relevant. // It does, it's a double spend against the pending pool, which makes it relevant.
return input; if (takeAction) {
// Look for the actual input object in tx that is double spending.
TransactionInput overridingInput = null;
for (TransactionInput txInput : tx.getInputs()) {
if (txInput.getOutpoint().equals(outpoint)) overridingInput = txInput;
}
killTx(tx, checkNotNull(overridingInput), p);
}
return true;
} }
} }
} }
return null; return false;
} }
/** /**
@ -1236,12 +1248,7 @@ public class Wallet implements Serializable, BlockChainListener {
addWalletTransaction(Pool.SPENT, tx); addWalletTransaction(Pool.SPENT, tx);
} }
TransactionInput doubleSpent = findDoubleSpendAgainstPending(tx); checkForDoubleSpendAgainstPending(tx, true);
if (doubleSpent != null) {
// This is mostly the same as the codepath in updateForSpends, but that one is only triggered when
// the transaction being double spent is actually in our wallet (ie, maybe we're double spending).
killTx(tx, doubleSpent, doubleSpent.getParentTransaction());
}
} }
/** /**
@ -1295,7 +1302,7 @@ public class Wallet implements Serializable, BlockChainListener {
// The outputs are already marked as spent by the connect call above, so check if there are any more for // The outputs are already marked as spent by the connect call above, so check if there are any more for
// us to use. Move if not. // us to use. Move if not.
Transaction connected = checkNotNull(input.getOutpoint().fromTx); Transaction connected = checkNotNull(input.getOutpoint().fromTx);
maybeMoveTxToSpent(connected, "prevtx"); maybeMovePool(connected, "prevtx");
} }
} }
// Now check each output and see if there is a pending transaction which spends it. This shouldn't normally // Now check each output and see if there is a pending transaction which spends it. This shouldn't normally
@ -1321,35 +1328,62 @@ public class Wallet implements Serializable, BlockChainListener {
} }
} }
// Updates the wallet when a double spend occurs.
private void killTx(Transaction overridingTx, TransactionInput overridingInput, Transaction killedTx) { private void killTx(Transaction overridingTx, TransactionInput overridingInput, Transaction killedTx) {
TransactionOutPoint overriddenOutPoint = overridingInput.getOutpoint(); TransactionOutPoint overriddenOutPoint = overridingInput.getOutpoint();
// It is expected that we may not have the overridden/double-spent tx in our wallet ... in the (common?!) case
// where somebody is stealing money from us, the overriden tx belongs to someone else.
log.warn("Saw double spend of {} from chain override pending tx {}", log.warn("Saw double spend of {} from chain override pending tx {}",
overriddenOutPoint, killedTx.getHashAsString()); overriddenOutPoint, killedTx.getHashAsString());
log.warn(" <-pending ->dead killed by {}", overridingTx.getHashAsString()); log.warn(" <-pending ->dead killed by {}", overridingTx.getHashAsString());
checkState(overridingInput.connect(overridingTx, TransactionInput.ConnectMode.DISCONNECT_ON_CONFLICT) == TransactionInput.ConnectionResult.SUCCESS);
pending.remove(killedTx.getHash()); pending.remove(killedTx.getHash());
addWalletTransaction(Pool.DEAD, killedTx); addWalletTransaction(Pool.DEAD, killedTx);
// Inform the [tx] event listeners of the newly dead tx and disconnect the other inputs. log.info("Disconnecting inputs of the newly dead tx");
for (TransactionInput deadInput : killedTx.getInputs()) deadInput.disconnect(); for (TransactionInput deadInput : killedTx.getInputs()) {
killedTx.getConfidence().setOverridingTransaction(overridingTx); Transaction connected = deadInput.getOutpoint().fromTx;
// TODO: Move newly unspent transactions (if any) back into the unspent pool to avoid inconsistency. if (connected == null) continue;
deadInput.disconnect();
maybeMovePool(connected, "kill");
}
// Try and connect the overriding input to something in our wallet. It's expected that this will mostly fail
// because when somebody else is double-spending away a payment they made to us, we won't have the overridden
// tx as it's not ours to begin with. It'll only be found if we're double spending our own payments.
log.info("Trying to connect overriding tx back");
TransactionInput.ConnectionResult result = overridingInput.connect(unspent, TransactionInput.ConnectMode.DISCONNECT_ON_CONFLICT);
if (result == TransactionInput.ConnectionResult.SUCCESS) {
maybeMovePool(overridingInput.getOutpoint().fromTx, "kill");
} else {
result = overridingInput.connect(spent, TransactionInput.ConnectMode.DISCONNECT_ON_CONFLICT);
if (result == TransactionInput.ConnectionResult.SUCCESS) {
maybeMovePool(overridingInput.getOutpoint().fromTx, "kill");
}
}
log.info("Informing tx listeners of double spend event");
killedTx.getConfidence().setOverridingTransaction(overridingTx); // RE-ENTRY POINT
// TODO: Recursively kill other transactions that were double spent. // TODO: Recursively kill other transactions that were double spent.
} }
/** /**
* If the transactions outputs are all marked as spent, and it's in the unspent map, move it. * If the transactions outputs are all marked as spent, and it's in the unspent map, move it.
* If the owned transactions outputs are not all marked as spent, and it's in the spent map, move it.
*/ */
private void maybeMoveTxToSpent(Transaction tx, String context) { private void maybeMovePool(Transaction tx, String context) {
checkState(lock.isLocked()); checkState(lock.isLocked());
if (tx.isEveryOwnedOutputSpent(this)) { if (tx.isEveryOwnedOutputSpent(this)) {
// There's nothing left I can spend in this transaction. // There's nothing left I can spend in this transaction.
if (unspent.remove(tx.getHash()) != null) { if (unspent.remove(tx.getHash()) != null) {
if (log.isInfoEnabled()) { if (log.isInfoEnabled()) {
log.info(" {} {} <-unspent", tx.getHashAsString(), context); log.info(" {} {} <-unspent ->spent", tx.getHashAsString(), context);
log.info(" {} {} ->spent", tx.getHashAsString(), context);
} }
spent.put(tx.getHash(), tx); spent.put(tx.getHash(), tx);
} }
} else {
if (spent.remove(tx.getHash()) != null) {
if (log.isInfoEnabled()) {
log.info(" {} {} <-spent ->unspent", tx.getHashAsString(), context);
}
unspent.put(tx.getHash(), tx);
}
} }
} }
@ -1947,7 +1981,7 @@ public class Wallet implements Serializable, BlockChainListener {
req.tx.getConfidence().setSource(TransactionConfidence.Source.SELF); req.tx.getConfidence().setSource(TransactionConfidence.Source.SELF);
req.completed = true; req.completed = true;
log.info(" completed {}", req.tx.getHashAsString()); log.info(" completed {} with {} inputs", req.tx.getHashAsString(), req.tx.getInputs().size());
return true; return true;
} finally { } finally {
lock.unlock(); lock.unlock();
@ -2564,7 +2598,7 @@ public class Wallet implements Serializable, BlockChainListener {
// The act of re-connecting this un-included transaction may have caused other transactions to become fully // The act of re-connecting this un-included transaction may have caused other transactions to become fully
// spent so move them into the right bucket here to keep performance good. // spent so move them into the right bucket here to keep performance good.
for (Transaction maybeSpent : connectedTransactions) { for (Transaction maybeSpent : connectedTransactions) {
maybeMoveTxToSpent(maybeSpent, "reorg"); maybeMovePool(maybeSpent, "reorg");
} }
} }

View File

@ -568,6 +568,30 @@ public class WalletTest {
assertEquals(coin1, wallet.getBalance()); assertEquals(coin1, wallet.getBalance());
} }
@Test
public void doubleSpendUnspendsOtherInputs() throws Exception {
// Test another Finney attack, but this time the killed transaction was also spending some other outputs in
// our wallet which were not themselves double spent. This test ensures the death of the pending transaction
// frees up the other outputs and makes them spendable again.
// Receive 1 coin and then 2 coins in separate transactions.
sendMoneyToWallet(Utils.toNanoCoins(1, 0), AbstractBlockChain.NewBlockType.BEST_CHAIN);
sendMoneyToWallet(Utils.toNanoCoins(2, 0), AbstractBlockChain.NewBlockType.BEST_CHAIN);
// Create a send to a merchant of all our coins.
Transaction send1 = wallet.createSend(new ECKey().toAddress(params), toNanoCoins(2, 90));
// Create a double spend of just the first one.
Transaction send2 = wallet.createSend(new ECKey().toAddress(params), toNanoCoins(1, 0));
send2 = new Transaction(params, send2.bitcoinSerialize());
// Broadcast send1, it's now pending.
wallet.commitTx(send1);
assertEquals(BigInteger.ZERO, wallet.getBalance());
// Receive a block that overrides the send1 using send2.
sendMoneyToWallet(send2, AbstractBlockChain.NewBlockType.BEST_CHAIN);
// send1 got rolled back and replaced with a smaller send that only used one of our received coins, thus ...
assertEquals(Utils.toNanoCoins(2, 0), wallet.getBalance());
assertTrue(wallet.isConsistent());
}
@Test @Test
public void doubleSpendFinneyAttack() throws Exception { public void doubleSpendFinneyAttack() throws Exception {
// A Finney attack is where a miner includes a transaction spending coins to themselves but does not // A Finney attack is where a miner includes a transaction spending coins to themselves but does not
@ -601,6 +625,7 @@ public class WalletTest {
// Receive 1 BTC. // Receive 1 BTC.
BigInteger nanos = Utils.toNanoCoins(1, 0); BigInteger nanos = Utils.toNanoCoins(1, 0);
sendMoneyToWallet(nanos, AbstractBlockChain.NewBlockType.BEST_CHAIN); sendMoneyToWallet(nanos, AbstractBlockChain.NewBlockType.BEST_CHAIN);
Transaction received = wallet.getTransactions(false, false).iterator().next();
// Create a send to a merchant. // Create a send to a merchant.
Transaction send1 = wallet.createSend(new ECKey().toAddress(params), toNanoCoins(0, 50)); Transaction send1 = wallet.createSend(new ECKey().toAddress(params), toNanoCoins(0, 50));
// Create a double spend. // Create a double spend.
@ -608,12 +633,14 @@ public class WalletTest {
send2 = new Transaction(params, send2.bitcoinSerialize()); send2 = new Transaction(params, send2.bitcoinSerialize());
// Broadcast send1. // Broadcast send1.
wallet.commitTx(send1); wallet.commitTx(send1);
assertEquals(send1, received.getOutput(0).getSpentBy().getParentTransaction());
// Receive a block that overrides it. // Receive a block that overrides it.
sendMoneyToWallet(send2, AbstractBlockChain.NewBlockType.BEST_CHAIN); sendMoneyToWallet(send2, AbstractBlockChain.NewBlockType.BEST_CHAIN);
assertEquals(send1, eventDead[0]); assertEquals(send1, eventDead[0]);
assertEquals(send2, eventReplacement[0]); assertEquals(send2, eventReplacement[0]);
assertEquals(TransactionConfidence.ConfidenceType.DEAD, assertEquals(TransactionConfidence.ConfidenceType.DEAD,
send1.getConfidence().getConfidenceType()); send1.getConfidence().getConfidenceType());
assertEquals(send2, received.getOutput(0).getSpentBy().getParentTransaction());
TestUtils.DoubleSpends doubleSpends = TestUtils.createFakeDoubleSpendTxns(params, myAddress); TestUtils.DoubleSpends doubleSpends = TestUtils.createFakeDoubleSpendTxns(params, myAddress);
// t1 spends to our wallet. t2 double spends somewhere else. // t1 spends to our wallet. t2 double spends somewhere else.