Peer: fix the case where dependency download is disabled. It wasn't sending transactions to the wallet before. Add a test for this and add accessors for the setting.

This commit is contained in:
Mike Hearn 2014-09-08 18:24:44 +02:00
parent 302bb3e21d
commit 2b01508e10
2 changed files with 59 additions and 15 deletions

View File

@ -82,7 +82,7 @@ public class Peer extends PeerSocketHandler {
// equivalent and other things. // equivalent and other things.
private final VersionMessage versionMessage; private final VersionMessage versionMessage;
// Switch for enabling download of pending transaction dependencies. // Switch for enabling download of pending transaction dependencies.
private final boolean downloadTxDependencies; private volatile boolean vDownloadTxDependencies;
// How many block messages the peer has announced to us. Peers only announce blocks that attach to their best chain // How many block messages the peer has announced to us. Peers only announce blocks that attach to their best chain
// so we can use this to calculate the height of the peers chain, by adding it to the initial height in the version // so we can use this to calculate the height of the peers chain, by adding it to the initial height in the version
// message. This method can go wrong if the peer re-orgs onto a shorter (but harder) chain, however, this is rare. // message. This method can go wrong if the peer re-orgs onto a shorter (but harder) chain, however, this is rare.
@ -201,7 +201,7 @@ public class Peer extends PeerSocketHandler {
super(params, remoteAddress); super(params, remoteAddress);
this.params = Preconditions.checkNotNull(params); this.params = Preconditions.checkNotNull(params);
this.versionMessage = Preconditions.checkNotNull(ver); this.versionMessage = Preconditions.checkNotNull(ver);
this.downloadTxDependencies = downloadTxDependencies; this.vDownloadTxDependencies = downloadTxDependencies;
this.blockChain = chain; // Allowed to be null. this.blockChain = chain; // Allowed to be null.
this.vDownloadData = chain != null; this.vDownloadData = chain != null;
this.getDataFutures = new CopyOnWriteArrayList<GetDataRequest>(); this.getDataFutures = new CopyOnWriteArrayList<GetDataRequest>();
@ -605,16 +605,22 @@ public class Peer extends PeerSocketHandler {
for (final Wallet wallet : wallets) { for (final Wallet wallet : wallets) {
try { try {
if (wallet.isPendingTransactionRelevant(fTx)) { if (wallet.isPendingTransactionRelevant(fTx)) {
// This transaction seems interesting to us, so let's download its dependencies. This has several if (vDownloadTxDependencies) {
// purposes: we can check that the sender isn't attacking us by engaging in protocol abuse games, // This transaction seems interesting to us, so let's download its dependencies. This has
// like depending on a time-locked transaction that will never confirm, or building huge chains // several purposes: we can check that the sender isn't attacking us by engaging in protocol
// of unconfirmed transactions (again - so they don't confirm and the money can be taken // abuse games, like depending on a time-locked transaction that will never confirm, or
// back with a Finney attack). Knowing the dependencies also lets us store them in a serialized // building huge chains of unconfirmed transactions (again - so they don't confirm and the
// wallet so we always have enough data to re-announce to the network and get the payment into // money can be taken back with a Finney attack). Knowing the dependencies also lets us
// the chain, in case the sender goes away and the network starts to forget. // store them in a serialized wallet so we always have enough data to re-announce to the
// network and get the payment into the chain, in case the sender goes away and the network
// starts to forget.
//
// TODO: Not all the above things are implemented. // TODO: Not all the above things are implemented.
//
if (downloadTxDependencies) { // Note that downloading of dependencies can end up walking around 15 minutes back even
// through transactions that have confirmed, as getdata on the remote peer also checks
// relay memory not only the mempool. Unfortunately we have no way to know that here. In
// practice it should not matter much.
Futures.addCallback(downloadDependencies(fTx), new FutureCallback<List<Transaction>>() { Futures.addCallback(downloadDependencies(fTx), new FutureCallback<List<Transaction>>() {
@Override @Override
public void onSuccess(List<Transaction> dependencies) { public void onSuccess(List<Transaction> dependencies) {
@ -636,6 +642,8 @@ public class Peer extends PeerSocketHandler {
// Not much more we can do at this point. // Not much more we can do at this point.
} }
}); });
} else {
wallet.receivePending(fTx, null);
} }
} }
} catch (VerificationException e) { } catch (VerificationException e) {
@ -1556,4 +1564,22 @@ public class Peer extends PeerSocketHandler {
sendMessage(new GetUTXOsMessage(params, outPoints, true)); sendMessage(new GetUTXOsMessage(params, outPoints, true));
return utxosFuture; return utxosFuture;
} }
/**
* Returns true if this peer will use getdata/notfound messages to walk backwards through transaction dependencies
* before handing the transaction off to the wallet. The wallet can do risk analysis on pending/recent transactions
* to try and discover if a pending tx might be at risk of double spending.
*/
public boolean getDownloadTxDependencies() {
return vDownloadTxDependencies;
}
/**
* Sets if this peer will use getdata/notfound messages to walk backwards through transaction dependencies
* before handing the transaction off to the wallet. The wallet can do risk analysis on pending/recent transactions
* to try and discover if a pending tx might be at risk of double spending.
*/
public void setDownloadTxDependencies(boolean value) {
vDownloadTxDependencies = value;
}
} }

View File

@ -514,17 +514,35 @@ public class PeerTest extends TestWithNetworkConnections {
assertEquals(7250, peer.getPingTime()); assertEquals(7250, peer.getPingTime());
} }
@Test
public void recursiveDependencyDownloadDisabled() throws Exception {
peer.setDownloadTxDependencies(false);
connect();
// Check that if we request dependency download to be disabled and receive a relevant tx, things work correctly.
Transaction tx = FakeTxBuilder.createFakeTx(unitTestParams, COIN, address);
final Transaction[] result = new Transaction[1];
wallet.addEventListener(new AbstractWalletEventListener() {
@Override
public void onCoinsReceived(Wallet wallet, Transaction tx, Coin prevBalance, Coin newBalance) {
result[0] = tx;
}
});
inbound(writeTarget, tx);
pingAndWait(writeTarget);
assertEquals(tx, result[0]);
}
@Test @Test
public void recursiveDownloadNew() throws Exception { public void recursiveDownloadNew() throws Exception {
recursiveDownload(true); recursiveDependencyDownload(true);
} }
@Test @Test
public void recursiveDownloadOld() throws Exception { public void recursiveDownloadOld() throws Exception {
recursiveDownload(false); recursiveDependencyDownload(false);
} }
public void recursiveDownload(boolean useNotFound) throws Exception { public void recursiveDependencyDownload(boolean useNotFound) throws Exception {
// Using ping or notfound? // Using ping or notfound?
connectWithVersion(useNotFound ? 70001 : 60001); connectWithVersion(useNotFound ? 70001 : 60001);
// Check that we can download all dependencies of an unconfirmed relevant transaction from the mempool. // Check that we can download all dependencies of an unconfirmed relevant transaction from the mempool.