AutobalanceLp pending-rewards function implementation#60
Conversation
11felix
commented
Apr 21, 2026
11felix
commented
- Adds autobalanceLpPendingRewardAmount(userAddress, poolId) to the SDK, which returns a user's pending non-ALPHA rewards for an AutobalanceLp pool as a map of reward coin type to decimal string (throws if the pool is not AutobalanceLp). 2. Also migrates a couple of remaining JSON-RPC reads (wallet balances, Bucket TVL) to GraphQL and updates the README.
jangid
left a comment
jangid
left a comment
There was a problem hiding this comment.
Review Summary
Two changes bundled: (1) new autobalanceLpPendingRewardAmount() function, (2) JSON-RPC → GraphQL migration for coins/balances/simulation/Bucket TVL.
The GraphQL migration portions look solid. The pending-rewards feature works but has a pagination bug in getAllBalances and silent error swallowing that should be addressed before merge.
| return undefined; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Bug: getAllBalances is not actually paginated. The query accepts a $cursor variable and the response includes pageInfo, but this method never loops — it fetches one page and returns. If a user holds more coin types than fit in a single page, balances will be silently truncated.
The getCoins migration above correctly paginates with a do/while loop — this method needs the same treatment.
| } | ||
| } | ||
|
|
||
| private collectReward(tx: Transaction) { |
There was a problem hiding this comment.
Silent error swallowing in a financial context. The catch block logs the error and returns {}. Callers cannot distinguish "no pending rewards" from "simulation failed."
At minimum, propagate the error (or return a discriminated result type) so callers know something went wrong. Returning empty data on failure is dangerous when users are making financial decisions based on this output.
| }); | ||
| tx.moveCall({ | ||
| target: `${this.poolLabel.packageId}::alphafi_bluefin_type_1_pool::get_cur_acc_per_xtoken`, | ||
| typeArguments: [this.poolLabel.assetA.type, this.poolLabel.assetB.type], |
There was a problem hiding this comment.
Heavy code duplication. The three branches (assetA.name === 'SUI', assetB.name === 'SUI', else) are nearly identical — only the Move module name differs (sui_first_pool / sui_second_pool / type_1_pool) and version suffix (v4 vs v3). This pattern already exists in collectReward below, but duplicating it again compounds the maintenance burden.
Consider extracting a helper that takes the module name and version string.
|
|
||
| const query = graphql(` | ||
| query simulate($tx: JSON!) { | ||
| simulateTransaction(transaction: $tx, checksEnabled: true, doGasSelection: false) { |
There was a problem hiding this comment.
Untyped simulation response. simulateTransaction returns any, and downstream code navigates deeply into res?.outputs?.[res.outputs.length - 1]?.returnValues?.[0]?.value?.json?.contents. If the GraphQL schema changes, this will silently break at runtime.
A typed response interface would catch regressions at compile time.
| .mul(userXtokenBalance) | ||
| .div(1e36) | ||
| .plus(pending) | ||
| .div(Math.pow(10, decimals)) |
There was a problem hiding this comment.
Nit: 1e36 is a magic number — a named constant would clarify its meaning (presumably accumulated reward precision scaling). Also, type as a variable name shadows the TypeScript keyword; rewardType would be clearer.
jangid
left a comment
jangid
left a comment
There was a problem hiding this comment.
Re-review
All five issues from the previous review have been addressed:
getAllBalancespagination — fixed, proper cursor loop now in place- Silent error swallowing — fixed, errors propagate to callers
- Code duplication — partially fixed via
getPoolModule()/getUpdatePoolFn()helpers - Untyped simulation response — fixed, typed interfaces in
types.ts - Magic number + variable naming — fixed,
ACC_REWARD_PER_XTOKEN_PRECISIONconstant +rewardType
Minor follow-up: collectReward() and getUserRewards() still repeat the three-branch pattern that getPoolModule() was introduced to eliminate. Tracked in a separate issue — not blocking this PR.
3 participants
