From 46b2704f7908e27e361fed4db7bc6ed46d7d48d8 Mon Sep 17 00:00:00 2001 From: Sean Gilligan Date: Sun, 13 Aug 2023 13:05:08 -0700 Subject: [PATCH] BlockFileLoader: correct `Iterable` implementation To be iterable, `iterator()` must be able to be called twice. This is a breaking change, as users will no longer be able to use `BlockFileLoader` directly as an `Iterator` and must call `.iterator()` first. See the change in `BitcoindComparisonTool`. Also adds tests. --- .../org/bitcoinj/utils/BlockFileLoader.java | 173 +++++++++--------- .../bitcoinj/core/BitcoindComparisonTool.java | 2 +- .../bitcoinj/utils/BlockFileLoaderTest.java | 70 +++++++ 3 files changed, 162 insertions(+), 83 deletions(-) create mode 100644 core/src/test/java/org/bitcoinj/utils/BlockFileLoaderTest.java diff --git a/core/src/main/java/org/bitcoinj/utils/BlockFileLoader.java b/core/src/main/java/org/bitcoinj/utils/BlockFileLoader.java index efc61dcf9..9bb255061 100644 --- a/core/src/main/java/org/bitcoinj/utils/BlockFileLoader.java +++ b/core/src/main/java/org/bitcoinj/utils/BlockFileLoader.java @@ -41,7 +41,7 @@ import static org.bitcoinj.base.internal.Preconditions.checkArgument; * blocks together. Importing block data with this tool can be a lot faster than syncing over the network, if you * have the files available.

* - *

In order to comply with {@link Iterator}, this class swallows a lot of {@link IOException}s, which may result in a few + *

In order to comply with {@link Iterable}, {@link BlockIterator} swallows a lot of {@link IOException}s, which may result in a few * blocks being missed followed by a huge set of orphan blocks.

* *

To blindly import all files which can be found in Bitcoin Core (version 0.8 or higher) datadir automatically, @@ -53,7 +53,7 @@ import static org.bitcoinj.base.internal.Preconditions.checkArgument; * } * }

