feat: aggregate claim-all rewards across all positions#864
Open
feat: aggregate claim-all rewards across all positions#864
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe PR refactors the claim-all reward flow from locally computing claimable positions to using server-derived position rewards data, introducing new query hooks, response types, and message construction logic keyed by explicit position IDs. Changes
Sequence DiagramsequenceDiagram
participant Component as WalletBalanceContainer
participant Query as useGetPositionRewards
participant Repo as PositionRepository
participant API as /users/{address}/position/reward
participant PositionHook as usePosition<br/>(claimAll)
participant MsgBuilder as makeClaimAllMessageWithApprovesByIds
Component->>Query: fetch position rewards (polling every 60s)
Query->>Repo: getPositionRewardsByAddress(address)
Repo->>API: GET /users/{address}/position/reward
API-->>Repo: PositionRewardsResponse
Repo-->>Query: PositionRewardsResponse | null
Query-->>Component: positionRewards + positions
Component->>Component: buildClaimAllInputFromPositions(positions)<br/>→ inspects reward categories
Component->>PositionHook: claimAll(claimAllInput)
PositionHook->>MsgBuilder: makeClaimAllMessageWithApprovesByIds<br/>(swapFeeTokenPaths, positionIDs)
MsgBuilder->>MsgBuilder: build CollectFee + CollectReward<br/>messages from position IDs
MsgBuilder->>Repo: getGRC20Allowance() for approvals
MsgBuilder-->>PositionHook: TransactionMessage[]
PositionHook-->>Component: transaction confirmed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/web/src/layouts/pool/pool-detail/containers/my-liquidity-container/MyLiquidityContainer.tsx (2)
111-115:⚠️ Potential issue | 🟡 MinorInvalidate
positionRewardsafter pool-detail claims.Claiming changes the data returned by
useGetPositionRewards, but this refresh path only invalidates balances, positions, and bins. Add the new query key so portfolio reward totals/claim-all inputs do not stay stale after claiming from pool detail.Proposed fix
invalidateQueryKey("MyLiquidity, Claim", [ [QUERY_KEY.tokenBalancesByAddress, address], [QUERY_KEY.positions, currentChainId, address], + [QUERY_KEY.positionRewards, currentChainId, address], [QUERY_KEY.poolPairBins], ]);
237-251:⚠️ Potential issue | 🟠 MajorAvoid entering loading state for an empty claim-all input.
claimAllreturnsnullwhen both reward position lists are empty, but this handler already shows the pending broadcast and setsloadingTransactionClaimtotrue; that no-op path never resets the loading state.Proposed fix
const claimAllReward = () => { - const amount = openedPosition - .filter(item => !item.closed) + const openedClaimablePositions = openedPosition.filter(item => !item.closed); + const claimAllInput = buildClaimAllInputFromPositions(openedClaimablePositions); + + if (claimAllInput.positionsWithSwapFee.length === 0 && claimAllInput.positionsWithStakingReward.length === 0) { + return; + } + + const amount = openedClaimablePositions .flatMap(item => item.rewards) .reduce((acc, item) => acc + Number(item.claimableUsd), 0); @@ broadcastLoading(getMessage(DexEvent.CLAIM_FEE, "pending", messageData)); setLoadingTransactionClaim(true); - const claimAllInput = buildClaimAllInputFromPositions(openedPosition.filter(item => !item.closed)); claimAll({ rpcProvider, input: claimAllInput }).then(response => {packages/web/src/layouts/portfolio/containers/wallet-balance-container/WalletBalanceContainer.tsx (1)
137-172:⚠️ Potential issue | 🟠 MajorReset claim loading when
claimAllreturnsnullor rejects.Line 137 turns on the loading state, but
claimAllcan resolvenullfor empty input/missing address/caught repository errors. That path never enters theif (response)block, leaving the claim button stuck in loading.🐛 Proposed fix
setLoadingTransactionClaim(true); - claimAll({ rpcProvider, input: claimAllInput }).then(response => { - if (response) { + claimAll({ rpcProvider, input: claimAllInput }) + .then(response => { + if (!response) { + broadcastError(BROADCAST_ERROR_VALUE.DEFAULT); + setLoadingTransactionClaim(false); + return; + } + if (response?.code === 0 || response?.code === ERROR_VALUE.TRANSACTION_FAILED.status) { enqueueEvent({ txHash: response?.data?.hash, action: DexEvent.CLAIM_FEE, checkWugnotTransfer: true, @@ broadcastError(BROADCAST_ERROR_VALUE.DEFAULT); setLoadingTransactionClaim(false); } - } - }); + }) + .catch(() => { + broadcastError(BROADCAST_ERROR_VALUE.DEFAULT); + setLoadingTransactionClaim(false); + });packages/web/src/layouts/portfolio/components/wallet-balance/wallet-balance-detail/WalletBalanceDetail.tsx (1)
106-148:⚠️ Potential issue | 🟠 MajorDon’t gate claim-all availability on tooltip token metadata.
Line 108 drops rewards when
tokenslacks metadata, and Line 147 then disables claim-all based on those tooltip arrays. A missing or late token entry can hide the claim-all button even whenpositionRewardscontains claimable position IDs.🐛 Proposed fix
const isClaimableAll = useMemo(() => { if (balanceDetailInfo.loadingPositions) return false; - return hasInfo(claimableRewardInfo); - }, [claimableRewardInfo, balanceDetailInfo.loadingPositions]); + return ( + !!positionRewards && + (positionRewards.positionsWithSwapFee.length > 0 || positionRewards.positionsWithStakingReward.length > 0) + ); + }, [balanceDetailInfo.loadingPositions, positionRewards]);
🧹 Nitpick comments (1)
packages/web/src/hooks/pool/data/use-position.ts (1)
54-55: Remove the unusedusePositionparameter instead of suppressing ESLint.The claim-all flow now receives
inputat call time, so_positionsis dead API surface. Please drop the parameter and update call sites such asusePosition([])tousePosition().♻️ Proposed refactor
-// eslint-disable-next-line `@typescript-eslint/no-unused-vars` -export const usePosition = (_positions?: PositionModel[]) => { +export const usePosition = () => {As per coding guidelines, “Do not leave unused variables in code — enforced via ESLint”.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9f796952-a334-4b0a-af5c-27ad281ee384
📒 Files selected for processing (16)
packages/web/src/hooks/pool/data/use-position.tspackages/web/src/layouts/pool/pool-detail/containers/my-liquidity-container/MyLiquidityContainer.tsxpackages/web/src/layouts/portfolio/components/wallet-balance/WalletBalance.spec.tsxpackages/web/src/layouts/portfolio/components/wallet-balance/WalletBalance.tsxpackages/web/src/layouts/portfolio/components/wallet-balance/wallet-balance-detail/WalletBalanceDetail.spec.tsxpackages/web/src/layouts/portfolio/components/wallet-balance/wallet-balance-detail/WalletBalanceDetail.tsxpackages/web/src/layouts/portfolio/containers/wallet-balance-container/WalletBalanceContainer.tsxpackages/web/src/react-query/positions/index.tspackages/web/src/react-query/positions/use-get-position-rewards.tspackages/web/src/react-query/query-keys.tspackages/web/src/repositories/position/position-repository-impl.tspackages/web/src/repositories/position/position-repository.tspackages/web/src/repositories/position/position.message.tspackages/web/src/repositories/position/request/claim-all-request.tspackages/web/src/repositories/position/response/index.tspackages/web/src/repositories/position/response/position-rewards-response.ts
| import { useGetAllTokenPrices } from "@query/token"; | ||
| import { DexEvent } from "@repositories/common"; | ||
| import { PositionConverter } from "@services/converters/position"; | ||
| import { delay } from "@utils/common"; |
There was a problem hiding this comment.
Normalize GNOT paths before deciding whether staker approval is needed.
Line 118 uses direct equality, but the position-based builder normalizes reward paths with checkGnotPath(...). If the rewards endpoint returns a native/alias GNOT path, claim-all skips the WUGNOT staker approval and the transaction can fail.
🐛 Proposed fix
-import { delay } from "@utils/common";
+import { checkGnotPath, delay } from "@utils/common"; const hasGnotStakingReward = [
...positionRewards.claimable.internalReward,
...positionRewards.claimable.externalReward,
- ].some(item => item.tokenPath === WRAPPED_GNOT_PATH);
+ ].some(item => checkGnotPath(item.tokenPath) === WRAPPED_GNOT_PATH);Also applies to: 115-118
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Descriptions
Summary
Introduces a server-aggregated position rewards endpoint and wires portfolio wallet balance and claim-all flows to it. Claim-all transaction building now uses explicit LP IDs and token paths instead of passing full
PositionModel[]through the repository.Changes
getPositionRewardsByAddress→GET /users/{address}/position/reward.PositionRewardsResponseDTOs (claimed / claimable groups, USD totals,positionsWithSwapFee,positionsWithStakingReward).ClaimAllRequest.positionswithswapFeeTokenPaths,hasGnotStakingReward,positionsWithSwapFee,positionsWithStakingReward, andrecipient.makeClaimAllMessageWithApprovesByIds: approvals for swap-fee tokens (pool + position), optional wugnot approval for staker when GNOT staking rewards exist, thenCollectFeeper LP ID andCollectRewardper LP ID on the staker.QUERY_KEY.positionRewardsanduseGetPositionRewards(60s refetch, invalidate on claim with positions + balances).WalletBalanceContainer: load rewards withuseGetPositionRewards, derive claim-all input and USD totals frompositionRewards, keep staked / unstaked liquidity frompositions(excluding closed).WalletBalance/WalletBalanceDetail: consumepositionRewards+tokens(path →TokenModel) for tooltips; remove client-side aggregation from converted pool positions; tooltip 1d accumulation fields are no longer populated from positions.MyLiquidityContainer: build claim-all input withbuildClaimAllInputFromPositionsfor non-closed opened positions and passinputintoclaimAll.usePosition:claimAllaccepts{ rpcProvider, input }; exportbuildClaimAllInputFromPositionsandClaimAllInput.