From 4763d513d991580bfe51ac0a54880c27d46c6b57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ux=C3=ADo?= Date: Mon, 31 Jan 2022 11:43:38 +0100 Subject: [PATCH 01/46] Optimize dockerfile (#3386) --- Dockerfile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Dockerfile b/Dockerfile index eb57b09d2a..4f4039b4ba 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,12 +1,12 @@ FROM node:14 -RUN apt-get update && apt-get install -y libusb-1.0-0 libusb-1.0-0-dev libudev-dev +RUN apt-get update \ + && apt-get install -y libusb-1.0-0 libusb-1.0-0-dev libudev-dev \ + && rm -rf /var/lib/apt/lists/* WORKDIR /app -COPY package.json ./ - -COPY yarn.lock ./ +COPY package.json yarn.lock . COPY src/logic/contracts/artifacts ./src/logic/contracts/artifacts From 0a2dc1765cb22843937239a6e1dcdb1c55c75bf7 Mon Sep 17 00:00:00 2001 From: Aaron Cook Date: Mon, 31 Jan 2022 17:14:46 +0100 Subject: [PATCH 02/46] fix: throttle queue transaction macro (#3384) * fix: debouce queued tx creation macro * fix: don't call `useDebounce` immediately * fix: debounce callback without hook * fix: throttle instead of debounce --- src/components/AppLayout/Sidebar/DevTools/index.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/components/AppLayout/Sidebar/DevTools/index.tsx b/src/components/AppLayout/Sidebar/DevTools/index.tsx index 9a97c9ba75..fb146bfc7b 100644 --- a/src/components/AppLayout/Sidebar/DevTools/index.tsx +++ b/src/components/AppLayout/Sidebar/DevTools/index.tsx @@ -1,4 +1,4 @@ -import { ReactElement } from 'react' +import { ReactElement, useMemo } from 'react' import { useSelector } from 'react-redux' import styled from 'styled-components' import { fireEvent, screen, waitForElementToBeRemoved } from '@testing-library/react' @@ -6,6 +6,7 @@ import { Button } from '@gnosis.pm/safe-react-components' import List from '@material-ui/core/List' import ListItem from '@material-ui/core/ListItem' import ListItemText from '@material-ui/core/ListItemText' +import throttle from 'lodash/throttle' import { currentSafe, currentSafeEthBalance } from 'src/logic/safe/store/selectors' import { extractSafeAddress } from 'src/routes/routes' @@ -80,6 +81,8 @@ const DevTools = (): ReactElement => { return hasFunds } + const throttledCreatedQueuedTx = useMemo(() => throttle(createQueuedTx, 1000), []) + return ( <> @@ -98,7 +101,7 @@ const DevTools = (): ReactElement => { createQueuedTx(safeAddress, threshold)} + onClick={() => throttledCreatedQueuedTx(safeAddress, threshold)} size="md" variant="bordered" disabled={!isGranted || !hasSufficientFunds()} From b66a905ff1797e5a04bff4751fa5605722d7d098 Mon Sep 17 00:00:00 2001 From: Aaron Cook Date: Mon, 31 Jan 2022 17:15:12 +0100 Subject: [PATCH 03/46] fix: remove sidebar scroll behaviour (#3383) * fix: remove sidebar scroll behaviour * fix: scroll to center --- src/components/SafeListSidebar/SafeList/SafeListItem.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/SafeListSidebar/SafeList/SafeListItem.tsx b/src/components/SafeListSidebar/SafeList/SafeListItem.tsx index 11db73e6b1..10e043165e 100644 --- a/src/components/SafeListSidebar/SafeList/SafeListItem.tsx +++ b/src/components/SafeListSidebar/SafeList/SafeListItem.tsx @@ -85,7 +85,7 @@ const SafeListItem = ({ useEffect(() => { if (isCurrentSafe && shouldScrollToSafe) { - safeRef?.current?.scrollIntoView({ behavior: 'smooth' }) + safeRef?.current?.scrollIntoView({ block: 'center' }) } }, [isCurrentSafe, shouldScrollToSafe]) From 39ce7a38913eefe28e7ae7b73ed4b810655eed2a Mon Sep 17 00:00:00 2001 From: Aaron Cook Date: Mon, 31 Jan 2022 18:18:29 +0100 Subject: [PATCH 04/46] fix: memoize dev tools (#3387) * fix: don't load devtools lazily * fix: memoize dev tools --- src/components/AppLayout/Sidebar/index.tsx | 66 +++++++++++----------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/src/components/AppLayout/Sidebar/index.tsx b/src/components/AppLayout/Sidebar/index.tsx index ab1887d1a3..f99269c858 100644 --- a/src/components/AppLayout/Sidebar/index.tsx +++ b/src/components/AppLayout/Sidebar/index.tsx @@ -1,4 +1,4 @@ -import { lazy } from 'react' +import { lazy, useMemo } from 'react' import styled from 'styled-components' import { Divider, IconText } from '@gnosis.pm/safe-react-components' @@ -66,41 +66,43 @@ const Sidebar = ({ onToggleSafeList, onReceiveClick, onNewTransactionClick, -}: Props): React.ReactElement => ( - <> - +}: Props): React.ReactElement => { + const devTools = useMemo(() => lazyLoad('./DevTools'), []) + const debugToggle = useMemo(() => lazyLoad('./DebugToggle'), []) + return ( + <> + - {items.length ? ( - <> - - - - ) : null} - - {!IS_PRODUCTION && safeAddress && ( + {items.length ? ( <> - {lazyLoad('./DevTools')} + - )} - - {!IS_PRODUCTION && lazyLoad('./DebugToggle')} - - + ) : null} + + {!IS_PRODUCTION && safeAddress && ( + <> + + {devTools} + + )} + {!IS_PRODUCTION && debugToggle} + - - - - - -) + + + + + + ) +} export default Sidebar From e736bb543221ce86f351842613e7d56680cd2df6 Mon Sep 17 00:00:00 2001 From: Mikhail Mikheev Date: Tue, 1 Feb 2022 08:24:52 +0100 Subject: [PATCH 05/46] bug: signing fails in brave (#3389) --- src/logic/safe/transactions/offchainSigner/index.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/logic/safe/transactions/offchainSigner/index.ts b/src/logic/safe/transactions/offchainSigner/index.ts index 46c1c7a36d..b7cbcbde57 100644 --- a/src/logic/safe/transactions/offchainSigner/index.ts +++ b/src/logic/safe/transactions/offchainSigner/index.ts @@ -33,8 +33,12 @@ const getSupportedSigners = (isHW: boolean, safeVersion: string) => { return signers } -const isKeystoneError = (err: Error): boolean => { - return err.message.startsWith('#ktek_error') +const isKeystoneError = (err: unknown): boolean => { + if (err instanceof Error) { + return err.message?.startsWith('#ktek_error') + } + + return false } export const tryOffChainSigning = async ( From f426b0551e30e76864c1dbedcc6dd5b71b70afdd Mon Sep 17 00:00:00 2001 From: katspaugh <381895+katspaugh@users.noreply.github.com> Date: Tue, 1 Feb 2022 09:23:22 +0100 Subject: [PATCH 06/46] refactor: use TxModalWrapper in ApproveTxModal (#3369) * refactor: use TxModalWrapper in ApproveTxModal PR comments Show only nonce when creating a multisig tx Revert parametersStatus changes Partially revert #3257 (it was buggy) Rm snackbar Onchain sig or execution * Fix: pass tx nonce * Fix 1/1 can execute * parseFloat -> parseInt * Check internal tx threshold, not current policy * Pass calculated safeTxGas to useEstimateTransactionGas Co-authored-by: Diogo Soares --- src/components/TransactionsFees/index.tsx | 6 +- .../hooks/__tests__/useCanTxExecute.test.ts | 253 +++++------------- src/logic/hooks/useCanTxExecute.tsx | 64 +---- src/logic/hooks/useEstimateTransactionGas.tsx | 6 +- .../ConfirmTxModal/ReviewConfirm.tsx | 2 +- .../SignMessageModal/ReviewMessage.tsx | 4 +- .../Settings/Advanced/RemoveGuardModal.tsx | 2 +- .../Settings/Advanced/RemoveModuleModal.tsx | 2 +- .../SpendingLimit/NewLimitModal/Review.tsx | 2 +- .../SpendingLimit/RemoveLimitModal.tsx | 1 + .../ChangeThreshold/index.tsx | 2 +- .../Settings/UpdateSafeModal/index.tsx | 2 +- .../Transactions/TxList/ActionModal.tsx | 10 - .../TxList/modals/ApproveTxModal.tsx | 244 ++++------------- .../helpers/EditableTxParameters.tsx | 2 +- .../helpers/TxModalWrapper/index.tsx | 68 ++++- .../helpers/TxParametersDetail/index.tsx | 32 +-- .../hooks/useTransactionParameters.ts | 4 +- 18 files changed, 206 insertions(+), 500 deletions(-) diff --git a/src/components/TransactionsFees/index.tsx b/src/components/TransactionsFees/index.tsx index 79ca5aef3b..95d3e3448c 100644 --- a/src/components/TransactionsFees/index.tsx +++ b/src/components/TransactionsFees/index.tsx @@ -3,7 +3,6 @@ import Paragraph from 'src/components/layout/Paragraph' import { getNativeCurrency } from 'src/config' import { TransactionFailText } from 'src/components/TransactionFailText' import { Text } from '@gnosis.pm/safe-react-components' -import useCanTxExecute from 'src/logic/hooks/useCanTxExecute' import { providerSelector } from 'src/logic/wallets/store/selectors' import { useSelector } from 'react-redux' import { currentSafe } from 'src/logic/safe/store/selectors' @@ -24,8 +23,7 @@ export const TransactionFees = ({ }: TransactionFailTextProps): React.ReactElement | null => { const { currentVersion: safeVersion } = useSelector(currentSafe) const { smartContractWallet } = useSelector(providerSelector) - const canTxExecute = useCanTxExecute(isExecution) - const isOffChainSignature = checkIfOffChainSignatureIsPossible(canTxExecute, smartContractWallet, safeVersion) + const isOffChainSignature = checkIfOffChainSignatureIsPossible(isExecution, smartContractWallet, safeVersion) const nativeCurrency = getNativeCurrency() let transactionAction @@ -46,7 +44,7 @@ export const TransactionFees = ({ You're about to {transactionAction} a transaction and will have to confirm it with your currently connected wallet.{' '} - {!isOffChainSignature && ( + {(!isOffChainSignature || isExecution) && ( <> Make sure you have diff --git a/src/logic/hooks/__tests__/useCanTxExecute.test.ts b/src/logic/hooks/__tests__/useCanTxExecute.test.ts index 9b4048cda0..29cc70d312 100644 --- a/src/logic/hooks/__tests__/useCanTxExecute.test.ts +++ b/src/logic/hooks/__tests__/useCanTxExecute.test.ts @@ -1,191 +1,70 @@ -import { calculateCanTxExecute } from '../useCanTxExecute' +import useCanTxExecute from '../useCanTxExecute' +import * as redux from 'react-redux' + +const mockedRedux = redux as jest.Mocked & { useSelector: any } + +jest.mock('react-redux', () => { + const original = jest.requireActual('react-redux') + return { + ...original, + useSelector: () => ({ threshold: 2 }), + } +}) describe('useCanTxExecute tests', () => { - describe('calculateCanTxExecute tests', () => { - beforeEach(() => { - threshold = 1 - isExecution = false - currentSafeNonce = 8 - recommendedNonce = 8 - txConfirmations = 0 - preApprovingOwner = '' - manualSafeNonce = recommendedNonce - }) - // to be overriden as necessary - let threshold - let preApprovingOwner - let txConfirmations - let currentSafeNonce - let recommendedNonce - let isExecution - let manualSafeNonce - it(`should return true if isExecution`, () => { - // given - isExecution = true - - // when - const result = calculateCanTxExecute( - currentSafeNonce, - preApprovingOwner, - threshold, - txConfirmations, - recommendedNonce, - isExecution, - ) - - // then - expect(result).toBe(true) - }) - it(`should return true if single owner and edited nonce is same as safeNonce`, () => { - // given - threshold = 1 - currentSafeNonce = 8 - recommendedNonce = 12 - manualSafeNonce = 8 - - // when - const result = calculateCanTxExecute( - currentSafeNonce, - preApprovingOwner, - threshold, - txConfirmations, - recommendedNonce, - undefined, - manualSafeNonce, - ) - - // then - expect(result).toBe(true) - }) - it(`should return false if single owner and edited nonce is different than safeNonce`, () => { - // given - threshold = 1 - currentSafeNonce = 8 - recommendedNonce = 8 - manualSafeNonce = 20 - - // when - const result = calculateCanTxExecute( - currentSafeNonce, - preApprovingOwner, - threshold, - txConfirmations, - recommendedNonce, - undefined, - manualSafeNonce, - ) - - // then - expect(result).toBe(false) - }) - it(`should return true if single owner and recommendedNonce is same as safeNonce`, () => { - // given - threshold = 1 - currentSafeNonce = 8 - recommendedNonce = 8 - - // when - const result = calculateCanTxExecute( - currentSafeNonce, - preApprovingOwner, - threshold, - txConfirmations, - recommendedNonce, - ) - - // then - expect(result).toBe(true) - }) - it(`should return false if single owner and recommendedNonce is greater than safeNonce and no edited nonce`, () => { - // given - threshold = 1 - currentSafeNonce = 8 - recommendedNonce = 11 - manualSafeNonce = undefined - - // when - const result = calculateCanTxExecute( - currentSafeNonce, - preApprovingOwner, - threshold, - txConfirmations, - recommendedNonce, - undefined, - manualSafeNonce, - ) - - // then - expect(result).toBe(false) - }) - it(`should return false if single owner and recommendedNonce is different than safeNonce`, () => { - // given - threshold = 1 - currentSafeNonce = 8 - recommendedNonce = 12 - - // when - const result = calculateCanTxExecute( - currentSafeNonce, - preApprovingOwner, - threshold, - txConfirmations, - recommendedNonce, - ) - - // then - expect(result).toBe(false) - }) - it(`should return true if the safe threshold is reached for the transaction`, () => { - // given - threshold = 3 - txConfirmations = 3 - - // when - const result = calculateCanTxExecute( - currentSafeNonce, - preApprovingOwner, - threshold, - txConfirmations, - recommendedNonce, - ) - - // then - expect(result).toBe(true) - }) - it(`should return false if the number of confirmations does not meet the threshold and there is no preApprovingOwner`, () => { - // given - threshold = 5 - txConfirmations = 4 - - // when - const result = calculateCanTxExecute( - currentSafeNonce, - preApprovingOwner, - threshold, - txConfirmations, - recommendedNonce, - ) - - // then - expect(result).toBe(false) - }) - it(`should return true if the number of confirmations is one bellow the threshold but there is a preApprovingOwner`, () => { - // given - threshold = 5 - preApprovingOwner = '0x29B1b813b6e84654Ca698ef5d7808E154364900B' - txConfirmations = 4 - - // when - const result = calculateCanTxExecute( - currentSafeNonce, - preApprovingOwner, - threshold, - txConfirmations, - recommendedNonce, - ) - - // then - expect(result).toBe(true) - }) + it(`should return true if owner of a 1/1 Safe`, () => { + mockedRedux.useSelector = jest.fn(() => ({ threshold: 1 })) + + const result = useCanTxExecute('0x000', 0) + expect(result).toBe(true) + }) + + it(`should return false if not an owner and not enough sigs`, () => { + mockedRedux.useSelector = jest.fn(() => ({ threshold: 1 })) + + const result = useCanTxExecute('', 0) + expect(result).toBe(false) + }) + + it(`should return true if 2/2 sigs`, () => { + mockedRedux.useSelector = jest.fn(() => ({ threshold: 2 })) + + const result = useCanTxExecute('', 2) + expect(result).toBe(true) + }) + + it(`should return true if 1/2 sigs and an owner`, () => { + mockedRedux.useSelector = jest.fn(() => ({ threshold: 2 })) + + const result = useCanTxExecute('0x000', 1) + expect(result).toBe(true) + }) + + it(`should return false if 1/3 sigs and an owner`, () => { + mockedRedux.useSelector = jest.fn(() => ({ threshold: 3 })) + + const result = useCanTxExecute('0x000', 1) + expect(result).toBe(false) + }) + + it(`should return false if 2/3 sigs and not an owner`, () => { + mockedRedux.useSelector = jest.fn(() => ({ threshold: 3 })) + + const result = useCanTxExecute('', 2) + expect(result).toBe(false) + }) + + it(`should return true if 3/10 sigs and threshold 3 passed from an arg`, () => { + mockedRedux.useSelector = jest.fn(() => ({ threshold: 10 })) + + const result = useCanTxExecute('', 3, 3) + expect(result).toBe(true) + }) + + it(`should return false if 3/3 sigs and threshold 10 passed from an arg`, () => { + mockedRedux.useSelector = jest.fn(() => ({ threshold: 3 })) + + const result = useCanTxExecute('', 3, 10) + expect(result).toBe(false) }) }) diff --git a/src/logic/hooks/useCanTxExecute.tsx b/src/logic/hooks/useCanTxExecute.tsx index 602ad044ca..3eaa3b04b2 100644 --- a/src/logic/hooks/useCanTxExecute.tsx +++ b/src/logic/hooks/useCanTxExecute.tsx @@ -1,69 +1,29 @@ import { useSelector } from 'react-redux' -import { extractSafeAddress } from 'src/routes/routes' import { currentSafe } from '../safe/store/selectors' -import useGetRecommendedNonce from './useGetRecommendedNonce' -export const calculateCanTxExecute = ( - currentSafeNonce: number, - preApprovingOwner: string, - threshold: number, - txConfirmations: number, - recommendedNonce?: number, - isExecution?: boolean, // when executing from the TxList - manualSafeNonce?: number, -): boolean => { - if (isExecution) return true +type UseCanTxExecuteType = ( + preApprovingOwner?: string, + txConfirmations?: number, + existingTxThreshold?: number, +) => boolean - // Single owner - if (threshold === 1) { - // nonce was changed manually to be executed - if (manualSafeNonce) { - return manualSafeNonce === currentSafeNonce - } - // is next tx - return recommendedNonce === currentSafeNonce - } +const useCanTxExecute: UseCanTxExecuteType = (preApprovingOwner = '', txConfirmations = 0, existingTxThreshold) => { + const safeInfo = useSelector(currentSafe) + + // A tx might have been created with a threshold that is different than the current policy + // If an existing tx threshold isn't passed, take the current safe threshold + const threshold = existingTxThreshold ?? safeInfo.threshold if (txConfirmations >= threshold) { return true } // When having a preApprovingOwner it is needed one less confirmation to execute the tx - if (preApprovingOwner && txConfirmations) { + if (preApprovingOwner) { return txConfirmations + 1 === threshold } return false } -type UseCanTxExecuteType = ( - isExecution?: boolean, - manualSafeNonce?: number, - preApprovingOwner?: string, - txConfirmations?: number, -) => boolean - -const useCanTxExecute: UseCanTxExecuteType = ( - isExecution = false, - manualSafeNonce, - preApprovingOwner = '', - txConfirmations = 0, -) => { - const { threshold } = useSelector(currentSafe) - - const safeAddress = extractSafeAddress() - const recommendedNonce = useGetRecommendedNonce(safeAddress) - const { nonce: currentSafeNonce } = useSelector(currentSafe) - - return calculateCanTxExecute( - currentSafeNonce, - preApprovingOwner, - threshold, - txConfirmations, - recommendedNonce, - isExecution, - manualSafeNonce, - ) -} - export default useCanTxExecute diff --git a/src/logic/hooks/useEstimateTransactionGas.tsx b/src/logic/hooks/useEstimateTransactionGas.tsx index b10a73294d..77c3fd7d6b 100644 --- a/src/logic/hooks/useEstimateTransactionGas.tsx +++ b/src/logic/hooks/useEstimateTransactionGas.tsx @@ -60,7 +60,6 @@ type UseEstimateTransactionGasProps = { manualMaxPrioFee?: string manualGasLimit?: string manualSafeNonce?: number // Edited nonce - isExecution?: boolean // If called from the TransactionList "next transaction" } export type TransactionGasEstimationResult = { @@ -116,7 +115,7 @@ export const calculateTotalGasCost = ( gasMaxPrioFee: string, decimals: number, ): [string, string] => { - const totalPricePerGas = parseFloat(gasPrice) + parseFloat(gasMaxPrioFee || '0') + const totalPricePerGas = parseInt(gasPrice, 10) + parseInt(gasMaxPrioFee || '0', 10) const estimatedGasCosts = parseInt(gasLimit, 10) * totalPricePerGas const gasCost = fromTokenUnit(estimatedGasCosts, decimals) const formattedGasCost = formatAmount(gasCost) @@ -136,7 +135,6 @@ export const useEstimateTransactionGas = ({ manualMaxPrioFee, manualGasLimit, manualSafeNonce, - isExecution, }: UseEstimateTransactionGasProps): TransactionGasEstimationResult => { const [gasEstimation, setGasEstimation] = useState( getDefaultGasEstimation({ @@ -151,7 +149,7 @@ export const useEstimateTransactionGas = ({ const { address: safeAddress = '', threshold = 1, currentVersion: safeVersion = '' } = useSelector(currentSafe) ?? {} const { account: from, smartContractWallet, name: providerName } = useSelector(providerSelector) - const canTxExecute = useCanTxExecute(isExecution, manualSafeNonce, preApprovingOwner, txConfirmations?.size) + const canTxExecute = useCanTxExecute(preApprovingOwner, txConfirmations?.size) useEffect(() => { const estimateGas = async () => { diff --git a/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx b/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx index b8f7d7b0d4..6537d6fb43 100644 --- a/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx +++ b/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx @@ -137,7 +137,7 @@ export const ReviewConfirm = ({ txValue={txValue} operation={operation} onSubmit={confirmTransactions} - isConfirmDisabled={!isOwner} + isSubmitDisabled={!isOwner} onBack={onReject} >