Skip to content

Commit

Permalink
refactor(rpc): clean-up error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
jurajpiar committed Apr 18, 2024
1 parent 75be73c commit 8fd8de6
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 111 deletions.
40 changes: 30 additions & 10 deletions rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,14 @@
import co.rsk.trie.TrieStoreImpl;
import co.rsk.util.HexUtils;
import com.google.common.annotations.VisibleForTesting;
import org.ethereum.core.*;
import org.apache.commons.lang3.tuple.Pair;
import org.ethereum.core.Block;
import org.ethereum.core.Blockchain;
import org.ethereum.core.CallTransaction;
import org.ethereum.core.Repository;
import org.ethereum.core.Transaction;
import org.ethereum.core.TransactionExecutor;
import org.ethereum.core.TransactionPool;
import org.ethereum.datasource.HashMapDB;
import org.ethereum.db.MutableRepository;
import org.ethereum.rpc.CallArguments;
Expand All @@ -51,7 +58,9 @@

import javax.annotation.Nonnull;
import java.io.IOException;
import java.util.*;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

import static java.util.Arrays.copyOfRange;
import static org.ethereum.rpc.exception.RskJsonRpcRequestException.invalidParamError;
Expand Down Expand Up @@ -138,13 +147,24 @@ public String call(CallArgumentsParam argsParam, BlockIdentifierParam bnOrId) {
res = callConstant(args, block);
}
if (res.isRevert()) {
Pair<String, byte[]> programRevert = decodeProgramRevert(res);
String revertReason = programRevert.getLeft();
byte[] revertData = programRevert.getRight();
if (revertData == null) {
throw RskJsonRpcRequestException.transactionRevertedExecutionError();
}

if (revertReason == null) {
throw RskJsonRpcRequestException.transactionRevertedExecutionError(revertData);
}

throw RskJsonRpcRequestException.transactionRevertedExecutionError(decodeProgramRevert(res));
throw RskJsonRpcRequestException.transactionRevertedExecutionError(revertReason, revertData);
}
hReturn = HexUtils.toUnformattedJsonHex(res.getHReturn());

