-
-
Notifications
You must be signed in to change notification settings - Fork 40
fix: createx402DelegationProvider should resolve redeemer addresses as interesection of facilitatorAddresses and redeemer.addresses
#256
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
Changes from 2 commits
e9c8f8c
fc0351e
7422f30
da3400b
4e1c90b
9c3a2a0
a1e3c41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,7 +34,7 @@ type EnsureRedeemerSufficientlyConstrainedParams = { | |
| redeemerEnforcer: Hex; | ||
| caveats: Caveat[]; | ||
| existingDelegations: Delegation[]; | ||
| redeemerAddresses: Hex[]; | ||
| redeemerAddresses: Address[] | undefined; | ||
| requireRedeemers: boolean; | ||
| }; | ||
|
|
||
|
|
@@ -58,6 +58,14 @@ type EnsureExpirySufficientlyConstrainedParams = { | |
| expirySeconds: number; | ||
| }; | ||
|
|
||
| /** | ||
| * Inputs for resolving allowed redeemer addresses. | ||
| */ | ||
| type ResolveRedeemerAddressesParams = { | ||
| facilitatorAddresses?: Address[]; | ||
| redeemerAddresses?: Address[]; | ||
| }; | ||
|
|
||
| /** | ||
| * Resolved context required to build and sign an x402 delegation. | ||
| */ | ||
|
|
@@ -236,6 +244,42 @@ export const ensureExpirySufficientlyConstrained = ({ | |
| return [...caveats, timestampCaveat]; | ||
| }; | ||
|
|
||
| /** | ||
| * Resolves the allowed redeemer addresses from the union of facilitator specified and caller specified redeemer addresses. | ||
| * If either are undefined, the other is returned. | ||
| * | ||
| * @param options - Redeemer address inputs. | ||
| * @param options.facilitatorAddresses - Optional facilitator addresses from the PaymentRequirements. | ||
| * @param options.redeemerAddresses - Optional redeemer addresses from the RedeemersConfig. | ||
| * @returns The allowed redeemer addresses. | ||
| */ | ||
| const resolveRedeemerAddresses = ({ | ||
| facilitatorAddresses, | ||
| redeemerAddresses, | ||
| }: ResolveRedeemerAddressesParams): Address[] | undefined => { | ||
| if (!facilitatorAddresses) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a payment server sends
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the behaviour is correct. I've refactored slightly, added some comments, and clarified the error message to "No valid redeemer addresses were resolved. If both If either extra.facilitatorAddresses or redeemers.addresses is specified it implies a constraint (if undefined, it implies no constraint). If the value is either [] or the intersection of the two values is [] it implies a constraint that cannot be satisfied, hence the error. If we treat [] as no constraint, it's error prone - ["0xaddress1","0xaddress2"] means either address can redeem, ["oxaddress1"] means only that address can redeem, [] means anyone can |
||
| if (!redeemerAddresses) { | ||
| return undefined; | ||
| } | ||
| return redeemerAddresses; | ||
| } | ||
|
|
||
| if (!redeemerAddresses) { | ||
| return facilitatorAddresses; | ||
|
jeffsmale90 marked this conversation as resolved.
|
||
| } | ||
|
|
||
| const normalizedFacilitatorAddresses = | ||
| facilitatorAddresses.map(normalizeAddress); | ||
|
|
||
| const normalizedRedeemerAddressSet = new Set( | ||
| redeemerAddresses.map(normalizeAddress), | ||
| ); | ||
|
|
||
| return normalizedFacilitatorAddresses.filter((address) => | ||
| normalizedRedeemerAddressSet.has(address), | ||
| ); | ||
| }; | ||
|
|
||
| /** | ||
| * Ensures caveats include a sufficiently strict redeemer constraint. | ||
| * | ||
|
|
@@ -258,9 +302,9 @@ export const ensureRedeemerSufficientlyConstrained = ({ | |
| redeemerAddresses, | ||
| requireRedeemers, | ||
| }: EnsureRedeemerSufficientlyConstrainedParams): Caveat[] => { | ||
| const redeemerAddressNormalized = normalizeAddress(redeemerEnforcer); | ||
| const redeemerEnforcerNormalized = normalizeAddress(redeemerEnforcer); | ||
|
|
||
| if (redeemerAddresses.length === 0) { | ||
| if (!redeemerAddresses || redeemerAddresses.length === 0) { | ||
| if (!requireRedeemers) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you clarify what this means exactly? From my understanding, setting I’m checking this because the current code handles an empty intersection the same as when no redeemer sources exist. |
||
| return caveats; | ||
| } | ||
|
|
@@ -269,7 +313,7 @@ export const ensureRedeemerSufficientlyConstrained = ({ | |
| caveats, | ||
| existingDelegations, | ||
| ({ enforcer }) => | ||
| normalizeAddress(enforcer) === redeemerAddressNormalized, | ||
| normalizeAddress(enforcer) === redeemerEnforcerNormalized, | ||
| ); | ||
|
|
||
| if (!hasExistingRedeemerCaveat) { | ||
|
|
@@ -288,7 +332,7 @@ export const ensureRedeemerSufficientlyConstrained = ({ | |
| caveats, | ||
| existingDelegations, | ||
| (caveat) => { | ||
| if (normalizeAddress(caveat.enforcer) !== redeemerAddressNormalized) { | ||
| if (normalizeAddress(caveat.enforcer) !== redeemerEnforcerNormalized) { | ||
| return false; | ||
| } | ||
|
|
||
|
|
@@ -420,17 +464,16 @@ export const resolvex402DelegationCaveats = ({ | |
| isScopeOptional: true, | ||
| }); | ||
|
|
||
| const allRedeemerAddresses = new Set( | ||
| [...(facilitatorAddresses ?? []), ...(redeemerAddresses ?? [])].map( | ||
| normalizeAddress, | ||
| ), | ||
| ); | ||
| const resolvedRedeemerAddresses = resolveRedeemerAddresses({ | ||
| facilitatorAddresses, | ||
| redeemerAddresses, | ||
| }); | ||
|
|
||
| const caveatsWithRedeemer = ensureRedeemerSufficientlyConstrained({ | ||
| redeemerEnforcer, | ||
| caveats: initialCaveats, | ||
| existingDelegations, | ||
| redeemerAddresses: Array.from(allRedeemerAddresses), | ||
| redeemerAddresses: resolvedRedeemerAddresses, | ||
| requireRedeemers, | ||
| }); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's
intersection, notunionThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it sure is!