-
Notifications
You must be signed in to change notification settings - Fork 0
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
Compute partner fees #52
Changes from 19 commits
02899a4
91d8446
d0b566a
800d62a
f82e436
85412aa
5f89a09
26de077
a6ad730
a9d621e
24546d2
add0483
491b1c1
443c743
0542d3c
22871ec
60f4842
f56c889
8b8698e
aa89381
5e48dfc
6405f17
84ce25d
8bc2d79
3f98527
9694f41
1c7291a
f5dcae2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,14 +5,14 @@ | |
import math | ||
import os | ||
from typing import Any | ||
import requests | ||
import json | ||
from dotenv import load_dotenv | ||
from eth_typing import Address | ||
from eth_typing import ChecksumAddress | ||
from hexbytes import HexBytes | ||
from web3 import Web3 | ||
|
||
from src.constants import ( | ||
REQUEST_TIMEOUT, | ||
) | ||
import requests | ||
from src.constants import REQUEST_TIMEOUT, NULL_ADDRESS | ||
|
||
# types for trades | ||
|
||
|
@@ -21,17 +21,39 @@ | |
class Trade: | ||
"""Class for""" | ||
|
||
order_uid: HexBytes | ||
sell_amount: int | ||
buy_amount: int | ||
sell_token: HexBytes | ||
buy_token: HexBytes | ||
limit_sell_amount: int | ||
limit_buy_amount: int | ||
kind: str | ||
sell_token_clearing_price: int | ||
buy_token_clearing_price: int | ||
fee_policies: list["FeePolicy"] | ||
def __init__( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a |
||
self, | ||
order_uid: HexBytes, | ||
sell_amount: int, | ||
buy_amount: int, | ||
sell_token: HexBytes, | ||
buy_token: HexBytes, | ||
limit_sell_amount: int, | ||
limit_buy_amount: int, | ||
kind: str, | ||
sell_token_clearing_price: int, | ||
buy_token_clearing_price: int, | ||
fee_policies: list["FeePolicy"], | ||
partner_fee_recipient: ChecksumAddress, | ||
harisang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
): | ||
self.order_uid = order_uid | ||
self.sell_amount = sell_amount | ||
self.buy_amount = buy_amount | ||
self.sell_token = sell_token | ||
self.buy_token = buy_token | ||
self.limit_sell_amount = limit_sell_amount | ||
self.limit_buy_amount = limit_buy_amount | ||
self.kind = kind | ||
self.sell_token_clearing_price = sell_token_clearing_price | ||
self.buy_token_clearing_price = buy_token_clearing_price | ||
self.fee_policies = fee_policies | ||
self.partner_fee_recipient = partner_fee_recipient # if there is no partner, then its value is set to the null address | ||
|
||
total_protocol_fee, partner_fee, network_fee = self.compute_all_fees() | ||
self.total_protocol_fee = total_protocol_fee | ||
self.partner_fee = partner_fee | ||
self.network_fee = network_fee | ||
return | ||
|
||
def volume(self) -> int: | ||
"""Compute volume of a trade in the surplus token""" | ||
|
@@ -62,20 +84,31 @@ def surplus(self) -> int: | |
return current_limit_sell_amount - self.sell_amount | ||
raise ValueError(f"Order kind {self.kind} is invalid.") | ||
|
||
def raw_surplus(self) -> int: | ||
"""Compute raw surplus of a trade in the surplus token | ||
First, the application of protocol fees is reversed. Then, surplus of the resulting trade | ||
is computed.""" | ||
def compute_all_fees(self) -> tuple[int, int, int]: | ||
raw_trade = deepcopy(self) | ||
for fee_policy in reversed(self.fee_policies): | ||
raw_trade = fee_policy.reverse_protocol_fee(raw_trade) | ||
return raw_trade.surplus() | ||
|
||
def protocol_fee(self): | ||
"""Compute protocol fees of a trade in the surplus token | ||
Protocol fees are computed as the difference of raw surplus and surplus.""" | ||
|
||
return self.raw_surplus() - self.surplus() | ||
total_protocol_fee = 0 | ||
partner_fee = 0 | ||
network_fee = 0 | ||
if self.fee_policies: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this line required? If not, it should be removed. If yes, a small comment might help understand why it is needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is line 98 that needs to execute after the for-loop finishes, but it only makes sense if there was at least one fee policy. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, i see. But to me it is simpler to write partner_fee = 0
for i, fee_policy in enumerate(reversed(self.fee_policies)):
raw_trade = fee_policy.reverse_protocol_fee(raw_trade)
## we assume that partner fee is the last to be applied
if i == 0 and self.partner_fee_recipient is not NULL_ADDRESS:
partner_fee = raw_trade.surplus() - self.surplus()
total_protocol_fee = raw_trade.surplus() - self.surplus() The other two initializations are not required. That part of the code might even benefit from a function def partner_fee(self):
if not self.fee_policies or self.partner_fee_recipient is NULL_ADDRESS:
return 0
# we assume that partner fee is the last to be applied
fee_policy = self.fee_policies[-1]
raw_trade = deepcopy(self)
raw_trade = fee_policy.reverse_protocol_fee(raw_trade)
return raw_trade.surplus() - self.surplus() and then just partner_fee = self.partner_fee()
total_protocol_fee = self.total_protocol_fee() The small loss in efficiency should not matter compared to the increase in readability. (Though you might disagree on the readability, then feel free to ignore). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually you are right and my concern was not really justified. I simplified the code and followed your first suggestion. |
||
for i, fee_policy in enumerate(reversed(self.fee_policies)): | ||
raw_trade = fee_policy.reverse_protocol_fee(raw_trade) | ||
## we assume that partner fee is the last to be applied | ||
if i == 0 and self.partner_fee_recipient is not NULL_ADDRESS: | ||
partner_fee = raw_trade.surplus() - self.surplus() | ||
total_protocol_fee = raw_trade.surplus() - self.surplus() | ||
|
||
surplus_fee = self.compute_surplus_fee() # in the surplus token | ||
network_fee_in_surplus_token = surplus_fee - self.total_protocol_fee | ||
if self.kind == "sell": | ||
network_fee = int( | ||
network_fee_in_surplus_token | ||
* Fraction( | ||
self.buy_token_clearing_price, self.sell_token_clearing_price | ||
) | ||
) | ||
else: | ||
network_fee = network_fee_in_surplus_token | ||
return total_protocol_fee, partner_fee, network_fee | ||
|
||
def surplus_token(self) -> HexBytes: | ||
"""Returns the surplus token""" | ||
|
@@ -336,6 +369,14 @@ def get_all_data(self, tx_hash: HexBytes) -> SettlementData: | |
buy_token_clearing_price = clearing_prices[buy_token] | ||
fee_policies = self.parse_fee_policies(trade_data["feePolicies"]) | ||
|
||
app_data = json.loads(order_data["fullAppData"]) | ||
if "partnerFee" in app_data["metadata"].keys(): | ||
partner_fee_recipient = Web3.to_checksum_address( | ||
HexBytes(app_data["metadata"]["partnerFee"]["recipient"]) | ||
) | ||
else: | ||
partner_fee_recipient = NULL_ADDRESS | ||
|
||
trade = Trade( | ||
order_uid=uid, | ||
sell_amount=executed_sell_amount, | ||
|
@@ -348,6 +389,7 @@ def get_all_data(self, tx_hash: HexBytes) -> SettlementData: | |
sell_token_clearing_price=sell_token_clearing_price, | ||
buy_token_clearing_price=buy_token_clearing_price, | ||
fee_policies=fee_policies, | ||
partner_fee_recipient=partner_fee_recipient, | ||
) | ||
trades.append(trade) | ||
|
||
|
@@ -436,48 +478,35 @@ def parse_fee_policies( | |
return fee_policies | ||
|
||
|
||
# computing fees | ||
def compute_fee_imbalances( | ||
settlement_data: SettlementData, | ||
) -> tuple[dict[str, tuple[str, int]], dict[str, tuple[str, int]]]: | ||
# function that computes all fees of all orders in a batch | ||
# Note that currently it is NOT working for CoW AMMs as they are not indexed. | ||
harisang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
def compute_all_fees_of_batch( | ||
tx_hash: HexBytes, | ||
) -> tuple[ | ||
dict[str, tuple[str, int]], | ||
dict[str, tuple[str, int, str]], | ||
dict[str, tuple[str, int]], | ||
]: | ||
orderbook_api = OrderbookFetcher() | ||
settlement_data = orderbook_api.get_all_data(tx_hash) | ||
Comment on lines
+484
to
+492
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Combining the functions |
||
protocol_fees: dict[str, tuple[str, int]] = {} | ||
network_fees: dict[str, tuple[str, int]] = {} | ||
partner_fees: dict[str, tuple[str, int, str]] = {} | ||
for trade in settlement_data.trades: | ||
# protocol fees | ||
protocol_fee_amount = trade.protocol_fee() | ||
protocol_fee_amount = trade.total_protocol_fee - trade.partner_fee | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that this also breaks the convention (maybe we should!) with what we upload to Dune. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed but somehow it makes sense to have a table where when we look for a protocol fee, this is indeed the protocol fee and not something that potentially includes other fees there as well. So i would like to keep this here to be as explicit as possible in the db, I.e., have:
And there are no subtractions or anything like that. Protocol fee goes to the DAO, partner fee goes to the partner, and network fee to solvers, and that's it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that the convention on Dune is a bit misleading. Maybe we should switch it there as well. I am not familiar with the legal accounting requirements, though. Partner fees might be revenue with an associated cost. Regarding partner fees going to partners: unfortunately that is not so easy as we take a cut. But generally I agree with this distinction. I expect the protocol fee mess to become even more ugly in the future, so having a reasonable intermediate representation sound most sustainable. In terms of consistency with backend: the planned changes might include protocol fees per fee policy. So it is even one level more fine grained in case we go crazy with stacking protocol fees (which I expect to happen). |
||
protocol_fee_token = trade.surplus_token() | ||
protocol_fees[trade.order_uid.to_0x_hex()] = ( | ||
protocol_fee_token.to_0x_hex(), | ||
protocol_fee_amount, | ||
) | ||
# network fees | ||
surplus_fee = trade.compute_surplus_fee() # in the surplus token | ||
network_fee = surplus_fee - protocol_fee_amount | ||
if trade.kind == "sell": | ||
network_fee_sell = int( | ||
network_fee | ||
* Fraction( | ||
trade.buy_token_clearing_price, trade.sell_token_clearing_price | ||
) | ||
) | ||
else: | ||
network_fee_sell = network_fee | ||
|
||
partner_fees[trade.order_uid.to_0x_hex()] = ( | ||
protocol_fee_token.to_0x_hex(), | ||
trade.partner_fee, | ||
trade.partner_fee_recipient, | ||
) | ||
network_fees[trade.order_uid.to_0x_hex()] = ( | ||
trade.sell_token.to_0x_hex(), | ||
network_fee_sell, | ||
trade.network_fee, | ||
) | ||
|
||
return protocol_fees, network_fees | ||
|
||
|
||
# combined function | ||
|
||
|
||
def batch_fee_imbalances( | ||
tx_hash: HexBytes, | ||
) -> tuple[dict[str, tuple[str, int]], dict[str, tuple[str, int]]]: | ||
orderbook_api = OrderbookFetcher() | ||
settlement_data = orderbook_api.get_all_data(tx_hash) | ||
protocol_fees, network_fees = compute_fee_imbalances(settlement_data) | ||
return protocol_fees, network_fees | ||
return protocol_fees, partner_fees, network_fees |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code now deviates sufficiently from the code in the circuit breaker to require actual tests.