search

TBD54566975 / tbdex

56 stars 25 forks source link

Dumb questions on tbDEX v1.0 spec #280

Open thehenrytsai opened 5 months ago

thehenrytsai commented 5 months ago

Thanks for the super reader-friendly tbDEX 1.0 spec! Really appreciate the clarity and thoughts that went into creating the protocol, which made the read-through an enjoyable experience!

I just have a few clarifying questions, happy to submit a PR once I have all the answers. Also happy to jump in a call anytime!

  1. Balance resource

    • [x] Sanity: are both IOS 3166 currency code and ISO 4217 currency code used or just one of them (both are referenced in spec)?

  2. Offering resource

    • [ ] Does fee in PayinMethod represent the absolute value in the currency of the payin and payout? It seems to be the case based on the description, but I'd imagine at least some PFIs would like to express a percentage value of the payin and/or payout instead. How does one express that and differentiate it from an absolute value?
    • [ ] For my learning: why doesn't PayinMethod require estimatedSettlementTime like PayoutMethod? e.g. a contrived method being bank check deposit.

  3. Jargon Decoder

    • [x] The term "KYC" listed in the Jargon Decoder but is never referenced anywhere in the document. 😆

  4. RFQ example

    • [ ] Is the requirePaymentDetails JSON schema tied to the payin/out method? (e.g. DEBIT_CARD method must be given detail in the form of a certain schema )?
    • If yes, what is the reason for needing the credit card info in an RFQ for DEBIT_CARD payin method, when the quote was going to provide a link that presumably asks for the credit card info again (based on the examples)? Could it be that the method that uses an external link is a different method from DEBIT_CARD?

  5. Close message

    • [ ] Does a successful exchange require a Close message (if so, from which party? Both?)? Or is looking into the orderStatus of the OrderStatus message the official way?
    • [ ] Regardless where the signal is, it might be worthwhile having an explicit code/value that differentiates the happy-path close from other closes. COMPLETED seems to be the value in an example, but is not explicitly called out to have first-class spec importance.

  6. Misc.

    • [x] For private ephemeral data that are swapped out with SHA256 hash, is there a spec rule to that decides what data needs to be ephemeral thus need to be swapped out in this manner? Or it's the sender's choice on what is private/ephemeral?
    • [x] For my learning: what's the reasoning for prefixing message/resource ID with metadata.kind?
    • [x] Sanity: exchangeId in the RFQ example is the same as the RFQ ID, does it have to be? The description seems to imply that it doesn't have to be, but I am not sure.
KendallWeihe commented 5 months ago
  1. Close message

@thehenrytsai does this answer your confusion? https://github.com/TBD54566975/tbdex/issues/257

I'm going to update the spec docs with what's in that ticket, but in different words

thehenrytsai commented 5 months ago

Hi @KendallWeihe, yes! Definitely clarifies things, especially the part on the newly introduced success property, but still unclear for me on:

  1. Is Alice REQUIRED to send a Close at all at the end of an order execution for the order to be considered "complete/success"? Doesn't look like this is something enforceable by the PFI, thus seems to be not very useful. Which brings us to the next question.

Close messages may be sent by Alice to the PFI during an actively executing order (AKA a state after, but not necessarily immediately after, an Order but prior to a Close where the Close.success is true), but that doesn't necessarily guarantee a successful canceled execution.

  1. Does the FPI create/send a Close (with success set to true obviously) for Alice at the very end of the order execution? Is the "prior to a Close where the Close.success is true" referring to the FPI's close here?

It might be worth considering to:

  1. Allow success to be exclusively used by the FPI to signal a "successful close" of the transaction.
  2. If Alice tried to use success at all, treat it as an invalid message, even if it is set to false, just for simplicity of processing.
  3. Addition consideration: Have "successful" close and "bailed" close being distinct messages, perhaps Terminate being the unhappy path, and Close being the happy path (still exclusive for PFI to use), this way there is no ambiguity and thus easy in spec language. Obviously use even better names if you can think of one.
KendallWeihe commented 5 months ago

Is the "prior to a Close where the Close.success is true" referring to the FPI's close here?

...being distinct messages...

I think you've hit the nail on the head here on one particular matter; in my own words, we're using Close to cover too many use cases and there's a distinction between Alice terminating (or attempting to terminate) an exchange versus the PFI terminating an exchange.

It's worth considering my framing from here:

There are 2 overarching phases of an exchange:

  1. Negotiation (RFQ and Quote)
  2. Execution (Order and OrderStatus)

I appreciate the ideas around success only being valid when used by the PFI. Opened a ticket for the proposal here and a PR here.

mistermoe commented 5 months ago

Sanity: are both IOS 3166 currency code and ISO 4217 currency code used or just one of them (both are referenced in spec)?

@thehenrytsai good catch. was meaning to update the spec to correct this mistake. ISO 3166 is actually country codes and ISO 4217 is currency codes so we want to use ISO 4217

created an issue and assigned u: https://github.com/TBD54566975/tbdex/issues/298

mistermoe commented 5 months ago

For private ephemeral data that are swapped out with SHA256 hash, is there a spec rule to that decides what data needs to be ephemeral thus need to be swapped out in this manner? Or it's the sender's choice on what is private/ephemeral?

@thehenrytsai, @diehuxx just tossed a PR up that explains the approach we landed on: https://github.com/TBD54566975/tbdex/pull/294

mistermoe commented 5 months ago

The term "KYC" listed in the Jargon Decoder but is never referenced anywhere in the document. 😆

good catch! opened and issue and assigned you 😄

https://github.com/TBD54566975/tbdex/issues/296

mistermoe commented 5 months ago

For my learning: what's the reasoning for prefixing message/resource ID with metadata.kind?

we're using typeID for all resource and message IDs. Technically speaking we could remove both metadata.kind and metadata.createdAt because that information can be derived from the id. Given that id, metadata.kind and metadata.createdAt all exist, we should probably include spec text around validating that the prefix in id matches kind. i'm going to guess that removing metadata.kind and metadata.createdAt is more controversial 😆

created an issue and assigned you: https://github.com/TBD54566975/tbdex/issues/299

mistermoe commented 5 months ago

Sanity: exchangeId in the RFQ example is the same as the RFQ ID, does it have to be? The description seems to imply that it doesn't have to be, but I am not sure.

yep it does. we should explicitly state in spec text that this is the case. good callout. an Exchange is a "virtual" concept (e.g. they don't as concrete type) because an RFQ is necessarily the first message in an exchange, it made sense to make the exchange id the rfq id

@thehenrytsai created an issue and assigned to u! https://github.com/TBD54566975/tbdex/issues/297