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

[Epic] Safe Proposers #4426

Merged
merged 23 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
7e491e7
init
usame-algan Oct 17, 2024
2e12779
feat: Add remove delegate option and adjust delegate list layout (#4390)
usame-algan Oct 25, 2024
e61ee66
feat: Show Proposal chip for unsigned transactions in the queue (#4422)
usame-algan Oct 25, 2024
bde0a17
feat: Allow deletion of delegate transactions from the queue [SW-297]…
usame-algan Oct 25, 2024
0fa2286
Merge remote-tracking branch 'origin/dev' into epic/safe-proposers
usame-algan Oct 25, 2024
1cdbdea
fix: Disable add proposer and delete proposer [SW-400] [SW-396] (#4429)
usame-algan Oct 28, 2024
0577a01
fix: Adjust message when proposing transaction (#4435)
usame-algan Oct 29, 2024
38e262c
feat: Edit proposer dialog [SW-391] [SW-396] (#4436)
usame-algan Oct 29, 2024
54ed079
feat: Show proposer address in queue (#4443)
usame-algan Oct 30, 2024
2e3ddde
fix: Hide tooltip on confirm button for proposers (#4444)
usame-algan Oct 30, 2024
b83c112
fix: Use safe owner address for tenderly simulation with proposer (#4…
usame-algan Oct 30, 2024
55c7b1b
fix: Only show proposal chip if transaction is not pending (#4450)
usame-algan Oct 30, 2024
808a966
Merge remote-tracking branch 'origin/dev' into epic/safe-proposers
usame-algan Oct 31, 2024
0db3629
fix: Allow owners to be added as proposers [SW-407] [SW-428] [SW-381]…
usame-algan Oct 31, 2024
f431e9d
fix: Update testid to fix add owner smoke test
usame-algan Oct 31, 2024
7d82e51
fix: Hide batch button for proposers (#4457)
usame-algan Nov 1, 2024
94d442c
Merge remote-tracking branch 'origin/dev' into epic/safe-proposers
usame-algan Nov 4, 2024
3fdcc4a
feat: Support hardware wallets for adding and removing proposers (#4466)
usame-algan Nov 5, 2024
8fe96b0
feat: Display creator in the proposer list [SW-408] [SW-470] (#4471)
usame-algan Nov 5, 2024
b14ea56
fix: AdjustVInSignature when managing proposers with a hardware walle…
usame-algan Nov 5, 2024
92111aa
Merge remote-tracking branch 'origin/dev' into epic/safe-proposers
usame-algan Nov 5, 2024
2396942
fix: Add proposers feature flag (#4488)
usame-algan Nov 6, 2024
b5b354d
Merge remote-tracking branch 'origin/dev' into epic/safe-proposers
usame-algan Nov 6, 2024
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
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
Loading