-
Notifications
You must be signed in to change notification settings - Fork 35
Jsonrpc #51
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
Conversation
Deploying ensips with
|
| Latest commit: |
bd8f929
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d218331f.ensips.pages.dev |
| Branch Preview URL: | https://jsonrpc.ensips.pages.dev |
| ## Abstract | ||
|
|
||
| This ENSIP specifies APIs for querying metadata directly on the resolver for EIP-3668 (CCIP Read: Secure offchain data retrieval) enabled names. EIP-3668 will power many of the domains in the future, however since the retrieval mechanism uses wildcard + offchain resolver, there is no standardised way to retrieve important metadata information such as the owner (who can change the records), or which L2/offchain database the records are stored on. | ||
| This ENSIP specifies APIs for querying metadata directly on the resolver for EIP-3668 (CCIP Read: Secure offchain data retrieval) enabled names. EIP-3668 will power many domains in the future, however since the retrieval mechanism uses wildcard + offchain resolver, there is no standardised way to retrieve important metadata information such as which L2/offchain database the records are stored on and where JSON RPC endpoint is to find event log information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need a rewrite, but we can wait for feedback first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question about the practicality of creating synthetic events is that (non EVM chain/dbms) has to create not just events but also synthetic smart contract address and blocknumber so that filtering paramater also works
| event MetadataChanged( | ||
| string name, | ||
| string graphqlUrl, | ||
| bytes name, // DNS-encoded name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should specify that all names that are suffixed with this name are considered to be covered by this event.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll also need a way for an event to signify removing this association, such as emitting it with default arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should specify that all names that are suffixed with this name are considered to be covered by this event I remember we had conversation about this but didn't fully understand. Can you elaborate?
| uint256[] values | ||
| ); | ||
| // Standard ERC721 transfer event for name ownership changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe we're using 721 anywhere in v2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comment below.
| resolver: Resolver! | ||
| expiryDate: BigInt | ||
| } | ||
| NOTE: Even though ENS v2 registry contract is ERC1155 (TransferSingle, TransferBatch), ERC721 events(Transfer) are also accepted if name owner choose to build their own registry with an NFT capability. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can support it that simply; registries have to implement 1155 IIRC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In V1, we support wrapped name (1155) and non wrapped (721) and so is many NFT market places, so don't see why not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to allow compatibility with other contracts like Durin which uses 721. Even though we could enforce 1155, there isn't technical issue supporting both
| ``` | ||
|
|
||
| ## Backwards Compatibility | ||
| NOTE: In ENS v1, a resolver has a node property which is derived by the hash of subname and node of its parent name using namehash algorithm. In ENS v2 system, a resolver is used by a single registry which may have multiple parent registries. For example, a registry has a label called `foo` that has parent registry of `eth` and `xyz`, providing name for both `foo.eth` and `foo.xyz`. For a resolver to represent records for multiple names, the node of the v2 resolver is kept as 0x indicating that the node needs to be derived by combining the label information of the registry traversing from the subname all the way to the base/root registry. If a name owner choose to use their own contract with single registry and resolver, the node can be the namehash of the name as in v1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean for If a name owner choose to use their own contract with single registry and resolver, the node can be the namehash of the name as in v1? This is the case to support compatibility with simpler registry/resolver like Durin
Co-authored-by: Nick Johnson <[email protected]>
| bytes name, // DNS-encoded name | ||
| string[] rpcURLs, // JSON RPC endpoint | ||
| uint256 chainId, // Chain identifier (optional) | ||
| address baseRegistry // Base registry address |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please document exactly what interface(s) baseRegistry is expected to implement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels simpler than the GraphQL version! I'm missing how it's intended to be used though. Seems like indexers such as ENSNode are intended to get events from their own RPC URLs for any chain which makes sense, but how do they then expose that to apps?
In other words: I'm building an app the wants to load all available records for a given name. I get the resolver of a name, then check if that contract has a metadata() method. If it does, I get the RPC URL. What request do I make to it in order to get the list of available records?
I think what I described above is actually the most common case - more so than being interested in the full history of a name. And if that's true, having to encode and decode data in an eth_getLogs format seems overkill to me. Not to mention, it assumes that offchain subname providers keep a log of every database operation and can trivially expose that data in a new endpoint.
Perhaps a simple REST API standard that returns the current state of a name is sufficient? Like CCIP Read, a metadata URL could accept requests via https://api.example.com/lookup/{name} and return:
{
"name": "ens.eth",
"texts": {
"description": "hi"
},
"coins": {
"60": "0x0000000000000000000000000000000000000000"
},
"contenthash": "0x",
"resolver": "0x0000000000000000000000000000000000000000"
}Or if we don't want to include values for concerns around freshness:
{
"name": "ens.eth",
"texts": ["description"],
"contenthash": true,
"coins": [60]
}New items could be added for additional metadata like expires, owner, etc.
I could be misunderstanding the eth_getLogs mention, but I think it's worth clarifying this type of thing in the spec.
| } | ||
| ```solidity | ||
| // Emitted when a new subname is registered | ||
| event NameRegistered(uint256 indexed tokenId, string label, uint64 expiration, address registeredBy); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For subnames that don't expire, is it safe to assume that expiration should be type(uint256).max?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes. I will mention that at comment
|
@gskril this PR is the version to provide consistent way to discover event input for indexers. Unlike my previous PR #41 which aim to provide consistent way to discover api for apps, apps is expected to query via their preferred indexing service or index on your own. We are currently waiting from namehash team about the expected api for apps |
|
Hmm since most RPC providers don't return all logs in a single request, how do we expect apps to use this directly to do something like get a list of all text record keys set for a name? I thought one design goal was for apps to be able to use ENSIP-16 directly without relying on a larger indexer, but could be misunderstanding. |
|
|
ENSIP-16: Metadata Event Discovery
This PR significantly refactors ENSIP-16 to focus on event discovery and metadata standardization for offchain ENS names, shifting away from the previous GraphQL-centric approach to a more flexible JSON-RPC based system.
graphqlUrlreturn value withrpcURLs,chainId, andbaseRegistry