Replace Tableland on-chain device definitions with the R2 catalog - #314
Open
zer0stars wants to merge 16 commits into
Open
Replace Tableland on-chain device definitions with the R2 catalog#314zer0stars wants to merge 16 commits into
zer0stars wants to merge 16 commits into
Conversation
Device definitions now live as JSON documents in R2 behind a CDN, written through the definitions-worker. This service reads docs and the catalog manifest over HTTP and sends writes to the worker with a bearer token. - New DeviceDefinitionCatalogService gateway replaces the on-chain service; same method surface, no chain, no KMS, no tableland-node. - Deleted contracts bindings, sender package, and all Ethereum settings. - DD gRPC RPCs (create/update/filtered-list) return Unimplemented; the worker and identity-api are the new write/read paths. VIN decoder, integrations, device styles, and device types gRPC stay. - decode-vin now creates missing definitions through the worker. - Typesense sync job reads the R2 manifest instead of tableland tables. - vin_repo uses the manufacturer token id from identity instead of a registry contract call.
Catalog ids can contain characters like & + ( ) " (e.g. dodge_town-&-country_2012), which must be percent-encoded in URL paths.
- catalogDoc now overrides UnmarshalJSON: the embedded tableland model's custom unmarshaler was promoted and silently dropped the manufacturer field, so every catalog read returned token id 0. Regression tests cover doc, empty-metadata, and manifest decoding. - Create/Update/Delete always return the definition id on success so callers (decode-vin NewTrxHash deref) can't hit a nil pointer when the worker URL is unset. - delete-dd CLI no longer polls Polygonscan for a transaction that is now a synchronous worker call; dead CheckTransactionStatus helper and POLYGON_SCAN_API_KEY setting/secret removed.
- Success/InternalError counters with CatalogRead / CatalogManifestRead / CatalogWrite method labels replace the retired Tableland* series. - Update() now reads its merge base through the worker (uncached) when DEFINITIONS_WORKER_URL is set, so read-modify-write can't merge a stale CDN-cached document.
- upsert-vin-decoding and device-style lookups now return NotFound instead of nil-dereferencing when a definition is missing from the catalog (the catalog returns nil, nil on 404). - Create re-gains the old already-exists guard, using a CDN-bypassing read, so a create can never silently overwrite a curated definition. - bulk-update-powertrain: inverted nil-check (pre-existing) wiped all device attributes on every processed row and panicked on absent metadata; now initializes only when metadata is actually nil.
The Typesense sync pass only ever upserted, and the prod CronJob runs it
without -create-index, so the index was never rebuilt from scratch. A
definition deleted from R2 therefore stayed searchable forever: the worker's
delete-on-write was the only path that removed it, and if that Typesense call
failed the orphan became permanent, with search returning a definition_id that
404s.
runSearchSync now collects every id it upserts and deletes anything else the
index still holds. Two guards, because this deletes from a live index:
- an empty catalog set skips the prune entirely, so a failed or empty
catalog read can never empty the index;
- a prune larger than 10% of the index (floor 100) is refused and reported,
since that is far more likely to mean a partial catalog read than a real
deletion of that size. -allow-bulk-prune overrides it.
Orphans are removed one id at a time rather than through a filter_by set:
definition ids legitimately contain & + ( ) and ", which would need escaping
inside a filter expression.
The prune pass built its keep-set from the documents it upserts, which are filtered to year >= minSearchYear. Anything older therefore looked like an orphan, so a definition that exists in the catalog but sits below the index cutoff would be deleted from the search index. Measured against prod: definitions_prod carries 1,216 documents that are not in the year >= 2007 catalog set, and 1,173 of them are real definitions from 2005 and 2006 -- acura_mdx_2005 and the like -- indexed before the cutoff existed and searchable today. Only 43 are genuinely absent from the catalog. The bulk guard would not have caught it either: the limit at that index size is 1,791, and 1,216 slides underneath. buildManufacturerDocuments now returns every id it saw alongside the documents it built, and the keep-set uses those. A definition below the cutoff is not an orphan -- it exists, it is simply not indexed going forward. Whether to drop pre-cutoff documents is a product decision, not something a cleanup pass should do silently on its first run.
…mpty sync Three ways a failure was being reported as success. An empty DEFINITIONS_WORKER_URL made every write a no-op that still answered success. workerRequest returned (false, nil) and all three callers discarded the bool, so Create returned an id, Delete logged "Deleted device definition", and a VIN decode answered 200 with a definitionId that had never been written to R2 -- traceable only through an Info log. A missing write endpoint is a misconfiguration, not a mode of operation, so it is now an error. catalogPageSize was declared twice, in package main and in package gateways. The sync loop terminated on len(page) < the main copy while pages actually came from the gateway copy. Lowering the gateway constant would have ended paging early for the 11 manufacturers holding more than 500 definitions -- BMW alone has 1618 -- leaving the rest out of the prune keep-set and deleting their search documents. Against a 16,697-document index the bulk guard sits at 1,669, so it would not have tripped. The gateway constant is now exported and used by both. identity-api answers HTTP 200 with a populated `errors` array and null data when it fails, which the client surfaces as (nil, nil). The daily cron then printed "Index Updated" having indexed nothing. Zero manufacturers is now a hard error.
…eSize panic bulk-update-powertrain read each definition through the CDN, mutated its metadata and wrote the result back. Documents are served with max-age=86400, so an edit made through the worker earlier that day was read as its pre-edit version and silently overwritten -- Update fetches a fresh base and then has it replaced by the caller's stale copy. Added GetDefinitionByIDFresh, which reads through the worker, and the command now uses it. Read-modify-write callers cannot use the cached path. decode-vin put the definition slug into NewTrxHash, surfaced as new_trx_hash / NewTransactionHash. Definitions are no longer written on-chain, so there is no transaction, and any consumer treating that field as a hash -- checking a 0x prefix, polling an explorer, storing it in a tx_hash column -- gets a slug instead. It stays empty now; the id was already returned in DefinitionId. manifest() counted only its decode failure, so a catalog outage made CatalogManifestRead drop to zero rather than spike, which no "error rate > X" alert would fire on. Every exit now counts itself. Finally, fiber's Query default applies only when a parameter is absent, so an explicit pageSize=0 or pageSize=abc reached the search handler as 0 and divided by zero computing totalPages. Pre-existing on main, but this PR is what makes admin call the endpoint. Both page and pageSize are clamped.
…eal worker host cc3bfd2 clamped the definitions search handler and missed the byte-identical one sixty lines above it, which feeds the same (found + pageSize - 1) / pageSize arithmetic. GET /device-definitions/search-r1?pageSize=0 still divided by zero; fiber's recover middleware turned it into a 500 rather than a crash, but it was the same defect the commit claimed to fix. DEFINITIONS_WORKER_URL also pointed at definitions-worker.dimo.workers.dev in both values files. That host has no A record -- wrangler disables workers.dev once custom domains are configured -- so every write would have failed on DNS.
The prune's keep-set decides what gets deleted and it was built from whatever `manifest()` happened to return. Two ways that was wrong. It came from the CDN, which serves manifest.json with max-age=300, so any definition created in the five minutes before a sync was absent from the keep-set and its search document deleted. And the per-page cache expires after a minute while a full run takes several, so pages could come from different snapshots -- a definition that shifted position between pages was returned by neither and deleted too. PinCatalogSnapshot fetches once, bypassing the CDN, and holds it for the run. The keep-set was also assembled by walking identity-api's manufacturer list, which made it only as complete as that list. One make missing meant every one of its definitions looked like an orphan: BMW alone is 1,618 documents, and the bulk guard at that index size is 1,669, so it would have deleted them all without tripping. The keep-set now comes from the manifest, which is the catalog. The walk still drives indexing, where an incomplete list only means less gets refreshed. Also: add_vin dereferenced a definition that is nil both for a missing id and for one owned by another manufacturer; an identity blip could cache an empty manufacturer map for ten minutes, turning a brief outage into ten minutes of silently skipped rows; and the prune now logs every id it removes, because documents below the index year cutoff are never re-upserted and would otherwise vanish without a record.
…l deletes PinCatalogSnapshot's justification was wrong. It claimed to bypass a CDN serving the manifest with max-age=300, but the worker generates every response for definitions.dimo.org -- no cf-cache-status, no age, and the date header advances per request across colos -- so there is no edge cache in front of it, and after the URL fix the worker and catalog hosts are the same anyway. The pin still earns its place for the other reason: the per-request cache expires after a minute while a full sync takes several, so without it pages come from different manifests and a definition that shifts between them is returned by neither and deleted as an orphan. The comments now say that instead. The prune guard was calibrated above the failure it exists to catch. Ten percent of the live index is ~1,790 deletions, and one large manufacturer missing from the keep-set is 1,618 documents for BMW alone, so the motivating disaster slid underneath it. Two percent, against routine churn of tens. An empty keep-set is now a hard error rather than a silent no-op sync, matching the treatment identity's manufacturer list already got -- and this is the input that drives deletions. The manifest's own count is checked against the definitions it carries. DeleteDocuments now reports how many it removed and logs each one as it succeeds. It deletes one at a time and stops on the first error, so logging the whole batch up front claimed ids that still existed -- in the audit trail for deletions that cannot be undone.
The guard was a percentage of the index with a floor underneath it, which meant two constants, a helper to combine them, and a regime where each silently took over from the other. Below ~5,000 documents the floor decided; above it the fraction did. Both existing bulk-prune tests sat in the floor regime, so the suite stayed green for any fraction between 0.02 and 0.99 -- which is how the last change to it shipped in a commit message and not in the code. The ratio was never doing the work. What matters is one number above routine churn and below the smallest failure worth catching: prod carries 41 orphans, and a keep-set that lost a whole manufacturer is ~1,615 documents. 500 sits between them and says so directly. The test now pins that relationship rather than an arithmetic result.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Removes the entire on-chain/Tableland device-definition path. Definitions now live as JSON documents in R2 behind the CDN, written through definitions-worker. Net −14k lines and every chain dependency (contracts bindings, KMS sender, tableland-node proxy, Ethereum settings).
DeviceDefinitionCatalogServicegateway: reads docs/manifest over HTTP (Prometheus counters onCatalogRead/CatalogManifestRead/CatalogWrite), writes via the worker with a bearer token.Update()reads its merge base through the worker (uncached) so it can never merge a stale CDN copy.Unimplemented— dimo-admin moves to the worker/REST in DIMO-Network/dimo-admin#315. Deploy together with that PR, admin first — new admin works against old dd-api, but old admin breaks against this PR. VIN decoder, integrations, device styles, and device types gRPC are untouched (attestation-api / vehicle-signal-decoding unaffected).DEFINITIONS_CATALOG_URL/DEFINITIONS_WORKER_URLenvs +DEFINITIONS_WORKER_TOKENsecret replace the Ethereum/Tableland config.Why
Adding a manufacturer required an on-chain table + privilege grants + a service restart (see the Ineos incident, manufacturer 147). The catalog is ~18k near-static reference docs; object storage + search index is the right tool. Public API contracts hold.
Testing
CATALOG_E2E_URL=... go test -run TestCatalogServiceE2E) passed against a full local backfill of all 17,996 live Tableland rows.