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

fix: ens lookup #297

Merged
merged 4 commits into from
Sep 14, 2024
Merged

fix: ens lookup #297

merged 4 commits into from
Sep 14, 2024

Conversation

hhio618
Copy link
Contributor

@hhio618 hhio618 commented Sep 10, 2024

Resolves #287

QA

You can see that the name is now being resolved correctly here:
qa

Copy link
Contributor

github-actions bot commented Sep 10, 2024

Unused exports (1)

Filename exports
static/scripts/rewards/cirip/ens-lookup.ts reverseEnsInterface
78aaac65b91a67aa2f8344b31763aaa5d0a18ce7
ff36cce6fda75bfd515236d063bc9db405bf567e
9237f0e9b52f6612e727f9b521d5c4842e38cd56

@ubiquity-os-deployer
Copy link

ubiquity-os-deployer bot commented Sep 10, 2024

@0x4007
Copy link
Member

0x4007 commented Sep 10, 2024

Delete the file static/scripts/rewards/cirip/ens-lookup.ts

@hhio618
Copy link
Contributor Author

hhio618 commented Sep 11, 2024

@0x4007 Sure, still need the sub-graph query?

@0x4007
Copy link
Member

0x4007 commented Sep 11, 2024

@0x4007 Sure, still need the sub-graph query?

I don't understand what you're talking about

@hhio618
Copy link
Contributor Author

hhio618 commented Sep 11, 2024

I meant this file: static/scripts/rewards/cirip/fetch-ens.ts, I believe this no longer needs to query the subgraph as we're reverse resolving the ens primary name!

@0x4007
Copy link
Member

0x4007 commented Sep 11, 2024

Your CI is failing because its unused. Delete the file.

@0x4007 0x4007 mentioned this pull request Sep 11, 2024
@0x4007
Copy link
Member

0x4007 commented Sep 11, 2024

Interesting bug but check this comment for (the commit hash links) deploys with a permit when the CI passes.

It doesn't work on Gnosis Chain, so you need to use an API which always checks mainnet, since thats the only name resolver we care about.

ens-lookup.ts:24 Error: network does not support ENS (operation="lookupAddress", network="xdai", code=UNSUPPORTED_OPERATION, version=providers/5.7.2)
    at _Logger2.makeError (index.ts:269:28)
    at _Logger2.throwError (index.ts:281:20)
    at Web3Provider.<anonymous> (base-provider.ts:1989:20)
    at Generator.next (<anonymous>)
    at fulfilled (formatter.ts:523:1)

@0x4007
Copy link
Member

0x4007 commented Sep 11, 2024

Certainly getting closer but use our RPC handler.

image

}),
});
const web3Provider = new ethers.providers.JsonRpcProvider(mainnetRpcUrl);
const ensName = await web3Provider.lookupAddress(address);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Keyrxng You can test the resolution by hard-coding the address parameter in your local deployment!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it just grabs the first name you have from an array of all the ones you own. I own many, and it picks one consistently that is not my main.

I don't have the proper context to test against the spec but your right actually we both can test it.

@0x4007 what is your main? Which addresses should resolve to which ENS names?

@hhio618 You could provide a screenshot of the name being correctly resolved as QA. Each task should have some kind provided if possible

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0x4007 what is your main? Which addresses should resolve to which ENS names?

アレクサンダー.eth

@0x4007 0x4007 merged commit 9237f0e into ubiquity:development Sep 14, 2024
3 checks passed
@ubiquity-os ubiquity-os bot mentioned this pull request Sep 14, 2024
This was referenced Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENS Fix
3 participants