* 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 8331f2a78f7fc8f53eb893899f16edd8238c68ff.

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

This reverts commit 03b81e31820ce4fe078a7131c2f0caa2af4870ac.

* 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 <fernando.greco@gmail.com>
This commit is contained in:
Mikhail Mikheev 2020-04-09 18:21:52 +04:00 committed by GitHub
parent 74d48c40ac
commit 4aeb5df55b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 76 additions and 87 deletions

View File

@ -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": {

View File

@ -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) => {

View File

@ -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 () => {

View File

@ -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

View File

@ -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} />

View File

@ -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]
}

View File

@ -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,

View File

@ -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)

View File

@ -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

View File

@ -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 }))
})
}

View File

@ -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))
},