Merge pull request #6 from peeramid-labs/security-fixes

Code review fixes

.changeset/odd-islands-teach.md

@@ -0,0 +1,5 @@
+---
+'@peeramid-labs/multipass': minor
+---
+
+added renewal fees and ability to change them

.changeset/odd-pets-sleep.md

@@ -0,0 +1,18 @@
+---
+'@peeramid-labs/multipass': major
+---
+
+- Added `validUntil` property to domain records
+- Added ability to renew records and corresponding nonce checks enforced. 
+- Reduced some interface visibility to follow least permission principle
+- Removed diamond proxy in favor of more simplistic transparent proxy pattern
+- Moved to solidiy compiler version to 0.8.28
+- Removed unusued internal functions and events from interfaces
+- Removed boolean literals from conditional expressions
+- Removed withdraw funds interface. All funds now are sent to owner by default.
+- Added security contact email address to docstrings 
+
+
+## Breaking changes
+Register() interface which now has less arguments and uses internal struct parameters, remove unused arguments 
+

deploy/01_Multipass.ts

@@ -3,27 +3,50 @@ import { DeployFunction } from 'hardhat-deploy/types';
 import { MULTIPASS_CONTRACT_VERSION, MULTIPASS_CONTRACT_NAME } from '../test/utils';
 import { getProcessEnv } from '../scripts/libraries/utils';
 import { ethers } from 'hardhat';
