Skip to content

fix yParity flakey test #6151

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.

Sign up for GitHub

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

Merged
shemnon merged 3 commits into from
Nov 9, 2023
Merged

fix yParity flakey test #6151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f63d5a7
fix yParity flakey test
shemnon Nov 9, 2023
79eba05
spotless
shemnon Nov 9, 2023
788267f
roll back unit test
shemnon Nov 9, 2023
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
29 changes: 23 additions & 6 deletions ...g/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetTransactionByHashTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -71,7 +72,8 @@ void returnsCorrectMethodName() {

@Test
void shouldReturnErrorResponseIfMissingRequiredParameter() {
final JsonRpcRequest request = new JsonRpcRequest("2.0", method.getName(), new Object[] {});
final JsonRpcRequest request =
new JsonRpcRequest(JSON_RPC_VERSION, method.getName(), new Object[] {});
final JsonRpcRequestContext context = new JsonRpcRequestContext(request);

final JsonRpcErrorResponse expectedResponse =
Expand All @@ -92,7 +94,7 @@ void shouldReturnNullResultWhenTransactionDoesNotExist() {
.thenReturn(Optional.empty());

final JsonRpcRequest request =
new JsonRpcRequest("2.0", method.getName(), new Object[] {transactionHash});
new JsonRpcRequest(JSON_RPC_VERSION, method.getName(), new Object[] {transactionHash});
final JsonRpcRequestContext context = new JsonRpcRequestContext(request);

final JsonRpcSuccessResponse expectedResponse =
Expand All @@ -115,7 +117,7 @@ void shouldReturnPendingTransactionWhenTransactionExistsAndIsPending() {

final JsonRpcRequest request =
new JsonRpcRequest(
"2.0", method.getName(), new Object[] {transaction.getHash().toHexString()});
JSON_RPC_VERSION, method.getName(), new Object[] {transaction.getHash().toHexString()});
final JsonRpcRequestContext context = new JsonRpcRequestContext(request);

final JsonRpcSuccessResponse expectedResponse =
Expand All @@ -141,7 +143,7 @@ void shouldReturnCompleteTransactionWhenTransactionExistsInBlockchain() {

final JsonRpcRequest request =
new JsonRpcRequest(
"2.0", method.getName(), new Object[] {transaction.getHash().toHexString()});
JSON_RPC_VERSION, method.getName(), new Object[] {transaction.getHash().toHexString()});
final JsonRpcRequestContext context = new JsonRpcRequestContext(request);

final JsonRpcSuccessResponse expectedResponse =
Expand Down Expand Up @@ -181,8 +183,23 @@ void validateResultSpec() {
assertThat(result.getRaw()).isNotNull();
assertThat(result.getTo()).isNotNull();
assertThat(result.getValue()).isNotNull();
assertThat(result.getYParity()).isNotNull();
assertThat(result.getV()).isNotNull();
switch (result.getType()) {
case "0x0":
assertThat(result.getYParity()).isNull();
assertThat(result.getV()).isNotNull();
break;
case "0x1":
case "0x2":
assertThat(result.getYParity()).isNotNull();
assertThat(result.getV()).isNotNull();
break;
case "0x3":
assertThat(result.getYParity()).isNotNull();
assertThat(result.getV()).isNull();
break;
default:
fail("unknownType " + result.getType());
}
assertThat(result.getR()).isNotNull();
assertThat(result.getS()).isNotNull();
}
Expand Down
7 changes: 5 additions & 2 deletions ...c/main/java/org/hyperledger/besu/ethereum/core/encoding/BlobPooledTransactionDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
*/
public class BlobPooledTransactionDecoder {

private BlobPooledTransactionDecoder() {
// no instances
}

/**
* Decodes a blob transaction from the provided RLP input.
*
Expand All @@ -44,7 +48,6 @@ public static Transaction decode(final RLPInput input) {
List<KZGCommitment> commitments = input.readList(KZGCommitment::readFrom);
List<KZGProof> proofs = input.readList(KZGProof::readFrom);
input.leaveList();
builder.kzgBlobs(commitments, blobs, proofs);
return builder.build();
return builder.kzgBlobs(commitments, blobs, proofs).build();
}
}
37 changes: 29 additions & 8 deletions .../src/test/java/org/hyperledger/besu/ethereum/core/encoding/TransactionRLPDecoderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.hyperledger.besu.evm.account.Account.MAX_NONCE;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.core.Transaction;
Expand All @@ -32,7 +33,6 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;

class TransactionRLPDecoderTest {

Expand Down Expand Up @@ -82,15 +82,22 @@ void shouldDecodeWithHighNonce() {
private static Collection<Object[]> dataTransactionSize() {
return Arrays.asList(
new Object[][] {
{FRONTIER_TX_RLP, "FRONTIER_TX_RLP"},
{EIP1559_TX_RLP, "EIP1559_TX_RLP"},
{NONCE_64_BIT_MAX_MINUS_2_TX_RLP, "NONCE_64_BIT_MAX_MINUS_2_TX_RLP"}
{FRONTIER_TX_RLP, "FRONTIER_TX_RLP", true},
{EIP1559_TX_RLP, "EIP1559_TX_RLP", true},
{NONCE_64_BIT_MAX_MINUS_2_TX_RLP, "NONCE_64_BIT_MAX_MINUS_2_TX_RLP", true},
{
"01f89a0130308263309430303030303030303030303030303030303030303030f838f7943030303030303030303030303030303030303030e0a0303030303030303030303030303030303030303030303030303030303030303001a03130303130303031313031313031303130303030323030323030323030313030a03030303030303030303030303030303030303030303030303030303030303030",
"EIP1559 list too small",
false
}
});
}

@ParameterizedTest(name = "[{index}] {1}")
@MethodSource("dataTransactionSize")
void shouldCalculateCorrectTransactionSize(final String rlp_tx, final String ignoredName) {
void shouldCalculateCorrectTransactionSize(
final String rlp_tx, final String ignoredName, final boolean valid) {
assumeTrue(valid);
// Create bytes from String
final Bytes bytes = Bytes.fromHexString(rlp_tx);
// Decode bytes into a transaction
Expand All @@ -101,13 +108,27 @@ void shouldCalculateCorrectTransactionSize(final String rlp_tx, final String ign
assertThat(transaction.getSize()).isEqualTo(transactionBytes.size());
}

@ParameterizedTest
@ValueSource(strings = {FRONTIER_TX_RLP, EIP1559_TX_RLP, NONCE_64_BIT_MAX_MINUS_2_TX_RLP})
void shouldReturnCorrectEncodedBytes(final String txRlp) {
@ParameterizedTest(name = "[{index}] {1}")
@MethodSource("dataTransactionSize")
void shouldReturnCorrectEncodedBytes(
final String txRlp, final String ignoredName, final boolean valid) {
assumeTrue(valid);
final Transaction transaction = decodeRLP(RLP.input(Bytes.fromHexString(txRlp)));
assertThat(transaction.encoded()).isEqualTo(Bytes.fromHexString(txRlp));
}

@ParameterizedTest(name = "[{index}] {1}")
@MethodSource("dataTransactionSize")
void shouldDecodeRLP(final String txRlp, final String ignoredName, final boolean valid) {
if (valid) {
// thrown exceptions will break test
decodeRLP(RLP.input(Bytes.fromHexString(txRlp)));
} else {
assertThatThrownBy(() -> decodeRLP(RLP.input(Bytes.fromHexString(txRlp))))
.isInstanceOf(RLPException.class);
}
}

private Transaction decodeRLP(final RLPInput input) {
return TransactionDecoder.decodeRLP(input, EncodingContext.POOLED_TRANSACTION);
}
Expand Down
30 changes: 16 additions & 14 deletions ethereum/rlp/src/main/java/org/hyperledger/besu/ethereum/rlp/AbstractRLPInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,17 +147,16 @@ private void prepareCurrentItem() {
}

private void validateCurrentItem() {
if (currentKind == RLPDecodingHelpers.Kind.SHORT_ELEMENT) {
// Validate that a single byte SHORT_ELEMENT payload is not <= 0x7F. If it is, is should have
// been written as a BYTE_ELEMENT.
if (currentPayloadSize == 1
&& currentPayloadOffset < size
&& (payloadByte(0) & 0xFF) <= 0x7F) {
throwMalformed(
"Malformed RLP item: single byte value 0x%s should have been "
+ "written without a prefix",
hex(currentPayloadOffset, currentPayloadOffset + 1));
}
// Validate that a single byte SHORT_ELEMENT payload is not <= 0x7F. If it is, is should have
// been written as a BYTE_ELEMENT.
if (currentKind == RLPDecodingHelpers.Kind.SHORT_ELEMENT
&& currentPayloadSize == 1
&& currentPayloadOffset < size
&& (payloadByte(0) & 0xFF) <= 0x7F) {
throwMalformed(
"Malformed RLP item: single byte value 0x%s should have been "
+ "written without a prefix",
hex(currentPayloadOffset, currentPayloadOffset + 1));
}

if (currentPayloadSize > 0 && currentPayloadOffset >= size) {
Expand Down Expand Up @@ -186,9 +185,9 @@ public boolean isDone() {

private String hex(final long start, final long taintedEnd) {
final long end = Math.min(taintedEnd, size);
final long size = end - start;
if (size < 10) {
return inputHex(start, Math.toIntExact(size));
final long length = end - start;
if (length < 10) {
return inputHex(start, Math.toIntExact(length));
} else {
return String.format("%s...%s", inputHex(start, 4), inputHex(end - 4, 4));
}
Expand Down Expand Up @@ -245,6 +244,9 @@ private void checkElt(final String what) {
if (currentItem >= size) {
throw error("Cannot read a %s, input is fully consumed", what);
}
if (depth > 0 && currentPayloadOffset + currentPayloadSize > endOfListOffset[depth - 1]) {
throw error("Cannot read a %s, too large for enclosing list", what);
}
if (isEndOfCurrentList()) {
throw error("Cannot read a %s, reached end of current list", what);
}
Expand Down