E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/15387c84-0c0e-4bf5-aaa7-5ce939e16650
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

I believe yes, but I just want to check with you!
When this error is thrown we are sure that it will be caught by the error boundary component?

I think we'll need to patch the keyring controller as well, since it calls syncIdentities with an empty array. That would result in this error being thrown.

Thanks @Gudahtt!

I think it's safe to just comment out the line, I'll run some test on my end to verify it. Let me know what you think

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (23dc52b) 41.06% compared to head (c6cd716) 41.04%.

@@            Coverage Diff             @@
##             main    #8414      +/-   ##
==========================================
- Coverage   41.06%   41.04%   -0.02%     
==========================================
  Files        1247     1247              
  Lines       30351    30363      +12     
  Branches     2963     2964       +1     
==========================================
  Hits        12464    12464              
- Misses      17142    17154      +12     
  Partials      745      745              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tommasini I believe that too but I have not checked it yet. I'm still researching the issue and have not tested the current code of this PR.

I think it's safe to just comment out the line, I'll run some test on my end to verify it. Let me know what you think

I think you're right, I'll try and verify this as best I can later though

@gantunesr I've taken another look, and I definitely think that updateIdentities([]) call is not useful and safe to comment out.

Could you update this PR again when you get the chance, and I can review it again?

Sure @Gudahtt, I just now returned to work on this code, but why is that? The next 2 lines updates the identities array to the accounts fetched from eth-keyring-controller

That method is replacing the existing vault with a new one, so it is supposed to replace all identities as well, as part of that process.

Not sure if that answers the question; could you clarify which part seems confusing if not?

Right, I was under the impression that it would replace a repeated identity with a new one, but in reality it keeps the old one so it's right to clean identities before providing a new array of addresses

@Gudahtt I decided to revert the patches because I was unsure of the impact. The preferences-controller patch would cause a crash under certain edge cases and I want to take a deeper look at the keyring-controller method. I have just kept the logs to monitor them in Sentry

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud