feat: fetch historical liquidation valuations from the v3 liquidations endpoint - #80
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds liquidation querying to the activity service, including public types, response normalization, v3 API integration, availability handling, SDK compatibility, enriched asset parsing, and type and runtime validation tests. ChangesLiquidations activity flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant EulerSDK
participant ActivityService
participant ActivityV3Adapter
participant LiquidationsAPI
Client->>EulerSDK: activityService.fetchLiquidations(args)
EulerSDK->>ActivityService: fetchLiquidations(args)
ActivityService->>ActivityV3Adapter: queryLiquidations(args)
ActivityV3Adapter->>LiquidationsAPI: GET /v3/liquidations
LiquidationsAPI-->>ActivityV3Adapter: response body
ActivityV3Adapter->>ActivityV3Adapter: normalize and validate page
ActivityV3Adapter-->>ActivityService: LiquidationsPage
ActivityService-->>EulerSDK: LiquidationsPage
EulerSDK-->>Client: LiquidationsPage
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/euler-v2-sdk/src/services/activityService/activityEvent.ts`:
- Around line 218-231: Update readOptionalUsdValue so its string branch
validates parsed content as a finite, non-negative numeric USD value, rejecting
strings such as "-1" and "abc" while preserving valid numeric strings and the
existing null/undefined behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: euler-xyz/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9eb226ad-3021-4499-b7fd-97a817704e36
📒 Files selected for processing (6)
packages/euler-v2-sdk/src/services/activityService/activityEvent.tspackages/euler-v2-sdk/src/services/activityService/activityService.tspackages/euler-v2-sdk/src/services/activityService/activityServiceTypes.tspackages/euler-v2-sdk/src/services/activityService/adapters/activityV3Adapter.tspackages/euler-v2-sdk/src/services/activityService/index.tspackages/euler-v2-sdk/test/activityService.test.ts
LeonardEulerXYZ
left a comment
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Review summary
Verdict: changes requested on 927310147463c8d19dc3a6e2348e931f1e7db95a.
The service wiring is orderly, the shared timeout/body-size/API-key path is preserved, and the new query-key normalization follows the existing service pattern. Four correctness/API-contract issues remain:
- A valid live historical page can be rejected because nullable token metadata from
/v3/liquidationsis modeled and parsed as required. - The new public method remains optional through
IActivityService, so strict TypeScript consumers cannot call the advertised SDK feature directly. - Widening the existing exported
ActivityAssetAmount.amountUsdfromstringtostring | numberbreaks previously valid downstream assignments. - Unlike the sibling account/vault activity routes, liquidation responses are not validated against the request scope; a wrong-chain/wrong-vault page is accepted successfully.
Evidence and validation
- Inspected all 6 paginated changed files and every changed hunk.
pnpm --filter @eulerxyz/euler-v2-sdk test -- --run test/activityService.test.ts— passed: 32 files / 480 tests.pnpm --filter @eulerxyz/euler-v2-sdk typecheck— passed.pnpm --filter @eulerxyz/euler-v2-sdk build— passed.pnpm --filter @eulerxyz/euler-v2-sdk lint— exit 0; existing warnings/infos are outside this diff.git diff --check origin/main...HEAD— passed.- Strict downstream compile fixtures reproduced both public-type failures:
fetchLiquidationsis possibly undefined, and a previously validstring | undefinedassignment foramountUsdfails only on this head. - Checked the live OpenAPI contract and a concrete production fixture. Mainnet
/v3/liquidations?chainId=1&limit=100&offset=800contained 3 rows withcollateralAsset: null; a focused local test confirmednormalizeLiquidationsResponserejects that valid page. - A focused adversarial fixture confirmed a chain-1/vault-A request accepts a structurally valid chain-8453/vault-B response and unrelated pagination metadata.
Coverage and maintainability
activityEvent.ts— finding: live nullable metadata rejection; CodeRabbit's numeric-string validation concern is also valid.activityService.ts— reviewed clean: delegation and unsupported-adapter error path behave as intended.activityServiceTypes.ts— findings: public optionality, nullable response mismatch, and source-incompatibleamountUsdwidening.activityV3Adapter.ts— finding: missing request-aware response validation.index.ts— reviewed clean: symbols reach the package root through the existing export chain.activityService.test.ts— finding: mocks do not cover the live nullable schema, publicEulerSDK.activityServicetype, or wrong-scope responses.
Scalability / maintainability hygiene: the sibling account and vault paths already centralize request-aware validation in activityEvent.ts. The narrow reusable shape here is a corresponding validateLiquidationsPage(page, args) rather than one-off checks in the adapter. Org code search found no existing downstream fetchLiquidations caller to migrate, but the package's exported type contract itself is affected.
Security/adversarial pass found no dependency, workflow, secret-handling, permission, transaction-construction, or new telemetry changes. The existing response-size cap, timeout, redirect policy, and API-key header path are retained.
No screenshots: this is an SDK/API-only change with no user-visible UI surface.
… token metadata - Model nullable historical token metadata (debtAsset, collateralAsset and their decimals) as omitted optionals so valid live pages parse - Require fetchLiquidations on IActivityService while keeping it optional on IActivityAdapter for existing custom adapters - Keep ActivityAssetAmount.amountUsd a string by normalizing numeric wire values to decimal strings, and validate numeric shape on string values - Add validateLiquidationsPage: chain/filter echo, pagination-window echo, clamping direction, row-count/total invariants, and timestamp bounds
|
Addressed all review findings in 5d351d4:
|
LeonardEulerXYZ
left a comment
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Review: changes requested
Reviewed exact head 5d351d48547eb6f2e6e95506149cc2b5bfaf2c36 against main. The earlier nullable-metadata issue is fixed, and the new request-aware validator correctly checks echoed request fields. Three current-head correctness/API-compatibility blockers remain; see inline comments.
Re-review reconciliation
- Historical nullable token metadata: fixed.
- USD public output type: restored to strings, but exponent-form numeric conversion is still lossy/corrupting.
- Request-aware validation: added, but its row-count invariant rejects valid empty pages returned by the live API.
- Service method availability: the concrete service now exposes the method, but making it required on exported
IActivityServicebreaks existing custom service overrides.
Validation
pnpm --filter @eulerxyz/euler-v2-sdk test— 32 files / 482 tests passed.pnpm --filter @eulerxyz/euler-v2-sdk typecheck— passed.pnpm --filter @eulerxyz/euler-v2-sdk build— passed.pnpm --filter @eulerxyz/euler-v2-sdk lint— passed with pre-existing warnings outside the PR diff.- Live
/v3/liquidationsfixture checked, including an out-of-range offset that validly returneddata: [],total: 2374,offset: 999999. - Built-package focused fixtures reproduced
1e-7precision corruption and1e-101 -> "0". - Identical strict TypeScript custom-service fixture: base compiled; PR head failed with
TS2741becausefetchLiquidationsbecame required.
Coverage and review frames
All six changed files were inspected: adapter routing/request construction, service delegation/unavailable behavior, exported interfaces/types, liquidation normalization/validation, public exports, and focused tests. Intent/invariants, changed-code correctness, integration/public API impact, adversarial/security, test discrimination, and an independent fresh-challenge pass are complete. No dependency, secret, auth, transaction-construction, or unexpected network surface was introduced.
Scalability / maintainability hygiene: sibling account/vault flows and SDK override consumers were traced. The reusable validation/normalization placement is sound, but liquidations support is currently encoded by method presence rather than the existing capability model. While preserving source compatibility, please keep the built-in service guarantee separate from the override-facing contract (or introduce an explicit extended capability/interface) and add downstream compile fixtures plus exact numeric/pagination regressions.
CodeRabbit’s earlier required-field suggestion is addressed in the latest range; its current clean summary does not exercise the three focused cases above.
… service contract - Expand exponent-form USD numbers textually from the serialized mantissa instead of toFixed, which re-rounded the value and floored at 100 decimals (1e-101 previously collapsed to "0") - Allow empty pages for offsets beyond the reported total while still rejecting positive rows past the remaining count - Restore IActivityService to its previous shape so existing custom service overrides keep compiling, and expose the built-in guarantee as IActivityServiceWithLiquidations (implemented by ActivityService) with downstream compile fixtures for both boundaries
|
Round-2 findings addressed in f62b7e6:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/euler-v2-sdk/src/services/activityService/activityEvent.ts`:
- Around line 224-225: Update usdNumberToDecimalString to accept and normalize
the raw USD lexeme before any numeric conversion, using a string or lossless
decimal representation instead of String(value) on a JavaScript number. Preserve
full integer and fractional precision, including values above 2^53 and long
fractional amounts, and add regression tests covering both cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: euler-xyz/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 02bde11a-64ae-427a-aa96-fc3002b9e438
📒 Files selected for processing (5)
packages/euler-v2-sdk/src/services/activityService/activityEvent.tspackages/euler-v2-sdk/src/services/activityService/activityService.tspackages/euler-v2-sdk/src/services/activityService/activityServiceTypes.tspackages/euler-v2-sdk/src/services/activityService/index.tspackages/euler-v2-sdk/test/activityService.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/euler-v2-sdk/src/services/activityService/index.ts
- packages/euler-v2-sdk/src/services/activityService/activityService.ts
- packages/euler-v2-sdk/test/activityService.test.ts
LeonardEulerXYZ
left a comment
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Review: changes requested
Reviewed exact head f62b7e6c4e355a78faa595854d9cc14add9c3197 against main. The historical liquidation parser, request-scoped response checks, pagination behavior, and exponent normalization are now sound against the production /v3/liquidations contract. One public SDK type boundary remains blocking: the built-in service guarantees fetchLiquidations, but buildEulerSDK() returns EulerSDK, whose activityService is still typed as the override-compatible IActivityService; strict consumers therefore see the new method as possibly undefined. See the inline finding.
Re-review reconciliation
- Nullable historical token metadata: fixed.
- Exact exponent normalization: fixed, including small/large exponent regressions.
- Valid empty overshoot pages: fixed.
- Legacy custom-service override compatibility: fixed by splitting the override-facing and built-in service contracts.
- Built SDK callable surface: still unresolved; the stronger built-in contract is erased at
EulerSDK.activityService.
Validation
pnpm --filter @eulerxyz/euler-v2-sdk test— 32 files / 484 tests passed.pnpm --filter @eulerxyz/euler-v2-sdk typecheck— passed.pnpm --filter @eulerxyz/euler-v2-sdk build— passed.pnpm --filter @eulerxyz/euler-v2-sdk lint— exit 0; only pre-existing warnings/infos outside this diff.git diff --check origin/main...HEAD— passed.- Strict consumer fixture:
sdk.activityService.fetchLiquidations({ chainId: 1 })fails withTS2722: Cannot invoke an object which is possibly 'undefined'. - Production Data API/OpenAPI pass: checked
/v3/liquidationsresponse shape, nullable metadata, filters, and overshoot pagination behavior.
Coverage map
All 6 paginated changed files were dispositioned.
activityEvent.ts— reviewed clean: number/string normalization, nullable historical metadata, response-field validation, request scope, and pagination invariants inspected and regression-tested.activityService.ts— reviewed clean: built-in delegation and unavailable-adapter failure path inspected.activityServiceTypes.ts— finding: override compatibility is preserved, but the stronger built-in contract does not reach the public SDK property.activityV3Adapter.ts— reviewed clean: query validation/encoding, API-key handling, 10s abort, redirect rejection, streamed 2 MiB cap, response parsing, and request-aware validation inspected.index.ts— reviewed clean: new types and built-in interface follow the existing package export chain.activityService.test.ts— reviewed clean for runtime parsing, malformed responses, filters, pagination, timeout/body cap, unavailable adapter, and exponent cases; unverified gap is the missing downstream compile fixture for the publicbuildEulerSDK()surface, reflected in the blocker.
Required review frames
Intent/invariants, changed-code correctness, impact/integration, adversarial/security, test discrimination, and a fresh challenge pass are complete. Negative hypotheses covered wrong-scope responses, nullable live metadata, lossy exponent conversion, empty-page overshoot, response-size/timeout/redirect abuse, malformed data, credential leakage, override source compatibility, and public method availability. No dependency, workflow, transaction-construction, permission, telemetry, or secret-exposure changes were introduced.
Scalability / maintainability hygiene: sibling account/vault activity flows, the shared parser/validator boundary, package exports, EulerSDKOptions, BuildSDKOverrides, SDK construction, and the public EulerSDK property were traced. The parser/validator placement is reusable and consistent. The remaining fix should model the distinction between arbitrary legacy overrides and the guaranteed built-in service once at the SDK type boundary (for example via a typed facade, generic/overloaded return, or wrapping legacy overrides), rather than requiring consumers to narrow at every call site; add a strict downstream compile fixture.
CodeRabbit's current raw-JSON-lexeme precision comment was checked but not promoted: the production/OpenAPI contract supplies these fields as JSON numbers, so exact pre-parse lexeme preservation is not part of the present SDK wire/type contract. The PR no longer adds precision loss beyond normal JavaScript number parsing.
No screenshots: SDK/API-only change; no user-visible UI surface.
EulerSDK.activityService is now IActivityServiceWithLiquidations: overrides that already expose fetchLiquidations pass through by identity, and legacy overrides without it are wrapped in an ActivityService that delegates every call and reports activity-unavailable at runtime. Both boundaries are enforced by type fixtures (vitest typecheck mode, scoped to test-d files): a strict consumer calling fetchLiquidations on the built SDK without narrowing, and a pre-liquidations override staying assignable to the SDK options.
|
Final blocker addressed in d7979a8: Since |
LeonardEulerXYZ
left a comment
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Review: changes requested
Reviewed exact head d7979a8759d412fe766d84d12ab9c18b2c36ecc1 against main using the anti-anchoring-v1 method.
The new v3 liquidation route is well integrated: request validation, bounded HTTP handling, page/request reconciliation, query-key normalization, public exports, and strict-consumer compile coverage are all in place. The previous public SDK typing blocker is resolved: ordinary consumers can call sdk.activityService.fetchLiquidations(...) directly, while legacy overrides remain assignable.
One response-contract blocker remains. The liquidation parser accepts valuation metadata that contradicts the v3 contract—for example status: "available" with neither USD leg present, and a missing or arbitrary source. That can make downstream consumers trust the status while receiving no historical valuation. See the inline finding.
This gap was already present on the initial PR head and was missed in my earlier review passes; it was not introduced by the latest fix.
Re-review reconciliation
- Nullable historical token metadata: fixed.
- Request-scoped response and pagination checks: fixed.
- Exact exponent expansion for the existing activity USD normalization path: fixed.
- Built SDK liquidations guarantee at the public
EulerSDKboundary: fixed, with downstream compile fixtures. - Remaining blocker: liquidation valuation status/source coherence.
Validation
corepack pnpm install --frozen-lockfile— passedcorepack pnpm --filter @eulerxyz/euler-v2-sdk test— passedcorepack pnpm --filter @eulerxyz/euler-v2-sdk typecheck— passedcorepack pnpm --filter @eulerxyz/euler-v2-sdk build— passedcorepack pnpm --filter @eulerxyz/euler-v2-sdk lint— exit 0; 5 warnings and 4 infos, outside this feature pathgit diff --check origin/main...HEAD— passed- Focused built-package reproduction — confirmed contradictory
valuation.status: "available"is accepted with bothrepayAssetsUsdandcollateralAssetsUsdabsent - Production contract check —
/v3/openapi.jsondefinesavailableas both USD legs present,partialas one,unavailableas neither, and requires sourcehistorical-price-snapshots - Live bounded samples — mainnet 100/100 available rows had both legs; Base sample contained available/both, partial/exactly-one, and unavailable/neither, with no contradictions
Scalability / maintainability hygiene
I traced the new route through the adapter, service facade, SDK construction, public export chain, query-key path, default configuration, and sibling account/vault activity validation. The route reuses the existing bounded transport and service abstractions rather than duplicating them. The legacy-service wrapper delegates every declared IActivityService method; its identity/custom-extension behavior changes for old overrides, but that surface is outside the declared interface and is not treated as a blocker. A focused test documenting that legacy wrapping behavior would make the compatibility boundary clearer.
Changed-file coverage
packages/euler-v2-sdk/src/sdk/sdk.ts— reviewed-clean: stronger public service type, constructor wiring, legacy/default paths, and emitted declaration behavior checked.packages/euler-v2-sdk/src/services/activityService/activityEvent.ts— finding: liquidation valuation discriminant/source coherence is not enforced; other numeric, nullable metadata, request-scope, timestamp, address, and pagination paths inspected and validated.packages/euler-v2-sdk/src/services/activityService/activityService.ts— reviewed-clean: facade delegation, unavailable fallback, liquidations query key, and legacy wrapping reproduced from built output.packages/euler-v2-sdk/src/services/activityService/activityServiceTypes.ts— reviewed-clean: request/record/meta/capability contracts and optional legacy boundary inspected against OpenAPI and consumers.packages/euler-v2-sdk/src/services/activityService/adapters/activityV3Adapter.ts— reviewed-clean: URL encoding, headers, timeout, redirect rejection, response-size cap, parsing, and request-aware validation inspected.packages/euler-v2-sdk/src/services/activityService/index.ts— reviewed-clean: value/type export chain checked through package build.packages/euler-v2-sdk/test/activityService.test.ts— finding: broad route coverage passes, but no discriminating table rejects available/partial/unavailable contradictions or invalid/missing liquidation valuation source.packages/euler-v2-sdk/test/activityServicePublicTypes.test-d.ts— reviewed-clean: direct built-SDK callability and legacy override assignability covered and typechecked.packages/euler-v2-sdk/tsconfig.typetest.json— reviewed-clean: scoped no-emit public type fixture configuration checked.packages/euler-v2-sdk/vitest.config.ts— reviewed-clean: runtime and type-test discovery remain separated and both execute under the package test command.
Bot feedback checked
The earlier CodeRabbit numeric-string validation comment is fixed. Its remaining raw-JSON-number lexeme comment is not promoted here: the v3 contract supplies JSON numbers, and precision beyond JavaScript number is already lost before this normalizer unless the wire contract or JSON parser changes. No other active bot finding changed this verdict.
Screenshots: not applicable; this PR changes SDK/API behavior only.
The v3 contract couples valuation status to the two USD legs (available = both repayAssetsUsd and collateralAssetsUsd, partial = exactly one, unavailable = neither) and requires the historical-price-snapshots source. The generic valuation reader enforced neither, so a contradictory row could report available while carrying no historical valuation. Covered by a table-driven rejection matrix over every contradictory combination plus the live partial shape as acceptance.
|
Valuation discriminant enforced in 3feddd5: |
LeonardEulerXYZ
left a comment
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Review: changes requested
Reviewed exact head 3feddd536d71bdee009e93c2d01ab424d66fe331 against main using the anti-anchoring-v1 method.
The latest commit correctly fixes the prior valuation source/status blocker: available, partial, and unavailable now agree with the two historical USD legs, and the source is pinned to historical-price-snapshots. Two response-contract gaps remain in the same new parser; see the inline findings.
bonusUsdis still accepted when the valuation is partial or unavailable, although the producer contract defines it as null unless both USD legs are present. The current test explicitly blesses this contradictory shape. Consumers can therefore receive a liquidation bonus/P&L without the valuations from which it is derived.- Historical token decimals accept values above 255, despite the producer discarding those values and the sibling activity-asset parser enforcing the uint8 bound.
Both gaps were present on the initial PR head and were missed in my earlier review passes; neither was introduced by the latest fix.
Re-review reconciliation
- Nullable historical token metadata: fixed.
- Request-scoped response and pagination checks: fixed.
- Exact exponent expansion: fixed.
- Built SDK callability plus legacy override compatibility: fixed.
- Liquidation valuation source/status discriminant: fixed on this head.
- Remaining: derived
bonusUsdcoherence and uint8 token-decimal validation.
Validation
pnpm install --frozen-lockfile— passed; worktree remained clean.pnpm --filter @eulerxyz/euler-v2-sdk test— 33 files / 488 tests passed, including 2 public type tests.pnpm --filter @eulerxyz/euler-v2-sdk typecheck— passed.pnpm --filter @eulerxyz/euler-v2-sdk build— passed.pnpm --filter @eulerxyz/euler-v2-sdk lint— exit 0; 5 warnings / 4 infos, all outside this feature path.git diff --check origin/main...HEAD— passed.- Focused built-package adversarial fixtures — confirmed acceptance of
bonusUsdwith zero or one USD leg and token decimals256. - Producer contract — current
euler-data-v3computesbonusUsdonly when both legs exist, nulls historical decimals outside0..255, and documents the bonus ascollateralAssetsUsd - repayAssetsUsd, null unless both legs are available. - Live fixture —
/v3/liquidations?chainId=1&limit=100returned 100 coherent available rows;limit=0normalized to 1,limit=101clamped to 100, and an overshoot offset returned a valid empty page.
Coverage and review frames
All 10 paginated changed files were inspected and dispositioned:
src/sdk/sdk.ts— reviewed-clean: public built-in guarantee, default/legacy/modern override construction, and declaration surface.activityEvent.ts— findings: derived bonus coherence and token-decimal upper bound; other number/string normalization, nullable metadata, valuation status/source, request scope, timestamps, addresses, and pagination paths were inspected and exercised.activityService.ts— reviewed-clean: delegation, unavailable fallback, query wrapping, cache key, and legacy-service wrapper.activityServiceTypes.ts— reviewed-clean: request, record, page, adapter, legacy override, and stronger built-in contracts checked against producer/OpenAPI shapes.activityV3Adapter.ts— reviewed-clean: URL construction, query validation/encoding, API-key header, redirect rejection, timeout, streamed 2 MiB cap, parsing, and request-aware validation.activityService/index.ts— reviewed-clean: package-root value/type export chain.activityService.test.ts— finding: a test currently asserts the contradictory unavailable-plus-bonus shape; other route, malformed response, timeout/body cap, pagination, and valuation regressions pass.activityServicePublicTypes.test-d.ts— reviewed-clean: direct SDK callability and legacy override assignment.tsconfig.typetest.json— reviewed-clean: scoped no-emit fixture coverage.vitest.config.ts— reviewed-clean: runtime and public type tests both execute under the package test command.
The bounded intent/invariant, changed-code correctness, impact/integration, adversarial/security, test-discrimination, and fresh-challenge frames are complete. Negative hypotheses included stale/wrong-scope pages, malformed pagination, nullable metadata, inconsistent derived fields, out-of-range decimals, numeric precision, public API compatibility, custom-service wrapping, cache-key collisions, unbounded/redirected responses, credential leakage, and unexpected dependency/workflow/transaction surfaces. No dependency, workflow, permission, transaction-construction, telemetry, or secret-exposure changes were introduced.
Scalability / maintainability hygiene
The route follows the existing shared adapter, transport, query-cache, and response-validation structure. The remaining narrow reusable abstraction is a single liquidation-enrichment validator that checks the USD-leg/bonus/token-metadata family together, rather than adding isolated conditions around individual return fields. The sibling readAsset path already demonstrates the shared 0..255 token-decimal invariant. Add table-driven regressions for available/partial/unavailable rows with coherent and incoherent bonus/metadata combinations.
Bot feedback
CodeRabbit’s earlier numeric-string finding is fixed. Its raw-JSON-number lexeme comment is not promoted: the v3 wire contract supplies JSON numbers, so precision beyond JavaScript number is already lost before this normalizer unless the wire type or parser changes. CodeRabbit’s current clean summary did not exercise the two focused cases above.
Screenshots: not applicable; this is an SDK/API-only change with no UI surface.
…al decimals The producer emits bonusUsd (collateralAssetsUsd - repayAssetsUsd) exactly when both legs are valued; the parser now rejects a bonus without both inputs and valued legs without their derived bonus. Historical token decimals share the uint8 bound the sibling asset parser enforces — the producer nulls out-of-range values rather than emitting them.
|
Both findings addressed in 80ced9b:
Verified through the built package that both cited repros reject while three live page shapes (default, vault-filtered partial-heavy, null-metadata offset-800) still parse. 488/488 tests + type pass. |
LeonardEulerXYZ
left a comment
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Review: approved
Reviewed exact head 80ced9b587380bd6b818b1e897d16efc4c092c0b against main using anti-anchoring-v1.
The latest commit resolves both remaining blockers. Historical token decimals now share the producer/sibling-parser 0..255 bound, and bonusUsd is present exactly when both USD legs are valued. The full liquidation path is coherent across normalization, request-scoped page validation, bounded transport, service/query wiring, the SDK public type, legacy overrides, and package exports.
Re-review reconciliation
- Nullable historical token metadata: fixed.
- Request-scoped response and pagination checks: fixed.
- Exact exponent expansion and established
amountUsd: stringcompatibility: fixed. - Built SDK callability plus legacy override compatibility: fixed.
- Valuation source/status discriminant: fixed.
- Derived bonus presence and historical decimal bounds: fixed on this head.
- No new blocking finding surfaced in the fresh current-head lane.
Validation
corepack pnpm install --frozen-lockfile— passed; review worktree remained clean.corepack pnpm --filter @eulerxyz/euler-v2-sdk exec vitest run test/activityService.test.ts— 59/59 tests passed; public typecheck also reported no errors.corepack pnpm --filter @eulerxyz/euler-v2-sdk test— 33 files / 488 runtime tests passed, plus 2 public type tests; no type errors.corepack pnpm --filter @eulerxyz/euler-v2-sdk typecheck— passed.corepack pnpm --filter @eulerxyz/euler-v2-sdk build— passed.corepack pnpm --filter @eulerxyz/euler-v2-sdk lint— exit 0; 5 warnings / 4 infos, all outside this diff.git diff --check origin/main...HEAD— passed.- Live contract pass — checked
/v3/openapi.jsonand all 2,375 current mainnet liquidation rows: 766 available, 39 partial, 1,570 unavailable, 23 negative bonuses; valuation status/source and bonus presence were coherent throughout. - Focused adversarial pass — malformed USD-leg/bonus presence, source/status, scope, pagination, decimal, address, timestamp, timeout, redirect, and response-size hypotheses were exercised or traced.
Changed-file coverage
src/sdk/sdk.ts— reviewed-clean: public stronger service type, default/legacy/modern construction, and runtime wrapper boundary checked.activityEvent.ts— reviewed-clean: enrichment normalization, nullable metadata, valuation discriminant, bonus presence, decimal bounds, request scope, timestamps, and pagination inspected against live/OpenAPI fixtures.activityService.ts— reviewed-clean: delegation, unavailable fallback, legacy wrapping, query method, and case-stable query key traced.activityServiceTypes.ts— reviewed-clean: request/record/page types and override-facing versus built-in service compatibility checked.activityV3Adapter.ts— reviewed-clean: argument validation, URL encoding, API-key path, timeout, redirect rejection, streamed 2 MiB cap, parsing, and request-aware validation traced.activityService/index.ts— reviewed-clean: value/type exports reach the package root.test/activityService.test.ts— reviewed-clean: 59 focused tests include live-like, malformed, pagination, transport, compatibility, and current-head regression cases.test/activityServicePublicTypes.test-d.ts— reviewed-clean: direct SDK callability and legacy override assignment both typecheck.tsconfig.typetest.json— reviewed-clean: no-emit public fixture scope is bounded to source and type tests.vitest.config.ts— reviewed-clean: runtime and type-test discovery are separated and both execute under the package test command.
Intent/invariants, changed-code correctness, impact/integration, adversarial/security, test discrimination, and the independent fresh-challenge frame were completed. No dependency, workflow, permission, transaction-construction, telemetry, secret, generated-package, or supply-chain surface changed.
Scalability / maintainability hygiene
I traced sibling account/vault activity validation, shared bounded transport, service wrapping, query caching, package exports, SDK construction, and downstream type fixtures. The reusable logic sits at the existing parser/validator and service boundaries rather than in a consumer-specific path; no sibling flow retains an old liquidation behavior. Two hardening ideas were considered but are not promoted to blockers: algebraically rechecking floating-point bonusUsd would need a contractually documented tolerance, and bounding response meta.limit would be stricter than the generic published pagination schema. The current code validates the producer's presence/discriminant invariants and live data conforms.
Bot feedback and evidence
CodeRabbit's numeric-string finding is fixed. Its raw-JSON-lexeme precision comment remains non-actionable for this patch because the wire contract supplies JSON numbers and precision beyond JavaScript number is lost before this normalizer. No active bot finding changes the verdict.
Screenshots: not applicable; this is an SDK/API-only change with no UI surface.
3 participants
Adds support for the new standalone
/v3/liquidationsendpoint to the activity service, and forward-compatible parsing for the asset-enrichment fields on activity event amounts.Liquidations endpoint
The v3 API now exposes historical liquidation records with USD valuations computed from price snapshots at the time of the event — including the seized collateral converted to underlying units and the liquidator bonus (
collateralAssetsUsd - repayAssetsUsd, which can be negative for underwater liquidations).ActivityService.fetchLiquidations(args)/queryLiquidations/getQueryKeyLiquidations— same service triple pattern as the events queries, with query keys checksumming optional address filters so differently-cased inputs hit the same cache entry.ActivityV3Adapter.fetchLiquidationsbuilds the request (chainIdrequired; optionalvault/violator/liquidator/from/to/limit/offset), validates args (limit capped at 100,from <= to, checksummed addresses), and applies the shared response-size cap and timeout.normalizeLiquidationsResponsestrictly validates the payload: decimal-string amounts, tx hashes, addresses,valuationstatus/source, and finite-number USD fields.bonusUsdexplicitly allows negative values;nullUSD fields (valuation unavailable) are normalized to omitted so consumers only branch on presence.fetchLiquidationsis optional onIActivityAdapter; the service throwsActivityUnavailableError("source-not-configured")for adapters that don't implement it (includingUnavailableActivityAdapter), so older adapters remain valid implementations.Event asset enrichment
ActivityAssetAmountgains optionalamountUnderlyingRaw,underlyingAddress,underlyingDecimals, andamountUsdfields matching the enrichment the API attaches to event amounts, with strict validation that tolerates their absence on older payloads.Covered by unit tests for URL construction, normalization (happy path, negative bonus, null tolerance, malformed rejection), argument validation, unavailable-adapter behavior, and query-key stability across address casing.
Summary by CodeRabbit
activityService.fetchLiquidationsat runtime (wrapping unsupported overrides).