ubiquity / pay.ubq.fi

Generate and claim spender permits (EIP-2612)
https://pay.ubq.fi
10 stars 41 forks source link

Decode transaction errors #346

Open rndquu opened 1 month ago

rndquu commented 1 month ago

There's this package which is able to decode transaction errors which really helps in finding out the root cause of why permit or card minting is throwing an error.

As a part of this issue we should decode and show the decoded error to a user on:

Original comment

ariesgun commented 1 month ago

/start

ubiquity-os[bot] commented 1 month ago
Deadline Fri, Oct 25, 5:49 PM UTC
Beneficiary 0xE80FC2700ec6faF5f0347a2E7E7798FAf548e1c3

[!TIP]

  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.
ariesgun commented 1 month ago

This library needs ethers v6, so we need to upgrade ethers from v5 to v6. We can't use /ethers-decode-error/v/1.0.0 because it supports node version < 19.

Shall we upgrade ethers to v6?

0x4007 commented 1 month ago

I think retaining v5 is more maintainable because LLMs are trained on it but not v6.

rndquu commented 1 month ago

This library needs ethers v6, so we need to upgrade ethers from v5 to v6. We can't use /ethers-decode-error/v/1.0.0 because it supports node version < 19.

Shall we upgrade ethers to v6?

We've just recently downgraded from v6 to v5. Perhaps there're other libs.

ariesgun commented 1 month ago

This library needs ethers v6, so we need to upgrade ethers from v5 to v6. We can't use /ethers-decode-error/v/1.0.0 because it supports node version < 19. Shall we upgrade ethers to v6?

We've just recently downgraded from v6 to v5. Perhaps there're other libs.

I see. Will check if there is any alternative..

Keyrxng commented 1 month ago

One potentially hacky method but it would give better insight maybe is the 4byte API where we can search for fn signatures and log them to the console on tx fail. Obv there are collisions and sometimes the sig has not been uploaded but it's something we can setup internally and it's free to use.

I personally copy paste into 4byte when I'm debugging errors so this would make my life easier at least lmao

ariesgun commented 3 weeks ago

One potentially hacky method but it would give better insight maybe is the 4byte API where we can search for fn signatures and log them to the console on tx fail. Obv there are collisions and sometimes the sig has not been uploaded but it's something we can setup internally and it's free to use.

I personally copy paste into 4byte when I'm debugging errors so this would make my life easier at least lmao

Do you still have the result from 4byte? Curious how it looks like

Keyrxng commented 3 weeks ago

Copy the selector from the start of the data and then paste it into the function signature search feature on 4byte and it'll show you. You'll need to write logic to deal with the API for my suggestion ofc

I do this when I'm writing tests, debugging, etc that sort of thing. It's tedious manually as sometimes there are 10-15 calls you need to manually track not always in the same order etc. I don't have any stored results or anything, whatever I gather at the time I comment in the code then remove it after I've fixed it.

ariesgun commented 3 weeks ago

Copy the selector from the start of the data and then paste it into the function signature search feature on 4byte and it'll show you. You'll need to write logic to deal with the API for my suggestion ofc

I do this when I'm writing tests, debugging, etc that sort of thing. It's tedious manually as sometimes there are 10-15 calls you need to manually track not always in the same order etc. I don't have any stored results or anything, whatever I gather at the time I comment in the code then remove it after I've fixed it.

I am not sure why we need to use function search feature from 4byte. I tried the data value from here. We can only get the function signature. And it is already visible in the console logger.

image

I thought what we need is to decode the transaction error and not function signature. The transaction error can be decoded by simply using this library ethers-decode-error/. I think it is sufficient.

Did I miss something?

Keyrxng commented 3 weeks ago

I thought what we need is to decode the transaction error and not function signature.

They both help when debugging in my opinion but both are not required. The spec says to use the lib and decode, so do that and ignore my suggestion.

My suggestion to implement it that way and then to embed it into our official rpc-handler is because it'd be repurposed and likely provide a lot more value across the org. MetaMask lives in the browser and is detached from our rpc-handler but also controls all writes so it does have it's own error handling.

Did I miss something?

No, both approaches were suggested but the spec only lists one. My comment was given a thumbs-up which is usually a sign of approval but only I've mentioned it beyond that. Implement the spec unless someone explicitly approves my suggestion.

ubiquity-os[bot] commented 1 week ago

Passed the deadline and no activity is detected, removing assignees: @ariesgun.