fix: display notification for decode contract error

[DiogoSoaress]

What it solves

Resolves #3772

How this PR fixes it

Throw a CodeException when unable to decode contract error message and wrap getContractErrorMessage in a try...catch so decoding message errors can be caught to show a notification.

How to test it

(From #3772)

Go to the safe matic:0xb412684F4F0B5d27cC4A4D287F42595aB3ae124D
try to execute tx with MNEP tokes

[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	1	N/A

• Result: ✅ success
• Annotations: 0 total

---

^{Report generated by eslint-plus-action}

[github-actions]

Deployment links

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

[coveralls]

Pull Request Test Coverage Report for Build 2353349317

• 3 of 5 (60.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.003%) to 37.017%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/logic/contracts/safeContractErrors.ts	3	5	60.0%

Totals
Change from base Build 2350983078:	-0.003%
Covered Lines:	3894
Relevant Lines:	9567

---

💛 - Coveralls

[schmanu]

Makes sense! Nice work!

I added a small idea we could discuss to avoid the nested try-catches.

[francovenica]

So the tx in that test safe is no longer there, So I just used another safe I own, took the MNEP token address (matic:0x0B91B07bEb67333225A5bA0259D55AeE10E3A578) and executed a "Bridge" method that I knew was going to fail.

The result is this:

Do we need to show all the data in the error as well?

[katspaugh]

We shouldn't display code 817 to the user. I would throw the original error from decode and logError 817 in the code that catches it.

[francovenica]

The main issue the snack bar wasn't showing and is showing now, so we should agree on what error to show. Maybe just a "transaction failed" is enough?
@katspaugh

[katspaugh]

In this particular case, I think we need to fix the root cause – that it doesn't decode the contract error properly. The error is GS013 – not enough gas. This is what should be displayed to the user.

[katspaugh]

You need to fix the error decoding, not tell the user that we failed decoding it.

[DiogoSoaress]

Agree, I was reproducing the GS013 and I'm working on it

[DiogoSoaress]

Displaying the decoded contract error in the notification now

and logging the CodedException

[DiogoSoaress]

Regarding throwing with the GS013 code, the way I've used to test this bug fix was based on #3879 (comment) as I didn't manage to reproduce the original bug from @liliya-soroka... Nevertheless this PR fixes the Notification not showing.

[DiogoSoaress]

After investigating the origin of GS013 which resulted in https://5afe.slack.com/archives/C03BHKS5K6D/p1652871917369379 can we move the current PR state to QA?

Shortly, the contract errors GSxxx will not reach the app level and the user can only see what really happened at the contract level by simulating the transaction. The code now shows a generic message as GS013 can be misleading.

A better call to action notification error handling makes sense to be tackled in safe-global/safe-pm#83 more broadly.

[francovenica]

Cannot test this.
The PR has the issue mentioned in slack this morning, that prevents the creation of tx (the MM pop up doesn't event show up)
Tried to switch the CGW switch to no avail.

[katspaugh]

@francovenica I've updated this branch from dev, should be good now 👍

[francovenica]

I still have the same issue. I cannot create tx in this PR

[katspaugh]

Damn, sorry, updated again. Now it should work.

[francovenica]

Ok, I was able to create a tx. I tried several times.
I get an red snackbar with an error akin to "The tx cannot be decoded". I don't get the GS013 error that Diogo showed in his comment.
The tx I made was the same as the one I described in the first comment I made in this ticket.

[katspaugh]

@francovenica can you try some other failing txns, as per Liliya's comment:

GS020: Signatures data too short
GS021: Invalid contract signature location: inside static part
GS022: Invalid contract signature location: length not present
GS023: Invalid contract signature location: data not complete
GS024: Invalid contract signature provided
GS025: Hash has not been approved
GS026: Invalid owner provided

[usame-algan]

I get an red snackbar with an error akin to "The tx cannot be decoded". I don't get the GS013 error that Diogo showed in his comment.

This looks correct to me. The error message was changed to a generic one since contract errors often return GS013 even if it doesn't have anything to do with missing safeTxGas.

[francovenica]

I'd agree with Usame here. Most of the errors you get are GS013 even if there is no gas related issues.
Regarding the list of GS errors, I think the safe itself prevents you from creating a tx with those issues (there is no way to create a tx with the wrong owner or execute a tx where the hash has not been approved)