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.

Sign up for GitHub

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

Jump to bottom

Remove Liquidity order type from Solvers #3059

Merged
merged 5 commits into from
Oct 16, 2024
Merged

Remove Liquidity order type from Solvers #3059

merged 5 commits into from
Oct 16, 2024

Conversation

sunce86
Copy link
Contributor

@sunce86 sunce86 commented Oct 14, 2024 •
edited
Loading

Description

A follow up for #3058

Removes Liquidity order type from solvers API.

Solvers PR: gnosis/solvers#77

Changes

  • Removed Liquidity order type from interface and from the domain

How to test

Existing tests.

@sunce86 sunce86 self-assigned this Oct 14, 2024
@sunce86 sunce86 requested a review from a team as a code owner October 14, 2024 15:20
@sunce86 sunce86 mentioned this pull request Oct 14, 2024
Merged
MartinquaXD
MartinquaXD reviewed Oct 15, 2024
View reviewed changes
Copy link
Contributor

@MartinquaXD MartinquaXD left a comment

Choose a reason for hiding this comment

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

Could you please also remove the liquidity order variant from the orderbook?
AFAIK it's not possible to create liquidity orders anymore so we should probably remove the related code in one big swoop.

@m-lord-renkse
Copy link
Contributor

Could you please also remove the liquidity order variant from the orderbook? AFAIK it's not possible to create liquidity orders anymore so we should probably remove the related code in one big swoop.

In that case, should we also remove it from the database and migrate current liquidity as limit? 🤔

@sunce86
Copy link
Contributor Author

sunce86 commented Oct 16, 2024 •
edited
Loading

Could you please also remove the liquidity order variant from the orderbook? AFAIK it's not possible to create liquidity orders anymore so we should probably remove the related code in one big swoop.

In that case, should we also remove it from the database and migrate current liquidity as limit? 🤔

This is a way more involving change I assume, will create a separate task for it.

#3060

@sunce86 sunce86 mentioned this pull request Oct 16, 2024
Open
@sunce86 sunce86 enabled auto-merge (squash) October 16, 2024 12:44
@sunce86 sunce86 merged commit 1c8c14f into main Oct 16, 2024
11 checks passed
@sunce86 sunce86 deleted the remove-liquidity-from-solvers branch October 16, 2024 12:48
@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@squadgazzz squadgazzz squadgazzz approved these changes

@MartinquaXD MartinquaXD MartinquaXD approved these changes

Assignees

@sunce86 sunce86

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sunce86 @m-lord-renkse @MartinquaXD @squadgazzz