Skip to content

Commit

Permalink
Remove BlockChainListener.isTransactionRelevant.
Browse files Browse the repository at this point in the history
The optimisation this was meant to support ceased to be relevant a long time ago.
  • Loading branch information
mikehearn committed Jul 23, 2015
1 parent 43b89f4 commit 30d2da2
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 83 deletions.
40 changes: 5 additions & 35 deletions core/src/main/java/org/bitcoinj/core/AbstractBlockChain.java
Original file line number Diff line number Diff line change
Expand Up @@ -374,21 +374,13 @@ private boolean add(Block block, boolean tryConnecting,
return true;
}

// Does this block contain any transactions we might care about? Check this up front before verifying the
// blocks validity so we can skip the merkle root verification if the contents aren't interesting. This saves
// a lot of time for big blocks.
boolean contentsImportant = shouldVerifyTransactions();
if (block.transactions != null) {
contentsImportant = contentsImportant || containsRelevantTransactions(block);
}

// Prove the block is internally valid: hash is lower than target, etc. This only checks the block contents
// if there is a tx sending or receiving coins using an address in one of our wallets. And those transactions
// are only lightly verified: presence in a valid connecting block is taken as proof of validity. See the
// article here for more details: http://code.google.com/p/bitcoinj/wiki/SecurityModel
try {
block.verifyHeader();
if (contentsImportant)
if (shouldVerifyTransactions())
block.verifyTransactions();
} catch (VerificationException e) {
log.error("Failed to verify block: ", e);
Expand Down Expand Up @@ -765,12 +757,10 @@ private static void sendTransactionsToListener(StoredBlock block, NewBlockType b
Set<Sha256Hash> falsePositives) throws VerificationException {
for (Transaction tx : transactions) {
try {
if (listener.isTransactionRelevant(tx)) {
falsePositives.remove(tx.getHash());
if (clone)
tx = new Transaction(tx.params, tx.bitcoinSerialize());
listener.receiveFromBlock(tx, block, blockType, relativityOffset++);
}
falsePositives.remove(tx.getHash());
if (clone)
tx = new Transaction(tx.params, tx.bitcoinSerialize());
listener.receiveFromBlock(tx, block, blockType, relativityOffset++);
} catch (ScriptException e) {
// We don't want scripts we don't understand to break the block chain so just note that this tx was
// not scanned here and continue.
Expand Down Expand Up @@ -826,26 +816,6 @@ private void tryConnectingOrphans() throws VerificationException, BlockStoreExce
} while (blocksConnectedThisRound > 0);
}

/**
* Returns true if any connected wallet considers any transaction in the block to be relevant.
*/
private boolean containsRelevantTransactions(Block block) {
// Does not need to be locked.
for (Transaction tx : block.transactions) {
try {
for (final ListenerRegistration<BlockChainListener> registration : listeners) {
if (registration.executor != Threading.SAME_THREAD) continue;
if (registration.listener.isTransactionRelevant(tx)) return true;
}
} catch (ScriptException e) {
// We don't want scripts we don't understand to break the block chain so just note that this tx was
// not scanned here and continue.
log.warn("Failed to parse a script: " + e.toString());
}
}
return false;
}

/**
* Returns the block at the head of the current best chain. This is the block which represents the greatest
* amount of cumulative work done.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ public void notifyNewBestBlock(StoredBlock block) throws VerificationException {
public void reorganize(StoredBlock splitPoint, List<StoredBlock> oldBlocks, List<StoredBlock> newBlocks) throws VerificationException {
}

@Override
public boolean isTransactionRelevant(Transaction tx) throws ScriptException {
return false;
}

@Override
public void receiveFromBlock(Transaction tx, StoredBlock block, BlockChain.NewBlockType blockType,
int relativityOffset) throws VerificationException {
Expand Down
9 changes: 0 additions & 9 deletions core/src/main/java/org/bitcoinj/core/BlockChainListener.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,6 @@ public interface BlockChainListener {
void reorganize(StoredBlock splitPoint, List<StoredBlock> oldBlocks,
List<StoredBlock> newBlocks) throws VerificationException;

/**
* Returns true if the given transaction is interesting to the listener. If yes, then the transaction will
* be provided via the receiveFromBlock method. This method is essentially an optimization that lets BlockChain
* bypass verification of a blocks merkle tree if no listeners are interested, which can save time when processing
* full blocks on mobile phones. It's likely the method will be removed in future and replaced with an alternative
* mechanism that involves listeners providing all keys that are interesting.
*/
boolean isTransactionRelevant(Transaction tx) throws ScriptException;

/**
* <p>Called by the {@link BlockChain} when we receive a new block that contains a relevant transaction.</p>
*
Expand Down
4 changes: 3 additions & 1 deletion core/src/main/java/org/bitcoinj/core/Wallet.java
Original file line number Diff line number Diff line change
Expand Up @@ -1694,7 +1694,6 @@ public boolean isPendingTransactionRelevant(Transaction tx) throws ScriptExcepti
* <p>Note that if the tx has inputs containing one of our keys, but the connected transaction is not in the wallet,
* it will not be considered relevant.</p>
*/
@Override
public boolean isTransactionRelevant(Transaction tx) throws ScriptException {
lock.lock();
try {
Expand Down Expand Up @@ -1761,6 +1760,8 @@ public void receiveFromBlock(Transaction tx, StoredBlock block,
int relativityOffset) throws VerificationException {
lock.lock();
try {
if (!isTransactionRelevant(tx))
return;
receive(tx, block, blockType, relativityOffset);
} finally {
lock.unlock();
Expand All @@ -1771,6 +1772,7 @@ private void receive(Transaction tx, StoredBlock block, BlockChain.NewBlockType
int relativityOffset) throws VerificationException {
// Runs in a peer thread.
checkState(lock.isHeldByCurrentThread());

Coin prevBalance = getBalance();
Sha256Hash txHash = tx.getHash();
boolean bestChain = blockType == BlockChain.NewBlockType.BEST_CHAIN;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ public class NativeBlockChainListener implements BlockChainListener {
@Override
public native void reorganize(StoredBlock splitPoint, List<StoredBlock> oldBlocks, List<StoredBlock> newBlocks) throws VerificationException;

@Override
public native boolean isTransactionRelevant(Transaction tx) throws ScriptException;

@Override
public native void receiveFromBlock(Transaction tx, StoredBlock block, BlockChain.NewBlockType blockType,
int relativityOffset) throws VerificationException;
Expand Down
31 changes: 1 addition & 30 deletions core/src/test/java/org/bitcoinj/core/BlockChainTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public void receiveFromBlock(Transaction tx, StoredBlock block, BlockChain.NewBl
int relativityOffset) throws VerificationException {
super.receiveFromBlock(tx, block, blockType, relativityOffset);
BlockChainTest.this.block[0] = block;
if (tx.isCoinBase()) {
if (isTransactionRelevant(tx) && tx.isCoinBase()) {
BlockChainTest.this.coinbaseTransaction = tx;
}
}
Expand Down Expand Up @@ -135,35 +135,6 @@ public void receiveCoins() throws Exception {
assertTrue(wallet.getBalance().signum() > 0);
}

@Test
public void merkleRoots() throws Exception {
// Test that merkle root verification takes place when a relevant transaction is present and doesn't when
// there isn't any such tx present (as an optimization).
Transaction tx1 = createFakeTx(unitTestParams,
COIN,
wallet.currentReceiveKey().toAddress(unitTestParams));
Block b1 = createFakeBlock(blockStore, tx1).block;
chain.add(b1);
resetBlockStore();
Sha256Hash hash = b1.getMerkleRoot();
b1.setMerkleRoot(Sha256Hash.ZERO_HASH);
try {
chain.add(b1);
fail();
} catch (VerificationException e) {
// Expected.
b1.setMerkleRoot(hash);
}
// Now add a second block with no relevant transactions and then break it.
Transaction tx2 = createFakeTx(unitTestParams, COIN,
new ECKey().toAddress(unitTestParams));
Block b2 = createFakeBlock(blockStore, tx2).block;
b2.getMerkleRoot();
b2.setMerkleRoot(Sha256Hash.ZERO_HASH);
b2.solve();
chain.add(b2); // Broken block is accepted because its contents don't matter to us.
}

@Test
public void unconnectedBlocks() throws Exception {
Block b1 = unitTestParams.getGenesisBlock().createNextBlock(coinbaseTo);
Expand Down
1 change: 1 addition & 0 deletions core/src/test/java/org/bitcoinj/core/TransactionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ public void testTheTXByHeightComparator() {
assertEquals(iterator.hasNext(), false);
}


@Test(expected = ScriptException.class)
public void testAddSignedInputThrowsExceptionWhenScriptIsNotToRawPubKeyAndIsNotToAddress() {
ECKey key = new ECKey();
Expand Down

0 comments on commit 30d2da2

Please sign in to comment.