-
Notifications
You must be signed in to change notification settings - Fork 58
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
Add Nile V3 specific fee support #518
base: add_linea
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -520,10 +520,10 @@ def _univ3_to_cpc(self) -> List[Any]: | |
"tick": self.tick, | ||
"liquidity": self.liquidity, | ||
} | ||
feeconst = self.FEE_LOOKUP.get(float(self.fee_float)) | ||
feeconst = self.FEE_LOOKUP.get(int(self.fee)/1e6) | ||
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. How is this change related to this PR? 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 maintained the original format of this by switching fee_float -> fee due to fee_float no longer being reliable. See above. |
||
if feeconst is None: | ||
raise ValueError( | ||
f"Illegal fee for Uniswap v3 pool: {self.fee_float} [{self.FEE_LOOKUP}]]" | ||
f"Illegal fee for Uniswap v3 pool: {int(self.fee)} [{self.FEE_LOOKUP}]]" | ||
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. How is this change related to this PR? 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. See above regarding fee_float & fee. |
||
) | ||
uni3 = Univ3Calculator.from_dict(args, feeconst, addrdec=self.ADDRDEC) | ||
params = uni3.cpc_params() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -206,7 +206,7 @@ def get_custom_int(self) -> int: | |
pool = self.pool | ||
custom_int = 0 | ||
if self.exchange_name in self.ConfigObj.UNI_V3_FORKS: | ||
custom_int = int(Decimal(pool.fee_float) * Decimal("1000000")) | ||
custom_int = int(pool.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. How is this change related to this PR? 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. Nile V3 includes a fee override. This PR stores the fee override for the pool in the The Fast Lane arb contract must know the Consequently the 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 did not understand this description. 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. Correct, and to handle it the same, we must now use the |
||
elif self.exchange_name in self.ConfigObj.SOLIDLY_V2_FORKS: | ||
custom_int = 0 if pool.pool_type != self.ConfigObj.network.POOL_TYPE_STABLE else 1 | ||
elif self.exchange_name in self.ConfigObj.BALANCER_NAME: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,8 @@ | |
from pandas import DataFrame | ||
from web3.contract import Contract | ||
|
||
from fastlane_bot.config.constants import NILE_V3_NAME | ||
|
||
load_dotenv() | ||
import os | ||
import requests | ||
|
@@ -61,7 +63,7 @@ | |
"coinbase_base": 250000, | ||
"fantom": 2000, | ||
"mantle": 10000000, | ||
"linea": 50000 | ||
"linea": 5000 | ||
} | ||
|
||
ALCHEMY_KEY_DICT = { | ||
|
@@ -466,6 +468,10 @@ def organize_pool_details_uni_v3( | |
return None | ||
pair = pair[:-1] | ||
fee = pool_data["args"]["fee"] | ||
fee_float = float(fee) / 1000000 | ||
if exchange == NILE_V3_NAME: | ||
pool = web3.eth.contract(address=web3.to_checksum_address(pool_address), abi=NILE_V3_POOL_ABI) | ||
fee_float = pool.caller.currentFee() / 1000000 | ||
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. Why function 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. And why you using 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. Also, creating a 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. Also, it doesn't make sense that we need to make an RPC at this point, as the fee has already been obtained previously. 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 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. Please see the PR spawned off of this one. The part:
Doesn't look like it belongs here. 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. You are correct that no other Uni V3 fork requires an additional call to fetch the fee, however, Nile V3 does. The fee for other Uni V3 forks is obtained from the pool creation event. However, this is not sufficient for Nile V3 due to implementing a fee override. Where else do you suggest placing this call given that no other Uni V3 forks require it? |
||
|
||
description = exchange + " " + pair + " " + str(fee) | ||
|
||
|
@@ -478,7 +484,7 @@ def organize_pool_details_uni_v3( | |
"pair_name": pair, | ||
"exchange_name": exchange, | ||
"fee": fee, | ||
"fee_float": float(fee) / 1000000, | ||
"fee_float": fee_float, | ||
"address": pool_address, | ||
"anchor": "", | ||
"tkn0_address": token_info["tkn0_address"], | ||
|
@@ -1182,7 +1188,7 @@ def terraform_blockchain(network_name: str, web3: Web3 = None, async_web3: Async | |
continue | ||
print(f"********************** Terraforming **********************\nStarting exchange: {exchange_name} from block {from_block}") | ||
|
||
if fork in "uniswap_v2": | ||
if fork == UNISWAP_V2_NAME: | ||
if fee == "TBD": | ||
continue | ||
if fork in SOLIDLY_FORKS: | ||
|
@@ -1205,7 +1211,7 @@ def terraform_blockchain(network_name: str, web3: Web3 = None, async_web3: Async | |
) | ||
m_df = m_df.reset_index(drop=True) | ||
univ2_mapdf = pd.concat([univ2_mapdf, m_df], ignore_index=True) | ||
elif fork in "uniswap_v3": | ||
elif fork == UNISWAP_V3_NAME: | ||
if fee == "TBD": | ||
continue | ||
add_to_exchange_ids(exchange=exchange_name, fork=fork) | ||
|
@@ -1223,7 +1229,7 @@ def terraform_blockchain(network_name: str, web3: Web3 = None, async_web3: Async | |
) | ||
m_df = m_df.reset_index(drop=True) | ||
univ3_mapdf = pd.concat([univ3_mapdf, m_df], ignore_index=True) | ||
elif "solidly" in fork: | ||
elif fork == SOLIDLY_V2_NAME: | ||
add_to_exchange_ids(exchange=exchange_name, fork=fork) | ||
|
||
factory_abi = SOLIDLY_EXCHANGE_INFO[exchange_name]["factory_abi"] | ||
|
@@ -1246,7 +1252,7 @@ def terraform_blockchain(network_name: str, web3: Web3 = None, async_web3: Async | |
) | ||
m_df = m_df.reset_index(drop=True) | ||
solidly_v2_mapdf = pd.concat([solidly_v2_mapdf, m_df], ignore_index=True) | ||
elif "balancer" in fork: | ||
elif fork == BALANCER_NAME: | ||
try: | ||
subgraph_url = BALANCER_SUBGRAPH_CHAIN_URL[network_name] | ||
u_df = get_balancer_pools(subgraph_url=subgraph_url, web3=web3) | ||
|
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.
There is something really wrong here, as in one case you are dividing by the resolution, and in one case you don't.
So
fee_float
is sometimes returned after being divided by16e
, and sometimes being returned as read from the contract.How could that possibly work correctly further down the road, when the caller of this function receives this returned value and needs to processes it?
Also note that you seem to have made a similar change in file
run_blockchain_terraformer.py
, where you actually do divide by1000000
in both cases.So even without taking the above into consideration, it is pretty clear that at least one of these two changes is incorrect.
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.
In addition to that, it makes much more sense to implement a separate file called
nile_v3.py
.This type of logic becomes increasingly confusing:
The guiding thumb-rule should be that once a simple branching (if/else) becomes too obfuscated, it is time to consider an alternative approach.
Note that an additional logic was also applied in function
get_fee
further down in this file, and on top of making it more cumbersome, this one actually introduces a bug:As the value of
fee_float
is scaled down by1e6
in some scenarios, and not scaled down at all in other scenarios.So unless you apply the same logic (
if exchange_name == NILE_V3_NAME
) in every function which calls this function, an incorrect calculation most likely takes place.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.
After a short research, the best approach here would be a paradigm similar to the one in solidly_v2.py.