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 Limit Order Modification Feature #2743

Merged
merged 31 commits into from
May 15, 2023
Merged

Implement Limit Order Modification Feature #2743

merged 31 commits into from
May 15, 2023

Conversation

abitmore
Copy link
Member

@abitmore abitmore commented Apr 28, 2023

PR for #1604.

Based on @nathanielhourt's work for #1604 (see nathanielhourt#1).

Changes:

  • New operation: limit_order_update_operation, so that traders can update their limit orders directly
    • operation_type = 77
    • default fee 0.375 BTS (slightly lower than the current order creation fee on the BitShares Mainnet, and applicable to all refund mechanisms)
     struct limit_order_update_operation : public base_operation
     {
         struct fee_params_t {
             uint64_t fee = ( GRAPHENE_BLOCKCHAIN_PRECISION * 3 ) / 8;
         };
    
         asset fee;
         account_id_type seller;
         limit_order_id_type order;
         optional<price> new_price;
         optional<asset> delta_amount_to_sell;
         optional<time_point_sec> new_expiration;
         /// Automatic actions when the limit order is filled or partially filled
         optional< vector< limit_order_auto_action > > on_fill;
    
         extensions_type extensions;
     };
    

NOTE 1:
The base amount in the new price is limited by an estimation of the order's maximum amount to sell. This is to mitigate against inappropriate price manipulation, although not completely avoid it.

NOTE 2:
When updating a limit order with a non-zero deferred fee, we charge the owner a small fee equal to order_cancel_fee * order_update_fee / order_create_fee (not higher than the deferred fee), and refund the remainder in the deferred fee, the deferred fee is then refreshed with the new operation fee paid.
The small fee is to mitigate potential transaction spam issues.
The deferred fee is used for all refund mechanisms.

Side effect:
Assuming that order update fee will be lower than order creation fee, traders can create an order and then update it immediately to reduce costs, both for making and taking.

NOTE 3:
Since the price can be modified, sell_price.base.amount is no longer always equal to the total amount allocated to the limit order. In the meanwhile, a new data field filled_amount is added to limit_order_object in PR #2776, this way users can still know the total amount.

NOTE 4:
Please see #2749 for more info about automatic actions (Order-Sends-Order).

Define and implement the limit_order_update_operation. Still needs
testing.
Some major facepalms in here... haha! but overall, it's looking good.
Add logic to match an updated limit order if the price is changed to be
higher than the previous top-of-book price.

I believe this completes #1604
Add dust check to limit order update logic, and test
Updating an order's expiration must make it expire later than before,
not the same or sooner.
I pulled this out of a2192ec because I
needed it separated from the rest of that commit.
Guard against the comittee setting fees for the operation which is not
yet defined.
@abitmore abitmore changed the title Limit Order Modification and Order-Sends-Order Limit Order Modification and Simple Order-Sends-Order Apr 28, 2023
Resolved conflicts:
- libraries/chain/include/graphene/chain/market_evaluator.hpp
- libraries/chain/include/graphene/chain/protocol/fee_schedule.hpp
- libraries/chain/include/graphene/chain/protocol/operations.hpp
- libraries/chain/proposal_evaluator.cpp
- libraries/protocol/include/graphene/protocol/market.hpp
- tests/common/database_fixture.cpp
libraries/chain/market_evaluator.cpp Outdated Show resolved Hide resolved
libraries/chain/market_evaluator.cpp Outdated Show resolved Hide resolved
libraries/chain/market_evaluator.cpp Outdated Show resolved Hide resolved
libraries/chain/market_evaluator.cpp Outdated Show resolved Hide resolved
libraries/chain/market_evaluator.cpp Outdated Show resolved Hide resolved
libraries/protocol/include/graphene/protocol/market.hpp Outdated Show resolved Hide resolved
libraries/chain/market_evaluator.cpp Show resolved Hide resolved
tests/tests/operation_tests.cpp Show resolved Hide resolved
@abitmore abitmore changed the title Limit Order Modification and Simple Order-Sends-Order Implement Limit Order Modification Feature Apr 30, 2023
@abitmore abitmore linked an issue Apr 30, 2023 that may be closed by this pull request
17 tasks
@abitmore abitmore self-assigned this May 10, 2023
@abitmore abitmore marked this pull request as ready for review May 12, 2023 20:17
@sonarcloud
Copy link

sonarcloud bot commented May 15, 2023

Please retry analysis of this Pull-Request directly on SonarCloud.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Operation to modify existing limit order
2 participants