Generating piece url based on curio multiaddr and contract mapper #22
Generating piece url based on curio multiaddr and contract mapper #22lukasz-wal merged 12 commits intomainfrom
Conversation
fe05d78 to
9419e25
Compare
url_finder/src/api/find_client.rs
Outdated
| let (_, retrievability_percent) = match timeout( | ||
| Duration::from_secs(RETRIEVABILITY_TIMEOUT_SEC), | ||
| url_tester::get_retrivability_with_head(urls), | ||
| url_tester::get_retrivability_with_get(urls), |
There was a problem hiding this comment.
| url_tester::get_retrivability_with_get(urls), | |
| url_tester::get_retrievability_with_get(urls), |
| let final_addr = if trimmed.ends_with("/https") { | ||
| trimmed.replace("/https", "/tcp/443/https") | ||
| } else if trimmed.ends_with("/http") { | ||
| trimmed.replace("/http", "/tcp/80/http") | ||
| } else { | ||
| trimmed.to_string() |
There was a problem hiding this comment.
We should work on this as a string, we should parse this with a proper multiaddr lib.
@CodeWarriorr did we try any? Were there any issues?
There was a problem hiding this comment.
This is an example of "multiaddr" from curio:
/dns/isabella.hegsx.com/https/http-path/%2Fipni-provider%2F12D3KooWSbAPqdYw1MGrQSV8LgjZDYBvha8bQ2YtHUCzRYyqLCzy
When we try to parse the original address by lib => UnknownProtocolString("http-path")
After removing unknown parts, our internal parser needs a port:
Failed to convert multiaddr: "/dns/isabella.hegsx.com/https" to URL: "Missing port"
url_finder/src/provider_endpoints.rs
Outdated
| } | ||
|
|
||
| pub async fn valid_curio_provider(address: &str) -> Result<Option<String>> { | ||
| let rpc_url = "https://api.node.glif.io/rpc/v1"; |
There was a problem hiding this comment.
Should be an env variable.
url_finder/src/provider_endpoints.rs
Outdated
| let contract_address: &str = "0x14183aD016Ddc83D638425D6328009aa390339Ce"; | ||
|
|
||
| let miner_peer_id_contract = Address::parse_checksummed(contract_address, None) | ||
| .map_err(|e| eyre!("Failed to parse miner id contact: {e}"))?; |
There was a problem hiding this comment.
Use https://docs.rs/alloy-primitives/latest/alloy_primitives/macro.address.html instead (that way it will verify that the address is correct at build time).
| let rpc_provider = ProviderBuilder::new() | ||
| .connect(rpc_url) | ||
| .await | ||
| .map_err(|err| eyre!("Building provider failed: {}", err))?; |
There was a problem hiding this comment.
Is this map_err needed here? Doesnt seem to add anything valueble and err already implements std error?
url_finder/src/provider_endpoints.rs
Outdated
| .map_err(|e| eyre!("Transaction failed: {e}"))?; | ||
|
|
||
| let peer_data: PeerData = | ||
| PeerData::abi_decode(response.as_ref()).map_err(|e| eyre!("Decode failed: {e}"))?; |
url_finder/src/url_tester.rs
Outdated
| const RETRI_TIMEOUT_SEC: u64 = 15; | ||
|
|
||
| /// return first working url through head requests | ||
| /// let't keep both head and get versions for now |
There was a problem hiding this comment.
| /// let't keep both head and get versions for now | |
| /// let's keep both head and get versions for now |
url_finder/src/url_tester.rs
Outdated
| && matches!( | ||
| content_type, | ||
| Some("application/octet-stream") | Some("application/piece") | ||
| ) | ||
| && etag.is_some() |
There was a problem hiding this comment.
Why check for these headers?
There was a problem hiding this comment.
just verifying whether it is a file, I notice that the curio and boost contain etag
curio - application/octet-stream
boost - application/piece
but generally, yes, it is not necessary :-)
| match client.get(&url).send().await { | ||
| Ok(resp) => { | ||
| let content_type = resp | ||
| .headers() | ||
| .get("content-type") | ||
| .and_then(|v| v.to_str().ok()); | ||
| let etag = resp.headers().get("etag"); | ||
|
|
||
| // check if the response has a content-type: | ||
| // * application/octet-stream => curio | ||
| // * application/piece => boost | ||
| // and an etag | ||
| // in header to indicating a file | ||
| if resp.status().is_success() | ||
| && matches!( | ||
| content_type, | ||
| Some("application/octet-stream") | Some("application/piece") | ||
| ) | ||
| && etag.is_some() | ||
| { | ||
| tracing::info!("url WORKING: {:?}", url); | ||
| success_clone.fetch_add(1, Ordering::SeqCst); | ||
| Some(url) | ||
| } else { | ||
| debug!("Retrivability: URL::GET not working url: {:?}", url); | ||
| None | ||
| } | ||
| } | ||
| Err(err) => { | ||
| debug!( | ||
| "Retrivability: Get request for working url failed for {:?}: {:?}", | ||
| url, err | ||
| ); | ||
| None | ||
| } | ||
| } |
There was a problem hiding this comment.
This is big and complicated enough to extract and deduplicate with filter_working_with_get
url_finder/src/url_tester.rs
Outdated
| } | ||
|
|
||
| /// return retrivable percent of the urls | ||
| /// let't keep both head and get versions for now |
There was a problem hiding this comment.
| /// let't keep both head and get versions for now | |
| /// let's keep both head and get versions for now |
| None => &cleaned, | ||
| }; | ||
|
|
||
| let final_addr = if trimmed.ends_with("/https") { |
There was a problem hiding this comment.
is it possible that tcp is already there ?
There was a problem hiding this comment.
no idea, we didn't know anyone had 'tcp' in multiaddr from curio....
url_finder/src/main.rs
Outdated
| .with_state(app_state.clone()); | ||
|
|
||
| let server_addr = SocketAddr::from(([0, 0, 0, 0], 3010)); | ||
| let server_addr = SocketAddr::from(([0, 0, 0, 0], 3030)); |
There was a problem hiding this comment.
this breaks docker-compose
url_finder/Cargo.toml
Outdated
| chrono = { version = "0.4.38", features = ["serde"] } | ||
| regex = "1.11.1" | ||
| moka = { version = "0.12.10", default-features = false, features = ["future"] } | ||
| alloy = "1.0.41" |
There was a problem hiding this comment.
alloy has a lot build into default feature set, this adds up to build time.
giving that we use only small part, we should narrow it down.
something like this should work:
alloy = { version = "1.0.41", default-features = false,
features = [
"sol-types",
"network",
"providers",
"rpc-types-eth"
]
}
There was a problem hiding this comment.
adjusted a little bit and done
68fea97 to
a254055
Compare
a254055 to
a080668
Compare
url_finder/src/multiaddr_parser.rs
Outdated
| } | ||
| } | ||
| Err(e) => { | ||
| println!("Failed to parse multiaddr: {:?} due to {:?}", addr, e); |
There was a problem hiding this comment.
Why do we need second log, and most importantly why it's println! instead of tracing::{error,info,debug} ?
There was a problem hiding this comment.
My mistake, it was left over from local testing, thanks ! fixed
url_finder/src/api/find_client.rs
Outdated
| result: ResultCode::Success, | ||
| working_url: first_url, | ||
| retrievability_percent, | ||
| retrievability_percent: retrievability_percent.unwrap(), |
There was a problem hiding this comment.
why do we need unwrap() here ? seems unnecessary and will panic on None
maybe use unwrap_or() or always return default 0.0:f64
url_finder/src/provider_endpoints.rs
Outdated
| break; | ||
| } | ||
| Err(e) => { | ||
| println!("Attempt {attempt}/3 failed: {e} for address: {address}"); |
af8f01b to
b6889f3
Compare
b6889f3 to
6cf47df
Compare
No description provided.