Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Warning for mismatching mastercopy in safe proxy #3908

Merged
merged 19 commits into from
Jun 21, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions src/components/AppLayout/InvalidMasterCopyError/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { Link } from '@gnosis.pm/safe-react-components'
import { useSelector } from 'react-redux'
import { getChainInfo } from 'src/config'
import { Errors, logError } from 'src/logic/exceptions/CodedException'
import useAsync from 'src/logic/hooks/useAsync'
import { currentSafe } from 'src/logic/safe/store/selectors'
import { isValidMasterCopy } from 'src/logic/safe/utils/safeVersion'
import MuiAlert from '@material-ui/lab/Alert'
import MuiAlertTitle from '@material-ui/lab/AlertTitle'
import { withStyles } from '@material-ui/core'

const CLI_LINK = 'https://github.com/5afe/safe-cli'

export const InvalidMasterCopyError = ({ onClose }: { onClose: () => void }): React.ReactElement | null => {
const chainInfo = getChainInfo()
const { currentVersion, address } = useSelector(currentSafe)

const [validMasterCopy, error] = useAsync(async () => {
if (address && currentVersion) {
return await isValidMasterCopy(chainInfo.chainId, address, currentVersion)
}
}, [address, chainInfo, currentVersion])

if (error) {
logError(Errors._620, error.message)
return null
}

if (typeof validMasterCopy === 'undefined' || validMasterCopy) {
return null
}

return (
<StyledAlert severity="error" onClose={onClose}>
<MuiAlertTitle>
This Safe is created with an unsupported base contract. The web interface might not work correctly. We recommend
katspaugh marked this conversation as resolved.
Show resolved Hide resolved
using the{' '}
<Link href={CLI_LINK} size="xl" target="_blank">
command line interface
</Link>{' '}
instead.
</MuiAlertTitle>
</StyledAlert>
)
}

const StyledAlert = withStyles({
icon: {
marginLeft: 'auto',
},
action: {
marginLeft: '0px',
marginRight: 'auto',
},
})(MuiAlert)
4 changes: 3 additions & 1 deletion src/components/AppLayout/Sidebar/SafeHeader/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ const IconContainer = styled.div`
display: flex;
gap: 8px;
justify-content: space-evenly;
align-items: center;
margin: 14px 0;
`
const StyledButton = styled(Button)`
Expand Down Expand Up @@ -220,6 +221,8 @@ const SafeHeader = ({
onNewTransactionClick()
}

const chainInfo = getChainInfo()

if (!address || !hasSafeOpen) {
return (
<Container>
Expand All @@ -233,7 +236,6 @@ const SafeHeader = ({
</Container>
)
}
const chainInfo = getChainInfo()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid making unnecessary changes like this.


return (
<>
Expand Down
5 changes: 5 additions & 0 deletions src/components/AppLayout/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { MobileNotSupported } from './MobileNotSupported'
import { SAFE_APP_LANDING_PAGE_ROUTE, SAFE_ROUTES, WELCOME_ROUTE } from 'src/routes/routes'
import useDarkMode from 'src/logic/hooks/useDarkMode'
import { screenSm } from 'src/theme/variables'
import { InvalidMasterCopyError } from 'src/components/AppLayout/InvalidMasterCopyError'

const Container = styled.div`
height: 100vh;
Expand All @@ -23,6 +24,7 @@ const Container = styled.div`

const HeaderWrapper = styled.nav`
height: 52px;
min-height: 52px;
width: 100%;
z-index: 1299;

Expand Down Expand Up @@ -134,6 +136,7 @@ const Layout: React.FC<Props> = ({
}): React.ReactElement => {
const [mobileNotSupportedClosed, setMobileNotSupportedClosed] = useState(false)
const [expanded, setExpanded] = useState(false)
const [showMasterCopyError, setShowMasterCopyError] = useState(true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you return null from InvalidMasterCopyError when it's valid then you no longer need this state.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this state is for when the user closes the banner

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used to hide the error if the user closes the alert.
I moved this state into the InvalidMasterCopyError component as it is not needed outside of it.

const { pathname } = useLocation()
useDarkMode()

Expand All @@ -157,6 +160,8 @@ const Layout: React.FC<Props> = ({
<HeaderWrapper>
<Header />
</HeaderWrapper>
{showMasterCopyError && <InvalidMasterCopyError onClose={() => setShowMasterCopyError(false)} />}

<BodyWrapper>
{showSideBar && (
<SidebarWrapper data-testid="sidebar" $expanded={expanded} onClick={onSidebarClick}>
Expand Down
8 changes: 7 additions & 1 deletion src/logic/contracts/safeContracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
getFallbackHandlerDeployment,
getMultiSendCallOnlyDeployment,
getSignMessageLibDeployment,
SingletonDeployment,
} from '@gnosis.pm/safe-deployments'
import Web3 from 'web3'
import { AbiItem } from 'web3-utils'
Expand All @@ -30,12 +31,17 @@ let safeMaster: GnosisSafe
let fallbackHandler: CompatibilityFallbackHandler
let multiSend: MultiSend

const getSafeContractDeployment = ({ safeVersion }: { safeVersion: string }) => {
export const getSafeContractDeployment = ({
safeVersion,
}: {
safeVersion: string
}): SingletonDeployment | undefined => {
// We check if version is prior to v1.0.0 as they are not supported but still we want to keep a minimum compatibility
const useOldestContractVersion = semverSatisfies(safeVersion, '<1.0.0')
// We have to check if network is L2
const networkId = _getChainId()
const chainConfig = getChainById(networkId)

// We had L1 contracts in three L2 networks, xDai, EWC and Volta so even if network is L2 we have to check that safe version is after v1.3.0
const useL2ContractVersion = chainConfig.l2 && semverSatisfies(safeVersion, '>=1.3.0')
const getDeployment = useL2ContractVersion ? getSafeL2SingletonDeployment : getSafeSingletonDeployment
Expand Down
1 change: 1 addition & 0 deletions src/logic/exceptions/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ enum ErrorCodes {
_617 = '617: Error fetching safeTxGas',
_618 = '618: Error fetching fee history',
_619 = '619: Failed to prepare a multisend tx',
_620 = '620: Failed to load master copy information',
_700 = '700: Failed to read from local/session storage',
_701 = '701: Failed to write to local/session storage',
_702 = '702: Failed to remove from local/session storage',
Expand Down
52 changes: 51 additions & 1 deletion src/logic/safe/utils/__tests__/safeVersion.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { getSafeL2SingletonDeployment, getSafeSingletonDeployment } from '@gnosis.pm/safe-deployments'
DiogoSoaress marked this conversation as resolved.
Show resolved Hide resolved
import * as config from 'src/config'
import * as safeContracts from 'src/logic/contracts/safeContracts'
import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'
import { checkIfSafeNeedsUpdate, hasFeature } from 'src/logic/safe/utils/safeVersion'
import { emptyChainInfo } from 'src/config/cache/chains'
import { checkIfSafeNeedsUpdate, hasFeature, isValidMasterCopy } from 'src/logic/safe/utils/safeVersion'

describe('Check safe version', () => {
it('Calls checkIfSafeNeedUpdate, should return true if the safe version is bellow the target one', async () => {
Expand Down Expand Up @@ -36,4 +40,50 @@ describe('Check safe version', () => {
expect(hasFeature(FEATURES.SAFE_APPS, '1.1.1')).toBe(true)
})
})

describe('isValidMasterCopy', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add some more tests so that we cover all deployed versions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the linter and prettier warnings and added tests for 1.0.0 and 1.2.0!

const safeContracts = require('src/logic/contracts/safeContracts')

it('returns true for L1 mastercopy in L1 chain', async () => {
jest.spyOn(config, '_getChainId').mockImplementation(() => '1')
jest.spyOn(config, 'getChainById').mockImplementation(() => ({ ...emptyChainInfo, chainId: '1', l2: false }))

jest
.spyOn(safeContracts, 'getMasterCopyAddressFromProxyAddress')
.mockImplementation(() => getSafeSingletonDeployment()?.networkAddresses['1'])
const isValid = await isValidMasterCopy('1', '0x0000000000000000000000000000000000000001', '1.3.0')
expect(isValid).toBe(true)
})

it('returns false for L1 mastercopy in L2 chain and version >=1.3.0', async () => {
jest.spyOn(config, '_getChainId').mockImplementation(() => '100')
jest.spyOn(config, 'getChainById').mockImplementation(() => ({ ...emptyChainInfo, chainId: '100', l2: true }))
jest
.spyOn(safeContracts, 'getMasterCopyAddressFromProxyAddress')
.mockImplementation(() => getSafeSingletonDeployment()?.networkAddresses['100'])
const isValid = await isValidMasterCopy('100', '0x0000000000000000000000000000000000000001', '1.3.0')
expect(isValid).toBe(false)
})

it('returns false for L1 mastercopy in L2 chain and version contains meta info', async () => {
jest.spyOn(config, '_getChainId').mockImplementation(() => '100')
jest.spyOn(config, 'getChainById').mockImplementation(() => ({ ...emptyChainInfo, chainId: '100', l2: true }))
jest
.spyOn(safeContracts, 'getMasterCopyAddressFromProxyAddress')
.mockImplementation(() => getSafeL2SingletonDeployment({ version: '1.1.1' })?.networkAddresses['100'])

const isValid = await isValidMasterCopy('100', '0x0000000000000000000000000000000000000001', '1.3.0+L2')
expect(isValid).toBe(false)
})

it('returns true for L1 mastercopy in L2 chain and version <1.3.0', async () => {
jest.spyOn(config, '_getChainId').mockImplementation(() => '100')
jest.spyOn(config, 'getChainById').mockImplementation(() => ({ ...emptyChainInfo, chainId: '100', l2: true }))
jest
.spyOn(safeContracts, 'getMasterCopyAddressFromProxyAddress')
.mockImplementation(() => getSafeSingletonDeployment({ version: '1.1.1' })?.networkAddresses['100'])
const isValid = await isValidMasterCopy('100', '0x0000000000000000000000000000000000000001', '1.1.1')
expect(isValid).toBe(true)
})
})
})
20 changes: 19 additions & 1 deletion src/logic/safe/utils/safeVersion.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
import semverLessThan from 'semver/functions/lt'
import semverSatisfies from 'semver/functions/satisfies'
import semverValid from 'semver/functions/valid'
import semverClean from 'semver/functions/clean'
import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'

import { GnosisSafe } from 'src/types/contracts/gnosis_safe.d'
import { getSafeMasterContract } from 'src/logic/contracts/safeContracts'
import {
getMasterCopyAddressFromProxyAddress,
getSafeContractDeployment,
getSafeMasterContract,
} from 'src/logic/contracts/safeContracts'
import { LATEST_SAFE_VERSION } from 'src/utils/constants'
import { Errors, logError } from 'src/logic/exceptions/CodedException'
import { getChainInfo } from 'src/config'
import { sameAddress } from 'src/logic/wallets/ethAddresses'

const FEATURES_BY_VERSION: Record<string, string> = {
[FEATURES.SAFE_TX_GAS_OPTIONAL]: '>=1.3.0',
Expand Down Expand Up @@ -86,3 +92,15 @@ export const getSafeVersionInfo = async (safeVersion: string): Promise<SafeVersi
logError(Errors._606, err.message)
}
}

export const isValidMasterCopy = async (
chainId: string,
safeAddress: string,
safeVersion: string,
): Promise<boolean> => {
const masterCopyAddressFromProxy = await getMasterCopyAddressFromProxyAddress(safeAddress)
const expectedDeployment = getSafeContractDeployment({ safeVersion: semverClean(safeVersion) })
const expectedMasterCopyAddress = expectedDeployment?.networkAddresses[chainId]

return sameAddress(masterCopyAddressFromProxy, expectedMasterCopyAddress)
}