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.

Sign up for GitHub

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

Jump to bottom

[Bug]: CCIP Read ENS resolution fails #27784

Open
Gudahtt opened this issue Oct 11, 2024 · 5 comments
Open

[Bug]: CCIP Read ENS resolution fails #27784

Gudahtt opened this issue Oct 11, 2024 · 5 comments
Assignees
@mikesposito
Labels
regression-prod-12.1.1 Regression bug that was found in production in release 12.1.1 Sev2-normal Normal severity; minor loss of service or inconvenience. team-identity team-wallet-api-platform team-wallet-framework type-bug

Comments

@Gudahtt
Copy link
Member

Gudahtt commented Oct 11, 2024

Describe the bug

Originally reported in MetaMask/eth-json-rpc-middleware#335:

ENS resolution sometimes uses CCIP read (eip-3668) to resolve an address.
That relies on a reverted execution being interpreted as a redirect.
When the provider used to perform such a read is managed by this middleware, the read operation fails.
My guess is that the reverted execution is somehow wrapped such that the error is no longer interpretable.

Expected behavior

ENS resolution should succeed, including addresses that use CCIP read.

Screenshots/Recordings

No response

Steps to reproduce

  1. Checkout the test-app repository locally to the ens-resolution branch (see feat: Add ENS resolution test-dapp#362)
  2. Start the test dapp using yarn start
  3. Install the MetaMask extension (any type of build)
  4. Ensure that the current selected chain is Ethereum Mainnet
  5. Navigate to the locally-hosted test dapp and connect at least one account
  6. Scroll to the bottom, enter jwt.ro into the "ENS Resolution" field, then click Submit

This should resolve to an address, but it currently resolves to null instead.

Error messages or log output

No response

Detection stage

In production (default)

Version

12.1.1

Build type

None

Browser

Chrome

Operating system

MacOS

Hardware wallet

No response

Additional context

No response

Severity

No response
@Gudahtt Gudahtt added type-bug Sev2-normal Normal severity; minor loss of service or inconvenience. regression-prod-12.1.1 Regression bug that was found in production in release 12.1.1 labels Oct 11, 2024
@FrederikBolding
Copy link
Member

@Gudahtt We'll need #22875

Blocked by:
MetaMask/core#4773
#24496
#27700

@snackman
Copy link

plz fiz this, PizzaDAO needs you. We'll buy pizza for whoever does it!

@legobeat legobeat mentioned this issue Oct 17, 2024
Merged
14 tasks
@legobeat
Copy link
Contributor

Also related:

github-merge-queue bot pushed a commit that referenced this issue Oct 18, 2024
@legobeat
chore(deps): upgrade from json-rpc-engine to @metamask/json-rpc-engine (
eedeb24 
#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.
vinistevam pushed a commit that referenced this issue Oct 20, 2024
@legobeat @vinistevam
chore(deps): upgrade from json-rpc-engine to @metamask/json-rpc-engine (
dd34756 
#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.
@mirceanis
Copy link
Contributor

looks like #22875 fixed it

Gudahtt pushed a commit that referenced this issue Oct 21, 2024
@legobeat @Gudahtt
chore(deps): upgrade from json-rpc-engine to @metamask/json-rpc-engine (
0914812 
#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.
@mirceanis
Copy link
Contributor

mirceanis commented Oct 31, 2024 •
edited
Loading

This issue has been fixed. Are we waiting for a release to close it?

cc @mikesposito @Gudahtt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mikesposito mikesposito

Labels
regression-prod-12.1.1 Regression bug that was found in production in release 12.1.1 Sev2-normal Normal severity; minor loss of service or inconvenience. team-identity team-wallet-api-platform team-wallet-framework type-bug
Projects
Bugs by severity
Status: To be fixed
Bugs by team
Status: To be fixed
Milestone
No milestone
Development

No branches or pull requests
6 participants
@snackman @mirceanis @FrederikBolding @Gudahtt @mikesposito @legobeat