v1.9.4 (#754)

* Bug: Invalid V in signature with eth_sign (#728)

* Fix invalid V with metamask/ledger

* DONT FORGET TO REVERT BEFORE MERGING: test deployment

* DONT FORGET TO REVERT BEFORE MERGING 2: test deployment

* Revert "DONT FORGET TO REVERT BEFORE MERGING 2: test deployment"

This reverts commit 8331f2a.

* Revert "DONT FORGET TO REVERT BEFORE MERGING: test deployment"

This reverts commit 03b81e3.

* BUG: Only injected providers are cached as last used provider (#733)

* cache every used provider, not only injected one

* package json update

* (Fix) Adapt app to back-end changes (#736)

* refactor: Set success status to `201` (CREATED)

* refactor: return `null` when there's no latestTx

* (Fix) Transaction not automatically executed (#716)

* feature: action/reducer to UPDATE_SAFE_NONCE

* refactor: when processing txs returned from backend, extract latest tx nonce value and store it in the safe's state

* chore: update `yarn.lock`

* refactor: `UPDATE_SAFE_THRESHOLD` and `UPDATE_SAFE_NONCE` discarded in favor of `UPDATE_SAFE`

* refactor: use `SAFE_REDUCER_ID` constant

* refactor: remove `updateSafeNonce` file

* (Fix) Change the order of the upgrade methods lookup (#740)

* fix: change the order of the upgrade methods lookup

The `isUpgradeTransaction` method was looking for the methods in an wrong order (#599).
The proper order was set in #610, but `isUpgradeTransaction` wasn't updated.

* fix: contract upgrade version lookup

* Feature: Use eth_sign for hardware wallets connected via onboard.js (#742)

* Use eth_sign for hardware wallets

* install onboard.js with fix from forked repo

* rebuild yarn.lock to fix cached onboard

* update bnc-onboard

* update package json (#743)

* (Fix) Properly decode threshold value in tx details (#749)

* fix: Display new threshold value when changing its value

There was a typo for the `changeThreshold` action

fixes #746

* refactor: use a constant for safe methods names

* fix: check for `decimals` method in transferredTokens (#748)

Previously we were looking for `decimals` hash in the contract `code`.
There are some contracts like USDC who happen to be behind a FiatTokenProxy, making it upgradable.
By directly calling the `decimals()` method, we interact with the contract and can be sure that the `decimals()` method is present.

fixes #678

* Bug #747: Don't use getLastTxNonce to fetch safe nonce (#750)

* don't use getLastTxNonce to fetch safe nonce

* fetch safe nonce in checkAndUpdateSafe

* checkAndUpdateSafe refactor

* remove nonce update logic from UPDATE_SAFE reducer

* handle the case when localSafe returns undefined

* handle the case when localSafe returns undefined in buildTransactionFrom

* bump package json version to 1.9.4

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

package.json

@@ -1,6 +1,6 @@
 {
   "name": "safe-react",
-  "version": "1.9.3",
+  "version": "1.9.4",
   "description": "Allowing crypto users manage funds in a safer way",
   "homepage": "https://github.com/gnosis/safe-react#readme",
   "bugs": {

src/logic/contracts/methodIds.js

@@ -39,11 +39,18 @@ import { getWeb3 } from '~/logic/wallets/getWeb3'
 //   { name: "getTransactionHash", id: "0xd8d11f78" }
 // ]
 
+export const SAFE_METHODS_NAMES = {
+  ADD_OWNER_WITH_THRESHOLD: 'addOwnerWithThreshold',
+  CHANGE_THRESHOLD: 'changeThreshold',
+  REMOVE_OWNER: 'removeOwner',
+  SWAP_OWNER: 'swapOwner',
+}
+
 const METHOD_TO_ID = {
-  '0xe318b52b': 'swapOwner',
-  '0x0d582f13': 'addOwnerWithThreshold',
-  '0xf8dc5dd9': 'removeOwner',
-  '0x694e80c3': 'changeThreshold',
+  '0xe318b52b': SAFE_METHODS_NAMES.SWAP_OWNER,
+  '0x0d582f13': SAFE_METHODS_NAMES.ADD_OWNER_WITH_THRESHOLD,
+  '0xf8dc5dd9': SAFE_METHODS_NAMES.REMOVE_OWNER,
+  '0x694e80c3': SAFE_METHODS_NAMES.CHANGE_THRESHOLD,
 }
 
 export const decodeParamsFromSafeMethod = async (data: string) => {

src/logic/tokens/store/actions/fetchTokens.js

@@ -12,8 +12,7 @@ import { fetchTokenList } from '~/logic/tokens/api'
 import { type TokenProps, makeToken } from '~/logic/tokens/store/model/token'
 import { tokensSelector } from '~/logic/tokens/store/selectors'
 import { getWeb3 } from '~/logic/wallets/getWeb3'
-import { type GlobalState } from '~/store'
-import { store } from '~/store/index'
+import { type GlobalState, store } from '~/store'
 import { ensureOnce } from '~/utils/singleton'
 
 const createStandardTokenContract = async () => {

src/logic/tokens/utils/tokenHelpers.js

@@ -1,4 +1,5 @@
 // @flow
+import ERC20Detailed from '@openzeppelin/contracts/build/contracts/ERC20Detailed.json'
 import { List } from 'immutable'
 
 import logo from '~/assets/icons/icon_etherTokens.svg'
@@ -56,6 +57,17 @@ export const isAddressAToken = async (tokenAddress: string) => {
   return call !== '0x'
 }
 
+export const hasDecimalsMethod = async (address: string): Promise<boolean> => {
+  try {
+    const web3 = getWeb3()
+    const token = new web3.eth.Contract(ERC20Detailed.abi, address)
+    await token.methods.decimals().call()
+    return true
+  } catch (e) {
+    return false
+  }
+}
+
 export const isTokenTransfer = (data: string, value: number): boolean =>
   !!data && data.substring(0, 10) === '0xa9059cbb' && value === 0
 

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

@@ -10,6 +10,7 @@ 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 { SAFE_METHODS_NAMES } from '~/logic/contracts/methodIds'
 import { shortVersionOf } from '~/logic/wallets/ethAddresses'
 import OwnerAddressTableCell from '~/routes/safe/components/Settings/ManageOwners/OwnerAddressTableCell'
 import { getTxAmount } from '~/routes/safe/components/Transactions/TxsTable/columns'
@@ -120,7 +121,7 @@ const NewThreshold = ({ newThreshold }: { newThreshold: string }) => (
 )
 
 const SettingsDescription = ({ action, addedOwner, newThreshold, removedOwner }: DescriptionDescProps) => {
-  if (action === 'removeOwner' && removedOwner && newThreshold) {
+  if (action === SAFE_METHODS_NAMES.REMOVE_OWNER && removedOwner && newThreshold) {
     return (
       <>
         <RemovedOwner removedOwner={removedOwner} />
@@ -129,11 +130,11 @@ const SettingsDescription = ({ action, addedOwner, newThreshold, removedOwner }:
     )
   }
 
-  if (action === 'changedThreshold' && newThreshold) {
+  if (action === SAFE_METHODS_NAMES.CHANGE_THRESHOLD && newThreshold) {
     return <NewThreshold newThreshold={newThreshold} />
   }
 
-  if (action === 'addOwnerWithThreshold' && addedOwner && newThreshold) {
+  if (action === SAFE_METHODS_NAMES.ADD_OWNER_WITH_THRESHOLD && addedOwner && newThreshold) {
     return (
       <>
         <AddedOwner addedOwner={addedOwner} />
@@ -142,7 +143,7 @@ const SettingsDescription = ({ action, addedOwner, newThreshold, removedOwner }:
     )
   }
 
-  if (action === 'swapOwner' && removedOwner && addedOwner) {
+  if (action === SAFE_METHODS_NAMES.SWAP_OWNER && removedOwner && addedOwner) {
     return (
       <>
         <RemovedOwner removedOwner={removedOwner} />

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/TxDescription/utils.js

@@ -1,4 +1,5 @@
 // @flow
+import { SAFE_METHODS_NAMES } from '~/logic/contracts/methodIds'
 import { getWeb3 } from '~/logic/wallets/getWeb3'
 import { type Transaction } from '~/routes/safe/store/models/transaction'
 
@@ -51,15 +52,15 @@ export const getTxData = (tx: Transaction): DecodedTxData => {
     if (tx.decodedParams) {
       txData.action = tx.decodedParams.methodName
 
-      if (txData.action === 'removeOwner') {
+      if (txData.action === SAFE_METHODS_NAMES.REMOVE_OWNER) {
         txData.removedOwner = tx.decodedParams.args[1]
         txData.newThreshold = tx.decodedParams.args[2]
-      } else if (txData.action === 'changeThreshold') {
+      } else if (txData.action === SAFE_METHODS_NAMES.CHANGE_THRESHOLD) {
         txData.newThreshold = tx.decodedParams.args[0]
-      } else if (txData.action === 'addOwnerWithThreshold') {
+      } else if (txData.action === SAFE_METHODS_NAMES.ADD_OWNER_WITH_THRESHOLD) {
         txData.addedOwner = tx.decodedParams.args[0]
         txData.newThreshold = tx.decodedParams.args[1]
-      } else if (txData.action === 'swapOwner') {
+      } else if (txData.action === SAFE_METHODS_NAMES.SWAP_OWNER) {
         txData.removedOwner = tx.decodedParams.args[1]
         txData.addedOwner = tx.decodedParams.args[2]
       }

src/routes/safe/container/actions.js

@@ -29,7 +29,7 @@ export type Actions = {
   fetchLatestMasterContractVersion: typeof fetchLatestMasterContractVersion,
   activateTokensByBalance: typeof activateTokensByBalance,
   activateAssetsByBalance: typeof activateAssetsByBalance,
-  checkAndUpdateSafeOwners: typeof checkAndUpdateSafe,
+  checkAndUpdateSafe: typeof checkAndUpdateSafe,
   fetchCurrencyValues: typeof fetchCurrencyValues,
   loadAddressBook: typeof loadAddressBookFromStorage,
   updateAddressBookEntry: typeof updateAddressBookEntry,
@@ -50,7 +50,7 @@ export default {
   fetchEtherBalance,
   fetchLatestMasterContractVersion,
   fetchCurrencyValues,
-  checkAndUpdateSafeOwners: checkAndUpdateSafe,
+  checkAndUpdateSafe,
   loadAddressBook: loadAddressBookFromStorage,
   updateAddressBookEntry,
   addViewedSafe,

src/routes/safe/container/index.jsx

@@ -121,14 +121,14 @@ class SafeView extends React.Component<Props, State> {
   checkForUpdates() {
     const {
       activeTokens,
-      checkAndUpdateSafeOwners,
+      checkAndUpdateSafe,
       fetchEtherBalance,
       fetchTokenBalances,
       fetchTransactions,
       safe,
       safeUrl,
     } = this.props
-    checkAndUpdateSafeOwners(safeUrl)
+    checkAndUpdateSafe(safeUrl)
     fetchTokenBalances(safeUrl, activeTokens)
     fetchEtherBalance(safe)
     fetchTransactions(safeUrl)

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

@@ -67,40 +67,47 @@ export const checkAndUpdateSafe = (safeAdd: string) => async (dispatch: ReduxDis
   // Check if the owner's safe did change and update them
   const [gnosisSafe, localSafe] = await Promise.all([getGnosisSafeInstanceAt(safeAddress), getLocalSafe(safeAddress)])
 
-  const remoteOwners = await gnosisSafe.getOwners()
+  const [remoteOwners, remoteNonce, remoteThreshold] = await Promise.all([
+    gnosisSafe.getOwners(),
+    gnosisSafe.nonce(),
+    gnosisSafe.getThreshold(),
+  ])
   // Converts from [ { address, ownerName} ] to address array
-  const localOwners = localSafe.owners.map((localOwner) => localOwner.address)
-  const localThreshold = localSafe.threshold
+  const localOwners = localSafe ? localSafe.owners.map((localOwner) => localOwner.address) : undefined
+  const localThreshold = localSafe ? localSafe.threshold : undefined
+  const localNonce = localSafe ? localSafe.nonce : undefined
 
-  // Updates threshold values
-  const remoteThreshold = await gnosisSafe.getThreshold()
-  localSafe.threshold = remoteThreshold.toNumber()
+  if (localNonce !== remoteNonce.toNumber()) {
+    dispatch(updateSafe({ address: safeAddress, nonce: remoteNonce.toNumber() }))
+  }
 
   if (localThreshold !== remoteThreshold.toNumber()) {
     dispatch(updateSafe({ address: safeAddress, threshold: remoteThreshold.toNumber() }))
   }
 
   // If the remote owners does not contain a local address, we remove that local owner
-  localOwners.forEach((localAddress) => {
-    const remoteOwnerIndex = remoteOwners.findIndex((remoteAddress) => sameAddress(remoteAddress, localAddress))
-    if (remoteOwnerIndex === -1) {
-      dispatch(removeSafeOwner({ safeAddress, ownerAddress: localAddress }))
-    }
-  })
+  if (localOwners) {
+    localOwners.forEach((localAddress) => {
+      const remoteOwnerIndex = remoteOwners.findIndex((remoteAddress) => sameAddress(remoteAddress, localAddress))
+      if (remoteOwnerIndex === -1) {
+        dispatch(removeSafeOwner({ safeAddress, ownerAddress: localAddress }))
+      }
+    })
 
-  // If the remote has an owner that we don't have locally, we add it
-  remoteOwners.forEach((remoteAddress) => {
-    const localOwnerIndex = localOwners.findIndex((localAddress) => sameAddress(remoteAddress, localAddress))
-    if (localOwnerIndex === -1) {
-      dispatch(
-        addSafeOwner({
-          safeAddress,
-          ownerAddress: remoteAddress,
-          ownerName: 'UNKNOWN',
-        }),
-      )
-    }
-  })
+    // If the remote has an owner that we don't have locally, we add it
+    remoteOwners.forEach((remoteAddress) => {
+      const localOwnerIndex = localOwners.findIndex((localAddress) => sameAddress(remoteAddress, localAddress))
+      if (localOwnerIndex === -1) {
+        dispatch(
+          addSafeOwner({
+            safeAddress,
+            ownerAddress: remoteAddress,
+            ownerName: 'UNKNOWN',
+          }),
+        )
+      }
+    })
+  }
 }
 
 // eslint-disable-next-line consistent-return

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

@@ -15,8 +15,8 @@ import { getLocalSafe } from '~/logic/safe/utils'
 import { getTokenInfos } from '~/logic/tokens/store/actions/fetchTokens'
 import { ALTERNATIVE_TOKEN_ABI } from '~/logic/tokens/utils/alternativeAbi'
 import {
-  DECIMALS_METHOD_HASH,
   SAFE_TRANSFER_FROM_WITHOUT_DATA_HASH,
+  hasDecimalsMethod,
   isMultisendTransaction,
   isTokenTransfer,
   isUpgradeTransaction,
@@ -25,7 +25,6 @@ import { ZERO_ADDRESS, sameAddress } from '~/logic/wallets/ethAddresses'
 import { EMPTY_DATA } from '~/logic/wallets/ethTransactions'
 import { getWeb3 } from '~/logic/wallets/getWeb3'
 import { addCancellationTransactions } from '~/routes/safe/store/actions/addCancellationTransactions'
-import updateSafe from '~/routes/safe/store/actions/updateSafe'
 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'
@@ -75,14 +74,14 @@ type IncomingTxServiceModel = {
 }
 
 export const buildTransactionFrom = async (safeAddress: string, tx: TxServiceModel): Promise<Transaction> => {
-  const { owners } = await getLocalSafe(safeAddress)
+  const localSafe = await getLocalSafe(safeAddress)
 
   const confirmations = List(
     tx.confirmations.map((conf: ConfirmationServiceModel) => {
       let ownerName = 'UNKNOWN'
 
-      if (owners) {
-        const storedOwner = owners.find((owner) => sameAddress(conf.owner, owner.address))
+      if (localSafe && localSafe.owners) {
+        const storedOwner = localSafe.owners.find((owner) => sameAddress(conf.owner, owner.address))
 
         if (storedOwner) {
           ownerName = storedOwner.name
@@ -102,7 +101,7 @@ export const buildTransactionFrom = async (safeAddress: string, tx: TxServiceMod
   const code = tx.to ? await web3.eth.getCode(tx.to) : ''
   const isERC721Token =
     code.includes(SAFE_TRANSFER_FROM_WITHOUT_DATA_HASH) ||
-    (isTokenTransfer(tx.data, Number(tx.value)) && !code.includes(DECIMALS_METHOD_HASH))
+    (isTokenTransfer(tx.data, Number(tx.value)) && !(await hasDecimalsMethod(tx.to)))
   const isSendTokenTx = !isERC721Token && isTokenTransfer(tx.data, Number(tx.value))
   const isMultiSendTx = isMultisendTransaction(tx.data, Number(tx.value))
   const isUpgradeTx = isMultiSendTx && isUpgradeTransaction(tx.data)
@@ -349,45 +348,16 @@ export const loadSafeIncomingTransactions = async (safeAddress: string) => {
   return Map().set(safeAddress, List(incomingTxsRecord))
 }
 
-/**
- * Returns nonce from the last tx returned by the server or defaults to 0
- * @param outgoingTxs
- * @returns {number|*}
- */
-const getLastTxNonce = (outgoingTxs) => {
-  if (!outgoingTxs) {
-    return 0
-  }
-
-  const mostRecentNonce = outgoingTxs.get(0).nonce
-
-  // if nonce is null, then we are in front of the creation-tx
-  if (mostRecentNonce === null) {
-    const tx = outgoingTxs.get(1)
-
-    if (tx) {
-      // if there's other tx than the creation one, we return its nonce
-      return tx.nonce
-    } else {
-      return 0
-    }
-  }
-
-  return mostRecentNonce
-}
-
 export default (safeAddress: string) => async (dispatch: ReduxDispatch<GlobalState>) => {
   web3 = await getWeb3()
 
   const transactions: SafeTransactionsType | undefined = await loadSafeTransactions(safeAddress)
   if (transactions) {
     const { cancel, outgoing } = transactions
-    const nonce = getLastTxNonce(outgoing && outgoing.get(safeAddress))
 
     batch(() => {
       dispatch(addCancellationTransactions(cancel))
       dispatch(addTransactions(outgoing))
-      dispatch(updateSafe({ address: safeAddress, nonce }))
     })
   }
 

src/routes/safe/store/reducer/safe.js

@@ -48,14 +48,6 @@ export default handleActions<SafeReducerState, *>(
     [UPDATE_SAFE]: (state: SafeReducerState, action: ActionType<Function>): SafeReducerState => {
       const safe = action.payload
       const safeAddress = safe.address
-      const isNonceUpdate = safe.nonce !== undefined
-
-      if (isNonceUpdate && safe.nonce <= state.getIn([SAFE_REDUCER_ID, safeAddress, 'nonce'])) {
-        // update only when nonce is greater than the one already stored
-        // this will prevent undesired changes in the safe's state when
-        // txs pagination is implemented
-        return state
-      }
 
       return state.updateIn([SAFE_REDUCER_ID, safeAddress], (prevSafe) => prevSafe.merge(safe))
     },