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

feat: universalresolver v3 #350

Draft
wants to merge 2 commits into
base: staging
Choose a base branch
from
Draft

feat: universalresolver v3 #350

wants to merge 2 commits into from

Conversation

TateB
Copy link
Contributor

@TateB TateB commented May 6, 2024

No description provided.

@adraffy
Copy link

adraffy commented Jun 6, 2024

  • Are you removing the resolve(bytes,bytes[]) API?
  • Should it still single call?
  • Should the inner form be considered too?resolve(name, abi.encodeCall(multicall, (calls)))
    • Outer: m([f(A),f(B),...]) vs. Inner: f(m([A,B,...]))
    • Since servers potentially need to upgrade, adding support is pretty simple
    • UR2 could dismantle the call when unsupported, and wrap it in abi.encode() on return, and let it pass through unmodified (automatic if single calls are allowed)
    • This enables efficient names[] × calls[] for names w/same resolver
    • I've found this useful but I don't know if anyone else does 😆️

I think this will be a good upgrade!

@TateB
Copy link
Contributor Author

TateB commented Jun 6, 2024

  • Are you removing the resolve(bytes,bytes[]) API?

yes

  • Should it still single call?

for sake of simplicity, i think not
single call optimisation isn't really that useful since a single call isn't going to use that much gas anyway

  • Should the inner form be considered too?resolve(name, abi.encodeCall(multicall, (calls)))

this is what replaces the resolve(bytes,bytes[]) api, already implemented - unless i'm misreading your comment

@adraffy
Copy link

adraffy commented Jun 6, 2024

Consider:

// (1)
multicall([
    resolve("a.eth", addr(60)),
    resolve("a.eth", text("avatar")),
    resolve("b.eth", addr(60))
])

// (2)
resolve("a.eth", multicall([addr(60), text("avatar")]))

// (3)
multicall([
    resolve("a.eth", multicall([addr(60), text("avatar")]))
    resolve("b.eth", multicall([addr(60), text("avatar")]))
])

// (4)
multi(["a.eth", "b.eth"], [addr(60), text("avatar")])

I forgot to mention (4). It's more of a power-user feature. I never implemented it as it can be accomplished with more verbose (1) or (3). Included for example.


For my implementations, I've been doing something like:

if (resolver.isRaffyResolver) {
    await contact.resolve(name, abi.encodeCall('multicall', [calls]), {enableCcipRead: true});
    // effectively same as:
    // await contract.multicall(calls.map(c => abi.encodeCall('resolve', (name, c))), {enableCcipRead: true); 
} else if (resolver.isExtendedResolver) {
    try {
        // optimistically try resolve(multicall)
        await contact.resolve(name, abi.encodeCall('multicall', [calls]), {enableCcipRead: true});
    } catch {
        // do individual calls 
        await Promise.all(calls.map(c => resolver.staticCall(c, {enableCcipRead: true})));
    }
} else {
    await Promise.all(calls.map(c => provider.call({to: resolver, data: c})));
    // effectively same as: 
    // multicall3.tryAggregate(false, calls.map(c => [resolver, c]));
    // but avoids issues with calling non-existent functions on old contracts
} 

I couldn't decide between the two variants, so I implemented and used both.

  • resolve(multicall) is nice because it can use existing callback infrastructure.
  • multicall(resolve) is nice because other CCIP-read apps can benefit from the same infrastructure.

From what I understand, contracts that don't implement supportsInterface(Multicall|MulticallSimple) need redeployed to return true for that case.

Contracts with an existing multicall() (but for write purposes) would be functionally different from a multicall() that would revert OffchainLookup. It's not clear how you add support for both types under the same signature. For example, TOR has both multicall() for writes and resolve(multicall) for reads.

For example, if ethers/viem want a native solution, it would be significantly easier for them to use resolve(multicall).

@TateB
Copy link
Contributor Author

TateB commented Jun 26, 2024

i started writing a long detailed explanation of my thoughts but i ended up concluding we should probably just not care about any extra latency added by using resolve(multicall) for pre-existing resolvers because we can just get all ccip-read server hosts to upgrade (or close to all).

only issue is release timing and how fast people would be able to update their servers, and also how many resolver devs will complain to me when their resolution is slow in-app suddenly (but i'll cross that bridge when i get to it)

i'll make the necessary changes and see how it goes

@adraffy
Copy link

adraffy commented Jul 16, 2024

I missed this—

I agree getting CCIP servers to upgrade to supporting generic multicall() is great, and then multicall(resolve) comes for free, but resolve(multicall) is significantly easier to implement since all of the current infrastructure knows how to pass around opaque resolve(name, bytes).

My point is that both forms are useful and synergize together.

I think resolve(multicall) is the right choice for ENS because it works out of the box with existing resolver contracts, even though it can only read multiple records of a single name.

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.

2 participants