Skip to content

Commit

Permalink
Show correct revert reason data in priv_call (hyperledger#5369)
Browse files Browse the repository at this point in the history 
* added revert reason in priv_call

---------

Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
  • Loading branch information
@NickSneo @macfarla
NickSneo and macfarla authored Apr 20, 2023
1 parent 25ab211 commit 839bddc
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
- Renamed --bonsai-maximum-back-layers-to-load option to --bonsai-historical-block-limit for clarity. Removed --Xbonsai-use-snapshots option as it is no longer functional [#5337](https://github.com/hyperledger/besu/pull/5337)
- Change Forest to use TransactionDB instead of OptimisticTransactionDB [#5328](https://github.com/hyperledger/besu/pull/5328)
- Performance: Reduced usage of UInt256 in EVM operations [#5331](https://github.com/hyperledger/besu/pull/5331)
- Changed wrong error message "Invalid params" when private tx is reverted to "Execution reverted" with correct revert reason in data. [#5369](https://github.com/hyperledger/besu/pull/5369)

### Bug Fixes

Expand Down
68 changes: 57 additions & 11 deletions ...s/src/test/java/org/hyperledger/besu/tests/acceptance/privacy/PrivCallAcceptanceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,31 @@
package org.hyperledger.besu.tests.acceptance.privacy;

import static org.assertj.core.api.Assertions.assertThat;
import static org.hyperledger.besu.tests.web3j.generated.RevertReason.FUNC_REVERTWITHREVERTREASON;
import static org.web3j.utils.Restriction.UNRESTRICTED;

import org.hyperledger.besu.tests.acceptance.dsl.privacy.ParameterizedEnclaveTestBase;
import org.hyperledger.besu.tests.acceptance.dsl.privacy.PrivacyNode;
import org.hyperledger.besu.tests.acceptance.dsl.privacy.account.PrivacyAccountResolver;
import org.hyperledger.besu.tests.web3j.generated.EventEmitter;
import org.hyperledger.besu.tests.web3j.generated.RevertReason;
import org.hyperledger.enclave.testutil.EnclaveEncryptorType;
import org.hyperledger.enclave.testutil.EnclaveType;

import java.io.IOException;
import java.math.BigInteger;
import java.nio.charset.Charset;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import javax.annotation.Nonnull;

import org.bouncycastle.util.encoders.Hex;
import org.junit.Test;
import org.web3j.abi.FunctionEncoder;
import org.web3j.abi.TypeReference;
import org.web3j.abi.datatypes.Bool;
import org.web3j.abi.datatypes.Function;
import org.web3j.abi.datatypes.Type;
import org.web3j.abi.datatypes.generated.Uint256;
Expand Down Expand Up @@ -93,7 +98,8 @@ public void mustReturnCorrectValue() throws Exception {
eventEmitter.getContractAddress(), minerNode.getAddress().toString())
.verify(eventEmitter);

final Request<Object, EthCall> priv_call = privCall(privacyGroupId, eventEmitter, false, false);
final Request<Object, EthCall> priv_call =
privCall(privacyGroupId, eventEmitter, false, false, false);

EthCall resp = priv_call.send();

Expand All @@ -109,6 +115,38 @@ public void mustReturnCorrectValue() throws Exception {
.isEqualByComparingTo(BigInteger.valueOf(VALUE));
}

@Test
public void mustRevertWithRevertReason() throws Exception {

final String privacyGroupId =
minerNode.execute(createPrivacyGroup("myGroupName", "my group description", minerNode));

final RevertReason revertReasonContract =
minerNode.execute(
privateContractTransactions.createSmartContractWithPrivacyGroupId(
RevertReason.class,
minerNode.getTransactionSigningKey(),
restriction,
minerNode.getEnclaveKey(),
privacyGroupId));

privateContractVerifier
.validPrivateContractDeployed(
revertReasonContract.getContractAddress(), minerNode.getAddress().toString())
.verify(revertReasonContract);

final Request<Object, EthCall> priv_call =
privCall(privacyGroupId, revertReasonContract, false, false, true);

EthCall resp = priv_call.send();
assertThat(resp.getRevertReason()).isEqualTo("Execution reverted");

byte[] bytes = Hex.decode(resp.getError().getData().substring(3, 203));
String revertMessage =
new String(Arrays.copyOfRange(bytes, 4, bytes.length), Charset.defaultCharset()).trim();
assertThat(revertMessage).isEqualTo("RevertReason");
}

@Test
public void shouldReturnEmptyResultWithNonExistingPrivacyGroup() throws IOException {

Expand All @@ -131,7 +169,7 @@ public void shouldReturnEmptyResultWithNonExistingPrivacyGroup() throws IOExcept

final String invalidPrivacyGroup = constructInvalidString(privacyGroupId);
final Request<Object, EthCall> privCall =
privCall(invalidPrivacyGroup, eventEmitter, false, false);
privCall(invalidPrivacyGroup, eventEmitter, false, false, false);

final EthCall result = privCall.send();

Expand All @@ -158,7 +196,8 @@ public void mustNotSucceedWithWronglyEncodedFunction() throws IOException {
eventEmitter.getContractAddress(), minerNode.getAddress().toString())
.verify(eventEmitter);

final Request<Object, EthCall> priv_call = privCall(privacyGroupId, eventEmitter, true, false);
final Request<Object, EthCall> priv_call =
privCall(privacyGroupId, eventEmitter, true, false, false);

final String errorMessage = priv_call.send().getError().getMessage();
assertThat(errorMessage).isEqualTo("Invalid params");
Expand All @@ -184,7 +223,8 @@ public void mustReturn0xUsingInvalidContractAddress() throws IOException {
eventEmitter.getContractAddress(), minerNode.getAddress().toString())
.verify(eventEmitter);

final Request<Object, EthCall> priv_call = privCall(privacyGroupId, eventEmitter, false, true);
final Request<Object, EthCall> priv_call =
privCall(privacyGroupId, eventEmitter, false, true, false);

final EthCall result = priv_call.send();

Expand All @@ -206,9 +246,10 @@ private String constructInvalidString(final String privacyGroupId) {
@Nonnull
private Request<Object, EthCall> privCall(
final String privacyGroupId,
final Contract eventEmitter,
final Contract contract,
final boolean useInvalidParameters,
final boolean useInvalidContractAddress) {
final boolean useInvalidContractAddress,
final boolean useRevertFunction) {

final Uint256 invalid = new Uint256(BigInteger.TEN);

Expand All @@ -217,10 +258,15 @@ private Request<Object, EthCall> privCall(
useInvalidParameters ? Arrays.asList(invalid) : Collections.emptyList();

final Function function =
new Function(
"value",
inputParameters,
Arrays.<TypeReference<?>>asList(new TypeReference<Uint256>() {}));
useRevertFunction
? new Function(
FUNC_REVERTWITHREVERTREASON,
inputParameters,
Arrays.<TypeReference<?>>asList(new TypeReference<Bool>() {}))
: new Function(
"value",
inputParameters,
Arrays.<TypeReference<?>>asList(new TypeReference<Uint256>() {}));

final String encoded = FunctionEncoder.encode(function);

Expand All @@ -231,7 +277,7 @@ private Request<Object, EthCall> privCall(
+ ":"
+ minerNode.getBesu().getJsonRpcPort().get());

final String validContractAddress = eventEmitter.getContractAddress();
final String validContractAddress = contract.getContractAddress();
final String invalidContractAddress = constructInvalidString(validContractAddress);
final String contractAddress =
useInvalidContractAddress ? invalidContractAddress : validContractAddress;
Expand Down
19 changes: 17 additions & 2 deletions ...ava/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/PrivCall.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.BlockParameter;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.JsonCallParameter;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.privacy.methods.PrivacyIdProvider;
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;
import org.hyperledger.besu.ethereum.api.query.BlockchainQueries;
import org.hyperledger.besu.ethereum.privacy.PrivacyController;
import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult;
import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason;

public class PrivCall extends AbstractBlockParameterMethod {

Expand Down Expand Up @@ -72,8 +75,7 @@ protected Object resultByBlockNumber(
request.getRequest().getId(), result.getOutput().toString())),
reason ->
new JsonRpcErrorResponse(
request.getRequest().getId(),
JsonRpcErrorConverter.convertTransactionInvalidReason(reason))))
request.getRequest().getId(), errorResponse(result, reason))))
.orElse(validRequestBlockNotFound(request));
}

Expand All @@ -86,6 +88,19 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
return (JsonRpcResponse) findResultByParamType(requestContext);
}

private JsonRpcError errorResponse(
final TransactionProcessingResult result, final TransactionInvalidReason reason) {
final JsonRpcError jsonRpcError;
if (result.getRevertReason().isPresent() && result.getRevertReason().get().size() >= 4) {
jsonRpcError = JsonRpcError.REVERT_ERROR;
jsonRpcError.setData(result.getRevertReason().get().toHexString());
} else {
jsonRpcError = JsonRpcErrorConverter.convertTransactionInvalidReason(reason);
}

return jsonRpcError;
}

private JsonCallParameter validateAndGetCallParams(final JsonRpcRequestContext request) {
final JsonCallParameter callParams = request.getRequiredParameter(1, JsonCallParameter.class);
if (callParams.getTo() == null) {
Expand Down

0 comments on commit 839bddc

Please sign in to comment.