Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

[NC-1561] Remove RLPUtils from RawBlockIterator #143

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,21 @@
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 = 2 << 15;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional style: would 1 << 16 or 0x100 be more clear?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I thought this format was pretty weird, but just moved the value around. Will update.


private final FileChannel fileChannel;
private final Function<RLPInput, BlockHeader> headerReader;
Expand All @@ -40,13 +40,22 @@ public final class RawBlockIterator implements Iterator<Block>, Closeable {

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;
Expand Down Expand Up @@ -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);
Expand All @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* 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.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 {
int blockCount = 2;
final Block block = gen.block();

// Write a few blocks to a tmp file
byte[] serializedBlock = serializeBlock(block);
final File blocksFile = tmp.newFolder().toPath().resolve("blocks").toFile();
DataOutputStream writer = new DataOutputStream(new FileOutputStream(blocksFile));
for (int i = 0; i < blockCount; i++) {
writer.write(serializedBlock);
}
writer.close();

// Read blocks
int initialCapacity = initialCapacityFromBlockSize.apply(serializedBlock.length);
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();
Block readBlock = iterator.next();
assertThat(readBlock).isEqualTo(block);
}

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();
}
}
1 change: 1 addition & 0 deletions ethereum/rlp/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional style: a javadoc comment would turn up un API docs.


// Information on the item the input currently is at (next thing to read).
protected long
Expand All @@ -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).
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required in the case of an input larger than Integer.MAX_VALUE. For example, Importing the blockchain with a 160GB file. The Interface needs to support the long offset. However, the bytes lib doesn't support long offsets.
Without this check, the long will be silently cast to an int, causing corrupted data in this implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a get() method to the BytesValue interface that accepts a long: https://github.com/PegaSysEng/pantheon/pull/143/files#diff-07c80cee998cbce23f1072fbe5c729f1R334

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Loading