feat: account nonce discrepancies tests

[natanasow]

Description:

There are no e2e tests for checks that are happening between the ingestion of the transaction and its evm execution.

Add e2e tests based on these in services and on the current test plan.

Related issue(s):

Fixes #653

Follow up issue:

• Activate disabled tests in nonce suite when HIP-844 images are updated in local node #659

Notes for reviewer:

Checklist

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

[natanasow]

A draft PR for nonce discrepancies tests. Several PRs need to be merged and released before we can run these tests in the CI. Locally I've tested it against a custom build of services (PR#11045) and mirror node importer (PR#7617) and it's all green which is such a relief right now 🍀.

[github-actions]

Test Results

242 tests  ±0   236 ✔️ ±0   7m 33s ⏱️ +9s
  72 suites ±0       6 💤 ±0 
  14 files   ±0       0 ❌ ±0 

Results for commit 1255097. ± Comparison against base commit 8cabc7f.

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

should NOT be able to aggregate 150 calls to processLongInputTx ‑ Multicall Test Suite payable calls with large input should NOT be able to aggregate 150 calls to processLongInputTx

should NOT be able to aggregate 200 calls to processLongInputTx ‑ Multicall Test Suite payable calls with large input should NOT be able to aggregate 200 calls to processLongInputTx

♻️ This comment has been updated with latest results.

[quiet-node]

Nice work and thanks for the PR! Some requests changes though

[quiet-node]

Please also include the artifact files for the new contracts when after compilation

[quiet-node]

@natanasow I attempted to run the test locally using the latest version, [email protected]. Unfortunately, all the units failed as null values were compared to the expected ones. After tracing down the issue, I identified that the error is originating from test/hts-precompile/utils.js:getAccountId().

Specifically, within the getAccountId() function, the error is triggered by the line await query.execute(client), resulting in the following error:

TypeError: Cannot read properties of undefined (reading 'accountId')

Any thoughts on what I might have missed?

[natanasow]

@natanasow I attempted to run the test locally using the latest version, [email protected]. Unfortunately, all the units failed as null values were compared to the expected ones. After tracing down the issue, I identified that the error is originating from test/hts-precompile/utils.js:getAccountId().

Specifically, within the getAccountId() function, the error is triggered by the line await query.execute(client), resulting in the following error:

TypeError: Cannot read properties of undefined (reading 'accountId')

Any thoughts on what I might have missed?

@quiet-node Just pushed some fixes and added a new pipeline for these tests. Could you try again locally, and keep in mind that if you're using the latest local node, some tests will fail because there are still no official images for HIP-844.

[quiet-node]

@natanasow I attempted to run the test locally using the latest version, [email protected]. Unfortunately, all the units failed as null values were compared to the expected ones. After tracing down the issue, I identified that the error is originating from test/hts-precompile/utils.js:getAccountId().
Specifically, within the getAccountId() function, the error is triggered by the line await query.execute(client), resulting in the following error:

TypeError: Cannot read properties of undefined (reading 'accountId')

Any thoughts on what I might have missed?

@quiet-node Just pushed some fixes and added a new pipeline for these tests. Could you try again locally, and keep in mind that if you're using the latest local node, some tests will fail because there are still no official images for HIP-844.

Thanks for the update! I have tried it out locally and looks like the 2nd and the 4th units failed.

 Discrepancies - Nonce Test Suite
    ✔ should not update nonce when intrinsic gas handler check failed (65ms)
    1) should not update nonce when offered gas price and allowance are zero handler check failed
    ✔ should not update nonce when offered gas price is less than current and sender does not have enough balance handler check failed (2062ms)
    2) should not update nonce when offered gas price is less than current and gas allowance is less than remaining fee handler check failed
    ✔ should not update nonce when offered gas price is bigger than current and sender does not have enough balance handler check failed (1712ms)
    ✔ should not update nonce  when sender does not have enough balance handler check failed (1397ms)
    ✔ should update nonce after evm reversion due contract logic (1574ms)
    ✔ should update nonce after evm reversion due insufficient gas (1324ms)
    ✔ should update nonce after evm reversion due insufficient transfer amount (1077ms)
    ✔ should update nonce after evm reversion due sending value to ethereum precompile 0x2 (1572ms)
    ✔ should update nonce after evm reversion due sending value to hedera precompile0 x167 (1324ms)
    ✔ should update nonce after successful internal call (1070ms)
    ✔ should update nonce after successful internal transfer (2935ms)
    ✔ should update nonce after successful internal contract deployment (1323ms)


  12 passing (23s)
  2 failing

  1) Discrepancies - Nonce Test Suite
       should not update nonce when offered gas price and allowance are zero handler check failed:

      AssertionError: expected 2 to equal 3
      + expected - actual

      -2
      +3

      at expectNonIncrementedNonce (test/discrepancies/Nonce.js:90:36)
      at Context.<anonymous> (test/discrepancies/Nonce.js:135:5)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

  2) Discrepancies - Nonce Test Suite
       should not update nonce when offered gas price is less than current and gas allowance is less than remaining fee handler check failed:

      AssertionError: expected 4 to equal 5
      + expected - actual

      -4
      +5

      at expectNonIncrementedNonce (test/discrepancies/Nonce.js:90:36)
      at Context.<anonymous> (test/discrepancies/Nonce.js:182:5)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

