Fix more tests in ContractCallServiceTests #10382

nickeynikolovv · 2025-02-14T11:36:33Z

Description:
This PR fixes some tests in ContractCallServicesTests

made changes to capture the result from handleFailedResult but still throw an exception
Lowered the initial balance of 0.0.2 to avoid FAILED_INVALID because it was set to maximum and wasn't able to receive reward
Fix INSUFFICIENT_TX_FEE

Tests left to be fixed - 4
Related issue(s):

Related to #10087

Checklist

Documented (Code comments, README, etc.)
Tested (unit, integration, etc.)

…iceTests Signed-off-by: Nikolay Nikolov <[email protected]>

Signed-off-by: Nikolay Nikolov <[email protected]>

…ll-services-test

Signed-off-by: Nikolay Nikolov <[email protected]>

codecov · 2025-02-14T12:28:55Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.29%. Comparing base (5393ea9) to head (38a491a).

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #10382      +/-   ##
============================================
+ Coverage     92.19%   92.29%   +0.09%     
+ Complexity     8111     8009     -102     
============================================
  Files           989      971      -18     
  Lines         33758    33509     -249     
  Branches       4268     4247      -21     
============================================
- Hits          31124    30926     -198     
+ Misses         1617     1571      -46     
+ Partials       1017     1012       -5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bilyana-gospodinova · 2025-02-14T12:51:08Z

...irror-web3/src/test/java/com/hedera/mirror/web3/service/AbstractContractCallServiceTest.java

@@ -133,6 +134,20 @@ protected void setup() {
                                treasuryEntity.getCreatedTimestamp(), treasuryEntity.toEntityId()))
                        .balance(treasuryEntity.getBalance()))
                .persist();
+        domainBuilder


This and the next account is needed only only in a couple of tests in ContractCallServiceTest. We don't need to initialize them in every test. We can add them only in the needed tests.

bilyana-gospodinova · 2025-02-14T12:56:39Z

hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractCallService.java

        }
