Skip to content
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

Callbacks spec update #222

Merged
merged 6 commits into from
Feb 6, 2024
Merged

Callbacks spec update #222

merged 6 commits into from
Feb 6, 2024

Conversation

phoebe-lew
Copy link
Contributor

closes #167

specs/http-api/README.md Outdated Show resolved Hide resolved
If `replyTo` is present, a PFI will send any/all new messages for a given exchange to the supplied URI. This makes the URI scoped to each exchange, allowing the caller to specify a different URI per exchange if they so wish.

If `replyTo` is _not_ present, the caller will have to poll the PFI for the exchange in question to receive new messages.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a general q about replyTo - does the URI change depending on whether Alice is talking tbdex via a self-custodial vs custodial wallet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The URI can actually be different per exchange. In general, no difference in terms of how the callback is used if coming from a self-custodied vs custodial wallet though.


### Endpoint
`POST /exchanges/:exchange_id/rfq`
`POST /exchanges/:exchange_id`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could take it a step further. exchange_id is already in the RFQ. It's superfluous in the URL.

Suggested change
`POST /exchanges/:exchange_id`
`POST /exchanges`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to this - could this actually be applied to all our POST /exchanges/xyz endpoints, since all tbdex message metadata contain exchangeId?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a fan of removing it, @mistermoe any opposition?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to open a separate issue for this because it'll require refactoring httpclient/httpserver in JS/KT outside the context of implementing callbacks

@jiyoontbd jiyoontbd merged commit 677f2e0 into main Feb 6, 2024
@jiyoontbd jiyoontbd deleted the plew.callbacks-spec branch February 6, 2024 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce Webhooks / Callbacks
3 participants