test: Extend the coverage of account nonce discrepancies tests #703

nickeynikolovv · 2024-03-13T15:21:19Z

This PR extends the coverage of account nonce discrepancies tests according to test plan - https://www.notion.so/limechain/Address-account-nonce-discrepancies-Test-Plan-4a186e8cfabc49888ba501750f68d8de?pvs=4

quiet-node

Nice work but some comments

artifacts/contracts/discrepancies/nonce/ChainedContracts.sol/ChainedContracts.json

contracts/discrepancies/nonce/InternalCallee.sol

Signed-off-by: Anastasia Kovaliova <anastasiya.kovaliova@limechain.tech>

- added and adapted internalTransfer function in internalCallee contract - skipped the blocked tests due to some issues Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

- refactored some expected to fail function in test 030 Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

github-actions · 2024-03-25T14:26:46Z

Test Results

  14 files ±  0   74 suites +2 7m 46s ⏱️ -28s
244 tests +  2 238 ✔️ +  2 6 💤 ±0 0 ❌ ±0
310 runs +68 304 ✔️ +68 6 💤 ±0 0 ❌ ±0

Results for commit 7c49202. ± Comparison against base commit 21aaeb3.

♻️ This comment has been updated with latest results.

Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

This reverts commit 1697084.

Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

quiet-node

LG! Great work thanks!

victor-yanev · 2024-04-04T12:54:31Z

test/discrepancies/Nonce.js

+    erc721Contract = await Utils.deployERC721Contract();
+    tokenAddress = await Utils.createNonFungibleToken(
+      tokenCreateContract,
+      signers[0].address
+    );


Let's avoid overriding variables which are reused in multiple tests - this creates dependencies between the tests where, depending on their execution order, the variable might hold a different value. This reduces the readability of code and makes it hard to trace what is happening without knowing the order of the tests or without going into debug mode.

If we are going to reuse a variable let's assign it only in one place (e.g., in a before() method or, where possible, directly turn it into a const.)

If there are subsets of tests which expect different values here for these variables, we can group them in multiple describes and declare + assign those variables there.

Please check for other such occurrences and try to fix them 🙂. Thank you!

Unneeded override is removed

victor-yanev · 2024-04-04T12:58:16Z

test/discrepancies/Nonce.js

+
+  //NONCE-023
+  //need to update the get contract nonce - waiting for SDK
+  xit('should update contract nonce for each deployed contract', async function () {


Just a lil reminder to enable this later (and other tests waiting for the SDK changes)

Nana-EC

Looking good, a few comments to address.

Nana-EC · 2024-05-07T01:55:24Z

contracts/discrepancies/nonce/ChainedContracts.sol

+    constructor() {
+        owner = msg.sender;
+        // Deploy Chain within ChildContract constructor
+        //  chainContract = new ChainContract();


nit: remove and other commented out code

Nana-EC · 2024-05-07T01:58:57Z

test/hts-precompile/utils.js

@@ -525,6 +530,12 @@ class Utils {
    return accountInfo.accountId.toString();
  }

+  static async getContractInfo(evmAddress, client){


Can you add a comment to note that the only reason we're using the SDK is to call the consensus node to confirm that there are no discrepancies between CN and MN.

Nana-EC · 2024-05-07T01:59:35Z

test/discrepancies/Nonce.js

+
+  //NONCE-022
+  //should fix the get contract nonce function - waiting for SDKs
+  xit('should update all nonces when chained deploys of contracts', async function () {


Q: why is this disabled?

There are few tests where we are not able to get nonce of a smart contract from the sdk - this issue: hashgraph/hedera-sdk-js#2192 . Basically we cannot assert that the nonce of a smart contract is correct. Also added a comments to the tests with the issue url

-added comments for additional info -removed test 29 Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

nickeynikolovv requested review from AlfredoG87, attest08, ebadiere, georgi-l95, Ivo-Yankov, natanasow, yiliev0, a team and Nana-EC as code owners March 13, 2024 15:21

quiet-node requested changes Mar 21, 2024

View reviewed changes

anastasiya-kovaliova and others added 6 commits March 25, 2024 16:08

test: first 4 tests

cd74568

Signed-off-by: Anastasia Kovaliova <anastasiya.kovaliova@limechain.tech>

add more tests for nonce discrepancies

2361992

Signed-off-by: Anastasia Kovaliova <anastasiya.kovaliova@limechain.tech>

add more contract functions

2b850df

Signed-off-by: Anastasia Kovaliova <anastasiya.kovaliova@limechain.tech>

- added additional tests for checking the nonce

c48e496

- added and adapted internalTransfer function in internalCallee contract - skipped the blocked tests due to some issues Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

- removed unneeded console.logs, polished some tests

ff7b22b

- refactored some expected to fail function in test 030 Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

removed .only from the describe

6c4a873

Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

nickeynikolovv force-pushed the test_677-account-nonce-discrepancies-additional-tests branch from fa0b04c to 6c4a873 Compare March 25, 2024 14:17

nickeynikolovv added 6 commits March 25, 2024 16:59

- applied gitignore json removal

74649fc

Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

- removed unneeded json files

1697084

Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

Revert "- removed unneeded json files"

9555c4f

This reverts commit 1697084.

- removed unneeded internalCalee json

951616b

Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

Added contracts-abi json files for the contracts

1cde0ca

Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

Renamed sefdestruct to selfDestructSample function in Sample contract

7c49202

Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

quiet-node previously approved these changes Mar 26, 2024

View reviewed changes

victor-yanev reviewed Apr 4, 2024

View reviewed changes

Nana-EC requested changes May 7, 2024

View reviewed changes

nickeynikolovv added 2 commits May 9, 2024 15:01

-removed unneeded console.logs

66a6a61

-added comments for additional info -removed test 29 Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

removed unneeded tokenAddress override

34105ff

Signed-off-by: Nikolay Nikolov <nikolay.nikolov@limechain.tech>

nickeynikolovv dismissed quiet-node’s stale review via 34105ff May 9, 2024 12:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: Extend the coverage of account nonce discrepancies tests #703

test: Extend the coverage of account nonce discrepancies tests #703

nickeynikolovv commented Mar 13, 2024

quiet-node left a comment

github-actions bot commented Mar 25, 2024 •

edited

quiet-node left a comment

victor-yanev Apr 4, 2024 •

edited

nickeynikolovv May 9, 2024

victor-yanev Apr 4, 2024

Nana-EC left a comment

Nana-EC May 7, 2024

nickeynikolovv May 9, 2024

Nana-EC May 7, 2024

nickeynikolovv May 9, 2024

Nana-EC May 7, 2024

nickeynikolovv May 9, 2024 •

edited

test: Extend the coverage of account nonce discrepancies tests #703

Are you sure you want to change the base?

test: Extend the coverage of account nonce discrepancies tests #703

Conversation

nickeynikolovv commented Mar 13, 2024

quiet-node left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 25, 2024 • edited

Test Results

quiet-node left a comment

Choose a reason for hiding this comment

victor-yanev Apr 4, 2024 • edited

Choose a reason for hiding this comment

nickeynikolovv May 9, 2024

Choose a reason for hiding this comment

victor-yanev Apr 4, 2024

Choose a reason for hiding this comment

Nana-EC left a comment

Choose a reason for hiding this comment

Nana-EC May 7, 2024

Choose a reason for hiding this comment

nickeynikolovv May 9, 2024

Choose a reason for hiding this comment

Nana-EC May 7, 2024

Choose a reason for hiding this comment

nickeynikolovv May 9, 2024

Choose a reason for hiding this comment

Nana-EC May 7, 2024

Choose a reason for hiding this comment

nickeynikolovv May 9, 2024 • edited

Choose a reason for hiding this comment

github-actions bot commented Mar 25, 2024 •

edited

victor-yanev Apr 4, 2024 •

edited

nickeynikolovv May 9, 2024 •

edited