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

Fix: show relays even when 0 relays left #2789

Merged
merged 5 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -3,7 +3,7 @@
"homepage": "https://github.com/safe-global/safe-wallet-web",
"license": "GPL-3.0",
"type": "module",
"version": "1.22.0",
"version": "1.21.0",
"scripts": {
"dev": "next dev",
"start": "next dev",
Expand Down
2 changes: 1 addition & 1 deletion src/components/tx/ExecutionMethodSelector/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const ExecutionMethodSelector = ({
</FormControl>
</div>

{shouldRelay && relays ? <SponsoredBy relays={relays} tooltip={tooltip} /> : null}
<SponsoredBy relays={relays} tooltip={tooltip} shouldRelay={shouldRelay} />
</Box>
)
}
25 changes: 14 additions & 11 deletions src/components/tx/SignOrExecuteForm/ExecuteForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { useIsExecutionLoop, useTxActions } from './hooks'
import { useRelaysBySafe } from '@/hooks/useRemainingRelays'
import useWalletCanRelay from '@/hooks/useWalletCanRelay'
import { ExecutionMethod, ExecutionMethodSelector } from '../ExecutionMethodSelector'
import { hasRemainingRelays } from '@/utils/relaying'
import type { SignOrExecuteProps } from '.'
import type { SafeTransaction } from '@safe-global/safe-core-sdk-types'
import { TxModalContext } from '@/components/tx-flow'
Expand Down Expand Up @@ -53,15 +52,13 @@ const ExecuteForm = ({
// Check that the transaction is executable
const isExecutionLoop = useIsExecutionLoop()

// We default to relay, but the option is only shown if we canRelay
const [executionMethod, setExecutionMethod] = useState(ExecutionMethod.RELAY)

// SC wallets can relay fully signed transactions
const [walletCanRelay] = useWalletCanRelay(safeTx)

const [canWalletRelay] = useWalletCanRelay(safeTx)
// We default to relay
const [executionMethod, setExecutionMethod] = useState(ExecutionMethod.RELAY)
// The transaction can/will be relayed
const canRelay = walletCanRelay && hasRemainingRelays(relays)
const willRelay = canRelay && executionMethod === ExecutionMethod.RELAY
const willRelay = executionMethod === ExecutionMethod.RELAY
const hasRelays = !!relays?.remaining

// Estimate gas limit
const { gasLimit, gasLimitError } = useGasLimit(safeTx)
Expand Down Expand Up @@ -102,12 +99,18 @@ const ExecuteForm = ({

const cannotPropose = !isOwner && !onlyExecute
const submitDisabled =
!safeTx || !isSubmittable || disableSubmit || isValidExecutionLoading || isExecutionLoop || cannotPropose
!safeTx ||
!isSubmittable ||
disableSubmit ||
isValidExecutionLoading ||
isExecutionLoop ||
cannotPropose ||
(willRelay && !hasRelays)

return (
<>
<form onSubmit={handleSubmit}>
<div className={classNames(css.params, { [css.noBottomBorderRadius]: canRelay })}>
<div className={classNames(css.params, { [css.noBottomBorderRadius]: canWalletRelay })}>
<AdvancedParams
willExecute
params={advancedParams}
Expand All @@ -117,7 +120,7 @@ const ExecuteForm = ({
willRelay={willRelay}
/>

{canRelay && (
{canWalletRelay && (
<div className={css.noTopBorder}>
<ExecutionMethodSelector
executionMethod={executionMethod}
Expand Down
87 changes: 58 additions & 29 deletions src/components/tx/SponsoredBy/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,43 +11,72 @@ export const SPONSOR_LOGOS = {
[chains.gor]: '/images/common/token-placeholder.svg',
}

const SponsoredBy = ({ relays, tooltip }: { relays: RelayResponse; tooltip?: string }) => {
const SponsoredBy = ({
relays,
tooltip,
shouldRelay,
}: {
relays?: RelayResponse
tooltip?: string
shouldRelay: boolean
}) => {
const chain = useCurrentChain()

return (
<Box className={css.sponsoredBy}>
<SvgIcon component={GasStationIcon} inheritViewBox className={css.icon} />
<div>
<Stack direction="row" spacing={0.5} alignItems="center">
<Typography variant="body2" fontWeight={700} letterSpacing="0.1px">
Sponsored by

{shouldRelay ? (
<div>
<Stack direction="row" spacing={0.5} alignItems="center">
<Typography variant="body2" fontWeight={700} letterSpacing="0.1px">
Sponsored by
</Typography>

<img src={SPONSOR_LOGOS[chain?.chainId || '']} alt={chain?.chainName} className={css.logo} />
<Typography variant="body2" fontWeight={700} letterSpacing="0.1px">
{chain?.chainName}
</Typography>

{tooltip ? (
<Tooltip title={tooltip} placement="top" arrow>
<span style={{ display: 'flex' }}>
<SvgIcon
component={InfoIcon}
inheritViewBox
color="info"
fontSize="small"
sx={{ verticalAlign: 'middle', color: '#B2B5B2' }}
/>
</span>
</Tooltip>
) : null}
</Stack>

<Typography variant="body2" color="primary.light">
Transactions per hour:{' '}
<Box component="span" sx={{ fontWeight: '700', color: 'text.primary' }}>
{relays?.remaining ?? 0} of {relays?.limit ?? 0}
</Box>
{relays && !relays.remaining && (
<Box component="span" sx={{ color: 'error.main' }}>
{' '}
&mdash; will reset in the next hour
</Box>
Comment on lines +61 to +65
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is slightly redundant since the label already says "Transactions per hour". Maybe we could instead make the "0 of 5" a red color?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this based on Liliya's feedback. It might be redundant, but it's definitely clearer. IMO we should err on the safe side.

)}
</Typography>
<img src={SPONSOR_LOGOS[chain?.chainId || '']} alt={chain?.chainName} className={css.logo} />
</div>
) : (
<div>
<Typography variant="body2" fontWeight={700} letterSpacing="0.1px">
{chain?.chainName}
Pay gas from the connected wallet
</Typography>

<Typography variant="body2" color="primary.light">
Please make sure your wallet has sufficient funds.
</Typography>
{tooltip ? (
<Tooltip title={tooltip} placement="top" arrow>
<span style={{ display: 'flex' }}>
<SvgIcon
component={InfoIcon}
inheritViewBox
color="info"
fontSize="small"
sx={{ verticalAlign: 'middle', color: '#B2B5B2' }}
/>
</span>
</Tooltip>
) : null}
</Stack>

<Typography variant="body2" color="primary.light">
Transactions per hour:{' '}
<Box component="span" sx={{ fontWeight: '700', color: 'text.primary' }}>
{relays.remaining} of {relays.limit}
</Box>
</Typography>
</div>
</div>
)}
</Box>
)
}
Expand Down
8 changes: 6 additions & 2 deletions src/hooks/useWalletCanRelay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,18 @@ import useWallet from '@/hooks/wallets/useWallet'
import { isSmartContractWallet } from '@/utils/wallets'
import { Errors, logError } from '@/services/exceptions'
import { type SafeTransaction } from '@safe-global/safe-core-sdk-types'
import { FEATURES, hasFeature } from '@/utils/chains'
import { useCurrentChain } from './useChains'

const useWalletCanRelay = (tx: SafeTransaction | undefined) => {
const { safe } = useSafeInfo()
const wallet = useWallet()
const chain = useCurrentChain()
const isFeatureEnabled = chain && hasFeature(chain, FEATURES.RELAYING)
const hasEnoughSignatures = tx && tx.signatures.size >= safe.threshold

return useAsync(() => {
if (!tx || !wallet) return
if (!isFeatureEnabled || !tx || !wallet) return

return isSmartContractWallet(wallet)
.then((isSCWallet) => {
Expand All @@ -23,7 +27,7 @@ const useWalletCanRelay = (tx: SafeTransaction | undefined) => {
logError(Errors._106, err.message)
return false
})
}, [hasEnoughSignatures, tx, wallet])
}, [isFeatureEnabled, hasEnoughSignatures, tx, wallet])
}

export default useWalletCanRelay
Loading