feat(ensindexer): advanced reverse address healing #750
Conversation
Include two addintional strategies to heal labels from reverse registry. Important note: the reverse address healing must only occur for events coming from one of ENS Deployment Chains.
… the ENS Deployment Chains.
🦋 Changeset detectedLatest commit: b3b7f43 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
lightwalker-eth
left a comment
There was a problem hiding this comment.
@tk-o Super work on this PR! 🚀 Reviewed and shared a few suggestions 👍
| const ensDeploymentChainId = getEnsDeploymentChainId(); | ||
| let healedLabel = null; | ||
|
|
||
| // 1. if healing label from reverse addresses is enabled, and the parent is a known |
There was a problem hiding this comment.
| // 1. if healing label from reverse addresses is enabled, and the parent is a known | |
| // 1. if healing labels from reverse addresses is enabled, and the parent is a known |
| // reverse node (i.e. addr.reverse), give it a go | ||
| if (config.healReverseAddresses && REVERSE_ROOT_NODES.has(parentNode)) { | ||
| // reverse node (i.e. addr.reverse), and | ||
| // the event comes from the chain that is the ENS Deployment Chain, give it a go. |
There was a problem hiding this comment.
| // the event comes from the chain that is the ENS Deployment Chain, give it a go. | |
| // the event comes from the chain that is the ENS Deployment Chain, then attempt to heal the unknown label. |
| if (config.healReverseAddresses && REVERSE_ROOT_NODES.has(parentNode)) { | ||
| // reverse node (i.e. addr.reverse), and | ||
| // the event comes from the chain that is the ENS Deployment Chain, give it a go. | ||
| // Note: the reverse address healing must only be enabled for ENS Deployment Chains, |
There was a problem hiding this comment.
Suggest to replace this comment.
The key idea we want to communicate here is ENSIP-19: https://docs.ens.domains/ensip/19
Based on this standard, only an ENS Deployment Chain (such as mainnet, holesky, or sepolia) may record primary names under the addr.reverse subname.
Currently, we are only healing primary names on ENS Deployment Chains. We will add support for non-ENS Deployment Chain primary name healing in the future.
| labelHash, | ||
| }); | ||
|
|
||
| // if that didn't work, try healing with the event's`owner` address |
There was a problem hiding this comment.
| // if that didn't work, try healing with the event's`owner` address | |
| // if that didn't work, try healing with the event's `owner` address |
| labelHash, | ||
| }); | ||
|
|
||
| // if that didn't work, try healing with the event's`owner` address |
There was a problem hiding this comment.
Can we document why sometimes the sender address won't work but the owner address will work? It would be nice to give a specific example transaction too, if possible.
This is very helpful for us when we or others look back on this logic months or years from now and we are trying to understand why we made some of these decisions.
Best to do it now while it is still fresh in our mind.
| if (!healedLabel) { | ||
| // by this point, we have exhausted all options for healing the reverse address | ||
| // and we still don't have a valid label, so we throw an error | ||
| throw new Error( |
There was a problem hiding this comment.
This is awesome! I love enforcing an expectation of a 100% success rate 👍
| const uniqueAddresses = new Set<Address>(); | ||
|
|
||
| // Helper function to search for plain addresses (0x followed by 40 hex characters) | ||
| const searchForPlainAddresses = (text: string) => { |
There was a problem hiding this comment.
Is there a special reason why we're defining this function inside of getAddressesFromTrace? Is there a special reason not to extract it out?
There was a problem hiding this comment.
I saw searchForPlainAddresses function as implementation detail of getAddressesFromTrace, and searchForPlainAddresses was not required anywhere on its own.
| }; | ||
|
|
||
| // Helper function to search for serialized addresses (hex strings in calldata) | ||
| const searchForSerializedAddresses = (text: string) => { |
There was a problem hiding this comment.
Is there a special reason why we're defining this function inside of getAddressesFromTrace? Is there a special reason not to extract it out?
There was a problem hiding this comment.
I saw searchForSerializedAddresses function as implementation detail of getAddressesFromTrace, and searchForSerializedAddresses was not required anywhere on its own.
| * Schema for the `debug_traceTransaction` method in Viem RPC client. | ||
| * This schema defines the parameters and return type. | ||
| * | ||
| * @see https://viem.sh/docs/contract/tracing.html#tracetransaction |
There was a problem hiding this comment.
This link gives a 404 error
| /** | ||
| * Options for the `debug_traceTransaction` RPC method. | ||
| **/ | ||
| type DebugTraceTransactionOptions = CallTracerOptions; |
There was a problem hiding this comment.
Is there a special reason why we create this type alias? Why define both CallTracerOptions and DebugTraceTransactionOptions? If so please ignore this comment. Others might have a similar question.
There was a problem hiding this comment.
There are more tracers available, but we only need the CallTracer once. I'll simplify the type to express only what's needed.
This PR makes two important changes:
Make it possible to attempt reverse address healing for events coming exclusively from one of the ENS Deployment Chains (
mainnet,sepolia,holesky,ens-test-env).Introduce two additional strategies of healing reverse addresses
a) If the original strategy failed to heal the label with transaction sender address, try healing the label with owner address from event arguments.
b) If the above strategy failed to heal the label with owner address from event arguments, call RPC for all transaction traces, extract all valid EVM address from those traces, and try to heal the label with each extracted address. One of these extracted addresses must be a hit.
Resolves #616