Just like in the CI.

So as you said some tests will fail because there are still no official images for HIP-844, I'm not sure if we should approve failing tests or we should wait for the official images for HIP-844.

@Nana-EC thoughts?

[natanasow]

@natanasow I attempted to run the test locally using the latest version, [email protected]. Unfortunately, all the units failed as null values were compared to the expected ones. After tracing down the issue, I identified that the error is originating from test/hts-precompile/utils.js:getAccountId().
Specifically, within the getAccountId() function, the error is triggered by the line await query.execute(client), resulting in the following error:

TypeError: Cannot read properties of undefined (reading 'accountId')

Any thoughts on what I might have missed?

@quiet-node Just pushed some fixes and added a new pipeline for these tests. Could you try again locally, and keep in mind that if you're using the latest local node, some tests will fail because there are still no official images for HIP-844.

Thanks for the update! I have tried it out locally and looks like the 2nd and the 4th units failed.

 Discrepancies - Nonce Test Suite
    ✔ should not update nonce when intrinsic gas handler check failed (65ms)
    1) should not update nonce when offered gas price and allowance are zero handler check failed
    ✔ should not update nonce when offered gas price is less than current and sender does not have enough balance handler check failed (2062ms)
    2) should not update nonce when offered gas price is less than current and gas allowance is less than remaining fee handler check failed
    ✔ should not update nonce when offered gas price is bigger than current and sender does not have enough balance handler check failed (1712ms)
    ✔ should not update nonce  when sender does not have enough balance handler check failed (1397ms)
    ✔ should update nonce after evm reversion due contract logic (1574ms)
    ✔ should update nonce after evm reversion due insufficient gas (1324ms)
    ✔ should update nonce after evm reversion due insufficient transfer amount (1077ms)
    ✔ should update nonce after evm reversion due sending value to ethereum precompile 0x2 (1572ms)
    ✔ should update nonce after evm reversion due sending value to hedera precompile0 x167 (1324ms)
    ✔ should update nonce after successful internal call (1070ms)
    ✔ should update nonce after successful internal transfer (2935ms)
    ✔ should update nonce after successful internal contract deployment (1323ms)


  12 passing (23s)
  2 failing

  1) Discrepancies - Nonce Test Suite
       should not update nonce when offered gas price and allowance are zero handler check failed:

      AssertionError: expected 2 to equal 3
      + expected - actual

      -2
      +3

      at expectNonIncrementedNonce (test/discrepancies/Nonce.js:90:36)
      at Context.<anonymous> (test/discrepancies/Nonce.js:135:5)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

  2) Discrepancies - Nonce Test Suite
       should not update nonce when offered gas price is less than current and gas allowance is less than remaining fee handler check failed:

      AssertionError: expected 4 to equal 5
      + expected - actual

      -4
      +5

      at expectNonIncrementedNonce (test/discrepancies/Nonce.js:90:36)
      at Context.<anonymous> (test/discrepancies/Nonce.js:182:5)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

Just like in the CI.

So as you said some tests will fail because there are still no official images for HIP-844, I'm not sure if we should approve failing tests or we should wait for the official images for HIP-844.

@Nana-EC thoughts?

IMO, we should NOT merge this into develop till official images of HIP-844 are released. Once they are released, we should update the local node first and release a new version of it with the updated images. After that, I have to circle back here and bump the local node version in this repo. With the newest version, which must include HIP-844, these tests have to succeed and we will be free to merge it.

[quiet-node]

Look good. Just 1 request

[quiet-node]

We were thinking moving this CI job into a dispatch workflow instead of the main flow. I think the best place would be in here https://github.com/hashgraph/hedera-smart-contracts/blob/main/.github/workflows/solidity-tests.yml. We can also at a flag [like this}(https://github.com/hashgraph/hedera-smart-contracts/blob/main/test/solidity/account/nonExisting.js#L25), but we can come up with something else like @discrepancies or something like that

[natanasow]

Done.

[quiet-node]

Wonderful work! Thanks much Nikolay!