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

Prevent double removal in staggered strategy #155

Merged

Conversation

bitphage
Copy link
Collaborator

Closes: #152

@mikakoi
Copy link
Collaborator

mikakoi commented May 31, 2018

This isn't a proper fix for the issue, the order needs to be removed from the database when it no longer exists.

@mikakoi mikakoi closed this May 31, 2018
@bitphage
Copy link
Collaborator Author

bitphage commented May 31, 2018

@mikakoi huh? Order actually gets removed a few lines before, I just removed double removal. Look at the full code

    def place_reverse_order(self, order):
        """ Replaces an order with a reverse order
            buy orders become sell orders and sell orders become buy orders
        """
        self.log.info('Change detected, updating orders')
        self.remove_order(order) # <---------------------------- THIS kept

        if order['base']['symbol'] == self.market['base']['symbol']:  # Buy order
            price = order['price'] * (1 + self.spread)
            amount = order['quote']['amount']
            new_order = self.market_sell(amount, price)
        else:  # Sell order
            price = (order['price'] ** -1) / (1 + self.spread)
            amount = order['base']['amount']
            new_order = self.market_buy(amount, price)

        if new_order:
            self.save_order(new_order)
            self.remove_order(order) # <---------------------------- REMOVED

@bitphage
Copy link
Collaborator Author

The real question is, what exactly self.remove_order(order) call should be removed - the first or the second.

@mikakoi
Copy link
Collaborator

mikakoi commented May 31, 2018

Oh, silly me. It's the first call that needs to be removed, so that the order isn't removed from the database when an error occurs during the order placement.

@mikakoi mikakoi reopened this May 31, 2018
@bitphage bitphage force-pushed the 152-staggered_orders-double-removal branch from cb18e9b to bc4dd66 Compare May 31, 2018 06:28
@bitphage
Copy link
Collaborator Author

OK, updated a PR.

@mikakoi mikakoi merged commit bc4dd66 into Codaone:master May 31, 2018
@bitphage bitphage deleted the 152-staggered_orders-double-removal branch May 31, 2018 06:58
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

2 participants