fix(multichain-account-service): prevent creating EVM account during discovery #6650
Conversation
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
| groupIndex, | ||
| }); | ||
| assert( | ||
| addressFromGroupIndex === address, |
There was a problem hiding this comment.
Can you remind me if the addresses are all lowercase or are they normalized?
There was a problem hiding this comment.
We usually use lowercase for EVM yes. But I'm pretty sure it's bit of a mix today... I just know that most people re-sanitize their addresses before using them.
JFYI, I just re-used the same code than here:
|
@metamaskbot publish-preview |
|
Hmm, my PR removes the |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
Well indeed, this will fix it for the service (with your PR), but the My initial idea was to "consume" the wallets/groups from the service in the This was working ok, but since (in my PR), the service still relies on So I guess, combining #6646 and your future PR could have solved that issue. Nevertheless, I still think that temporary creating the account is not the right pattern here, we just want to "peek" the next address and check for activity and THEN create the account if we have to. So all in all, we might want to combine all 3 PRs 😄
|
Ok that makes sense, no issue on my end then with this PR 🙂 |
Co-authored-by: Daniel Rocha <68558152+danroc@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Explanation
Derive next address instead of creating the account entirely (which triggers
:accountAddedand adds side-effects for this temporary account).References
N/A
Checklist