From bc75df35d511ca99c5d90d0507e33c6d9973e7a1 Mon Sep 17 00:00:00 2001 From: shemnon Date: Fri, 13 Sep 2019 15:26:35 -0600 Subject: [PATCH] [PAN-3144] Don't store log blooms with transaction receipts The log bloom of a transaction receipt can be trivially calculated with the data on hand, so we don't need to store that data. Doing so will save gigabytes of data of historical blocks with no loss in data. --- .../ethereum/core/LogsBloomFilter.java | 7 +- .../ethereum/core/TransactionReceipt.java | 67 +++++++------------ ...ueStoragePrefixedKeyBlockchainStorage.java | 2 +- .../ethereum/core/TransactionReceiptTest.java | 4 +- 4 files changed, 33 insertions(+), 47 deletions(-) diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/LogsBloomFilter.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/LogsBloomFilter.java index 3203c74986..efd2163e3b 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/LogsBloomFilter.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/LogsBloomFilter.java @@ -82,12 +82,15 @@ public static LogsBloomFilter compute(final Collection logs) { */ public static LogsBloomFilter readFrom(final RLPInput input) { final BytesValue bytes = input.readBytesValue(); - if (bytes.size() != BYTE_SIZE) { + if (bytes == null || bytes.size() == 0) { + return null; + } else if (bytes.size() != BYTE_SIZE) { throw new RLPException( String.format( "LogsBloomFilter unexpected size of %s (needs %s)", bytes.size(), BYTE_SIZE)); + } else { + return new LogsBloomFilter(bytes); } - return new LogsBloomFilter(bytes); } /** diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/TransactionReceipt.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/TransactionReceipt.java index d9e38b5a5a..f9c0fe08d1 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/TransactionReceipt.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/TransactionReceipt.java @@ -21,8 +21,10 @@ import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.function.Supplier; import com.google.common.base.MoreObjects; +import com.google.common.base.Suppliers; /** * A transaction receipt, containing information pertaining a transaction execution. @@ -42,7 +44,7 @@ public class TransactionReceipt { private final Hash stateRoot; private final long cumulativeGasUsed; private final List logs; - private final LogsBloomFilter bloomFilter; + private final Supplier bloomFilterSupplier; private final int status; private final TransactionReceiptType transactionReceiptType; private final Optional revertReason; @@ -60,22 +62,7 @@ public TransactionReceipt( final long cumulativeGasUsed, final List logs, final Optional revertReason) { - this( - stateRoot, - NONEXISTENT, - cumulativeGasUsed, - logs, - LogsBloomFilter.compute(logs), - revertReason); - } - - private TransactionReceipt( - final Hash stateRoot, - final long cumulativeGasUsed, - final List logs, - final LogsBloomFilter bloomFilter, - final Optional revertReason) { - this(stateRoot, NONEXISTENT, cumulativeGasUsed, logs, bloomFilter, revertReason); + this(stateRoot, NONEXISTENT, cumulativeGasUsed, logs, revertReason); } /** @@ -91,16 +78,7 @@ public TransactionReceipt( final long cumulativeGasUsed, final List logs, final Optional revertReason) { - this(null, status, cumulativeGasUsed, logs, LogsBloomFilter.compute(logs), revertReason); - } - - private TransactionReceipt( - final int status, - final long cumulativeGasUsed, - final List logs, - final LogsBloomFilter bloomFilter, - final Optional revertReason) { - this(null, status, cumulativeGasUsed, logs, bloomFilter, revertReason); + this(null, status, cumulativeGasUsed, logs, revertReason); } private TransactionReceipt( @@ -108,13 +86,12 @@ private TransactionReceipt( final int status, final long cumulativeGasUsed, final List logs, - final LogsBloomFilter bloomFilter, final Optional revertReason) { this.stateRoot = stateRoot; this.cumulativeGasUsed = cumulativeGasUsed; this.status = status; this.logs = logs; - this.bloomFilter = bloomFilter; + this.bloomFilterSupplier = Suppliers.memoize(() -> LogsBloomFilter.compute(this.logs)); transactionReceiptType = stateRoot == null ? TransactionReceiptType.STATUS : TransactionReceiptType.ROOT; this.revertReason = revertReason; @@ -129,11 +106,11 @@ public void writeTo(final RLPOutput out) { writeTo(out, false); } - public void writeToWithRevertReason(final RLPOutput out) { + public void writeForStorage(final RLPOutput out) { writeTo(out, true); } - private void writeTo(final RLPOutput out, final boolean withRevertReason) { + private void writeTo(final RLPOutput out, final boolean forStorage) { out.startList(); // Determine whether it's a state root-encoded transaction receipt @@ -144,9 +121,13 @@ private void writeTo(final RLPOutput out, final boolean withRevertReason) { out.writeLongScalar(status); } out.writeLongScalar(cumulativeGasUsed); - out.writeBytesValue(bloomFilter.getBytes()); + if (forStorage) { + out.writeBytesValue(BytesValue.EMPTY); + } else { + out.writeBytesValue(bloomFilterSupplier.get().getBytes()); + } out.writeList(logs, Log::writeTo); - if (withRevertReason && revertReason.isPresent()) { + if (forStorage && revertReason.isPresent()) { out.writeBytesValue(revertReason.get()); } out.endList(); @@ -165,11 +146,11 @@ public static TransactionReceipt readFrom(final RLPInput input) { * Creates a transaction receipt for the given RLP * * @param input the RLP-encoded transaction receipt - * @param revertReasonAllowed whether the rlp input is allowed to have a revert reason + * @param fromStorage If we are reading from storage, which allows a revert reason and an empty + * bloom filter. * @return the transaction receipt */ - public static TransactionReceipt readFrom( - final RLPInput input, final boolean revertReasonAllowed) { + public static TransactionReceipt readFrom(final RLPInput input, final boolean fromStorage) { input.enterList(); try { @@ -178,14 +159,17 @@ public static TransactionReceipt readFrom( final RLPInput firstElement = input.readAsRlp(); final long cumulativeGas = input.readLongScalar(); // The logs below will populate the bloom filter upon construction. - // TODO consider validating that the logs and bloom filter match. final LogsBloomFilter bloomFilter = LogsBloomFilter.readFrom(input); final List logs = input.readList(Log::readFrom); + if (bloomFilter != null && !bloomFilter.equals(LogsBloomFilter.compute(logs))) { + // What identification do we pass in the Exception? Or do we log and run? + throw new RuntimeException("Stored Bloom Filter was computed wrong"); + } final Optional revertReason; if (input.isEndOfCurrentList()) { revertReason = Optional.empty(); } else { - if (!revertReasonAllowed) { + if (!fromStorage) { throw new RLPException("Unexpected value at end of TransactionReceipt"); } revertReason = Optional.of(input.readBytesValue()); @@ -195,10 +179,10 @@ public static TransactionReceipt readFrom( // byte for success (0x01) or failure (0x80). if (firstElement.raw().size() == 1) { final int status = firstElement.readIntScalar(); - return new TransactionReceipt(status, cumulativeGas, logs, bloomFilter, revertReason); + return new TransactionReceipt(status, cumulativeGas, logs, revertReason); } else { final Hash stateRoot = Hash.wrap(firstElement.readBytes32()); - return new TransactionReceipt(stateRoot, cumulativeGas, logs, bloomFilter, revertReason); + return new TransactionReceipt(stateRoot, cumulativeGas, logs, revertReason); } } finally { input.leaveList(); @@ -238,7 +222,7 @@ public List getLogs() { * @return the logs bloom filter for the logs generated by the transaction */ public LogsBloomFilter getBloomFilter() { - return bloomFilter; + return bloomFilterSupplier.get(); } /** @@ -284,7 +268,6 @@ public String toString() { .add("stateRoot", stateRoot) .add("cumulativeGasUsed", cumulativeGasUsed) .add("logs", logs) - .add("bloomFilter", bloomFilter) .add("status", status) .add("transactionReceiptType", transactionReceiptType) .toString(); diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorage.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorage.java index c8725d36f7..e59a85b1fb 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorage.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorage.java @@ -203,7 +203,7 @@ private void remove(final BytesValue prefix, final BytesValue key) { } private BytesValue rlpEncode(final List receipts) { - return RLP.encode(o -> o.writeList(receipts, TransactionReceipt::writeToWithRevertReason)); + return RLP.encode(o -> o.writeList(receipts, TransactionReceipt::writeForStorage)); } } } diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionReceiptTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionReceiptTest.java index df6b7bd1d6..e2e9feede0 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionReceiptTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionReceiptTest.java @@ -26,7 +26,7 @@ public void toFromRlp() { final BlockDataGenerator gen = new BlockDataGenerator(); final TransactionReceipt receipt = gen.receipt(); final TransactionReceipt copy = - TransactionReceipt.readFrom(RLP.input(RLP.encode(receipt::writeToWithRevertReason)), false); + TransactionReceipt.readFrom(RLP.input(RLP.encode(receipt::writeForStorage)), false); assertThat(copy).isEqualTo(receipt); } @@ -35,7 +35,7 @@ public void toFromRlpWithReason() { final BlockDataGenerator gen = new BlockDataGenerator(); final TransactionReceipt receipt = gen.receipt(BytesValue.fromHexString("0x1122334455667788")); final TransactionReceipt copy = - TransactionReceipt.readFrom(RLP.input(RLP.encode(receipt::writeToWithRevertReason))); + TransactionReceipt.readFrom(RLP.input(RLP.encode(receipt::writeForStorage))); assertThat(copy).isEqualTo(receipt); } }