return hReturn;
} finally {
}
finally {
LOGGER.debug("eth_call(): {}", hReturn);
}
}
Expand Down Expand Up @@ -296,26 +316,26 @@ public ProgramResult callConstant(CallArguments args, Block executionBlock) {
* @param programResult contains the result of execution of a smart contract
* @return revert reason and revert data. reason may be nullable or empty
*/
public static ProgramRevert decodeProgramRevert(ProgramResult programResult) {
public static Pair<String, byte[]> decodeProgramRevert(ProgramResult programResult) {
byte[] bytes = programResult.getHReturn();
if (bytes.length < 4) {
if (bytes == null || bytes.length < 4) {

return new ProgramRevert(bytes);
return Pair.of(null, null);
}

final byte[] signature = copyOfRange(bytes, 0, 4);
if (!Arrays.equals(signature, ERROR_ABI_FUNCTION_SIGNATURE)) {

return new ProgramRevert(bytes);
return Pair.of(null, bytes);
}

final Object[] decode = ERROR_ABI_FUNCTION.decode(bytes);
if (decode == null || decode.length == 0) {

return new ProgramRevert(bytes);
return Pair.of(null, bytes);
}

return new ProgramRevert((String) decode[0], bytes);
return Pair.of((String) decode[0], bytes);
}

private ProgramResult callConstantWithState(CallArguments args, Block executionBlock, Trie state) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@
import co.rsk.mine.MinerServer;
import co.rsk.net.TransactionGateway;
import co.rsk.util.HexUtils;
import org.apache.commons.lang3.tuple.Pair;
import org.ethereum.config.Constants;
import org.ethereum.core.Blockchain;
import org.ethereum.core.TransactionPool;
import org.ethereum.db.TransactionInfo;
import org.ethereum.rpc.exception.RskJsonRpcRequestException;
import org.ethereum.rpc.parameters.CallArgumentsParam;
import org.ethereum.rpc.parameters.HexDataParam;
import org.ethereum.vm.program.ProgramResult;
Expand Down Expand Up @@ -111,7 +113,18 @@ private String getReturnMessage(String txHash) {
ProgramResult programResult = this.blockExecutor.getProgramResult(hash);

if (programResult != null && programResult.isRevert()) {
throw transactionRevertedExecutionError(EthModule.decodeProgramRevert(programResult));
Pair<String, byte[]> programRevert = EthModule.decodeProgramRevert(programResult);
String revertReason = programRevert.getLeft();
byte[] revertData = programRevert.getRight();
if (revertData == null) {
throw RskJsonRpcRequestException.transactionRevertedExecutionError();
}

if (revertReason == null) {
throw RskJsonRpcRequestException.transactionRevertedExecutionError(revertData);
}

throw RskJsonRpcRequestException.transactionRevertedExecutionError(revertReason, revertData);
}

if (!transactionInfo.getReceipt().isSuccessful()) {
Expand Down
47 changes: 0 additions & 47 deletions rskj-core/src/main/java/co/rsk/rpc/modules/eth/ProgramRevert.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import co.rsk.core.exception.InvalidRskAddressException;
import co.rsk.jsonrpc.JsonRpcError;
import co.rsk.util.HexUtils;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.exc.InvalidFormatException;
Expand All @@ -14,8 +15,6 @@
import java.util.Iterator;
import java.util.List;

import static javax.xml.bind.DatatypeConverter.printHexBinary;

/**
* Created by mario on 17/10/2016.
*/
Expand All @@ -35,7 +34,7 @@ public JsonError resolveError(Throwable t, Method method, List<JsonNode> argumen
} else if (t instanceof RskJsonRpcRequestException) {
RskJsonRpcRequestException rskJsonRpcRequestException = (RskJsonRpcRequestException) t;
byte[] revertData = rskJsonRpcRequestException.getRevertData();
String errorDataHexString = "0x" + printHexBinary(revertData == null ? new byte[]{} : revertData).toLowerCase();
String errorDataHexString = revertData == null ? null : HexUtils.toUnformattedJsonHex(revertData);
error = new JsonError(rskJsonRpcRequestException.getCode(), t.getMessage(), errorDataHexString);
} else if (t instanceof InvalidFormatException) {
error = new JsonError(JsonRpcError.INTERNAL_ERROR, "Internal server error, probably due to invalid parameter type", null);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
package org.ethereum.rpc.exception;

import co.rsk.rpc.modules.eth.ProgramRevert;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

public class RskJsonRpcRequestException extends RuntimeException {

private final Integer code;

@Nullable
private final byte[] revertData;

protected RskJsonRpcRequestException(Integer code, byte[] revertData, String message, Exception e) {
protected RskJsonRpcRequestException(Integer code, @Nullable byte[] revertData, String message, Exception e) {
super(message, e);
this.code = code;
this.revertData = revertData;
Expand All @@ -20,7 +19,7 @@ protected RskJsonRpcRequestException(Integer code, String message, Exception e)
this(code, new byte[]{}, message, e);
}

public RskJsonRpcRequestException(Integer code, byte[] revertData, String message) {
public RskJsonRpcRequestException(Integer code, @Nullable byte[] revertData, String message) {
super(message);
this.code = code;
this.revertData = revertData;
Expand All @@ -34,24 +33,28 @@ public Integer getCode() {
return code;
}

@Nullable
public byte[] getRevertData() {
return revertData;
}

public static RskJsonRpcRequestException transactionRevertedExecutionError(ProgramRevert programRevert) {
byte[] revertData = programRevert.getData();
String revertReason = programRevert.getReason();
if (revertReason.isEmpty()) {

return executionError("transaction reverted, no reason specified", revertData);
}
public static RskJsonRpcRequestException transactionRevertedExecutionError() {
return executionError("transaction reverted", null);
}

return executionError("revert " + revertReason, revertData);
public static RskJsonRpcRequestException transactionRevertedExecutionError(@Nonnull byte[] revertData) {
return executionError("transaction reverted", revertData);
}

public static RskJsonRpcRequestException transactionRevertedExecutionError() {
return executionError("transaction reverted", null);
public static RskJsonRpcRequestException transactionRevertedExecutionError(
@Nonnull String revertReason,
@Nonnull byte[] revertData
) {
return executionError(
revertReason.isEmpty()
? "transaction reverted, no reason specified"
: "revert " + revertReason,
revertData
);
}

public static RskJsonRpcRequestException unknownError(String message) {
Expand Down
13 changes: 11 additions & 2 deletions rskj-core/src/test/java/co/rsk/rpc/modules/eth/EthModuleTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -189,10 +189,19 @@ void test_revertedTransaction() {
config.getGasEstimationCap(),
config.getCallGasCap());

RskJsonRpcRequestException exception = assertThrows(RskJsonRpcRequestException.class, () -> eth.call(TransactionFactoryHelper.toCallArgumentsParam(args), new BlockIdentifierParam("latest")));
BlockIdentifierParam blockIdentifierParam = new BlockIdentifierParam("latest");

CallArgumentsParam callArgumentsParam = TransactionFactoryHelper.toCallArgumentsParam(args);
RskJsonRpcRequestException exception = assertThrows(
RskJsonRpcRequestException.class,
() -> eth.call(
callArgumentsParam,
blockIdentifierParam
)
);
assertThat(exception.getMessage(), Matchers.containsString("deposit too big"));
assertNotNull(exception.getRevertData());
assertEquals(hReturn, exception.getRevertData());
assertArrayEquals(hReturn, exception.getRevertData());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import co.rsk.rpc.ModuleDescription;
import co.rsk.rpc.exception.JsonRpcResponseLimitError;
import co.rsk.rpc.exception.JsonRpcTimeoutError;
import co.rsk.rpc.modules.eth.ProgramRevert;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand All @@ -34,7 +33,6 @@
import org.ethereum.rpc.parameters.CallArgumentsParam;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;

import java.util.ArrayList;
Expand All @@ -46,7 +44,11 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.*;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@ExtendWith(MockitoExtension.class)
class JsonRpcCustomServerTest {
Expand Down Expand Up @@ -93,7 +95,7 @@ void testHandleJsonNodeRequest_WithMethodModule() throws Exception {
String response = "test_method_response";

when(handler.test_second("param", "param2")).thenAnswer(invocation -> {
waitFor(150);
waitFor(150);
return response;
});

Expand Down Expand Up @@ -246,11 +248,8 @@ void failedEthCall_givenRskErrorResolver_shouldThrowWithData() throws JsonProces
JsonNode request = objectMapper.readTree(jsonRequest);

FakeWeb3ForEthCall web3ForEthCall = mock(FakeWeb3ForEthCall.class);
ProgramRevert fakeProgramRevert = new ProgramRevert("deposit too big", revertBytes);
RskJsonRpcRequestException exception = RskJsonRpcRequestException.transactionRevertedExecutionError(fakeProgramRevert);
when(web3ForEthCall.eth_call(any(), any())).then(invocation -> {
throw exception;
});
RskJsonRpcRequestException exception = RskJsonRpcRequestException.transactionRevertedExecutionError("deposit too big", revertBytes);
when(web3ForEthCall.eth_call(any(), any())).thenThrow(exception);
jsonRpcCustomServer = new JsonRpcCustomServer(web3ForEthCall, FakeWeb3ForEthCall.class, modules, objectMapper);
jsonRpcCustomServer.setErrorResolver(new RskErrorResolver());

Expand Down
Loading

0 comments on commit 8fd8de6

Please sign in to comment.