V1.6.1 - Bugfixes (#432)

* Change label for cancelled tx to 'cancelled' (#393)

* Feature 309: Listen for web3connect disconnect event (#324)

* add 'disconnect' event listener for web3connect

* Update web3connect, set preventDuplicate to true for disconnected message

Co-authored-by: Germán Martínez <germartinez@users.noreply.github.com>

* Update package.json

* Fix signatures order in txData (#429)

* Fix signatures order in tx data

* extract generateSignaturesFromTxConfirmations to a separate file, rewrite test for signatures

Co-authored-by: Mikhail Mikheev <mmvsha73@gmail.com>

* check if txNonce is defined in createTransaction, not for truthy

* fix txNonce check in createTransaction

* check if transaction.nonce is defined, not for thruthy value

* fix check for defined nonce

Co-authored-by: Germán Martínez <germartinez@users.noreply.github.com>

package.json

@@ -1,6 +1,6 @@
 {
   "name": "safe-react",
-  "version": "1.6.0",
+  "version": "1.6.1",
   "description": "Allowing crypto users manage funds in a safer way",
   "homepage": "https://github.com/gnosis/safe-react#readme",
   "bugs": {
@@ -76,7 +76,7 @@
     "semver": "^7.1.1",
     "squarelink": "^1.1.4",
     "web3": "1.2.4",
-    "web3connect": "^1.0.0-beta.23"
+    "web3connect": "^1.0.0-beta.25"
   },
   "devDependencies": {
     "@babel/cli": "7.7.5",

src/components/ConnectButton/index.jsx

@@ -7,7 +7,7 @@ import Fortmatic from 'fortmatic'
 import Portis from '@portis/web3'
 import Squarelink from 'squarelink'
 import Button from '~/components/layout/Button'
-import { fetchProvider } from '~/logic/wallets/store/actions'
+import { fetchProvider, removeProvider } from '~/logic/wallets/store/actions'
 import { getNetwork } from '~/config'
 import { store } from '~/store'
 
@@ -62,6 +62,10 @@ web3Connect.on('connect', (provider: any) => {
   }
 })
 
+web3Connect.on('disconnect', () => {
+  store.dispatch(removeProvider())
+})
+
 type Props = {
   enqueueSnackbar: Function,
   closeSnackbar: Function,

src/logic/notifications/notificationTypes.js

@@ -89,6 +89,7 @@ export const NOTIFICATIONS: Notifications = {
       variant: SUCCESS,
       persist: false,
       autoHideDuration: shortDuration,
+      preventDuplicate: true,
     },
   },
   UNLOCK_WALLET_MSG: {

src/logic/safe/safeTxSigner.js

@@ -1,19 +1,38 @@
 // @flow
 import { List } from 'immutable'
+import { type Confirmation } from '~/routes/safe/store/models/confirmation'
 
-const generateSignatureFrom = (account: string) => `000000000000000000000000${account.replace(
-  '0x',
-  '',
-)}000000000000000000000000000000000000000000000000000000000000000001`
+// https://gnosis-safe.readthedocs.io/en/latest/contracts/signatures.html#pre-validated-signatures
+// https://github.com/gnosis/safe-contracts/blob/master/test/gnosisSafeTeamEdition.js#L26
+export const generateSignaturesFromTxConfirmations = (
+  confirmations: List<Confirmation>,
+  preApprovingOwner?: string,
+) => {
+  // The constant parts need to be sorted so that the recovered signers are sorted ascending
+  // (natural order) by address (not checksummed).
+  const confirmationsMap = confirmations.reduce((map, obj) => {
+    map[obj.owner.address.toLowerCase()] = obj // eslint-disable-line no-param-reassign
+    return map
+  }, {})
 
-export const buildSignaturesFrom = (ownersWhoHasSigned: List<string>, sender: string) => {
-  const signatures = ownersWhoHasSigned.push(sender)
-  const orderedSignatures = signatures.sort() // JS by default sorts in a non case-senstive way
+  if (preApprovingOwner) {
+    confirmationsMap[preApprovingOwner.toLowerCase()] = { owner: preApprovingOwner }
+  }
 
   let sigs = '0x'
-  orderedSignatures.forEach((owner: string) => {
-    sigs += generateSignatureFrom(owner)
-  })
-
+  Object.keys(confirmationsMap)
+    .sort()
+    .forEach((addr) => {
+      const conf = confirmationsMap[addr]
+      if (conf.signature) {
+        sigs += conf.signature.slice(2)
+      } else {
+        // https://gnosis-safe.readthedocs.io/en/latest/contracts/signatures.html#pre-validated-signatures
+        sigs += `000000000000000000000000${addr.replace(
+          '0x',
+          '',
+        )}000000000000000000000000000000000000000000000000000000000000000001`
+      }
+    })
   return sigs
 }

src/logic/safe/transactions/gasNew.js

@@ -2,7 +2,7 @@
 import GnosisSafeSol from '@gnosis.pm/safe-contracts/build/contracts/GnosisSafe.json'
 import { type Transaction } from '~/routes/safe/store/models/transaction'
 import { getWeb3, getAccountFrom } from '~/logic/wallets/getWeb3'
-import { generateSignaturesFromTxConfirmations } from '~/routes/safe/store/actions/processTransaction'
+import { generateSignaturesFromTxConfirmations } from '~/logic/safe/safeTxSigner'
 import { calculateGasOf, calculateGasPrice } from '~/logic/wallets/ethTransactions'
 import { ZERO_ADDRESS } from '~/logic/wallets/ethAddresses'
 import { CALL } from '.'

src/logic/wallets/store/actions/removeProvider.js

@@ -1,15 +1,16 @@
 // @flow
 import { createAction } from 'redux-actions'
 import type { Dispatch as ReduxDispatch } from 'redux'
-import { NOTIFICATIONS, showSnackbar } from '~/logic/notifications'
+import { NOTIFICATIONS, enhanceSnackbarForAction } from '~/logic/notifications'
 import { getWeb3, resetWeb3 } from '~/logic/wallets/getWeb3'
+import enqueueSnackbar from '~/logic/notifications/store/actions/enqueueSnackbar'
 
 export const REMOVE_PROVIDER = 'REMOVE_PROVIDER'
 
 const removeProvider = createAction<string, *, *>(REMOVE_PROVIDER)
 
-export default (enqueueSnackbar: Function, closeSnackbar: Function) => (dispatch: ReduxDispatch<*>) => {
-  showSnackbar(NOTIFICATIONS.WALLET_DISCONNECTED_MSG, enqueueSnackbar, closeSnackbar)
+export default () => (dispatch: ReduxDispatch<*>) => {
+  dispatch(enqueueSnackbar(enhanceSnackbarForAction(NOTIFICATIONS.WALLET_DISCONNECTED_MSG)))
 
   const web3 = getWeb3()
 

src/routes/safe/components/Transactions/TxsTable/Status/index.jsx

@@ -27,7 +27,7 @@ const statusToIcon = {
 
 const statusToLabel = {
   success: 'Success',
-  cancelled: 'Failed',
+  cancelled: 'Cancelled',
   awaiting_your_confirmation: 'Awaiting your confirmation',
   awaiting_confirmations: 'Awaiting confirmations',
   awaiting_execution: 'Awaiting execution',

src/routes/safe/container/selector.js

@@ -1,6 +1,10 @@
 // @flow
 import { List, Map } from 'immutable'
-import { createSelector, createStructuredSelector, type Selector } from 'reselect'
+import {
+  createSelector,
+  createStructuredSelector,
+  type Selector,
+} from 'reselect'
 import {
   safeSelector,
   safeActiveTokensSelector,
@@ -11,16 +15,29 @@ import {
   type RouterProps,
   type SafeSelectorProps,
 } from '~/routes/safe/store/selectors'
-import { providerNameSelector, userAccountSelector, networkSelector } from '~/logic/wallets/store/selectors'
+import {
+  providerNameSelector,
+  userAccountSelector,
+  networkSelector,
+} from '~/logic/wallets/store/selectors'
 import { type Safe } from '~/routes/safe/store/models/safe'
 import { type GlobalState } from '~/store'
 import { isUserOwner } from '~/logic/wallets/ethAddresses'
-import { orderedTokenListSelector, tokensSelector } from '~/logic/tokens/store/selectors'
+import {
+  orderedTokenListSelector,
+  tokensSelector,
+} from '~/logic/tokens/store/selectors'
 import { type Token } from '~/logic/tokens/store/model/token'
-import { type Transaction, type TransactionStatus } from '~/routes/safe/store/models/transaction'
+import {
+  type Transaction,
+  type TransactionStatus,
+} from '~/routes/safe/store/models/transaction'
 import { safeParamAddressSelector } from '../store/selectors'
 import { getEthAsToken } from '~/logic/tokens/utils/tokenHelpers'
-import { currencyValuesListSelector, currentCurrencySelector } from '~/logic/currencyValues/store/selectors'
+import {
+  currencyValuesListSelector,
+  currentCurrencySelector,
+} from '~/logic/currencyValues/store/selectors'
 import type { BalanceCurrencyType } from '~/logic/currencyValues/store/model/currencyValues'
 import type { IncomingTransaction } from '~/routes/safe/store/models/incomingTransaction'
 
@@ -35,10 +52,14 @@ export type SelectorProps = {
   safeUrl: string,
   currencySelected: string,
   currencyValues: BalanceCurrencyType[],
-  transactions: List<Transaction | IncomingTransaction>,
+  transactions: List<Transaction | IncomingTransaction>
 }
 
-const getTxStatus = (tx: Transaction, userAddress: string, safe: Safe): TransactionStatus => {
+const getTxStatus = (
+  tx: Transaction,
+  userAddress: string,
+  safe: Safe,
+): TransactionStatus => {
   let txStatus
   if (tx.executionTxHash) {
     txStatus = 'success'
@@ -51,37 +72,54 @@ 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.address === userAddress)
+      .size === 1
     const userIsSafeOwner = safe.owners.filter((owner) => owner.address === userAddress).size === 1
-    txStatus = !userConfirmed && userIsSafeOwner ? 'awaiting_your_confirmation' : 'awaiting_confirmations'
+    txStatus = !userConfirmed && userIsSafeOwner
+      ? 'awaiting_your_confirmation'
+      : 'awaiting_confirmations'
   }
 
   return txStatus
 }
 
-export const grantedSelector: Selector<GlobalState, RouterProps, boolean> = createSelector(
+export const grantedSelector: Selector<
+  GlobalState,
+  RouterProps,
+  boolean
+> = createSelector(
   userAccountSelector,
   safeSelector,
   (userAccount: string, safe: Safe | typeof undefined): boolean => isUserOwner(safe, userAccount),
 )
 
-const safeEthAsTokenSelector: Selector<GlobalState, RouterProps, ?Token> = createSelector(
-  safeSelector,
-  (safe: Safe) => {
-    if (!safe) {
-      return undefined
-    }
+const safeEthAsTokenSelector: Selector<
+  GlobalState,
+  RouterProps,
+  ?Token
+> = createSelector(safeSelector, (safe: Safe) => {
+  if (!safe) {
+    return undefined
+  }
 
-    return getEthAsToken(safe.ethBalance)
-  },
-)
+  return getEthAsToken(safe.ethBalance)
+})
 
-const extendedSafeTokensSelector: Selector<GlobalState, RouterProps, List<Token>> = createSelector(
+const extendedSafeTokensSelector: Selector<
+  GlobalState,
+  RouterProps,
+  List<Token>
+> = createSelector(
   safeActiveTokensSelector,
   safeBalancesSelector,
   tokensSelector,
   safeEthAsTokenSelector,
-  (safeTokens: List<string>, balances: Map<string, string>, tokensList: Map<string, Token>, ethAsToken: Token) => {
+  (
+    safeTokens: List<string>,
+    balances: Map<string, string>,
+    tokensList: Map<string, Token>,
+    ethAsToken: Token,
+  ) => {
     const extendedTokens = Map().withMutations((map) => {
       safeTokens.forEach((tokenAddress: string) => {
         const baseToken = tokensList.get(tokenAddress)
@@ -101,7 +139,11 @@ const extendedSafeTokensSelector: Selector<GlobalState, RouterProps, List<Token>
   },
 )
 
-const extendedTransactionsSelector: Selector<GlobalState, RouterProps, List<Transaction | IncomingTransaction>> = createSelector(
+const extendedTransactionsSelector: Selector<
+  GlobalState,
+  RouterProps,
+  List<Transaction | IncomingTransaction>
+> = createSelector(
   safeSelector,
   userAccountSelector,
   safeTransactionsSelector,
@@ -114,17 +156,21 @@ const extendedTransactionsSelector: Selector<GlobalState, RouterProps, List<Tran
       // it means that the transaction was cancelled (Replaced) and shouldn't get executed
       let replacementTransaction
       if (!tx.isExecuted) {
-        replacementTransaction = transactions.size > 1 && transactions.findLast(
-          (transaction) => (
-            transaction.isExecuted && transaction.nonce && transaction.nonce >= tx.nonce
-          ),
-        )
+        replacementTransaction = transactions.size > 1
+          && transactions.findLast(
+            (transaction) => transaction.isExecuted
+              && transaction.nonce != null
+              && transaction.nonce >= tx.nonce,
+          )
         if (replacementTransaction) {
           extendedTx = tx.set('cancelled', true)
         }
       }
 
-      return extendedTx.set('status', getTxStatus(extendedTx, userAddress, safe))
+      return extendedTx.set(
+        'status',
+        getTxStatus(extendedTx, userAddress, safe),
+      )
     })
 
     return List([...extendedTransactions, ...incomingTransactions])

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

@@ -78,7 +78,7 @@ const createTransaction = ({
   const from = userAccountSelector(state)
   const safeInstance = await getGnosisSafeInstanceAt(safeAddress)
   const threshold = await safeInstance.getThreshold()
-  const nonce = txNonce || await getLastPendingTxNonce(safeAddress)
+  const nonce = typeof txNonce !== 'undefined' ? txNonce : await getLastPendingTxNonce(safeAddress)
   const isExecution = threshold.toNumber() === 1 || shouldExecute
 
   // https://gnosis-safe.readthedocs.io/en/latest/contracts/signatures.html#pre-validated-signatures

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

@@ -1,7 +1,5 @@
 // @flow
 import type { Dispatch as ReduxDispatch } from 'redux'
-import { List } from 'immutable'
-import { type Confirmation } from '~/routes/safe/store/models/confirmation'
 import { type Transaction } from '~/routes/safe/store/models/transaction'
 import { userAccountSelector } from '~/logic/wallets/store/selectors'
 import fetchSafe from '~/routes/safe/store/actions/fetchSafe'
@@ -16,44 +14,10 @@ import {
   TX_TYPE_EXECUTION,
   TX_TYPE_CONFIRMATION,
 } from '~/logic/safe/transactions'
+import { generateSignaturesFromTxConfirmations } from '~/logic/safe/safeTxSigner'
 import { type NotificationsQueue, getNotificationsFromTxType, showSnackbar } from '~/logic/notifications'
 import { getErrorMessage } from '~/test/utils/ethereumErrors'
 
-// https://gnosis-safe.readthedocs.io/en/latest/contracts/signatures.html#pre-validated-signatures
-// https://github.com/gnosis/safe-contracts/blob/master/test/gnosisSafeTeamEdition.js#L26
-export const generateSignaturesFromTxConfirmations = (
-  confirmations: List<Confirmation>,
-  preApprovingOwner?: string,
-) => {
-  // The constant parts need to be sorted so that the recovered signers are sorted ascending
-  // (natural order) by address (not checksummed).
-  const confirmationsMap = confirmations.reduce((map, obj) => {
-    map[obj.owner.address] = obj // eslint-disable-line no-param-reassign
-    return map
-  }, {})
-
-  if (preApprovingOwner) {
-    confirmationsMap[preApprovingOwner] = { owner: preApprovingOwner }
-  }
-
-  let sigs = '0x'
-  Object.keys(confirmationsMap)
-    .sort()
-    .forEach((addr) => {
-      const conf = confirmationsMap[addr]
-      if (conf.signature) {
-        sigs += conf.signature.slice(2)
-      } else {
-        // https://gnosis-safe.readthedocs.io/en/latest/contracts/signatures.html#pre-validated-signatures
-        sigs += `000000000000000000000000${addr.replace(
-          '0x',
-          '',
-        )}000000000000000000000000000000000000000000000000000000000000000001`
-      }
-    })
-  return sigs
-}
-
 type ProcessTransactionArgs = {
   safeAddress: string,
   tx: Transaction,

src/test/signatures.blockchain.test.js

@@ -1,6 +1,8 @@
 // @flow
 import { List } from 'immutable'
-import { buildSignaturesFrom } from '~/logic/safe/safeTxSigner'
+import { generateSignaturesFromTxConfirmations } from '~/logic/safe/safeTxSigner'
+
+const makeMockConfirmation = (address: string) => ({ owner: { address } })
 
 describe('Signatures Blockchain Test', () => {
   it('generates signatures in natural order even checksumed', async () => {
@@ -8,39 +10,104 @@ describe('Signatures Blockchain Test', () => {
     const userA = 'baR'
     const userB = 'baz'
     const userC = 'foZa'
-    const sender = 'foZ'
+    const userD = 'foZ'
+
+    const confirmationA = makeMockConfirmation(userA)
+    const confirmationB = makeMockConfirmation(userB)
+    const confirmationC = makeMockConfirmation(userC)
+    const confirmationD = makeMockConfirmation(userD)
 
     // WHEN
     const result = '0x'
       // eslint-disable-next-line
-      + '000000000000000000000000' +
-      'baR'
+      + "000000000000000000000000" +
+      'bar'
       + '000000000000000000000000000000000000000000000000000000000000000001'
       // eslint-disable-next-line
-      + '000000000000000000000000' +
+      + "000000000000000000000000" +
       'baz'
       + '000000000000000000000000000000000000000000000000000000000000000001'
       // eslint-disable-next-line
-      + '000000000000000000000000' +
-      'foZ'
+      + "000000000000000000000000" +
+      'foz'
       + '000000000000000000000000000000000000000000000000000000000000000001'
       // eslint-disable-next-line
-      + '000000000000000000000000' +
-      'foZa'
+      + "000000000000000000000000" +
+      'foza'
       + '000000000000000000000000000000000000000000000000000000000000000001'
 
     // THEN
-    expect(buildSignaturesFrom(List([userA, userB, userC]), sender)).toEqual(result)
-    expect(buildSignaturesFrom(List([userB, userA, userC]), sender)).toEqual(result)
-    expect(buildSignaturesFrom(List([userA, sender, userC]), userB)).toEqual(result)
-    expect(buildSignaturesFrom(List([sender, userA, userC]), userB)).toEqual(result)
-    expect(buildSignaturesFrom(List([userB, sender, userC]), userA)).toEqual(result)
-    expect(buildSignaturesFrom(List([sender, userB, userC]), userA)).toEqual(result)
-    expect(buildSignaturesFrom(List([userA, userB, sender]), userC)).toEqual(result)
-    expect(buildSignaturesFrom(List([userB, userA, sender]), userC)).toEqual(result)
-    expect(buildSignaturesFrom(List([userA, sender, userB]), userC)).toEqual(result)
-    expect(buildSignaturesFrom(List([sender, userA, userB]), userC)).toEqual(result)
-    expect(buildSignaturesFrom(List([userB, sender, userA]), userC)).toEqual(result)
-    expect(buildSignaturesFrom(List([sender, userB, userA]), userC)).toEqual(result)
+    expect(
+      generateSignaturesFromTxConfirmations(
+        List([confirmationA, confirmationB, confirmationC]),
+        userD,
+      ),
+    ).toEqual(result)
+    expect(
+      generateSignaturesFromTxConfirmations(
+        List([confirmationB, confirmationA, confirmationC]),
+        userD,
+      ),
+    ).toEqual(result)
+    expect(
+      generateSignaturesFromTxConfirmations(
+        List([confirmationA, confirmationD, confirmationC]),
+        userB,
+      ),
+    ).toEqual(result)
+    expect(
+      generateSignaturesFromTxConfirmations(
+        List([confirmationD, confirmationA, confirmationC]),
+        userB,
+      ),
+    ).toEqual(result)
+    expect(
+      generateSignaturesFromTxConfirmations(
+        List([confirmationB, confirmationD, confirmationC]),
+        userA,
+      ),
+    ).toEqual(result)
+    expect(
+      generateSignaturesFromTxConfirmations(
+        List([confirmationD, confirmationB, confirmationC]),
+        userA,
+      ),
+    ).toEqual(result)
+    expect(
+      generateSignaturesFromTxConfirmations(
+        List([confirmationA, confirmationB, confirmationD]),
+        userC,
+      ),
+    ).toEqual(result)
+    expect(
+      generateSignaturesFromTxConfirmations(
+        List([confirmationB, confirmationA, confirmationD]),
+        userC,
+      ),
+    ).toEqual(result)
+    expect(
+      generateSignaturesFromTxConfirmations(
+        List([confirmationA, confirmationD, confirmationB]),
+        userC,
+      ),
+    ).toEqual(result)
+    expect(
+      generateSignaturesFromTxConfirmations(
+        List([confirmationD, confirmationA, confirmationB]),
+        userC,
+      ),
+    ).toEqual(result)
+    expect(
+      generateSignaturesFromTxConfirmations(
+        List([confirmationB, confirmationD, confirmationA]),
+        userC,
+      ),
+    ).toEqual(result)
+    expect(
+      generateSignaturesFromTxConfirmations(
+        List([confirmationD, confirmationB, confirmationA]),
+        userC,
+      ),
+    ).toEqual(result)
   })
 })