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

Feat: decode native transfers + show Multisend in the tx info block #1899

Merged
merged 6 commits into from
Apr 26, 2023

Conversation

katspaugh
Copy link
Member

What it solves

Enhances the "Transaction details" block in SignOrExecuteForm:

  • Show decoded native token transfers (previously we didn't show them at all)
  • Show multisend actions (previously it showed only the raw data)

How this PR fixes it

I've removed from custom Review modals, and it's now shown in Transaction details instead.

How to test it

Test native token transfers, NFT batches, Safe App batches.

Screenshots

Screenshot 2023-04-24 at 12 16 03

Screenshot 2023-04-24 at 12 14 55

Screenshot 2023-04-24 at 12 13 20

@katspaugh katspaugh requested a review from iamacook April 24, 2023 09:19
@github-actions
Copy link

github-actions bot commented Apr 24, 2023

Branch preview

✅ Deploy successful!

https://decode_multisend--webcore.review-web-core.5afe.dev

@github-actions
Copy link

github-actions bot commented Apr 24, 2023

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@francovenica
Copy link
Contributor

francovenica commented Apr 25, 2023

Question:
The Expand All / Collapse all was removed on purpose? This snapshot is a NFT batch tx in prod, and is missing in this PR for the same type of tx
image

@francovenica
Copy link
Contributor

francovenica commented Apr 25, 2023

Batch tx of send funds are not showing "Transaction details" for any of it or like multi send. Is that expected as well?
As a Batch
image

As a single tx
image

@katspaugh
Copy link
Member Author

katspaugh commented Apr 26, 2023

We show Expand/Collapse only in the details in the queue/history for space reasons.

Regarding the batch modal, it uses a different component with its own logic, so it's normal. We should eventually refactor it to use the regular SignOrExecute component, I'll make a tech debt issue for that.

Update: issue: #1909

@francovenica
Copy link
Contributor

So is correct that the expand/collapse all was removed and the batch tx modal will be tackled in another ticket.

LGTM then.

@katspaugh katspaugh merged commit 3d3652c into dev Apr 26, 2023
@katspaugh katspaugh deleted the decode-multisend branch April 26, 2023 12:46
@github-actions github-actions bot locked and limited conversation to collaborators Apr 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants