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

Implement standalone BuyDirect msg #905

Closed
4 tasks
technicallyty opened this issue Mar 17, 2022 · 3 comments · Fixed by #967
Closed
4 tasks

Implement standalone BuyDirect msg #905

technicallyty opened this issue Mar 17, 2022 · 3 comments · Fixed by #967
Assignees
Labels
Scope: x/ecocredit Issues that update the x/ecocredit module Type: Feature New feature or request

Comments

@technicallyty
Copy link
Contributor

Summary

Currently, the Buy msg has two options to submit buy orders, one of which acts more like BuyDirect and the other for DEX purposes.

Issues

There are a few UX quirks as a result of this:

  • we have two fields disable_partial_fill and expiration that do not apply to the message. These fields would probably be ignored, even if supplied.
  • the two types of orders are selected via a OneOf and will need to be refactored anyways, as this causes issues for amino signing.

Proposal

Create a standalone BuyDirect RPC endpoint, separate from the current monolithic Buy


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@ryanchristo ryanchristo added Scope: x/ecocredit Issues that update the x/ecocredit module Status: Proposed labels Mar 17, 2022
@ryanchristo ryanchristo added this to the v4.0 - Llangorse Upgrade milestone Mar 17, 2022
@ryanchristo
Copy link
Member

ryanchristo commented Mar 26, 2022

I support this. Less complexity and cleaner solution to the amino signing issue. Also prevents breaking changes when we introduce the orderbook model. Essentially Buy would be create buy order and Sell would be create sell order, BuyDirect would not create a buy order and not include disable_partial_fills and expiration.

@ryanchristo ryanchristo changed the title Consider a standalone BuyDirect msg Implement standalone BuyDirect msg Mar 26, 2022
@ryanchristo
Copy link
Member

We should probably get a second review for #899 and merge, then implement this as a followup.

@aaronc
Copy link
Member

aaronc commented Mar 29, 2022

I support this. Less complexity and cleaner solution to the amino signing issue. Also prevents breaking changes when we introduce the orderbook model. Essentially Buy would be create buy order and Sell would be create sell order, BuyDirect would not create a buy order and not include disable_partial_fills and expiration.

When there is an order book approach, BuyDirect will create a buy order, but it should either succeed or fail in the next batch so expiration isn't needed. I think separate methods are okay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: x/ecocredit Issues that update the x/ecocredit module Type: Feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants