Skip to content

Commit

Permalink
Fix: warn if wallet already signed a tx
Browse files Browse the repository at this point in the history
  • Loading branch information
katspaugh committed Jul 28, 2023
1 parent cd2e9ac commit ac031a9
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 6 deletions.
4 changes: 2 additions & 2 deletions src/components/tx-flow/flows/SuccessScreen/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ export const SuccessScreen = ({ txId }: { txId: string }) => {
<Divider />
<div className={classnames(css.row, css.buttons)}>
<Link href={homeLink} passHref>
<Button variant="outlined" size="small">
Back to dashboard
<Button variant="contained" size="small">
Finish
</Button>
</Link>
{txLink && (
Expand Down
2 changes: 1 addition & 1 deletion src/components/tx/SignOrExecuteForm/ExecuteForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ const ExecuteForm = ({
<CheckWallet allowNonOwner={onlyExecute}>
{(isOk) => (
<Button variant="contained" type="submit" disabled={!isOk || submitDisabled}>
Submit
Execute
</Button>
)}
</CheckWallet>
Expand Down
7 changes: 5 additions & 2 deletions src/components/tx/SignOrExecuteForm/SignForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import ErrorMessage from '@/components/tx/ErrorMessage'
import { logError, Errors } from '@/services/exceptions'
import useIsSafeOwner from '@/hooks/useIsSafeOwner'
import CheckWallet from '@/components/common/CheckWallet'
import { useTxActions } from './hooks'
import { useAlreadySigned, useTxActions } from './hooks'
import type { SignOrExecuteProps } from '.'
import type { SafeTransaction } from '@safe-global/safe-core-sdk-types'
import { TxModalContext } from '@/components/tx-flow'
Expand All @@ -32,6 +32,7 @@ const SignForm = ({
const { signTx } = useTxActions()
const { setTxFlow } = useContext(TxModalContext)
const { needsRiskConfirmation, isRiskConfirmed, setIsRiskIgnored } = useContext(TxSecurityContext)
const hasSigned = useAlreadySigned(safeTx)

// On modal submit
const handleSubmit = async (e: SyntheticEvent) => {
Expand Down Expand Up @@ -64,6 +65,8 @@ const SignForm = ({

return (
<form onSubmit={handleSubmit}>
{hasSigned && <ErrorMessage level="warning">You have already signed this transaction.</ErrorMessage>}

{cannotPropose ? (
<NonOwnerError />
) : (
Expand All @@ -79,7 +82,7 @@ const SignForm = ({
<CheckWallet>
{(isOk) => (
<Button variant="contained" type="submit" disabled={!isOk || submitDisabled}>
Submit
Sign
</Button>
)}
</CheckWallet>
Expand Down
41 changes: 40 additions & 1 deletion src/components/tx/SignOrExecuteForm/hooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as pending from '@/hooks/usePendingTxs'
import * as txSender from '@/services/tx/tx-sender/dispatch'
import * as onboardHooks from '@/hooks/wallets/useOnboard'
import { type OnboardAPI } from '@web3-onboard/core'
import { useImmediatelyExecutable, useIsExecutionLoop, useTxActions, useValidateNonce } from './hooks'
import { useAlreadySigned, useImmediatelyExecutable, useIsExecutionLoop, useTxActions, useValidateNonce } from './hooks'

const createSafeTx = (data = '0x'): SafeTransaction => {
return {
Expand Down Expand Up @@ -542,4 +542,43 @@ describe('SignOrExecute hooks', () => {
expect(relaySpy).not.toHaveBeenCalled()
})
})

describe('useAlreadySigned', () => {
it('should return true if wallet already signed a tx', () => {
// Wallet
jest.spyOn(wallet, 'default').mockReturnValue({
chainId: '1',
label: 'MetaMask',
address: '0x1234567890000000000000000000000000000000',
} as unknown as ConnectedWallet)

const tx = createSafeTx()
tx.addSignature({
signer: '0x1234567890000000000000000000000000000000',
data: '0x0001',
staticPart: () => '',
dynamicPart: () => '',
})
const { result } = renderHook(() => useAlreadySigned(tx))
expect(result.current).toEqual(true)
})
})
it('should return false if wallet has not signed a tx yet', () => {
// Wallet
jest.spyOn(wallet, 'default').mockReturnValue({
chainId: '1',
label: 'MetaMask',
address: '0x1234567890000000000000000000000000000000',
} as unknown as ConnectedWallet)

const tx = createSafeTx()
tx.addSignature({
signer: '0x00000000000000000000000000000000000000000',
data: '0x0001',
staticPart: () => '',
dynamicPart: () => '',
})
const { result } = renderHook(() => useAlreadySigned(tx))
expect(result.current).toEqual(false)
})
})
7 changes: 7 additions & 0 deletions src/components/tx/SignOrExecuteForm/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,10 @@ export const useSafeTxGas = (safeTx: SafeTransaction | undefined): number | unde

return safeTxGas
}

export const useAlreadySigned = (safeTx: SafeTransaction | undefined): boolean => {
const wallet = useWallet()
const hasSigned =
safeTx && wallet && (safeTx.signatures.has(wallet.address.toLowerCase()) || safeTx.signatures.has(wallet.address))
return Boolean(hasSigned)
}

0 comments on commit ac031a9

Please sign in to comment.