Skip to content

Commit

Permalink
Update change log, add unit tests
Browse files Browse the repository at this point in the history 
Signed-off-by: Matthew Whitehead <[email protected]>
  • Loading branch information
@matthew1001
matthew1001 committed Jul 17, 2023
1 parent 734c3fa commit b369fd8
Show file tree
Hide file tree
Showing 4 changed files with 202 additions and 23 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
### Breaking Changes

- Removed support for version 0 of the database as it is no longer used by any active node.
- Add ABI-decoded revert reason to `eth_call` and `eth_estimateGas` responses [#5705](https://github.com/hyperledger/besu/issues/5705)

### Additions and Improvements
- EvmTool now executes the `execution-spec-tests` via the `t8n` and `b11r`. See the [README](ethereum/evmtool/README.md) in EvmTool for more instructions.
Expand Down
19 changes: 12 additions & 7 deletions ...ava/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/JsonRpcErrorResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,19 @@ public static Optional<String> decodeRevertReason(final Bytes revertReason) {
final String encodedReasonText =
revertReason.toHexString().substring(errorMethodABI.length());

List<TypeReference<Type>> revertReasonTypes =
Collections.singletonList(TypeReference.create((Class<Type>) AbiTypes.getType("string")));
List<Type> decoded = FunctionReturnDecoder.decode(encodedReasonText, revertReasonTypes);
try {
List<TypeReference<Type>> revertReasonTypes =
Collections.singletonList(
TypeReference.create((Class<Type>) AbiTypes.getType("string")));
List<Type> decoded = FunctionReturnDecoder.decode(encodedReasonText, revertReasonTypes);

// Expect a single decoded string
if (decoded.size() == 1 && (decoded.get(0) instanceof Utf8String)) {
Utf8String decodedRevertReason = (Utf8String) decoded.get(0);
return Optional.of(decodedRevertReason.getValue());
// Expect a single decoded string
if (decoded.size() == 1 && (decoded.get(0) instanceof Utf8String)) {
Utf8String decodedRevertReason = (Utf8String) decoded.get(0);
return Optional.of(decodedRevertReason.getValue());
}
} catch (StringIndexOutOfBoundsException exception) {
return Optional.of("ABI decode error");
}
}
return Optional.empty();
Expand Down
112 changes: 112 additions & 0 deletions ...src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthCallTest.java
View file
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.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError.BLOCK_NOT_FOUND;
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError.INTERNAL_ERROR;
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError.REVERT_ERROR;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
Expand All @@ -32,6 +33,7 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequest;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.JsonCallParameter;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
Expand All @@ -44,6 +46,7 @@
import org.hyperledger.besu.ethereum.mainnet.ImmutableTransactionValidationParams;
import org.hyperledger.besu.ethereum.mainnet.TransactionValidationParams;
import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult;
import org.hyperledger.besu.ethereum.transaction.CallParameter;
import org.hyperledger.besu.ethereum.transaction.PreCloseStateHandler;
import org.hyperledger.besu.ethereum.transaction.TransactionSimulator;
Expand Down Expand Up @@ -161,6 +164,115 @@ public void shouldReturnExecutionResultWhenExecutionIsSuccessful() {
verifyNoMoreInteractions(blockchainQueries);
}

@Test
public void shouldReturnBasicExecutionRevertErrorWithoutReason() {
final JsonRpcRequestContext request = ethCallRequest(callParameter(), "latest");

// Expect a revert error with no decoded reason (error doesn't begin "Error(string)" so ignored)
final JsonRpcError expectedError = REVERT_ERROR;
final String abiHexString = "0x1234";
final JsonRpcResponse expectedResponse = new JsonRpcErrorResponse(null, expectedError);

mockTransactionProcessorSuccessResult(expectedResponse);
when(blockchainQueries.getBlockchain()).thenReturn(blockchain);
when(blockchain.getChainHead()).thenReturn(chainHead);

final BlockHeader blockHeader = mock(BlockHeader.class);
when(blockHeader.getBaseFee()).thenReturn(Optional.of(Wei.ZERO));
when(chainHead.getBlockHeader()).thenReturn(blockHeader);

final JsonRpcResponse response = method.response(request);

final TransactionProcessingResult processingResult =
new TransactionProcessingResult(
null, null, 0, 0, null, null, Optional.of(Bytes.fromHexString(abiHexString)));

final TransactionSimulatorResult result = mock(TransactionSimulatorResult.class);
when(result.isSuccessful()).thenReturn(false);
when(result.getValidationResult()).thenReturn(ValidationResult.valid());
when(result.getResult()).thenReturn(processingResult);
verify(transactionSimulator).process(any(), any(), any(), mapperCaptor.capture(), any());
assertThat(mapperCaptor.getValue().apply(mock(MutableWorldState.class), Optional.of(result)))
.isEqualTo(Optional.of(expectedResponse));

assertThat(response).usingRecursiveComparison().isEqualTo(expectedResponse);
assertThat(((JsonRpcErrorResponse) response).getError().getMessage())
.isEqualTo("Execution reverted");
}

@Test
public void shouldReturnExecutionRevertErrorWithABIParseError() {
final JsonRpcRequestContext request = ethCallRequest(callParameter(), "latest");

// Expect a revert error with no decoded reason (error begins with "Error(string)" but trailing
// bytes are invalid ABI)
final String abiHexString = "0x08c379a002d36d";
final JsonRpcError expectedError = REVERT_ERROR;
final JsonRpcResponse expectedResponse = new JsonRpcErrorResponse(null, expectedError);

mockTransactionProcessorSuccessResult(expectedResponse);
when(blockchainQueries.getBlockchain()).thenReturn(blockchain);
when(blockchain.getChainHead()).thenReturn(chainHead);

final BlockHeader blockHeader = mock(BlockHeader.class);
when(blockHeader.getBaseFee()).thenReturn(Optional.of(Wei.ZERO));
when(chainHead.getBlockHeader()).thenReturn(blockHeader);

final JsonRpcResponse response = method.response(request);
final TransactionProcessingResult processingResult =
new TransactionProcessingResult(
null, null, 0, 0, null, null, Optional.of(Bytes.fromHexString(abiHexString)));

final TransactionSimulatorResult result = mock(TransactionSimulatorResult.class);
when(result.isSuccessful()).thenReturn(false);
when(result.getValidationResult()).thenReturn(ValidationResult.valid());
when(result.getResult()).thenReturn(processingResult);
verify(transactionSimulator).process(any(), any(), any(), mapperCaptor.capture(), any());
assertThat(mapperCaptor.getValue().apply(mock(MutableWorldState.class), Optional.of(result)))
.isEqualTo(Optional.of(expectedResponse));

assertThat(response).usingRecursiveComparison().isEqualTo(expectedResponse);
assertThat(((JsonRpcErrorResponse) response).getError().getMessage())
.isEqualTo("Execution reverted: ABI decode error");
}

@Test
public void shouldReturnExecutionRevertErrorWithParsedABI() {
final JsonRpcRequestContext request = ethCallRequest(callParameter(), "latest");

// Expect a revert error with decoded reason (error begins with "Error(string)", trailing bytes
// = "ERC20: transfer from the zero address")
final String abiHexString =
"0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002545524332303a207472616e736665722066726f6d20746865207a65726f2061646472657373000000000000000000000000000000000000000000000000000000";
final JsonRpcError expectedError = REVERT_ERROR;
final JsonRpcResponse expectedResponse = new JsonRpcErrorResponse(null, expectedError);

mockTransactionProcessorSuccessResult(expectedResponse);
when(blockchainQueries.getBlockchain()).thenReturn(blockchain);
when(blockchain.getChainHead()).thenReturn(chainHead);

final BlockHeader blockHeader = mock(BlockHeader.class);
when(blockHeader.getBaseFee()).thenReturn(Optional.of(Wei.ZERO));
when(chainHead.getBlockHeader()).thenReturn(blockHeader);

final JsonRpcResponse response = method.response(request);
final TransactionProcessingResult processingResult =
new TransactionProcessingResult(
null, null, 0, 0, null, null, Optional.of(Bytes.fromHexString(abiHexString)));

final TransactionSimulatorResult result = mock(TransactionSimulatorResult.class);
when(result.isSuccessful()).thenReturn(false);
when(result.getValidationResult()).thenReturn(ValidationResult.valid());
when(result.getResult()).thenReturn(processingResult);
verify(transactionSimulator).process(any(), any(), any(), mapperCaptor.capture(), any());
assertThat(mapperCaptor.getValue().apply(mock(MutableWorldState.class), Optional.of(result)))
.isEqualTo(Optional.of(expectedResponse));

assertThat(response).usingRecursiveComparison().isEqualTo(expectedResponse);
assertThat(((JsonRpcErrorResponse) response).getError().getMessage())
.isEqualTo("Execution reverted: ERC20: transfer from the zero address");
}

@Test
public void shouldUseCorrectBlockNumberWhenLatest() {
final JsonRpcRequestContext request = ethCallRequest(callParameter(), "latest");
Expand Down
93 changes: 77 additions & 16 deletions ...t/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthEstimateGasTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public void shouldUseGasPriceParameterWhenIsPresent() {
final Wei gasPrice = Wei.of(1000);
final JsonRpcRequestContext request =
ethEstimateGasRequest(defaultLegacyTransactionCallParameter(gasPrice));
mockTransientProcessorResultGasEstimate(1L, true, false, gasPrice);
mockTransientProcessorResultGasEstimate(1L, true, gasPrice, Optional.empty());

final JsonRpcResponse expectedResponse = new JsonRpcSuccessResponse(null, Quantity.create(1L));

Expand Down Expand Up @@ -253,9 +253,60 @@ public void shouldReturnErrorWhenTransactionReverted() {
final JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(null, JsonRpcError.REVERT_ERROR);

Assertions.assertThat(method.response(request))
.usingRecursiveComparison()
.isEqualTo(expectedResponse);
JsonRpcResponse actualResponse = method.response(request);

Assertions.assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);

assertThat(((JsonRpcErrorResponse) actualResponse).getError().getMessage())
.isEqualTo("Execution reverted");
}

@Test
public void shouldReturnErrorReasonWhenTransactionReverted() {
final JsonRpcRequestContext request =
ethEstimateGasRequest(defaultLegacyTransactionCallParameter(Wei.ZERO));

// ABI encoding of EVM "Error(string)" prefix + "ERC20: transfer from the zero address"
final String executionRevertedReason =
"0x08c379a000000000000000000000000000000000000000000000000000000000"
+ "000000200000000000000000000000000000000000000000000000000000000000"
+ "00002545524332303a207472616e736665722066726f6d20746865207a65726f20"
+ "61646472657373000000000000000000000000000000000000000000000000000000";

mockTransientProcessorTxReverted(1L, false, Bytes.fromHexString(executionRevertedReason));

final JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(null, JsonRpcError.REVERT_ERROR);

JsonRpcResponse actualResponse = method.response(request);

Assertions.assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);

assertThat(((JsonRpcErrorResponse) actualResponse).getError().getMessage())
.isEqualTo("Execution reverted: ERC20: transfer from the zero address");
}

@Test
public void shouldReturnABIDecodeErrorReasonWhenInvalidRevertReason() {
final JsonRpcRequestContext request =
ethEstimateGasRequest(defaultLegacyTransactionCallParameter(Wei.ZERO));

// Invalid ABI bytes
final String invalidRevertReason =
"0x08c379a000000000000000000000000000000000000000000000000000000000"
+ "123451234512345123451234512345123451234512345123451234512345123451";

mockTransientProcessorTxReverted(1L, false, Bytes.fromHexString(invalidRevertReason));

final JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(null, JsonRpcError.REVERT_ERROR);

JsonRpcResponse actualResponse = method.response(request);

Assertions.assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);

assertThat(((JsonRpcErrorResponse) actualResponse).getError().getMessage())
.isEqualTo("Execution reverted: ABI decode error");
}

@Test
Expand Down Expand Up @@ -300,28 +351,38 @@ public void shouldNotIgnoreSenderBalanceAccountWhenStrictModeDisabled() {

private void mockTransientProcessorResultTxInvalidReason(final TransactionInvalidReason reason) {
final TransactionSimulatorResult mockTxSimResult =
getMockTransactionSimulatorResult(false, false, 0, Wei.ZERO);
getMockTransactionSimulatorResult(false, 0, Wei.ZERO, Optional.empty());
when(mockTxSimResult.getValidationResult()).thenReturn(ValidationResult.invalid(reason));
}

private void mockTransientProcessorTxReverted(
final long estimateGas, final boolean isSuccessful, final Bytes revertReason) {
mockTransientProcessorResultGasEstimate(
estimateGas, isSuccessful, Wei.ZERO, Optional.of(revertReason));
}

private void mockTransientProcessorResultGasEstimate(
final long estimateGas, final boolean isSuccessful, final boolean isReverted) {
mockTransientProcessorResultGasEstimate(estimateGas, isSuccessful, isReverted, Wei.ZERO);
mockTransientProcessorResultGasEstimate(
estimateGas,
isSuccessful,
Wei.ZERO,
isReverted ? Optional.of(Bytes.of(0)) : Optional.empty());
}

private void mockTransientProcessorResultGasEstimate(
final long estimateGas,
final boolean isSuccessful,
final boolean isReverted,
final Wei gasPrice) {
getMockTransactionSimulatorResult(isSuccessful, isReverted, estimateGas, gasPrice);
final Wei gasPrice,
final Optional<Bytes> revertReason) {
getMockTransactionSimulatorResult(isSuccessful, estimateGas, gasPrice, revertReason);
}

private TransactionSimulatorResult getMockTransactionSimulatorResult(
final boolean isSuccessful,
final boolean isReverted,
final long estimateGas,
final Wei gasPrice) {
final Wei gasPrice,
final Optional<Bytes> revertReason) {
final TransactionSimulatorResult mockTxSimResult = mock(TransactionSimulatorResult.class);
when(transactionSimulator.process(
eq(modifiedLegacyTransactionCallParameter(gasPrice)),
Expand All @@ -335,11 +396,11 @@ private TransactionSimulatorResult getMockTransactionSimulatorResult(
any(OperationTracer.class),
eq(1L)))
.thenReturn(Optional.of(mockTxSimResult));
final TransactionProcessingResult mockResult = mock(TransactionProcessingResult.class);
when(mockResult.getEstimateGasUsedByTransaction()).thenReturn(estimateGas);
when(mockResult.getRevertReason())
.thenReturn(isReverted ? Optional.of(Bytes.of(0)) : Optional.empty());
when(mockTxSimResult.getResult()).thenReturn(mockResult);

final TransactionProcessingResult processingResult =
new TransactionProcessingResult(null, null, estimateGas, 0, null, null, revertReason);

when(mockTxSimResult.getResult()).thenReturn(processingResult);
when(mockTxSimResult.isSuccessful()).thenReturn(isSuccessful);
return mockTxSimResult;
}
Expand Down

0 comments on commit b369fd8

Please sign in to comment.