mirror of
https://github.com/Qortal/altcoinj.git
synced 2025-01-31 15:22:16 +00:00
Refactor/bugfix broadcast of pending transactions when a peergroup starts up.
Previously the PeerGroup itself would broadcast the pending transactions by simply sending an inv with them all to every peer. This is a good way to get a transaction blasted out if there are no problems with it, but it means we cannot track propagation and the numBroadcastPeers() value was correspondingly not increased. This seems to be causing issues with the Android wallet. So try out a different approach - have the wallet use broadcastTransaction as per normal on the PeerGroup when it's added. The TX will be propagated and watched as with a normal spend.
This commit is contained in:
parent
2e924e217d
commit
f27430c356
@ -624,9 +624,6 @@ public class PeerGroup extends AbstractIdleService implements TransactionBroadca
|
||||
checkState(!wallets.contains(wallet));
|
||||
wallets.add(wallet);
|
||||
wallet.setTransactionBroadcaster(this);
|
||||
// TODO: Make wallets announce their own pending transactions.
|
||||
// The only reason it's not done that way now is to try and reduce late, risky changes before 0.10
|
||||
announcePendingWalletTransactions(Collections.singletonList(wallet), peers);
|
||||
wallet.addEventListener(walletEventListener); // TODO: Run this in the current peer thread.
|
||||
addPeerFilterProvider(wallet);
|
||||
} finally {
|
||||
@ -666,6 +663,7 @@ public class PeerGroup extends AbstractIdleService implements TransactionBroadca
|
||||
wallets.remove(checkNotNull(wallet));
|
||||
peerFilterProviders.remove(wallet);
|
||||
wallet.removeEventListener(walletEventListener);
|
||||
wallet.setTransactionBroadcaster(null);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -869,16 +867,6 @@ public class PeerGroup extends AbstractIdleService implements TransactionBroadca
|
||||
}
|
||||
// Make sure the peer knows how to upload transactions that are requested from us.
|
||||
peer.addEventListener(getDataListener, Threading.SAME_THREAD);
|
||||
// Now tell the peers about any transactions we have which didn't appear in the chain yet. These are not
|
||||
// necessarily spends we created. They may also be transactions broadcast across the network that we saw,
|
||||
// which are relevant to us, and which we therefore wish to help propagate (ie they send us coins).
|
||||
//
|
||||
// Note that this can cause a DoS attack against us if a malicious remote peer knows what keys we own, and
|
||||
// then sends us fake relevant transactions. We'll attempt to relay the bad transactions, our badness score
|
||||
// in the Satoshi client will increase and we'll get disconnected.
|
||||
//
|
||||
// TODO: Find a way to balance the desire to propagate useful transactions against DoS attacks.
|
||||
announcePendingWalletTransactions(wallets, Collections.singletonList(peer));
|
||||
// And set up event listeners for clients. This will allow them to find out about new transactions and blocks.
|
||||
for (ListenerRegistration<PeerEventListener> registration : peerEventListeners) {
|
||||
peer.addEventListener(registration.listener, registration.executor);
|
||||
@ -951,30 +939,6 @@ public class PeerGroup extends AbstractIdleService implements TransactionBroadca
|
||||
pingRunnable[0].run();
|
||||
}
|
||||
|
||||
/** Returns true if at least one peer received an inv. */
|
||||
private boolean announcePendingWalletTransactions(List<Wallet> announceWallets,
|
||||
List<Peer> announceToPeers) {
|
||||
checkState(lock.isHeldByCurrentThread());
|
||||
// Build up an inv announcing the hashes of all pending transactions in all our wallets.
|
||||
InventoryMessage inv = new InventoryMessage(params);
|
||||
for (Wallet w : announceWallets) {
|
||||
for (Transaction tx : w.getPendingTransactions()) {
|
||||
inv.addTransaction(tx);
|
||||
}
|
||||
}
|
||||
// Don't send empty inv messages.
|
||||
if (inv.getItems().size() == 0) {
|
||||
return true;
|
||||
}
|
||||
boolean success = false;
|
||||
for (Peer p : announceToPeers) {
|
||||
log.info("{}: Announcing {} pending wallet transactions", p.getAddress(), inv.getItems().size());
|
||||
p.sendMessage(inv);
|
||||
success = true;
|
||||
}
|
||||
return success;
|
||||
}
|
||||
|
||||
private void setDownloadPeer(Peer peer) {
|
||||
lock.lock();
|
||||
try {
|
||||
|
@ -3275,7 +3275,24 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi
|
||||
* optimise itself to reduce fees or improve privacy.</p>
|
||||
*/
|
||||
public void setTransactionBroadcaster(@Nullable com.google.bitcoin.core.TransactionBroadcaster broadcaster) {
|
||||
lock.lock();
|
||||
try {
|
||||
if (vTransactionBroadcaster == broadcaster)
|
||||
return;
|
||||
vTransactionBroadcaster = broadcaster;
|
||||
if (broadcaster == null)
|
||||
return;
|
||||
// Now use it to upload any pending transactions we have that are marked as not being seen by any peers yet.
|
||||
for (Transaction tx : pending.values()) {
|
||||
checkState(tx.getConfidence().getConfidenceType() == ConfidenceType.PENDING);
|
||||
if (tx.getConfidence().numBroadcastPeers() == 0) {
|
||||
log.info("New broadcaster so uploading waiting tx {}", tx.getHash());
|
||||
broadcaster.broadcastTransaction(tx);
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
lock.unlock();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -33,6 +33,7 @@ import java.util.Set;
|
||||
import java.util.concurrent.Semaphore;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import static com.google.common.base.Preconditions.checkNotNull;
|
||||
import static org.junit.Assert.*;
|
||||
|
||||
public class PeerGroupTest extends TestWithPeerGroup {
|
||||
@ -303,7 +304,7 @@ public class PeerGroupTest extends TestWithPeerGroup {
|
||||
Threading.waitForUserCode();
|
||||
assertTrue(sendResult.broadcastComplete.isDone());
|
||||
assertEquals(transactions[0], sendResult.tx);
|
||||
assertEquals(transactions[0].getConfidence().numBroadcastPeers(), 2);
|
||||
assertEquals(2, transactions[0].getConfidence().numBroadcastPeers());
|
||||
// Confirm it.
|
||||
Block b2 = TestUtils.createFakeBlock(blockStore, t1).block;
|
||||
inbound(p1, b2);
|
||||
@ -313,24 +314,12 @@ public class PeerGroupTest extends TestWithPeerGroup {
|
||||
peerGroup.removeWallet(wallet);
|
||||
Wallet.SendRequest req = Wallet.SendRequest.to(dest, Utils.toNanoCoins(2, 0));
|
||||
req.ensureMinRequiredFee = false;
|
||||
Transaction t3 = wallet.sendCoinsOffline(req);
|
||||
Transaction t3 = checkNotNull(wallet.sendCoinsOffline(req));
|
||||
assertNull(outbound(p1)); // Nothing sent.
|
||||
// Add the wallet to the peer group (simulate initialization). Transactions should be announced.
|
||||
peerGroup.addWallet(wallet);
|
||||
// Transaction announced to the first peer.
|
||||
InventoryMessage inv1 = (InventoryMessage) outbound(p1);
|
||||
// Filter is still the same as it was, so it is not rebroadcast
|
||||
assertEquals(t3.getHash(), inv1.getItems().get(0).hash);
|
||||
// Peer asks for the transaction, and get it.
|
||||
GetDataMessage getdata = new GetDataMessage(params);
|
||||
getdata.addItem(inv1.getItems().get(0));
|
||||
inbound(p1, getdata);
|
||||
Transaction t4 = (Transaction) outbound(p1);
|
||||
assertEquals(t3, t4);
|
||||
|
||||
FakeChannel p3 = connectPeer(3);
|
||||
assertTrue(outbound(p3) instanceof InventoryMessage);
|
||||
control.verify();
|
||||
assertEquals(t3.getHash(), ((Transaction) outbound(p1)).getHash());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -69,6 +69,7 @@ public class WalletTool {
|
||||
" --force Overrides any safety checks on the requested action.\n" +
|
||||
" --date Provide a date in form YYYY/MM/DD to any action that requires one.\n" +
|
||||
" --peers=1.2.3.4 Comma separated IP addresses/domain names for connections instead of peer discovery.\n" +
|
||||
" --offline If specified when sending, don't try and connect, just write the tx to the wallet.\n" +
|
||||
" --condition=... Allows you to specify a numeric condition for other commands. The format is\n" +
|
||||
" one of the following operators = < > <= >= immediately followed by a number.\n" +
|
||||
" For example --condition=\">5.10\" or --condition=\"<=1\"\n" +
|
||||
@ -259,6 +260,7 @@ public class WalletTool {
|
||||
conditionFlag = parser.accepts("condition").withRequiredArg();
|
||||
parser.accepts("locktime").withRequiredArg();
|
||||
parser.accepts("allow-unconfirmed");
|
||||
parser.accepts("offline");
|
||||
OptionSpec<String> passwordFlag = parser.accepts("password").withRequiredArg();
|
||||
options = parser.parse(args);
|
||||
|
||||
@ -397,7 +399,7 @@ public class WalletTool {
|
||||
shutdown();
|
||||
}
|
||||
|
||||
private static void send(List<String> outputs, BigInteger fee, String lockTimeStr, boolean allowUnconfirmed) {
|
||||
private static void send(List<String> outputs, BigInteger fee, String lockTimeStr, boolean allowUnconfirmed) throws VerificationException {
|
||||
try {
|
||||
// Convert the input strings to outputs.
|
||||
Transaction t = new Transaction(params);
|
||||
@ -467,6 +469,12 @@ public class WalletTool {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
t = req.tx; // Not strictly required today.
|
||||
System.out.println(t.getHashAsString());
|
||||
if (options.has("offline")) {
|
||||
wallet.commitTx(t);
|
||||
return;
|
||||
}
|
||||
|
||||
setup();
|
||||
peers.startAndWait();
|
||||
// Wait for peers to connect, the tx to be sent to one of them and for it to be propagated across the
|
||||
@ -478,7 +486,6 @@ public class WalletTool {
|
||||
// completes before the remote peer actually hears the message.
|
||||
Thread.sleep(5000);
|
||||
}
|
||||
System.out.println(t.getHashAsString());
|
||||
} catch (BlockStoreException e) {
|
||||
throw new RuntimeException(e);
|
||||
} catch (KeyCrypterException e) {
|
||||
|
Loading…
Reference in New Issue
Block a user