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

bug: pools silently fail to populate #79

Open
georgewhewell opened this issue Aug 3, 2023 · 4 comments
Open

bug: pools silently fail to populate #79

georgewhewell opened this issue Aug 3, 2023 · 4 comments

Comments

@georgewhewell
Copy link

hi, thanks for the lib, looks great 👍

i found several pools that don't populate properly and remain with all values initialized to zero. can verify with the following test:

#[tokio::test]
async fn test_get_pool_data_regression() -> eyre::Result<()> {
    let rpc_endpoint = std::env::var("ETHEREUM_RPC_ENDPOINT")?;
    let middleware = Arc::new(Provider::<Http>::try_from(rpc_endpoint)?);

    let mut pool = UniswapV2Pool {
        address: H160::from_str("0x02f4b5deaee235e0c86f1af1831fba68f98b6ba3")?,
        ..Default::default()
    };

    pool.populate_data(None, middleware.clone()).await?;

    println!("Populating data for pool: {:?}", pool);
    assert!(false);
    Ok(())
}

the pools i found are

  • 0x02f4b5deaee235e0c86f1af1831fba68f98b6ba3
  • 0x5346f32df22b39f24fa8f2cd67ad03def11c45f6
  • 0x3e268993ad21c5756bfa5ce408b8961160d5a2d5
  • 0xaed6f7f0a68bbd1b7c25d050e46f01d93c66ae45

i don't see anything wrong with them on etherscan. it would be great if the populate_data method returned an error in this case rather than silently failing

@0xKitsune
Copy link
Collaborator

Thanks for opening up this issue, I'll take a look at this later today.

@0xOsiris
Copy link
Member

0xOsiris commented Aug 3, 2023

0x02f4b5deaee235e0c86f1af1831fba68f98b6ba3 returns an empty pool because the token1 decimals are 0.
0x3e268993ad21c5756bfa5ce408b8961160d5a2d5 token0 decimals are 0.
0xaed6f7f0a68bbd1b7c25d050e46f01d93c66ae45 token0 decimals are 0.

Checkout the GetUniswapV2PoolDataBatchRequest.sol contract to see the criteria to which we discard the pool.

Feel free to change it if you would like to keep these pools.

@georgewhewell
Copy link
Author

cool, thanks for the quick response. i don't care about these pools but i don't want these empty/failed pools in the output. maybe we can add a check in populate_pool_data_from_tokens to return None if all fields are zero? lmk what works and i can pr

@0xOsiris
Copy link
Member

0xOsiris commented Aug 3, 2023

We have a function

pub fn remove_empty_amms(amms: Vec<AMM>) -> Vec<AMM> {}

in the sync module. It is called during sync which cleans out all of the empty AMMs from the batch requests. So, when you sync the v2 pools from the factory it should be cleaning out the empty AMMs AFAIK. But I don't have a problem with cleaning up empty AMMs in the pool modules as well if you are calling the pool modules directly to populate the data.

@0xKitsune What do you think

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

3 participants