Skip to content
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

Implement personal_ecRecover - signing to unlock L2 Looping account fails #19621

Closed
markrtgh opened this issue Nov 20, 2021 · 9 comments · Fixed by brave/brave-core#11590
Closed
Assignees
Labels
feature/web3/wallet Integrating Ethereum+ wallet support OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA/No release-notes/include

Comments

@markrtgh
Copy link

Description

Unlocking an ethereum account on L2 loopring requires a signature. Signing fails using Brave Wallet but succeeds using Crypto Wallets (depreciated).

Steps to Reproduce

  1. Use an ethereum account previously used to connect to L2 Loopring https://loopring.io/#/layer2
  2. Connect account by following prompts and selecting Metamask. Connection successful.
  3. Attempt to unlock account. Window pops up requesting signature. Press sign button.

Actual result:

After pressing sign button Result is "Unlock failed"

Expected result:

Successful signing and account unlocking which occurs when using Crypto Wallets (depreciated)

Reproduces how often:

Easily reproduced

Brave version (brave://version info)

Brave 1.32.106 Chromium: 96.0.4664.45 (Official Build) (arm64)
Revision 76e4c1bb2ab4671b8beba3444e61c0f17584b2fc-refs/branch-heads/4664@{#947}
OS macOS Version 12.0.1 (Build 21A559)

Version/Channel Information:

  • Can you reproduce this issue with the current release? Yes
  • Can you reproduce this issue with the beta channel? Not tested
  • Can you reproduce this issue with the nightly channel? Not tested

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? N/A
  • Does the issue resolve itself when disabling Brave Rewards? N/A
  • Is the issue reproducible on the latest version of Chrome? N/A

Miscellaneous Information:

Signature window for Brave wallet displays the same information as the Crypto Wallets (depreciated) window popup but signing fails for the Brave wallet but succeeds with Crypto Wallets (depreciated).

@bbondy
Copy link
Member

bbondy commented Nov 26, 2021

Any errors in the console? I wonder if it's trying to use sign typed data which will be supported in 1.33.x

@bbondy bbondy added priority/P2 A bad problem. We might uplift this to the next planned release. feature/web3/wallet Integrating Ethereum+ wallet support QA/Yes release-notes/include labels Nov 26, 2021
@Fillicia
Copy link

Fillicia commented Dec 6, 2021

Only error I can see in the console is that it throws

[Violation] 'setTimeout' handler took 55ms main~402f6fec.289c83e0.chunk.js:2

as soon as you click unlock, then nothing when you sign the message other than failing site-wise

@darkdh
Copy link
Member

darkdh commented Dec 7, 2021

can anyone show me how to trigger the sign message request? I didn't see "unlock" button on the website after connected.

@darkdh darkdh added the needs-more-info The report requires more detail before we can decide what to do with this issue. label Dec 7, 2021
@bbondy
Copy link
Member

bbondy commented Dec 7, 2021

I also tried, I was able to do steps 1 and 2, but I'm not sure what to do in step 3. It seems like a step is missing between 2 and 3.

@Fillicia
Copy link

Fillicia commented Dec 9, 2021

Sorry for the delay, it's a 2 step process for Loopring. You have to connect your wallet and then, on the "L2 Wallet" tabs, unlock it if you want to manage your assets. The unlock part is only if you have an active loopring account.

Screenshot from 2021-12-09 08-46-33
Screenshot from 2021-12-09 08-46-47
Screenshot from 2021-12-09 08-46-52

@bbondy bbondy changed the title Brave wallet: signing to unlock L2 Looping account fails Implement personal_ecRecover - signing to unlock L2 Looping account fails Dec 9, 2021
@bbondy
Copy link
Member

bbondy commented Dec 9, 2021

Looks like we need support for personal_ecRecover which I think is used to verify a signature from personal_sign.

@bbondy
Copy link
Member

bbondy commented Dec 9, 2021

@srirambv
Copy link
Contributor

srirambv commented Feb 1, 2022

@bbondy confirmed that this was verified. Marking the issue as QA/No

@srirambv srirambv added QA/No and removed QA/Yes needs-more-info The report requires more detail before we can decide what to do with this issue. labels Feb 1, 2022
@bbondy
Copy link
Member

bbondy commented Feb 3, 2022

This is live on today's release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/web3/wallet Integrating Ethereum+ wallet support OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA/No release-notes/include
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants