refactor transaction confirmation owner property

src/logic/addressBook/store/selectors/index.js

@@ -1,7 +1,6 @@
 /* eslint-disable import/named */
 // @flow
 import { List, Map } from 'immutable'
-import { useSelector } from 'react-redux'
 import { Selector, createSelector } from 'reselect'
 
 import type { AddressBook } from '~/logic/addressBook/model/addressBook'
@@ -36,14 +35,15 @@ export const getAddressBookListSelector: Selector<GlobalState, {}, List<AddressB
   },
 )
 
-export const getNameFromAddressBook = (userAddress: string): string | null => {
-  if (!userAddress) {
-    return null
-  }
-  const addressBook = useSelector(getAddressBook)
-  const result = addressBook.filter((addressBookItem) => addressBookItem.address === userAddress)
-  if (result.size > 0) {
-    return result.get(0).name
-  }
-  return null
-}
+export const getNameFromAddressBook = (userAddress: string): string | null =>
+  createSelector(getAddressBook, (addressBook: AddressBook, address: string) => {
+    if (!address) {
+      return 'UNKNOWN'
+    }
+
+    const adbkEntry = addressBook.find((addressBookItem) => addressBookItem.address === userAddress)
+    if (adbkEntry) {
+      return adbkEntry.name
+    }
+    return 'UNKNOWN'
+  })

src/logic/addressBook/utils/index.js

@@ -1,8 +1,5 @@
 // @flow
-import { useSelector } from 'react-redux'
-
 import type { AddressBook, AddressBookProps } from '~/logic/addressBook/model/addressBook'
-import { getAddressBook } from '~/logic/addressBook/store/selectors'
 import type { Owner } from '~/routes/safe/store/models/owner'
 import { loadFromStorage, saveToStorage } from '~/utils/storage'
 
@@ -33,14 +30,6 @@ const getNameFromAdbk = (addressBook: AddressBook, userAddress: string): string
   return null
 }
 
-export const getNameFromAddressBook = (userAddress: string): string | null => {
-  if (!userAddress) {
-    return null
-  }
-  const addressBook = useSelector(getAddressBook)
-  return getNameFromAdbk(addressBook, userAddress)
-}
-
 export const getOwnersWithNameFromAddressBook = (addressBook: AddressBook, ownerList: List<Owner>) => {
   if (!ownerList) {
     return []

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/IncomingTxDescription/index.jsx

@@ -1,11 +1,12 @@
 // @flow
 import { makeStyles } from '@material-ui/core/styles'
 import React from 'react'
+import { useSelector } from 'react-redux'
 
 import EtherscanLink from '~/components/EtherscanLink'
 import Block from '~/components/layout/Block'
 import Bold from '~/components/layout/Bold'
-import { getNameFromAddressBook } from '~/logic/addressBook/utils'
+import { getNameFromAddressBook } from '~/logic/addressBook/store/selectors'
 import OwnerAddressTableCell from '~/routes/safe/components/Settings/ManageOwners/OwnerAddressTableCell'
 import { getIncomingTxAmount } from '~/routes/safe/components/Transactions/TxsTable/columns'
 import type { IncomingTransaction } from '~/routes/safe/store/models/incomingTransaction'
@@ -46,7 +47,7 @@ const TransferDescription = ({ from, txFromName, value = '' }: TransferDescProps
 
 const IncomingTxDescription = ({ tx }: Props) => {
   const classes = useStyles()
-  const txFromName = getNameFromAddressBook(tx.from)
+  const txFromName = useSelector(getNameFromAddressBook(tx.from))
   return (
     <Block className={classes.txDataContainer}>
       <TransferDescription from={tx.from} txFromName={txFromName} value={getIncomingTxAmount(tx)} />

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/OwnersColumn/OwnerComponent.jsx

@@ -2,6 +2,7 @@
 import { withStyles } from '@material-ui/core/styles'
 import cn from 'classnames'
 import React from 'react'
+import { useSelector } from 'react-redux'
 
 import CancelSmallFilledCircle from './assets/cancel-small-filled.svg'
 import ConfirmSmallFilledCircle from './assets/confirm-small-filled.svg'
@@ -16,8 +17,7 @@ import Block from '~/components/layout/Block'
 import Button from '~/components/layout/Button'
 import Img from '~/components/layout/Img'
 import Paragraph from '~/components/layout/Paragraph'
-import { getNameFromAddressBook } from '~/logic/addressBook/utils'
-import { type Owner } from '~/routes/safe/store/models/owner'
+import { getNameFromAddressBook } from '~/logic/addressBook/store/selectors'
 
 export const CONFIRM_TX_BTN_TEST_ID = 'confirm-btn'
 export const EXECUTE_TX_BTN_TEST_ID = 'execute-btn'
@@ -32,7 +32,7 @@ type OwnerProps = {
   onTxReject?: Function,
   onTxConfirm: Function,
   onTxExecute: Function,
-  owner: Owner,
+  owner: string,
   showRejectBtn: boolean,
   showExecuteRejectBtn: boolean,
   showConfirmBtn: boolean,
@@ -57,8 +57,8 @@ const OwnerComponent = ({
   thresholdReached,
   userAddress,
 }: OwnerProps) => {
-  const nameInAdbk = getNameFromAddressBook(owner.address)
-  const ownerName = nameInAdbk || owner.name
+  const nameInAdbk = useSelector(getNameFromAddressBook(owner))
+  const ownerName = nameInAdbk || 'UNKNOWN'
   const [imgCircle, setImgCircle] = React.useState(ConfirmSmallGreyCircle)
 
   React.useMemo(() => {
@@ -77,15 +77,15 @@ const OwnerComponent = ({
       <div className={classes.circleState}>
         <Img alt="" src={imgCircle} />
       </div>
-      <Identicon address={owner.address} className={classes.icon} diameter={32} />
+      <Identicon address={owner} className={classes.icon} diameter={32} />
       <Block>
         <Paragraph className={classes.name} noMargin>
           {ownerName}
         </Paragraph>
-        <EtherscanLink className={classes.address} cut={4} type="address" value={owner.address} />
+        <EtherscanLink className={classes.address} cut={4} type="address" value={owner} />
       </Block>
       <Block className={classes.spacer} />
-      {owner.address === userAddress && (
+      {owner === userAddress && (
         <Block>
           {isCancelTx ? (
             <>

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/OwnersColumn/OwnersList.jsx

@@ -48,7 +48,7 @@ const OwnersList = ({
         confirmed
         executor={executor}
         isCancelTx={isCancelTx}
-        key={owner.address}
+        key={owner}
         onTxExecute={onTxExecute}
         onTxReject={onTxReject}
         owner={owner}
@@ -64,7 +64,7 @@ const OwnersList = ({
         classes={classes}
         executor={executor}
         isCancelTx={isCancelTx}
-        key={owner.address}
+        key={owner}
         onTxConfirm={onTxConfirm}
         onTxExecute={onTxExecute}
         onTxReject={onTxReject}

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/OwnersColumn/index.jsx

@@ -41,7 +41,7 @@ function getOwnersConfirmations(tx, userAddress) {
   let currentUserAlreadyConfirmed = false
 
   tx.confirmations.forEach((conf) => {
-    if (conf.owner.address === userAddress) {
+    if (conf.owner === userAddress) {
       currentUserAlreadyConfirmed = true
     }
 
@@ -54,18 +54,20 @@ function getOwnersConfirmations(tx, userAddress) {
 }
 
 function getPendingOwnersConfirmations(owners, tx, userAddress) {
-  const ownersUnconfirmed = owners.filter(
-    (owner) => tx.confirmations.findIndex((conf) => conf.owner.address === owner.address) === -1,
-  )
+  const ownersNotConfirmed = []
+  let currentUserNotConfirmed = true
 
-  let userIsUnconfirmedOwner = false
-
-  ownersUnconfirmed.some((owner) => {
-    userIsUnconfirmedOwner = owner.address === userAddress
-    return userIsUnconfirmedOwner
+  owners.forEach((owner) => {
+    const confirmationsEntry = tx.confirmations.find((conf) => conf.owner === owner.address)
+    if (!confirmationsEntry) {
+      ownersNotConfirmed.push(owner.address)
+    }
+    if (confirmationsEntry && confirmationsEntry.owner === userAddress) {
+      currentUserNotConfirmed = false
+    }
   })
 
-  return [ownersUnconfirmed, userIsUnconfirmedOwner]
+  return [ownersNotConfirmed, currentUserNotConfirmed]
 }
 
 const OwnersColumn = ({

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/TxDescription/index.jsx

@@ -1,6 +1,7 @@
 // @flow
 import { withStyles } from '@material-ui/core/styles'
 import React, { useState } from 'react'
+import { useSelector } from 'react-redux'
 
 import { getTxData } from './utils'
 
@@ -9,7 +10,7 @@ import Block from '~/components/layout/Block'
 import Bold from '~/components/layout/Bold'
 import LinkWithRef from '~/components/layout/Link'
 import Paragraph from '~/components/layout/Paragraph'
-import { getNameFromAddressBook } from '~/logic/addressBook/utils'
+import { getNameFromAddressBook } from '~/logic/addressBook/store/selectors'
 import { SAFE_METHODS_NAMES } from '~/logic/contracts/methodIds'
 import { shortVersionOf } from '~/logic/wallets/ethAddresses'
 import OwnerAddressTableCell from '~/routes/safe/components/Settings/ManageOwners/OwnerAddressTableCell'
@@ -68,7 +69,7 @@ type CustomDescProps = {
 }
 
 const TransferDescription = ({ amount = '', recipient }: TransferDescProps) => {
-  const recipientName = getNameFromAddressBook(recipient)
+  const recipientName = useSelector(getNameFromAddressBook(recipient))
   return (
     <Block data-testid={TRANSACTIONS_DESC_SEND_TEST_ID}>
       <Bold>Send {amount} to:</Bold>
@@ -82,7 +83,7 @@ const TransferDescription = ({ amount = '', recipient }: TransferDescProps) => {
 }
 
 const RemovedOwner = ({ removedOwner }: { removedOwner: string }) => {
-  const ownerChangedName = getNameFromAddressBook(removedOwner)
+  const ownerChangedName = useSelector(getNameFromAddressBook(removedOwner))
 
   return (
     <Block data-testid={TRANSACTIONS_DESC_REMOVE_OWNER_TEST_ID}>
@@ -97,7 +98,7 @@ const RemovedOwner = ({ removedOwner }: { removedOwner: string }) => {
 }
 
 const AddedOwner = ({ addedOwner }: { addedOwner: string }) => {
-  const ownerChangedName = getNameFromAddressBook(addedOwner)
+  const ownerChangedName = useSelector(getNameFromAddressBook(addedOwner))
 
   return (
     <Block data-testid={TRANSACTIONS_DESC_ADD_OWNER_TEST_ID}>
@@ -161,7 +162,7 @@ const SettingsDescription = ({ action, addedOwner, newThreshold, removedOwner }:
 
 const CustomDescription = ({ amount = 0, classes, data, recipient }: CustomDescProps) => {
   const [showTxData, setShowTxData] = useState(false)
-  const recipientName = getNameFromAddressBook(recipient)
+  const recipientName = useSelector(getNameFromAddressBook(recipient))
   return (
     <>
       <Block data-testid={TRANSACTIONS_DESC_CUSTOM_VALUE_TEST_ID}>

src/routes/safe/container/selector.js

@@ -58,7 +58,7 @@ const getTxStatus = (tx: Transaction, userAddress: string, safe: Safe): Transact
   } else if (!tx.confirmations.size) {
     txStatus = 'pending'
   } else {
-    const userConfirmed = tx.confirmations.filter((conf) => conf.owner.address === userAddress).size === 1
+    const userConfirmed = tx.confirmations.filter((conf) => conf.owner === userAddress).size === 1
     const userIsSafeOwner = safe.owners.filter((owner) => owner.address === userAddress).size === 1
     txStatus = !userConfirmed && userIsSafeOwner ? 'awaiting_your_confirmation' : 'awaiting_confirmations'
   }
@@ -122,11 +122,23 @@ const extendedTransactionsSelector: Selector<
   safeTransactionsSelector,
   safeCancellationTransactionsSelector,
   safeIncomingTransactionsSelector,
-  (safe, userAddress, transactions, cancellationTransactions, incomingTransactions) => {
+  getAddressBook,
+  (safe, userAddress, transactions, cancellationTransactions, incomingTransactions, addressBook) => {
     const cancellationTransactionsByNonce = cancellationTransactions.reduce((acc, tx) => acc.set(tx.nonce, tx), Map())
     const extendedTransactions = transactions.map((tx: Transaction) => {
       let extendedTx = tx
 
+      // add owner names to confirmations
+      const txConfirmations = tx.get('confirmations').map((confirmation) => {
+        const confirmationOwnerAdbkEntry = addressBook.find((adbkEntry) => adbkEntry.address === confirmation.owner)
+
+        return confirmation.set(
+          'ownerName',
+          (confirmationOwnerAdbkEntry && confirmationOwnerAdbkEntry.name) || 'UNKNOWN',
+        )
+      })
+      extendedTx = tx.set('confirmations', txConfirmations)
+
       if (!tx.isExecuted) {
         if (
           (cancellationTransactionsByNonce.get(tx.nonce) &&

src/routes/safe/store/actions/fetchTransactions.js

@@ -11,7 +11,6 @@ import { addTransactions } from './addTransactions'
 import { decodeParamsFromSafeMethod } from '~/logic/contracts/methodIds'
 import { buildIncomingTxServiceUrl } from '~/logic/safe/transactions/incomingTxHistory'
 import { type TxServiceType, buildTxServiceUrl } from '~/logic/safe/transactions/txHistory'
-import { getLocalSafe } from '~/logic/safe/utils'
 import { getTokenInfos } from '~/logic/tokens/store/actions/fetchTokens'
 import { ALTERNATIVE_TOKEN_ABI } from '~/logic/tokens/utils/alternativeAbi'
 import {
@@ -27,7 +26,6 @@ import { getWeb3 } from '~/logic/wallets/getWeb3'
 import { addCancellationTransactions } from '~/routes/safe/store/actions/addCancellationTransactions'
 import { makeConfirmation } from '~/routes/safe/store/models/confirmation'
 import { type IncomingTransaction, makeIncomingTransaction } from '~/routes/safe/store/models/incomingTransaction'
-import { makeOwner } from '~/routes/safe/store/models/owner'
 import type { TransactionProps } from '~/routes/safe/store/models/transaction'
 import { type Transaction, makeTransaction } from '~/routes/safe/store/models/transaction'
 import { type GlobalState } from '~/store'
@@ -74,27 +72,15 @@ type IncomingTxServiceModel = {
 }
 
 export const buildTransactionFrom = async (safeAddress: string, tx: TxServiceModel): Promise<Transaction> => {
-  const localSafe = await getLocalSafe(safeAddress)
-
   const confirmations = List(
-    tx.confirmations.map((conf: ConfirmationServiceModel) => {
-      let ownerName = 'UNKNOWN'
-
-      if (localSafe && localSafe.owners) {
-        const storedOwner = localSafe.owners.find((owner) => sameAddress(conf.owner, owner.address))
-
-        if (storedOwner) {
-          ownerName = storedOwner.name
-        }
-      }
-
-      return makeConfirmation({
-        owner: makeOwner({ address: conf.owner, name: ownerName }),
+    tx.confirmations.map((conf: ConfirmationServiceModel) =>
+      makeConfirmation({
+        owner: conf.owner,
         type: ((conf.confirmationType.toLowerCase(): any): TxServiceType),
         hash: conf.transactionHash,
         signature: conf.signature,
-      })
-    }),
+      }),
+    ),
   )
   const modifySettingsTx = sameAddress(tx.to, safeAddress) && Number(tx.value) === 0 && !!tx.data
   const cancellationTx = sameAddress(tx.to, safeAddress) && Number(tx.value) === 0 && !tx.data

src/routes/safe/store/models/confirmation.js

@@ -3,17 +3,16 @@ import { Record } from 'immutable'
 import type { RecordFactory, RecordOf } from 'immutable'
 
 import { type TxServiceType } from '~/logic/safe/transactions/txHistory'
-import { type Owner, makeOwner } from '~/routes/safe/store/models/owner'
 
 export type ConfirmationProps = {
-  owner: Owner,
+  owner: string,
   type: TxServiceType,
   hash: string,
   signature?: string,
 }
 
 export const makeConfirmation: RecordFactory<ConfirmationProps> = Record({
-  owner: makeOwner(),
+  owner: '',
   type: 'initialised',
   hash: '',
   signature: null,

src/routes/safe/store/selectors/index.js

@@ -5,7 +5,6 @@ import { type OutputSelector, createSelector, createStructuredSelector } from 'r
 
 import { getWeb3 } from '~/logic/wallets/getWeb3'
 import { SAFELIST_ADDRESS, SAFE_PARAM_ADDRESS } from '~/routes/routes'
-import { type Confirmation } from '~/routes/safe/store/models/confirmation'
 import type { IncomingTransaction } from '~/routes/safe/store/models/incomingTransaction'
 import { type Safe } from '~/routes/safe/store/models/safe'
 import { type Transaction } from '~/routes/safe/store/models/transaction'
@@ -29,10 +28,6 @@ export type SafeProps = {
   safeAddress: string,
 }
 
-type TransactionProps = {
-  transaction: Transaction,
-}
-
 const safesStateSelector = (state: GlobalState): Map<string, *> => state[SAFE_REDUCER_ID]
 
 export const safesMapSelector = (state: GlobalState): Map<string, Safe> => state[SAFE_REDUCER_ID].get('safes')
@@ -68,8 +63,6 @@ const cancellationTransactionsSelector = (state: GlobalState): CancelTransaction
 const incomingTransactionsSelector = (state: GlobalState): IncomingTransactionsState =>
   state[INCOMING_TRANSACTIONS_REDUCER_ID]
 
-const oneTransactionSelector = (state: GlobalState, props: TransactionProps) => props.transaction
-
 export const safeParamAddressSelector = (state: GlobalState, props: RouterProps) => {
   const urlAdd = props.match.params[SAFE_PARAM_ADDRESS]
   return urlAdd ? getWeb3().utils.toChecksumAddress(urlAdd) : ''
@@ -148,22 +141,6 @@ export const safeIncomingTransactionsSelector: IncomingTxSelectorType = createSe
   },
 )
 
-export const confirmationsTransactionSelector: OutputSelector<GlobalState, TransactionProps, number> = createSelector(
-  oneTransactionSelector,
-  (tx: Transaction) => {
-    if (!tx) {
-      return 0
-    }
-
-    const confirmations: List<Confirmation> = tx.get('confirmations')
-    if (!confirmations) {
-      return 0
-    }
-
-    return confirmations.filter((confirmation: Confirmation) => confirmation.get('type') === 'confirmation').count()
-  },
-)
-
 export type SafeSelectorProps = Safe | typeof undefined
 
 export const safeSelector: OutputSelector<GlobalState, RouterProps, SafeSelectorProps> = createSelector(