From 1a786a2d1aaf18a19f11032bde9a6b6382b1a18c Mon Sep 17 00:00:00 2001 From: Oscar Guindzberg Date: Wed, 16 Dec 2020 13:38:28 -0300 Subject: [PATCH] Update bloom filter if coins were sent Fixes https://github.com/bitcoinj/bitcoinj/issues/2070#issuecomment-744846418 --- .../java/org/bitcoinj/core/PeerGroup.java | 81 +++++++++++-------- 1 file changed, 48 insertions(+), 33 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/PeerGroup.java b/core/src/main/java/org/bitcoinj/core/PeerGroup.java index 1f711026260..8f533a7d3f5 100644 --- a/core/src/main/java/org/bitcoinj/core/PeerGroup.java +++ b/core/src/main/java/org/bitcoinj/core/PeerGroup.java @@ -33,6 +33,7 @@ import org.bitcoinj.wallet.listeners.KeyChainEventListener; import org.bitcoinj.wallet.listeners.ScriptsChangeEventListener; import org.bitcoinj.wallet.listeners.WalletCoinsReceivedEventListener; +import org.bitcoinj.wallet.listeners.WalletCoinsSentEventListener; import org.slf4j.*; import javax.annotation.*; @@ -188,42 +189,54 @@ public static void setIgnoreHttpSeeds(boolean ignoreHttpSeeds) { private final WalletCoinsReceivedEventListener walletCoinsReceivedEventListener = new WalletCoinsReceivedEventListener() { @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 or P2WPKH output. - // - // 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/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/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/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 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()) { - 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; - } + onCoinsReceivedOrSent(wallet, tx); + } + }; + + private final WalletCoinsSentEventListener walletCoinsSentEventListener = new WalletCoinsSentEventListener() { + @Override + public void onCoinsSent(Wallet wallet, Transaction tx, Coin prevBalance, Coin newBalance) { + onCoinsReceivedOrSent(wallet, tx); + } + }; + + + private void onCoinsReceivedOrSent(Wallet wallet, Transaction tx) { + // 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 or P2WPKH output. + // + // 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/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/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/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 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()) { + 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; } } } - }; + } // Exponential backoff for peers starts at 1 second and maxes at 10 minutes. private final ExponentialBackoff.Params peerBackoffParams = new ExponentialBackoff.Params(1000, 1.5f, 10 * 60 * 1000); @@ -1204,6 +1217,7 @@ public void addWallet(Wallet wallet) { wallets.add(wallet); wallet.setTransactionBroadcaster(this); wallet.addCoinsReceivedEventListener(Threading.SAME_THREAD, walletCoinsReceivedEventListener); + wallet.addCoinsSentEventListener(Threading.SAME_THREAD, walletCoinsSentEventListener); wallet.addKeyChainEventListener(Threading.SAME_THREAD, walletKeyEventListener); wallet.addScriptChangeEventListener(Threading.SAME_THREAD, walletScriptEventListener); addPeerFilterProvider(wallet); @@ -1276,6 +1290,7 @@ public void removeWallet(Wallet wallet) { wallets.remove(checkNotNull(wallet)); peerFilterProviders.remove(wallet); wallet.removeCoinsReceivedEventListener(walletCoinsReceivedEventListener); + wallet.removeCoinsSentEventListener(walletCoinsSentEventListener); wallet.removeKeyChainEventListener(walletKeyEventListener); wallet.removeScriptChangeEventListener(walletScriptEventListener); wallet.setTransactionBroadcaster(null);