[NC-1561] Remove RLPUtils from RawBlockIterator (hyperledger#179)

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/util/RawBlockIterator.java

@@ -17,36 +17,45 @@
 import tech.pegasys.pantheon.ethereum.core.BlockHeader;
 import tech.pegasys.pantheon.ethereum.core.Transaction;
 import tech.pegasys.pantheon.ethereum.rlp.BytesValueRLPInput;
+import tech.pegasys.pantheon.ethereum.rlp.RLP;
 import tech.pegasys.pantheon.ethereum.rlp.RLPInput;
-import tech.pegasys.pantheon.ethereum.rlp.RlpUtils;
 import tech.pegasys.pantheon.util.bytes.BytesValue;
 
 import java.io.Closeable;
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
 import java.nio.file.Path;
-import java.util.Arrays;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
 import java.util.function.Function;
 
 public final class RawBlockIterator implements Iterator<Block>, Closeable {
+  private static final int DEFAULT_INIT_BUFFER_CAPACITY = 1 << 16;
 
   private final FileChannel fileChannel;
   private final Function<RLPInput, BlockHeader> headerReader;
 
-  private ByteBuffer readBuffer = ByteBuffer.allocate(2 << 15);
+  private ByteBuffer readBuffer;
 
   private Block next;
 
-  public RawBlockIterator(final Path file, final Function<RLPInput, BlockHeader> headerReader)
+  RawBlockIterator(
+      final Path file,
+      final Function<RLPInput, BlockHeader> headerReader,
+      final int initialCapacity)
       throws IOException {
     fileChannel = FileChannel.open(file);
     this.headerReader = headerReader;
+    readBuffer = ByteBuffer.allocate(initialCapacity);
     nextBlock();
   }
 
+  public RawBlockIterator(final Path file, final Function<RLPInput, BlockHeader> headerReader)
+      throws IOException {
+    this(file, headerReader, DEFAULT_INIT_BUFFER_CAPACITY);
+  }
+
   @Override
   public boolean hasNext() {
     return next != null;
@@ -75,7 +84,7 @@ private void nextBlock() throws IOException {
     fillReadBuffer();
     int initial = readBuffer.position();
     if (initial > 0) {
-      final int length = RlpUtils.decodeLength(readBuffer, 0);
+      final int length = RLP.calculateSize(BytesValue.wrapBuffer(readBuffer));
       if (length > readBuffer.capacity()) {
         readBuffer.flip();
         final ByteBuffer newBuffer = ByteBuffer.allocate(2 * length);
@@ -84,8 +93,9 @@ private void nextBlock() throws IOException {
         fillReadBuffer();
         initial = readBuffer.position();
       }
-      final RLPInput rlp =
-          new BytesValueRLPInput(BytesValue.wrap(Arrays.copyOf(readBuffer.array(), length)), false);
+
+      final BytesValue rlpBytes = BytesValue.wrapBuffer(readBuffer, 0, length).copy();
+      final RLPInput rlp = new BytesValueRLPInput(rlpBytes, false);
       rlp.enterList();
       final BlockHeader header = headerReader.apply(rlp);
       final BlockBody body =

ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/util/RawBlockIteratorTest.java

@@ -0,0 +1,107 @@
+/*
+ * Copyright 2018 ConsenSys AG.
+ *
+ * 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 tech.pegasys.pantheon.ethereum.util;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import tech.pegasys.pantheon.ethereum.core.Block;
+import tech.pegasys.pantheon.ethereum.core.BlockHeader;
+import tech.pegasys.pantheon.ethereum.core.Transaction;
+import tech.pegasys.pantheon.ethereum.mainnet.MainnetBlockHashFunction;
+import tech.pegasys.pantheon.ethereum.rlp.BytesValueRLPOutput;
+import tech.pegasys.pantheon.ethereum.testutil.BlockDataGenerator;
+
+import java.io.DataOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.List;
+import java.util.function.Function;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class RawBlockIteratorTest {
+
+  @Rule public final TemporaryFolder tmp = new TemporaryFolder();
+  private BlockDataGenerator gen;
+
+  @Before
+  public void setup() {
+    gen = new BlockDataGenerator(1);
+  }
+
+  @Test
+  public void readsBlockAtBoundaryOfInitialCapacity() throws IOException {
+    readsBlocksWithInitialCapacity(Function.identity());
+  }
+
+  @Test
+  public void readsBlockThatExtendsPastInitialCapacity() throws IOException {
+    readsBlocksWithInitialCapacity((size) -> size / 2);
+  }
+
+  @Test
+  public void readsBlockWithinInitialCapacity() throws IOException {
+    readsBlocksWithInitialCapacity((size) -> size * 2);
+  }
+
+  public void readsBlocksWithInitialCapacity(
+      final Function<Integer, Integer> initialCapacityFromBlockSize) throws IOException {
+    final int blockCount = 3;
+    final List<Block> blocks = gen.blockSequence(blockCount);
+
+    // Write a few blocks to a tmp file
+    byte[] firstSerializedBlock = null;
+    final File blocksFile = tmp.newFolder().toPath().resolve("blocks").toFile();
+    final DataOutputStream writer = new DataOutputStream(new FileOutputStream(blocksFile));
+    for (Block block : blocks) {
+      final byte[] serializedBlock = serializeBlock(block);
+      writer.write(serializedBlock);
+      if (firstSerializedBlock == null) {
+        firstSerializedBlock = serializedBlock;
+      }
+    }
+    writer.close();
+
+    // Read blocks
+    final int initialCapacity = initialCapacityFromBlockSize.apply(firstSerializedBlock.length);
+    final RawBlockIterator iterator =
+        new RawBlockIterator(
+            blocksFile.toPath(),
+            rlp -> BlockHeader.readFrom(rlp, MainnetBlockHashFunction::createHash),
+            initialCapacity);
+
+    // Read blocks and check that they match
+    for (int i = 0; i < blockCount; i++) {
+      assertThat(iterator.hasNext()).isTrue();
+      final Block readBlock = iterator.next();
+      final Block expectedBlock = blocks.get(i);
+      assertThat(readBlock).isEqualTo(expectedBlock);
+    }
+
+    assertThat(iterator.hasNext()).isFalse();
+  }
+
+  private byte[] serializeBlock(final Block block) {
+    final BytesValueRLPOutput out = new BytesValueRLPOutput();
+    out.startList();
+    block.getHeader().writeTo(out);
+    out.writeList(block.getBody().getTransactions(), Transaction::writeTo);
+    out.writeList(block.getBody().getOmmers(), BlockHeader::writeTo);
+    out.endList();
+    return out.encoded().extractArray();
+  }
+}

ethereum/rlp/build.gradle

@@ -34,6 +34,7 @@ dependencies {
   testImplementation project(':testutil')
   testImplementation project(path:':ethereum:referencetests', configuration: 'testOutput')
 
+  testImplementation 'org.assertj:assertj-core'
   testImplementation 'com.fasterxml.jackson.core:jackson-databind'
   testImplementation 'junit:junit'
 }

ethereum/rlp/src/main/java/tech/pegasys/pantheon/ethereum/rlp/AbstractRLPInput.java

@@ -15,6 +15,7 @@
 import static com.google.common.base.Preconditions.checkState;
 
 import tech.pegasys.pantheon.ethereum.rlp.RLPDecodingHelpers.Kind;
+import tech.pegasys.pantheon.ethereum.rlp.RLPDecodingHelpers.RLPElementMetadata;
 import tech.pegasys.pantheon.util.bytes.Bytes32;
 import tech.pegasys.pantheon.util.bytes.BytesValue;
 import tech.pegasys.pantheon.util.bytes.MutableBytes32;
@@ -31,7 +32,7 @@ abstract class AbstractRLPInput implements RLPInput {
 
   private final boolean lenient;
 
-  protected long size;
+  protected long size; // The number of bytes in this rlp-encoded byte string
 
   // Information on the item the input currently is at (next thing to read).
   protected long
@@ -56,7 +57,7 @@ protected void init(final long inputSize, final boolean shouldFitInputSizeExactl
     }
 
     currentItem = 0;
-    // Initially set the size to the input as prepareCurrentTime() needs it. Once we've prepare the
+    // Initially set the size to the input as prepareCurrentItem() needs it. Once we've prepared the
     // top level item, we know where that item ends exactly and can update the size to that more
     // precise value (which basically mean we'll throw errors on malformed inputs potentially
     // sooner).
@@ -127,31 +128,17 @@ protected void setTo(final long item) {
   private void prepareCurrentItem() {
     // Sets the kind of the item, the offset at which his payload starts and the size of this
     // payload.
-    final int prefix = inputByte(currentItem) & 0xFF;
-    currentKind = Kind.of(prefix);
-    switch (currentKind) {
-      case BYTE_ELEMENT:
-        currentPayloadOffset = currentItem;
-        currentPayloadSize = 1;
-        break;
-      case SHORT_ELEMENT:
-        currentPayloadOffset = currentItem + 1;
-        currentPayloadSize = prefix - 0x80;
-        break;
-      case LONG_ELEMENT:
-        final int sizeLengthElt = prefix - 0xb7;
-        currentPayloadOffset = currentItem + 1 + sizeLengthElt;
-        currentPayloadSize = readLongSize(currentItem, sizeLengthElt);
-        break;
-      case SHORT_LIST:
-        currentPayloadOffset = currentItem + 1;
-        currentPayloadSize = prefix - 0xc0;
-        break;
-      case LONG_LIST:
-        final int sizeLengthList = prefix - 0xf7;
-        currentPayloadOffset = currentItem + 1 + sizeLengthList;
-        currentPayloadSize = readLongSize(currentItem, sizeLengthList);
-        break;
+    try {
+      RLPElementMetadata elementMetadata =
+          RLPDecodingHelpers.rlpElementMetadata(this::inputByte, size, currentItem);
+      currentKind = elementMetadata.kind;
+      currentPayloadOffset = elementMetadata.payloadStart;
+      currentPayloadSize = elementMetadata.payloadSize;
+    } catch (RLPException exception) {
+      String message =
+          String.format(
+              exception.getMessage() + getErrorMessageSuffix(), getErrorMessageSuffixParams());
+      throw new RLPException(message, exception);
     }
   }
 
@@ -182,32 +169,6 @@ private void validateCurrentItem() {
     }
   }
 
-  /** The size of the item payload for a "long" item, given the length in bytes of the said size. */
-  private int readLongSize(final long item, final int sizeLength) {
-    // We will read sizeLength bytes from item + 1. There must be enough bytes for this or the input
-    // is corrupted.
-    if (size - (item + 1) < sizeLength) {
-      throw corrupted(
-          "Invalid RLP item: value of size %d has not enough bytes to read the %d "
-              + "bytes payload size",
-          size, sizeLength);
-    }
-
-    // That size (which is at least 1 byte by construction) shouldn't have leading zeros.
-    if (inputByte(item + 1) == 0) {
-      throwMalformed("Malformed RLP item: size of payload has leading zeros");
-    }
-
-    final int res = RLPDecodingHelpers.extractSizeFromLong(this::inputByte, item + 1, sizeLength);
-
-    // We should not have had the size written separately if it was less than 56 bytes long.
-    if (res < 56) {
-      throwMalformed("Malformed RLP item: written as a long item, but size %d < 56 bytes", res);
-    }
-
-    return res;
-  }
-
   private long nextItem() {
     return currentPayloadOffset + currentPayloadSize;
   }
@@ -246,21 +207,28 @@ private RLPException error(final Throwable cause, final String msg, final Object
   }
 
   private String errorMsg(final String message, final Object... params) {
+    return String.format(
+        message + getErrorMessageSuffix(), concatParams(params, getErrorMessageSuffixParams()));
+  }
+
+  private String getErrorMessageSuffix() {
+    return " (at bytes %d-%d: %s%s[%s]%s%s)";
+  }
+
+  private Object[] getErrorMessageSuffixParams() {
     final long start = currentItem;
     final long end = Math.min(size, nextItem());
     final long realStart = Math.max(0, start - 4);
     final long realEnd = Math.min(size, end + 4);
-    return String.format(
-        message + " (at bytes %d-%d: %s%s[%s]%s%s)",
-        concatParams(
-            params,
-            start,
-            end,
-            realStart == 0 ? "" : "...",
-            hex(realStart, start),
-            hex(start, end),
-            hex(end, realEnd),
-            realEnd == size ? "" : "..."));
+    return new Object[] {
+      start,
+      end,
+      realStart == 0 ? "" : "...",
+      hex(realStart, start),
+      hex(start, end),
+      hex(end, realEnd),
+      realEnd == size ? "" : "..."
+    };
   }
 
   private static Object[] concatParams(final Object[] initial, final Object... others) {

ethereum/rlp/src/main/java/tech/pegasys/pantheon/ethereum/rlp/BytesValueRLPInput.java

@@ -32,7 +32,7 @@ public BytesValueRLPInput(final BytesValue value, final boolean lenient) {
 
   @Override
   protected byte inputByte(final long offset) {
-    return value.get(Math.toIntExact(offset));
+    return value.get(offset);
   }
 
   @Override

ethereum/rlp/src/main/java/tech/pegasys/pantheon/ethereum/rlp/RLP.java

@@ -272,4 +272,15 @@ public static void validate(final BytesValue encodedValue) {
       }
     }
   }
+
+  /**
+   * Given a {@link BytesValue} containing rlp-encoded data, determines the full length of the
+   * encoded value (including the prefix) by inspecting the prefixed metadata.
+   *
+   * @param value the rlp-encoded byte string
+   * @return the length of the encoded data, according to the prefixed metadata
+   */
+  public static int calculateSize(final BytesValue value) {
+    return RLPDecodingHelpers.rlpElementMetadata(value::get, value.size(), 0).getEncodedSize();
+  }
 }