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
fcd90fd to
f81c949
Compare
There was a problem hiding this comment.
Automated GPT 5.6-sol re-review of this PR (medium depth). 15 findings, inline below.
Summary of blocking ones to address before merge:
Blocking
registry.rs— aFetchingcache entry whose spawned task dies is never evicted andreceiver.awaithas no timeout, so that asset id is poisoned for the process lifetime and every later request for it hangs a connection.rest.rs— a registry outage now returns200withprecision/name/tickersilently absent. A consumer defaulting a missingprecisionto 0 shows 1.5 units as150000000; the old on-disk path could not produce that.registry.rs— 16 permits withtry_acquire(reject, no queue) plus a 1s TTL and no negative caching: metadata drops out non-deterministically under ordinary concurrency, and a slow registry becomes a permanent 16-in-flight herd with each request paying the full 5s timeout.
Worth fixing before merge
- The raw
RegistryErrorstring — including the registry URL and any embedded credentials — is written into the public response body. --asset-db-pathis deleted with no alias or deprecation, so existing command lines fail at startup.
The rest are a pagination edge case, the no-store vs max-age=10 split between /asset/:id and its supply sibling, a panic on invalid config, a test that can hang the binary, and cleanup items.
f81c949 to
d0f5f69
Compare
d0f5f69 to
501da7a
Compare
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: