Skip to content

Commit

Permalink
Fix(Multichain): Show owner setup warning also when replacing owners …
Browse files Browse the repository at this point in the history
…and changing threshold [SW-150] (#4198)

* feat: show signer setup warning for swapping signer or changing threshold.

* feat: move the change signer warning to the relavent block in the confirmation scree
  • Loading branch information
jmealy authored Sep 24, 2024
1 parent 11754e1 commit 5c9be78
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 17 deletions.
3 changes: 3 additions & 0 deletions src/components/tx-flow/flows/AddOwner/ReviewOwner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { OwnerList } from '../../common/OwnerList'
import MinusIcon from '@/public/images/common/minus.svg'
import EthHashInfo from '@/components/common/EthHashInfo'
import commonCss from '@/components/tx-flow/common/styles.module.css'
import { ChangeSignerSetupWarning } from '@/features/multichain/components/SignerSetupWarning/ChangeSignerSetupWarning'

export const ReviewOwner = ({ params }: { params: AddOwnerFlowProps | ReplaceOwnerFlowProps }) => {
const dispatch = useAppDispatch()
Expand Down Expand Up @@ -73,6 +74,8 @@ export const ReviewOwner = ({ params }: { params: AddOwnerFlowProps | ReplaceOwn
</Paper>
)}
<OwnerList owners={[{ name: newOwner.name, value: newOwner.address }]} />
<ChangeSignerSetupWarning />

<Divider className={commonCss.nestedDivider} />
<Box>
<Typography variant="body2">Any transaction requires the confirmation of:</Typography>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { ChangeThresholdFlowFieldNames } from '@/components/tx-flow/flows/Change
import type { ChangeThresholdFlowProps } from '@/components/tx-flow/flows/ChangeThreshold'

import commonCss from '@/components/tx-flow/common/styles.module.css'
import { ChangeSignerSetupWarning } from '@/features/multichain/components/SignerSetupWarning/ChangeSignerSetupWarning'

const ReviewChangeThreshold = ({ params }: { params: ChangeThresholdFlowProps }) => {
const { safe } = useSafeInfo()
Expand All @@ -28,6 +29,8 @@ const ReviewChangeThreshold = ({ params }: { params: ChangeThresholdFlowProps })

return (
<SignOrExecuteForm onSubmit={onChangeThreshold}>
<ChangeSignerSetupWarning />
<Divider className={commonCss.nestedDivider} />
<div>
<Typography variant="body2" color="text.secondary" mb={0.5}>
Any transaction will require the confirmation of:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type { RemoveOwnerFlowProps } from '.'
import EthHashInfo from '@/components/common/EthHashInfo'

import commonCss from '@/components/tx-flow/common/styles.module.css'
import { ChangeSignerSetupWarning } from '@/features/multichain/components/SignerSetupWarning/ChangeSignerSetupWarning'

export const ReviewRemoveOwner = ({ params }: { params: RemoveOwnerFlowProps }): ReactElement => {
const addressBook = useAddressBook()
Expand Down Expand Up @@ -46,6 +47,8 @@ export const ReviewRemoveOwner = ({ params }: { params: RemoveOwnerFlowProps }):
hasExplorer
/>
</Paper>
<ChangeSignerSetupWarning />

<Divider className={commonCss.nestedDivider} />
<Box m={1}>
<Typography variant="body2" color="text.secondary" mb={0.5}>
Expand Down
7 changes: 0 additions & 7 deletions src/components/tx/SignOrExecuteForm/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import { trackEvent } from '@/services/analytics'
import useChainId from '@/hooks/useChainId'
import ExecuteThroughRoleForm from './ExecuteThroughRoleForm'
import { findAllowingRole, findMostLikelyRole, useRoles } from './ExecuteThroughRoleForm/hooks'
import { isSettingTwapFallbackHandler } from '@/features/swap/helpers/utils'
import { isCustomTxInfo, isGenericConfirmation, isMigrateToL2MultiSend } from '@/utils/transaction-guards'
import useIsSafeOwner from '@/hooks/useIsSafeOwner'
import { BlockaidBalanceChanges } from '../security/blockaid/BlockaidBalanceChange'
Expand All @@ -42,9 +41,7 @@ import { extractMigrationL2MasterCopyAddress } from '@/utils/transactions'
import type { TransactionDetails } from '@safe-global/safe-gateway-typescript-sdk'
import { useGetTransactionDetailsQuery, useLazyGetTransactionDetailsQuery } from '@/store/gateway'
import { skipToken } from '@reduxjs/toolkit/query/react'
import { isChangingSignerSetup } from '@/features/multichain/utils/utils'
import NetworkWarning from '@/components/new-safe/create/NetworkWarning'
import { ChangeSignerSetupWarning } from '@/features/multichain/components/SignerSetupWarning/ChangeSignerSetupWarning'

export type SubmitCallback = (txId: string, isExecuted?: boolean) => void

Expand Down Expand Up @@ -118,8 +115,6 @@ export const SignOrExecuteForm = ({
const { safe } = useSafeInfo()
const isSafeOwner = useIsSafeOwner()
const isCounterfactualSafe = !safe.deployed
const isChangingFallbackHandler = isSettingTwapFallbackHandler(decodedData)
const isChangingSigners = isChangingSignerSetup(decodedData)
const isMultiChainMigration = isMigrateToL2MultiSend(decodedData)
const multiChainMigrationTarget = extractMigrationL2MasterCopyAddress(decodedData)

Expand Down Expand Up @@ -207,8 +202,6 @@ export const SignOrExecuteForm = ({

<NetworkWarning />

{isChangingSigners && <ChangeSignerSetupWarning />}

{!isMultiChainMigration && <UnknownContractError />}

<Blockaid />
Expand Down
4 changes: 1 addition & 3 deletions src/components/tx/security/blockaid/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import BlockaidIcon from '@/public/images/transactions/blockaid-icon.svg'
import { SafeTxContext } from '@/components/tx-flow/SafeTxProvider'
import { type SecurityWarningProps, mapSecuritySeverity } from '../utils'
import { BlockaidHint } from './BlockaidHint'
import Warning from '@/public/images/notifications/alert.svg'
import { SecuritySeverity } from '@/services/security/modules/types'

export const REASON_MAPPING: Record<string, string> = {
Expand Down Expand Up @@ -65,7 +64,6 @@ const BlockaidResultWarning = ({
<>
<Alert
severity={severityProps?.color}
icon={<Warning />}
className={css.customAlert}
sx={
needsRiskConfirmation
Expand Down Expand Up @@ -136,7 +134,7 @@ const ResultDescription = ({

const BlockaidError = () => {
return (
<Alert severity="warning" icon={<Warning />} className={css.customAlert}>
<Alert severity="warning" className={css.customAlert}>
<AlertTitle>
<Typography fontWeight={700} variant="subtitle1">
Proceed with caution
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Box } from '@mui/material'
import { Alert } from '@mui/material'
import { useIsMultichainSafe } from '../../hooks/useIsMultichainSafe'
import ErrorMessage from '@/components/tx/ErrorMessage'
import { useCurrentChain } from '@/hooks/useChains'

export const ChangeSignerSetupWarning = () => {
Expand All @@ -10,10 +9,8 @@ export const ChangeSignerSetupWarning = () => {
if (!isMultichainSafe) return

return (
<Box mt={1} mb={1}>
<ErrorMessage level="warning">
{`Signers are not consistent across networks on this account. Changing signers will only affect the account on ${currentChain}`}
</ErrorMessage>
</Box>
<Alert severity="info" sx={{ border: 'none', mt: 0, mb: 0 }}>
{`Signers are not consistent across networks on this account. Changing signers will only affect the account on ${currentChain?.chainName}`}
</Alert>
)
}

0 comments on commit 5c9be78

Please sign in to comment.