Decrypt API Rework #560
Decrypt API Rework #560
Conversation
✅ Deploy Preview for taco-demo canceled.
|
✅ Deploy Preview for taco-nft-demo canceled.
|
|
Replaces #554 which was auto-closed once |
…et the ritual so early; obtaining it later saves us from passing additional parameters.
Allow Context to be created with a condition, and subsequently populated with authProviders and customParameters. Make :userAddressExternalEIP4361 a reserved context variable because the context will use the provided authProviders to populate its (and :userAddress) values in the overall context used for decryption.
… replaces authProvider and customParmeters since those are now encompassed in the context itself.
…common elements for both ei4361 provider and single sign-on provider. Move user address context variable for external eip4361 to taco-auth as well. Update relevant imports.
…s the only thing using it.
…d of directly passing auth provider. Expose USER_ADDRESS_PARAM* constants from taco for now.
…viding auth provider.
…roviding authProvider.
derekpierre
force-pushed
the
api-rework
branch
from
f20c260
to
8c30bc4
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #560 +/- ##
===========================================
+ Coverage 23.12% 79.10% +55.98%
===========================================
Files 62 66 +4
Lines 10175 6801 -3374
Branches 260 300 +40
===========================================
+ Hits 2353 5380 +3027
+ Misses 7763 1377 -6386
+ Partials 59 44 -15 ☔ View full report in Codecov by Sentry. |
… then pass the client to calls later in the stack instead of passing around the porterUri(s).
…related to context parameters.
There was a problem hiding this comment.
requestedContextParameters = conditionContext.requestedContextParameters // list of context variables required for underlying condition // Devs can loop over the list and populate context variables appropriately.
Could we get an example of iterating over this?
Otherwise it looks great!!!
… on ConditionContext.
Great idea - added an illustrative optional example to the nodejs example - see 1e528aa. |
| conditionContext.addAuthProvider(USER_ADDRESS_PARAM_DEFAULT, authProvider); | ||
|
|
||
| // illustrative optional example of checking what context parameters are required | ||
| // unnecessary if you already know what the condition contains |
| conditionContext.requestedContextParameters.has(USER_ADDRESS_PARAM_DEFAULT) | ||
| ) { | ||
| const authProvider = new EIP4361AuthProvider( | ||
| provider, | ||
| consumerSigner, | ||
| siweParams, | ||
| ); | ||
| conditionContext.addAuthProvider(USER_ADDRESS_PARAM_DEFAULT, authProvider); | ||
| } | ||
| return decrypt(provider, domain, messageKit, conditionContext); |
There was a problem hiding this comment.
So this avoids having to hardcode context parameters – i.e. the app doesn't need to know the conditions in advance in order to match them to the right authentication method? That's pretty amazing!
One thing I'm wondering is – what resource is being queried here? Do we need to create a public resource that maps conditions to context parameters? Or is this a list that developers will create for their own domain?
There was a problem hiding this comment.
So this avoids having to hardcode context parameters – i.e. the app doesn't need to know the conditions in advance in order to match them to the right authentication method? That's pretty amazing!
Indeed. This is the case where an application doesn't just use the same condition every time, but perhaps some permutations of a set of conditions, each with varying context parameters. In this way the dev can use logic to get the set of expected parameters and populate them based on their own logic.
They could have dynamic logic like the following:
if (requestedContextParmeters.has(":tokenId")) {
// user subjected to condition 1 which uses ":tokenId" custom context parameter
const value = ...
conditionContext.addCustomContextParameters({":tokenId": value});
}
if (requestedContextParmeters.has(":otherParam")) {
// user subjected to condition 2 which uses ":otherParam" custom context parameter
const otherValue = ...
conditionContext.addCustomContextParameters({":otherParam": otherValue});
}
if (requestedContextParmeters.has(":userAddress")) {
// user subjected to condition with taco SIWE
const authProvider = new EIP4361AuthProvider(...);
conditionContext.addAuthProvider(":userAddress", auth provider);
}
if (requestedContextParmeters.has(":userAddressEIP4361")) {
// user subjected to condition with single-sign on SIWE
const authProvider = SingleSignOnEIP4361AuthProvider.fromExistingSiweInfo(...)
conditionContext.addAuthProvider(":userAddressEIP4361", authProvider);
}
...One thing I'm wondering is – what resource is being queried here?
Assuming I correctly understand the question, the ConditionContext looks at the underlying condition in the ThresholdMessageKit (encrypted data), and then returns all the context parameters in the underlying condition (requestedContextParameters property). It is then up to the app to populate the values appropriately. How they do that is up to them - we are not being prescriptive.
Do we need to create a public resource that maps conditions to context parameters? Or is this a list that developers will create for their own domain?
Interesting. To me, it's the latter. Devs can decide how they want to get/generate the values to use for conditions.
IMHO, on the decryption side, we should endeavour for apps not to have to deal with the underlying condition object(string) themselves. They can if they want to, of course, but they shouldn't really have to. It's part of the reason I made the ConditionContext object take a ThresholdMessageKit and not the condition object contained within the ThresholdMessageKit.
With something like a public mapping resource (condition string/type -> context parameters...?), it may be easy for simple conditions eg. BalanceOf, or OwnsNFT, but it's tough cover all cases with a mapping because our conditions are extensible, which is intentional for breadth of functionality. Devs can use whatever custom context parameters they want, and they can also use :userAddressExternalEIP4361 (single sign on SIWE) OR :userAddress (taco SIWE) if they like...it would be tough to cover all cases. Even with the mapping, they'll still need to know/determine what value to use for the context parameters - so they are kind of back to something like the 'if' statements above.
Type of PR:
Required reviews:
What this does:
ConditionContextinstead of splitting up params for authProvider and customParameters.ConditionContextto be populated by developers with authProviders and customParameters before being used for decryptionThe updated use of the code will look like the following:
Add any required auth provider(s):
:userAddress, if necessary::userAddressExternalEIP4361, if necessary:Add any required custom context variables, if necessary:
Additionally, developers can also better query what parameters are needed for a condition in a message kit. This helps when apps have various permutations of conditions that they use, which include different parameters:
Issues fixed/closed:
Why it's needed:
Notes for reviewers: