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

Remove the ability to change a sell order without cancelling it first #39

Closed
wants to merge 1 commit into from

Conversation

maran
Copy link
Contributor

@maran maran commented Jan 20, 2014

This pull request could replace #38.

The general idea is to remove the ability to change a sell order without cancelling the existing one first. This should remove the race condition where you end up broadcasting a new order instead of changing the old one.

@grazcoin
Copy link

+1

@dacoinminster
Copy link
Contributor

+1 on this one. Seems simpler than #40 to me.

Don't feel strongly about it though.

@zathras-crypto
Copy link
Contributor

+1 for #39 - simplicity = good :) Rather avoid an additional message if possible, good to keep all selloffer logic together IMO.

Sells must be confirmed cancelled before a new one can be created; love it +100 - will cut down code complexity

@Bitoy
Copy link

Bitoy commented Jan 20, 2014

+1. Simpler the better

@marv-engine
Copy link

See the ongoing discussion in #40

@dacoinminster
Copy link
Contributor

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

@dacoinminster
Copy link
Contributor

There may be a new pull request with a replace/cancel command. See #40 for discussion.

@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

6 participants