Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enforce that BlobTransactions have at least one blob #5826

Merged
merged 3 commits into from
Aug 30, 2023
Merged
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 @@ -38,11 +38,15 @@ public BlobsWithCommitments(
final List<Blob> blobs,
final List<KZGProof> kzgProofs,
final List<VersionedHash> versionedHashes) {
if (blobs.size() == 0) {
throw new InvalidParameterException(
"There needs to be a minimum of one blob in a blob transaction with commitments");
}
if (blobs.size() != kzgCommitments.size()
|| blobs.size() != kzgProofs.size()
|| kzgCommitments.size() != versionedHashes.size()) {
|| blobs.size() != versionedHashes.size()) {
throw new InvalidParameterException(
"There must be an equal number of blobs, commitments and proofs");
"There must be an equal number of blobs, commitments, proofs, and versioned hashes");
}
this.kzgCommitments = kzgCommitments;
this.blobs = blobs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,70 @@
import java.util.List;

import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import org.junit.jupiter.api.Test;

public class BlobsWithCommitmentsTest {
@Test
public void blobsWithCommitmentsMustHaveSameNumberOfElements() {
public void blobsWithCommitmentsMustHaveAtLeastOneBlob() {
String actualMessage =
assertThrows(
InvalidParameterException.class,
() -> new BlobsWithCommitments(List.of(), List.of(), List.of(), List.of()))
.getMessage();
final String expectedMessage =
"There needs to be a minimum of one blob in a blob transaction with commitments";
assertThat(actualMessage).isEqualTo(expectedMessage);
}

@Test
public void blobsWithCommitmentsMustHaveSameNumberOfElementsVersionedHashes() {
String actualMessage =
assertThrows(
InvalidParameterException.class,
() ->
new BlobsWithCommitments(
List.of(new KZGCommitment(Bytes.of(1))),
List.of(new Blob(Bytes.EMPTY)),
List.of(new KZGProof(Bytes.EMPTY)),
List.of()))
.getMessage();
final String expectedMessage =
"There must be an equal number of blobs, commitments, proofs, and versioned hashes";
assertThat(actualMessage).isEqualTo(expectedMessage);
}

@Test
public void blobsWithCommitmentsMustHaveSameNumberOfElementsKZGCommitment() {
String actualMessage =
assertThrows(
InvalidParameterException.class,
() ->
new BlobsWithCommitments(
List.of(),
List.of(new Blob(Bytes.EMPTY)),
List.of(new KZGProof(Bytes.EMPTY)),
List.of(new VersionedHash(Bytes32.rightPad(Bytes.fromHexString("0x01"))))))
.getMessage();
final String expectedMessage =
"There must be an equal number of blobs, commitments, proofs, and versioned hashes";
assertThat(actualMessage).isEqualTo(expectedMessage);
}

@Test
public void blobsWithCommitmentsMustHaveSameNumberOfElementsKZGProof() {
String actualMessage =
assertThrows(
InvalidParameterException.class,
() ->
new BlobsWithCommitments(
List.of(new KZGCommitment(Bytes.of(1))), List.of(), List.of(), List.of()))
List.of(new KZGCommitment(Bytes.of(1))),
List.of(new Blob(Bytes.EMPTY)),
List.of(),
List.of(new VersionedHash(Bytes32.rightPad(Bytes.fromHexString("0x01"))))))
.getMessage();
final String expectedMessage = "There must be an equal number of blobs, commitments and proofs";
final String expectedMessage =
"There must be an equal number of blobs, commitments, proofs, and versioned hashes";
assertThat(actualMessage).isEqualTo(expectedMessage);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
blockParam.getPrevRandao(),
0,
maybeWithdrawals.map(BodyValidation::withdrawalsRoot).orElse(null),
blockParam.getBlobGasUsed() == null ? null : blockParam.getBlobGasUsed(),
blockParam.getBlobGasUsed(),
blockParam.getExcessBlobGas() == null
? null
: BlobGas.fromHexString(blockParam.getExcessBlobGas()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,15 @@ public Transaction(
if (versionedHashes.isPresent() || maxFeePerBlobGas.isPresent()) {
checkArgument(
transactionType.supportsBlob(),
"Must not specify blob versioned hashes of max fee per blob gas for transaction not supporting it");
"Must not specify blob versioned hashes or max fee per blob gas for transaction not supporting it");
}

if (transactionType.supportsBlob()) {
checkArgument(
versionedHashes.isPresent(), "Must specify blob versioned hashes for blob transaction");
checkArgument(
!versionedHashes.get().isEmpty(), "Blob transaction must have at least one blob");
!versionedHashes.get().isEmpty(),
"Blob transaction must have at least one versioned hash");
checkArgument(
maxFeePerBlobGas.isPresent(), "Must specify max fee per blob gas for blob transaction");
}
Expand Down Expand Up @@ -680,13 +681,13 @@ private void memoizeHashAndSize() {

if (transactionType.supportsBlob()) {
if (getBlobsWithCommitments().isPresent()) {
size = TransactionEncoder.encodeOpaqueBytes(this).size();
size = bytes.size();
return;
}
} else {
final BytesValueRLPOutput rlpOutput = new BytesValueRLPOutput();
TransactionEncoder.encodeForWire(transactionType, bytes, rlpOutput);
size = rlpOutput.encodedSize();
}
final BytesValueRLPOutput rlpOutput = new BytesValueRLPOutput();
TransactionEncoder.encodeForWire(transactionType, bytes, rlpOutput);
size = rlpOutput.encodedSize();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void zeroBlobTransactionIsInvalid() {
.createTransaction(senderKeys);
fail();
} catch (IllegalArgumentException iea) {
assertThat(iea).hasMessage("Blob transaction must have at least one blob");
assertThat(iea).hasMessage("Blob transaction must have at least one versioned hash");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ public static Responder partialResponder(
final TransactionPool transactionPool,
final ProtocolSchedule protocolSchedule,
final float portion) {
checkArgument(portion >= 0.0 && portion <= 1.0, "Portion is in the range [0.0..1.0]");
checkArgument(portion >= 0.0 && portion <= 1.0, "Portion is not in the range [0.0..1.0]");

final Responder fullResponder =
blockchainResponder(blockchain, worldStateArchive, transactionPool);
Expand Down
Loading