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

Wallet: improve double spend handling.

Now connects all inputs of an overriding transaction, meaning balance is correct if a bit-tweaked but semantically identical transaction double spends its shadow (e.g. during key rotation on a cloned device).

Still does not recursively kill transactions however.

Resolves issue 439.
This commit is contained in:
Mike Hearn 2013-11-18 00:16:48 +01:00
parent e49255c9e0
commit c11456c9f4
2 changed files with 67 additions and 52 deletions

View File

@ -728,6 +728,7 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi
outpoints.add(input.getOutpoint()); outpoints.add(input.getOutpoint());
} }
// 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.
LinkedList<Transaction> doubleSpentTxns = Lists.newLinkedList();
for (Transaction p : pending.values()) { for (Transaction p : pending.values()) {
for (TransactionInput input : p.getInputs()) { for (TransactionInput input : p.getInputs()) {
// This relies on the fact that TransactionOutPoint equality is defined at the protocol not object // This relies on the fact that TransactionOutPoint equality is defined at the protocol not object
@ -735,19 +736,15 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi
TransactionOutPoint outpoint = input.getOutpoint(); TransactionOutPoint outpoint = input.getOutpoint();
if (outpoints.contains(outpoint)) { 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.
if (takeAction) { if (!doubleSpentTxns.isEmpty() && doubleSpentTxns.getLast() == p) continue;
// Look for the actual input object in tx that is double spending. doubleSpentTxns.add(p);
TransactionInput overridingInput = null;
for (TransactionInput txInput : tx.getInputs()) {
if (txInput.getOutpoint().equals(outpoint)) overridingInput = txInput;
}
killTx(tx, checkNotNull(overridingInput), p);
}
return true;
} }
} }
} }
return false; if (takeAction) {
killTx(tx, doubleSpentTxns);
}
return !doubleSpentTxns.isEmpty();
} }
/** /**
@ -1082,50 +1079,47 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi
} }
} }
// Updates the wallet when a double spend occurs. private void killCoinbase(Transaction coinbase) {
private void killTx(Transaction overridingTx, TransactionInput overridingInput, Transaction killedTx) { log.warn("Coinbase killed by re-org: {}", coinbase.getHashAsString());
final Sha256Hash killedTxHash = killedTx.getHash(); coinbase.getConfidence().setOverridingTransaction(null);
if (overridingTx == null) { confidenceChanged.put(coinbase, TransactionConfidence.Listener.ChangeReason.TYPE);
// killedTx depended on a transaction that died because it was double spent or a coinbase that got re-orgd. final Sha256Hash hash = coinbase.getHash();
killedTx.getConfidence().setOverridingTransaction(null); pending.remove(hash);
confidenceChanged.put(killedTx, TransactionConfidence.Listener.ChangeReason.TYPE); unspent.remove(hash);
pending.remove(killedTxHash); spent.remove(hash);
unspent.remove(killedTxHash); addWalletTransaction(Pool.DEAD, coinbase);
spent.remove(killedTxHash); // TODO: Properly handle the recursive nature of killing transactions here.
addWalletTransaction(Pool.DEAD, killedTx); }
// TODO: Properly handle the recursive nature of killing transactions here.
return; // Updates the wallet when a double spend occurs. overridingTx/overridingInput can be null for the case of coinbases
private void killTx(Transaction overridingTx, List<Transaction> killedTx) {
for (Transaction tx : killedTx) {
log.warn("Saw double spend from chain override pending tx {}", tx.getHashAsString());
log.warn(" <-pending ->dead killed by {}", overridingTx.getHashAsString());
log.warn("Disconnecting each input and moving connected transactions.");
pending.remove(tx.getHash());
addWalletTransaction(Pool.DEAD, tx);
for (TransactionInput deadInput : tx.getInputs()) {
Transaction connected = deadInput.getOutpoint().fromTx;
if (connected == null) continue;
deadInput.disconnect();
maybeMovePool(connected, "kill");
}
tx.getConfidence().setOverridingTransaction(overridingTx);
confidenceChanged.put(tx, TransactionConfidence.Listener.ChangeReason.TYPE);
} }
TransactionOutPoint overriddenOutPoint = overridingInput.getOutpoint(); log.warn("Now attempting to connect the inputs of the overriding transaction.");
// It is expected that we may not have the overridden/double-spent tx in our wallet ... in the (common?!) case for (TransactionInput input : overridingTx.getInputs()) {
// where somebody is stealing money from us, the overriden tx belongs to someone else. TransactionInput.ConnectionResult result = input.connect(unspent, TransactionInput.ConnectMode.DISCONNECT_ON_CONFLICT);
log.warn("Saw double spend of {} from chain override pending tx {}",
overriddenOutPoint, killedTx.getHashAsString());
log.warn(" <-pending ->dead killed by {}", overridingTx.getHashAsString());
pending.remove(killedTxHash);
addWalletTransaction(Pool.DEAD, killedTx);
log.info("Disconnecting inputs of the newly dead tx");
for (TransactionInput deadInput : killedTx.getInputs()) {
Transaction connected = deadInput.getOutpoint().fromTx;
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) { if (result == TransactionInput.ConnectionResult.SUCCESS) {
maybeMovePool(overridingInput.getOutpoint().fromTx, "kill"); maybeMovePool(input.getOutpoint().fromTx, "kill");
} else {
result = input.connect(spent, TransactionInput.ConnectMode.DISCONNECT_ON_CONFLICT);
if (result == TransactionInput.ConnectionResult.SUCCESS) {
maybeMovePool(input.getOutpoint().fromTx, "kill");
}
} }
} }
killedTx.getConfidence().setOverridingTransaction(overridingTx);
confidenceChanged.put(killedTx, TransactionConfidence.Listener.ChangeReason.TYPE);
// TODO: Recursively kill other transactions that were double spent. // TODO: Recursively kill other transactions that were double spent.
} }
@ -2308,7 +2302,6 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi
Transaction tx = pair.tx; Transaction tx = pair.tx;
final Sha256Hash txHash = tx.getHash(); final Sha256Hash txHash = tx.getHash();
if (tx.isCoinBase()) { if (tx.isCoinBase()) {
log.warn("Coinbase tx {} -> dead", tx.getHash());
// All the transactions that we have in our wallet which spent this coinbase are now invalid // All the transactions that we have in our wallet which spent this coinbase are now invalid
// and will never confirm. Hopefully this should never happen - that's the point of the maturity // and will never confirm. Hopefully this should never happen - that's the point of the maturity
// rule that forbids spending of coinbase transactions for 100 blocks. // rule that forbids spending of coinbase transactions for 100 blocks.
@ -2319,7 +2312,7 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi
// can do our best. // can do our best.
// //
// TODO: Is it better to try and sometimes fail, or not try at all? // TODO: Is it better to try and sometimes fail, or not try at all?
killTx(null, null, tx); killCoinbase(tx);
} else { } else {
for (TransactionOutput output : tx.getOutputs()) { for (TransactionOutput output : tx.getOutputs()) {
TransactionInput input = output.getSpentBy(); TransactionInput input = output.getSpentBy();

View File

@ -54,6 +54,7 @@ import java.util.concurrent.atomic.AtomicInteger;
import static com.google.bitcoin.utils.TestUtils.*; import static com.google.bitcoin.utils.TestUtils.*;
import static com.google.bitcoin.core.Utils.*; import static com.google.bitcoin.core.Utils.*;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.junit.Assert.*; import static org.junit.Assert.*;
public class WalletTest extends TestWithWallet { public class WalletTest extends TestWithWallet {
@ -562,6 +563,27 @@ public class WalletTest extends TestWithWallet {
assertTrue(wallet.isConsistent()); assertTrue(wallet.isConsistent());
} }
@Test
public void doubleSpendIdenticalTx() throws Exception {
// Test the case where two semantically identical but bitwise different transactions double spend each other.
// This can (and has!) happened when a wallet is cloned between devices, and both devices decide to make the
// same spend simultaneously - for example due a re-keying operation.
final BigInteger value = Utils.toNanoCoins(1, 0);
// Give us two outputs.
sendMoneyToWallet(value, AbstractBlockChain.NewBlockType.BEST_CHAIN);
sendMoneyToWallet(value, AbstractBlockChain.NewBlockType.BEST_CHAIN);
final BigInteger value2 = Utils.toNanoCoins(2, 0);
// The two transactions will have different hashes due to the lack of deterministic signing, but will be
// otherwise identical. Once deterministic signatures are implemented, this test will have to be tweaked.
Transaction send1 = checkNotNull(wallet.createSend(new ECKey().toAddress(params), value2));
Transaction send2 = checkNotNull(wallet.createSend(new ECKey().toAddress(params), value2));
send1 = roundTripTransaction(params, send1);
wallet.commitTx(send2);
assertEquals(BigInteger.ZERO, wallet.getBalance());
sendMoneyToWallet(send1, AbstractBlockChain.NewBlockType.BEST_CHAIN);
assertEquals(BigInteger.ZERO, wallet.getBalance());
}
@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