ONCHAINID v2 (#73)

## [2.0.0]

Version 2.0.0 Audited by Hacken, more details [here](https://tokeny.com/wp-content/uploads/2023/04/Tokeny_ONCHAINID_SC-Audit_Report.pdf)

### Breaking changes

## Deprecation Notice
- ClaimIssuer `revokeClaim` is now deprecated, usage of `revokeClaimBySignature(bytes signature)` is preferred.

### Added
- Add typechain-types (targeting ethers v5).
- Add tests cases for `execute` and `approve` methods.
- Add method `revokeClaimBySignature(bytes signature)` in ClaimIssuer, prefer using this method instead of the now
deprecated `revokeClaim` method.
- Add checks on ClaimIssuer to prevent revoking an already revoked claim.
- Added Factory for ONCHAINIDs

### Updated
- Switch development tooling to hardhat.
- Implemented tests for hardhat (using fixture for faster testing time).
- Prevent calling `approve` method with a non-request execute nonce (added a require on `executionNone`).
- Update NatSpec of `execute` and `approve` methods.

---------

Co-authored-by: Joachim <[email protected]>

Author: Nakasar

.eslintrc.js

@@ -1,11 +1,5 @@
 module.exports = {
-  extends: [
-    'airbnb-base',
-    'plugin:prettier/recommended',
-    'plugin:import/errors',
-    'plugin:import/warnings',
-    'plugin:import/typescript',
-  ],
+  extends: [],
   parserOptions: { ecmaVersion: 2018 },
   root: true,
   rules: {

.github/workflows/publish-prerelease.yml

@@ -9,21 +9,21 @@ jobs:
     if: "github.event.release.prerelease"
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v1
-      - uses: actions/setup-node@v1
+      - uses: actions/checkout@v2
+      - uses: actions/setup-node@v3
         with:
-          node-version: 12
+          node-version: 16
       - run: npm ci
       - run: npm run build
 
   publish-gpr:
     needs: build
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v1
-      - uses: actions/setup-node@v1
+      - uses: actions/checkout@v2
+      - uses: actions/setup-node@v3
         with:
-          node-version: 12
+          node-version: 16
           registry-url: https://npm.pkg.github.com/
           scope: '@onchain-id'
       - run: npm ci
@@ -36,10 +36,10 @@ jobs:
     needs: build
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v1
-      - uses: actions/setup-node@v1
+      - uses: actions/checkout@v2
+      - uses: actions/setup-node@v3
         with:
-          node-version: 12
+          node-version: 16
           registry-url: https://registry.npmjs.org/
           scope: '@onchain-id'
       - run: npm ci

.github/workflows/publish-release.yml

@@ -9,21 +9,21 @@ jobs:
     if: "!github.event.release.prerelease"
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v1
-      - uses: actions/setup-node@v1
+      - uses: actions/checkout@v2
+      - uses: actions/setup-node@v3
         with:
-          node-version: 12
+          node-version: 16
       - run: npm ci
       - run: npm run build
 
   publish-gpr:
     needs: build
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v1
-      - uses: actions/setup-node@v1
+      - uses: actions/checkout@v2
+      - uses: actions/setup-node@v3
         with:
-          node-version: 12
+          node-version: 16
           registry-url: https://npm.pkg.github.com/
           scope: '@onchain-id'
       - run: npm ci
@@ -36,10 +36,10 @@ jobs:
     needs: build
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v1
-      - uses: actions/setup-node@v1
+      - uses: actions/checkout@v2
+      - uses: actions/setup-node@v3
         with:
-          node-version: 12
+          node-version: 16
           registry-url: https://registry.npmjs.org/
           scope: '@onchain-id'
       - run: npm ci

.github/workflows/push_checking.yml

@@ -6,16 +6,16 @@ jobs:
   lint:
     name: "Lint"
     runs-on: ubuntu-latest
-    container: node:12
+    container: node:16
 
     strategy:
       matrix:
-        node-version: [12.x]
+        node-version: [16.x]
 
     steps:
       - uses: actions/checkout@v2
       - name: Use Node.js ${{ matrix.node-version }}
-        uses: actions/setup-node@v1
+        uses: actions/setup-node@v3
         with:
           node-version: ${{ matrix.node-version }}
           registry-url: https://npm.pkg.github.com/
@@ -32,11 +32,11 @@ jobs:
   test:
     name: "Build and Test"
     runs-on: ubuntu-latest
-    container: node:12
+    container: node:16
 
     strategy:
       matrix:
-        node-version: [12.x]
+        node-version: [16.x]
 
     steps:
       - uses: actions/checkout@v2
@@ -53,4 +53,4 @@ jobs:
       - name: Build application
         run: npm run build
       - name: Run tests
-        run: npm run test
+        run: npm run coverage

.gitignore

@@ -17,3 +17,9 @@ build/
 # IDE
 .idea
 .vscode
+
+# Artifacts
+artifacts
+coverage.json
+cache
+typechain-types

.solhint.json

@@ -1,16 +1,33 @@
 {
   "extends": "solhint:recommended",
   "rules": {
-    "func-order": "off",
-    "mark-callable-contracts": "off",
-    "compiler-version": ["error", "^0.8.0"],
-    "no-inline-assembly": "off",
-    "avoid-call-value": "off",
-    "no-empty-blocks": "off",
-    "state-visibility": "off",
-    "func-visibility": "off",
-    "reason-string": "off",
-    "no-complex-fallback": "off",
-    "avoid-low-level-calls": "off"
-  }
+    "compiler-version": ["error", "^0.8.17"],
+    "func-visibility": ["warn", { "ignoreConstructors": true }],
+    "reentrancy": "error",
+    "state-visibility": "error",
+    "quotes": ["error", "double"],
+    "const-name-snakecase": "error",
+    "contract-name-camelcase": "error",
+    "event-name-camelcase": "error",
+    "func-name-mixedcase": "error",
+    "func-param-name-mixedcase": "error",
+    "modifier-name-mixedcase": "error",
+    "private-vars-leading-underscore": ["error", { "strict": false }],
+    "use-forbidden-name": "error",
+    "var-name-mixedcase": "error",
+    "imports-on-top": "error",
+    "ordering": "error",
+    "visibility-modifier-order": "error",
+    "code-complexity": ["error", 7],
+    "function-max-lines": ["error", 50],
+    "max-line-length": ["error", 130],
+    "max-states-count": ["error", 15],
+    "no-empty-blocks": "error",
+    "no-unused-vars": "error",
+    "payable-fallback": "error",
+    "constructor-syntax": "error",
+    "not-rely-on-time": "off",
+    "reason-string": "off"
+  },
+  "plugins": ["prettier"]
 }

CHANGELOG.md

@@ -4,6 +4,29 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [2.0.0]
+
+Version 2.0.0 Audited by Hacken, more details [here](https://tokeny.com/wp-content/uploads/2023/04/Tokeny_ONCHAINID_SC-Audit_Report.pdf)
+
+### Breaking changes
+
+## Deprecation Notice
+- ClaimIssuer `revokeClaim` is now deprecated, usage of `revokeClaimBySignature(bytes signature)` is preferred.
+
+### Added
+- Add typechain-types (targeting ethers v5).
+- Add tests cases for `execute` and `approve` methods.
+- Add method `revokeClaimBySignature(bytes signature)` in ClaimIssuer, prefer using this method instead of the now
+deprecated `revokeClaim` method.
+- Add checks on ClaimIssuer to prevent revoking an already revoked claim.
+- Added Factory for ONCHAINIDs
+
+### Updated
+- Switch development tooling to hardhat.
+- Implemented tests for hardhat (using fixture for faster testing time).
+- Prevent calling `approve` method with a non-request execute nonce (added a require on `executionNone`).
+- Update NatSpec of `execute` and `approve` methods.
+
 ## [1.4.0] - 2021-01-26
 ### Updated
 - Remove constructor's visibility

README.md

@@ -1,10 +1,18 @@
-![OnchainID Smart Contracts](./onchainid_logo_small.png)
+![OnchainID Smart Contracts](./onchainid_logo_final.png)
+---
 
+![GitHub](https://img.shields.io/github/license/onchain-id/solidity?color=green)
+![GitHub release (latest by date)](https://img.shields.io/github/v/release/onchain-id/solidity)
+![GitHub Workflow Status (branch)](https://img.shields.io/github/actions/workflow/status/onchain-id/solidity/publish-release.yml)
+![GitHub repo size](https://img.shields.io/github/repo-size/onchain-id/solidity)
+![GitHub Release Date](https://img.shields.io/github/release-date/onchain-id/solidity)
+
+---
 # OnchainID Smart Contracts
 
 Smart Contracts for secure Blockchain Identities, implementation of the ERC734 and ERC735 proposal standards.
 
-Learn more about OnchainID and Blockchain Identities on the official OnchainID website: [https://onchainid.com](https://onchainid.com). 
+Learn more about OnchainID and Blockchain Identities on the official OnchainID website: [https://onchainid.com](https://onchainid.com).
 
 ## Usage
 
@@ -21,15 +29,21 @@ Learn more about OnchainID and Blockchain Identities on the official OnchainID w
 
 ## Development
 
-- Install dev dependencies `npm i`
+- Install dev dependencies `npm ci`
 - Update interfaces and contracts code.
 - Run lint `npm run lint`
 - Compile code `npm run compile`
 
 ### Testing
 
-- Install [Ganache](https://www.trufflesuite.com/ganache) or run another blockchain network.
-  
-  > The contracts WILL NOT WORK without at least version 2 of Ganache!
-  
+- Run `npm ci`
 - Run `npm test`
+  - Test will be executed against a local Hardhat network.
+
+---
+
+<div style="padding: 16px;">
+   <a href="https://tokeny.com/wp-content/uploads/2023/04/Tokeny_ONCHAINID_SC-Audit_Report.pdf" target="_blank">
+       <img src="https://hacken.io/wp-content/uploads/2023/02/ColorWBTypeSmartContractAuditBackFilled.png" alt="Proofed by Hacken - Smart contract audit" style="width: 258px; height: 100px;">
+   </a>
+</div>

contracts/ClaimIssuer.sol

@@ -1,59 +1,54 @@
 // SPDX-License-Identifier: GPL-3.0
-pragma solidity ^0.8.0;
+pragma solidity 0.8.17;
 
 import "./interface/IClaimIssuer.sol";
 import "./Identity.sol";
 
 contract ClaimIssuer is IClaimIssuer, Identity {
     mapping (bytes => bool) public revokedClaims;
 
+    // solhint-disable-next-line no-empty-blocks
     constructor(address initialManagementKey) Identity(initialManagementKey, false) {}
 
     /**
-     * @dev Revoke a claim previously issued, the claim is no longer considered as valid after revocation.
-     * @param _claimId the id of the claim
-     * @param _identity the address of the identity contract
-     * @return isRevoked true when the claim is revoked
+     *  @dev See {IClaimIssuer-revokeClaimBySignature}.
      */
-    function revokeClaim(bytes32 _claimId, address _identity) public override delegatedOnly returns(bool) {
+    function revokeClaimBySignature(bytes calldata signature) external override delegatedOnly onlyManager {
+        require(!revokedClaims[signature], "Conflict: Claim already revoked");
+
+        revokedClaims[signature] = true;
+
+        emit ClaimRevoked(signature);
+    }
+
+    /**
+     *  @dev See {IClaimIssuer-revokeClaim}.
+     */
+    function revokeClaim(bytes32 _claimId, address _identity) external override delegatedOnly onlyManager returns(bool) {
         uint256 foundClaimTopic;
         uint256 scheme;
         address issuer;
-        bytes memory  sig;
-        bytes  memory data;
-
-        if (msg.sender != address(this)) {
-            require(keyHasPurpose(keccak256(abi.encode(msg.sender)), 1), "Permissions: Sender does not have management key");
-        }
+        bytes memory sig;
+        bytes memory data;
 
         ( foundClaimTopic, scheme, issuer, sig, data, ) = Identity(_identity).getClaim(_claimId);
 
+        require(!revokedClaims[sig], "Conflict: Claim already revoked");
+
         revokedClaims[sig] = true;
+        emit ClaimRevoked(sig);
         return true;
     }
 
     /**
-     * @dev Returns revocation status of a claim.
-     * @param _sig the signature of the claim
-     * @return isRevoked true if the claim is revoked and false otherwise
+     *  @dev See {IClaimIssuer-isClaimValid}.
      */
-    function isClaimRevoked(bytes memory _sig) public override view returns (bool) {
-        if (revokedClaims[_sig]) {
-            return true;
-        }
-
-        return false;
-    }
-
-    /**
-     * @dev Checks if a claim is valid.
-     * @param _identity the identity contract related to the claim
-     * @param claimTopic the claim topic of the claim
-     * @param sig the signature of the claim
-     * @param data the data field of the claim
-     * @return claimValid true if the claim is valid, false otherwise
-     */
-    function isClaimValid(IIdentity _identity, uint256 claimTopic, bytes memory sig, bytes memory data) public override view returns (bool claimValid)
+    function isClaimValid(
+        IIdentity _identity,
+        uint256 claimTopic,
+        bytes memory sig,
+        bytes memory data)
+    external override view returns (bool claimValid)
     {
         bytes32 dataHash = keccak256(abi.encode(_identity, claimTopic, data));
         // Use abi.encodePacked to concatenate the message prefix and the message to sign.
@@ -74,6 +69,20 @@ contract ClaimIssuer is IClaimIssuer, Identity {
         return false;
     }
 
+    /**
+     *  @dev See {IClaimIssuer-isClaimRevoked}.
+     */
+    function isClaimRevoked(bytes memory _sig) public override view returns (bool) {
+        if (revokedClaims[_sig]) {
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     *  @dev See {IClaimIssuer-getRecoveredAddress}.
+     */
     function getRecoveredAddress(bytes memory sig, bytes32 dataHash)
         public override
         pure
@@ -89,6 +98,7 @@ contract ClaimIssuer is IClaimIssuer, Identity {
         }
 
         // Divide the signature in r, s and v variables
+        // solhint-disable-next-line no-inline-assembly
         assembly {
             ra := mload(add(sig, 32))
             sa := mload(add(sig, 64))