Skip to content

fix: drop withdrawal type options below the asset minimum (ORD-2049) - #619

Open
aaronmboyd wants to merge 1 commit into
mainfrom
feature/ord-2049
Open

aaronmboyd wants to merge 1 commit into
mainfrom
feature/ord-2049

Conversation

@aaronmboyd

Copy link
Copy Markdown
Collaborator

Closes ORD-2049
Linear: https://linear.app/lifi-linear/issue/ORD-2049

Intent

PerpsClient.getWithdrawalTypes now joins each source asset against the core /assets registry and removes each option whose max is below the asset's minWithdrawalAmount. The Lighter FAST option with a 0.134502 USDC cap no longer appears next to a 1.000000 USDC minimum.

What changed

  • packages/perps-sdk/src/client/PerpsClient.ts
    • New module-private helper minWithdrawalAmountOf(asset). It holds the parse and the SDKError message that getWithdrawableBalances had inline. getWithdrawableBalances now calls it, and its behaviour does not change.
    • getWithdrawalTypes syncs the asset registry after the plugin read. For each row, it looks up row.source.asset.id. When the asset has a minimum, it keeps only the options where max >= minimum (isDecimalStringGreaterThan, no floats). A row whose asset has no minimum, or that the registry does not carry, is returned as the same object.
    • The TSDoc names the removal rule and the new throw cases (asset sync failure, bad minWithdrawalAmount).
  • packages/perps-sdk/README.md: one sentence on the removal rule in the getWithdrawalTypes() paragraph.
  • lighterWithdrawalTypes does not change. The venue cap stays verbatim.

Assumptions

  • A row whose options are all below the minimum stays in the result with an empty options array. This cannot occur for Lighter today: the STANDARD max equals the row max, and getWithdrawableBalances already removes a row whose available is below the minimum.
  • getWithdrawalTypes now does one /assets read (cached by the registry) per call. That is the same cost as getWithdrawableBalances.

Tests

packages/perps-sdk/src/client/PerpsClient.unit.spec.ts, describe('getWithdrawalTypes'):

  • Added: FAST max 0.134502 with a 1.000000 minimum gives STANDARD only (regression).
  • Added: FAST max 25 and FAST max 1 (equal to the minimum) keep both options.
  • Added: an asset with no minWithdrawalAmount keeps all options.
  • Added: an asset that the registry does not carry keeps all options.
  • Added: a minWithdrawalAmount that is not a decimal throws the same SDKError message as getWithdrawableBalances.
  • Edited: the delegation case now serves /assets and asserts toEqual in place of toBe.

The getWithdrawableBalances cases are unchanged and pass.

Follow-ups

(none). ORD-2052 (widget pin bump) already exists and this issue blocks it.

Dependencies / upstream branches

None. A human runs the release after merge. ORD-2052 consumes the published @lifi/perps-sdk patch.

@aaronmboyd aaronmboyd added release-preview Publish a 0.0.0-preview-<sha> build of this PR to npm Agent Review Request labels Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

📦 Preview published under the preview dist-tag.

Install the exact version(s) — @preview moves with the newest preview across PRs:

npm i @lifi/perps-sdk@0.0.0-preview-3530448

@github-actions github-actions Bot removed the release-preview Publish a 0.0.0-preview-<sha> build of this PR to npm label Oct 9, 2026

@lifi-qa-agent lifi-qa-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zeus QA Review — ORD-2049 · #619

Verdict: ✅ Approve — 0 Critical · 0 High · 0 Medium · 0 Low

getWithdrawalTypes now syncs the asset registry and drops every option whose max is below the source asset's minWithdrawalAmount.

All six acceptance criteria are met:

  • Shared helper. minWithdrawalAmountOf is lifted verbatim from getWithdrawableBalances, so both methods use the same parse and the same SDKError message.
  • Decimal math. The filter keeps an option when max >= minimum, using !isDecimalStringGreaterThan(minimum, max). There is no float math, and an equal max stays.
  • Rows returned unchanged. A row with no minimum, or whose asset is not in the registry, comes back as the same object.
  • No mutation. Rows are rebuilt with a spread, so the plugin's array is not touched.
  • Plugin result. undefined from the plugin still returns undefined, and no /assets read happens in that case.
  • Sync call. registry.sync() takes no arguments, as in the other call sites.

Specs cover the regression (FAST 0.134502 against a 1.000000 minimum gives STANDARD only), the equal and above cases, a missing minimum, an asset the registry does not carry, and a non-decimal minimum. The README and TSDoc are updated, and the patch changeset is correct.

🔵 Notes, none of them gating:

  • CI checks was still pending when I reviewed, and I did not re-run the gates.
  • getWithdrawalTypes can now throw if the /assets sync fails. The TSDoc discloses this, and it matches getWithdrawableBalances.
  • A row whose options are all removed stays in the result with options: []. The author discloses this, and it cannot happen for Lighter today.
  • The live withdraw-sheet check is for a human, through the follow-up widget pin bump (ORD-2052).

@lifi-qa-agent

lifi-qa-agent Bot commented Oct 9, 2026

Copy link
Copy Markdown

Zeus QA Review — ORD-2049 · #619

Verdict: ✅ Approve — 0 Critical · 0 High · 0 Medium · 0 Low

getWithdrawalTypes now syncs the asset registry and drops every option whose max is below the source asset's minWithdrawalAmount.

All six acceptance criteria are met:

  • Shared helper. minWithdrawalAmountOf is lifted verbatim from getWithdrawableBalances, so both methods use the same parse and the same SDKError message.
  • Decimal math. The filter keeps an option when max >= minimum, using !isDecimalStringGreaterThan(minimum, max). There is no float math, and an equal max stays.
  • Rows returned unchanged. A row with no minimum, or whose asset is not in the registry, comes back as the same object.
  • No mutation. Rows are rebuilt with a spread, so the plugin's array is not touched.
  • Plugin result. undefined from the plugin still returns undefined, and no /assets read happens in that case.
  • Sync call. registry.sync() takes no arguments, as in the other call sites.

Specs cover the regression (FAST 0.134502 against a 1.000000 minimum gives STANDARD only), the equal and above cases, a missing minimum, an asset the registry does not carry, and a non-decimal minimum. The README and TSDoc are updated, and the patch changeset is correct.

🔵 Notes, none of them gating:

  • CI checks was still pending when I reviewed, and I did not re-run the gates.
  • getWithdrawalTypes can now throw if the /assets sync fails. The TSDoc discloses this, and it matches getWithdrawableBalances.
  • A row whose options are all removed stays in the result with options: []. The author discloses this, and it cannot happen for Lighter today.
  • The live withdraw-sheet check is for a human, through the follow-up widget pin bump (ORD-2052).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant