Skip to content

[wip] - support for shorting#450

Open
ddragosd wants to merge 9 commits intotensortrade-org:masterfrom
ddragosd:issue-317
Open

[wip] - support for shorting#450
ddragosd wants to merge 9 commits intotensortrade-org:masterfrom
ddragosd:issue-317

Conversation

@ddragosd
Copy link

@ddragosd ddragosd commented Sep 13, 2022

I'm opening a PR to get feedback on adding support for shorting.

I need this feature, and I'd like to share it with the community too.

Details

The implementation intentionally keeps the original behavior in place as much as possible - which is no shorting support . Developers have to be specific in the code to enable it.
What this PR adds:

  • MarginWallet - a class that should be explicitly used when shorting is needed. Instead of initializing a Wallet instance, developers can initialize a MarginWallet
  • NegativeQuantity - I created this class as the current implementation is pretty strict with throwing errors for negative quantities, and I wanted to leave that functionality untouched.

TODO:

  • Add documentation and sample code showing how to setup shorting.

Related Issues

@spytensor
Copy link

Great Job!

@ddragosd
Copy link
Author

ddragosd commented Oct 5, 2022

I'm still testing this in production for a little longer before removing the WIP, and making it ready for review.

@carlogrisetti
Copy link
Collaborator

Thanks a lot for your effort!

@BarakBa1
Copy link

@ddragosd you should take a look at this implementation from MrDaubinet
could be helpful for your work to support shorting

@ddragosd
Copy link
Author

@BarakBa1 thanks for referencing the extension. Are you suggesting we don't merge this PR and instead use the extension you referenced ?

@Buxiulei
Copy link

Hope the shorting can be merged into the master

@qwpto
Copy link

qwpto commented Oct 1, 2023

Thanks for the efforts, hope it gets merged.

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.

6 participants

Comments