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

bug: External Solvers Invalid Trade Execution Errors #1996

Closed
fleupold opened this issue Oct 20, 2023 · 0 comments · Fixed by #1998
Closed

bug: External Solvers Invalid Trade Execution Errors #1996

fleupold opened this issue Oct 20, 2023 · 0 comments · Fixed by #1998
Assignees
@fleupold
Labels
bug Something isn't working E:3.1 Driver Colocation See https://github.com/cowprotocol/pm/issues/14 for details

Comments

@fleupold
Copy link
Contributor

Problem

We have not yet seen a single external solver settlement since colocation has been enabled in staging (only dex + baseline solvers have found solutions)

To reproduce

Place an order, observe e.g. Barter logs and see we are getting

solvers::domain::solver::legacy: failed to solve auction err=invalid trade execution

Expected behaviour

Produce a solution that gets evaluated
@fleupold fleupold added bug Something isn't working E:3.1 Driver Colocation See https://github.com/cowprotocol/pm/issues/14 for details labels Oct 20, 2023
@fleupold fleupold changed the title bug: bug: External Solvers Invalid Trade Execution Errors Oct 20, 2023
@fleupold fleupold self-assigned this Oct 20, 2023
@fleupold fleupold mentioned this issue Oct 20, 2023
Merged
fleupold added a commit that referenced this issue Oct 20, 2023
@fleupold
Fix Legacy Solver (#1998)
11f62fd 
# Description
Solvers already submit self computed fee amounts even for orders that
still work with a protocol specified fee (e.g. market orders). See this
barter response for instance: http://jsonblob.com/1164883806913421312

The code currently rejects those solutions with "invalid trade
execution" errors. This PR makes the legacy adapter compliant with the
current system where we ignore the fee_amount for orders that don't
require them (rather than erroring if it specified)

# Changes

- Instead of erroring when a surplus fee is specified although the
solver doesn't determine the fee we now always use the protocol fee
instead
- adjust unit test to demonstrate the response is now accepted

## How to test
Unit test

Fixes #1996
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fleupold fleupold

Labels
bug Something isn't working E:3.1 Driver Colocation See https://github.com/cowprotocol/pm/issues/14 for details
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix Legacy Solver cowprotocol/services
1 participant
@fleupold