Skip to content

Commit

Permalink
feat: Safe Proposers (#4426)
Browse files Browse the repository at this point in the history
* init

* feat: Add remove delegate option and adjust delegate list layout (#4390)

* feat: Add remove delegate button, adjust delegate list layout

* feat: Update gateway-sdk package, add delegate form

* feat: Optimistically update delegates cache when adding or removing delegate

* fix: Add missing ga events, add enum

* chore: Update gateway-sdk package

* fix: Update setup settings layout

* fix: Add notifications when adding and removing proposer

* fix: Add validation for add proposer

* fix: Rename delegate to proposer

* fix: Rename variable for add proposer dialog

* fix: Align remove icons in tables

* fix: Handle update proposer in rtk query

* feat: Show Proposal chip for unsigned transactions in the queue (#4422)

* feat: Allow deletion of delegate transactions from the queue [SW-297] (#4400)

* init

* feat: Allow deletion of delegate transactions from the queue

* feat: Add text to signer view if tx is from proposer

* fix: Disable add proposer and delete proposer [SW-400] [SW-396] (#4429)

* fix: Disable add proposer and delete proposer

* fix: Account for owners that are proposers in CheckWallet

* fix: Adjust message when proposing transaction (#4435)

* feat: Edit proposer dialog [SW-391] [SW-396] (#4436)

* feat: Show proposer address in queue (#4443)

* fix: Hide tooltip on confirm button for proposers (#4444)

* fix: Use safe owner address for tenderly simulation with proposer (#4445)

* fix: Only show proposal chip if transaction is not pending (#4450)

* fix: Allow owners to be added as proposers [SW-407] [SW-428] [SW-381] (#4446)

* fix: Remove scrollbar when adding proposer

* fix: Allow owners being added as proposers

* fix: Add check that isProposing only when its also a creation

* fix: Update testid to fix add owner smoke test

* fix: Hide batch button for proposers (#4457)

* feat: Support hardware wallets for adding and removing proposers (#4466)

* feat: Display creator in the proposer list [SW-408] [SW-470] (#4471)

* feat: Display creator in proposer list

* fix: Correctly update proposers when editing and deleting

* fix: Remove dangling console.log

* fix: Add network switch to delete proposer dialog and reset values when closing

* fix: AdjustVInSignature when managing proposers with a hardware wallet (#4477)

* fix: Add proposers feature flag (#4488)
  • Loading branch information
usame-algan authored Nov 6, 2024
1 parent d90cbfa commit 50e44fe
Show file tree
Hide file tree
Showing 42 changed files with 1,211 additions and 209 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
"@safe-global/protocol-kit": "^4.1.1",
"@safe-global/safe-apps-sdk": "^9.1.0",
"@safe-global/safe-deployments": "1.37.12",
"@safe-global/safe-client-gateway-sdk": "v1.60.1",
"@safe-global/safe-client-gateway-sdk": "1.60.1-next-069fa2b",
"@safe-global/safe-gateway-typescript-sdk": "3.22.3-beta.15",
"@safe-global/safe-modules-deployments": "^2.2.1",
"@sentry/react": "^7.91.0",
Expand Down
1 change: 0 additions & 1 deletion src/components/common/AddressInput/styles.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,4 @@

.readOnly :global .MuiInputBase-input {
visibility: hidden;
position: absolute;
}
32 changes: 27 additions & 5 deletions src/components/common/CheckWallet/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import useIsSafeOwner from '@/hooks/useIsSafeOwner'
import useIsWrongChain from '@/hooks/useIsWrongChain'
import useWallet from '@/hooks/wallets/useWallet'
import { chainBuilder } from '@/tests/builders/chains'
import { useIsWalletDelegate } from '@/hooks/useDelegates'
import { useIsWalletProposer } from '@/hooks/useProposers'
import { faker } from '@faker-js/faker'
import { extendedSafeInfoBuilder } from '@/tests/builders/safe'
import useSafeInfo from '@/hooks/useSafeInfo'
Expand Down Expand Up @@ -42,9 +42,9 @@ jest.mock('@/hooks/useIsWrongChain', () => ({
default: jest.fn(() => false),
}))

jest.mock('@/hooks/useDelegates', () => ({
jest.mock('@/hooks/useProposers', () => ({
__esModule: true,
useIsWalletDelegate: jest.fn(() => false),
useIsWalletProposer: jest.fn(() => false),
}))

jest.mock('@/hooks/useSafeInfo', () => ({
Expand Down Expand Up @@ -125,15 +125,37 @@ describe('CheckWallet', () => {
expect(allowContainer.querySelector('button')).not.toBeDisabled()
})

it('should not disable the button for delegates', () => {
it('should not disable the button for proposers', () => {
;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(false)
;(useIsWalletDelegate as jest.MockedFunction<typeof useIsWalletDelegate>).mockReturnValueOnce(true)
;(useIsWalletProposer as jest.MockedFunction<typeof useIsWalletProposer>).mockReturnValueOnce(true)

const { container } = renderButton()

expect(container.querySelector('button')).not.toBeDisabled()
})

it('should disable the button for proposers if specified via flag', () => {
;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(false)
;(useIsWalletProposer as jest.MockedFunction<typeof useIsWalletProposer>).mockReturnValueOnce(true)

const { getByText } = render(
<CheckWallet allowProposer={false}>{(isOk) => <button disabled={!isOk}>Continue</button>}</CheckWallet>,
)

expect(getByText('Continue')).toBeDisabled()
})

it('should not disable the button for proposers that are also owners', () => {
;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(true)
;(useIsWalletProposer as jest.MockedFunction<typeof useIsWalletProposer>).mockReturnValueOnce(true)

const { getByText } = render(
<CheckWallet allowProposer={false}>{(isOk) => <button disabled={!isOk}>Continue</button>}</CheckWallet>,
)

expect(getByText('Continue')).not.toBeDisabled()
})

it('should disable the button for counterfactual Safes', () => {
;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(true)

Expand Down
16 changes: 12 additions & 4 deletions src/components/common/CheckWallet/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useIsWalletDelegate } from '@/hooks/useDelegates'
import { useIsWalletProposer } from '@/hooks/useProposers'
import { useMemo, type ReactElement } from 'react'
import useIsOnlySpendingLimitBeneficiary from '@/hooks/useIsOnlySpendingLimitBeneficiary'
import useIsSafeOwner from '@/hooks/useIsSafeOwner'
Expand All @@ -15,6 +15,7 @@ type CheckWalletProps = {
noTooltip?: boolean
checkNetwork?: boolean
allowUndeployedSafe?: boolean
allowProposer?: boolean
}

enum Message {
Expand All @@ -30,13 +31,14 @@ const CheckWallet = ({
noTooltip,
checkNetwork = false,
allowUndeployedSafe = false,
allowProposer = true,
}: CheckWalletProps): ReactElement => {
const wallet = useWallet()
const isSafeOwner = useIsSafeOwner()
const isOnlySpendingLimit = useIsOnlySpendingLimitBeneficiary()
const connectWallet = useConnectWallet()
const isWrongChain = useIsWrongChain()
const isDelegate = useIsWalletDelegate()
const isProposer = useIsWalletProposer()

const { safe } = useSafeInfo()

Expand All @@ -46,18 +48,24 @@ const CheckWallet = ({
if (!wallet) {
return Message.WalletNotConnected
}

if (isUndeployedSafe && !allowUndeployedSafe) {
return Message.SafeNotActivated
}

if (!allowNonOwner && !isSafeOwner && !isDelegate && (!isOnlySpendingLimit || !allowSpendingLimit)) {
if (!allowNonOwner && !isSafeOwner && !isProposer && (!isOnlySpendingLimit || !allowSpendingLimit)) {
return Message.NotSafeOwner
}

if (!allowProposer && isProposer && !isSafeOwner) {
return Message.NotSafeOwner
}
}, [
allowNonOwner,
allowProposer,
allowSpendingLimit,
allowUndeployedSafe,
isDelegate,
isProposer,
isOnlySpendingLimit,
isSafeOwner,
isUndeployedSafe,
Expand Down
129 changes: 129 additions & 0 deletions src/components/common/Header/index.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import Header from '@/components/common/Header/index'
import * as useChains from '@/hooks/useChains'
import * as useIsSafeOwner from '@/hooks/useIsSafeOwner'
import * as useProposers from '@/hooks/useProposers'
import * as useSafeAddress from '@/hooks/useSafeAddress'
import * as useSafeTokenEnabled from '@/hooks/useSafeTokenEnabled'
import { render } from '@/tests/test-utils'
import { faker } from '@faker-js/faker'
import { screen, fireEvent } from '@testing-library/react'

jest.mock(
'@/components/common/SafeTokenWidget',
() =>
function SafeTokenWidget() {
return <div>SafeTokenWidget</div>
},
)

jest.mock(
'@/features/walletconnect/components',
() =>
function WalletConnect() {
return <div>WalletConnect</div>
},
)

jest.mock(
'@/components/common/NetworkSelector',
() =>
function NetworkSelector() {
return <div>NetworkSelector</div>
},
)

describe('Header', () => {
beforeEach(() => {
jest.resetAllMocks()
})

it('renders the menu button when onMenuToggle is provided', () => {
render(<Header onMenuToggle={jest.fn()} />)
expect(screen.getByLabelText('menu')).toBeInTheDocument()
})

it('does not render the menu button when onMenuToggle is not provided', () => {
render(<Header />)
expect(screen.queryByLabelText('menu')).not.toBeInTheDocument()
})

it('calls onMenuToggle when menu button is clicked', () => {
const onMenuToggle = jest.fn()
render(<Header onMenuToggle={onMenuToggle} />)

const menuButton = screen.getByLabelText('menu')
fireEvent.click(menuButton)

expect(onMenuToggle).toHaveBeenCalled()
})

it('renders the SafeTokenWidget when showSafeToken is true', () => {
jest.spyOn(useSafeTokenEnabled, 'useSafeTokenEnabled').mockReturnValue(true)

render(<Header />)
expect(screen.getByText('SafeTokenWidget')).toBeInTheDocument()
})

it('does not render the SafeTokenWidget when showSafeToken is false', () => {
jest.spyOn(useSafeTokenEnabled, 'useSafeTokenEnabled').mockReturnValue(false)

render(<Header />)
expect(screen.queryByText('SafeTokenWidget')).not.toBeInTheDocument()
})

it('displays the safe logo', () => {
render(<Header />)
expect(screen.getAllByAltText('Safe logo')[0]).toBeInTheDocument()
})

it('renders the BatchIndicator when showBatchButton is true', () => {
jest.spyOn(useSafeAddress, 'default').mockReturnValue(faker.finance.ethereumAddress())
jest.spyOn(useProposers, 'useIsWalletProposer').mockReturnValue(false)
jest.spyOn(useIsSafeOwner, 'default').mockReturnValue(false)

render(<Header />)
expect(screen.getByTitle('Batch')).toBeInTheDocument()
})

it('does not render the BatchIndicator when there is no safe address', () => {
jest.spyOn(useSafeAddress, 'default').mockReturnValue('')

render(<Header />)
expect(screen.queryByTitle('Batch')).not.toBeInTheDocument()
})

it('does not render the BatchIndicator when connected wallet is a proposer', () => {
jest.spyOn(useProposers, 'useIsWalletProposer').mockReturnValue(true)

render(<Header />)
expect(screen.queryByTitle('Batch')).not.toBeInTheDocument()
})

it('renders the WalletConnect component when enableWc is true', () => {
jest.spyOn(useChains, 'useHasFeature').mockReturnValue(true)

render(<Header />)
expect(screen.getByText('WalletConnect')).toBeInTheDocument()
})

it('does not render the WalletConnect component when enableWc is false', () => {
jest.spyOn(useChains, 'useHasFeature').mockReturnValue(false)

render(<Header />)
expect(screen.queryByText('WalletConnect')).not.toBeInTheDocument()
})

it('renders the NetworkSelector when safeAddress exists', () => {
jest.spyOn(useSafeAddress, 'default').mockReturnValue(faker.finance.ethereumAddress())

render(<Header />)
expect(screen.getByText('NetworkSelector')).toBeInTheDocument()
})

it('does not render the NetworkSelector when safeAddress is falsy', () => {
jest.spyOn(useSafeAddress, 'default').mockReturnValue('')

render(<Header />)
expect(screen.queryByText('NetworkSelector')).not.toBeInTheDocument()
})
})
8 changes: 7 additions & 1 deletion src/components/common/Header/index.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import useIsSafeOwner from '@/hooks/useIsSafeOwner'
import { useIsWalletProposer } from '@/hooks/useProposers'
import type { Dispatch, SetStateAction } from 'react'
import { type ReactElement } from 'react'
import { useRouter } from 'next/router'
Expand Down Expand Up @@ -39,6 +41,8 @@ function getLogoLink(router: ReturnType<typeof useRouter>): Url {
const Header = ({ onMenuToggle, onBatchToggle }: HeaderProps): ReactElement => {
const safeAddress = useSafeAddress()
const showSafeToken = useSafeTokenEnabled()
const isProposer = useIsWalletProposer()
const isSafeOwner = useIsSafeOwner()
const router = useRouter()
const enableWc = useHasFeature(FEATURES.NATIVE_WALLETCONNECT)

Expand All @@ -59,6 +63,8 @@ const Header = ({ onMenuToggle, onBatchToggle }: HeaderProps): ReactElement => {
}
}

const showBatchButton = safeAddress && (!isProposer || isSafeOwner)

return (
<Paper className={css.container}>
<div className={classnames(css.element, css.menuButton)}>
Expand Down Expand Up @@ -91,7 +97,7 @@ const Header = ({ onMenuToggle, onBatchToggle }: HeaderProps): ReactElement => {
<NotificationCenter />
</div>

{safeAddress && (
{showBatchButton && (
<div className={classnames(css.element, css.hideMobile)}>
<BatchIndicator onClick={handleBatchToggle} />
</div>
Expand Down
67 changes: 0 additions & 67 deletions src/components/settings/DelegatesList/index.tsx

This file was deleted.

Loading

0 comments on commit 50e44fe

Please sign in to comment.