Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding separate message to cancel an existing Selling Offer. #40

Closed
wants to merge 2 commits into from

Conversation

maran
Copy link
Contributor

@maran maran commented Jan 20, 2014

A variation of #39 where we use a separate message to cancel an order instead of re-using the Selling Offer.

I think it makes more sense to officially cancel an order, I understand that broadcasting a new Selling Offer with a zero amount makes it clear enough that there is nothing for sale yet but it still feels ugly. Any implementation that wants to show Selling Offers needs to create custom logic to deal with Selling Offers with zero amounts.

This is mostly just to gauge interest in this one and see if anybody feels the same way about this.

@dacoinminster
Copy link
Contributor

I slightly prefer #39, but I don't feel strongly about it.

@grazcoin
Copy link

I would rather keep the amount of different messages as small as possible.
Zero amount is clear enough, and not much extra logic is required. Simplicity is also beauty.

@maran
Copy link
Contributor Author

maran commented Jan 20, 2014

Ok, closing this one :)

@maran maran closed this Jan 20, 2014
@marv-engine
Copy link

+1
In general, I prefer explicit actions that don't require someone to infer/assume what was intended.

Note a race condition is still possible - the offer could be accepted before the Cancel tx. The client/user still has to confirm the offer was cancelled before creating an updated sell offer. A Modify message could make it a single step, with no risk of creating an unintentional second sell offer. A zero amount in the Modify message could still mean cancel.

@maran maran reopened this Jan 20, 2014
@maran
Copy link
Contributor Author

maran commented Jan 20, 2014

That's why you should always wait until your previous message is confirmed before opening a new one. This goes for most messages though right?

We have 2 against 1 for. I will keep it open a little longer.

@marv-engine
Copy link

Is there a benefit to having the extra logic, extra message transmission and the time delay to accomplish the update? In high volume situations, that extra time could be critical. From the user's standpoint, an update is a single logical action.

@maran
Copy link
Contributor Author

maran commented Jan 21, 2014

Mastercoin will always work on block delays. It can never do high frequency trading without a centralised solution. If three people send an 'I accept your Selling Offer' message they will always need to wait until it gets included in a block before they know if they got it.

I would also comment in #39 since the other devs seem to read that one but not this one :)

@ripper234
Copy link
Contributor

Yeah, but still saving 10 minutes is important even if we can't do HFT.
On Jan 21, 2014 10:33 AM, "Maran" notifications@github.com wrote:

Mastercoin will always work on block delays. It can never do high
frequency trading without a centralised solution. If three people send an
'I accept your Selling Offer' message they will always need to wait until
it gets included in a block before they know if they got it.

I would also comment in #39https://github.com/mastercoin-MSC/spec/pull/39since the other devs seem to read that one but not this one :)


Reply to this email directly or view it on GitHubhttps://github.com//pull/40#issuecomment-32829379
.

@marv-engine
Copy link

There's probably background here that I'm not familiar with, but the 2 step approach seems to put an extra burden on the human or the client trying to update the sell offer, and it can also cause disruption for a human or client looking for sell offers.

As I understand it, the seller cancels the offer then has to wait for it to be confirmed. If the human seller or the client app is not available when it is confirmed, there will be an unknown delay before the seller is able to send a new sell offer. During this time, potential buyers have only seen a sell offer cancelled. So they can't buy.

If the human knows it's a two step process, then he has to look for the cancellation confirmation in order to send the new sell offer as soon as possible. Is this the intention? If the client hides the 2 steps from the human (because logically it's just one action - an update) then the client has to maintain state info about the 2 steps until it is complete (and report all those states to the user), and send the new sell offer as soon as it detects the cancellation has been confirmed.

@maran
Copy link
Contributor Author

maran commented Jan 21, 2014

Guys, I know I created this pull to begin with but I don't have the time right now to really go into it and discuss it. This is my last week for my old employer and I can't give this the energy it deserved at the moment.

Please make a decision on this and I will accept any outcome you deem best and change my implementation accordingly. I think it's up to @dacoinminster to merge one of these, or none and keep the original change. (although I don't think that solved the race condition). Perhaps a third option is needed where we have a new/change and cancel message.

@dacoinminster
Copy link
Contributor

Both this one and #39 are identical from the user's perspective. I plan to merge #39 soon if there are no objections.

@marv-engine
Copy link

Not to belabor this, but is it preferred for the user/client to implement a cancel Sell Offer, new Sell Offer sequence rather than a modify Sell Offer?

@ripper234
Copy link
Contributor

Marn has some good points here.
J.R in order to get a faster turnaround, can you discuss this with Marv over skype/phone and hash things out?

I haven't delved into the technicals here, but I tend to agree with Marv - usability and order execution time is super important here.

@dacoinminster
Copy link
Contributor

I'm also ok with a new pull request which replaces these with something that has a replace/cancel order. Marv, would you like to do a new one?

I should be on the sync-up call today if we need voice discussion of this.

@marv-engine
Copy link

I can do a new PR for this. When is the sync-up call?

@dacoinminster
Copy link
Contributor

Weird - I don't see your email on the invite for the sync call. I did a reply-all to include you.

@dacoinminster
Copy link
Contributor

Guys, I need you to weigh in on #41, which seems like the best solution from a user-experience perspective.

@maran
Copy link
Contributor Author

maran commented Jan 28, 2014

Going for #41, closing this.

@maran maran closed this Jan 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants