Updated registry integration to utilize v2 functionality. - #250
Conversation
d6253e8 to
6a734f0
Compare
|
Some review analysis comments:
registry.rs:502 types the contract and from_registry_asset re-serializes it via serde_json::to_value. But the optional fields have #[serde(default)] without skip_serializing_if: #[serde(default)] pub ticker: Option, An absent ticker round-trips to an explicit "ticker": null. Previously AssetMeta.contract was a raw JsonValue passed through untouched (6a734f0^:src/elements/registry.rs:124). A Liquid asset ID commits to the contract bytes. Any client verifying that the asset ID matches the returned contract now computes a different hash for every asset whose contract omits an
.ok_or(RegistryError::MissingLocalAsset(asset_id))? collected into Result<Vec<_>, _>, so a single registry entry absent from the local index fails the whole listing. The old code had the same shape, but the old asset DB was local and synced MissingLocalAsset also maps to 503, which advertises "retry later" for a condition that may never resolve. Skip the entry (or return it registry-only) rather than failing the page. |
6a734f0 to
3781bf9
Compare
|
Updated branch to no longer serialize missing contract fields with null (if not explicitly present and set to null in the registry), and to skip unavailable registry entries instead of returning a 503 |
3781bf9 to
fcd90fd
Compare
| match lookup_asset(self, &asset_id, Some(metadata)) | ||
| .map_err(|error| RegistryError::LocalLookup(error.to_string()))? | ||
| { | ||
| Some(asset) => results.push(asset), | ||
| None => warn!( | ||
| "registered asset {} is not yet available in the local index", | ||
| asset_id | ||
| ), | ||
| } |
There was a problem hiding this comment.
lookup_asset here does synchronous RocksDB work. This runs inline on the tokio worker thread, list_registry_assets is async and reached from handle_asset_registry_request without spawn_blocking, and this loop repeats it up to ASSETS_MAX_PER_PAGE times per request.
Worth wrapping the local-lookup portion in spawn_blocking.
| fn is_valid_registry_domain(value: &str) -> bool { | ||
| if !(3..=255).contains(&value.len()) || !value.is_ascii() { | ||
| return false; | ||
| } | ||
| let value = value.strip_suffix('.').unwrap_or(value); | ||
| let labels: Vec<&str> = value.split('.').collect(); | ||
| if labels.len() < 2 { | ||
| return false; | ||
| } | ||
| labels.iter().all(|label| is_valid_registry_domain_label(label)) | ||
| && labels | ||
| .last() | ||
| .and_then(|label| label.bytes().next()) | ||
| .map(|byte| byte.is_ascii_alphabetic()) | ||
| .unwrap_or(false) | ||
| } | ||
|
|
||
| fn is_valid_registry_domain_label(label: &str) -> bool { | ||
| if label.is_empty() || label.len() > 63 { | ||
| return false; | ||
| } | ||
| let first = label.bytes().next().unwrap(); | ||
| let last = label.bytes().last().unwrap(); | ||
| first.is_ascii_alphanumeric() | ||
| && last.is_ascii_alphanumeric() | ||
| && label | ||
| .bytes() | ||
| .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-') | ||
| } |
There was a problem hiding this comment.
Can we use url::Host::parse?
3 participants
The
GET /assets/registryresponse still includes all the fields before but this PR adds aregistryproperty to it. Theregistry.rsfile was also changed to pull information from the service instead of from the asset cache made from the git registry.The added registry property takes the following shape: