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

chore: Port settlement encoding to driver domain #2215

Closed
3 tasks
fleupold opened this issue Dec 27, 2023 · 1 comment · Fixed by #2661 or #2670
Closed
3 tasks

chore: Port settlement encoding to driver domain #2215

fleupold opened this issue Dec 27, 2023 · 1 comment · Fixed by #2661 or #2670
Assignees
@fleupold
Labels
E:3.1 Driver Colocation See https://github.com/cowprotocol/pm/issues/14 for details

Comments

@fleupold
Copy link
Contributor

Background

We are still relying on some legacy solver modules mainly for settlement encoding and scoring. Those are currently encapsulated inside driver::boundary::setttlement. Since we are not using the solver crate anymore, we can carefully move out the parts that are still relevant (I found a bunch of code related to requirements we no longer have)

Details

The best plan of attack is not yet immediately clear to me, but aspects of this include

  • Port the interaction encoding for protocol provided liquidity (uniswap, balancer, etc) to domain
  • Port general encoding of trades, clearing prices, approvals, etc
  • Port settlement merging

Acceptance criteria

No more dependencies on solver::{interactions, liquidity, settlement} in the driver crate
@fleupold fleupold added the E:3.1 Driver Colocation See https://github.com/cowprotocol/pm/issues/14 for details label Dec 27, 2023
@fleupold fleupold mentioned this issue Dec 28, 2023
Merged
@github-actions GitHub Actions
Copy link

This issue has been marked as stale because it has been inactive a while. Please update this issue or it will be automatically closed.

