-
-
Notifications
You must be signed in to change notification settings - Fork 205
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
bugfix/collectible detection by acccount #487
Conversation
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.
one small thing, but otherwise looks good to me.
@@ -355,9 +355,10 @@ export class AssetsDetectionController extends BaseController< | |||
if (!this.isMainnet()) { | |||
return; | |||
} | |||
const { selectedAddress } = this.config; | |||
const requestedSelectedAddress = this.config.selectedAddress; |
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.
any particular reason for the rename? seems selectedAddress
would still be fine no?
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.
hm not really, just to be clear that it was the address from where it was being called
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.
I can go back to selectedAddress if you want 👌
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.
nah it's fine.
just to be clear that it was the address from where it was being called
I think that's plenty reason enough to rename. feel free to merge.
When auto detecting collectibles there are some cases where some collectibles were detected in the wrong account. One way I could reproduce that was in the following case
0x9
and we're waiting for the response from API https://github.com/MetaMask/controllers/compare/bugfix/assets-detection-by-acccount?expand=1#diff-3202c2273084eb58dae3e0d4e23765dce262bb24c93eb3bf166c56d2d7bf02c8R3650x12
0x9
but they are added to0x12
, so we have the wrong assets into the account.I could repro that with tests, removing this check https://github.com/MetaMask/controllers/compare/bugfix/assets-detection-by-acccount?expand=1#diff-3202c2273084eb58dae3e0d4e23765dce262bb24c93eb3bf166c56d2d7bf02c8R401