Skip to content

Conversation

@shrugs
Copy link
Collaborator

@shrugs shrugs commented May 6, 2025

closes #678

the threedns resolvers don't emit TextChanged, they emit DNSRecordChanged which we previously didn't parse. this implements RRSet parsing to bring the behavior in-line with the other plugins'

@changeset-bot
Copy link

changeset-bot bot commented May 6, 2025

🦋 Changeset detected

Latest commit: f5c624c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
ensindexer Minor
ensadmin Minor
ensrainbow Minor
@ensnode/ens-deployments Minor
@ensnode/utils Minor
@ensnode/ensrainbow-sdk Minor
@ensnode/ponder-metadata Minor
@ensnode/ensnode-schema Minor
@ensnode/ponder-subgraph Minor
@ensnode/shared-configs Minor

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

@vercel
Copy link
Contributor

vercel bot commented May 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
admin.ensnode.io ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 12, 2025 8:55pm
ensnode.io ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 12, 2025 8:55pm
ensrainbow.io ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 12, 2025 8:55pm

@0xBison
Copy link
Contributor

0xBison commented May 7, 2025

@shrugs shrugs changed the title checkpoint: dnsrecordchanged parsing ThreeDNS DNSRecordChanged Parsing May 9, 2025
@shrugs shrugs marked this pull request as ready for review May 9, 2025 17:54
@shrugs shrugs requested a review from a team as a code owner May 9, 2025 17:54
Comment on lines 332 to 393
// subgraph ignores this event
if (pluginName === PluginName.Subgraph) return;

// but for non-subgraph plugins, we parse the RR set data for relevant records
const { node, name, resource, record } = event.args;

// we only index TXT records (resource id 16)
if (resource !== 16) return;

// parse the record's name, which is the key of the DNS record
const [, recordName] = decodeDNSPacketBytes(hexToBytes(name));

// invariant: recordName is always available and parsed correctly
if (!recordName) throw new Error(`Invalid DNSPacket, cannot parse name '${name}'.`);

// relevant keys end with .ens
if (!recordName.endsWith(".ens")) return;

// trim the .ens off the end to match ENS record naming
const key = recordName.slice(0, -4);

// parse the `record` parameter, which is an RRSet describing the value of the DNS record
const answers = parseRRSet(record);
for (const answer of answers) {
switch (answer.type) {
case "TXT": {
// > When decoding, the return value will always be an array of Buffer.
// https://github.com/mafintosh/dns-packet
const value = decodeTXTData(answer.data as Buffer[]);

const id = makeResolverId(event.log.address, node);
const resolver = await upsertResolver(context, {
id,
domainId: node,
address: event.log.address,
});

// upsert new key
await context.db
.update(schema.resolver, { id })
.set({ texts: uniq([...(resolver.texts ?? []), key]) });

// log ResolverEvent
await context.db.insert(schema.textChanged).values({
...sharedEventValues(context.network.chainId, event),
resolverId: id,
key,
// note: coalesce empty string to null, see `handleTextChanged` for context
value: value || null,
});
break;
}
default: {
// no-op unhandled records
// NOTE: should never occur due to resource id check above
console.warn(
`Invariant: received answer ${JSON.stringify(answer)} that is not type === TXT despite resource === 16 check!`,
);
break;
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Making exceptions based on the plugin makes me think we could inject the new logic behind an interface from a particular plugin. Meaning, the subgraph plugin would implement a no-op for that interface, and both basenames and lineanames would implement the interface with the exact logic you proposed above.

Thoughts on that approach?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

now that we have the tooling to confirm that subgraph compatibility isn't affected by changes to the shared handlers, i'm less worried about including per-plugin logic here

Comment on lines 407 to 445
// subgraph ignores this event
if (pluginName === PluginName.Subgraph) return;

const { node, name, resource } = event.args;

// we only index TXT records (resource id 16)
if (resource !== 16) return;

// parse the record's name, which is the key of the DNS record
const [, recordName] = decodeDNSPacketBytes(hexToBytes(name));

// invariant: recordName is always available and parsed correctly
if (!recordName) throw new Error(`Invalid DNSPacket, cannot parse name '${name}'.`);

// relevant keys end with .ens
if (!recordName.endsWith(".ens")) return;

// trim the .ens off the end to match ENS record naming
const key = recordName.slice(0, -4);

const id = makeResolverId(event.log.address, node);
const resolver = await upsertResolver(context, {
id,
domainId: node,
address: event.log.address,
});

// remove relevant key
await context.db
.update(schema.resolver, { id })
.set({ texts: (resolver.texts ?? []).filter((text) => text !== key) });

// log ResolverEvent
await context.db.insert(schema.textChanged).values({
...sharedEventValues(context.network.chainId, event),
resolverId: id,
key,
value: null,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Making exceptions based on the plugin makes me think we could inject the new logic behind an interface from a particular plugin. Meaning, the subgraph plugin would implement a no-op for that interface, and both basenames and lineanames would implement the interface with the exact logic you proposed above.

Thoughts on that approach?

Copy link
Contributor

@tk-o tk-o left a comment

Choose a reason for hiding this comment

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

Awesome update, @shrugs & @0xBison. Merge when ready 🚀

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.

3DNS Text Records Not showing up?

4 participants