Skip to content

fix(lending): accept --amount max sentinel in supply/borrow/repay/withdraw - #13

Merged
Hiksang merged 1 commit into
mainfrom
fix/lending-amount-max
May 6, 2026
Merged

fix(lending): accept --amount max sentinel in supply/borrow/repay/withdraw#13
Hiksang merged 1 commit into
mainfrom
fix/lending-amount-max

Conversation

@Hiksang

@Hiksang Hiksang commented May 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

defi --chain <X> lending withdraw --asset <T> --amount max --broadcast failed with Cannot convert max to a BigInt because lending.ts called BigInt(opts.amount) directly. token.ts approve has supported the "max" sentinel since the SSOT 7.x baseline; lending lagged.

Reproduced live on Mantle Aave V3 during the 2026-05-07 multi-chain broadcast verification — the workaround was to call lending position first to read the exact balance, then pass it back as --amount <integer>. That extra round-trip is exactly what --amount max was meant to spare the user.

Fix

ts/packages/defi-cli/src/commands/lending.ts:

  • Adds parseAmount(s) helper that maps "max" / "all" / "ALL" (case-insensitive) to viem.maxUint256, otherwise defers to BigInt(s) so every existing wei/integer input is preserved verbatim.
  • Applies the helper in all four state-changing handlers — supply, borrow, repay, withdraw — for symmetry with the Aave V3 / Compound V2 sentinel convention (type(uint256).max = "repay all" / "withdraw all").
  • Updates --amount help strings to mention 'max'.

Adapter compatibility (no changes needed)

Adapter uint256.max behaviour
aave_v3.ts Already logs "Withdraw all (auto-max)" in the description — confirmed live during the 2026-05-07 verification when an explicit integer amount equal to the user's balance was supplied.
compound_v2.ts Forwards redeemUnderlying(amount) verbatim to the cToken; uint256.max is the standard Compound V2 sentinel for "withdraw all".

Test plan

  • 12 new tests in commands/lending.test.ts (4 subcommands × 3 cases each):
    • --amount maxmaxUint256 in tx description
    • --amount ALL → same (case-insensitive)
    • --amount 12345 → stays a literal bigint
  • vi.mock stubs createLending() so the tests run offline. The stub's build* methods echo params.amount back into the description, making the parser's round-trip observable end-to-end through the Executor's dry-run preview.
  • pnpm -C ts -r build — clean.
  • pnpm -C ts -r lint — 3 packages, tsc --noEmit clean.
  • pnpm -C ts -r test — defi-core 32/32, defi-protocols 39/39, defi-cli 87/87 (was 75; +12 lending tests).
  • Live broadcast verification of the new max path is intentionally deferred — the parser fix is unit-test-covered and the adapter side-effects of uint256.max are unchanged from prior on-chain behaviour.

Out of scope

A second bug surfaced during the same broadcast verification: Compound V2 (Venus) on BNB rejects native BNB (0x0 sentinel) because the adapter's resolveVtoken() looks up by underlying() and vBNB.underlying() reverts. That's a separate, larger fix tracked in a follow-up PR.

🤖 Generated with Claude Code

…hdraw

Pre-2026-05-07 behaviour: `defi --chain <X> lending withdraw --asset <T>
--amount max --broadcast` failed with `Cannot convert max to a BigInt`
because lending.ts called BigInt(opts.amount) directly. token.ts approve
had supported the "max" sentinel since the SSOT 7.x baseline; lending
lagged.

Reproduced live on Mantle Aave V3 during the 2026-05-07 multi-chain
broadcast verification — the workaround was to call `lending position`
first to read the exact balance and pass it back. That round-trip is
exactly what `--amount max` was supposed to spare the user.

Fix:
  - Add a small `parseAmount()` helper that maps "max" / "ALL" / "all"
    (case-insensitive) to viem's maxUint256, and otherwise defers to
    BigInt() — preserving every existing decimal/wei input verbatim.
  - Apply the helper in all four state-changing handlers (supply,
    borrow, repay, withdraw) for symmetry — Aave V3 and Compound V2
    forks accept type(uint256).max for "repay all" the same way they
    accept it for "withdraw all".
  - Update the `--amount` help strings to mention `'max'`.

The downstream adapters already understand uint256.max:
  - lending/aave_v3.ts already logs "Withdraw all (auto-max)" in the
    description when the user-supplied amount equals their balance,
    confirmed live during the 2026-05-07 verification with an explicit
    integer amount.
  - lending/compound_v2.ts forwards redeemUnderlying(amount) verbatim
    to the cToken; uint256.max is the standard Compound V2 sentinel
    for "withdraw all".

Test plan:
  - 12 new tests in lending.test.ts (4 subcommands × 3 cases each):
    * --amount max -> maxUint256 in tx description
    * --amount ALL -> same (case-insensitive)
    * --amount 12345 -> stays a literal bigint
  - vi.mock stubs createLending() so the tests run offline; the stub's
    build* methods echo `params.amount` back into the description, so
    the parser's round-trip is observable end-to-end through the
    Executor's dry-run preview.
  - Live broadcast verification of the new max path is intentionally
    deferred — the parser fix is unit-test-covered and the adapter
    side-effects of uint256.max are unchanged from prior on-chain
    behaviour (Aave V3's auto-max log + Compound V2's standard
    sentinel handling).

Verified:
  - pnpm -C ts -r build  — clean.
  - pnpm -C ts -r lint   — 3 packages, tsc --noEmit clean.
  - pnpm -C ts -r test   — defi-core 32/32, defi-protocols 39/39,
                           defi-cli 87/87 (was 75; +12 lending tests).
@Hiksang
Hiksang merged commit ac55a13 into main May 6, 2026
4 checks passed
@Hiksang
Hiksang deleted the fix/lending-amount-max branch May 6, 2026 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant