darkforestry / amms-rs

A Rust library to interact with automated market makers across EVM chains.
457 stars 124 forks source link

bug: pools silently fail to populate #79

Open georgewhewell opened 1 year ago

georgewhewell commented 1 year ago

hi, thanks for the lib, looks great :+1:

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

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 commented 1 year ago

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

0xOsiris commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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