Slither and Solhint enabled on all future PRs (#172)

This PR resolves all Slither and Solhint issues (but disables them for the recently added PaymentSplitter). It adds the checks to that they will apply for each PR. Documentation is added to provide more information on building new contracts.

Author: drinkcoffee

.github/workflows/test.yml

@@ -52,7 +52,6 @@ jobs:
       - name: Install dependencies
         run: yarn install --frozen-lockfile --network-concurrency 1
       - name: Run eslint
-        continue-on-error: true
         run: yarn run eslint
   solhint:
     name: Run solhint
@@ -68,8 +67,23 @@ jobs:
       - name: Install dependencies
         run: yarn install --frozen-lockfile --network-concurrency 1
       - name: Run solhint
-        continue-on-error: true
         run: yarn run solhint contracts/**/*.sol
+  slither:
+    name: Run slither
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout Code
+        uses: actions/checkout@v3
+      - name: Install Slither
+        run: sudo pip3 install slither-analyzer
+      - name: Show Slither Version
+        run: slither --version
+      - name: Install Foundry
+        uses: foundry-rs/foundry-toolchain@v1
+      - name: Show Forge Version
+        run: forge --version
+      - name: Run slither
+        run: slither --compile-force-framework forge  --foundry-out-directory foundry-out .
   publish:
     name: Publish to NPM (dry run)
     runs-on: ubuntu-latest

BUILD.md

@@ -0,0 +1,50 @@
+# Build and Test Information
+
+## Install
+
+Install dependencies:
+
+```
+yarn install
+sudo pip3 install slither-analyzer
+```
+
+## Build and Test
+
+To build and test the contracts:
+
+```
+forge test -vvv
+yarn test
+```
+
+## Solidity Linter
+
+To execute solhint:
+
+```
+yarn run solhint contracts/**/*.sol
+```
+
+To resolve formatting issues:
+
+```
+npx prettier --write --plugin=prettier-plugin-solidity 'contracts/**/*.sol'
+```
+
+
+## Static Code Analysis
+
+To run slither:
+
+```
+slither --compile-force-framework forge  --foundry-out-directory foundry-out .
+```
+
+## Test Coverage
+
+To check the test coverage based on Foundry tests use:
+
+ ```
+ forge coverage
+ ```

CONTRIBUTING.md

@@ -58,12 +58,19 @@ git pull upstream main
 git checkout -b <your-branch-name>
 ```
 
-6. Be sure to run the tests and set up the relevant linters to ensure all GitHub checks pass (see GitHub issues: https://docs.github.com/en/issues/tracking-your-work-with-issues/about-issues for more information).
+6. Be sure to run the tests and set up the relevant linters to ensure all GitHub checks pass (see GitHub issues: https://docs.github.com/en/issues/tracking-your-work-with-issues/about-issues, and [build information](BUILD.md) for more information). New test code should use Foundry tests and not Hardhat.
 
 ```
+forge test -vvv
 yarn test
 ```
 
+Test coverage for all new code must be 100%. To check your test coverage:
+
+ ```
+ forge coverage
+ ```
+
 7. Add and commit your changes, including a comprehensive commit message summarising your changes, then push changes to your fork. (e.g. Fixed formatting issue with ImmutableERC721MintByID.sol)
 
 ```

README.md

@@ -12,6 +12,8 @@ Immutable Contracts is a library of smart contracts targeted at developers who w
 
 - Smart Contract Wallets
 
+- Random Number Generation
+
 These contracts are feature-rich and are the recommended standard on Immutable zkEVM intended for all users and partners within the ecosystem.
 
 ## Setup
@@ -77,6 +79,10 @@ const mintTransaction = await client.populateMint(receiver, 1);
 const tx = await signer.sendTransaction(mintTransaction);
 ```
 
+## Build and Test
+
+Information about how to build and test the contracts can be found in our [build information](BUILD.md).
+
 ## Contribution
 
 We aim to build robust and feature-rich standards to help all developers onboard and build their projects on Immuable zkEVM, and we welcome any and all feedback and contributions to this repository! See our [contribution guideline](CONTRIBUTING.md) for more details on opening Github issues, pull requests requesting features, minor security vulnerabilities and providing general feedback.

clients/erc20.ts

@@ -29,7 +29,7 @@ export class ERC20Client {
   public async balanceOf(
     provider: Provider,
     account: PromiseOrValue<string>,
-    overrides: CallOverrides = {}
+    overrides: CallOverrides = {},
   ): Promise<BigNumber> {
     return this.contract.connect(provider).balanceOf(account, overrides);
   }
@@ -41,7 +41,7 @@ export class ERC20Client {
     provider: Provider,
     owner: PromiseOrValue<string>,
     spender: PromiseOrValue<string>,
-    overrides: CallOverrides = {}
+    overrides: CallOverrides = {},
   ): Promise<BigNumber> {
     return this.contract.connect(provider).allowance(owner, spender, overrides);
   }
@@ -52,7 +52,7 @@ export class ERC20Client {
   public async populateTransfer(
     to: PromiseOrValue<string>,
     amount: PromiseOrValue<BigNumberish>,
-    overrides: Overrides & { from?: PromiseOrValue<string> } = {}
+    overrides: Overrides & { from?: PromiseOrValue<string> } = {},
   ): Promise<PopulatedTransaction> {
     return this.contract.populateTransaction.transfer(to, amount, { ...defaultGasOverrides, ...overrides });
   }
@@ -63,7 +63,7 @@ export class ERC20Client {
   public async populateApprove(
     spender: PromiseOrValue<string>,
     amount: PromiseOrValue<BigNumberish>,
-    overrides: Overrides & { from?: PromiseOrValue<string> } = {}
+    overrides: Overrides & { from?: PromiseOrValue<string> } = {},
   ): Promise<PopulatedTransaction> {
     return this.contract.populateTransaction.approve(spender, amount, { ...defaultGasOverrides, ...overrides });
   }
@@ -75,7 +75,7 @@ export class ERC20Client {
     from: PromiseOrValue<string>,
     to: PromiseOrValue<string>,
     amount: PromiseOrValue<BigNumberish>,
-    overrides: Overrides & { from?: PromiseOrValue<string> } = {}
+    overrides: Overrides & { from?: PromiseOrValue<string> } = {},
   ): Promise<PopulatedTransaction> {
     return this.contract.populateTransaction.transferFrom(from, to, amount, { ...defaultGasOverrides, ...overrides });
   }