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

feat: Reject orders with signed fee_amount != 0 #2437

Closed
sunce86 opened this issue Feb 26, 2024 · 2 comments · Fixed by #2454
Closed

feat: Reject orders with signed fee_amount != 0 #2437

sunce86 opened this issue Feb 26, 2024 · 2 comments · Fixed by #2454

Comments

@sunce86
Copy link
Contributor

sunce86 commented Feb 26, 2024

Problem

Starting from 19th of March (as stated in CIP38) orders with non-zero fee should be rejected by the protocol.

Suggested solution

Rejection means the orders with non-zero fees will no longer be settled by the protocol, implying:

  1. Backend API needs to reject creation of new orders and return a specific error type in this case.
  2. Existing open orders within the orderbook should be filtered out in solvable orders query (when autopilot auction is built).

Additional context

Market orders cleanup in all components can be done once this is released.

Acceptance criteria

POST-ing a new order with non-zero fee fails with specific error.

@harisang
Copy link
Contributor

harisang commented Feb 26, 2024

I wonder whether we can avoid (2) (i.e., open orders being ignored and rejected) by instead doing the following:

  1. Decide on an activation block (some block after Tuesday March 19 midnight)
  2. Prevent new fixed-fee orders to be created that have an expiration deadline after the block decided in (1). I suspect this should only affect orders being created the last few hours before activation of the CIP.

@sunce86
Copy link
Contributor Author

sunce86 commented Feb 28, 2024

Possibly duplicate of #2384.

Will keep the issue open as it contains more detailed info around implementation itself.

sunce86 added a commit that referenced this issue Mar 4, 2024
# Description
Fixes #2437

Once merged, will create a pairing infra PR to set the date.

# Changes
<!-- List of detailed changes (how the change is accomplished) -->

- [ ] Rejects market orders from API
- [ ] Rejects market orders from eth flow
- [ ] Prevents market orders to be included into `Auction` just in case,
since the PR will land together with CIP38.

## How to test
Locally set the configuration parameter and run e2e tests with market
orders and they fail. For example,
`local_node_mixed_limit_and_market_orders`

<!--
## Related Issues

Fixes #
-->
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 a pull request may close this issue.

2 participants