MercuricChloride / substreams_module_repo

A collection of handy dandy substreams modules
5 stars 1 forks source link

Some unecessary `clone` #1

Open maoueh opened 1 year ago

maoueh commented 1 year ago

You should try to avoid clone on "big" especially nested object, as a clone is a doing a deep clone.

    let block_timestamp = blk
        .header
        .clone()
        .unwrap()
        .timestamp
        .unwrap()
        .seconds
        .to_string();

For example here, the clone on the header is not necessary, use a reference to header you will save a clone of the header:

     let block_timestamp = blk
        .header
        .as_ref()
        .unwrap()
        .timestamp
        .as_ref()
        .unwrap()
        .seconds
        .to_string();

Note We should probably offer a convenience for block timestamp, I realize :)

Here:

    let filtered_names: Vec<&str> = param.split("&&").collect::<Vec<_>>();
    let mut filtered_hotdogs: Vec<Hotdog> = vec![];
    for hotdog in hotdogs.hotdogs {
        if filtered_names.contains(&hotdog.hotdog_name.as_str()) {
            filtered_hotdogs.push(hotdog.clone());
        }
    }

The clone is not necessary at all, you own hotdog and giving ownership of it to .push(...), so cloning is not needed.

Here a re-write of filter_blur_trades with all clone removed:

 #[substreams::handlers::map]
fn filter_blur_trades(param: String, hotdogs: Hotdogs) -> Result<Hotdogs, SubstreamError> {
    let filtered_addresses: Vec<String> = param
        .split("&&")
        .map(|address| address.to_lowercase())
        .collect::<Vec<_>>();

    if filtered_addresses.len() == 1 {
        return Ok(Hotdogs {
            hotdogs: hotdogs.hotdogs,
        });
    }

    let mut filtered_hotdogs: Vec<Hotdog> = vec![];

    for hotdog in hotdogs.hotdogs {
        if hotdog.hotdog_name != "OrdersMatched" {
            continue;
        }

        let mut map = hotdog.to_hashmap();

        let buy = match map.remove("buy") {
            Some(buy) => buy,
            None => panic!("map does not contain a buy field {:?}", hotdog),
        };

        let sell = match map.remove("sell") {
            Some(sell) => sell,
            None => panic!("map does not contain a sell field {:?}", map),
        };

        match (buy, sell) {
            (ValueEnum::MapValue(mut buy_map), ValueEnum::MapValue(mut sell_map)) => {
                let buy_collection = buy_map.keys.remove("collection").unwrap();
                let sell_collection = sell_map.keys.remove("collection").unwrap();
                match (buy_collection.into(), sell_collection.into()) {
                    (
                        ValueEnum::StringValue(buy_collection),
                        ValueEnum::StringValue(sell_collection),
                    ) => {
                        if filtered_addresses.contains(&buy_collection)
                            || filtered_addresses.contains(&sell_collection)
                        {
                            filtered_hotdogs.push(hotdog);
                        }
                    }
                    _ => {}
                }
            }
            _ => {}
        };
    }

    Ok(Hotdogs {
        hotdogs: filtered_hotdogs,
    })
}

95% of Substreams mapping code can be done just by moving/destructing objects. I'll be happy to offer clone-less variant so code you have problem convertin.

maoueh commented 1 year ago

For the timestamp in seconds, latest substreams-ethereum already have an helper actually:

blk.timestamp_seconds()