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

Support setAddr for L2 resolvers #83

Open
tk-o opened this issue Jan 27, 2025 · 7 comments
Open

Support setAddr for L2 resolvers #83

tk-o opened this issue Jan 27, 2025 · 7 comments
Labels
enhancement New feature or request ensnode ENSNode related v2+ For ENSNode v2+
Milestone

Comments

@tk-o
Copy link
Contributor

tk-o commented Jan 27, 2025

Updating resolved address for a subname under linea.eth is not changing the state of resolvers data in ENSNode.

Context

Linea’s PublicResolver has it’s setAddr method, that DOES NOT always emit an event:

https://github.com/Consensys/linea-ens/blob/727f5f509591056ed3ede8eb835039801087f097/packages/linea-ens-contracts/contracts/resolvers/profiles/AddrResolver.sol#L45-L55

If coinType param is different than 60. In my case, it’s 2147542792 (the ENSIP-11 value for Linea’s chain ID). However, the AddressChanged event is emitted regardless.

What can we do about that?

@tk-o
Copy link
Contributor Author

tk-o commented Jan 27, 2025

We do handle AddrChanged event and AddressChanged a bit differently:

export async function handleAddrChanged({
context,
event,
}: {
context: Context;
event: {
args: { node: Hex; a: Hex };
log: Log;
};
}) {
const { a: address, node } = event.args;
await upsertAccount(context, address);
const id = makeResolverId(event.log.address, node);
await upsertResolver(context, {
id,
domainId: node,
address: event.log.address,
addrId: address,
});
// materialize the resolved addr to the domain iff this resolver is active
const domain = await context.db.find(schema.domain, { id: node });
if (domain?.resolverId === id) {
await context.db.update(schema.domain, { id: node }).set({ resolvedAddressId: address });
}
// TODO: log ResolverEvent
}

export async function handleAddressChanged({
context,
event,
}: {
context: Context;
event: {
args: { node: Hex; coinType: bigint; newAddress: Hex };
log: Log;
};
}) {
const { node, coinType, newAddress } = event.args;
await upsertAccount(context, newAddress);
const id = makeResolverId(event.log.address, node);
const resolver = await upsertResolver(context, {
id,
domainId: node,
address: event.log.address,
});
// upsert the new coinType
await context.db
.update(schema.resolver, { id })
.set({ coinTypes: uniq([...(resolver.coinTypes ?? []), coinType]) });
// TODO: log ResolverEvent
}

@tk-o tk-o changed the title Linea's PublicResolver does not always emit AddrChanged event Linea's PublicResolver updates not reflected in indexed data Jan 27, 2025
@lightwalker-eth
Copy link
Member

Here's some additional background info that may be helpful in investigating this issue:

  1. ENSIP-1. In particular check details in the linked section, including docs for the AddrChanged event: https://docs.ens.domains/ensip/1#contract-address-interface
  2. ENSIP-9 introduced the standard for resolver records to contain deposit addresses for chains that differ from the chain that the ENS root is operating on (ex: mainnet or a testnet). For example: Provide a bitcoin deposit address or a deposit address on Base or Linea, etc... https://docs.ens.domains/ensip/9

There's a number of details here that need special consideration.

One detail that might help us a little: We don't need to index the values of any of these resolver records, only the keys. It seems the ENS Subgraph makes an exception to this though for the ETH deposit address? It will help to make this exception more formally defined. A more formal definition of how the ENS Subgraph is handling this seems key to informing the path we take for solving this in our base.eth and linea.eth plugins.

@tk-o
Copy link
Contributor Author

tk-o commented Jan 27, 2025

Modifying handleAddressChanged handler

  1. We should set the addrId param to newAddress when upserting resolver data to keep the resolvers state in sync.
  2. We should run the // materialize the resolved addr to the domain iff this resolver is active code part from the handleAddrChanged to keep the domains state in sync.

Thoughts on that, @shrugs & @lightwalker-eth?

@shrugs
Copy link
Contributor

shrugs commented Jan 27, 2025

from the subgraph's perspective a Resolver's addr field (and the materialized version on Domain.resolvedAddress) are exclusively for tracking the resolver's ETH coinType records — so in my opinion the current behavior is correct, at least from the perspective of the .eth plugin. the subgraph specifies that the address in Resolver.addr or Domain.resolvedAddress is "The current value of the 'addr' record for this resolver". it didn't consider multiple chains here, so the user experience when setting a record for another chain's coinType is understandably unexpected.

in this situation it sounds like setting an address on linea with the linea coinType doesn't place that address in Resolver.addr (or Domain.resolvedAddress) which does seem 'incorrect' from the user's perspective (though technically correct from the subgraph's limited perspective)

it seems to me that in the case of non-.eth-plugins we want to, (iff the coinType is that chain's coinType) record the value of that record into Resolver.addr and Domain.resolvedAddress . we should not do this for any other record emitted in AddressChanged. does that seem like the correct behavior?

that said, @lightwalker-eth perhaps you can chime in here: what does it mean if example.linea.eth sets 1) their ETH coinType record to 0xABCD and their LINEA coinType to 0xXYZ on this Linea resolver? my assumption is that it means that someone sending tokens to example.linea.eth on mainnet will resolve 0xABCD and send to 0xABCD on mainnet. but if they were sending tokens to example.linea.eth on Linea, however, would they look up that domain's address using the LINEA cointype? if that coinType doesn't exist, do they fall back to the ETH coinType record? if someone queries our indexer in order to find "The current value of the 'addr' record for this resolver", what should our api tell them in the case of example.linea.eth?


sidenote: the real issue here is once again our inherited subgraph assumptions and a schema that is not multichain-aware or multi-registry-aware. i look forward to redesigning this schema with y'all for v2

@tk-o
Copy link
Contributor Author

tk-o commented Jan 27, 2025

Thanks @shrugs, adding extra logic "(iff the coinType is that chain's coinType"™ sounds good to me. Also, feels a bit like V2 scope 🙃

@lightwalker-eth lightwalker-eth added the ensnode ENSNode related label Jan 31, 2025
@lightwalker-eth lightwalker-eth moved this to Todo in ENSNode Jan 31, 2025
@lightwalker-eth
Copy link
Member

@tk-o @shrugs Hey thanks for your follow ups on this.

A few quick ideas:

  1. Agreed that this looks like an issue we should come back to in v2. It's difficult to handle these cases in a mature way while also maintaining ENS Subgraph backwards compatibility.
  2. Current behaviour in v1 seems correct for ENS Subgraph backwards compatibility based on my understanding of your messages above.

@lightwalker-eth lightwalker-eth changed the title Linea's PublicResolver updates not reflected in indexed data Support setAddr for L2 resolvers Feb 3, 2025
@lightwalker-eth lightwalker-eth added the enhancement New feature or request label Feb 3, 2025
@lightwalker-eth lightwalker-eth moved this from Todo to Backlog in ENSNode Feb 3, 2025
@lightwalker-eth lightwalker-eth added this to the v2+ milestone Feb 3, 2025
@lightwalker-eth lightwalker-eth added the v2+ For ENSNode v2+ label Feb 3, 2025
@lightwalker-eth
Copy link
Member

Modified this issue to reclassify it from a "bug" to an "enhancement" for v2+ and moved it into our backlog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ensnode ENSNode related v2+ For ENSNode v2+
Projects
Status: Backlog
Development

No branches or pull requests

3 participants