tfImmediateOrCancel should use tecKILLED

[mDuo13]

The fix1578 amendment changed OfferCreate transactions so that if they used tfFillorKill and the transaction was killed, it used the code tecKILLED rather than tesSUCCESS. We should do the same for the tfImmediateOrCancel flag, which still results in tesSUCCESS when the Offer can't be immediately and fully filled.

Summary

It's confusing and unintuitive to see an "immediate or cancel" transaction treated as "successful" when it did nothing because the "or cancel" part applied. A different result code makes it apparent at a glance what happened.

This is a transaction processing change so it should go on an amendment.

Aside

An unintuitive edge case is the interaction between tecKILLED and specifying a previous Offer to cancel in the OfferSequence field. My understanding is that post-fix1578, a "Fill or Kill" offer that gets tecKILLED does not cancel the previous Offer. We should make "Immediate or Cancel" work the same way. This may be a functional change beyond the result code but the consistency is worth it.

[scottschurr]

Is tecKILLED really the right tec code to use if the tfImmediateOrCancel flag is set? The descriptive text that goes with tecKILLED is "FillOrKill offer killed". Seems like that might be confusing descriptive text to get back from a tfImmediateOrCancel transaction.

I think there are three options:

1. Reuse tecKILLED unchanged because we guess most folks won't be confused.
2. Reuse tecKILLED but change the descriptive text to something that describes both situations. I'm not sure what that text would be.
3. Create a new tecCANCELED code with descriptive text, "ImmediateOrCancel offer canceled". Use the new code exclusively in the amendment.

Option 2 would be difficult to implement in an amendment-friendly way. Currently there's no way to make the descriptive text of a TER code be dependent on an amendment. We could decide that the descriptive text of a TER code is not part of the API I suppose.

Thoughts?

[scottschurr]

Hmmmm. I'm digging into the details of this a little harder. If I'm understanding correctly, a tfImmediateOrCancel offer never, under any circumstances, creates an on-ledger offer. Instead it crosses as much of the "created" offer as possible given the current state of the ledger and then stops. The amount crossed may vary anywhere from zero to the full amount of the offer.

I'm guessing that you would like to see a tec code returned in the case where the amount crossed is exactly zero. Any non-zero amount crossed would result in a tesSUCCESS. Is that correct? I think that's the only way it can work. It would be highly unusual to return a non-tesSUCCESS code if any funds were transferred other than for paying the transaction fee.

I'm dubious about the usefulness of the tec code, since the amount crossed could be as small as, say, 1 drop; which is to say almost zero. The person submitting the transaction really should be looking at the transaction metadata to see how much of the offer was actually crossed. The tec code can only be returned if absolutely zero was transferred.

[mDuo13]

Regarding the first point: option 2 is the way we should go. And this part is not a transaction processing change, so no amendment is necessary for the description change. Per transaction results documentation:

This message is intended for developers to diagnose problems, and is subject to change without notice.

As for the point on how the amount can be very small: I agree, the metadata is pretty important here, but I still think it's a helpful shortcut and consistency thing to have the transaction return a code for the case where the amount crossed is actually 0. Practically speaking, I think the cases where the entire Offer is canceled because it did not cross any existing Offer are going to be more common—especially during learning & early developing—than cases where it did cross for a very small amount, and a tecKILLED error code provides a clear and useful signpost for why the Offer did nothing. In cases where the Offer did do something, the metadata contains the balance changes clarifying what it did do. But in cases where the Offer was canceled under the current rules, the metadata does not contain an explicit indication of what happened: the only way you know that it got canceled is from the lack of any balance changes.

A major benefit of this change is that you could write code to assume that, if the code is tesSUCCESS (and the transaction is post-amendment) then there will be balance changes of some kind in the metadata, and if the code is tecKILLED then you don't have to look at the metadata to know that no balance change occurred (other than burning the tx cost of course).

[scottschurr]

Okay, I can work with that. So we change the descriptive text associated with tecKilled to read "No funds transferred and no offer created"? I believe that applies in both situations. Can you suggest a better message?