Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

(Feature) Proper rejections #2096

Merged
merged 21 commits into from
Apr 7, 2021
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
6e76a2a
remove `isCancelTransaction` utility function in favor of `txInfo.isC…
fernandomg Mar 26, 2021
4cabdcd
replace "cancel" concept in favor of "reject"
fernandomg Mar 26, 2021
dbcdf99
add circle-cross-red icon to "On-chain rejection" transaction info
fernandomg Mar 26, 2021
38d1ad2
identify queued on-chain rejection
fernandomg Mar 29, 2021
68406cc
apply styles to on-chain rejection type identifier
fernandomg Mar 29, 2021
8e812e4
update awaiting messages wording
fernandomg Mar 29, 2021
2f0bb0e
fix styles on styles to on-chain rejection
alongoni Mar 29, 2021
b35d9c0
upgrade SRC dependency and add errorTooltip color
alongoni Mar 29, 2021
f6d28dd
Merge branch 'development' into feature/1652-proper-rejection
nicosampler Mar 30, 2021
67512e6
replace local svg with SRC `Icon` component wherever is possible
fernandomg Mar 30, 2021
faeba47
fix type
fernandomg Mar 30, 2021
e5d3860
Merge branch 'development' into feature/1652-proper-rejection
fernandomg Mar 30, 2021
4d858af
update SRC
fernandomg Mar 31, 2021
ae19e8d
Merge branch 'development' into feature/1652-proper-rejection
fernandomg Mar 31, 2021
670afc6
Merge branch 'development' into feature/1652-proper-rejection
Apr 1, 2021
1440b21
Merge branch 'development' into feature/1652-proper-rejection
fernandomg Apr 6, 2021
b069bea
fix color for Executed/Execute item
fernandomg Apr 6, 2021
bf6bc1d
fix missing `Nonce` label for nonce = 0
fernandomg Apr 6, 2021
5c2113f
Merge branch 'development' into feature/1652-proper-rejection
fernandomg Apr 6, 2021
42a332d
fix legend color
fernandomg Apr 6, 2021
19a5564
Merge branch 'development' into feature/1652-proper-rejection
Apr 7, 2021
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 @@ -161,7 +161,7 @@
"@gnosis.pm/safe-apps-sdk": "1.0.3",
"@gnosis.pm/safe-apps-sdk-v1": "npm:@gnosis.pm/[email protected]",
"@gnosis.pm/safe-contracts": "1.1.1-dev.2",
"@gnosis.pm/safe-react-components": "https://github.com/gnosis/safe-react-components.git#a68a67e",
"@gnosis.pm/safe-react-components": "https://github.com/gnosis/safe-react-components.git#7ebc414",
"@gnosis.pm/util-contracts": "2.0.6",
"@ledgerhq/hw-transport-node-hid-singleton": "5.45.0",
"@material-ui/core": "^4.11.0",
Expand Down
6 changes: 2 additions & 4 deletions src/components/CustomIconText/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Text } from '@gnosis.pm/safe-react-components'
import React from 'react'
import React, { ReactElement } from 'react'
import styled from 'styled-components'

const Wrapper = styled.div`
Expand All @@ -12,11 +12,9 @@ const Icon = styled.img`
margin-right: 9px;
`

