-
Notifications
You must be signed in to change notification settings - Fork 0
feat: directed identity #26
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
base: main
Are you sure you want to change the base?
Conversation
| The client instance provides its public key directly in the request, eliminating the need for the AS to fetch it from a wallet address. This approach enhances privacy by not requiring the client to expose a persistent wallet address identifier. | ||
|
|
||
| If sending a grant initiation request that requires RO interaction, the wallet address MUST serve necessary client display information. | ||
| oneOf: |
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.
Before proceeding, I would suggest pulling the updated spec in one of the SDKs and inspect the generated types.
Previous discussions:
- feat: update outgoing payment limits spec open-payments#564 (review)
- feat: update outgoing payment limits spec open-payments#564 (review)
Theoretically, the use of oneOf should be correct in this case since all properties are exclusive (not present in multiple schemas), but a sanity check would be helpful since we had our fights with the generation for mutually exclusive types.
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.
Mixed results (see typegen changes here). I will look for a better way.
This top level oneOf looks fine. How im doing the mutually exclusive object properties (jwk, wallet address) seems like it might be causing some weirdness. In general I'd prefer not to write the spec to satisfy the generator. But in this case it makes me think there is a simpler way even from an open-api perspective.
Here is what it gives us (comments stripped for brevity):
client: string | ({
walletAddress?: string;
jwk?: components["schemas"]["json-web-key"];
} & (unknown | unknown));The issues:
1.& (unknown | unknown) is weird, but also completely benign (equivalent to just string | { ... }). Guessing it's driven by using oneOf for the required properties.
2. both properties of the object are optional. could include none or both, which isn't quite right.
What I would expect, ideally would be more like:
type Client =
| string
| { walletAddress: string; jwk?: never }
| { jwk: JWK; walletAddress?: never };Working on it.
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.
Also, I'm aware we have some sort of XOR ts utils in the node client. We might have to massage the types a bit and that's fine. In any case, this part is striking me as perhaps not the best way to represent mutual exclusivity (although I think it should still be valid):
oneOf:
- required:
- walletAddress
- required:
- jwkThere 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.
This should be better:
Spec:
oneOf:
- type: string
format: uri
description: >
DEPRECATED: This string format of the client wallet address is maintained
only for backwards compatibility. Migrate to the object form.
deprecated: true
- type: object
required: [walletAddress]
properties:
walletAddress:
type: string
format: uri
additionalProperties: false
- type: object
required: [jwk]
properties:
jwk:
$ref: '#/components/schemas/json-web-key'
additionalProperties: falseGenerated Types:
client: string | {
walletAddress: string;
} | {
jwk: components["schemas"]["json-web-key"];
};I also tried to include this, thinking maybe it would set jwk: never but it did not. The generated types were the same as above:
not:
required: [jwk]Then in the client types we can do this:
export type Client = XOR<{ walletAddress: string }, { jwk: JWK }> | string;
const a: Client = "deprecated-string"; // ok
const b: Client = { walletAddress }; // ok
const c: Client = { jwk }; // ok
const d: Client = { walletAddress, jwk }; // type errorEDIT:
Although that technically doesnt use the generated type (just JWK). We could derive it from ASOperations['post-request']['requestBody']['content']['application/json']['client'] but that feels like we are really trying to bend over backwards. In fact it seems easier to forgo our XOR and just do:
type Client =
| string
| { walletAddress: string; jwk?: never }
| { jwk: JWK; walletAddress?: never };Co-authored-by: Radu-Cristian Popa <praducristian@gmail.com>
Co-authored-by: Radu-Cristian Popa <praducristian@gmail.com>
Co-authored-by: Radu-Cristian Popa <praducristian@gmail.com>
| The client instance provides its public key directly in the request, eliminating the need for the AS to fetch it from a wallet address. This approach enhances privacy by not requiring the client to expose a persistent wallet address identifier. The `jwk` property can only be used for non-interactive grant requests (i.e.: incoming payments). | ||
|
|
||
| If sending a grant initiation request that requires RO interaction, the wallet address MUST serve necessary client display information. | ||
| oneOf: |
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.
Go is cleaner with this new way as well.
New way:
type Client0 = string // for the deprecated string format
type Client1 struct {
WalletAddress string `json:"walletAddress"`
}
type Client2 struct {
Jwk JsonWebKey `json:"jwk"`
}Old way
type Client0 = string // for the deprecated string format
type Client1 struct {
Jwk *JsonWebKey `json:"jwk,omitempty"`
WalletAddress *string `json:"walletAddress,omitempty"`
union json.RawMessage
}
type Client10 = interface{}
type Client11 = interface{}
fixes: #16