EIP1271 Support

[derekpierre]

Type of PR:

• Bugfix
• Feature
• Documentation
• Other

Required reviews:

• 1
• 2
• 3

Co-authored by @theref

What this does:
Based over #616 .

• Support for EIP1271 authentication
• Allow AuthSignatures that can be used with :userAddress to be EIP4361, EIP4361 Single Sign On, EIP1271 (Smart contract wallet). Before we only allowed EIP4361 (non-single sign-on but this was too restrictive)

Issues fixed/closed:
Related to nucypher/nucypher#3576.

Follow-up issue: #620

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?

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (epic-v0.6.x@4f713cf). Learn more about missing BASE report.

Additional details and impacted files

@@              Coverage Diff               @@
##             epic-v0.6.x     #617   +/-   ##
==============================================
  Coverage               ?   89.34%           
==============================================
  Files                  ?       76           
  Lines                  ?     6671           
  Branches               ?      367           
==============================================
  Hits                   ?     5960           
  Misses                 ?      675           
  Partials               ?       36           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[manumonti]

Very complete tests, great! 🙌

I just left a small question.

[manumonti]

Maybe I'm missing something, but the function name (getOrCREATEAuthSignature()) makes me think that the function is able to create a new EIP1271AuthProvider object, but this is not correct, right? This function will be called only for already created objects. Should we rename it?

[derekpierre]

makes me think that the function is able to create a new EIP1271AuthProvider object

It's the flip. AuthProviders implement the function getOrCreateAuthSignature(). The AuthProvider is specified as a parameter to the ConditionContext, which then uses it to generate an AuthSignature via the getOrCreateAuthSignature() function.

So for EIP1271 as an example the code looks like the following:

const authProvider = new EIP1271AuthProvider(...);
conditionContext.addAuthProvider(':userAddressEIP1271', authProvider)
...
const decryptedMessage = await decrypt(
      ...
      messageKit,
      conditionContext,
    );

This conditionContext can then provide the auth signature data needed for proving ownership using the auth provider - see https://github.com/derekpierre/taco-web/blob/eip1271-support/packages/taco/src/conditions/context/context.ts#L116.

This function will be called only for already created objects. Should we rename it?

The getOrCreateAuthSignature() was named (perhaps incorrectly so) as such because it may not actually create the object because the auth signature object could be cached. For example, EIP4361 caches the auth signature after first creating it for 2 hours and doesn't need to create it again within that 2 hours i.e. subsequet calls to getOrCreateAuthSignature simply uses the cached object if called again within that 2-hour window. That being said, perhaps the name is too tied to implementation. we can always rename it to getAuthSignature() or something else ... although let's do that in a different PR ...?

[manumonti]

Got it!
|
Yes, something to consider, but in a different PR.

[derekpierre]

Filed #621

[derekpierre]

Found a separate issue. Moving to draft.

[derekpierre]

Found a separate issue. Moving to draft.

Issue resolved.

Will be resolved in separate PR

Need to re-review updated changes

[manumonti]

Great! LGTM 🙌