Merge pull request #87 from gnosisguild/connection-fixes

Fixes for connection setup and Roles v2 specific error handling

extension/package.json

@@ -41,12 +41,12 @@
     "@types/react-modal": "^3.16.3",
     "@typescript-eslint/eslint-plugin": "^5.48.1",
     "@typescript-eslint/parser": "^5.48.1",
-    "@walletconnect/core": "^2.10.6",
-    "@walletconnect/ethereum-provider": "^2.10.6",
+    "@walletconnect/core": "^2.11.1",
+    "@walletconnect/ethereum-provider": "^2.11.1",
     "@walletconnect/keyvaluestorage": "^1.1.1",
     "@walletconnect/modal": "^2.6.2",
-    "@walletconnect/sign-client": "^2.10.6",
-    "@walletconnect/universal-provider": "^2.10.6",
+    "@walletconnect/sign-client": "^2.11.1",
+    "@walletconnect/universal-provider": "^2.11.1",
     "classnames": "^2.3.1",
     "copy-to-clipboard": "^3.3.1",
     "cspell": "^5.21.2",
@@ -84,4 +84,4 @@
     "typescript-plugin-css-modules": "^3.4.0"
   },
   "packageManager": "[email protected]"
-}
+}

extension/src/browser/Drawer/RolePermissionCheck.tsx

@@ -6,13 +6,15 @@ import { encodeSingle, TransactionInput } from 'react-multisend'
 import { Flex, Tag } from '../../components'
 import { useApplicableTranslation } from '../../transactionTranslations'
 import { JsonRpcError } from '../../types'
-import { decodeRolesError } from '../../utils'
-import { isPermissionsError } from '../../utils/decodeRolesError'
+import { decodeRolesV1Error } from '../../utils'
+import { decodeRolesV2Error, isPermissionsError } from '../../utils/decodeError'
 import { useWrappingProvider } from '../ProvideProvider'
 
 import CopyToClipboard from './CopyToClipboard'
 import { Translate } from './Translate'
 import classes from './style.module.css'
+import { useConnection } from '../../connections'
+import { KnownContracts } from '@gnosis.pm/zodiac'
 
 const RolePermissionCheck: React.FC<{
   transaction: TransactionInput
@@ -22,6 +24,9 @@ const RolePermissionCheck: React.FC<{
 }> = ({ transaction, isDelegateCall, index, mini = false }) => {
   const [error, setError] = useState<string | undefined | false>(undefined)
   const wrappingProvider = useWrappingProvider()
+  const {
+    connection: { moduleType },
+  } = useConnection()
 
   const encodedTransaction = encodeSingle(transaction)
   const translationAvailable = !!useApplicableTranslation(encodedTransaction)
@@ -37,19 +42,26 @@ const RolePermissionCheck: React.FC<{
         if (!canceled) setError(false)
       })
       .catch((e: JsonRpcError) => {
-        const decodedError = decodeRolesError(e)
-        if (!canceled) {
-          setError(isPermissionsError(decodedError) ? decodedError : false)
+        const decodedError =
+          moduleType === KnownContracts.ROLES_V1
+            ? decodeRolesV1Error(e)
+            : decodeRolesV2Error(e)
+        if (!canceled && decodedError) {
+          setError(
+            isPermissionsError(decodedError.signature)
+              ? decodedError.signature
+              : false
+          )
         }
-        if (!isPermissionsError(decodedError)) {
+        if (!decodedError || !isPermissionsError(decodedError.signature)) {
           throw e
         }
       })
 
     return () => {
       canceled = true
     }
-  }, [wrappingProvider, encodedTransaction])
+  }, [wrappingProvider, moduleType, encodedTransaction])
 
   if (error === undefined) return null
 

extension/src/browser/Drawer/Submit.tsx

@@ -10,7 +10,11 @@ import { waitForMultisigExecution } from '../../providers'
 // import { shallExecuteDirectly } from '../../safe/sendTransaction'
 import { useConnection } from '../../connections'
 import { JsonRpcError, ProviderType } from '../../types'
-import { decodeRolesError } from '../../utils'
+import {
+  decodeGenericError,
+  decodeRolesV1Error,
+  decodeRolesV2Error,
+} from '../../utils'
 import { useSubmitTransactions } from '../ProvideProvider'
 import { useDispatch, useNewTransactions } from '../state'
 
