bug: signing fails in brave

[mmv08]

What it solves

May solve #3388, but I couldn't reproduce the issue
Fixes error thrown while signing in brave (was reported internally in Slack)
The error is only thrown when sending a native token. If you send ERC20 tokens, the error isn't thrown. There seems to be a problem with Brave wallet's EIP712 implementation.

How this PR fixes it

When signing a transaction with brave wallet, if you try to sign with a non-supported signing method (eth_signTypedData), brave wallet throws a custom error that is not an instance of JS built-in Error class, but one of our error checkers expects error to be an instance of Error class

How to test it

Test signing transactions in brave wallet, both tokens and native currency

Screenshots

[github-actions]

CLA Assistant Lite All Contributors have signed the CLA.

[github-actions]

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	0	N/A

• Result: ✅ success
• Annotations: 0 total

---

^{Report generated by eslint-plus-action}

[coveralls]

Pull Request Test Coverage Report for Build 1768545612

• 1 of 4 (25.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.01%) to 33.621%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/logic/safe/transactions/offchainSigner/index.ts	1	4	25.0%

Totals
Change from base Build 1760720190:	-0.01%
Covered Lines:	3187
Relevant Lines:	8486

---

💛 - Coveralls

[github-actions]

Deployment links

🟠 Rinkeby	⚪ Mainnet	🟣 Polygon	🟡 BSC	⚫ Arbitrum	🟢 Gnosis Chain

[github-actions]

E2E Tests Failed
Check the results here: https://github.com/gnosis/safe-react-e2e-tests/actions/runs/1768557280

Failed tests:

• ❌ Read-only transaction creation and review Read-only transaction creation and review
• ❌ Safe Balances Safe Balances

[iamacook]

Looks good to me!

[katspaugh]

Thanks Mikhail!

We should probably re-evaluate why we're even throwing only a subset of errors when signing.

[iamacook]

We should probably re-evaluate why we're even throwing only a subset of errors when signing.

It's probably worthwhile if @mikheevm added some comments for this reason.

[mmv08]

Thanks Mikhail!

We should probably re-evaluate why we're even throwing only a subset of errors when signing.

The whole off-chain signing flow works by calling different signing methods until it finds one that works/is implemented in the wallet. Therefore, we ignore thrown errors and move to the next signing function or fall back to the on-chain approval. I also added a handler for an error when the user rejects the signing proposal in metamask because, in this situation, we don't want to continue the signing process at all. I'm not sure why we throw in the case with keystone wallet.

We can add a console.log to the catch block to not swallow an error, but not console.error because the error is expected and handled

[mmv08]

Is it good to go, or do we want to wait for QA?

[iamacook]

Is it good to go, or do we want to wait for QA?

Let's wait for QA. We agreed this morning to add it to the code freeze for Wednesday's release.

[francovenica]

Looks good to me. Did a bunch of different tx using the Brave wallet (that becomes MM for some reason, I think is for the onboard thing since I even uninstalled MM in brave)
I had no issues signing with the estimated gaslimit/gas price by default
The safe https://pr3389--safereact.review-safe.gnosisdev.com/app/rin:0x3dc660f45A8207e9Fe47AE0cc6ecC0660d462b92/transactions/history