-
Notifications
You must be signed in to change notification settings - Fork 5k
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
fix: parse tx logs on contractInteraction to refresh NFT state #25380
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
@@ -145,8 +145,6 @@ describe('Send NFT', function () { | |||
// Go back to NFTs tab and check the imported NFT is shown as previously owned | |||
await driver.clickElement('[data-testid="account-overview__nfts-tab"]'); | |||
|
|||
await driver.clickElement('[data-testid="refresh-list-button"]'); | |||
|
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.
No need to click on refresh button because this Tx is safetransferfrom and ownership has been already refreshed in MM controller
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #25380 +/- ##
===========================================
- Coverage 65.19% 65.12% -0.07%
===========================================
Files 1405 1405
Lines 55571 55636 +65
Branches 14592 14636 +44
===========================================
+ Hits 36229 36231 +2
- Misses 19342 19405 +63 ☔ View full report in Codecov by Sentry. |
Builds ready [dd12fd3]
Page Load Metrics (118 ± 139 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [88f1388]
Page Load Metrics (56 ± 6 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Description
On this PR, we aim to refresh NFT ownership status or add NFTs to state if necessary by parsing transaction logs once the user submits a transaction with MM.
MM controller already had the logic that calls
_updateNFTOwnership
after creating the transaction notification.That logic refreshed NFT ownerhsip when transaction type is
transferfrom
.We are adding the case when transaction type is a contract interaction and then look for specific topics.
Related issues
Fixes:
Manual testing steps
Im using Ethereum mainnet in the videos because we support NFT detection on Ethereum mainnet.
But this should also work if you are submitting transaction on Sepolia or any other test network.
Screenshots/Recordings
Before
Screen.Recording.2024-06-20.at.10.02.42.mov
After
Screen.Recording.2024-06-20.at.10.37.15.mov
Pre-merge author checklist
Pre-merge reviewer checklist