TBD54566975 / tbdex

56 stars 25 forks source link

Provide Enum of OrderStatus and Close status #256

Closed angiejones closed 6 months ago

angiejones commented 6 months ago

Can we provide a few enums for message.data.orderStatus for OrderStatus? The method signatures don't have to change, so the respective properties can continue to be String, allowing PFIs to enter custom details there. But I'm thinking, right now, this is wide open. We're talking across borders, the Strings might not even be in English, so as a wallet app, I'm showing a Spanish string as a status update from the PFI because there's no enums or standards here.

KendallWeihe commented 6 months ago

We spoke about this at Office Hours, so re-articulating what was the strongest consensus. No need to implement status enums, for the time being, because we can bucket order execution into one of three statuses:

  1. Pending/processing/executing (use whatever word ya want which means it's actively being executed)
  2. Completed
  3. Failed

And so, the mechanism for wallets to detect which state the order execution is within (so that the wallet GUI can display to the end user) is as such:

if Close does not exist: EXECUTING
else if Close.success == true: EXECUTED
else if Close.success == false: EXECUTION_FAILED

(I used consistent verbiage of "execute" across all three semantic buckets, just keeping you all on your toes 😄)


There is a broader question of how this plays out as things progress, but the above consensus seemed pragmatic for the time being.

phoebe-lew commented 6 months ago

Closing this because we will implement Close.success to distinguish between success/failures

angiejones commented 6 months ago

@phoebe-lew this is a different issue that's around random OrderStatus strings and offering a few standard ones.

here's an example of a plain string order status that's being shown in a totally different language than the app. that's the problem I'm looking to solve for (in addition to being able to add any conditional logic based on expected strings)

image
phoebe-lew commented 6 months ago

@angiejones can you explain your use case vs Kendall's comment?

angiejones commented 6 months ago

@angiejones can you explain your use case vs Kendall's comment?

I'm talking about OrderStatus which comes before the Close message. Specifically I'm talking about the Strings that PFIs are sending back to the Client

From Kendall:

Pending/processing/executing (use whatever word ya want which means it's actively being executed)

The client has no idea which one of these the PFI is going to send, if it's spelled correctly, if it's in a different language, if it's something totally different. Random arbitrary strings are not much use. What's the purpose of having them if you'll just always show "EXECUTING" in the absence of a Close message?

KendallWeihe commented 6 months ago

Terminology: for the sake of this ticket let's refer to two things

  1. OrderStatus.status is the value within the OrderStatus message
  2. "Natural language OrderStatus" (AKA "NLO") is the value which is presented to the end user in the GUI

If I understood the consensus we came to is, there are two distinct matters, but feel free to challenge that as perhaps there's an argument to be made that this feature ought to be supported at the protocol-level (I believe the case against doing so is, the complexity of trying to solve this at the protocol-level is practically intractable).

The idea being that, the wallet will make the choice as to what language is presented to the user for the case of (what I defined as) "EXECUTING" and will be able to do so based on the ruleset (re-pasting from comment above):

if Close does not exist: EXECUTING
else if Close.success == true: EXECUTED
else if Close.success == false: EXECUTION_FAILED
angiejones commented 6 months ago

Terminology: for the sake of this ticket let's refer to two things

  1. OrderStatus.status is the value within the OrderStatus message
  2. "Natural language OrderStatus" (AKA "NLO") is the value which is presented to the end user in the GUI

If I understood the consensus we came to is, there are two distinct matters, but feel free to challenge that as perhaps there's an argument to be made that this feature ought to be supported at the protocol-level (I believe the case against doing so is, the complexity of trying to solve this at the protocol-level is practically intractable).

The idea being that, the wallet will make the choice as to what language is presented to the user for the case of (what I defined as) "EXECUTING" and will be able to do so based on the ruleset (re-pasting from comment above):

if Close does not exist: EXECUTING
else if Close.success == true: EXECUTED
else if Close.success == false: EXECUTION_FAILED

So is your proposal to get rid of OrderStatus.data.orderStatus? If so, OrderStatus needs an entire overhaul. There's no point in creating a bunch of OrderStatus objects that have no info in them. They should be more of an OrderReceived message.

If we are shipping with OrderStatus in place, we need to enable people to globalize (at least a few of) the strings.

Also with the proposal below, I'd argue that Close.success needs to be a required field, not an optional one:

if Close does not exist: EXECUTING
else if Close.success == true: EXECUTED
else if Close.success == false: EXECUTION_FAILED