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

Fix network fee computation #400

Merged
merged 4 commits into from
Sep 24, 2024
Merged

Fix network fee computation #400

merged 4 commits into from
Sep 24, 2024

Conversation

fhenneke
Copy link
Collaborator

This PR changes the computation of network fees for solver payments. This change is required due to a change to how the fee entry in the settlement_observations table is computed, see cowprotocol/services#2956.

Network fee computation

With this PR, network fees are computed using data of executed trades. The surplus_fee computed in order_executions is always such that

(sell_amount - surplus_fee) / buy_amount = buy_clearing_price / sell_clearing_price

with uniform clearing prices.
If it were not for protocol fees and network fees, the order would trade at uniform clearing price.

(sell_amount - sell_protocol_fee - network_fee) / (buy_amount + buy_protocol_fee) = buy_clearing_price / sell_clearing_price

Combining it with the formula for surplus_fee gives

(sell_amount - sell_protocol_fee - network_fee) / (buy_amount + buy_protocol_fee) = (sell_amount - surplus_fee) / buy_amount

Traded amounts and surplus_fee are stored in the database. Protocol fees can be computed independently. Thus we can rearange the formula for network_fee,

network_fee = surplus_fee - sell_protocol_fee - (sell_amount - surplus_fee) / buy_amount * buy_protocol_fee

If the protocol fee is charged only in the sell token (i.e. for buy orders), this gives

network_fee = surplus_fee - sell_protocol_fee

If the protocol fee is charged in the buy token (i.e. for sell orders), this gives

network_fee = surplus_fee - (sell_amount - surplus_fee) / buy_amount * buy_protocol_fee

These formulas are implemented now.

Additional changes

There were other minor changes required to make this work.

  • Information on sell and buy tokens was not available for all trades in the orders table. Now, the orders table is joined with the jit_orders table.
  • Protocol fees are not computed for all trades. There is an additional join on order_surplus to pick up all orders for network fee computation. Missing protocol fees are coalesced to zero.
  • Not all prices need to be available for jit orders. Thus all joins on prices were changed to left outer joins.
  • The query became super slow for some reason. I added a materialized for one of the tables (which I found to be fast during debugging) and the query runs reasonable fast now.

I realized that there are edge cases where a jit order is supposed to charge a fee but the price of the sell token is not available. In that case, with this PR and the old code, the network fee of such a trad would have been set to zero.

Test plan

I adapted unit tests. Network fee amounts changed a bit since before the computation depended on the settlement_observations table and not it depends on order_executions. Those tables are not consistent in that the fee entry does correspond to what the autopilot would compute when observing data stored in order_executions. The order of magnitude of the new results is easy to check as the difference of the sum of surplus fees (converted to ETH) and the sum of protocol fees. So the new tests seem correct.

I ran the query on the one-day period 2024-09-16--2024-09-17 and compared old and new network fees. They were identical up to rounding errors.

I ran the query on the one-day period 2024-09-17--2024-09-18 and compared old and new network fees. They were different, as is to be expected.

I looked into transaction with hash 0xcca1897297913e58fa9fae51c9b8f93c4f6c4aebce0b7a6a5fbceaff309ef37e in particular since that trade had negative network fees with the old query. The new query gives numbers consistent with our new Dune queries.

I also ran the full payment script for the accounting period 2024-09-10--2024-09-17 (commenting out slippage, which seems to have a bug). The only difference in rewards, protocol fees, and partner fees is that the actual payment reported a protocol fee of 27.8759 ETH while the new run gives 27.8662 ETH. There should be no difference. So there might be some small bug in the code somewhere, potentially ignoring some trade or converting using a wrong price.

instead of relying on the fee entry in the settlement scores table
@fhenneke fhenneke requested a review from harisang September 23, 2024 11:40
WHERE
ss.block_deadline >= {{start_block}}
AND ss.block_deadline <= {{end_block}}
ss.block_deadline >= 20759465
Copy link
Contributor

Choose a reason for hiding this comment

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

probably leftover from local testing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. That will hopefully explain the small discrepancy in total protocol fees.

fhenneke added a commit to cowprotocol/dune-sync that referenced this pull request Sep 24, 2024
This PR mirrors the implementation of
cowprotocol/solver-rewards#400

It should be adapted when the original implementation if changed. It
needs to be merged, hotfixed into prod, and data for the accounting week
starting on 2024-09-17 needs to be resynced. Otherwise the dashboard for
the next payment will be out of sync with solver-rewards.
@harisang harisang merged commit 867cae4 into main Sep 24, 2024
5 checks passed
@harisang harisang deleted the fix_network_fees branch September 24, 2024 10:27
@github-actions github-actions bot locked and limited conversation to collaborators Sep 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants