-
Notifications
You must be signed in to change notification settings - Fork 454
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
feat: Add batch execute button and tx highlighting #352
Changes from 4 commits
2cc0fac
2352b55
3541a7f
3a82c49
5fefecf
4b33419
8b14be1
eb71155
6a47cfe
e12dcbf
d318c75
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import { styled } from '@mui/material/styles' | ||
import Tooltip, { tooltipClasses, TooltipProps } from '@mui/material/Tooltip' | ||
|
||
const CustomTooltip = styled(({ className, ...props }: TooltipProps) => ( | ||
<Tooltip {...props} classes={{ popper: className }} /> | ||
))(({ theme }) => ({ | ||
[`& .${tooltipClasses.tooltip}`]: { | ||
color: theme.palette.common.black, | ||
backgroundColor: theme.palette.common.white, | ||
borderRadius: '8px', | ||
boxShadow: '1px 2px 10px rgba(40, 54, 61, 0.18)', | ||
fontSize: '14px', | ||
padding: '16px', | ||
lineHeight: 'normal', | ||
}, | ||
[`& .${tooltipClasses.arrow}`]: { | ||
'&::before': { | ||
backgroundColor: theme.palette.common.white, | ||
boxShadow: '1px 2px 10px rgba(40, 54, 61, 0.18)', | ||
}, | ||
}, | ||
})) | ||
|
||
export default CustomTooltip |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import { createContext, ReactElement, ReactNode, useState } from 'react' | ||
|
||
export const BatchExecuteHoverContext = createContext<{ | ||
activeHover?: string[] | ||
setActiveHover: (activeHover?: string[]) => void | ||
}>({ | ||
activeHover: undefined, | ||
setActiveHover: () => {}, | ||
}) | ||
|
||
// Used for highlighting transactions that will be included when executing them as a batch | ||
export const BatchExecuteHoverProvider = ({ children }: { children: ReactNode }): ReactElement => { | ||
const [activeHover, setActiveHover] = useState<string[]>() | ||
|
||
return ( | ||
<BatchExecuteHoverContext.Provider value={{ activeHover, setActiveHover }}> | ||
{children} | ||
</BatchExecuteHoverContext.Provider> | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
import { useCallback, useContext, useMemo } from 'react' | ||
import { Button } from '@mui/material' | ||
import css from './styles.module.css' | ||
import { BatchExecuteHoverContext } from '@/components/transactions/BatchExecuteButton/BatchExecuteHoverProvider' | ||
import { Transaction, TransactionListItem } from '@gnosis.pm/safe-react-gateway-sdk' | ||
import useSafeInfo from '@/hooks/useSafeInfo' | ||
import { isMultisigExecutionInfo, isTransactionListItem } from '@/utils/transaction-guards' | ||
import { useAppSelector } from '@/store' | ||
import { selectPendingTxs } from '@/store/pendingTxsSlice' | ||
import CustomTooltip from '@/components/common/CustomTooltip' | ||
|
||
const BATCH_LIMIT = 10 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know it was 10 txs before. But I'm not 100% sure where the 10 is coming from. Is it about the block gas limit? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I remember we chose 10 arbitrarily. By default we get 20 per page now I think so we could increase it to that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually now infinite scroll makes more sense. 🤔 |
||
|
||
const getBatchableTransactions = (items: Transaction[][], nonce: number) => { | ||
const batchableTransactions: Transaction[] = [] | ||
let currentNonce = nonce | ||
|
||
items.forEach((txs) => { | ||
const sorted = txs.slice().sort((a, b) => b.transaction.timestamp - a.transaction.timestamp) | ||
sorted.forEach((tx) => { | ||
if ( | ||
batchableTransactions.length < BATCH_LIMIT && | ||
isMultisigExecutionInfo(tx.transaction.executionInfo) && | ||
tx.transaction.executionInfo.nonce === currentNonce && | ||
tx.transaction.executionInfo.confirmationsSubmitted >= tx.transaction.executionInfo.confirmationsRequired | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know its just a minor bug but lets add a condition here that we stop adding txs to the batch if an update safe tx was added. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed lets tackle this in a separate task as it involves decoding each tx which could blow up the scope |
||
) { | ||
batchableTransactions.push(tx) | ||
currentNonce = tx.transaction.executionInfo.nonce + 1 | ||
} | ||
iamacook marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}) | ||
}) | ||
|
||
return batchableTransactions | ||
} | ||
|
||
const BatchExecuteButton = ({ items }: { items: (TransactionListItem | Transaction[])[] }) => { | ||
const pendingTxs = useAppSelector(selectPendingTxs) | ||
const hoverContext = useContext(BatchExecuteHoverContext) | ||
const { safe } = useSafeInfo() | ||
|
||
const currentNonce = safe.nonce | ||
|
||
const groupedTransactions = useMemo( | ||
() => | ||
items | ||
.map((item) => { | ||
if (Array.isArray(item)) return item | ||
if (isTransactionListItem(item)) return [item] | ||
}) | ||
.filter((item) => item !== undefined) as Transaction[][], | ||
[items], | ||
) | ||
|
||
const batchableTransactions = useMemo( | ||
() => getBatchableTransactions(groupedTransactions, currentNonce), | ||
[currentNonce, groupedTransactions], | ||
) | ||
|
||
const isBatchable = batchableTransactions.length > 1 | ||
const hasPendingTx = batchableTransactions.some((tx) => pendingTxs[tx.transaction.id]) | ||
const isDisabled = !isBatchable || hasPendingTx | ||
|
||
const handleOnMouseEnter = useCallback(() => { | ||
hoverContext.setActiveHover(batchableTransactions.map((tx) => tx.transaction.id)) | ||
}, [batchableTransactions, hoverContext]) | ||
|
||
const handleOnMouseLeave = useCallback(() => { | ||
hoverContext.setActiveHover() | ||
}, [hoverContext]) | ||
|
||
return ( | ||
<CustomTooltip | ||
placement="top-start" | ||
arrow | ||
title={ | ||
isDisabled | ||
? 'Batch execution is only available for transactions that have been fully signed and are strictly sequential in Safe Nonce.' | ||
iamacook marked this conversation as resolved.
Show resolved
Hide resolved
|
||
: 'All transactions highlighted in light green will be included in the batch execution.' | ||
} | ||
> | ||
<Button | ||
onMouseEnter={handleOnMouseEnter} | ||
onMouseLeave={handleOnMouseLeave} | ||
className={css.button} | ||
variant="contained" | ||
size="small" | ||
disabled={isDisabled} | ||
> | ||
Execute Batch {isBatchable && ` (${batchableTransactions.length})`} | ||
</Button> | ||
</CustomTooltip> | ||
) | ||
} | ||
|
||
export default BatchExecuteButton |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
.button { | ||
position: absolute; | ||
right: 0; | ||
top: -50px; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
.active { | ||
border-color: var(--color-primary-light); | ||
} | ||
|
||
.active :global .MuiAccordionSummary-root { | ||
background-color: var(--color-primary-background); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only extracted this so it can be reused