Skip to content

navi sui transactions - #94

Merged
harshaalphafi merged 10 commits into
mainfrom
bugfix/navi
Aug 3, 2026
Merged

navi sui transactions#94
harshaalphafi merged 10 commits into
mainfrom
bugfix/navi

Conversation

@Zorag44

@Zorag44 Zorag44 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@Zorag44
Zorag44 requested a review from jangid as a code owner August 1, 2026 13:08
@Zorag44
Zorag44 requested review from 11felix and rg-alpha August 1, 2026 13:08
@jangid

jangid commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

operate.md review — alphafi-sdk-js#94

First review on this PR (no prior human or bot comments). Re-derived against HEAD 2026-08-01; CI green (test/lint/build SUCCESS), MERGEABLE / BLOCKED (pending review, not a conflict).

Verified — the new SUI branch is correct against the Move source

src/strategies/lending.ts adds a poolLabel.asset.name === 'SUI' arm between the USDY and vSUI arms. I checked both new moveCalls against alpha/alphafi/sources/navi/alphafi_navi_investor.move:

Move signature PR call
collect_v3_rewards_with_one_swap_bluefin<T,S> (:271) self, version, clock, storage, asset: u8, incentive_v3, funds_pool: &mut RewardFund<S>, bluefin_pool: &mut BluefinPool<S,T>, config: &BluefinConfig T=SUI, S=NAVX; REWARDS_POOL.NAVX → pool(NAVX,SUI,bluefin) → GLOBAL_CONFIGS.BLUEFIN
collect_v3_rewards_with_one_swap<T,S> (:194) …, funds_pool: &mut RewardFund<S>, cetus_pool: &mut CetusPool<S,T>, config: &GlobalConfig T=SUI, S=vSUI; REWARDS_POOL.vSUI → pool(vSUI,SUI,cetus) → GLOBAL_CONFIGS.CETUS

Type-argument order ([asset.type, rewardCoinType] = <T,S>), the BluefinPool<S,T> / CetusPool<S,T> pair ordering, and the pool-before-config argument order all match. NAVI_CONFIG.ASSET_MAP.SUI = '0' and both REWARDS_POOL.NAVX / REWARDS_POOL.vSUI exist (src/utils/constants.ts:136,138,171). getPoolIdBySymbolsAndProtocol throws on an unregistered pair (src/models/strategyContext.ts:681-685), so a missing pool fails loudly rather than building a malformed PTB. Branch placement is correct — SUI cannot be shadowed by an earlier arm.

🔴 What this PR gets right that the code it was ported from gets wrong (out of scope, but worth acting on)

The same call exists in two places on main elsewhere, and both have config and bluefin_pool swapped relative to the Move signature:

  • alphafi-sdk-rust/src/tx/autocompound.rs:3591-3593vec![investor, version, clock, storage, idx, incentive, rp, config, p1]
  • alpha/sui-alpha-sdk/src/crons/autocompound.ts:3227-3238BLUEFIN_GLOBAL_CONFIG then bluefinPoolMap["NAVX-SUI"]

In each file the sibling cetus branch immediately below it has the order right (…, rp, p1, config), and the two collect_v3_rewards_with_two_swaps_bluefin call sites in the Rust SDK (:3686, :3738) are also right (rp, p1, p2, config) — so one_swap_bluefin is the lone outlier in both repos, the same mistake copied across. The Move function has had this signature since it was introduced (single commit 6e901ed2), so there is no older deployed ordering that would explain it.

Consequence: the NAVI-SUI pool's NAVX reward autocompound via Bluefin builds a PTB passing &BluefinConfig where &mut BluefinPool<S,T> is expected, which aborts at execution. Not introduced here — flagging because this PR is the correct version and the two live ones are not. Worth a separate fix in alphafi-sdk-rust (that is the path the crons actually run).

🟢 Non-blocking notes on scripts/testRun.ts

Dev-scratch churn, no production impact, but it is committed:

  1. The active call flips from withdraw() to deposit() and the withdraw() poolId is changed to the same id as deposit(), so the previous withdraw target is lost from the file.
  2. The two new helpers disagree on environment: withdrawApi posts to production https://api.alphalend.xyz/slush/v1/withdraw while depositApi posts to https://api-staging.alphalend.xyz/slush/v1/deposit. Both are dry-run-only today (executeTransactionBlock commented out), but a scratch helper pointed at prod is an easy foot-gun to leave lying around.
  3. They also disagree on decoding: Transaction.fromKind(res.bytes) vs Transaction.from(res.bytes). Only one can be right for a given endpoint's response shape — worth making them consistent, or commenting why they differ.

No secrets in the diff (the hardcoded values are public object ids and a sender address).

Approving — the production change is correct and CI is green; everything above is either advisory or out of scope. Approvals: 1/2 after this one — needs one more before merge.

@jangid jangid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

operate.md: clean — CI green (test/lint/build). Verified both new moveCalls against alphafi_navi_investor.move: type args <T=SUI, S=reward>, BluefinPool<S,T>/CetusPool<S,T> pairing, and pool-before-config argument order all match the signatures. Note in the summary comment: the same call on alphafi-sdk-rust main (src/tx/autocompound.rs:3591) and in alpha's legacy cron has config and pool swapped — this PR is the correct version, those two are not. Approvals: 1/2 — needs one more before merge.

@harshaalphafi
harshaalphafi merged commit 34eef8b into main Aug 3, 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.

4 participants