feat: add public labels v3 adapter - #87
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: euler-xyz/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Add a focused package entrypoint for the public labels adapter, normalizer, types, and branding helpers so consumers can avoid loading the full SDK barrel.
LeonardEulerXYZ
left a comment
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Draft review on exact head 18145a8738d852a93fe09cbbd57a108d4d65bd69. I found five changes needed across verification-state semantics, scoped entity attribution, the focused declaration surface, untrusted product-key handling, and post-failure concurrency. Local validation otherwise passed: focused 8/8, full SDK 519/519, typecheck, build, and lint (existing warnings only); packed runtime imports and the live immutable KPK/Securitize fixture also loaded successfully. A strict packed subpath declaration probe failed as described inline. The review event is COMMENT because the PR is still a draft.
| vault.isDeprecated || | ||
| vault.deprecationReason || | ||
| vault.tags.length || | ||
| vault.campaigns?.length, |
There was a problem hiding this comment.
Potential issue · blocking: campaigns are rewards metadata, but including vault.campaigns?.length in this verification-content predicate promotes points-only inventory rows into trust state. The live pinned snapshot has EVKs 0x35d4f830543700B7280084280ae3236f178E88e3 and 0xbC35161043EE2D74816d421EfD6a45fDa73B050A with campaigns and no product/entity/name/deprecation/tag content. Both exist in euler-labels/1/points.json and not 1/products.json; this normalizer nevertheless creates __vault_* products and adds them to verifiedVaultAddresses. Keep campaign extraction over all chain inventory rows, but exclude campaigns from the predicate that admits rows into products/verification/Earn membership. Add a campaign-only EVK regression proving points remain present while no product or verified address is created.
| `Public Labels entity profile mismatch for ${entityId}`, | ||
| ); | ||
| } | ||
| return { profile, addresses }; |
There was a problem hiding this comment.
Potential issue · blocking trust boundary: validate that every address row returned from /entities/${entityId}/addresses has row.entityId === entityId before flattening it. Profiles get this scoped-identity check above, but address rows do not; normalization later groups them by the row's claimed entityId. I reproduced /entities/brand/addresses returning { entityId: "owner", address: attacker }, after which entities.owner.addresses contains the attacker address. The linked Lite consumer builds hasEntityAddress directly from these sets for governor/owner verification. Reject cross-keyed rows here and add a co-brand-to-owner regression. Current production rows are consistent; the missing guard is what permits a malformed or cache-misrouted response to cross the identity boundary.
| export { | ||
| getEulerLabelProductBrandEntities, | ||
| getEulerLabelProductBrandEntityKeys, | ||
| } from "./utils/eulerLabels.js"; |
There was a problem hiding this comment.
Potential issue · package compatibility: this makes the focused subpath's declaration graph broad again. utils/eulerLabels.d.ts imports EulerEarn, which pulls in unrelated vault/service declarations; in a clean packed consumer importing only @eulerxyz/euler-v2-sdk/public-labels with skipLibCheck: false, tsc reaches eVaultLensTypes.d.ts and fails on its package-internal src/utils/oracle.js import (plus unrelated dependency declarations). Runtime resolution and skipLibCheck: true pass, but the advertised focused type surface is not isolated. Move these two label-only helpers into a narrow module that imports only EulerLabelProduct/EulerLabelEntity, re-export it from both entrypoints, and add a strict packed-consumer declaration smoke test.
|
|
||
| const products: Record<string, EulerLabelProduct> = {}; | ||
| for (const product of productRows) { | ||
| products[product.id] = buildProduct( |
There was a problem hiding this comment.
Potential issue · untrusted key handling: product.id comes from the remote dataset and is assigned into a normal {} record. With a product ID of __proto__, this assignment mutates the record's prototype instead of creating an enumerable product; the product disappears from Object.keys/Object.values, while its vault is still added to verifiedVaultAddresses. Use a null-prototype record/Map (and preferably validate IDs before object-key use), then add an exact __proto__ regression so malformed publication data cannot create split-brain verification state.
| let nextIndex = 0; | ||
|
|
||
| const worker = async () => { | ||
| while (nextIndex < values.length) { |
There was a problem hiding this comment.
Potential issue · failure liveness: once one mapper rejects, Promise.all rejects to the caller but the other workers keep claiming fresh indices. In a 20-entity fixture with the first profile request failing, there were 16 detail requests at caller rejection and all 40 profile/address requests were eventually issued. That spends API quota/bandwidth—and can keep sending an API key—after the operation is already known to have failed. Record a shared failure/stop flag so workers do not claim new indices after the first error (in-flight calls may finish), and add a deferred-request regression that checks the count does not continue to the full batch.
WalkthroughOverviewThis PR adds an opt-in Public Labels V3 transport and normalizer, keeps the file-backed labels service as the default, and exposes a focused Review scope
Reviewed head: VerdictChanges requested on this draft:
Inline comments contain the exact code paths and minimal discriminating tests. Validation
Cross-repo noteThe linked Lite head consumes the new subpath correctly. Its stock GitHub build/typecheck currently fail because the committed published SDK does not contain the unpublished subpath; the Railway preview build with branch injection is green. That is an explicit publication/sequencing gate, not a separate defect in this SDK diff. What looks good
|
LeonardEulerXYZ
left a comment
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Follow-up challenge pass on unchanged head 18145a8738d852a93fe09cbbd57a108d4d65bd69. Four additional actionable findings survived independent reproduction: unbounded follow-up request amplification, inconsistent pagination totals returning incorrect data, a source-incompatible public social type, and the default HTTP transport being absent from tests. See inline comments for exact probes and minimal fixes.
| ...new Set([ | ||
| ...products.flatMap((product) => [ | ||
| product.entityId, | ||
| ...(product.coBrandEntityIds ?? []), |
There was a problem hiding this comment.
Potential issue · blocking resource bound: mapper concurrency limits simultaneous work, but this expansion does not bound how much work is created. Every owner/co-brand/vault entity ID triggers two detail requests; coBrandEntityIds is unbounded, and referenced IDs are not required to exist in the fetched /entities inventory. I reproduced one product with 250 referenced IDs and an empty entity inventory: the adapter accepted it, issued 500 detail requests, and returned 250 hydrated entities. With up to 10,000 top-level products, malformed arrays can amplify far beyond the list cap. Before hydration, require referential integrity against the fetched entity inventory and cap both per-product co-brand IDs and total unique hydration IDs. Add a fixture proving an oversized/unlisted reference set rejects without issuing detail calls.
| limit: PUBLIC_LABELS_PAGE_SIZE, | ||
| offset, | ||
| }); | ||
| const { items, total } = assertListResponse(response, path); |
There was a problem hiding this comment.
Potential issue · immutable pagination integrity: each page's fresh meta.total controls termination and slice(). I reproduced page one returning 100 items with total=101, then page two returning one item with total=1; this function silently returned only [0] instead of rejecting the inconsistent snapshot. Capture the first page's total and require every later page to match it; also validate returned offset/limit against the request. Add changing-total and short non-final-page fixtures so immutable pagination cannot silently truncate or skip records.
| discord: string; | ||
| telegram: string; | ||
| github: string; | ||
| [key: string]: string | undefined; |
There was a problem hiding this comment.
Potential issue · public type compatibility: this open index signature changes keyof EulerLabelEntity["social"] from the finite required-key union to string | number, and indexed access becomes string | undefined. A downstream generic fixture assigning social[keyof typeof social] to string compiles against the base type and fails on this head with TS2322. If the new V3 field is the goal, add defillama?: string explicitly instead of widening every social key, and retain a compile-time compatibility fixture.
| }, | ||
| }); | ||
|
|
||
| const fixtureRequest = (options?: { productEntityId?: string }) => { |
There was a problem hiding this comment.
Test adequacy: every adapter test injects this request double, so none exercise buildPublicLabelsRequest. A concrete mutant changing X-API-Key to Authorization still passed all eight focused tests; wrong endpoint joining, query encoding, JSON handling, and non-2xx behavior would also survive. Add a stubbed-global-fetch test around the default adapter that asserts the final /v3 URL, version/chain/pagination query, X-API-Key, and HTTP error propagation.
2 participants
Summary
public-labelspackage entrypoint for the immutable Public Labels V3 dataset.Changes
latestonce and pin the complete snapshot to its concrete publication version.meta.totalpagination with 100-record pages for vaults, products, entities, entity addresses, and geo policies.@eulerxyz/euler-v2-sdk/public-labelsso server consumers avoid loading the full SDK barrel.Test plan
pnpm --filter @eulerxyz/euler-v2-sdk lintpnpm --filter @eulerxyz/euler-v2-sdk typecheckpnpm --filter @eulerxyz/euler-v2-sdk test(35 files, 519 tests)pnpm --filter @eulerxyz/euler-v2-sdk buildpublic-labelsruntime exportsv20260804151305236, paginated totals, KPK ownership, Securitize co-branding, and both hosted logos