Jump to content
rammican

fillOrKill flag not working??

Recommended Posts

I may be misinterpreting how the fillOrKill flag works but I have had multiple orders now that are coming through as successful but that are not filling? I am getting tesSUCCESS as the final result yet my balances do not change. How is this possible? Doesn't fillOrKill ensure that the balances change or the order is cancelled?

Share this post


Link to post
Share on other sites

This is correct:

You asked the server to execute some order and to do so only if the order could be filled (i.e. executed immediately and completely). If it couldn’t, you asked the server to not execute it.

The server tried to comply with your request; it decided that it couldn’t fill the order so it killed it—as you asked.

You got back a tesSUCCESS which is reasonable: the server successfully executed your request.

I guess it could have returned something like tecKILLED but it doesn’t. Something to think about.

 

Share this post


Link to post
Share on other sites

Returning tecKILLED would be really helpful.

This probably warrants a different topic on this forum but what is current best practice for determining orders that are filled vs. killed. Or we can be even more general, what is the best practice for determining if any order is filled.

 E.g. For the Ripple API- get balance for currency -> place order -> if tesSUCCESS ->  monitor if balance changes?

But this seems less effective then just subscribing to an orderbook through a websocket request on rippled. Then you can monitor your orders. 

I'd love to hear others thoughts. 

Share this post


Link to post
Share on other sites

For all transactions the only reliable way to understand what happened is checking the meta, so I don't see why this transaction should be different. Also because tesSUCCESS is returned before being validated, so the node you submitted to cannot know if the order was filled or killed because it can fail also to be validated in a consensus round.

Share this post


Link to post
Share on other sites

Thanks!!

@tulo I'm really new to all of this so I just want to make sure that I'm understanding your metadata suggestion. Basically, I need to check the balanceChanges when using getTransaction to verify? 

I do think tecKILLED will add a lot of value because from my limited experience so far, it can sometimes take 4 or more seconds to verify the transaction after getting a tesSUCCESS. I'd much rather just get tecKILLED and forgo process of getTransaction. It's also just less confusing for newbies ;).

Share this post


Link to post
Share on other sites
7 hours ago, mDuo13 said:

I opened a feature request for tecKILLED because this also sounds like a good idea to me.

So can this "policy" be applied to other transactions? For example OfferCancel always returns tesSUCCESS also if no offers were deleted because the offer with that sequence didn't exist.  It would be nice to have tesSUCCESS if deleted and tecNONEXISTING when no offer exists. And probably many other transactions could have different tes/tec codes...

Share this post


Link to post
Share on other sites
5 hours ago, rammican said:

@tulo I'm really new to all of this so I just want to make sure that I'm understanding your metadata suggestion. Basically, I need to check the balanceChanges when using getTransaction to verify? 

Yeah, because also if you get tesSUCCESS, it doesn't mean that the transaction was successfully included in the ledger. It could always fail the consensus, so you need always to check for validated transactions. But if you get tecKILLED (with this proposal), you are sure that the offer won't be "published". So it doesn't really help for positive cases, because even for tesSUCCESS you need to check the meta, but it helps for negative cases when the offer is killed.

Share this post


Link to post
Share on other sites
17 hours ago, tulo said:

So can this "policy" be applied to other transactions? For example OfferCancel always returns tesSUCCESS also if no offers were deleted because the offer with that sequence didn't exist.  It would be nice to have tesSUCCESS if deleted and tecNONEXISTING when no offer exists. And probably many other transactions could have different tes/tec codes...

I'm really new to all of this so I hope my logic makes sense here. I do agree that it would be nice to have more nuanced messages such as tecNONEXISTING but I also see fillOrKill as having a higher need than others for two reasons:

1) fillOrKill has a certain time sensitive connotation that stems from its use in securities trading. New people to ripple will probably get confused by tesSUCCESS.

2)If OfferCancel returns tesSUCCESS you know that your offer is either canceled or nonexistent which should lead to the same desired outcome. So with OfferCancel tesSUCCESS means that you know the potential outcome: no longer having an open order if validated. fillOrKill is different in that when tesSUCCESS is returned you know nothing about the potential outcome and you have to wait for validation to see what happened.  

Share this post


Link to post
Share on other sites
17 hours ago, tulo said:

Yeah, because also if you get tesSUCCESS, it doesn't mean that the transaction was successfully included in the ledger. It could always fail the consensus, so you need always to check for validated transactions. But if you get tecKILLED (with this proposal), you are sure that the offer won't be "published". So it doesn't really help for positive cases, because even for tesSUCCESS you need to check the meta, but it helps for negative cases when the offer is killed.

BE AWARE: tecKILLED IS NO MORE FINAL THAN tesSUCCESS. In both cases, the final result of the transaction can change when consensus puts transactions in their canonical order. Even transactions that "failed" get relayed to the network—that's what the tec code is for. The provisional result you get from submitting a transaction is just that: provisional. (For more detail, see Reliable Transaction Submission and Finality of Results.)

In all cases, you have to wait for validation (or LastLedgerSequence expiry) see what really happened. (Well, technically, if you got a tem-class error, you know your transaction is malformed so it cannot be included in any ledger later, either.)

Share this post


Link to post
Share on other sites
11 minutes ago, Sukrim said:

Maybe tesSUCCESS should rather be tesSUBMITTED? Sounds more like "sounds good, let's wait and see" than "this will work for sure"...

Good idea. But unlikely to change now.

Share this post


Link to post
Share on other sites
5 hours ago, mDuo13 said:

BE AWARE: tecKILLED IS NO MORE FINAL THAN tesSUCCESS. In both cases, the final result of the transaction can change when consensus puts transactions in their canonical order. Even transactions that "failed" get relayed to the network—that's what the tec code is for. The provisional result you get from submitting a transaction is just that: provisional. (For more detail, see Reliable Transaction Submission and Finality of Results.)

In all cases, you have to wait for validation (or LastLedgerSequence expiry) see what really happened. (Well, technically, if you got a tem-class error, you know your transaction is malformed so it cannot be included in any ledger later, either.)

Oh really? I believed tec codes would fail for sure :o

EDIT: yeah it makes much sense.

Edited by tulo

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×
×
  • Create New...