nucypher / taco-web

🌮 A TypeScript client for TACo (Threshold Access Control)
https://docs.threshold.network/app-development/threshold-access-control-tac
GNU General Public License v3.0
14 stars 22 forks source link

Added condition ENS ownership #564

Open andresceballosm opened 1 month ago

andresceballosm commented 1 month ago

Type of PR:

Required reviews:

What this does:

Issues fixed/closed:

  • Fixes #512

Why it's needed:

Explain how this PR fits in the greater context of the NuCypher Network. E.g., if this PR address a nucypher/productdev issue, let reviewers know!

Notes for reviewers:

What should reviewers focus on? Is there a particular commit/function/section of your PR that requires more attention from reviewers?

theref commented 1 month ago

This is a great start, thank you! I've got 2 comments :)

  1. With the current design, you're converting the ens name to an address using viem. This happens at condition creation time. The problem here is that ens ownership can change, and the condition won't stay in line with the ownership.

  2. This implementation would also require us to build an equivalent ensOwnership condition in nucypher/nucypher. Would it be possible to have ensOwnership be a wrapper of ContractCondition?

The condition is still defined exactly as you specified:

     const ensCondition = new conditions.ensOwnership.EnsAddressOwnershipCondition({
        ensName: 'andrestest.eth',
        signer,
        domain
      });

but behind the scenes it would create a contract condition that reads from the ens registry. This would fix both of my comments. @KPrasch thoughts?

piotr-roslaniec commented 1 month ago

Please see this issue comment https://github.com/nucypher/taco-web/issues/104#issuecomment-2079254236 for the context on ENS resolution. TLDR, it's a multi-step process that requires some development in nucypher/nucypher and we can't support it out-of-the-box using ContractConditions at the moment. Maybe with sequential conditions?