From d114998f0bfb57b69bed5fb5f1cc537ea0cc6044 Mon Sep 17 00:00:00 2001 From: Volodymyr Kravets Date: Tue, 28 Jan 2025 14:40:58 +0200 Subject: [PATCH 1/2] fix(callTracer): fix missing top-level error field in response --- .../trace/call/CallTraceTransformer.java | 9 ++- .../debug/trace/call/CallTracerTest.java | 43 +++++++++++++-- .../dsl/call_tracer_internal_oog.txt | 55 +++++++++++++++++++ 3 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 rskj-core/src/test/resources/dsl/call_tracer_internal_oog.txt diff --git a/rskj-core/src/main/java/co/rsk/rpc/modules/debug/trace/call/CallTraceTransformer.java b/rskj-core/src/main/java/co/rsk/rpc/modules/debug/trace/call/CallTraceTransformer.java index 67fa1a6232..699a9416c5 100644 --- a/rskj-core/src/main/java/co/rsk/rpc/modules/debug/trace/call/CallTraceTransformer.java +++ b/rskj-core/src/main/java/co/rsk/rpc/modules/debug/trace/call/CallTraceTransformer.java @@ -24,6 +24,7 @@ import co.rsk.rpc.modules.trace.ProgramSubtrace; import co.rsk.rpc.modules.trace.TraceType; import co.rsk.util.HexUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.Pair; import org.bouncycastle.util.encoders.Hex; import org.ethereum.db.TransactionInfo; @@ -53,6 +54,9 @@ public static TransactionTrace toTrace(SummarizedProgramTrace trace, Transaction if (trace.getReverted()) { programResult.setRevert(); } + if (!StringUtils.isEmpty(trace.getError())) { + programResult.setException(new Exception(trace.getError())); + } if (withLog) { programResult.addLogInfos(txInfo.getReceipt().getLogInfoList()); @@ -80,7 +84,7 @@ public static TransactionTrace toTrace(SummarizedProgramTrace trace, Transaction } } - return new TransactionTrace(txInfo.getReceipt().getTransaction().getHash().toHexString(), traceOutput); + return new TransactionTrace(txInfo.getReceipt().getTransaction().getHash().toJsonString(), traceOutput); } private static TxTraceResult toTrace(ProgramSubtrace programSubtrace, boolean withLog) { @@ -152,7 +156,7 @@ private static TxTraceResult toTrace(TraceType traceType, CallType callType, Inv } if (programResult.getException() != null) { - error = programResult.getException().toString(); + error = programResult.getException().getMessage(); } } @@ -160,7 +164,6 @@ private static TxTraceResult toTrace(TraceType traceType, CallType callType, Inv logInfoResultList = getLogs(programResult); } - return TxTraceResult.builder() .type(type) .from(from) diff --git a/rskj-core/src/test/java/co/rsk/rpc/modules/debug/trace/call/CallTracerTest.java b/rskj-core/src/test/java/co/rsk/rpc/modules/debug/trace/call/CallTracerTest.java index 2b7b93c434..1d4fc7b941 100644 --- a/rskj-core/src/test/java/co/rsk/rpc/modules/debug/trace/call/CallTracerTest.java +++ b/rskj-core/src/test/java/co/rsk/rpc/modules/debug/trace/call/CallTracerTest.java @@ -33,14 +33,12 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.*; class CallTracerTest { private final ObjectMapper objectMapper = new ObjectMapper(); - @Test void retrieveSimpleStorageContractCreationTrace() throws Exception { DslParser parser = DslParser.fromResource("dsl/simple_storage.txt"); @@ -49,7 +47,6 @@ void retrieveSimpleStorageContractCreationTrace() throws Exception { ExecutionBlockRetriever executionBlockRetriever = Mockito.mock(ExecutionBlockRetriever.class); Web3InformationRetriever web3InformationRetriever = new Web3InformationRetriever(world.getTransactionPool(), world.getBlockChain(), world.getRepositoryLocator(), executionBlockRetriever); - WorldDslProcessor processor = new WorldDslProcessor(world); processor.processCommands(parser); @@ -69,7 +66,43 @@ void retrieveSimpleStorageContractCreationTrace() throws Exception { assertEquals("CREATE", traceResult.getType()); assertEquals("0x608060405234801561000f575f80fd5b506101438061001d5f395ff3fe608060405234801561000f575f80fd5b5060043610610034575f3560e01c80636057361d146100385780636d4ce63c14610054575b5f80fd5b610052600480360381019061004d91906100ba565b610072565b005b61005c61007b565b60405161006991906100f4565b60405180910390f35b805f8190555050565b5f8054905090565b5f80fd5b5f819050919050565b61009981610087565b81146100a3575f80fd5b50565b5f813590506100b481610090565b92915050565b5f602082840312156100cf576100ce610083565b5b5f6100dc848285016100a6565b91505092915050565b6100ee81610087565b82525050565b5f6020820190506101075f8301846100e5565b9291505056fea2646970667358221220271ba6597ab51821beed677d25c76e319892db25c1f66b3dc76e547fdc1fd0e164736f6c63430008140033", traceResult.getInput()); assertEquals(account.getAddress().toJsonString(), traceResult.getFrom()); + } + + @Test + void internalOoGErrorTrace() throws Exception { + DslParser parser = DslParser.fromResource("dsl/call_tracer_internal_oog.txt"); + ReceiptStore receiptStore = new ReceiptStoreImpl(new HashMapDB()); + World world = new World(receiptStore); + ExecutionBlockRetriever executionBlockRetriever = Mockito.mock(ExecutionBlockRetriever.class); + Web3InformationRetriever web3InformationRetriever = new Web3InformationRetriever(world.getTransactionPool(), world.getBlockChain(), world.getRepositoryLocator(), executionBlockRetriever); + + WorldDslProcessor processor = new WorldDslProcessor(world); + processor.processCommands(parser); + + TransactionReceipt contractTransactionReceipt = world.getTransactionReceiptByName("tx02"); + CallTracer callTracer = new CallTracer(world.getBlockStore(), world.getBlockExecutor(), web3InformationRetriever, receiptStore, world.getBlockChain()); + + JsonNode result = callTracer.traceTransaction(contractTransactionReceipt.getTransaction().getHash().toJsonString(), new TraceOptions()); + + assertNotNull(result); + + TxTraceResult traceResult = objectMapper.treeToValue(result, TxTraceResult.class); + + assertNotNull(traceResult); + + assertError(traceResult); + + assertFalse(traceResult.getCalls().isEmpty()); + assertError(traceResult.getCalls().get(0)); + + assertFalse(traceResult.getCalls().get(0).getCalls().isEmpty()); + assertError(traceResult.getCalls().get(0).getCalls().get(0)); } -} \ No newline at end of file + private static void assertError(TxTraceResult trace) { + String error = trace.getError(); + assertNotNull(error); + assertTrue(error.contains("Not enough gas for")); + } +} diff --git a/rskj-core/src/test/resources/dsl/call_tracer_internal_oog.txt b/rskj-core/src/test/resources/dsl/call_tracer_internal_oog.txt new file mode 100644 index 0000000000..c1d6163bed --- /dev/null +++ b/rskj-core/src/test/resources/dsl/call_tracer_internal_oog.txt @@ -0,0 +1,55 @@ +comment + +pragma solidity ^0.8.25; + +contract NestedCallOoG { + + function checkOoG() public view { + this.callNested(); + } + + function callNested() external view { + this.consumeAllGas(); + } + + function consumeAllGas() external pure { + while (true) { + // This loop will run indefinitely until all gas is consumed + } + } +} + +end + +account_new acc1 10000000 + +# deploy NestedCallOoG contract +transaction_build tx01 + sender acc1 + receiverAddress 00 + value 0 + data 6080604052348015600e575f80fd5b506101588061001c5f395ff3fe608060405234801561000f575f80fd5b506004361061003f575f3560e01c80631c65505a14610043578063b8a5ffe51461004d578063e543085114610057575b5f80fd5b61004b610061565b005b6100556100bd565b005b61005f610119565b005b3073ffffffffffffffffffffffffffffffffffffffff1663b8a5ffe56040518163ffffffff1660e01b81526004015f6040518083038186803b1580156100a5575f80fd5b505afa1580156100b7573d5f803e3d5ffd5b50505050565b3073ffffffffffffffffffffffffffffffffffffffff1663e54308516040518163ffffffff1660e01b81526004015f6040518083038186803b158015610101575f80fd5b505afa158015610113573d5f803e3d5ffd5b50505050565b5b600161011a5756fea264697066735822122049d6d5dcd90a95239b39979726f88d35071e45c414f56da3d326b25da49349d564736f6c63430008190033 + gas 1000000 + build + +# make checkOoG() call +transaction_build tx02 + sender acc1 + receiverAddress 6252703f5ba322ec64d3ac45e56241b7d9e481ad + value 0 + data 1c65505a + gas 100000 + nonce 1 + build + +block_build b01 + parent g00 + gasLimit 7500000 + transactions tx01 tx02 + build + +block_connect b01 + +# Assert best block +assert_best b01 + From 1fbc3b1baef9b5adb49ad72c6875e8abea11925e Mon Sep 17 00:00:00 2001 From: Volodymyr Kravets Date: Wed, 29 Jan 2025 12:14:55 +0200 Subject: [PATCH 2/2] fix(callTracer): address pr comments and sonar complaints --- .../trace/call/CallTraceTransformer.java | 129 ++++++++---------- .../debug/trace/call/CallTracerTest.java | 8 +- 2 files changed, 62 insertions(+), 75 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/rpc/modules/debug/trace/call/CallTraceTransformer.java b/rskj-core/src/main/java/co/rsk/rpc/modules/debug/trace/call/CallTraceTransformer.java index 699a9416c5..6826952673 100644 --- a/rskj-core/src/main/java/co/rsk/rpc/modules/debug/trace/call/CallTraceTransformer.java +++ b/rskj-core/src/main/java/co/rsk/rpc/modules/debug/trace/call/CallTraceTransformer.java @@ -97,87 +97,27 @@ private static TxTraceResult toTrace(ProgramSubtrace programSubtrace, boolean wi } private static TxTraceResult toTrace(TraceType traceType, CallType callType, InvokeData invoke, DataWord codeAddress, ProgramResult programResult, CreationData creationData, boolean withLog) { - String type = traceType == TraceType.CREATE ? "CREATE" : callType.name(); - String from; - String to = null; - String gas = null; - String input = null; - String value = null; - String output = null; - String gasUsed = null; - String revertReason = null; - String error = null; - - - from = getFrom(callType, invoke); - - List logInfoResultList = null; - - DataWord callValue = invoke.getCallValue(); + TxTraceResult.Builder builder = TxTraceResult.builder(); + builder.type(traceType == TraceType.CREATE ? "CREATE" : callType.name()); + builder.from(getFrom(callType, invoke)); if (traceType == TraceType.CREATE) { - if (creationData != null) { - input = HexUtils.toUnformattedJsonHex(creationData.getCreationInput()); - output = creationData.getCreatedAddress().toJsonString(); - } - value = HexUtils.toQuantityJsonHex(callValue.getData()); - gas = HexUtils.toQuantityJsonHex(invoke.getGas()); - } - - if (traceType == TraceType.CALL) { - input = HexUtils.toUnformattedJsonHex(invoke.getDataCopy(DataWord.ZERO, invoke.getDataSize())); - value = HexUtils.toQuantityJsonHex(callValue.getData()); - - if (callType == CallType.DELEGATECALL) { - // The code address should not be null in a DELEGATECALL case - // but handling the case here - if (codeAddress != null) { - to = new RskAddress(codeAddress.getLast20Bytes()).toJsonString(); - } - } else { - to = new RskAddress(invoke.getOwnerAddress().getLast20Bytes()).toJsonString(); - } - - gas = HexUtils.toQuantityJsonHex(invoke.getGas()); + buildForCreate(builder, invoke, creationData); + } else if (traceType == TraceType.CALL) { + buildForCall(builder, invoke, callType, codeAddress); } - if (programResult != null) { - gasUsed = HexUtils.toQuantityJsonHex(programResult.getGasUsed()); - - if (programResult.isRevert()) { - Pair programRevert = EthModule.decodeProgramRevert(programResult); - revertReason = programRevert.getLeft(); - output = HexUtils.toQuantityJsonHex(programRevert.getRight()); - error = "execution reverted"; - } else if (traceType != TraceType.CREATE) { - output = HexUtils.toQuantityJsonHex(programResult.getHReturn()); - } - - if (programResult.getException() != null) { - error = programResult.getException().getMessage(); - } + buildForProgramResult(builder, traceType, programResult); } if (withLog) { - logInfoResultList = getLogs(programResult); - } - - return TxTraceResult.builder() - .type(type) - .from(from) - .to(to) - .value(value) - .gas(gas) - .input(input) - .gasUsed(gasUsed) - .output(output) - .revertReason(revertReason) - .error(error) - .logs(logInfoResultList) - .build(); + List logInfoResultList = getLogs(programResult); + builder.logs(logInfoResultList); + } + return builder.build(); } private static String getFrom(CallType callType, InvokeData invoke) { @@ -188,6 +128,53 @@ private static String getFrom(CallType callType, InvokeData invoke) { } } + private static void buildForCreate(TxTraceResult.Builder builder, InvokeData invoke, CreationData creationData) { + DataWord callValue = invoke.getCallValue(); + + if (creationData != null) { + builder.input(HexUtils.toUnformattedJsonHex(creationData.getCreationInput())); + builder.output(creationData.getCreatedAddress().toJsonString()); + } + builder.value(HexUtils.toQuantityJsonHex(callValue.getData())); + builder.gas(HexUtils.toQuantityJsonHex(invoke.getGas())); + } + + private static void buildForCall(TxTraceResult.Builder builder, InvokeData invoke, CallType callType, DataWord codeAddress) { + DataWord callValue = invoke.getCallValue(); + + builder.input(HexUtils.toUnformattedJsonHex(invoke.getDataCopy(DataWord.ZERO, invoke.getDataSize()))); + builder.value(HexUtils.toQuantityJsonHex(callValue.getData())); + + if (callType == CallType.DELEGATECALL) { + // The code address should not be null in a DELEGATECALL case + // but handling the case here + if (codeAddress != null) { + builder.to(new RskAddress(codeAddress.getLast20Bytes()).toJsonString()); + } + } else { + builder.to(new RskAddress(invoke.getOwnerAddress().getLast20Bytes()).toJsonString()); + } + + builder.gas(HexUtils.toQuantityJsonHex(invoke.getGas())); + } + + private static void buildForProgramResult(TxTraceResult.Builder builder, TraceType traceType, ProgramResult programResult) { + builder.gasUsed(HexUtils.toQuantityJsonHex(programResult.getGasUsed())); + + if (programResult.isRevert()) { + Pair programRevert = EthModule.decodeProgramRevert(programResult); + builder.revertReason(programRevert.getLeft()); + builder.output(HexUtils.toQuantityJsonHex(programRevert.getRight())); + builder.error("execution reverted"); + } else if (traceType != TraceType.CREATE) { + builder.output(HexUtils.toQuantityJsonHex(programResult.getHReturn())); + } + + if (programResult.getException() != null) { + builder.error(programResult.getException().getMessage()); + } + } + private static List getLogs(ProgramResult programResult) { if (programResult == null) { return Collections.emptyList(); diff --git a/rskj-core/src/test/java/co/rsk/rpc/modules/debug/trace/call/CallTracerTest.java b/rskj-core/src/test/java/co/rsk/rpc/modules/debug/trace/call/CallTracerTest.java index 1d4fc7b941..1d68c6ffa8 100644 --- a/rskj-core/src/test/java/co/rsk/rpc/modules/debug/trace/call/CallTracerTest.java +++ b/rskj-core/src/test/java/co/rsk/rpc/modules/debug/trace/call/CallTracerTest.java @@ -91,16 +91,16 @@ void internalOoGErrorTrace() throws Exception { assertNotNull(traceResult); - assertError(traceResult); + assertOOGError(traceResult); assertFalse(traceResult.getCalls().isEmpty()); - assertError(traceResult.getCalls().get(0)); + assertOOGError(traceResult.getCalls().get(0)); assertFalse(traceResult.getCalls().get(0).getCalls().isEmpty()); - assertError(traceResult.getCalls().get(0).getCalls().get(0)); + assertOOGError(traceResult.getCalls().get(0).getCalls().get(0)); } - private static void assertError(TxTraceResult trace) { + private static void assertOOGError(TxTraceResult trace) { String error = trace.getError(); assertNotNull(error); assertTrue(error.contains("Not enough gas for"));