@@ -47,11 +51,14 @@ const Submit: React.FC = () => {
       console.warn(e)
       setSignaturePending(false)
       const err = e as JsonRpcError
+
+      const { message } = decodeRolesV1Error(err) ||
+        decodeRolesV2Error(err) || { message: decodeGenericError(err) }
       toast.error(
         <>
           <p>Submitting the transaction batch failed:</p>
           <br />
-          <code>{decodeRolesError(err)}</code>
+          <code>{message}</code>
         </>,
         { className: toastClasses.toastError }
       )

extension/src/browser/Drawer/Transaction.tsx

@@ -127,7 +127,9 @@ export const Transaction: React.FC<Props> = ({
   const [expanded, setExpanded] = useState(true)
   const { connection } = useConnection()
   const elementRef = useScrollIntoView(scrollIntoView)
-  const showRoles = connection.moduleType === KnownContracts.ROLES_V1
+  const showRoles =
+    connection.moduleType === KnownContracts.ROLES_V1 ||
+    connection.moduleType === KnownContracts.ROLES_V2
 
   return (
     <Box ref={elementRef} p={2} className={classes.container}>
@@ -179,7 +181,9 @@ export const TransactionBadge: React.FC<Props> = ({
   scrollIntoView,
 }) => {
   const { connection } = useConnection()
-  const showRoles = connection.moduleType === KnownContracts.ROLES_V1
+  const showRoles =
+    connection.moduleType === KnownContracts.ROLES_V1 ||
+    connection.moduleType === KnownContracts.ROLES_V2
 
   const elementRef = useScrollIntoView(scrollIntoView)
 

extension/src/connections/ConnectionsDrawer/ConnectButton/index.tsx

@@ -31,7 +31,7 @@ const ConnectButton: React.FC<{ id: string }> = ({ id }) => {
   const { connected, connection } = useConnection(id)
 
   const metamask = useMetaMask()
-  const walletConnect = useWalletConnect(connection.id, connection.chainId || 1)
+  const walletConnect = useWalletConnect(connection.id)
 
   const connect = (
     providerType: ProviderType,
@@ -88,12 +88,58 @@ const ConnectButton: React.FC<{ id: string }> = ({ id }) => {
     )
   }
 
-  // right account, wrong network
+  // WalletConnect: wrong chain
+  if (
+    connection.providerType === ProviderType.WalletConnect &&
+    walletConnect &&
+    connection.pilotAddress &&
+    walletConnect.accounts.some(
+      (acc) => acc.toLowerCase() === connection.pilotAddress
+    ) &&
+    connection.chainId &&
+    walletConnect.chainId !== connection.chainId
+  ) {
+    return (
+      <div
+        className={classNames(
+          classes.connectedContainer,
+          classes.connectionWarning
+        )}
+      >
+        <code className={classes.pilotAddress}>
+          {validateAddress(connection.pilotAddress)}
+        </code>
+        <Tag head={<RiAlertLine />} color="warning">
+          Chain mismatch
+        </Tag>
+        {walletConnect.chainId && (
+          <Button
+            className={classes.disconnectButton}
+            onClick={() => {
+              connect(
+                ProviderType.WalletConnect,
+                walletConnect.chainId as ChainId,
+                connection.pilotAddress
+              )
+            }}
+          >
+            Connect to{' '}
+            {CHAIN_NAME[walletConnect.chainId as ChainId] ||
+              `#${walletConnect.chainId}`}
+          </Button>
+        )}
+      </div>
+    )
+  }
+
+  // Metamask: right account, wrong chain
   if (
     connection.providerType === ProviderType.MetaMask &&
     metamask.provider &&
     connection.pilotAddress &&
-    metamask.accounts.includes(connection.pilotAddress) &&
+    metamask.accounts.some(
+      (acc) => acc.toLowerCase() === connection.pilotAddress
+    ) &&
     connection.chainId &&
     metamask.chainId !== connection.chainId
   ) {
@@ -104,18 +150,25 @@ const ConnectButton: React.FC<{ id: string }> = ({ id }) => {
           classes.connectionWarning
         )}
       >
+        <code className={classes.pilotAddress}>
+          {validateAddress(connection.pilotAddress)}
+        </code>
         <Tag head={<RiAlertLine />} color="warning">
-          Network mismatch
+          Chain mismatch
         </Tag>
-        <Button
-          className={classes.disconnectButton}
-          onClick={() => {
-            metamask.switchChain(connection.chainId)
-          }}
-        >
-          Switch wallet to{' '}
-          {CHAIN_NAME[connection.chainId] || `#${connection.chainId}`}
-        </Button>
+        {connection.chainId && (
+          <Button
+            className={classes.disconnectButton}
+            onClick={() => {
+              if (connection.chainId) {
+                metamask.switchChain(connection.chainId)
+              }
+            }}
+          >
+            Switch wallet to{' '}
+            {CHAIN_NAME[connection.chainId] || `#${connection.chainId}`}
+          </Button>
+        )}
 
         {metamask.chainId && (
           <Button
@@ -136,12 +189,14 @@ const ConnectButton: React.FC<{ id: string }> = ({ id }) => {
     )
   }
 
-  // wrong account
+  // MetaMask: wrong account
   if (
     connection.providerType === ProviderType.MetaMask &&
     metamask.provider &&
     connection.pilotAddress &&
-    !metamask.accounts.includes(connection.pilotAddress)
+    !metamask.accounts.some(
+      (acc) => acc.toLowerCase() === connection.pilotAddress
+    )
   ) {
     return (
       <div className={classes.connectedContainer}>

extension/src/connections/ConnectionsDrawer/Edit/index.tsx

@@ -1,7 +1,6 @@
 import { KnownContracts } from '@gnosis.pm/zodiac'
 import React, { useEffect } from 'react'
 import { RiDeleteBinLine } from 'react-icons/ri'
-import { formatBytes32String } from 'ethers/lib/utils'
 
 import { Box, Button, Field, Flex, IconButton } from '../../../components'
 import { useConfirmationModal } from '../../../components/ConfirmationModal'
@@ -25,6 +24,7 @@ import useConnectionDryRun from '../../useConnectionDryRun'
 import { useClearTransactions } from '../../../browser/state/transactionHooks'
 
 import classes from './style.module.css'
+import { decodeRoleKey, encodeRoleKey } from '../../../utils'
 
 interface Props {
   connectionId: string
@@ -60,6 +60,8 @@ const EditConnection: React.FC<Props> = ({ connectionId, onLaunched }) => {
   const { safes } = useSafesWithOwner(pilotAddress, connectionId)
   const { delegates } = useSafeDelegates(avatarAddress, connectionId)
 
+  const decodedRoleKey = roleId && decodeRoleKey(roleId)
+
   // TODO modules is a nested list, but we currently only render the top-level items
   const {
     loading: loadingMods,
@@ -267,10 +269,11 @@ const EditConnection: React.FC<Props> = ({ connectionId, onLaunched }) => {
               <Field label="Role Key">
                 <input
                   type="text"
-                  value={roleId}
+                  key={connection.id} // makes sure the defaultValue is reset when switching connections
+                  defaultValue={decodedRoleKey || roleId}
                   onChange={(ev) => {
                     try {
-                      const roleId = parseRoleKey(ev.target.value)
+                      const roleId = encodeRoleKey(ev.target.value)
                       setRoleIdError(null)
                       updateConnection({ roleId })
                     } catch (e) {
@@ -280,6 +283,7 @@ const EditConnection: React.FC<Props> = ({ connectionId, onLaunched }) => {
                   }}
                   placeholder="Enter key as bytes32 hex string or in human-readable decoding"
                 />
+
                 {roleIdError && (
                   <Box p={3} className={classes.error}>
                     {roleIdError}
@@ -296,16 +300,3 @@ const EditConnection: React.FC<Props> = ({ connectionId, onLaunched }) => {
 }
 
 export default EditConnection
-
-const parseRoleKey = (key: string) => {
-  if (key.length === 66 && key.startsWith('0x')) {
-    const keyLower = key.toLowerCase()
-    // validate bytes32 hex string
-    if (!keyLower.match(/^0x[0-9a-f]{64}$/)) {
-      throw new Error('Invalid hex string')
-    }
-    return key.toLowerCase()
-  }
-
-  return formatBytes32String(key)
-}

extension/src/connections/connectionHooks.tsx

@@ -123,7 +123,7 @@ export const useConnection = (id?: string) => {
   }
 
   const metamask = useMetaMask()
-  const walletConnect = useWalletConnect(connection.id, connection.chainId || 1)
+  const walletConnect = useWalletConnect(connection.id)
   const defaultProvider = getEip1193ReadOnlyProvider(
     connection.chainId,
     connection.pilotAddress
@@ -138,8 +138,8 @@ export const useConnection = (id?: string) => {
     connection.providerType === ProviderType.MetaMask
       ? metamask
       : walletConnect,
-    connection.chainId,
-    connection.pilotAddress
+    connection.pilotAddress,
+    connection.chainId
   )
 
   const chainId =
@@ -155,7 +155,7 @@ export const useConnection = (id?: string) => {
     !connected &&
     !!validateAddress(pilotAddress) &&
     connection.providerType === ProviderType.MetaMask &&
-    metamask.accounts.includes(pilotAddress) &&
+    metamask.accounts.some((acc) => acc.toLowerCase() === pilotAddress) &&
     metamask.chainId !== connection.chainId
 
   const connectMetaMask = metamask.connect
@@ -169,7 +169,7 @@ export const useConnection = (id?: string) => {
   }, [mustConnectMetaMask, connectMetaMask])
 
   const connect = useCallback(async () => {
-    if (chainId !== requiredChainId) {
+    if (requiredChainId && chainId !== requiredChainId) {
       try {
         await switchChain(requiredChainId)
       } catch (e) {
@@ -194,14 +194,15 @@ export const useConnection = (id?: string) => {
 
 const isConnectedTo = (
   providerContext: MetaMaskContextT | WalletConnectResult | null,
-  chainId: number,
-  account: string
+  account: string,
+  chainId?: number
 ) => {
   if (!providerContext) return false
   const accountLower = account.toLowerCase()
+
   return (
     providerContext &&
-    providerContext.chainId === chainId &&
+    (!chainId || chainId === providerContext.chainId) &&
     providerContext.accounts?.some(
       (acc) => acc.toLowerCase() === accountLower
     ) &&