@github-actions github-actions bot added the stale label Feb 26, 2024
@github-actions github-actions bot closed this as completed Mar 4, 2024
@fleupold fleupold reopened this Mar 4, 2024
@fleupold fleupold self-assigned this Mar 4, 2024
@github-actions github-actions bot removed the stale label Mar 5, 2024
This was referenced Apr 18, 2024
Merged
Closed
fleupold added a commit that referenced this issue Apr 20, 2024
@fleupold
Isolate Settlement Encoding logic (#2632)
2729e8a 
# Description
In order to move the settlement encoding logic from the boundar
module/legacy solver crate into the domain it helps to first isolate the
call to encode into a single place. This will then allow us to wrap this
invocation behind a conditional config variable to control the rollout
(once implemented)

# Changes

- [x] Instead of storing a `boundary::Settlement` on
`domain::Settlement` store a struct that references `domain::eth::Tx`
directly (one for internalized interactions, one for uninternalised
interactions repr)
- [x] Find alternatives for the remaining calls to `settlement.boundary`
(most of them are already exposed on the solution directly)

## How to test
CI

## Related Issues

First step of #2215
@squadgazzz squadgazzz mentioned this issue Apr 20, 2024
Closed
This was referenced Apr 22, 2024
Closed
Merged
Merged
Merged
Merged
Merged
fleupold added a commit that referenced this issue Apr 29, 2024
@fleupold
Extract custom clearing price logic (#2659)
b402875 
# Description
For the domain based settlement encoding we need to encode both the
uniform clearing prices of the solution as well as the custom (fee
adjusted) clearing prices of each trade. This PR create a utility
function on `Trade` that allows this logic to be shared.

After rebasing, I noticed @Mateo-mro already introduced a similar
function (`calculate_custom_price`). However, this one wasn't reusing
the existing math to calculate net buy and sell amounts and had a
slightly longer name, so this is now mainly a refactoring/renaming of
this method.

# Changes
- [x] Move `CustomClearingPrice` type to up since it won't be `scoring`
specific
- [x] Reuse buy/sell amount calculation to derive the clearing price
- [x] Rename use of `calculate_custom_price`

## How to test
CI

## Related Issues

Part of #2215
fleupold added a commit that referenced this issue Apr 29, 2024
@fleupold
Prepare domain submission config (#2660)
d8c89e9 
# Description
Since settlement encoding is a crucial and hairy part of our logic we
don't want to simply switch from the battle-tested existing system to a
new implementation but rather make this configurable so we can roll it
out per network/environment and have an easy lever to revert in case
something goes wrong.

# Changes
- [x] Introduce new domain/boundary settlement encoding config type
- [x] Prepare conditional logic (with a TODO for the domain logic for
now)

## How to test

cargo run --bin driver -- --ethrpc http://localhost:8545 --config
crates/driver/example.toml

- See Boundary is used by default
- Add encoding = "Domain" to the top of the yaml file, see we are now
using Domain encoding

## Related Issues

Part of #2215
fleupold added a commit that referenced this issue Apr 29, 2024
@fleupold @MartinquaXD
Domain Settlement Encoding (#2661)
6bb059b 
# Description
Main part of #2215. This PR ports the settlement encoding logic which is
currently contained in the legacy solver crate's
[`SettlemenEncoder`](https://github.com/cowprotocol/services/blob/81831022cc41a05e76547b90e87694dc349a0388/crates/solver/src/settlement/settlement_encoder.rs#L37)
to our domain.

It is importantly still missing application of slippage tolerance (which
is contained in
[`SlippageContext`](https://github.com/cowprotocol/services/blob/81831022cc41a05e76547b90e87694dc349a0388/crates/solver/src/liquidity/slippage.rs#L123),
porting which would have made this PR even larger.

# Changes
- [x] Introduce encoding module which exposes a function to take a
solution and returns a settlement tx object
- [x] Encode clearing prices, trades & interactions the way the
settlement contract expects (guided by the boundary implementation)
- [x] Implement `revertable` logic on `Solutions` (this also fixes
#2640)
- [x] Make allowance computation internalisation aware (currently
allowances for internalised interactions are added to the execution plan
even if we are building an internalised one)
- [x] Unify `swap` implementation of the liquidity types to always
return `Result` (one returned `Option`)

## How to test
- [ ] Change default `encoding::Strategy` from Boundary to Domain and
run tests.

> [!IMPORTANT]  
> We still need to implement the slippage computation to make this
strategy fully working


Fixes #2215 & #2640

---------

Co-authored-by: Martin Beckmann <[email protected]>
This was referenced Apr 29, 2024
Merged
Merged
fleupold added a commit that referenced this issue Apr 30, 2024
@fleupold
Domain Slippage Calculation (#2669)
908f13f 
# Description
Follow up PR for #2661

This PR adds slippage tolerance to the AMM interactions when encoding
their execution.

# Changes
- [x] Introduce slippage modules which takes a similar role as
`SlippageContext` and `SlippageCalculator` in the old codebase
- [x] Expose auction prices to settlement (as those are required for
slippage)
- [x] Reuse exsiting `in_eth` method on `Ether` and add an equivalent
`from_eth` method for conversion in and out of native tokens.
- [x] Unit tests for slippage calculations

## How to test
Unit tests + CI on next PR, which makes new encoding the default

## Related Issues

Part of #2215
fleupold added a commit that referenced this issue Apr 30, 2024
@fleupold
Encode unwraps in domain based settlement encoding (#2670)
55b5f17 
# Description
Last part of the settlement encoding PR stack (for now). We didn't yet
encode WETH unwraps in the domain settlement which made native eth buy
orders fail.

# Changes
- [x] Pass in WETH contracts to encode the unwrap interaction
- [x] Track the total buy amount of ETH while encoding orders. If non
zero, append an unwrap interaction at the end

## How to test
Integration & e2e tests are now passing when run with Domain based
settlement encoding 🤞

> [!Important]
> 5df0650 (making Domain default)
should be reverted before merging

## Related Issues

Fixes #2215
fleupold added a commit that referenced this issue May 1, 2024
@fleupold
Log domain settlement even when using boundary encoding (#2677)
6b6e516 
# Description
This is to ensure we can soft rollout in production (keeping the
original encoding but warning cases in which the two don't match)

# Changes
- [x] Log when calldata of both ways to encode doesn't match

## How to test
Run the driver in shadow mode, see solutions being produced and no error
log happening

## Related Issues

Prepares rollout of #2215
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fleupold fleupold

Labels
E:3.1 Driver Colocation See https://github.com/cowprotocol/pm/issues/14 for details
Projects
None yet
Milestone
No milestone
1 participant
@fleupold