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

[Bug]: Support for Solidity custom error is broken #18510

Open
0xnakato opened this issue Apr 7, 2023 · 3 comments
Open

[Bug]: Support for Solidity custom error is broken #18510

0xnakato opened this issue Apr 7, 2023 · 3 comments
Labels
Sev3-low Low severity; minimal to no impact upon users team-confirmations Push issues to confirmations team team-confirmations-system DEPRECATED: please use "team-confirmations" label instead type-bug

Comments

@0xnakato
Copy link

0xnakato commented Apr 7, 2023

Describe the bug

After the recent upgrade, the support for Solidity custom error is broken. Previously, it could return the function selector of the custom error that caused the transaction to be reverted when estimating gas/submitting the transaction.

Currently, if the transaction was reverted with a string reason, the error is like

"Failed to submit transaction: cannot estimate gas : [THE STRING ERROR REASON]."

However, for custom errors, the reason is no longer provided.

"Failed to submit transaction: cannot estimate gas."

Steps to reproduce

Call a transaction that will be reverted with Solidity custom errors.

Error messages or log output

{
    "code": -32603,
    "message": "Internal JSON-RPC error.",
    "data": {
        "code": 3,
        "message": "Failed to submit transaction: cannot estimate gas",
        "data": {
            "code": 104,
            "message": "cannot estimate gas"
        }
    }
}

Version

10.28.2

Build type

None

Browser

Chrome

Operating system

Windows

Hardware wallet

No response

Additional context

No response

@0xnakato 0xnakato changed the title [Bug]: Support for Solidity Custom Error is broken [Bug]: Support for Solidity custom error is broken Apr 7, 2023
@danjm danjm added team-confirmations-system DEPRECATED: please use "team-confirmations" label instead Sev3-low Low severity; minimal to no impact upon users labels Apr 11, 2023
@bergarces bergarces self-assigned this Apr 17, 2023
@bergarces
Copy link
Contributor

bergarces commented Apr 20, 2023

Upon investigating the issue, it appears that the message itself comes from the RPC provider.

I cannot find any existing or previous code that attempts to extract selector of the error and attach it to the message part.

Do you know by any chance what RPC provider were you using when the error was being displayed with the selector?

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.

@github-actions github-actions bot added the stale issues and PRs marked as stale label Oct 24, 2023
Copy link
Contributor

github-actions bot commented Dec 8, 2023

This issue was closed because there has been no follow up activity in the last 45 days. If you feel this was closed in error, please reopen and provide evidence on the latest release of the extension. Thank you for your contributions.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 8, 2023
@legobeat legobeat reopened this Oct 18, 2024
@github-actions github-actions bot removed the stale issues and PRs marked as stale label Oct 18, 2024
github-merge-queue bot pushed a commit that referenced this issue Oct 18, 2024
#22875)

## **Description**

## **Related issues**

- #27784
- MetaMask/eth-json-rpc-middleware#335
- #27917
- #18510
- #15250
- MetaMask/metamask-improvement-proposals#36

### Blocked by
- [x] #24496

### Follow-up to
- #24496

## **Manual testing steps**

## **Screenshots/Recordings**

### **Before**

### **After**

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
@cryptotavares cryptotavares added the team-confirmations Push issues to confirmations team label Oct 18, 2024
vinistevam pushed a commit that referenced this issue Oct 20, 2024
#22875)

## **Description**

## **Related issues**

- #27784
- MetaMask/eth-json-rpc-middleware#335
- #27917
- #18510
- #15250
- MetaMask/metamask-improvement-proposals#36

### Blocked by
- [x] #24496

### Follow-up to
- #24496

## **Manual testing steps**

## **Screenshots/Recordings**

### **Before**

### **After**

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
Gudahtt pushed a commit that referenced this issue Oct 21, 2024
#22875)

- #27784
- MetaMask/eth-json-rpc-middleware#335
- #27917
- #18510
- #15250
- MetaMask/metamask-improvement-proposals#36

- [x] #24496

- #24496

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Sev3-low Low severity; minimal to no impact upon users team-confirmations Push issues to confirmations team team-confirmations-system DEPRECATED: please use "team-confirmations" label instead type-bug
Projects
Status: To be fixed
Status: To be fixed
Development

No branches or pull requests

5 participants