*/ -public class BlockFileLoader implements Iterable, Iterator { +public class BlockFileLoader implements Iterable { /** * Gets the list of files which contain blocks from Bitcoin Core. */ @@ -81,10 +81,7 @@ public class BlockFileLoader implements Iterable, Iterator { return defaultBlocksDir; } - private final Iterator fileIt; - private File file = null; - private FileInputStream currentFileStream = null; - private Block nextBlock = null; + private final List files; private final long packetMagic; private final MessageSerializer serializer; @@ -93,7 +90,7 @@ public class BlockFileLoader implements Iterable, Iterator { } public BlockFileLoader(Network network, List files) { - fileIt = files.iterator(); + this.files = files; NetworkParameters params = NetworkParameters.of(network); packetMagic = params.getPacketMagic(); serializer = params.getDefaultSerializer(); @@ -106,101 +103,113 @@ public class BlockFileLoader implements Iterable, Iterator { @Deprecated public BlockFileLoader(NetworkParameters params, List files) { - fileIt = files.iterator(); + this.files = files; packetMagic = params.getPacketMagic(); serializer = params.getDefaultSerializer(); } - @Override - public boolean hasNext() { - if (nextBlock == null) - loadNextBlock(); - return nextBlock != null; - } - @Override - public Block next() throws NoSuchElementException { - if (!hasNext()) - throw new NoSuchElementException(); - Block next = nextBlock; - nextBlock = null; - return next; - } - - private void loadNextBlock() { - while (true) { - try { - if (!fileIt.hasNext() && (currentFileStream == null || currentFileStream.available() < 1)) - break; - } catch (IOException e) { - currentFileStream = null; - if (!fileIt.hasNext()) - break; - } + public class BlockIterator implements Iterator { + private final Iterator fileIt; + private File file = null; + private FileInputStream currentFileStream = null; + private Block nextBlock = null; + + public BlockIterator(List fileList) { + this.fileIt = fileList.iterator(); + } + + @Override + public boolean hasNext() { + if (nextBlock == null) + loadNextBlock(); + return nextBlock != null; + } + + @Override + public Block next() throws NoSuchElementException { + if (!hasNext()) + throw new NoSuchElementException(); + Block next = nextBlock; + nextBlock = null; + return next; + } + + private void loadNextBlock() { while (true) { try { - if (currentFileStream != null && currentFileStream.available() > 0) + if (!fileIt.hasNext() && (currentFileStream == null || currentFileStream.available() < 1)) break; - } catch (IOException e1) { + } catch (IOException e) { currentFileStream = null; + if (!fileIt.hasNext()) + break; } - if (!fileIt.hasNext()) { - nextBlock = null; - currentFileStream = null; - return; - } - file = fileIt.next(); - try { - currentFileStream = new FileInputStream(file); - } catch (FileNotFoundException e) { - currentFileStream = null; - } - } - try { - int nextChar = currentFileStream.read(); - while (nextChar != -1) { - if (nextChar != ((packetMagic >>> 24) & 0xff)) { - nextChar = currentFileStream.read(); - continue; + while (true) { + try { + if (currentFileStream != null && currentFileStream.available() > 0) + break; + } catch (IOException e1) { + currentFileStream = null; + } + if (!fileIt.hasNext()) { + nextBlock = null; + currentFileStream = null; + return; + } + file = fileIt.next(); + try { + currentFileStream = new FileInputStream(file); + } catch (FileNotFoundException e) { + currentFileStream = null; } - nextChar = currentFileStream.read(); - if (nextChar != ((packetMagic >>> 16) & 0xff)) - continue; - nextChar = currentFileStream.read(); - if (nextChar != ((packetMagic >>> 8) & 0xff)) - continue; - nextChar = currentFileStream.read(); - if (nextChar == (packetMagic & 0xff)) - break; } - byte[] bytes = new byte[4]; - currentFileStream.read(bytes, 0, 4); - long size = ByteUtils.readUint32(bytes, 0); - bytes = new byte[(int) size]; - currentFileStream.read(bytes, 0, (int) size); try { - nextBlock = serializer.makeBlock(ByteBuffer.wrap(bytes)); - } catch (ProtocolException e) { - nextBlock = null; + int nextChar = currentFileStream.read(); + while (nextChar != -1) { + if (nextChar != ((packetMagic >>> 24) & 0xff)) { + nextChar = currentFileStream.read(); + continue; + } + nextChar = currentFileStream.read(); + if (nextChar != ((packetMagic >>> 16) & 0xff)) + continue; + nextChar = currentFileStream.read(); + if (nextChar != ((packetMagic >>> 8) & 0xff)) + continue; + nextChar = currentFileStream.read(); + if (nextChar == (packetMagic & 0xff)) + break; + } + byte[] bytes = new byte[4]; + currentFileStream.read(bytes, 0, 4); + long size = ByteUtils.readUint32(bytes, 0); + bytes = new byte[(int) size]; + currentFileStream.read(bytes, 0, (int) size); + try { + nextBlock = serializer.makeBlock(ByteBuffer.wrap(bytes)); + } catch (ProtocolException e) { + nextBlock = null; + continue; + } catch (Exception e) { + throw new RuntimeException("unexpected problem with block in " + file, e); + } + break; + } catch (IOException e) { + currentFileStream = null; continue; - } catch (Exception e) { - throw new RuntimeException("unexpected problem with block in " + file, e); } - break; - } catch (IOException e) { - currentFileStream = null; - continue; } } + + @Override + public void remove() throws UnsupportedOperationException { + throw new UnsupportedOperationException(); + } } - @Override - public void remove() throws UnsupportedOperationException { - throw new UnsupportedOperationException(); - } - @Override public Iterator iterator() { - return this; + return new BlockIterator(files); } } diff --git a/core/src/test/java/org/bitcoinj/core/BitcoindComparisonTool.java b/core/src/test/java/org/bitcoinj/core/BitcoindComparisonTool.java index 70ba905ae..a34b33173 100644 --- a/core/src/test/java/org/bitcoinj/core/BitcoindComparisonTool.java +++ b/core/src/test/java/org/bitcoinj/core/BitcoindComparisonTool.java @@ -78,7 +78,7 @@ public class BitcoindComparisonTool { FullBlockTestGenerator generator = new FullBlockTestGenerator(PARAMS); final RuleList blockList = generator.getBlocksToTest(false, runExpensiveTests, blockFile); final Map preloadedBlocks = new HashMap<>(); - final Iterator blocks = new BlockFileLoader(PARAMS.network(), Arrays.asList(blockFile)); + final Iterator blocks = new BlockFileLoader(PARAMS.network(), Arrays.asList(blockFile)).iterator(); try { FullPrunedBlockStore store = new MemoryFullPrunedBlockStore(PARAMS, blockList.maximumReorgBlockCount); diff --git a/core/src/test/java/org/bitcoinj/utils/BlockFileLoaderTest.java b/core/src/test/java/org/bitcoinj/utils/BlockFileLoaderTest.java new file mode 100644 index 000000000..845407259 --- /dev/null +++ b/core/src/test/java/org/bitcoinj/utils/BlockFileLoaderTest.java @@ -0,0 +1,70 @@ +/* + * Copyright by the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.bitcoinj.utils; + +import org.bitcoinj.base.BitcoinNetwork; +import org.bitcoinj.core.Block; +import org.bitcoinj.core.Context; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.util.Collection; +import java.util.Collections; +import java.util.Objects; + +import static org.junit.Assert.assertEquals; + +public class BlockFileLoaderTest { + @Before + public void setUp() throws Exception { + Context.propagate(new Context()); + } + + @Test + public void iterateFirst100kCount() { + File blockFile = new File(getClass().getResource("../core/first-100k-blocks.dat").getFile()); + BlockFileLoader loader = new BlockFileLoader(BitcoinNetwork.MAINNET, Collections.singletonList(blockFile)); + + long blockCount = 0; + for (Block b : loader) { + blockCount++; + } + + assertEquals(439, blockCount); + } + + @Test + public void iterateFirst100kTwice() { + File blockFile = new File(getClass().getResource("../core/first-100k-blocks.dat").getFile()); + BlockFileLoader loader = new BlockFileLoader(BitcoinNetwork.MAINNET, Collections.singletonList(blockFile)); + + long blockCount = 0; + for (Block b : loader) { + blockCount++; + } + + assertEquals(439, blockCount); + + long blockCount2 = 0; + for (Block b : loader) { + blockCount2++; + } + + assertEquals(439, blockCount2); + } +}