PeerGroup: Make private walletCoinsReceivedEventListener also take care of P2WPKH outputs.

P2WPKH outputs are a similar case as P2PK outputs in that inputs that spend them
cannot be matched by a bloom filter: their scriptSig is empty and their witness
(which would contain a matching pubkey) is not tested for a match. This is a
shortcoming of the SegWit spec.

Hopefully this fixes https://github.com/bitcoinj/bitcoinj/issues/1690
This commit is contained in:
Andreas Schildbach 2019-10-01 21:37:26 +02:00
parent 7f97c8afa6
commit 2288c2150f
2 changed files with 17 additions and 12 deletions

View file

@ -176,34 +176,37 @@ public class PeerGroup implements TransactionBroadcaster {
@Override
public void onCoinsReceived(Wallet wallet, Transaction tx, Coin prevBalance, Coin newBalance) {
// We received a relevant transaction. We MAY need to recalculate and resend the Bloom filter, but only
// if we have received a transaction that includes a relevant P2PK output.
// if we have received a transaction that includes a relevant P2PK or P2WPKH output.
//
// The reason is that P2PK outputs, when spent, will not repeat any data we can predict in their
// The reason is that P2PK and P2WPKH outputs, when spent, will not repeat any data we can predict in their
// inputs. So a remote peer will update the Bloom filter for us when such an output is seen matching the
// existing filter, so that it includes the tx hash in which the P2PK output was observed. Thus
// existing filter, so that it includes the tx hash in which the P2PK/P2WPKH output was observed. Thus
// the spending transaction will always match (due to the outpoint structure).
//
// Unfortunately, whilst this is required for correct sync of the chain in blocks, there are two edge cases.
//
// (1) If a wallet receives a relevant, confirmed P2PK output that was not broadcast across the network,
// (1) If a wallet receives a relevant, confirmed P2PK/P2WPKH output that was not broadcast across the network,
// for example in a coinbase transaction, then the node that's serving us the chain will update its filter
// but the rest will not. If another transaction then spends it, the other nodes won't match/relay it.
//
// (2) If we receive a P2PK output broadcast across the network, all currently connected nodes will see
// (2) If we receive a P2PK/P2WPKH output broadcast across the network, all currently connected nodes will see
// it and update their filter themselves, but any newly connected nodes will receive the last filter we
// calculated, which would not include this transaction.
//
// For this reason we check if the transaction contained any relevant P2PKs and force a recalc
// For this reason we check if the transaction contained any relevant P2PKs or P2WPKHs and force a recalc
// and possibly retransmit if so. The recalculation process will end up including the tx hash into the
// filter. In case (1), we need to retransmit the filter to the connected peers. In case (2), we don't
// and shouldn't, we should just recalculate and cache the new filter for next time.
for (TransactionOutput output : tx.getOutputs()) {
if (ScriptPattern.isP2PK(output.getScriptPubKey()) && output.isMine(wallet)) {
if (tx.getConfidence().getConfidenceType() == TransactionConfidence.ConfidenceType.BUILDING)
recalculateFastCatchupAndFilter(FilterRecalculateMode.SEND_IF_CHANGED);
else
recalculateFastCatchupAndFilter(FilterRecalculateMode.DONT_SEND);
return;
Script scriptPubKey = output.getScriptPubKey();
if (ScriptPattern.isP2PK(scriptPubKey) || ScriptPattern.isP2WPKH(scriptPubKey)) {
if (output.isMine(wallet)) {
if (tx.getConfidence().getConfidenceType() == TransactionConfidence.ConfidenceType.BUILDING)
recalculateFastCatchupAndFilter(FilterRecalculateMode.SEND_IF_CHANGED);
else
recalculateFastCatchupAndFilter(FilterRecalculateMode.DONT_SEND);
return;
}
}
}
}

View file

@ -622,6 +622,7 @@ public class PeerGroupTest extends TestWithPeerGroup {
tx2.addInput(tx.getOutput(0));
TransactionOutPoint outpoint = tx2.getInput(0).getOutpoint();
assertTrue(p1.lastReceivedFilter.contains(key.getPubKey()));
assertTrue(p1.lastReceivedFilter.contains(key.getPubKeyHash()));
assertFalse(p1.lastReceivedFilter.contains(tx.getTxId().getBytes()));
inbound(p1, tx);
// p1 requests dep resolution, p2 is quiet.
@ -635,6 +636,7 @@ public class PeerGroupTest extends TestWithPeerGroup {
// Now we connect p3 and there is a new bloom filter sent, that DOES match the relevant outpoint.
InboundMessageQueuer p3 = connectPeer(3);
assertTrue(p3.lastReceivedFilter.contains(key.getPubKey()));
assertTrue(p3.lastReceivedFilter.contains(key.getPubKeyHash()));
assertTrue(p3.lastReceivedFilter.contains(outpoint.unsafeBitcoinSerialize()));
}