-import { MultipassDiamond } from '../types';
+import { Multipass } from '../types';
+// import { MultipassDiamond } from '../types';
 const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => {
   const { deployments, getNamedAccounts } = hre;
-  const { diamond } = deployments;
+  const { diamond, deploy } = deployments;
   const { deployer, owner } = await getNamedAccounts();
-
-  const deployment = await diamond.deploy('Multipass', {
+  const deployment = await deploy('Multipass', {
     from: deployer,
-    owner: deployer,
-    log: true,
-    facets: ['Multipass', 'EIP712InspectorFacet', 'MultipassInit'],
-    execute: {
-      methodName: 'init',
-      args:
-        process.env.NODE_ENV === 'TEST'
-          ? [MULTIPASS_CONTRACT_NAME, MULTIPASS_CONTRACT_VERSION]
-          : [getProcessEnv(false, 'MULTIPASS_CONTRACT_NAME'), getProcessEnv(false, 'MULTIPASS_CONTRACT_VERSION')],
+    proxy: {
+      execute: {
+        init: {
+          methodName: 'initialize',
+          args:
+            process.env.NODE_ENV === 'TEST'
+              ? [MULTIPASS_CONTRACT_NAME, MULTIPASS_CONTRACT_VERSION, owner]
+              : [
+                  getProcessEnv(false, 'MULTIPASS_CONTRACT_NAME'),
+                  getProcessEnv(false, 'MULTIPASS_CONTRACT_VERSION'),
+                  owner,
+                ],
+        },
+      },
+      proxyContract: 'OpenZeppelinTransparentProxy',
     },
+
+    log: true,
+    autoMine: true,
   });
-  const multipass = (await ethers.getContractAt(deployment.abi, deployment.address)) as MultipassDiamond;
-  await multipass.connect(await hre.ethers.getSigner(deployer)).transferOwnership(owner);
+
+  //   const deployment = await diamond.deploy('Multipass', {
+  //     from: deployer,
+  //     owner: deployer,
+  //     log: true,
+  //     facets: ['Multipass', 'EIP712InspectorFacet', 'MultipassInit'],
+  //     execute: {
+  //       methodName: 'init',
+  //       args:
+  //         process.env.NODE_ENV === 'TEST'
+  //           ? [MULTIPASS_CONTRACT_NAME, MULTIPASS_CONTRACT_VERSION]
+  //           : [getProcessEnv(false, 'MULTIPASS_CONTRACT_NAME'), getProcessEnv(false, 'MULTIPASS_CONTRACT_VERSION')],
+  //     },
+  //   });
+//   const multipass = (await ethers.getContractAt(deployment.abi, deployment.address)) as Multipass;
+//   await multipass.connect(await hre.ethers.getSigner(deployer)).transferOwnership(owner);
 };
 
 export default func;

eslint.config.mjs

@@ -0,0 +1,46 @@
+import promise from 'eslint-plugin-promise';
+import globals from 'globals';
+import tsParser from '@typescript-eslint/parser';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import js from '@eslint/js';
+import { FlatCompat } from '@eslint/eslintrc';
+
+const __filename = fileURLToPath(import.meta.url);
+const __dirname = path.dirname(__filename);
+const compat = new FlatCompat({
+  baseDirectory: __dirname,
+  recommendedConfig: js.configs.recommended,
+  allConfig: js.configs.all,
+});
+
+export default [
+  {
+    ignores: ['docs/templates/', '**/node_modules/', 'types/', 'coverage/'],
+  },
+  ...compat.extends('eslint:recommended', 'plugin:promise/recommended', 'prettier'),
+  {
+    plugins: {
+      promise,
+    },
+
+    languageOptions: {
+      globals: {
+        ...globals.browser,
+        ...globals.node,
+        ...globals.mocha,
+        artifacts: 'readonly',
+        contract: 'readonly',
+        assert: 'readonly',
+        web3: 'readonly',
+      },
+
+      parser: tsParser,
+    },
+
+    rules: {
+      'no-unused-vars': 'off',
+    },
+    files: ['**/*.js', '**/*.jsx', '**/*.ts', '**/*.tsx'],
+  },
+];

hardhat.config.ts

@@ -1,10 +1,8 @@
+import 'solidity-coverage';
 import { task } from 'hardhat/config';
 import '@nomicfoundation/hardhat-chai-matchers';
-import 'hardhat-diamond-abi';
 import '@nomicfoundation/hardhat-toolbox';
 import 'hardhat-abi-exporter';
-import { toSignature, isIncluded } from './scripts/diamond';
-import { cutFacets, replaceFacet } from './scripts/libraries/diamond';
 import 'hardhat-gas-reporter';
 import 'hardhat-contract-sizer';
 import 'hardhat-deploy';
@@ -19,30 +17,6 @@ task('accounts', 'Prints the list of accounts', async (taskArgs, hre) => {
   }
 });
 
-task('replaceFacet', 'Upgrades facet')
-  .addParam('facet', 'facet')
-  .addParam('address', 'contract address')
-  .setAction(async (taskArgs, hre) => {
-    const accounts = await hre.ethers.getSigners();
-    await replaceFacet(taskArgs.address, taskArgs.facet, accounts[0]);
-  });
-
-task('addFacet', 'adds a facet')
-  .addParam('facet', 'facet')
-  .addParam('address', 'contract address')
-  .setAction(async (taskArgs, hre) => {
-    const Facet = await hre.ethers.getContractFactory(taskArgs.facet);
-    const accounts = await hre.ethers.getSigners();
-    const facet = await Facet.deploy();
-    await facet.deployed();
-
-    await cutFacets({
-      facets: [facet],
-      diamondAddress: taskArgs.address,
-      signer: accounts[0],
-    });
-  });
-
 export default {
   docgen: {
     outputDir: './docs/contracts',
@@ -116,7 +90,7 @@ export default {
   solidity: {
     compilers: [
       {
-        version: '0.8.20',
+        version: '0.8.28',
         settings: {
           optimizer: {
             enabled: true,
@@ -126,21 +100,6 @@ export default {
       },
     ],
   },
-  diamondAbi: [
-    {
-      // (required) The name of your Diamond ABI
-      name: 'MultipassDiamond',
-      include: ['DNSFacet', 'OwnershipFacet', 'DiamondLoupeFacet', 'EIP712InspectorFacet'],
-      // We explicitly set `strict` to `true` because we want to validate our facets don't accidentally provide overlapping functions
-      strict: true,
-      // We use our diamond utils to filter some functions we ignore from the combined ABI
-      filter(abiElement: unknown, index: number, abi: unknown[], fullyQualifiedName: string) {
-        // const changes = new diamondUtils.DiamondChanges();
-        const signature = toSignature(abiElement);
-        return isIncluded(fullyQualifiedName, signature);
-      },
-    },
-  ],
   typechain: {
     outDir: 'types',
     target: 'ethers-v5',

package.json

@@ -11,8 +11,8 @@
   ],
   "description": "Contracts for public cross domain username query",
   "scripts": {
-    "lint:fix": "eslint ./ --ext js,jsx,ts,tsx --fix && pnpm run lint:sol:fix",
-    "lint": "eslint ./ --ext js,jsx,ts,tsx && pnpm run lint:sol",
+    "lint:fix": "eslint ./ --fix && pnpm run lint:sol:fix",
+    "lint": "eslint ./ && pnpm run lint:sol",
     "upgrade:game:mumbai": "source .secrets/hh_mumbai.env &&  pnpm hardhat --network mumbai deploy --tags upgrade_game",
     "test": "export NODE_ENV=TEST && pnpm hardhat test",
     "test:parallel": "export NODE_ENV=TEST && pnpm hardhat test --parallel",
@@ -46,8 +46,9 @@
     "@types/node-fetch": "^2.6.2",
     "@typescript-eslint/parser": "^6.7.4",
     "chai": "^4.3.6",
+    "globals": "^15.9.0",
     "crypto-js": "^4.1.1",
-    "eslint": "^8.31.0",
+    "eslint": "^9.6.0",
     "eslint-config-prettier": "^9.0.0",
     "eslint-plugin-promise": "^6.1.1",
     "eth-create2-calculator": "^1.1.5",

playbook/initializeDomain.ts

@@ -1,5 +1,5 @@
 import { task, types } from 'hardhat/config';
-import { MultipassDiamond } from '../types';
+import { Multipass } from '../types';
 import { HardhatRuntimeEnvironment } from 'hardhat/types';
 
 task('initializeDomain', 'Initialize domain name and activate it')
@@ -20,7 +20,15 @@ task('initializeDomain', 'Initialize domain name and activate it')
         discount,
         registrarAddress,
         activate,
-      }: { domain: string; freeRegistrationsNumber: string; fee: string; reward: string; discount: string; registrarAddress: string; activate: boolean },
+      }: {
+        domain: string;
+        freeRegistrationsNumber: string;
+        fee: string;
+        reward: string;
+        discount: string;
+        registrarAddress: string;
+        activate: boolean;
+      },
       hre,
     ) => {
       const { deployments, getNamedAccounts } = hre;
@@ -31,22 +39,20 @@ task('initializeDomain', 'Initialize domain name and activate it')
         multipassDeployment.address,
         multipassDeployment.abi,
         hre.ethers.provider.getSigner(owner),
-      ) as MultipassDiamond;
+      ) as Multipass;
       const tx = await multipassContract.initializeDomain(
         registrarAddress,
         freeRegistrationsNumber,
-        hre.ethers.utils.parseEther(fee),
         hre.ethers.utils.formatBytes32String(domain),
         hre.ethers.utils.parseEther(reward),
         hre.ethers.utils.parseEther(discount),
       );
       console.log(tx.wait(1));
 
-      if(activate === true) {
-        const tx = await multipassContract
-          .activateDomain(hre.ethers.utils.formatBytes32String(domain));
+      if (activate) {
+        const tx = await multipassContract.activateDomain(hre.ethers.utils.formatBytes32String(domain));
         console.log(tx.wait(1));
-        console.log('Domain name "' + domain + '" successfully initialized and activated!')
+        console.log('Domain name "' + domain + '" successfully initialized and activated!');
       }
     },
   );