const CustomIconText = ({ iconUrl, text }: { iconUrl: string; text?: string }) => (
export const CustomIconText = ({ iconUrl, text }: { iconUrl: string; text?: string }): ReactElement => (
<Wrapper>
<Icon alt={text} src={iconUrl} />
{text && <Text size="xl">{text}</Text>}
</Wrapper>
)

export default CustomIconText
2 changes: 1 addition & 1 deletion src/components/TransactionFailText/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const TransactionFailText = ({
if (isExecution) {
errorMessage =
threshold && threshold > 1
? `To save gas costs, cancel this transaction`
? `To save gas costs, reject this transaction`
Copy link
Contributor

@nicosampler nicosampler Mar 30, 2021

Choose a reason for hiding this comment

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

shouldn't it say To save gas, reject..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably, but not sure. @lukasschor?

Copy link
Member

Choose a reason for hiding this comment

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

I would even say "to save fees, ..." but we should check that this is aligned with other places where we might use similar wording.

: `To save gas costs, avoid executing the transaction.`
}

Expand Down
2 changes: 1 addition & 1 deletion src/logic/safe/store/models/types/gateway.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ type MultiSigExecutionDetails = {
type DetailedExecutionInfo = ModuleExecutionDetails | MultiSigExecutionDetails

type ExpandedTxDetails = {
executedAt: number
executedAt: number | null
txStatus: TransactionStatus
txInfo: TransactionInfo
txData: TransactionData | null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import CircularProgress from '@material-ui/core/CircularProgress'
import React, { ReactElement, useContext, useRef } from 'react'
import styled from 'styled-components'

import CustomIconText from 'src/components/CustomIconText'
import { CustomIconText } from 'src/components/CustomIconText'
import {
isCustomTxInfo,
isMultiSendTxInfo,
Expand All @@ -24,6 +24,7 @@ import { TokenTransferAmount } from './TokenTransferAmount'
import { TxsInfiniteScrollContext } from './TxsInfiniteScroll'
import { TxLocationContext } from './TxLocationProvider'
import { CalculatedVotes } from './TxQueueCollapsed'
import { isCancelTxDetails } from './utils'

const TxInfo = ({ info }: { info: AssetInfo }) => {
if (isTokenTransferAsset(info)) {
Expand Down Expand Up @@ -116,6 +117,8 @@ export const TxCollapsed = ({
const { ref, lastItemId } = useContext(TxsInfiniteScrollContext)

const willBeReplaced = transaction?.txStatus === 'WILL_BE_REPLACED' ? ' will-be-replaced' : ''
const onChainRejection =
isCancelTxDetails(transaction.txInfo) && txLocation !== 'history' ? ' on-chain-rejection' : ''

const txCollapsedNonce = (
<div className={'tx-nonce' + willBeReplaced}>
Expand All @@ -124,7 +127,7 @@ export const TxCollapsed = ({
)

const txCollapsedType = (
<div className={'tx-type' + willBeReplaced}>
<div className={'tx-type' + willBeReplaced + onChainRejection}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we use string literal instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if I tell you I did it this way because I wanted to avoid the white space at the end? (or adding a trim() call to remove it), will it be enough as a justification? 😬

<CustomIconText iconUrl={type.icon} text={type.text} />
</div>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export const TxCollapsedActions = ({ transaction }: TxCollapsedActionsProps): Re
</span>
</Tooltip>
{canCancel && (
<Tooltip title="Cancel" placement="top">
<Tooltip title="Reject" placement="top">
<span>
<IconButton size="small" type="button" onClick={handleCancelButtonClick} disabled={isPending}>
<Icon type="circleCross" color="error" size="sm" />
Expand Down
58 changes: 33 additions & 25 deletions src/routes/safe/components/Transactions/TxList/TxDetails.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Icon, Link, Loader, Text } from '@gnosis.pm/safe-react-components'
import cn from 'classnames'
import React, { ReactElement, useContext } from 'react'
import { useSelector } from 'react-redux'
import styled from 'styled-components'

import {
Expand All @@ -12,7 +11,6 @@ import {
MultiSigExecutionDetails,
Transaction,
} from 'src/logic/safe/store/models/types/gateway.d'
import { safeParamAddressFromStateSelector } from 'src/logic/safe/store/selectors'
import { TransactionActions } from './hooks/useTransactionActions'
import { useTransactionDetails } from './hooks/useTransactionDetails'
import { TxDetailsContainer, Centered, AlignItemsWithMargin } from './styled'
Expand All @@ -30,34 +28,44 @@ const NormalBreakingText = styled(Text)`
`

const TxDataGroup = ({ txDetails }: { txDetails: ExpandedTxDetails }): ReactElement | null => {
const safeAddress = useSelector(safeParamAddressFromStateSelector)

if (isTransferTxInfo(txDetails.txInfo) || isSettingsChangeTxInfo(txDetails.txInfo)) {
return <TxInfo txInfo={txDetails.txInfo} />
}

if (isCancelTxDetails({ executedAt: txDetails.executedAt, txInfo: txDetails.txInfo, safeAddress })) {
if (isCancelTxDetails(txDetails.txInfo)) {
const txNonce = `${(txDetails.detailedExecutionInfo as MultiSigExecutionDetails).nonce ?? NOT_AVAILABLE}`
const isTxExecuted = txDetails.executedAt

// executed rejection transaction
let message = `This is an on-chain rejection that didn't send any funds.
This on-chain rejection replaced all transactions with nonce ${txNonce}.`

if (!isTxExecuted) {
// queued rejection transaction
message = `This is an on-chain rejection that doesn't send any funds.
Executing this on-chain rejection will replace all currently awaiting transactions with nonce ${txNonce}.`
}
return (
<>
<NormalBreakingText size="xl">
{`This is an empty cancelling transaction that doesn't send any funds.
Executing this transaction will replace all currently awaiting transactions with nonce ${
(txDetails.detailedExecutionInfo as MultiSigExecutionDetails).nonce ?? NOT_AVAILABLE
}.`}
</NormalBreakingText>
<Link
href="https://help.gnosis-safe.io/en/articles/4738501-why-do-i-need-to-pay-for-cancelling-a-transaction"
target="_blank"
rel="noreferrer"
title="Why do I need to pay for cancelling a transaction?"
>
<AlignItemsWithMargin>
<Text size="xl" as="span" color="primary">
Why do I need to pay for cancelling a transaction?
</Text>
<Icon size="sm" type="externalLink" color="primary" />
</AlignItemsWithMargin>
</Link>
<NormalBreakingText size="xl">{message}</NormalBreakingText>
{!isTxExecuted && (
<>
<br />
<Link
href="https://help.gnosis-safe.io/en/articles/4738501-why-do-i-need-to-pay-for-cancelling-a-transaction"
target="_blank"
rel="noreferrer"
title="Why do I need to pay for rejecting a transaction?"
>
<AlignItemsWithMargin>
<Text size="xl" as="span" color="primary">
Why do I need to pay for rejecting a transaction?
</Text>
<Icon size="sm" type="externalLink" color="primary" />
</AlignItemsWithMargin>
</Link>
</>
)}
</>
)
}
Expand Down Expand Up @@ -116,7 +124,7 @@ export const TxDetails = ({ transaction, actions }: TxDetailsProps): ReactElemen
'will-be-replaced': transaction.txStatus === 'WILL_BE_REPLACED',
})}
>
<TxOwners detailedExecutionInfo={data.detailedExecutionInfo} />
<TxOwners txDetails={data} />
</div>
{!data.executedAt && txLocation !== 'history' && actions?.isUserAnOwner && (
<div className={cn('tx-details-actions', { 'will-be-replaced': transaction.txStatus === 'WILL_BE_REPLACED' })}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const TxExpandedActions = ({ transaction }: TxExpandedActionsProps): Reac
</Button>
{canCancel && (
<Button size="md" color="error" onClick={handleCancelButtonClick} className="error" disabled={isPending}>
Cancel
Reject
</Button>
)}
</>
Expand Down
56 changes: 36 additions & 20 deletions src/routes/safe/components/Transactions/TxList/TxOwners.tsx
Original file line number Diff line number Diff line change
@@ -1,48 +1,60 @@
import { Text } from '@gnosis.pm/safe-react-components'
import { Text, Icon } from '@gnosis.pm/safe-react-components'
import React, { ReactElement } from 'react'
import styled from 'styled-components'

import Img from 'src/components/layout/Img'
import { ExpandedTxDetails, isModuleExecutionDetails } from 'src/logic/safe/store/models/types/gateway.d'
import TransactionListActive from './assets/transactions-list-active.svg'
import TransactionListInactive from './assets/transactions-list-inactive.svg'
import CheckCircleGreen from './assets/check-circle-green.svg'
import PlusCircleGreen from './assets/plus-circle-green.svg'
import { OwnerRow } from './OwnerRow'
import { OwnerList, OwnerListItem } from './styled'

type TxOwnersProps = {
detailedExecutionInfo: ExpandedTxDetails['detailedExecutionInfo']
}
import { isCancelTxDetails } from './utils'

const StyledImg = styled(Img)`
background-color: transparent;
border-radius: 50%;
`

export const TxOwners = ({ detailedExecutionInfo }: TxOwnersProps): ReactElement | null => {
export const TxOwners = ({ txDetails }: { txDetails: ExpandedTxDetails }): ReactElement | null => {
const { txInfo, detailedExecutionInfo } = txDetails

if (!detailedExecutionInfo || isModuleExecutionDetails(detailedExecutionInfo)) {
return null
}

const confirmationsNeeded = detailedExecutionInfo.confirmationsRequired - detailedExecutionInfo.confirmations.length

const CreationNode = isCancelTxDetails(txInfo) ? (
<OwnerListItem>
<span className="icon">
<Icon size="sm" type="circleCross" color="error" />
</span>
<div className="legend">
<Text color="error" size="xl" strong>
On-chain rejection created
</Text>
</div>
</OwnerListItem>
) : (
<OwnerListItem>
<span className="icon">
<Icon size="sm" type="add" color="primary" />
</span>
<div className="legend">
<Text color="primary" size="xl" strong>
Created
</Text>
</div>
</OwnerListItem>
)

return (
<OwnerList>
<OwnerListItem>
<span className="icon">
<StyledImg alt="" src={PlusCircleGreen} />
</span>
<div className="legend">
<Text color="primary" size="xl" strong>
Created
</Text>
</div>
</OwnerListItem>
{CreationNode}
{detailedExecutionInfo.confirmations.map(({ signer }) => (
<OwnerListItem key={signer}>
<span className="icon">
<StyledImg alt="" src={CheckCircleGreen} />
<Icon size="sm" type="circleCheck" color="primary" />
</span>
<div className="legend">
<Text color="primary" size="xl" strong>
Expand All @@ -55,7 +67,11 @@ export const TxOwners = ({ detailedExecutionInfo }: TxOwnersProps): ReactElement
{confirmationsNeeded <= 0 ? (
<OwnerListItem>
<span className="icon">
<StyledImg alt="" src={detailedExecutionInfo.executor ? CheckCircleGreen : TransactionListActive} />
{detailedExecutionInfo.executor ? (
<Icon type="circleCheck" size="sm" color="primary" />
) : (
<StyledImg alt="" src={TransactionListActive} />
)}
</span>
<div className="legend">
<Text color="primary" size="xl" strong>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const TxSummary = ({ txDetails }: { txDetails: ExpandedTxDetails }): Reac
</Text>
)}
</div>
{nonce && (
{nonce !== undefined && (
<div className="tx-nonce">
<Text size="xl" strong as="span">
Nonce:{' '}
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { getQueuedTransactionsByNonce } from 'src/logic/safe/store/selectors/gat
import { sameAddress } from 'src/logic/wallets/ethAddresses'
import { userAccountSelector } from 'src/logic/wallets/store/selectors'
import { TxLocationContext } from 'src/routes/safe/components/Transactions/TxList/TxLocationProvider'
import { isCancelTransaction } from 'src/routes/safe/components/Transactions/TxList/utils'
import { grantedSelector } from 'src/routes/safe/container/selector'
import { AppReduxState } from 'src/store'

Expand Down Expand Up @@ -60,14 +59,7 @@ export const useTransactionActions = (transaction: Transaction): TransactionActi
canConfirm,
canConfirmThenExecute: txLocation === 'queued.next' && canConfirm && oneToGo,
canExecute: txLocation === 'queued.next' && thresholdReached,
canCancel: !transactionsByNonce.some(
({ txInfo }) =>
isCustomTxInfo(txInfo) &&
isCancelTransaction({
txInfo,
safeAddress,
}),
),
canCancel: !transactionsByNonce.some(({ txInfo }) => isCustomTxInfo(txInfo) && txInfo.isCancellation),
isUserAnOwner,
oneToGo,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ export const useTransactionStatus = (transaction: Transaction): TransactionStatu

switch (transaction.txStatus) {
case 'AWAITING_CONFIRMATIONS':
text = signaturePending(currentUser) ? 'Awaiting your confirmation' : 'Awaiting confirmations'
text = signaturePending(currentUser) ? 'Needs your confirmation' : 'Needs confirmations'
break
case 'AWAITING_EXECUTION':
text = 'Awaiting execution'
text = 'Needs execution'
break
case 'PENDING':
case 'PENDING_FAILED':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import { useSelector } from 'react-redux'
import { Transaction } from 'src/logic/safe/store/models/types/gateway.d'
import { safeParamAddressFromStateSelector } from 'src/logic/safe/store/selectors'
import CustomTxIcon from 'src/routes/safe/components/Transactions/TxList/assets/custom.svg'
import CircleCrossRed from 'src/routes/safe/components/Transactions/TxList/assets/circle-cross-red.svg'
import IncomingTxIcon from 'src/routes/safe/components/Transactions/TxList/assets/incoming.svg'
import OutgoingTxIcon from 'src/routes/safe/components/Transactions/TxList/assets/outgoing.svg'
import SettingsTxIcon from 'src/routes/safe/components/Transactions/TxList/assets/settings.svg'
import { isCancelTransaction } from 'src/routes/safe/components/Transactions/TxList/utils'

export type TxTypeProps = {
icon: string
Expand Down Expand Up @@ -40,20 +40,8 @@ export const useTransactionType = (tx: Transaction): TxTypeProps => {
break
}

// TODO: isCancel
// there are two 'cancelling' tx identification
// this one is the candidate to remain when the client gateway implements
// https://github.com/gnosis/safe-client-gateway/issues/255
if (typeof tx.txInfo.isCancellation === 'boolean' && tx.txInfo.isCancellation) {
setType({ icon: CustomTxIcon, text: 'Cancelling transaction' })
break
}

// TODO: isCancel
// remove the following condition when issue#255 is implemented
// also remove `isCancelTransaction` function
if (isCancelTransaction({ txInfo: tx.txInfo, safeAddress })) {
setType({ icon: CustomTxIcon, text: 'Cancelling transaction' })
if (tx.txInfo.isCancellation) {
setType({ icon: CircleCrossRed, text: 'On-chain rejection' })
break
}

Expand Down
Loading