-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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: token approvals from nested transactions in simulation details #30511
base: feat/atomic-batch-transactions
Are you sure you want to change the base?
feat: token approvals from nested transactions in simulation details #30511
Conversation
Temporarily add mock simulation data. Add useBatchApproveTokenSimulation hook. Add BatchSimulationDetails component.
Remove batch spending cap.
const { args, name } = transactionDescription ?? {}; | ||
|
||
if ( | ||
!['approve', 'increaseAllowance', 'setApprovalForAll'].includes(name ?? '') |
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.
Perhaps we can compare token method names by leveraging the TransactionType
s like we do in determineTransactionType
, instead of hardcoding the strings?
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.
We technically could, but it's coincidence that the types match the names as some of the transaction types for example don't align with any contract methods.
Though I'm not opposed to using a central enum, assuming one exists or we add one.
@@ -48,7 +67,25 @@ export const BalanceChangeRow: React.FC<{ | |||
style={{ minWidth: 0 }} | |||
> | |||
<Box display={Display.Flex} flexDirection={FlexDirection.Row} gap={1}> | |||
<AmountPill asset={asset} amount={amount} /> | |||
{onEdit && ( |
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.
This is a great addition, and directly addresses a pain point recently reported internally. 🔥 See https://consensys.slack.com/archives/C03ETQA9EPK/p1739549199067819
Description
Decode approvals from nested transaction data and include them in the simulation details.
Specifically:
BatchSimulationDetails
component to display simulation details including approvals.useBatchApproveBalanceChanges
hook to generate balance changes for approvals.staticRows
property toSimulationDetails
to display additional static data.parseApprovalTransactionData
function to shared utils to centralise approval data parsing.buildXTransactionData
helper functions to avoid hardcoding transaction data strings.isApproval
,isAllApproval
, andisUnlimitedApproval
toBalanceChange
type.AmountPill
to support approval styles and handle unlimited and all.BalanceChangeRow
to include edit icon ifonEdit
provided in balance change.EditSpendingCapModal
to support optionaldata
andto
properties.Note that edit approval will not submit as it is pending an additional ticket to connect to a new method from the
TransactionController
.Related issues
Fixes #4283
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist