Skip to content

Migrate Navi integration to @naviprotocol/lending v2 - #87

Merged
11felix merged 7 commits into
mainfrom
feature/navi-sdk-v2
Jul 24, 2026
Merged

Migrate Navi integration to @naviprotocol/lending v2#87
11felix merged 7 commits into
mainfrom
feature/navi-sdk-v2

Conversation

@11felix

@11felix 11felix commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Fetches Navi rewards directly via getUserAvailableLendingRewards from @naviprotocol/lending@^2.0.3 (over a new gRPC Core client on Blockchain) instead of the integration API's /navi-params/rewards endpoint. Adds a local .d.ts shim since NAVI v2's split declarations don't resolve under moduleResolution: NodeNext.

@11felix
11felix requested a review from jangid as a code owner July 20, 2026 16:12
@11felix
11felix requested review from OctoSauce and Zorag44 July 20, 2026 16:13
Comment thread src/strategies/looping.ts Outdated
@11felix
11felix requested a review from Zorag44 July 22, 2026 14:23
Comment thread package.json Outdated
@11felix
11felix requested a review from Zorag44 July 22, 2026 19:33
Zorag44
Zorag44 previously approved these changes Jul 22, 2026
@jangid

jangid commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

operate.md full-sweep review — PR #87 (Navi v1→v2 migration). CI green (lint/test/build); mergeable BLOCKED; approvals 1/2 (needs a 2nd).

Prior blocker resolved ✅ — the SUI-reward 0x-prefix key mismatch is fixed in 7bb84a4; verified every reward-map lookup now routes through normalizeStructTag(...) (looping.ts:549–594, lending.ts:305–306). Version pin ✅ (@naviprotocol/lending@2.0.3 exact). Supply chain clean — the added lockfile entries carry no prepare/install/postinstall lifecycle scripts (relevant given the #85 incident).

New (non-blocking, but same silent-failure class the prior 🔴 was):

  1. 🟡 Asymmetric key normalization — looping.ts:300 / lending.ts:283 build the reward-group map with the raw reward.assetCoinType key while every consumer looks it up with normalizeStructTag(...). Correctness is now coupled to NAVI's output being byte-identical to @mysten normalizeStructTag. Recommend normalizing the group key too (normalizeStructTag(reward.assetCoinType)) so both sides share one function — otherwise the silent SUI-reward-drop bug can recur on any NAVI format change.
  2. 🟡 Inner rewardCoinType match unnormalized — looping.ts:327/329/350… and the lending.ts equivalents use strict reward.rewardCoinType === coin.coinType / rewardCoinSet.has(...) with no normalization on either side. The migration changed the reward source, so please confirm NAVI v2's rewardCoinType string form matches getCoinsBySymbols().coinType. (Also: pre-existing snake_case reward.reward_coin_type at looping.ts:490 vs camelCase elsewhere — verify against NAVI v2's actual shape.)
  3. 🟡 Missing test — the integration test for the silent SUI-reward path requested in review was not added. Given both paths above fail silently (lost yield, no error), a test against a live position with SUI rewards would de-risk the migration.

Recommendation: hold for the 2nd approval; addressing the symmetric-normalization fix (#1) and the test (#3) before merge is advised since these are silent yield-loss paths.

@11felix

11felix commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in 5a816b4.

Verified NAVI v2's shape against the actual @naviprotocol/lending@2.0.3 type: LendingReward is { assetCoinType: string; rewardCoinType: string; ... } — camelCase, and no reward_coin_type field.

#1 (asymmetric key normalization) — fixed. Both looping.ts and lending.ts now build the reward-group map key with normalizeStructTag(reward.assetCoinType), so build and lookup share one function. normalizeStructTag is idempotent, so it's safe on already-canonical input.

#2 (snake_case dedup field) — fixed as a real bug, not just a naming nit. looping.ts:490 did rewardCoinSet.add(reward.reward_coin_type); that field is undefined on v2, so the set only ever held undefined and the rewardCoinSet.has(reward.rewardCoinType) guard never fired — the cross-group dedup (supply-asset vs. SUI rewards) silently never worked. Now reward.rewardCoinType, symmetric with the .has() check.

Not done yet:

  • The inner reward.rewardCoinType === coin.coinType comparisons (~20 sites) remain unnormalized. Field name confirmed via the type above; the string-format coupling you flagged is real but making all sites robust is a larger change I'd rather do as a focused follow-up than fold into this PR.
  • Integration test (deposit and withdraw function fix #3) still outstanding — needs a live position with SUI rewards / fixtures. Tracking separately.

@11felix
11felix requested a review from Zorag44 July 24, 2026 06:34
@11felix
11felix merged commit fae8045 into main Jul 24, 2026
3 checks passed
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.

5 participants