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

[PAN-3144] Don't store log blooms with transaction receipts #1941

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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 @@ -82,12 +82,15 @@ public static LogsBloomFilter compute(final Collection<Log> 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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -42,7 +44,7 @@ public class TransactionReceipt {
private final Hash stateRoot;
private final long cumulativeGasUsed;
private final List<Log> logs;
private final LogsBloomFilter bloomFilter;
private final Supplier<LogsBloomFilter> bloomFilterSupplier;
private final int status;
private final TransactionReceiptType transactionReceiptType;
private final Optional<BytesValue> revertReason;
Expand All @@ -60,22 +62,7 @@ public TransactionReceipt(
final long cumulativeGasUsed,
final List<Log> logs,
final Optional<BytesValue> revertReason) {
this(
stateRoot,
NONEXISTENT,
cumulativeGasUsed,
logs,
LogsBloomFilter.compute(logs),
revertReason);
}

private TransactionReceipt(
final Hash stateRoot,
final long cumulativeGasUsed,
final List<Log> logs,
final LogsBloomFilter bloomFilter,
final Optional<BytesValue> revertReason) {
this(stateRoot, NONEXISTENT, cumulativeGasUsed, logs, bloomFilter, revertReason);
this(stateRoot, NONEXISTENT, cumulativeGasUsed, logs, revertReason);
}

/**
Expand All @@ -91,30 +78,20 @@ public TransactionReceipt(
final long cumulativeGasUsed,
final List<Log> logs,
final Optional<BytesValue> revertReason) {
this(null, status, cumulativeGasUsed, logs, LogsBloomFilter.compute(logs), revertReason);
}

private TransactionReceipt(
final int status,
final long cumulativeGasUsed,
final List<Log> logs,
final LogsBloomFilter bloomFilter,
final Optional<BytesValue> revertReason) {
this(null, status, cumulativeGasUsed, logs, bloomFilter, revertReason);
this(null, status, cumulativeGasUsed, logs, revertReason);
}

private TransactionReceipt(
final Hash stateRoot,
final int status,
final long cumulativeGasUsed,
final List<Log> logs,
final LogsBloomFilter bloomFilter,
final Optional<BytesValue> 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;
Expand All @@ -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
Expand All @@ -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();
Expand All @@ -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 {
Expand All @@ -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<Log> 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<BytesValue> 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());
Expand All @@ -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();
Expand Down Expand Up @@ -238,7 +222,7 @@ public List<Log> getLogs() {
* @return the logs bloom filter for the logs generated by the transaction
*/
public LogsBloomFilter getBloomFilter() {
return bloomFilter;
return bloomFilterSupplier.get();
}

/**
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ private void remove(final BytesValue prefix, final BytesValue key) {
}

private BytesValue rlpEncode(final List<TransactionReceipt> receipts) {
return RLP.encode(o -> o.writeList(receipts, TransactionReceipt::writeToWithRevertReason));
return RLP.encode(o -> o.writeList(receipts, TransactionReceipt::writeForStorage));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}
}