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

Feature/floating maker changes #67

Conversation

ricardojmendez
Copy link
Contributor

Multiple changes to floatingMaker, including adding configuration options, avoiding crashes on exceptions, and handling issues #65 and #66.

- Handling potential exceptions on floatingMaker
- Canceling and placing all orders at once instead of serially - that
  will make it cleaner to set up multiple orders later.
src/config.ts Outdated Show resolved Hide resolved
} catch (e) {
console.log('Error placing new orders');
console.error(e);
console.log('----');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what error are you typically hitting that leads to this?

Copy link
Contributor Author

@ricardojmendez ricardojmendez Sep 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually it seems to be "This transaction has already been processed" (at simulation time) but I've seen the rare timeout.

src/bots/floatingMaker.ts Outdated Show resolved Hide resolved
const orderSizeShort =
exposureDir == PositionDirection.SHORT
? orderSizeCurrent
: orderSizeOther;
Copy link
Member

@0xbigz 0xbigz Sep 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think it'd be better/safer if orderSize was specified in quote and then floored to the base step size?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean as an API option?

We can now configure the timing and the order size
I'm getting some errors on testing with all markets, and this makes it
easier to narrow down what the issue is. It's also unlikely anyone
would like to test all markets at once.
The bot was canceling existing orders when one side had been fully
filled, *except* for SOL-PERP, in which case it always canceled them
even if they were partially filled. This had the effect that a SOL_PERP
position grew rapidly unbalanced, regardless of how much the market
was moving.

See issue drift-labs#66.
Trivial implementation, see comments on the code for details.

This addresses issue drift-labs#65.
Two changes:

- Canceling all PERP orders on start, in case there were any leftover
  orders by a previous instance;
- Canceling all orders on for a market with a single instruction.

See comments on patch for a possible improvement.
@ricardojmendez ricardojmendez marked this pull request as draft September 6, 2023 08:36
- Moved maxPositionExposure to the configuration, it makes no sense to
  keep it hardcoded.
- Updated comments around the way it behaves when approaching the
  maximum exposure, since the ones there did not match the
  implementation (and the bot wasn't handling it before).
- Removed unused constant RESTRICT_POSITION_SIZE. If you don't want it
  restricted, simply use a high value as the maximum exposure.
- Moved all order related calculations into the `if` we are placing
  orders block.
- Fixed a mistaken log on the remaining allowance.
It's possible that the position has exceeded the total allowed value
due to multiple reasons: configuration changes, collateral value
changes, violent position swings, or simply user-executed transfers.
Decided to handle what happens if we get too lopsided, and we need to
start closing one side. We identify that case by the proportion being
such  that one of the sides would be smaller than the minimum order
size for the market. In that case, we *will* allow a single order on
the other side.
@ricardojmendez ricardojmendez force-pushed the feature/floating-maker-changes branch from a29d7d6 to b9212da Compare September 6, 2023 21:27
@ricardojmendez
Copy link
Contributor Author

Fixed some issues, and added a few commits, including implementing bias order skewing and handling positions in a "closing" state, as well as cleaning up outdated comments.

@ricardojmendez ricardojmendez marked this pull request as ready for review September 6, 2023 21:28
@ricardojmendez ricardojmendez deleted the feature/floating-maker-changes branch October 25, 2023 10:04
@ricardojmendez
Copy link
Contributor Author

Closing, since the PR is old and the code has drifted. I may re-submit after a review.

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.

2 participants