Skip to content

Commit 4b58d07

Browse files
shemnonshemnon
authored
fix yParity flakey test (#6151)
Fix the flakeiness in EthGetTransactionByHashTest as well as some other sonar identified cleanup. Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
1 parent 9710a9a commit 4b58d07

File tree

4 files changed

+73
-30
lines changed

4 files changed

+73
-30
lines changed

‎ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetTransactionByHashTest.java‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods;
1616

1717
import static org.assertj.core.api.Assertions.assertThat;
18+
import static org.junit.jupiter.api.Assertions.fail;
1819
import static org.mockito.Mockito.verifyNoInteractions;
1920
import static org.mockito.Mockito.verifyNoMoreInteractions;
2021
import static org.mockito.Mockito.when;
@@ -71,7 +72,8 @@ void returnsCorrectMethodName() {
7172

7273
@Test
7374
void shouldReturnErrorResponseIfMissingRequiredParameter() {
74-
final JsonRpcRequest request = new JsonRpcRequest("2.0", method.getName(), new Object[] {});
75+
final JsonRpcRequest request =
76+
new JsonRpcRequest(JSON_RPC_VERSION, method.getName(), new Object[] {});
7577
final JsonRpcRequestContext context = new JsonRpcRequestContext(request);
7678

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

9496
final JsonRpcRequest request =
95-
new JsonRpcRequest("2.0", method.getName(), new Object[] {transactionHash});
97+
new JsonRpcRequest(JSON_RPC_VERSION, method.getName(), new Object[] {transactionHash});
9698
final JsonRpcRequestContext context = new JsonRpcRequestContext(request);
9799

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

116118
final JsonRpcRequest request =
117119
new JsonRpcRequest(
118-
"2.0", method.getName(), new Object[] {transaction.getHash().toHexString()});
120+
JSON_RPC_VERSION, method.getName(), new Object[] {transaction.getHash().toHexString()});
119121
final JsonRpcRequestContext context = new JsonRpcRequestContext(request);
120122

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

142144
final JsonRpcRequest request =
143145
new JsonRpcRequest(
144-
"2.0", method.getName(), new Object[] {transaction.getHash().toHexString()});
146+
JSON_RPC_VERSION, method.getName(), new Object[] {transaction.getHash().toHexString()});
145147
final JsonRpcRequestContext context = new JsonRpcRequestContext(request);
146148

147149
final JsonRpcSuccessResponse expectedResponse =
@@ -181,8 +183,23 @@ void validateResultSpec() {
181183
assertThat(result.getRaw()).isNotNull();
182184
assertThat(result.getTo()).isNotNull();
183185
assertThat(result.getValue()).isNotNull();
184-
assertThat(result.getYParity()).isNotNull();
185-
assertThat(result.getV()).isNotNull();
186+
switch (result.getType()) {
187+
case "0x0":
188+
assertThat(result.getYParity()).isNull();
189+
assertThat(result.getV()).isNotNull();
190+
break;
191+
case "0x1":
192+
case "0x2":
193+
assertThat(result.getYParity()).isNotNull();
194+
assertThat(result.getV()).isNotNull();
195+
break;
196+
case "0x3":
197+
assertThat(result.getYParity()).isNotNull();
198+
assertThat(result.getV()).isNull();
199+
break;
200+
default:
201+
fail("unknownType " + result.getType());
202+
}
186203
assertThat(result.getR()).isNotNull();
187204
assertThat(result.getS()).isNotNull();
188205
}

‎ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/BlobPooledTransactionDecoder.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@
3030
*/
3131
public class BlobPooledTransactionDecoder {
3232

33+
private BlobPooledTransactionDecoder() {
34+
// no instances
35+
}
36+
3337
/**
3438
* Decodes a blob transaction from the provided RLP input.
3539
*
@@ -44,7 +48,6 @@ public static Transaction decode(final RLPInput input) {
4448
List<KZGCommitment> commitments = input.readList(KZGCommitment::readFrom);
4549
List<KZGProof> proofs = input.readList(KZGProof::readFrom);
4650
input.leaveList();
47-
builder.kzgBlobs(commitments, blobs, proofs);
48-
return builder.build();
51+
return builder.kzgBlobs(commitments, blobs, proofs).build();
4952
}
5053
}

‎ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/encoding/TransactionRLPDecoderTest.java‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static org.assertj.core.api.Assertions.assertThat;
1818
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1919
import static org.hyperledger.besu.evm.account.Account.MAX_NONCE;
20+
import static org.junit.jupiter.api.Assumptions.assumeTrue;
2021

2122
import org.hyperledger.besu.datatypes.Wei;
2223
import org.hyperledger.besu.ethereum.core.Transaction;
@@ -32,7 +33,6 @@
3233
import org.junit.jupiter.api.Test;
3334
import org.junit.jupiter.params.ParameterizedTest;
3435
import org.junit.jupiter.params.provider.MethodSource;
35-
import org.junit.jupiter.params.provider.ValueSource;
3636

3737
class TransactionRLPDecoderTest {
3838

@@ -82,15 +82,22 @@ void shouldDecodeWithHighNonce() {
8282
private static Collection<Object[]> dataTransactionSize() {
8383
return Arrays.asList(
8484
new Object[][] {
85-
{FRONTIER_TX_RLP, "FRONTIER_TX_RLP"},
86-
{EIP1559_TX_RLP, "EIP1559_TX_RLP"},
87-
{NONCE_64_BIT_MAX_MINUS_2_TX_RLP, "NONCE_64_BIT_MAX_MINUS_2_TX_RLP"}
85+
{FRONTIER_TX_RLP, "FRONTIER_TX_RLP", true},
86+
{EIP1559_TX_RLP, "EIP1559_TX_RLP", true},
87+
{NONCE_64_BIT_MAX_MINUS_2_TX_RLP, "NONCE_64_BIT_MAX_MINUS_2_TX_RLP", true},
88+
{
89+
"01f89a0130308263309430303030303030303030303030303030303030303030f838f7943030303030303030303030303030303030303030e0a0303030303030303030303030303030303030303030303030303030303030303001a03130303130303031313031313031303130303030323030323030323030313030a03030303030303030303030303030303030303030303030303030303030303030",
90+
"EIP1559 list too small",
91+
false
92+
}
8893
});
8994
}
9095

9196
@ParameterizedTest(name = "[{index}] {1}")
9297
@MethodSource("dataTransactionSize")
93-
void shouldCalculateCorrectTransactionSize(final String rlp_tx, final String ignoredName) {
98+
void shouldCalculateCorrectTransactionSize(
99+
final String rlp_tx, final String ignoredName, final boolean valid) {
100+
assumeTrue(valid);
94101
// Create bytes from String
95102
final Bytes bytes = Bytes.fromHexString(rlp_tx);
96103
// Decode bytes into a transaction
@@ -101,13 +108,27 @@ void shouldCalculateCorrectTransactionSize(final String rlp_tx, final String ign
101108
assertThat(transaction.getSize()).isEqualTo(transactionBytes.size());
102109
}
103110

104-
@ParameterizedTest
105-
@ValueSource(strings = {FRONTIER_TX_RLP, EIP1559_TX_RLP, NONCE_64_BIT_MAX_MINUS_2_TX_RLP})
106-
void shouldReturnCorrectEncodedBytes(final String txRlp) {
111+
@ParameterizedTest(name = "[{index}] {1}")
112+
@MethodSource("dataTransactionSize")
113+
void shouldReturnCorrectEncodedBytes(
114+
final String txRlp, final String ignoredName, final boolean valid) {
115+
assumeTrue(valid);
107116
final Transaction transaction = decodeRLP(RLP.input(Bytes.fromHexString(txRlp)));
108117
assertThat(transaction.encoded()).isEqualTo(Bytes.fromHexString(txRlp));
109118
}
110119

120+
@ParameterizedTest(name = "[{index}] {1}")
121+
@MethodSource("dataTransactionSize")
122+
void shouldDecodeRLP(final String txRlp, final String ignoredName, final boolean valid) {
123+
if (valid) {
124+
// thrown exceptions will break test
125+
decodeRLP(RLP.input(Bytes.fromHexString(txRlp)));
126+
} else {
127+
assertThatThrownBy(() -> decodeRLP(RLP.input(Bytes.fromHexString(txRlp))))
128+
.isInstanceOf(RLPException.class);
129+
}
130+
}
131+
111132
private Transaction decodeRLP(final RLPInput input) {
112133
return TransactionDecoder.decodeRLP(input, EncodingContext.POOLED_TRANSACTION);
113134
}

‎ethereum/rlp/src/main/java/org/hyperledger/besu/ethereum/rlp/AbstractRLPInput.java‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -147,17 +147,16 @@ private void prepareCurrentItem() {
147147
}
148148

149149
private void validateCurrentItem() {
150-
if (currentKind == RLPDecodingHelpers.Kind.SHORT_ELEMENT) {
151-
// Validate that a single byte SHORT_ELEMENT payload is not <= 0x7F. If it is, is should have
152-
// been written as a BYTE_ELEMENT.
153-
if (currentPayloadSize == 1
154-
&& currentPayloadOffset < size
155-
&& (payloadByte(0) & 0xFF) <= 0x7F) {
156-
throwMalformed(
157-
"Malformed RLP item: single byte value 0x%s should have been "
158-
+ "written without a prefix",
159-
hex(currentPayloadOffset, currentPayloadOffset + 1));
160-
}
150+
// Validate that a single byte SHORT_ELEMENT payload is not <= 0x7F. If it is, is should have
151+
// been written as a BYTE_ELEMENT.
152+
if (currentKind == RLPDecodingHelpers.Kind.SHORT_ELEMENT
153+
&& currentPayloadSize == 1
154+
&& currentPayloadOffset < size
155+
&& (payloadByte(0) & 0xFF) <= 0x7F) {
156+
throwMalformed(
157+
"Malformed RLP item: single byte value 0x%s should have been "
158+
+ "written without a prefix",
159+
hex(currentPayloadOffset, currentPayloadOffset + 1));
161160
}
162161

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

187186
private String hex(final long start, final long taintedEnd) {
188187
final long end = Math.min(taintedEnd, size);
189-
final long size = end - start;
190-
if (size < 10) {
191-
return inputHex(start, Math.toIntExact(size));
188+
final long length = end - start;
189+
if (length < 10) {
190+
return inputHex(start, Math.toIntExact(length));
192191
} else {
193192
return String.format("%s...%s", inputHex(start, 4), inputHex(end - 4, 4));
194193
}
@@ -245,6 +244,9 @@ private void checkElt(final String what) {
245244
if (currentItem >= size) {
246245
throw error("Cannot read a %s, input is fully consumed", what);
247246
}
247+
if (depth > 0 && currentPayloadOffset + currentPayloadSize > endOfListOffset[depth - 1]) {
248+
throw error("Cannot read a %s, too large for enclosing list", what);
249+
}
248250
if (isEndOfCurrentList()) {
249251
throw error("Cannot read a %s, reached end of current list", what);
250252
}

0 commit comments

Comments
 (0)