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

Introduce Webhooks / Callbacks #167

Closed
mistermoe opened this issue Oct 9, 2023 · 22 comments · Fixed by #222, TBD54566975/tbdex-js#142 or TBD54566975/tbdex-kt#140
Closed

Introduce Webhooks / Callbacks #167

mistermoe opened this issue Oct 9, 2023 · 22 comments · Fixed by #222, TBD54566975/tbdex-js#142 or TBD54566975/tbdex-kt#140
Assignees
Milestone

Comments

@mistermoe
Copy link
Member

Motivation

Currently, in order to receive new messages for a given exchange, Alice must poll the respective PFI's http api. A handful of people have brought up adding webhook / callback support.

Two approaches came to mind that may work.

1. replyTo property

We could consider adding a replyTo property to rfq.metadata or more generally message.metadata. replyTo is a fully qualified URI which effectively acts as a callback URL / webhook. The value of replyTo could either be a DID or just a URL.

if replyTo is present, a PFI would send any/all new messages for a given exchange to the supplied URL

2. DID service endpoint

Alternatively, this could happen implicitly by resolving the sender's DID and looking for a predefined service endpoint type. if the predefined service endpoint is present, a PFI would send any/all new messages for a given exchange to that service endpoint.


In order to support either approach, we'll need to specify how the webhook url works. e.g.

  • which http method?
  • how does authn work?
    • could happen implicitly since every tbdex message is signed and alice presumably has record of the exchange any incoming message is in response to
@michaelneale
Copy link
Contributor

michaelneale commented Oct 13, 2023

the replyTo feels more self documenting as it invites you to consider it as you construct your first message (and the replyTo could itself be a did I guess?). but not basing that on any super deep thinking.

@bradleydwyer
Copy link

We should while thinking through this design consider compatibility with DWMsgs/DWNs. HTTP URLs may couple tbDEX to HTTP.

@michaelneale
Copy link
Contributor

michaelneale commented Oct 13, 2023

good point @bradleydwyer - as if its an "alice" sending a message, her replyto/DID could point to a DWN to receive messages in a self-custodial situation.

edit: even if they aren't http the terminology "webhook" is probably ok if used (replyto is more explicit in that regard) come to think of it.

@diehuxx
Copy link
Contributor

diehuxx commented Jan 2, 2024

3. Move callback into HTTP API

Another approach is to move the callback URL up an abstraction into the HTTP API. For ex, we could update the POST /exchanges/:exchange_id/rfq endpoint request body to take both the RFQ itself plus a replyTo field.

Putting the callback URL into the HTTP API instead of the TBDex message has a couple benefits:

  • We sidestep the risk of tying TBDex to HTTP. While supporting HTTP is important, we want to leave the door open to other systems like DWNs as @bradleydwyer mentioned.
  • The callback URL might be sensitive information. In the recent conversation about private fields, it's clear that supporting sensitive information inside TBDex messages is (while sometimes necessary) a hassle. If we can avoid that can of worms without consequence, we should.

Against DID Service Endpoint

My least favorite option is the DID service endpoint approach because:

  • Updating a DID doc can be a heavy operation
  • Alice may want to use different callback endpoints for different TBDex message exchanges
  • Alice may want to keep her callback URL private

@mistermoe
Copy link
Member Author

Alice may want to keep her callback URL private

good point!

@mistermoe
Copy link
Member Author

@diehuxx i imagine we'd want something like a TbdexRequest then yeah? an object that contains message and in this case replyTo?

@michaelneale
Copy link
Contributor

if the replyTo is itself a did URL, then it doesn't have to be coupled to http? then it could remain(?) in the message. I'm not sure about the callback url containing sensitive information, feels like it wouldn't be optimal if it was assumed to be sensitive.

@diehuxx
Copy link
Contributor

diehuxx commented Jan 5, 2024

i imagine we'd want something like a TbdexRequest then yeah? an object that contains message and in this case replyTo?

Exactly. Though, I'm not sure how much we care about keeping the notion of TbdexRequest standard across different HTTP endpoints. My gut instinct is it doesn't matter as much. What's more important is keeping each endpoint's payload backwards compatible over time.

@diehuxx
Copy link
Contributor

diehuxx commented Jan 5, 2024

if the replyTo is itself a did URL, then it doesn't have to be coupled to http?

I'm not sure if that would work with DWNs currently. Even if that does work, there's transports beyond http and did url that would be excluded.

then it could remain(?) in the message.

What's the benefit of doing so?

I'm not sure about the callback url containing sensitive information, feels like it wouldn't be optimal if it was assumed to be sensitive.

Not sure I understand what you mean by optimal?

@michaelneale
Copy link
Contributor

michaelneale commented Jan 5, 2024

@diehuxx yeah I might not really get the context here so feel free to ignore me!

the idea of using dids as they are universal (but then if the doc is heavy to update then that isn't ideal - but that may be an assumption, and a user can have dids for many purposes). Chanelling @csuwildcat here! (and it would mean you could have another did separate to sender with a service endpoint, but maybe that is too chonky of an idea!).

Remaining in the message is related to sensitive information: if there is sensitive information in the url, then isn't that about privacy and or trust, and in one of those cases it is better in the message than out of band? (again just being more general, not sure of specific case here).

@michaelneale
Copy link
Contributor

@diehuxx i think I get you now I read it again. If we want it to be “web” hook then pulling replyTo into http api like you said may be simpler and the right thing while still allowing service endpoints at some point in future?

@mistermoe
Copy link
Member Author

@diehuxx i'm on board.

technically speaking replyTo at the request level can support DID and vanilla URLs since we'll state the value of replyTo must be a valid URI.

this allows for people to leverage service + relativeRef DID Parameters like so: did:example:123?service=tbdex-callback&relativeRef=/callbacks/tbdex. cool thing here is that we don't have to be prescriptive at all about service ids or refs

at some point we can explore using replyTo to point to a DWN in the same manner described above though PFIs would need to explicitly support it via the DID Relative URLs section of the DWN spec here

@phoebe-lew
Copy link
Contributor

+1 for proposal 3, although I would suggest naming the request something like createExchangeRequest or exchangeRequest over tbdexRequest, since it will contain the thing that kicks off an exchange (the Rfq), and exchange-level data.

@mistermoe and I talked through specifying callbacks at an exchange level rather than at a DID level...in summary, it feels like there could be some potential extra flexibility with how wallets want to leverage the callback URL, for not much of a UX tradeoff (looking at a one off registerCallback request vs populating a field with the URI per exchange).

@diehuxx
Copy link
Contributor

diehuxx commented Jan 11, 2024

(looking at a one off registerCallback request vs populating a field with the URI per exchange).

@phoebe-lew Making sure I understand. No pushback, just clarifying. By this do you mean we could have a separate endpoint called registerCallback that takes exchange_id and callback URI? Sounds like this would allow submitRfq to keep its current request structure. If this is what you mean, I'm into it.

@phoebe-lew
Copy link
Contributor

phoebe-lew commented Jan 15, 2024

@diehuxx nop, let's just go with proposal 3 and introduce a createExchangeRequest (or other name) endpoint, which will effectively replace the existing data structure for the request to submit an RFQ. It will wrap the RFQ itself and a callback URL (optional).

The registerCallback option is generally scoped to whatever your system's concept of a tenant is (probably a DID in our case), but allowing the caller to scope it per exchange allows for more flexibility.

We can always change it in future if we decide it's better to register the callback against the DID.

@jiyoontbd
Copy link
Contributor

happy to implement option 3, but can someone tell me the difference between option 3 and option 1?

seems like both are suggesting that we add the replyTo field to the POST /exchanges/:exchange_id request body, except 1 suggests we put it in the metadata, and 3 suggests we put it in the RfqData. is that the only difference or am i missing something

@diehuxx
Copy link
Contributor

diehuxx commented Jan 17, 2024

@jiyoontbd Option 1 places replyTo in the TBDex Message (in the metadata) making it part of the protocol spec. Option 3 places replyTo in the HTTP request separate of the TBDex message, making it part of the http-api but not part of the underlying message protocol.

@jiyoontbd
Copy link
Contributor

@jiyoontbd Option 1 places replyTo in the TBDex Message (in the metadata) making it part of the protocol spec. Option 3 places replyTo in the HTTP request separate of the TBDex message, making it part of the http-api but not part of the underlying message protocol.

@diehuxx oooh, i think i am understanding where i've misunderstood. do you mean put replyTo as an http request header, and not inside RfqData in a tbdex message? or are you thinking of some other way in which to incorporate replyTo as part of the http request

@diehuxx
Copy link
Contributor

diehuxx commented Jan 18, 2024

Doing a little bookkeeping because I often lose track of these things.

PRs in progress:
Spec change -- thanks @phoebe-lew! #222
Javascript change -- thanks @jiyoontbd! TBD54566975/tbdex-js#142

@amika-sq Flagging this issue so it's on your radar for tbdex-swift. See Phoebe's spec change PR for the new design.

@diehuxx
Copy link
Contributor

diehuxx commented Jan 18, 2024

@jiyoontbd Option 1 places replyTo in the TBDex Message (in the metadata) making it part of the protocol spec. Option 3 places replyTo in the HTTP request separate of the TBDex message, making it part of the http-api but not part of the underlying message protocol.

@diehuxx oooh, i think i am understanding where i've misunderstood. do you mean put replyTo as an http request header, and not inside RfqData in a tbdex message? or are you thinking of some other way in which to incorporate replyTo as part of the http request

That's mostly right. You're right that replyTo will not be in the RfqData. My only correction is that replyTo will be in the http request body of submitRfq, not the header.

Also (as noted in in @phoebe-lew's spec PR), we are changing the name of submitRfq to createExchange and changing the endpoint.

@jiyoontbd
Copy link
Contributor

@amika-sq tagging you for tbdex swift work. please see tbdex-js and/or tbdex-kt impls for details

@KendallWeihe
Copy link
Contributor

So I hate to come late to the party here and advocate for a pivot, but I feel compelled to articulate a stance for Option 1 actually. With full recognition it may not be worth pursuing, at least at this time, but I want to make the articulation here while it's fresh in my focus (I just came across this ticket).

Here's my alternative proposal:

  • callback field in the Message metadata field (optional)
  • Every message may have it's own callback location, this would enable cross-device (or hey, we could even do more-than-2-participants) features
  • The value would have the given resolution protocol prefix, such as https:// or did:, or if we wanted to extend to support some weird communications protocol then we could also introduce a callbackProtocol adjacent to the callback property which would inform the given process of the communication should be made
  • The callback would inform the next logical message in the state diagram (this may tie our hands if we introduce weird message types which make the state diagram a non-directed-acyclic-graph but we can climb that hill when we get there)

Here are my reasons why I don't like where we landed:

  • Introducing both tbDEX Messages and tbDEX Requests (which may be the case outside of more than just this ticket) is not readily clear, it's an abstraction which can be comprehended but upon first glance will almost certainly cause confusion. We should pick one paradigm, messaging or request/response and stick with it (I vote messaging).
  • The concept of a callback is an abstraction outside the bounds of HTTP so IMO it makes sense to tightly couple it to the protocol. A callback is a declaration of, "I know you can't respond with the data right now, but I don't want to have to pull the data, so please push it to this location."
  • The current solution creates a weird paradigm where some of our HTTP bodies are 1:1 tbDEX Message types, but others aren't, and the inconsistency is... awkward (actually on this note, why don't we call the property data instead of rfq, because that would be consistent with elsewhere)

Again, I just wanted to document this for future reference. Not a priority by any means, and can be ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment