extension icon indicating copy to clipboard operation
extension copied to clipboard

Display an error message from an RPC request instead of 'the user rejected the request'

Open kkosiorowska opened this issue 2 years ago • 4 comments

Closes #2672

This PR propagates a real error message to the user. The issue described in the task no longer occurs or happens sporadically and is difficult to catch. PR is to help define the issues with transaction failure. During testing the solution, please try to execute several transactions to define possible issues, which will be fixed in subsequent tasks.

UI

208640296-75a6fb80-958c-45de-9739-47bab8dec15d Screenshot 2022-12-19 at 10 51 22

Steps to Recreate

Use revoke.cash to revoke the spend approval for the velodrome contract if needed.

  • Swap on Velodrome (Optimism)
  • Make sure that transaction need an approval first
  • Make sure that the approval process passes. (Also, in this case, an error may occur.)
  • Next, try to sign the transaction.
  • Check if a propagated error is displayed.

Latest build: extension-builds-2802 (as of Thu, 12 Jan 2023 07:59:20 GMT).

kkosiorowska avatar Dec 19 '22 11:12 kkosiorowska

This is technically not in line with EIP-1193 but i definitely see the value in it. Curious to hear others thoughts on this.

0xDaedalus avatar Dec 20 '22 14:12 0xDaedalus

So do I understand it correctly that because of this change we will be responding to dapps with a message with a possibly modified error code?

The error code must stay the same - but the spec dictates that:

message SHOULD adhere to the specifications in the Error Standards section below

which does specify the description sent along with code 4001.

Especially with the wording of "should" I'm thinking its ok for us to bend the rules here a little bit.

0xDaedalus avatar Dec 21 '22 15:12 0xDaedalus

So do I understand it correctly that because of this change we will be responding to dapps with a message with a possibly modified error code? Isn't it going to break something or cause wrong error handling in the dapp itself?

Only the message is modified. The change is intended to describe the issues more precisely and it shouldn't break anything. The message is extracted from the error and then displayed in the dapp.

kkosiorowska avatar Dec 22 '22 07:12 kkosiorowska

Lets use this PR as an opportunity to write our first test for the provider-bridge.

I created some first simple tests for provider-bridge.

kkosiorowska avatar Dec 28 '22 13:12 kkosiorowska