hedera-services icon indicating copy to clipboard operation
hedera-services copied to clipboard

fix: services bugs found when running ConcurrentSuites.ethereumSuites tests with Ethereum contrext

Open vtronkov opened this issue 10 months ago • 9 comments

Description: These bugs were found as part of https://github.com/hashgraph/hedera-services/pull/12118. In it we are fixing framework bugs and we separated the services bugs we found in this PR

Related issue(s):

Fixes #12917

Notes for reviewer:

Bug 1: EthTxData negative value This bug was found at https://github.com/hashgraph/hedera-services/issues/11730.

The following op in callFailsWhenAmountIsNegativeButStillChargedFee test was failing:

final var subop2 = contractCall(PAY_RECEIVABLE_CONTRACT)
        .via(PAY_TXN)
        .payingWith(payer)
        .sending(-DEPOSIT_AMOUNT)
    .hasKnownStatus(CONTRACT_NEGATIVE_VALUE);

Everything works when we execute the test as is(when we are using Hedera contract create/call), but when we convert the test to use Ethereum create and call - we send -1000 and at the service level we receive 27147. Because of that the CONTRACT_NEGATIVE_VALUE validation is not triggered and the call is successful.

The problem is that the RLPItem.asBigInt doesn't handle negative values very well. The solution is to wrap the bytes in BigInteger directly.

Bug 2: 0x prefix: We have tests the call data is prefixed with 0x. To be consistent we are removing the prefix.

Bug 3: Missed CONTRACT_NEGATIVE_VALUE validation

Checklist

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

vtronkov avatar Apr 16 '24 07:04 vtronkov

Node: HAPI Test (Restart) Results

2 tests   2 :white_check_mark:  5m 22s :stopwatch: 2 suites  0 :zzz: 2 files    0 :x:

Results for commit 253e1da1.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Apr 16 '24 07:04 github-actions[bot]

Node: HAPI Test (Node Death Reconnect) Results

2 tests   2 :white_check_mark:  7m 1s :stopwatch: 2 suites  0 :zzz: 2 files    0 :x:

Results for commit 253e1da1.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Apr 16 '24 07:04 github-actions[bot]

Node: HAPI Test (Token) Results

235 tests   234 :white_check_mark:  21m 13s :stopwatch:  17 suites    1 :zzz:  17 files      0 :x:

Results for commit 253e1da1.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Apr 16 '24 08:04 github-actions[bot]

Node: HAPI Test (Crypto) Results

335 tests   335 :white_check_mark:  37m 58s :stopwatch:  25 suites    0 :zzz:  25 files      0 :x:

Results for commit 253e1da1.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Apr 16 '24 08:04 github-actions[bot]

Node: HAPI Test (Misc) Results

459 tests   449 :white_check_mark:  38m 51s :stopwatch:  77 suites   10 :zzz:  77 files      0 :x:

Results for commit 253e1da1.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Apr 16 '24 08:04 github-actions[bot]

Node: HAPI Test (Time Consuming) Results

21 tests   21 :white_check_mark:  54m 41s :stopwatch:  3 suites   0 :zzz:  3 files     0 :x:

Results for commit 253e1da1.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Apr 16 '24 09:04 github-actions[bot]

Node: HAPI Test (Smart Contract) Results

589 tests   589 :white_check_mark:  1h 4m 22s :stopwatch:  63 suites    0 :zzz:  63 files      0 :x:

Results for commit 253e1da1.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Apr 16 '24 09:04 github-actions[bot]

Node: Unit Test Results

  2 278 files  + 2    2 278 suites  +2   3h 22m 44s :stopwatch: - 10m 34s 118 755 tests +21  118 688 :white_check_mark: +21  67 :zzz: ±0  0 :x: ±0  127 243 runs  +24  127 176 :white_check_mark: +24  67 :zzz: ±0  0 :x: ±0 

Results for commit 253e1da1. ± Comparison against base commit 4f8f9818.

This pull request removes 4006 and adds 3790 tests. Note that renamed tests count towards both.

  
             IssuerDN: CN=s-aaaa
            SubjectDN: CN=s-aaaa
           Final Date: Fri Jan 01 00:00:00 UTC 2100
           Public Key: RSA Public Key [2e:28:bc:1e:d3:83:25:92:8e:cb:98:b1:b6:84:06:9c:d5:d8:14:d5],[56:66:d1:a4]
           Start Date: Sat Jan 01 00:00:00 UTC 2000
         SerialNumber: 12482092706667292405
        modulus: c1a0ff5d2372b53d12d12bb87dd03f5e…
        modulus: c1a0ff5d2372b53d12d12bb87dd03f5…
…
com.hedera.node.app.grpc.impl.netty.GrpcServiceBuilderTest ‑ [4] 

com.hedera.node.app.grpc.impl.netty.GrpcServiceBuilderTest ‑ [6] 

com.hedera.node.app.grpc.impl.netty.GrpcServiceBuilderTest ‑ [7]   
  
com.hedera.node.app.hapi.utils.ethereum.EthTxDataTest ‑ [1] LEGACY_ETHEREUM
com.hedera.node.app.hapi.utils.ethereum.EthTxDataTest ‑ [2] EIP2930
com.hedera.node.app.hapi.utils.ethereum.EthTxDataTest ‑ [3] EIP1559
com.hedera.node.app.service.contract.impl.test.exec.TransactionModuleTest ‑ providesCorrespondingKeyForAvailableEthTxData()
com.hedera.node.app.service.contract.impl.test.exec.TransactionModuleTest ‑ providesNullSenderEcdsaKeyWithUnavailableEthTxData()
com.hedera.node.app.service.contract.impl.test.exec.TransactionModuleTest ‑ providesNullSenderEcdsaKeyWithoutHydratedEthTxData()
com.hedera.node.app.service.contract.impl.test.exec.scope.ActiveContractVerificationStrategyTest ‑ signatureTestApprovesEthSenderKeyWhenDelegating()
…

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Apr 16 '24 09:04 github-actions[bot]

Are there any HAPI tests that should go along with this pr?

@lukelee-sl this bug was found at https://github.com/hashgraph/hedera-services/pull/12834. The callFailsWhenAmountIsNegativeButStillChargedFee HAPI test, when converted to Ethereum, is testing just that. So we have it covered.

vtronkov avatar Apr 22 '24 09:04 vtronkov