Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix(Multichain): Show owner setup warning also when replacing owners and changing threshold [SW-150] #4198

Merged
merged 3 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
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>
)
}
Loading