+        if (exceptionToThrow != null) {


You don't need to do this separately. A finally block is guaranteed to execute no matter if an exception is thrown.

Signed-off-by: Nikolay Nikolov <[email protected]>

…ll-services-test # Conflicts: # hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/TransactionExecutionService.java

bilyana-gospodinova · 2025-02-14T20:23:27Z

...ra-mirror-web3/src/main/java/com/hedera/mirror/web3/service/TransactionExecutionService.java

@@ -82,7 +82,8 @@ public HederaEvmTransactionProcessingResult execute(
        TransactionBody transactionBody;
        HederaEvmTransactionProcessingResult result = null;
        if (isContractCreate) {
-            transactionBody = buildContractCreateTransactionBody(params, estimatedGas, maxLifetime);
+            transactionBody = buildContractCreateTransactionBody(
+                    params, estimatedGas, maxLifetime, mirrorNodeEvmProperties.getContractCreateTxFee());


The 4th parameter is always the same value, so it can be used in buildContractCreateTransactionBody() directly.

bilyana-gospodinova · 2025-02-14T20:26:31Z

...irror-web3/src/test/java/com/hedera/mirror/web3/service/AbstractContractCallServiceTest.java

@@ -142,15 +142,21 @@ void cleanup() {

    protected long gasUsedAfterExecution(final ContractExecutionParameters serviceParameters) {
        try {
-            return contractExecutionService.callContract(serviceParameters).getGasUsed();
+            if (mirrorNodeEvmProperties.isModularizedServices()) {


The if and the else are the same statement. Can be simplified.

bilyana-gospodinova · 2025-02-14T20:29:31Z

...mirror-web3/src/main/java/com/hedera/mirror/web3/evm/properties/MirrorNodeEvmProperties.java

@@ -140,6 +140,9 @@ public class MirrorNodeEvmProperties implements EvmProperties {
    @Min(21_000L)
    private long maxGasLimit = 15_000_000L;

+    @Getter
+    private long contractCreateTxFee = 100_000_000L;


Does this really need to be a property? I am not sure if this needs to be configurable. I think a constant in the TransactionExecutionService should be enough.

we don't need it, removed

bilyana-gospodinova · 2025-02-14T20:32:05Z

hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractCallService.java

@@ -152,7 +156,9 @@ private void restoreGasToBucket(HederaEvmTransactionProcessingResult result, lon
        // of the gasLimit value back in the bucket.
        final var gasLimitToRestoreBaseline =
                (long) (Math.floorDiv(gasLimit, gasUnit) * throttleProperties.getGasLimitRefundPercent() / 100f);
-        if (!result.isSuccessful() && gasLimit == result.getGasUsed()) {
+        if (result == null) {


The if and the else-if can be combined since their body is the same.

bilyana-gospodinova · 2025-02-14T20:35:24Z

...ra-mirror-web3/src/main/java/com/hedera/mirror/web3/service/TransactionExecutionService.java

@@ -145,7 +146,7 @@ private HederaEvmTransactionProcessingResult handleFailedResult(
            var detail = maybeDecodeSolidityErrorStringToReadableMessage(errorMessage);
            updateErrorGasUsedMetric(gasUsedCounter, result.gasUsed(), 1);
            if (ContractCallContext.get().getOpcodeTracerOptions() == null) {
-                throw new MirrorEvmTransactionException(status.protoName(), detail, errorMessage.toHexString());
+                throw new MirrorEvmTransactionException(status.protoName(), detail, errorMessage.toHexString(), HederaEvmTransactionProcessingResult.failed(result.gasUsed(), 0L, 0L, Optional.empty(), Optional.empty()));


We already have the Optional.of(errorMessage) - might as well pass it in the constructed result. Can become useful at some point.

bilyana-gospodinova · 2025-02-14T20:38:53Z

...irror-web3/src/test/java/com/hedera/mirror/web3/service/AbstractContractCallServiceTest.java

@@ -116,7 +116,7 @@ protected void setup() {
                .entity()
                .customize(e -> e.id(2L)
                        .num(2L)
-                        .balance(5000000000000000000L)
+                        .balance(1000000000000000000L)


We can add a comment here - to say that if a balance is set near the max value, some unexpected errors occur as the balance overflows the max value if we use the same account for the node operator. This might save us some headaches in the future to avoid the same issue.

Signed-off-by: Nikolay Nikolov <[email protected]>

sonarqubecloud · 2025-02-15T07:50:31Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

nickeynikolovv added 3 commits February 13, 2025 11:16

fix estimateGasForDirectCreateContractDeploy test in ContractCallServ…

5679f08

…iceTests Signed-off-by: Nikolay Nikolov <[email protected]>

fixes in handleFailedResult to get gasUsed

93f2ab5

Signed-off-by: Nikolay Nikolov <[email protected]>

some cleanup

822a38a

Signed-off-by: Nikolay Nikolov <[email protected]>

nickeynikolovv added enhancement Type: New feature web3 Area: Web3 API modularizedEVM labels Feb 14, 2025

nickeynikolovv requested a review from a team February 14, 2025 11:36

nickeynikolovv self-assigned this Feb 14, 2025

nickeynikolovv changed the title ~~Fix insufficient tx fee issue in contract call services test~~ Fix more tests in ContractCallServiceTests Feb 14, 2025

nickeynikolovv changed the title ~~Fix more tests in ContractCallServiceTests~~ Fix more tests in ContractCallServiceTests Feb 14, 2025

nickeynikolovv added 2 commits February 14, 2025 13:48

Merge branch 'main' into fix-insufficient-tx-fee-issue-in-contract-ca…

000b29c

…ll-services-test

fixing code smells

236181e

Signed-off-by: Nikolay Nikolov <[email protected]>

bilyana-gospodinova reviewed Feb 14, 2025

View reviewed changes

steven-sheehy added this to the 0.124.0 milestone Feb 14, 2025

nickeynikolovv added 2 commits February 14, 2025 18:06

addressed comments

9bf32d1

Signed-off-by: Nikolay Nikolov <[email protected]>

Merge branch 'main' into fix-insufficient-tx-fee-issue-in-contract-ca…

e32692b

…ll-services-test # Conflicts: # hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/TransactionExecutionService.java

nickeynikolovv requested a review from bilyana-gospodinova February 14, 2025 16:23

bilyana-gospodinova reviewed Feb 14, 2025

View reviewed changes

address some comments

38a491a

Signed-off-by: Nikolay Nikolov <[email protected]>

nickeynikolovv requested a review from bilyana-gospodinova February 15, 2025 07:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix more tests in ContractCallServiceTests #10382

Fix more tests in ContractCallServiceTests #10382

nickeynikolovv commented Feb 14, 2025

codecov bot commented Feb 14, 2025 •

edited

Loading

bilyana-gospodinova Feb 14, 2025

nickeynikolovv Feb 14, 2025

bilyana-gospodinova Feb 14, 2025

nickeynikolovv Feb 14, 2025

bilyana-gospodinova Feb 14, 2025

nickeynikolovv Feb 15, 2025

bilyana-gospodinova Feb 14, 2025

nickeynikolovv Feb 15, 2025

bilyana-gospodinova Feb 14, 2025

nickeynikolovv Feb 15, 2025

bilyana-gospodinova Feb 14, 2025

nickeynikolovv Feb 15, 2025

bilyana-gospodinova Feb 14, 2025

nickeynikolovv Feb 15, 2025

bilyana-gospodinova Feb 14, 2025

nickeynikolovv Feb 15, 2025

sonarqubecloud bot commented Feb 15, 2025

Fix more tests in ContractCallServiceTests #10382

Are you sure you want to change the base?

Fix more tests in ContractCallServiceTests #10382

Conversation

nickeynikolovv commented Feb 14, 2025

codecov bot commented Feb 14, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Feb 15, 2025

Quality Gate passed

codecov bot commented Feb 14, 2025 •

edited

Loading