From 3663f32860e19ec60038ab951c13a82ca32628b2 Mon Sep 17 00:00:00 2001 From: katspaugh Date: Fri, 28 Jul 2023 07:48:59 +0200 Subject: [PATCH] Await txDetails; disallow batching delegate calls; other PR comments --- .../batch/BatchSidebar/BatchTxItem.tsx | 15 ++++---- .../tx/SignOrExecuteForm/SignForm.tsx | 6 ++-- src/components/tx/SignOrExecuteForm/hooks.ts | 2 +- src/components/tx/SignOrExecuteForm/index.tsx | 7 ++-- src/hooks/useDecodeTx.ts | 6 +++- src/hooks/useDraftBatch.ts | 24 ++++++------- .../tx/tx-sender/__tests__/ts-sender.test.ts | 36 ++++++------------- src/store/batchSlice.ts | 3 +- 8 files changed, 43 insertions(+), 56 deletions(-) diff --git a/src/components/batch/BatchSidebar/BatchTxItem.tsx b/src/components/batch/BatchSidebar/BatchTxItem.tsx index 86fa930851..ec0ca14770 100644 --- a/src/components/batch/BatchSidebar/BatchTxItem.tsx +++ b/src/components/batch/BatchSidebar/BatchTxItem.tsx @@ -32,14 +32,13 @@ const BatchTxItem = ({ draggable = false, }: BatchTxItemProps) => { const txSummary = useMemo( - () => - ({ - timestamp, - id: txDetails.txId, - txInfo: txDetails.txInfo, - txStatus: txDetails.txStatus, - safeAppInfo: txDetails.safeAppInfo, - }), + () => ({ + timestamp, + id: txDetails.txId, + txInfo: txDetails.txInfo, + txStatus: txDetails.txStatus, + safeAppInfo: txDetails.safeAppInfo, + }), [timestamp, txDetails], ) diff --git a/src/components/tx/SignOrExecuteForm/SignForm.tsx b/src/components/tx/SignOrExecuteForm/SignForm.tsx index 34686bc4c9..c5bbc39fa1 100644 --- a/src/components/tx/SignOrExecuteForm/SignForm.tsx +++ b/src/components/tx/SignOrExecuteForm/SignForm.tsx @@ -22,7 +22,7 @@ const SignForm = ({ disableSubmit = false, origin, isBatch, - isMultisend, + isBatchable, }: SignOrExecuteProps & { safeTx?: SafeTransaction }): ReactElement => { @@ -90,8 +90,8 @@ const SignForm = ({ {isCreation && !isBatch && ( )} diff --git a/src/components/tx/SignOrExecuteForm/hooks.ts b/src/components/tx/SignOrExecuteForm/hooks.ts index b7db5b5f8a..b3f94d218e 100644 --- a/src/components/tx/SignOrExecuteForm/hooks.ts +++ b/src/components/tx/SignOrExecuteForm/hooks.ts @@ -68,7 +68,7 @@ export const useTxActions = (): TxActions => { assertWallet(wallet) const id = await proposeTx(wallet.address, safeTx, undefined, origin) - addTxToBatch(id) + await addTxToBatch(id) return id } diff --git a/src/components/tx/SignOrExecuteForm/index.tsx b/src/components/tx/SignOrExecuteForm/index.tsx index 59f68fc0fd..362a363843 100644 --- a/src/components/tx/SignOrExecuteForm/index.tsx +++ b/src/components/tx/SignOrExecuteForm/index.tsx @@ -15,7 +15,7 @@ import { selectSettings } from '@/store/settingsSlice' import { RedefineBalanceChanges } from '../security/redefine/RedefineBalanceChange' import UnknownContractError from './UnknownContractError' import RiskConfirmationError from './RiskConfirmationError' -import useDecodeTx, { isMultisendTx } from '@/hooks/useDecodeTx' +import useDecodeTx, { isDelegateCall, isMultisendTx } from '@/hooks/useDecodeTx' import { ErrorBoundary } from '@sentry/react' import ApprovalEditor from '../ApprovalEditor' @@ -26,7 +26,7 @@ export type SignOrExecuteProps = { isExecutable?: boolean isRejection?: boolean isBatch?: boolean - isMultisend?: boolean + isBatchable?: boolean onlyExecute?: boolean disableSubmit?: boolean origin?: string @@ -40,6 +40,7 @@ const SignOrExecuteForm = (props: SignOrExecuteProps): ReactElement => { const isNewExecutableTx = useImmediatelyExecutable() && isCreation const isCorrectNonce = useValidateNonce(safeTx) const [decodedData, decodedDataError, decodedDataLoading] = useDecodeTx(safeTx) + const isBatchable = safeTx && !isDelegateCall(safeTx) && !isMultisendTx(decodedData) // If checkbox is checked and the transaction is executable, execute it, otherwise sign it const canExecute = isCorrectNonce && (props.isExecutable || isNewExecutableTx) @@ -93,7 +94,7 @@ const SignOrExecuteForm = (props: SignOrExecuteProps): ReactElement => { {willExecute ? ( ) : ( - + )} diff --git a/src/hooks/useDecodeTx.ts b/src/hooks/useDecodeTx.ts index 18819f6598..c23bdd9f1d 100644 --- a/src/hooks/useDecodeTx.ts +++ b/src/hooks/useDecodeTx.ts @@ -1,4 +1,4 @@ -import { type SafeTransaction } from '@safe-global/safe-core-sdk-types' +import { OperationType, type SafeTransaction } from '@safe-global/safe-core-sdk-types' import { type DecodedDataResponse, getDecodedData } from '@safe-global/safe-gateway-typescript-sdk' import { getNativeTransferData } from '@/services/tx/tokenTransferParams' import { isEmptyHexData } from '@/utils/hex' @@ -25,4 +25,8 @@ export const isMultisendTx = (decodedData?: DecodedDataResponse): boolean => { return !!decodedData?.parameters?.[0]?.valueDecoded } +export const isDelegateCall = (safeTx: SafeTransaction): boolean => { + return safeTx.data.operation === OperationType.DelegateCall +} + export default useDecodeTx diff --git a/src/hooks/useDraftBatch.ts b/src/hooks/useDraftBatch.ts index e14e6dd897..d7687440ec 100644 --- a/src/hooks/useDraftBatch.ts +++ b/src/hooks/useDraftBatch.ts @@ -14,20 +14,20 @@ export const useUpdateBatch = () => { const dispatch = useAppDispatch() const onAdd = useCallback( - (id: string) => { - getTransactionDetails(chainId, id).then((tx) => { - dispatch( - addTx({ - chainId, - safeAddress, - txDetails: tx, - }), - ) + async (id: string): Promise => { + const txDetails = await getTransactionDetails(chainId, id) + + dispatch( + addTx({ + chainId, + safeAddress, + txDetails, + }), + ) - txDispatch(TxEvent.BATCH_ADD, { txId: id }) + txDispatch(TxEvent.BATCH_ADD, { txId: id }) - trackEvent({ ...BATCH_EVENTS.BATCH_TX_APPENDED, label: tx.txInfo.type }) - }) + trackEvent({ ...BATCH_EVENTS.BATCH_TX_APPENDED, label: txDetails.txInfo.type }) }, [dispatch, chainId, safeAddress], ) diff --git a/src/services/tx/tx-sender/__tests__/ts-sender.test.ts b/src/services/tx/tx-sender/__tests__/ts-sender.test.ts index 90b5f09433..e12c1fa26c 100644 --- a/src/services/tx/tx-sender/__tests__/ts-sender.test.ts +++ b/src/services/tx/tx-sender/__tests__/ts-sender.test.ts @@ -1,6 +1,6 @@ import { setSafeSDK } from '@/hooks/coreSDK/safeCoreSDK' import type Safe from '@safe-global/safe-core-sdk' -import type { SafeSignature, SafeTransaction, TransactionResult } from '@safe-global/safe-core-sdk-types' +import type { TransactionResult } from '@safe-global/safe-core-sdk-types' import { type TransactionDetails } from '@safe-global/safe-gateway-typescript-sdk' import { getTransactionDetails } from '@safe-global/safe-gateway-typescript-sdk' import extractTxInfo from '../../extractTxInfo' @@ -31,29 +31,7 @@ const setupFetchStub = (data: any) => (_url: string) => { import type { EIP1193Provider, OnboardAPI, WalletState, AppState } from '@web3-onboard/core' import { hexZeroPad } from 'ethers/lib/utils' import { generatePreValidatedSignature } from '@safe-global/safe-core-sdk/dist/src/utils/signatures' - -const createSafeTx = (data = '0x'): SafeTransaction => { - return { - data: { - to: '0x0000000000000000000000000000000000000000', - value: '0x0', - data, - operation: 0, - nonce: 100, - }, - signatures: new Map([]), - addSignature: function (sig: SafeSignature): void { - this.signatures.set(sig.signer, sig) - }, - encodedSignatures: function (): string { - return Array.from(this.signatures) - .map(([, sig]) => { - return [sig.signer, sig.data].join(' = ') - }) - .join('; ') - }, - } as SafeTransaction -} +import { createMockSafeTransaction } from '@/tests/transactions' // Mock getTransactionDetails jest.mock('@safe-global/safe-gateway-typescript-sdk', () => ({ @@ -237,7 +215,10 @@ describe('txSender', () => { }) it('should dispatch a PROPOSED event if tx is signed and has no id', async () => { - const tx = createSafeTx() + const tx = createMockSafeTransaction({ + to: '0x123', + data: '0x0', + }) tx.addSignature(generatePreValidatedSignature('0x1234567890123456789012345678901234567890')) const proposedTx = await dispatchTxProposal({ chainId: '4', safeAddress: '0x123', sender: '0x456', safeTx: tx }) @@ -249,7 +230,10 @@ describe('txSender', () => { }) it('should dispatch a SIGNATURE_PROPOSED event if tx has signatures and an id', async () => { - const tx = createSafeTx() + const tx = createMockSafeTransaction({ + to: '0x123', + data: '0x0', + }) tx.addSignature(generatePreValidatedSignature('0x1234567890123456789012345678901234567890')) const proposedTx = await dispatchTxProposal({ diff --git a/src/store/batchSlice.ts b/src/store/batchSlice.ts index f7aaffe75c..3a7a84ea45 100644 --- a/src/store/batchSlice.ts +++ b/src/store/batchSlice.ts @@ -31,7 +31,7 @@ export const batchSlice = createSlice({ ) => { const { chainId, safeAddress, items } = action.payload state[chainId] = state[chainId] || {} - // @ts-ignore + // @ts-expect-error state[chainId][safeAddress] = items }, @@ -47,7 +47,6 @@ export const batchSlice = createSlice({ const { chainId, safeAddress, txDetails } = action.payload state[chainId] = state[chainId] || {} state[chainId][safeAddress] = state[chainId][safeAddress] || [] - // @ts-ignore state[chainId][safeAddress].push({ id: Math.random().toString(36).slice(2), timestamp: Date.now(),