novitski / bitcoinj

Automatically exported from code.google.com/p/bitcoinj
Apache License 2.0
0 stars 0 forks source link

Peer is added to Confidence.broadcastBy just for sending the tx #339

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Currently, if I am sending a payment, the peer the tx is being sent to is 
already counted into Confidence.broadcastBy. Just for sending. I think this is 
an unfortunate decision. It means that you need to handle own tx differently 
from incoming. For example in DefaultCoinSelector.isSelectable() there is this 
magical "if (confidence.numBroadcastPeers() <= 1)". Why 1? 0 would be more 
intuitive. For incoming payments you indeed need to check against 0.

My proposal is to only count incoming tx in broadcastBy. If we really need to 
track which peers we broadcasted *to*, I propose to add a separate set of peer 
addresses. (But probably we don't need that for now.)

If at any point in future the P2P protocol will acknowledge transactions (in 
the sense that they are valid & checked against double spend + fee), I agree 
that we can count this as a broadcastBy. But not before.

The case of only one connected peer needs special consideration. But there is 
already special handling for this case in PeerGroup.broadcastTransaction.

I'd say that if we agree to change this it should be soon. Clients already 
start adapting to the magic number 1.

Original issue reported on code.google.com by andreas....@gmail.com on 7 Mar 2013 at 1:40

GoogleCodeExporter commented 9 years ago
I don't see what the issue is. What do you mean by, "for incoming payments you 
indeed need to check against zero"? Unconfirmed inbound payments won't be 
considered for coin selection anyway.

I agree the name is a bit misleading, but the goal is to count "how many nodes 
have seen this transaction", so having it be < num connected peers after an 
initial announcement would be odd.

Original comment by hearn@google.com on 7 Mar 2013 at 2:12

GoogleCodeExporter commented 9 years ago
This was fixed in 0.10.3

Original comment by hearn@google.com on 14 Dec 2013 at 10:55