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

discover factories fails #87

Open
iht76 opened this issue Aug 16, 2023 · 8 comments
Open

discover factories fails #87

iht76 opened this issue Aug 16, 2023 · 8 comments

Comments

@iht76
Copy link

iht76 commented Aug 16, 2023

I am trying discover factories and it fails after a while with Discovering new factories...Error: Middleware error

this is using alchemy :
{
jsonrpc:"2.0",
id:174,
error:{
code:-32602,
message:"Log response size exceeded. You can make eth_getLogs requests with up to a 2K block range and no limit on the response size, or you can request any block range with a cap of 10K logs in the response. Based on your parameters and the response size limit, this block range should work: [0x1067380, 0x1072ee6]"
}
}

the request from our side :
{
id:174,
jsonrpc:"2.0",
method:"eth_getLogs",
params:[
{
fromBlock:"0x1067380",
toBlock:"0x107fa1f",
topics:[
[
"0x0d3648bd0f6ba80134a33ba9275ac585d9d315f0ad8355cddefde31afa28d0e9",
"0x783cca1c0412dd0d695e784568c96da2e9c22ff989357a2e8b1d9b2b4e6b7118"
]
]
}
]
}

@iht76
Copy link
Author

iht76 commented Aug 16, 2023

same thing happens with BASE, using their RPC

@0xKitsune
Copy link
Collaborator

You can decrease the step when discovering factories as well as syncing. As a side note, we need to update our docs across the codebase so that things like this are clear.

@vyorkin
Copy link
Contributor

vyorkin commented Aug 16, 2023

For Alchemy step = 50000 should work

@iht76
Copy link
Author

iht76 commented Aug 17, 2023

I tried on BASE, rpc_endpoint = "https://mainnet.base.org", step =2000 is the max, and it discovers only 1 factory, which is not correct :

All factories discovered
Factories: [UniswapV2Factory(UniswapV2Factory { address: 0x1b8128c3a1b7d20053d10763ff02466ca7ff99fc, creation_block: 1931623, fee: 0 })]

Also the data for this factory is not correct - creation block is wrong.

However, I was doing some debugging and it seems meanwhile in

identified_factories.insert(log.address, (factory, 0));

identified_factories grows to some reasonable size, like 100+ which is probably correct for BASE.

further I noticed that :

for (_, (factory, amms_length)) in identified_factories {
if amms_length >= number_of_amms_threshold {
filtered_factories.push(factory);
}
}

why >= ? (amms_length >= number_of_amms_threshold {

shouldn't it be <= ?

@vyorkin
Copy link
Contributor

vyorkin commented Aug 17, 2023

As I understand this parameter exists to filter out AMMs/DEXes that are not "popular"/"active"/etc. You can set the threshold to 0 making the condition amms_length >= number_of_amms_threshold always be true.


More thoughts on this...
While a simple numerical threshold like the number_of_amms_threshold parameter can help filter out not-so-interesting DEXes, we might want to have a more general filtering mechanism (to ensure liquidity depth, quality & safety of AMMs, LPs & tokens and so on). Maybe something like fn(&Factory) -> bool, fn(&AMM) -> bool.

@vyorkin
Copy link
Contributor

vyorkin commented Aug 17, 2023

Ah, just noticed that amms-rs has some filtering strategies already included. Nice

@0xOsiris
Copy link
Member

I tried on BASE, rpc_endpoint = "https://mainnet.base.org", step =2000 is the max, and it discovers only 1 factory, which is not correct :

All factories discovered Factories: [UniswapV2Factory(UniswapV2Factory { address: 0x1b8128c3a1b7d20053d10763ff02466ca7ff99fc, creation_block: 1931623, fee: 0 })]

Also the data for this factory is not correct - creation block is wrong.

However, I was doing some debugging and it seems meanwhile in

identified_factories.insert(log.address, (factory, 0));

identified_factories grows to some reasonable size, like 100+ which is probably correct for BASE.

further I noticed that :

for (_, (factory, amms_length)) in identified_factories { if amms_length >= number_of_amms_threshold { filtered_factories.push(factory); } }

why >= ? (amms_length >= number_of_amms_threshold {

shouldn't it be <= ?

Yeah exactly, The number_of_amms_threshold is the minimum amount of pools a factory needs to have to be discovered. So, if a factory has at least that many pools we push it to the filtered_factories

@0xOsiris
Copy link
Member

0xOsiris commented Aug 17, 2023

The creation block number is actually a bug I think, thanks for bringing this up. We are setting the creation_block to the log.block_number but the log is just the first instance of a PoolCreated/PairCreated event signature. So, this is actually not the creation block of the factory it's the block number of the first pool creation from the factory. However using that as the creation block should still keep state synced since there's no state to sync prior to that block number. @0xKitsune

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants