Skip to content

Commit

Permalink
Refactor: unify confirmations in the transaction flow (#4177)
Browse files Browse the repository at this point in the history
* chore: create a wrapper on top of signOrExecuteForm to fetch async data before showing the ui

* chore: show the confirmation view component based on the transaction type

* chore: move presentational layer of reviewOwner to SettingsChange component

* chore: do not use showMethodCall to signOrExecuteForm component

* feat: Add a loader in the signOrExecuteForm component while the necessary data is not fetched

* fix: unit tests

* fix: lint errors

* mend

* fix: pass the txInfo in the useTxDetails mock to avoid null pointer exception

* fix: generated snapshots

* fix: generated snapshots

* fix: change the fundReceiver address on snapshots

* fix: remove unnecessary casting

* chore: rename useDetailsHook to be useProposeTx

* fix: grab the transaction id from the txDetails in case the txId is not provided

* fix: rename file from utils to mockData

* chore: move the settings component to be exported directly in the index file

* fix: add the batch button back to the confirmation views

* fix: use the txData component in the confirmation view in case any confirmation view component is found

* feat: show an error screen if something happens while fetching the txDetails

* fix: show contract name when it is a multisend transaction

* fix: eslint errors

* fix: move error condition to top of the confirmation view component

* fix: do not propose transaction for contrafactual safes

* fix: cypress drain e2e test

* fix: remove duplicated data in the DecodedTx component

* fix: Add back showMethodCall and keep prop drilling it

* fix: do not show method call for approve transactions

* fix: add showMethodCall into the confirmationView component

* fix: pass isApproval down to the confirmation view component

* fix: Add isCreation check inside DecodedTx to render partial summary

* fix: Update snapshot and mock hex data generation to be even length

* chore: generated snapshots

* chore: refactor changeThresholdReview screen (#4212)

* Approval editor

* Fix error display in confirmation screen

* chore: unify confirmBatch screen (#4217)

* fix(account-flow-import): change import src in the recover account flow screen

* fix(eslint): eslint hook dependencies

* fix(unit-tests): change txDetails mocked data in unit tests

* fix(eslint): change operators order

* fix(unit-tests): mock useSafeAddress hook

* fix(settings-change): add address name in the change owner screen

* fix(settings-change): duplicated owner name in the add owner flow

---------

Co-authored-by: Usame Algan <[email protected]>
Co-authored-by: katspaugh <[email protected]>
  • Loading branch information
3 people authored Nov 1, 2024
1 parent 132b4a2 commit aad2bb5
Show file tree
Hide file tree
Showing 52 changed files with 3,682 additions and 641 deletions.
2 changes: 1 addition & 1 deletion cypress/e2e/pages/safeapps.pages.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export const testBooleanValue3 = '3 testBooleanValue'
export const transfer2AssetsStr = 'Transfer 2 assets'

export const testTransfer1 = '1 transfer'
export const testTransfer2 = '2 transfer'
export const testTransfer2 = '2 MetaMultiSigWallet: transfer'
export const nativeTransfer2 = '2 native transfer'
export const nativeTransfer1 = '1 native transfer'

Expand Down
7 changes: 5 additions & 2 deletions src/components/common/ErrorBoundary/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Typography, Link } from '@mui/material'
import type { FallbackRender } from '@sentry/react'

import { HELP_CENTER_URL, IS_PRODUCTION } from '@/config/constants'
import { AppRoutes } from '@/config/routes'
Expand All @@ -8,8 +7,12 @@ import WarningIcon from '@/public/images/notifications/warning.svg'
import css from '@/components/common/ErrorBoundary/styles.module.css'
import CircularIcon from '../icons/CircularIcon'
import ExternalLink from '../ExternalLink'
interface ErrorBoundaryProps {
error: Error
componentStack: string
}

const ErrorBoundary: FallbackRender = ({ error, componentStack }) => {
const ErrorBoundary = ({ error, componentStack }: ErrorBoundaryProps) => {
return (
<div className={css.container}>
<div className={css.wrapper}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const SafeTxHashDataRow = ({
}) => {
const chainId = useChainId()
const safeAddress = useSafeAddress()

const domainHash = TypedDataEncoder.hashDomain({
chainId,
verifyingContract: safeAddress,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ export const Multisend = ({ txData, compact = false }: MultisendProps): ReactEle
if (!multiSendTransactions) {
return null
}

return (
<>
<MultisendActionsHeader setOpen={setOpenMap} amount={multiSendTransactions.length} compact={compact} />
Expand Down
11 changes: 8 additions & 3 deletions src/components/tx-flow/SafeTxProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import useSafeInfo from '@/hooks/useSafeInfo'
import { useCurrentChain } from '@/hooks/useChains'
import { prependSafeToL2Migration } from '@/utils/transactions'

export const SafeTxContext = createContext<{
export type SafeTxContextParams = {
safeTx?: SafeTransaction
setSafeTx: Dispatch<SetStateAction<SafeTransaction | undefined>>

Expand All @@ -28,7 +28,9 @@ export const SafeTxContext = createContext<{
setSafeTxGas: Dispatch<SetStateAction<string | undefined>>

recommendedNonce?: number
}>({
}

export const SafeTxContext = createContext<SafeTxContextParams>({
setSafeTx: () => {},
setSafeMessage: () => {},
setSafeTxError: () => {},
Expand Down Expand Up @@ -81,7 +83,10 @@ const SafeTxProvider = ({ children }: { children: ReactNode }): ReactElement =>
if (safeTx.data.nonce === finalNonce && safeTx.data.safeTxGas === finalSafeTxGas) return

createTx({ ...safeTx.data, safeTxGas: String(finalSafeTxGas) }, finalNonce)
.then(setSafeTx)
.then((tx) => {
console.log('SafeTxProvider: Updated tx with nonce and safeTxGas', tx)
setSafeTx(tx)
})
.catch(setSafeTxError)
}, [isSigned, finalNonce, finalSafeTxGas, safeTx?.data])

Expand Down
38 changes: 4 additions & 34 deletions src/components/tx-flow/flows/AddOwner/ReviewOwner.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { useCurrentChain } from '@/hooks/useChains'
import { useContext, useEffect } from 'react'
import { Typography, Divider, Box, SvgIcon, Paper } from '@mui/material'

import SignOrExecuteForm from '@/components/tx/SignOrExecuteForm'
import useSafeInfo from '@/hooks/useSafeInfo'
Expand All @@ -11,11 +10,7 @@ import { upsertAddressBookEntries } from '@/store/addressBookSlice'
import { SafeTxContext } from '../../SafeTxProvider'
import type { AddOwnerFlowProps } from '.'
import type { ReplaceOwnerFlowProps } from '../ReplaceOwner'
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'
import { SettingsChangeContext } from './context'

export const ReviewOwner = ({ params }: { params: AddOwnerFlowProps | ReplaceOwnerFlowProps }) => {
const dispatch = useAppDispatch()
Expand Down Expand Up @@ -57,33 +52,8 @@ export const ReviewOwner = ({ params }: { params: AddOwnerFlowProps | ReplaceOwn
}

return (
<SignOrExecuteForm onSubmit={addAddressBookEntryAndSubmit}>
{params.removedOwner && (
<Paper sx={{ backgroundColor: ({ palette }) => palette.warning.background, p: 2 }}>
<Typography color="text.secondary" mb={2} display="flex" alignItems="center">
<SvgIcon component={MinusIcon} inheritViewBox fontSize="small" sx={{ mr: 1 }} />
Previous signer
</Typography>
<EthHashInfo
name={params.removedOwner.name}
address={params.removedOwner.address}
shortAddress={false}
showCopyButton
hasExplorer
/>
</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>
<Typography>
<b>{threshold}</b> out of <b>{safe.owners.length + (removedOwner ? 0 : 1)} signers</b>
</Typography>
</Box>
<Divider className={commonCss.nestedDivider} />
</SignOrExecuteForm>
<SettingsChangeContext.Provider value={params}>
<SignOrExecuteForm onSubmit={addAddressBookEntryAndSubmit} showMethodCall />
</SettingsChangeContext.Provider>
)
}
7 changes: 7 additions & 0 deletions src/components/tx-flow/flows/AddOwner/context.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { type Context, createContext } from 'react'
import { type AddOwnerFlowProps } from '.'
import { type ReplaceOwnerFlowProps } from '../ReplaceOwner'

type SettingsChange = Context<AddOwnerFlowProps | ReplaceOwnerFlowProps>

export const SettingsChangeContext: SettingsChange = createContext({} as AddOwnerFlowProps | ReplaceOwnerFlowProps)
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import useSafeInfo from '@/hooks/useSafeInfo'
import { useContext, useEffect } from 'react'
import { Box, Divider, Typography } from '@mui/material'

import { createUpdateThresholdTx } from '@/services/tx/tx-sender'
import { SETTINGS_EVENTS, trackEvent } from '@/services/analytics'
Expand All @@ -9,8 +8,7 @@ import { SafeTxContext } from '@/components/tx-flow/SafeTxProvider'
import { ChangeThresholdFlowFieldNames } from '@/components/tx-flow/flows/ChangeThreshold'
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'
import { ChangeThresholdReviewContext } from './context'

const ReviewChangeThreshold = ({ params }: { params: ChangeThresholdFlowProps }) => {
const { safe } = useSafeInfo()
Expand All @@ -28,22 +26,9 @@ 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:
</Typography>

<Typography>
<b>{newThreshold}</b> out of <b>{safe.owners.length} signer(s)</b>
</Typography>
</div>
<Box my={1}>
<Divider className={commonCss.nestedDivider} />
</Box>
</SignOrExecuteForm>
<ChangeThresholdReviewContext.Provider value={{ newThreshold }}>
<SignOrExecuteForm onSubmit={onChangeThreshold} showMethodCall />
</ChangeThresholdReviewContext.Provider>
)
}

Expand Down
5 changes: 5 additions & 0 deletions src/components/tx-flow/flows/ChangeThreshold/context.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { createContext } from 'react'

export const ChangeThresholdReviewContext = createContext({
newThreshold: 0,
})
7 changes: 1 addition & 6 deletions src/components/tx-flow/flows/ConfirmBatch/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { OperationType } from '@safe-global/safe-core-sdk-types'
import TxLayout from '../../common/TxLayout'
import BatchIcon from '@/public/images/common/batch.svg'
import { useDraftBatch } from '@/hooks/useDraftBatch'
import BatchTxList from '@/components/batch/BatchSidebar/BatchTxList'

type ConfirmBatchProps = {
onSubmit: () => void
Expand All @@ -32,11 +31,7 @@ const ConfirmBatch = ({ onSubmit }: ConfirmBatchProps): ReactElement => {
createMultiSendCallOnlyTx(calls).then(setSafeTx).catch(setSafeTxError)
}, [batchTxs, setSafeTx, setSafeTxError])

return (
<SignOrExecuteForm onSubmit={onSubmit} isBatch>
<BatchTxList txItems={batchTxs} />
</SignOrExecuteForm>
)
return <SignOrExecuteForm onSubmit={onSubmit} isBatch />
}

const ConfirmBatchFlow = (props: ConfirmBatchProps) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import type { TransactionSummary } from '@safe-global/safe-gateway-typescript-sd
import useSafeInfo from '@/hooks/useSafeInfo'
import { useChainId } from '@/hooks/useChainId'
import useWallet from '@/hooks/wallets/useWallet'
import SignOrExecuteForm from '@/components/tx/SignOrExecuteForm'
import { isExecutable, isMultisigExecutionInfo, isSignableBy } from '@/utils/transaction-guards'
import { createExistingTx } from '@/services/tx/tx-sender'
import { SafeTxContext } from '../../SafeTxProvider'
import SignOrExecuteForm from '@/components/tx/SignOrExecuteForm'

type ConfirmProposedTxProps = {
txSummary: TransactionSummary
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import WalletRejectionError from '@/components/tx/SignOrExecuteForm/WalletReject
import commonCss from '@/components/tx-flow/common/styles.module.css'
import { BlockaidBalanceChanges } from '@/components/tx/security/blockaid/BlockaidBalanceChange'
import NetworkWarning from '@/components/new-safe/create/NetworkWarning'
import { useGetTransactionDetailsQuery } from '@/store/api/gateway'
import { skipToken } from '@reduxjs/toolkit/query'

export function RecoverAccountFlowReview({ params }: { params: RecoverAccountFlowProps }): ReactElement | null {
// Form state
Expand All @@ -52,6 +54,8 @@ export function RecoverAccountFlowReview({ params }: { params: RecoverAccountFlo
const recovery = data && selectDelayModifierByRecoverer(data, wallet?.address ?? '')
const [, executionValidationError] = useIsValidRecoveryExecTransactionFromModule(recovery?.address, safeTx)

const { data: txDetails } = useGetTransactionDetailsQuery(skipToken)

// Proposal
const newThreshold = Number(params[RecoverAccountFlowFields.threshold])
const newOwners = params[RecoverAccountFlowFields.owners]
Expand Down Expand Up @@ -127,7 +131,7 @@ export function RecoverAccountFlowReview({ params }: { params: RecoverAccountFlo

<Divider className={commonCss.nestedDivider} />

<DecodedTx tx={safeTx} decodedData={decodedData} />
<DecodedTx txDetails={txDetails} tx={safeTx} decodedData={decodedData} />

<BlockaidBalanceChanges />
</TxCard>
Expand Down
Loading

0 comments on commit aad2bb5

Please sign in to comment.