feat(nameguard): Add ENSNode API for primary name resolution #713
Conversation
…c primary name resolution
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
The decision is that ENSNode API behaviour is the correct one (if the primary name is unnormalized then we say there is no primary name) so we need to fix tests. Probably unnormalized state in the endpoint is no longer valid. |
…ling to use ENSNode API exclusively
… to current issues
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| the primary name is unnormalized. | ||
| """ | ||
| if network_name == NetworkName.MAINNET: | ||
| url = f'https://api.alpha.ensnode.io/api/resolve/primary-name/{address}/1' |
There was a problem hiding this comment.
Ethereum addresses are not being normalized to lowercase before being used in the ENSNode API URL, causing requests with checksummed/mixed-case addresses to fail.
View Details
📝 Patch Details
diff --git a/packages/nameguard-python/nameguard/nameguard.py b/packages/nameguard-python/nameguard/nameguard.py
index 1fff110..a6f64c5 100644
--- a/packages/nameguard-python/nameguard/nameguard.py
+++ b/packages/nameguard-python/nameguard/nameguard.py
@@ -452,6 +452,8 @@ class NameGuard:
The normalized primary name, or None if no primary name exists or
the primary name is unnormalized.
"""
+ # Normalize address to lowercase for ENSNode API compatibility
+ address = address.lower()
if network_name == NetworkName.MAINNET:
url = f'https://api.alpha.ensnode.io/api/resolve/primary-name/{address}/1'
elif network_name == NetworkName.SEPOLIA:
diff --git a/packages/nameguard-python/tests/test_nameguard.py b/packages/nameguard-python/tests/test_nameguard.py
index fc3090f..5cb3481 100644
--- a/packages/nameguard-python/tests/test_nameguard.py
+++ b/packages/nameguard-python/tests/test_nameguard.py
@@ -694,10 +694,12 @@ async def test_secure_primary_name(nameguard: NameGuard):
@pytest.mark.asyncio
async def test_secure_primary_name_wrong_casing(nameguard: NameGuard):
network = 'mainnet'
- with pytest.raises(ProviderUnavailable):
- await nameguard.secure_primary_name(
- '0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96046', network, return_nameguard_report=True
- )
+ # Test that mixed-case addresses are properly normalized and handled
+ r = await nameguard.secure_primary_name(
+ '0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96046', network, return_nameguard_report=True
+ )
+ # This address has no primary name
+ assert r.primary_name_status == 'no_primary_name'
@pytest.mark.asyncio
Analysis
ENSNode API rejects mixed-case Ethereum addresses without normalization
What fails: The get_primary_name() method in nameguard/nameguard.py constructs ENSNode API URLs using address parameters without normalizing them to lowercase, causing requests with checksummed/mixed-case addresses to fail with HTTP 400 Bad Request errors.
How to reproduce:
# Call with mixed-case (checksummed) address
result = await nameguard.secure_primary_name(
'0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96046', # Mixed case
'mainnet'
)
# Previously raised: ProviderUnavailable exceptionResult: Before the fix, mixed-case addresses caused the ENSNode API to reject the request with HTTP 400, which was caught and converted to a ProviderUnavailable exception. This breaks legitimate use cases where Ethereum addresses are passed in checksummed format.
Expected: Mixed-case addresses should be automatically normalized to lowercase before being sent to the ENSNode API, allowing the API call to succeed.
Fix: Normalize the address parameter to lowercase in get_primary_name() method before embedding it in the API URL. The ENSNode API accepts lowercase addresses but rejects mixed-case ones. Updated test test_secure_primary_name_wrong_casing to verify that mixed-case addresses are now properly handled and return the expected primary name status.
… for nameguard report retrieval
| ); | ||
|
|
||
| it( | ||
| "should return error for wrong address casing", |
There was a problem hiding this comment.
Test expects securePrimaryName to throw error for mixed-case address, but function returns a result instead of throwing
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_secure_primary_name_wrong_casing(nameguard: NameGuard): |
There was a problem hiding this comment.
Test expects ProviderUnavailable exception when calling with mixed-case address, but address normalization should allow the lookup to succeed
| the primary name is unnormalized. | ||
| """ | ||
| if network_name == NetworkName.MAINNET: | ||
| url = f'https://api.alpha.ensnode.io/api/resolve/primary-name/{address}/1' |
There was a problem hiding this comment.
Ethereum addresses are not being normalized to lowercase before being used in the ENSNode API URL, causing requests with checksummed/mixed-case addresses to fail with HTTP 400 Bad Request errors
…vironment variable usage for ENSNode API URLs
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@djstrong Thanks for updates. Reviewed and shared feedback
| # ENSNode URLs | ||
| ENSNODE_URL_MAINNET=https://api.alpha.ensnode.io | ||
| ENSNODE_URL_SEPOLIA=https://api.alpha-sepolia.ensnode.io | ||
|
|
There was a problem hiding this comment.
@djstrong Several lines below I still see the following environment variables:
ENS_SUBGRAPH_URL_MAINNETENS_SUBGRAPH_URL_SEPOLIA
These should be fully removed because we can instead build ENS Subgraph API URLs as a function of ENSNODE_URL_* values by taking any ENSNode URL and just appending /subgraph to the end.
Ex:
| export ENSNODE_URL_SEPOLIA=https://api.alpha-sepolia.ensnode.io | ||
| export ALCHEMY_URI_MAINNET=https://eth-mainnet.g.alchemy.com/v2/[YOUR_ALCHEMY_API_KEY] | ||
| export ALCHEMY_URI_SEPOLIA=https://eth-sepolia.g.alchemy.com/v2/[YOUR_ALCHEMY_API_KEY] | ||
| export ENS_SUBGRAPH_URL_MAINNET="https://gateway-arbitrum.network.thegraph.com/api/[YOUR_SUBGRAPH_API_KEY]/subgraphs/id/5XqPmWe6gjyrJtFn9cLy237i4cWw2j9HcUJEXsP5qGtH" |
There was a problem hiding this comment.
We should be able to completely remove any reference to any ENS Subgraphs that are hosted by The Graph.
| ```bash | ||
| export ALCHEMY_URI_MAINNET=https://eth-mainnet.g.alchemy.com/v2/[YOUR_ALCHEMY_API_KEY] | ||
| export ALCHEMY_URI_SEPOLIA=https://eth-sepolia.g.alchemy.com/v2/[YOUR_ALCHEMY_API_KEY] | ||
| export ENS_SUBGRAPH_URL_MAINNET="https://gateway-arbitrum.network.thegraph.com/api/[YOUR_SUBGRAPH_API_KEY]/subgraphs/id/5XqPmWe6gjyrJtFn9cLy237i4cWw2j9HcUJEXsP5qGtH" |
There was a problem hiding this comment.
There shouldn't be any references to The Graph anymore.
| # For more details, see: | ||
| # - https://docs.ens.domains/web/subgraph | ||
| # - https://discuss.ens.domains/t/ens-subgraph-migration-to-the-decentralised-version/19183 | ||
| # - https://thegraph.com/explorer/subgraphs/5XqPmWe6gjyrJtFn9cLy237i4cWw2j9HcUJEXsP5qGtH?view=Query&chain=arbitrum-one |
There was a problem hiding this comment.
This is still an open issue
| # - https://docs.ens.domains/web/subgraph | ||
| # - https://discuss.ens.domains/t/ens-subgraph-migration-to-the-decentralised-version/19183 | ||
| # - https://thegraph.com/explorer/subgraphs/5XqPmWe6gjyrJtFn9cLy237i4cWw2j9HcUJEXsP5qGtH?view=Query&chain=arbitrum-one | ||
| ENS_SUBGRAPH_URL_MAINNET=https://api.thegraph.com/subgraphs/name/ensdomains/ens |
There was a problem hiding this comment.
These need to be updated. There's no need to configure subgraph URLs anymore.
Subgraph API URLs should be a function of ENSNODE_URL_MAINNET or ENSNODE_URL_SEPOLIA with the correct path added to talk to the subgraph API endpoint.
| PROVIDER_URI_MAINNET=https://rpc.ankr.com/eth | ||
| PROVIDER_URI_SEPOLIA=https://rpc.ankr.com/eth_sepolia | ||
| # ENSNode URLs (optional - defaults to public ENSNode API endpoints) | ||
| # The ENSNode API is used for primary name lookups in secure-primary-name. |
There was a problem hiding this comment.
It should also be used for looking up labelhashes / namehashes
| } | ||
|
|
||
| class NameGuardJS extends NameGuard { | ||
| private publicClient: PublicClient; |
There was a problem hiding this comment.
This should be creating an instance of ENSNodeClient as imported from ensnode-sdk.
Definition can be found here: https://github.com/namehash/ensnode/blob/main/packages/ensnode-sdk/src/client.ts
| : process.env.ENSNODE_URL_SEPOLIA || "https://api.alpha-sepolia.ensnode.io"; | ||
|
|
||
| const chainId = network === "mainnet" ? 1 : 11155111; | ||
| const url = `${baseUrl}/api/resolve/primary-name/${address}/${chainId}`; |
There was a problem hiding this comment.
We're supposed to use ENSNodeClient as imported from ensnode-sdk
| "PROVIDER_URI_MAINNET is not defined. Defaulting to viem's default provider, which may have rate limiting and other performance limitations.", | ||
| "ENSNODE_URL_MAINNET is not defined. Defaulting to https://api.alpha.ensnode.io.", | ||
| ); | ||
| ENSNODE_URL_MAINNET = "https://api.alpha.ensnode.io"; |
There was a problem hiding this comment.
If we make correct use of ENSNodeClient as imported from ensnode-sdk then there's no need to redefine a default URL here.
| "PROVIDER_URI_SEPOLIA is not defined. Defaulting to viem's default provider, which may have rate limiting and other performance limitations.", | ||
| "ENSNODE_URL_SEPOLIA is not defined. Defaulting to https://api.alpha-sepolia.ensnode.io.", | ||
| ); | ||
| ENSNODE_URL_SEPOLIA = "https://api.alpha-sepolia.ensnode.io"; |
There was a problem hiding this comment.
Hmm, maybe it would help if ensnode-sdk gave a helper function to get the default URL for a particular ENS namespace. I'll work on a quick PR for this.
Tests fail because output from ENSNode API is different than from RPC.
E.g. for address 0xFD9eE68000Dc92aa6c67F8f6EB5d9d1a24086fAd we got exampleprimary.cb.id from ENSNode but null from Alchemy.
https://api.alpha.ensnode.io/api/resolve/primary-name/0xFD9eE68000Dc92aa6c67F8f6EB5d9d1a24086fAd/1 ;
https://api.alpha.ensnode.io/api/resolve/primary-name/0xc9f598bc5bb554b6a15a96d19954b041c9fdbf14/1 gives null but Alchemy returns vıtalik.eth (unnormalized)