search

TBD54566975 / tbdex

56 stars 26 forks source link

adding orderinstructions message description #355

Closed jiyoonie9 closed 3 months ago

jiyoonie9 commented 4 months ago

closes #335

KendallWeihe commented 4 months ago

Oh so, wait, at what point during the exchange is an orderinstructions valid? In between a quote and an order?

So the happy path would be: RFQ --> Quote --> OrderInstructions --> Order --> OrderStatus --> Close

jiyoonie9 commented 4 months ago

Oh so, wait, at what point during the exchange is an orderinstructions valid? In between a quote and an order?

So the happy path would be: RFQ --> Quote --> OrderInstructions --> Order --> OrderStatus --> Close

Happy path is RFQ > Quote > Order > OrderInstructions > {alice uses the instructions to execute payin} > OrderStatus(es) > Close

jiyoonie9 commented 4 months ago

I don't see any issues which will arise out of this, but what was the original motivating factor? An application-level requirement?

The original motivating factor came about while testing with frontend. If Alice had to take action to pay in (i.e. use some link), it wasn't clear whether she should send the Order message and then pay, or pay and then send an Order message.

having order instructions breaks out the steps more, so that she can:

  1. get the quote
  2. if she's satisfied with the quote, she signals her intent to go through with the exchange by sending an Order message
  3. PFI sends instructions on how to execute her order
  4. alice uses the instructions to begin payin (or decides she doesn't like the instructions, sends a cancel instead)
nitro-neal commented 4 months ago

adding orderinstructions from the pfi after order makes a lot of sense ✅

nitro-neal commented 4 months ago

@jiyoontbd does there need to be something added to the http client?

get_order_instructions()

or would that come back in a new field in teh response in submit_order()?

nitro-neal commented 4 months ago

oh no I can't unsee it! On the naming we have:

OrderInstructions paymentinstruction

Thoughts on making it OrderInstruction ?

...

But on second thought, it does contains 2 order instructions, payin payout... so maybe thats fine :hmm:

KendallWeihe commented 3 months ago

No strong opinions on the singular-vs-plural naming. Any of the above are fine. In my head the word "instructions" is always plural, but maybe that's just me. In which case, we should actually rename paymentInstruction to paymentInstructions (plural), but I can see why that would be overly disruptive at this time. Totally fine for the names to be inconsistent right now too, or to make OrderInstruction singular. Like I said, any are fine.

angiejones commented 3 months ago

is OrderInstructions an optional message in an exchange?

jiyoonie9 commented 3 months ago

@jiyoontbd does there need to be something added to the http client?

get_order_instructions()

or would that come back in a new field in teh response in submit_order()?

order instructions is a net new message and is not returned as a response to submitting an order. so if you're following a pattern of get_quote() or get_order_status() then yea that makes sense to me

jiyoonie9 commented 3 months ago

is OrderInstructions an optional message in an exchange?

good question @angiejones - i was thinking it would be required, but payin and payout instructions can be empty if there's no instructions needed (i.e. for an offering that's just a stored_balance > stored_balance transfer)

angiejones commented 3 months ago

hmm, programmatically, how do you envision a wallet app handling order instructions? Present the text from the message to the user? Then what? How does the Wallet know once those instructions have been executed? Do they wait for the PFI to provide an OrderStatus or Close?

jiyoonie9 commented 3 months ago

hmm, programmatically, how do you envision a wallet app handling order instructions? Present the text from the message to the user? Then what? How does the Wallet know once those instructions have been executed? Do they wait for the PFI to provide an OrderStatus or Close?

orderinstructions contains payin and payout, with link and/or instruction fields

if there's a payment link that alice needs to navigate to in order to fulfill payin, the payin object of orderinstructions should be populated. once the payment is done, the wallet should detect completion and redirect back to the wallet view where alice is now waiting for PFI to confirm that her payin has been received, and PFI will initiate payout.

cc-ing @ethan-tbd for how didpay accommodated redirect after payin was completed

jiyoonie9 commented 3 months ago

i'm going to merge this because we're up against an internal team deadline. creating issues for questions not yet resolved now

jiyoonie9 commented 3 months ago

hmm, programmatically, how do you envision a wallet app handling order instructions? Present the text from the message to the user? Then what? How does the Wallet know once those instructions have been executed? Do they wait for the PFI to provide an OrderStatus or Close?

orderinstructions contains payin and payout, with link and/or instruction fields

if there's a payment link that alice needs to navigate to in order to fulfill payin, the payin object of orderinstructions should be populated. once the payment is done, the wallet should detect completion and redirect back to the wallet view where alice is now waiting for PFI to confirm that her payin has been received, and PFI will initiate payout.

cc-ing @ethan-tbd for how didpay accommodated redirect after payin was completed

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