lint: add error rules for assert.ok and assert.notOk (#3927)

* monorepo: add warn rules for assert.ok and assert.notOk

* monorepo: fix some assert.ok and notok

* chore: move from warn to error

* RLP: Move EthereumJSError form util to RLP and start using EthereumJSError in RLP (#3924)

* util/rlp: move ethereumjserror to rlp

* rlp: use ethereumjserror

* rlp: add linter rules to not allow `new Error` (use EthereumJSError in RLP also)

---------

Co-authored-by: Holger Drewes <[email protected]>

* Configure lint to require .ts

* Warn on js extension

* disable ban-comment rule in client cli utils

* block: fix remaining assert.ok

* restrict js in relative imports

* monorepo: more assert.ok cases

* monorepo: fix remaining assertions

* monorepo: fix remaining errors

* mpt: fix tests

* statemanager: fix failing tests

* client: fix failing tests

* monorepo: fix more type issues

* lint: more fixes

* statemanager: fix test

* Update config/eslint.config.mjs

Co-authored-by: Jochem Brouwer <[email protected]>

* Update packages/block/test/block.spec.ts

Co-authored-by: Jochem Brouwer <[email protected]>

* block: fix type issue

* chore: fix mpt isTrue

* Test cleanup

* fix another test

* chore: correct wording of errors

* client: adjust fullethereumservice tests

* block: remove unused async

* blockchain: improve async testing

* vm: apply suggestions

* vm: remove .only

---------

Co-authored-by: Jochem Brouwer <[email protected]>
Co-authored-by: Holger Drewes <[email protected]>
Co-authored-by: acolytec3 <[email protected]>

Author: 4 people

config/eslint.config.mjs

@@ -40,7 +40,7 @@ export default [
         {
           selector: "ThrowStatement > NewExpression[callee.name='Error']",
           message: "Throwing default JS Errors is not allowed. Only throw `EthereumJSError` (see the util package)",
-        }
+        },
       ],
       "no-restricted-globals": [
         "error",
@@ -70,7 +70,7 @@ export default [
       '@typescript-eslint/no-explicit-any': 'off',
       '@typescript-eslint/no-unsafe-function-type': 'off', // TODO: Decide if this is needed
       '@typescript-eslint/no-unused-expressions': 'off', // TODO: Decide if this is needed
-      '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_', varsIgnorePattern: '^_'}],
+      '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }],
       '@typescript-eslint/ban-ts-comment': 'warn',  // TODO: We should clean up ts comments and replace with ts-expect-error
       '@typescript-eslint/no-empty-object-type': ['error', {
         allowInterfaces: 'with-single-extends',
@@ -88,7 +88,6 @@ export default [
       'import/default': 'off',
       'import/export': 'error',
       'import/exports-last': 'off',
-      'import/extensions': ['error','ignorePackages'],
       'import/first': 'error',
       'import/group-exports': 'off',
       'import/named': 'off',
@@ -110,13 +109,25 @@ export default [
 
     },
   },
+  {
+    files: ["**/*.ts"],
+    ignores: ["**/examples/*.ts"],
+    rules: {
+      "no-restricted-imports": ["error", {
+        "patterns": [{
+          "group": ["../**/*.js", "./**/*.js"],
+          "message": "use .ts extensions in relative imports"
+        }]
+      }],
+    }
+  },
   {
     plugins: {
       i: importPlugin,
     },
     files: ['**/src/**/*.ts', '**/bin/**/*.ts'],
     rules: {
-      'i/no-extraneous-dependencies': 'error'
+      'i/no-extraneous-dependencies': 'error',
     },
   },
   {
@@ -128,7 +139,7 @@ export default [
     }
   },
   {
-    files: ['**/*.js', '**/*.cjs','**/*.cts'],
+    files: ['**/*.js', '**/*.cjs', '**/*.cts'],
     rules: {
       '@typescript-eslint/no-require-imports': 'off',
       'no-undef': 'off',
@@ -158,7 +169,20 @@ export default [
   {
     files: ['packages/devp2p/src/ext/**', 'packages/client/src/ext/**', '**/test/**/*.ts',],
     rules: {
-      'no-restricted-syntax': 'off'
+      'no-restricted-syntax': 'off',
+      "no-restricted-properties": [
+        "error",
+        {
+          "object": "assert",
+          "property": "ok",
+          "message": "Usage of assert.ok is forbidden because it relies on truthiness."
+        },
+        {
+          "object": "assert",
+          "property": "notOk",
+          "message": "Usage of assert.notOk is forbidden because it relies on falseness."
+        }
+      ],
     },
   },
   {

packages/binarytree/test/binarytree.spec.ts

@@ -76,7 +76,7 @@ describe('insert', () => {
 
     await tree.put(stem, [index], [value])
     const [retrievedValue] = await tree.get(stem, [index])
-    assert.exists(retrievedValue, 'Retrieved value should exist')
+    assert.isDefined(retrievedValue, 'Retrieved value should exist')
     assert.isTrue(
       equalsBytes(retrievedValue!, value),
       'Retrieved value should match inserted value',
@@ -113,8 +113,8 @@ describe('insert', () => {
     const [retrievedValue2] = await tree.get(stem2, [index2])
 
     // Retrieved values should exist
-    assert.exists(retrievedValue1, 'Value for key1 should exist')
-    assert.exists(retrievedValue2, 'Value for key2 should exist')
+    assert.isDefined(retrievedValue1, 'Value for key1 should exist')
+    assert.isDefined(retrievedValue2, 'Value for key2 should exist')
 
     // Check that the computed state root matches the expected hash.
     assert.equal(
@@ -139,7 +139,7 @@ describe('insert', () => {
 
     for (let i = 0; i < suffixes.length; i++) {
       const [retrievedValue] = await tree.get(stem, [suffixes[i]])
-      assert.exists(retrievedValue, `Value at suffix ${suffixes[i]} should exist`)
+      assert.isDefined(retrievedValue, `Value at suffix ${suffixes[i]} should exist`)
       assert.isTrue(
         equalsBytes(retrievedValue!, values[i]),
         `Value at suffix ${suffixes[i]} should match inserted value`,
@@ -166,8 +166,8 @@ describe('insert', () => {
     for (let i = 0; i < suffixes1.length; i++) {
       const [retrievedValue1] = await tree.get(stem1, [suffixes1[i]])
       const [retrievedValue2] = await tree.get(stem2, [suffixes2[i]])
-      assert.exists(retrievedValue1, `Value at suffix ${suffixes1[i]} should exist`)
-      assert.exists(retrievedValue2, `Value at suffix ${suffixes2[i]} should exist`)
+      assert.isDefined(retrievedValue1, `Value at suffix ${suffixes1[i]} should exist`)
+      assert.isDefined(retrievedValue2, `Value at suffix ${suffixes2[i]} should exist`)
       assert.isTrue(
         equalsBytes(retrievedValue1!, values1[i]),
         `Value at suffix ${suffixes1[i]} should match inserted value`,
@@ -199,8 +199,8 @@ describe('insert', () => {
     const [retrievedValue1] = await tree.get(stem1, [index1])
     const [retrievedValue2] = await tree.get(stem2, [index2])
 
-    assert.exists(retrievedValue1, 'Value for key1 should exist')
-    assert.exists(retrievedValue2, 'Value for key2 should exist')
+    assert.isDefined(retrievedValue1, 'Value for key1 should exist')
+    assert.isDefined(retrievedValue2, 'Value for key2 should exist')
     assert.isTrue(
       equalsBytes(retrievedValue1!, value1),
       'Value for key1 should match inserted value',
@@ -237,9 +237,9 @@ describe('insert', () => {
     const [retrievedValue2] = await tree1.get(stem2, [index2])
     const [retrievedValue3] = await tree1.get(stem3, [index3])
 
-    assert.exists(retrievedValue1, 'Value for key1 should exist')
-    assert.exists(retrievedValue2, 'Value for key2 should exist')
-    assert.exists(retrievedValue3, 'Value for key3 should exist')
+    assert.isDefined(retrievedValue1, 'Value for key1 should exist')
+    assert.isDefined(retrievedValue2, 'Value for key2 should exist')
+    assert.isDefined(retrievedValue3, 'Value for key3 should exist')
     assert.isTrue(
       equalsBytes(retrievedValue1!, value1),
       'Value for key1 should match inserted value',
@@ -287,9 +287,9 @@ describe('insert', () => {
     const [retrievedValue2] = await tree1.get(stem2, [index2])
     const [retrievedValue3] = await tree1.get(stem3, [index3])
 
-    assert.exists(retrievedValue1, 'Value for key1 should exist')
-    assert.exists(retrievedValue2, 'Value for key2 should exist')
-    assert.exists(retrievedValue3, 'Value for key3 should exist')
+    assert.isDefined(retrievedValue1, 'Value for key1 should exist')
+    assert.isDefined(retrievedValue2, 'Value for key2 should exist')
+    assert.isDefined(retrievedValue3, 'Value for key3 should exist')
     assert.isTrue(
       equalsBytes(retrievedValue1!, value1),
       'Value for key1 should match inserted value',
@@ -405,7 +405,7 @@ describe('insert', () => {
       const stem = hashedKey.slice(0, 31)
       const index = hashedKey[31]
       const [retrievedValue] = await tree1.get(stem, [index])
-      assert.exists(
+      assert.isDefined(
         retrievedValue,
         `Value for key  ${bytesToHex(hashedKey)} | unhashed: ${bytesToHex(originalKey)} should exist`,
       )
@@ -448,7 +448,7 @@ describe('insert', () => {
 
     // Ensure that we can retrieve the updated value
     const [retrievedValue] = await tree1.get(stemToUpdate, [indexToUpdate])
-    assert.exists(retrievedValue, 'Updated value should exist')
+    assert.isDefined(retrievedValue, 'Updated value should exist')
     assert.isTrue(
       equalsBytes(retrievedValue!, updatedValue),
       'Retrieved value should match the updated value',
@@ -481,7 +481,7 @@ describe('insert', () => {
     }
 
     const leafValues = await dumpLeafValues(tree, tree.root())
-    assert.exists(leafValues)
+    assert.isDefined(leafValues)
     assert.equal(leafValues!.length, 100)
 
     const expectedValues = keyValuePairs.map(({ value }) => bytesToHex(value)).sort()
@@ -493,7 +493,7 @@ describe('insert', () => {
     assert.deepEqual(actualKeys, expectedKeys)
 
     const nodeHashes = await dumpNodeHashes(tree, tree.root())
-    assert.exists(nodeHashes)
+    assert.isDefined(nodeHashes)
     expect(nodeHashes!.length).toBeGreaterThan(100)
     assert.equal(nodeHashes![0][1], bytesToHex(tree.root()))
   })
@@ -513,7 +513,7 @@ describe('insert', () => {
 
     const [retrievedValue] = await tree.get(stem, [index])
 
-    assert.exists(retrievedValue, 'Retrieved value should exist')
+    assert.isDefined(retrievedValue, 'Retrieved value should exist')
     assert.isTrue(
       equalsBytes(retrievedValue!, value2),
       'Retrieved value should match the updated value',
@@ -553,7 +553,7 @@ describe('insert', () => {
     const [retrievedValue1] = await tree.get(stem1, [index1])
     const [retrievedValue2] = await tree.get(stem2, [index2])
 
-    assert.exists(retrievedValue1, 'Retrieved value should exist')
+    assert.isDefined(retrievedValue1, 'Retrieved value should exist')
     assert.notExists(retrievedValue2, 'Deleted value should not exist')
     assert.isTrue(
       equalsBytes(initialRoot, recoveredRoot),

packages/binarytree/test/node/internalNode.spec.ts

@@ -22,8 +22,8 @@ describe('InternalBinaryNode', () => {
     // Verify the type
     assert.equal(decoded.type, BinaryNodeType.Internal)
     const [leftRecoveredChild, rightRecoveredChild] = (decoded as InternalBinaryNode).children
-    assert.exists(leftRecoveredChild, 'Left child should exist')
-    assert.exists(rightRecoveredChild, 'Right child should exist')
+    assert.isDefined(leftRecoveredChild, 'Left child should exist')
+    assert.isDefined(rightRecoveredChild, 'Right child should exist')
     assert.isTrue(
       equalsBytes(leftRecoveredChild!.hash, leftCanonicalChild.hash),
       'Left child hash should round-trip',

packages/binarytree/test/node/stemNode.spec.ts

@@ -40,19 +40,19 @@ describe('StemBinaryNode', () => {
     assert.equal(recovered.values.length, 256, 'Values array should have 256 elements')
 
     // Check that the non-null values round-trip.
-    assert.exists(recovered.values[3], 'Value at index 3 should exist')
+    assert.isDefined(recovered.values[3], 'Value at index 3 should exist')
     assert.isTrue(
       equalsBytes(recovered.values[3]!, value3),
       'Value at index 3 should round-trip correctly',
     )
 
-    assert.exists(recovered.values[100], 'Value at index 100 should exist')
+    assert.isDefined(recovered.values[100], 'Value at index 100 should exist')
     assert.isTrue(
       equalsBytes(recovered.values[100]!, value100),
       'Value at index 100 should round-trip correctly',
     )
 
-    assert.exists(recovered.values[255], 'Value at index 255 should exist')
+    assert.isDefined(recovered.values[255], 'Value at index 255 should exist')
     assert.isTrue(
       equalsBytes(recovered.values[255]!, value255),
       'Value at index 255 should round-trip correctly',

packages/block/test/block.spec.ts

@@ -29,7 +29,7 @@ describe('[Block]: block functions', () => {
   it('should test block initialization', () => {
     const common = new Common({ chain: Mainnet, hardfork: Hardfork.Chainstart })
     const genesis = createBlock({}, { common })
-    assert.exists(bytesToHex(genesis.hash()), 'block should initialize')
+    assert.isDefined(bytesToHex(genesis.hash()), 'block should initialize')
 
     const params = JSON.parse(JSON.stringify(paramsBlock))
     params['1']['minGasLimit'] = 3000 // 5000
@@ -41,28 +41,22 @@ describe('[Block]: block functions', () => {
     )
 
     const emptyBlock = createEmptyBlock({}, { common })
-    assert.exists(bytesToHex(emptyBlock.hash()), 'block should initialize')
+    assert.isDefined(bytesToHex(emptyBlock.hash()), 'block should initialize')
 
     // test default freeze values
     // also test if the options are carried over to the constructor
     block = createBlock({})
-    assert.isTrue(Object.isFrozen(block), 'block should be frozen by default')
+    assert.isFrozen(block, 'block should be frozen by default')
 
     block = createBlock({}, { freeze: false })
-    assert.ok(
-      !Object.isFrozen(block),
-      'block should not be frozen when freeze deactivated in options',
-    )
+    assert.isNotFrozen(block, 'block should not be frozen when freeze deactivated in options')
 
     const rlpBlock = block.serialize()
     block = createBlockFromRLP(rlpBlock)
-    assert.isTrue(Object.isFrozen(block), 'block should be frozen by default')
+    assert.isFrozen(block, 'block should be frozen by default')
 
     block = createBlockFromRLP(rlpBlock, { freeze: false })
-    assert.ok(
-      !Object.isFrozen(block),
-      'block should not be frozen when freeze deactivated in options',
-    )
+    assert.isNotFrozen(block, 'block should not be frozen when freeze deactivated in options')
 
     const zero = new Uint8Array(0)
     const headerArray: Uint8Array[] = []
@@ -82,13 +76,10 @@ describe('[Block]: block functions', () => {
     const valuesArray = <BlockBytes>[headerArray, [], []]
 
     block = createBlockFromBytesArray(valuesArray, { common })
-    assert.isTrue(Object.isFrozen(block), 'block should be frozen by default')
+    assert.isFrozen(block, 'block should be frozen by default')
 
     block = createBlockFromBytesArray(valuesArray, { common, freeze: false })
-    assert.ok(
-      !Object.isFrozen(block),
-      'block should not be frozen when freeze deactivated in options',
-    )
+    assert.isNotFrozen(block, 'block should not be frozen when freeze deactivated in options')
   })
 
   it('initialization -> setHardfork option', () => {
@@ -165,8 +156,8 @@ describe('[Block]: block functions', () => {
   })
 
   async function testTransactionValidation(block: Block) {
-    assert.ok(block.transactionsAreValid())
-    assert.ok(block.getTransactionsValidationErrors().length === 0)
+    assert.isTrue(block.transactionsAreValid())
+    assert.isEmpty(block.getTransactionsValidationErrors())
   }
 
   it('should test transaction validation - invalid tx trie', async () => {
@@ -441,13 +432,14 @@ describe('[Block]: block functions', () => {
       },
     )
 
-    assert.ok(
-      blockWithDifficultyCalculation.header.difficulty > BigInt(0),
+    assert.notEqual(
+      blockWithDifficultyCalculation.header.difficulty,
+      BigInt(0),
       'header difficulty should be set if difficulty header is given',
     )
-    assert.ok(
-      blockWithDifficultyCalculation.header.ethashCanonicalDifficulty(genesis.header) ===
-        blockWithDifficultyCalculation.header.difficulty,
+    assert.equal(
+      blockWithDifficultyCalculation.header.ethashCanonicalDifficulty(genesis.header),
+      blockWithDifficultyCalculation.header.difficulty,
       'header difficulty is canonical difficulty if difficulty header is given',
     )
 
@@ -467,7 +459,7 @@ describe('[Block]: block functions', () => {
       },
     )
 
-    assert.ok(
+    assert.isTrue(
       block_farAhead.header.difficulty > BigInt(0),
       'should allow me to provide a bogus next block to calculate difficulty on when providing a difficulty header',
     )

packages/block/test/clique.spec.ts

@@ -30,13 +30,13 @@ describe('[Header]: Clique PoA Functionality', () => {
     )
 
     header = createBlockHeader({ extraData: new Uint8Array(97) }, { common })
-    assert.ok(
+    assert.isTrue(
       cliqueIsEpochTransition(header),
       'cliqueIsEpochTransition() -> should indicate an epoch transition for the genesis block',
     )
 
     header = createBlockHeader({ number: 1, extraData: new Uint8Array(97) }, { common })
-    assert.notOk(
+    assert.isFalse(
       cliqueIsEpochTransition(header),
       'cliqueIsEpochTransition() -> should correctly identify a non-epoch block',
     )
@@ -60,7 +60,7 @@ describe('[Header]: Clique PoA Functionality', () => {
     )
 
     header = createBlockHeader({ number: 60000, extraData: new Uint8Array(137) }, { common })
-    assert.ok(
+    assert.isTrue(
       cliqueIsEpochTransition(header),
       'cliqueIsEpochTransition() -> should correctly identify an epoch block',
     )
@@ -107,11 +107,14 @@ describe('[Header]: Clique PoA Functionality', () => {
     )
 
     assert.equal(header.extraData.length, 97)
-    assert.ok(cliqueVerifySignature(header, [A.address]), 'should verify signature')
-    assert.ok(cliqueSigner(header).equals(A.address), 'should recover the correct signer address')
+    assert.isTrue(cliqueVerifySignature(header, [A.address]), 'should verify signature')
+    assert.isTrue(
+      cliqueSigner(header).equals(A.address),
+      'should recover the correct signer address',
+    )
 
     header = createBlockHeader({ extraData: new Uint8Array(97) }, { common })
-    assert.ok(
+    assert.isTrue(
       cliqueSigner(header).equals(createZeroAddress()),
       'should return zero address on default block',
     )