From c96e3192ff4b36715298e7b6a2d66e3fe1f3d6c1 Mon Sep 17 00:00:00 2001 From: Fernando Date: Wed, 7 Apr 2021 07:40:33 -0300 Subject: [PATCH] (Feature) Proper rejections (#2096) * remove `isCancelTransaction` utility function in favor of `txInfo.isCancellation` flag provided by client-gateway * replace "cancel" concept in favor of "reject" * add circle-cross-red icon to "On-chain rejection" transaction info Adjust owner's list text color * identify queued on-chain rejection * apply styles to on-chain rejection type identifier * update awaiting messages wording * fix styles on styles to on-chain rejection * replace local svg with SRC `Icon` component wherever is possible --- package.json | 2 +- src/components/CustomIconText/index.tsx | 6 +- src/components/TransactionFailText/index.tsx | 2 +- .../safe/store/models/types/gateway.d.ts | 2 +- .../Transactions/TxList/TxCollapsed.tsx | 7 ++- .../TxList/TxCollapsedActions.tsx | 2 +- .../Transactions/TxList/TxDetails.tsx | 58 +++++++++++-------- .../Transactions/TxList/TxExpandedActions.tsx | 2 +- .../Transactions/TxList/TxOwners.tsx | 56 +++++++++++------- .../Transactions/TxList/TxSummary.tsx | 2 +- .../TxList/assets/circle-cross-red.svg | 11 ++++ .../TxList/hooks/useTransactionActions.ts | 10 +--- .../TxList/hooks/useTransactionStatus.ts | 4 +- .../TxList/hooks/useTransactionType.ts | 18 +----- .../components/Transactions/TxList/styled.tsx | 30 ++++++++++ .../components/Transactions/TxList/utils.ts | 24 +------- yarn.lock | 4 +- 17 files changed, 134 insertions(+), 106 deletions(-) create mode 100644 src/routes/safe/components/Transactions/TxList/assets/circle-cross-red.svg diff --git a/package.json b/package.json index c8f82367..82ed3cfe 100644 --- a/package.json +++ b/package.json @@ -161,7 +161,7 @@ "@gnosis.pm/safe-apps-sdk": "1.0.3", "@gnosis.pm/safe-apps-sdk-v1": "npm:@gnosis.pm/safe-apps-sdk@0.4.2", "@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", diff --git a/src/components/CustomIconText/index.tsx b/src/components/CustomIconText/index.tsx index 31fc13ba..cc059de1 100644 --- a/src/components/CustomIconText/index.tsx +++ b/src/components/CustomIconText/index.tsx @@ -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` @@ -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 => ( {text && {text}} ) - -export default CustomIconText diff --git a/src/components/TransactionFailText/index.tsx b/src/components/TransactionFailText/index.tsx index d61664cc..25c6b56b 100644 --- a/src/components/TransactionFailText/index.tsx +++ b/src/components/TransactionFailText/index.tsx @@ -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` : `To save gas costs, avoid executing the transaction.` } diff --git a/src/logic/safe/store/models/types/gateway.d.ts b/src/logic/safe/store/models/types/gateway.d.ts index c462971a..cf45553d 100644 --- a/src/logic/safe/store/models/types/gateway.d.ts +++ b/src/logic/safe/store/models/types/gateway.d.ts @@ -236,7 +236,7 @@ type MultiSigExecutionDetails = { type DetailedExecutionInfo = ModuleExecutionDetails | MultiSigExecutionDetails type ExpandedTxDetails = { - executedAt: number + executedAt: number | null txStatus: TransactionStatus txInfo: TransactionInfo txData: TransactionData | null diff --git a/src/routes/safe/components/Transactions/TxList/TxCollapsed.tsx b/src/routes/safe/components/Transactions/TxList/TxCollapsed.tsx index 4223be90..4e12f101 100644 --- a/src/routes/safe/components/Transactions/TxList/TxCollapsed.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxCollapsed.tsx @@ -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, @@ -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)) { @@ -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 = (
@@ -124,7 +127,7 @@ export const TxCollapsed = ({ ) const txCollapsedType = ( -
+
) diff --git a/src/routes/safe/components/Transactions/TxList/TxCollapsedActions.tsx b/src/routes/safe/components/Transactions/TxList/TxCollapsedActions.tsx index 1e753ecd..4751cc37 100644 --- a/src/routes/safe/components/Transactions/TxList/TxCollapsedActions.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxCollapsedActions.tsx @@ -46,7 +46,7 @@ export const TxCollapsedActions = ({ transaction }: TxCollapsedActionsProps): Re {canCancel && ( - + diff --git a/src/routes/safe/components/Transactions/TxList/TxDetails.tsx b/src/routes/safe/components/Transactions/TxList/TxDetails.tsx index 789afd82..675f59c1 100644 --- a/src/routes/safe/components/Transactions/TxList/TxDetails.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxDetails.tsx @@ -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 { @@ -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' @@ -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 } - 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 ( <> - - {`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 - }.`} - - - - - Why do I need to pay for cancelling a transaction? - - - - + {message} + {!isTxExecuted && ( + <> +
+ + + + Why do I need to pay for rejecting a transaction? + + + + + + )} ) } @@ -116,7 +124,7 @@ export const TxDetails = ({ transaction, actions }: TxDetailsProps): ReactElemen 'will-be-replaced': transaction.txStatus === 'WILL_BE_REPLACED', })} > - +
{!data.executedAt && txLocation !== 'history' && actions?.isUserAnOwner && (
diff --git a/src/routes/safe/components/Transactions/TxList/TxExpandedActions.tsx b/src/routes/safe/components/Transactions/TxList/TxExpandedActions.tsx index 71e0c28c..81f7d804 100644 --- a/src/routes/safe/components/Transactions/TxList/TxExpandedActions.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxExpandedActions.tsx @@ -41,7 +41,7 @@ export const TxExpandedActions = ({ transaction }: TxExpandedActionsProps): Reac {canCancel && ( )} diff --git a/src/routes/safe/components/Transactions/TxList/TxOwners.tsx b/src/routes/safe/components/Transactions/TxList/TxOwners.tsx index 6b97e081..22b45998 100644 --- a/src/routes/safe/components/Transactions/TxList/TxOwners.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxOwners.tsx @@ -1,4 +1,4 @@ -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' @@ -6,43 +6,55 @@ 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) ? ( + + + + +
+ + On-chain rejection created + +
+ + ) : ( + + + + +
+ + Created + +
+ + ) + return ( - - - - -
- - Created - -
- + {CreationNode} {detailedExecutionInfo.confirmations.map(({ signer }) => ( - +
@@ -55,7 +67,11 @@ export const TxOwners = ({ detailedExecutionInfo }: TxOwnersProps): ReactElement {confirmationsNeeded <= 0 ? ( - + {detailedExecutionInfo.executor ? ( + + ) : ( + + )}
diff --git a/src/routes/safe/components/Transactions/TxList/TxSummary.tsx b/src/routes/safe/components/Transactions/TxList/TxSummary.tsx index 84fb6cc7..4bc50b77 100644 --- a/src/routes/safe/components/Transactions/TxList/TxSummary.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxSummary.tsx @@ -27,7 +27,7 @@ export const TxSummary = ({ txDetails }: { txDetails: ExpandedTxDetails }): Reac )}
- {nonce && ( + {nonce !== undefined && (
Nonce:{' '} diff --git a/src/routes/safe/components/Transactions/TxList/assets/circle-cross-red.svg b/src/routes/safe/components/Transactions/TxList/assets/circle-cross-red.svg new file mode 100644 index 00000000..ff8f6306 --- /dev/null +++ b/src/routes/safe/components/Transactions/TxList/assets/circle-cross-red.svg @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionActions.ts b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionActions.ts index 5e3b7566..3609d6bc 100644 --- a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionActions.ts +++ b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionActions.ts @@ -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' @@ -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, }) diff --git a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionStatus.ts b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionStatus.ts index e64637b7..4df71af3 100644 --- a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionStatus.ts +++ b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionStatus.ts @@ -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': diff --git a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionType.ts b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionType.ts index ca04efbc..e33b81b7 100644 --- a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionType.ts +++ b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionType.ts @@ -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 @@ -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 } diff --git a/src/routes/safe/components/Transactions/TxList/styled.tsx b/src/routes/safe/components/Transactions/TxList/styled.tsx index de8d7f19..9058bc80 100644 --- a/src/routes/safe/components/Transactions/TxList/styled.tsx +++ b/src/routes/safe/components/Transactions/TxList/styled.tsx @@ -163,6 +163,32 @@ const failedTransaction = css` } ` +const onChainRejection = css` + &.on-chain-rejection { + background-color: ${({ theme }) => theme.colors.errorTooltip}; + border-left: 4px solid ${({ theme }) => theme.colors.error}; + border-radius: 4px; + padding-left: 7px; + height: 22px; + max-width: 165px; + + > div { + height: 17px; + align-items: center; + padding-top: 3px; + } + + p { + font-size: 11px; + line-height: 16px; + letter-spacing: 1px; + font-weight: bold; + text-transform: uppercase; + margin-left: -2px; + } + } +` + export const StyledTransaction = styled.div` ${willBeReplaced}; ${failedTransaction}; @@ -175,6 +201,10 @@ export const StyledTransaction = styled.div` align-self: center; } + .tx-type { + ${onChainRejection}; + } + .tx-votes { justify-self: center; } diff --git a/src/routes/safe/components/Transactions/TxList/utils.ts b/src/routes/safe/components/Transactions/TxList/utils.ts index d461e8cc..13b15057 100644 --- a/src/routes/safe/components/Transactions/TxList/utils.ts +++ b/src/routes/safe/components/Transactions/TxList/utils.ts @@ -2,7 +2,6 @@ import { BigNumber } from 'bignumber.js' import { getNetworkInfo } from 'src/config' import { - Custom, isCustomTxInfo, isTransferTxInfo, Transaction, @@ -12,7 +11,6 @@ import { import { formatAmount } from 'src/logic/tokens/utils/formatAmount' import { sameAddress } from 'src/logic/wallets/ethAddresses' -import { sameString } from 'src/utils/strings' export const NOT_AVAILABLE = 'n/a' @@ -90,27 +88,11 @@ export const getTxTokenData = (txInfo: Transfer): txTokenData => { } } -// TODO: isCancel -// how can we be sure that it's a cancel tx without asking for tx-details? -// can the client-gateway service provide info about the tx, Like: `isCancelTransaction: boolean`? -// it will be solved as part of: https://github.com/gnosis/safe-client-gateway/issues/255 -export const isCancelTransaction = ({ txInfo, safeAddress }: { txInfo: Custom; safeAddress: string }): boolean => - sameAddress(txInfo.to, safeAddress) && - sameString(txInfo.dataSize, '0') && - sameString(txInfo.value, '0') && - txInfo.methodName === null - -type IsCancelTxDetailsProps = { - executedAt: number | null - txInfo: Transaction['txInfo'] - safeAddress: string -} -export const isCancelTxDetails = ({ executedAt, txInfo, safeAddress }: IsCancelTxDetailsProps): boolean => - !executedAt && +export const isCancelTxDetails = (txInfo: Transaction['txInfo']): boolean => // custom transaction isCustomTxInfo(txInfo) && - // verify that it's a cancel tx based on it's info - isCancelTransaction({ safeAddress, txInfo }) + // flag-based identification + txInfo.isCancellation export const addressInList = (list: string[] = []) => (address: string): boolean => list.some((ownerAddress) => sameAddress(ownerAddress, address)) diff --git a/yarn.lock b/yarn.lock index 56873e07..b65e2fb0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1596,9 +1596,9 @@ solc "0.5.14" truffle "^5.1.21" -"@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": version "0.5.0" - resolved "https://github.com/gnosis/safe-react-components.git#a68a67e634d0be091856ebba9f6874eebb767cd7" + resolved "https://github.com/gnosis/safe-react-components.git#7ebc414ae975d60846704c5a8db5e61c30348d12" dependencies: classnames "^2.2.6" react-media "^1.10.0"