[UserTransactionOverviewTab] Parsing 0x1 FA/Coin transfer function #1184
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements parsing of 0x1 FA/Coin transfer functions to display transfer actions in the user transaction overview tab. Since no events are emitted for transfer actions, the solution parses function arguments directly from successful transactions.
- Adds support for parsing 5 different transfer functions from the 0x1 module
- Introduces new function-based action parsing alongside existing event-based parsing
- Displays transfer actions with sender, recipient, asset, and amount information
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| asset: | ||
| payload.arguments[0] && | ||
| typeof payload.arguments[0] === "object" && | ||
| "inner" in payload.arguments[0] | ||
| ? payload.arguments[0].inner | ||
| : undefined, |
There was a problem hiding this comment.
The asset value could be undefined, but the AssetTransfer type expects asset to be a string. This will cause type mismatch errors when rendering.
| asset: | ||
| payload.arguments[0] && | ||
| typeof payload.arguments[0] === "object" && | ||
| "inner" in payload.arguments[0] | ||
| ? payload.arguments[0].inner | ||
| : undefined, |
There was a problem hiding this comment.
Same issue as above - asset could be undefined but AssetTransfer type expects string.
| asset: | ||
| Array.isArray(payload.type_arguments) && | ||
| payload.type_arguments.length > 0 | ||
| ? payload.type_arguments.join("::") | ||
| : "unknown", |
There was a problem hiding this comment.
Inconsistent fallback values: 'unknown' is used here while undefined is used in the FA cases above. Consider using consistent fallback handling across all asset parsing.
| asset: | ||
| Array.isArray(payload.type_arguments) && | ||
| payload.type_arguments.length > 0 | ||
| ? payload.type_arguments.join("::") | ||
| : "unknown", |
There was a problem hiding this comment.
Same inconsistent fallback issue as Comment 3.
| for (let i = 0; i < payload.arguments[1].length; i++) { | ||
| actions.push({ | ||
| actionType: "asset transfer", | ||
| from: sender, | ||
| to: | ||
| payload.arguments[1][i] && | ||
| typeof payload.arguments[1][i] === "string" | ||
| ? payload.arguments[1][i] | ||
| : "unknown", | ||
| asset: | ||
| payload.arguments[0] && | ||
| typeof payload.arguments[0] === "object" && | ||
| "inner" in payload.arguments[0] | ||
| ? payload.arguments[0].inner | ||
| : undefined, | ||
| amount: Number(payload.arguments[2][i]), | ||
| }); |
There was a problem hiding this comment.
No validation that payload.arguments[1] exists or is an array before accessing .length property. This could cause runtime errors.
| for (let i = 0; i < payload.arguments[1].length; i++) { | |
| actions.push({ | |
| actionType: "asset transfer", | |
| from: sender, | |
| to: | |
| payload.arguments[1][i] && | |
| typeof payload.arguments[1][i] === "string" | |
| ? payload.arguments[1][i] | |
| : "unknown", | |
| asset: | |
| payload.arguments[0] && | |
| typeof payload.arguments[0] === "object" && | |
| "inner" in payload.arguments[0] | |
| ? payload.arguments[0].inner | |
| : undefined, | |
| amount: Number(payload.arguments[2][i]), | |
| }); | |
| if (Array.isArray(payload.arguments[1])) { | |
| for (let i = 0; i < payload.arguments[1].length; i++) { | |
| actions.push({ | |
| actionType: "asset transfer", | |
| from: sender, | |
| to: | |
| payload.arguments[1][i] && | |
| typeof payload.arguments[1][i] === "string" | |
| ? payload.arguments[1][i] | |
| : "unknown", | |
| asset: | |
| payload.arguments[0] && | |
| typeof payload.arguments[0] === "object" && | |
| "inner" in payload.arguments[0] | |
| ? payload.arguments[0].inner | |
| : undefined, | |
| amount: Number(payload.arguments[2][i]), | |
| }); | |
| } |
| Array.isArray(payload.arguments[1]) && | ||
| Array.isArray(payload.arguments[0]) | ||
| ) { | ||
| for (let i = 0; i < payload.arguments[1].length; i++) { |
There was a problem hiding this comment.
Same issue as Comment 5 - accessing .length without validation that payload.arguments[1] exists.
Description
solves #1157
reorganised to remove unnecessary rebased commit
As no event will be emitted for transfer action, it will parse the arguments for the following function if the status is success:
This is a interim solution right now. Only work for using those entry function directly. Maybe the 0x1 library should consider emitting transfer event.
Related Links
For verifying - view Binance's addresses
https://explorer.aptoslabs.com/account/0xae1a6f3d3daccaf77b55044cea133379934bba04a11b9d0bbd643eae5e6e9c70?network=mainnet
Checklist