-
-
Notifications
You must be signed in to change notification settings - Fork 276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(suite): add support for Solana staking rewards #16527
Conversation
...ges/suite/src/views/wallet/staking/components/SolStakingDashboard/components/RewardsList.tsx
Outdated
Show resolved
Hide resolved
d57a76b
to
f700b29
Compare
...e/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx
Outdated
Show resolved
Hide resolved
9c30965
to
9fca587
Compare
...e/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx
Outdated
Show resolved
Hide resolved
...e/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx
Outdated
Show resolved
Hide resolved
9fca587
to
961a180
Compare
WalkthroughThis pull request introduces functionality for fetching and displaying staking rewards for Solana accounts. It adds a new allowed domain for the Everstake API in the configuration file. A custom React hook, Suggested labels
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/suite/src/hooks/wallet/useSolanaRewards.ts (1)
21-23
:⚠️ Potential issueGroup rewards by epoch to fix duplicate entries.
Based on past review comments, rewards should be grouped by epoch to prevent duplicate entries.
const { rewards } = data ?? {}; -const selectedAccountRewards = rewards?.[account.descriptor]; +const selectedAccountRewards = rewards?.[account.descriptor]?.reduce((acc, reward) => { + const key = `${reward.epoch}`; + if (!acc[key]) { + acc[key] = []; + } + acc[key].push(reward); + return acc; +}, {});packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx (1)
170-179
:⚠️ Potential issueFix pagination rendering timing.
The pagination component renders before the data is fully loaded, causing a visual inconsistency. Consider moving the pagination inside the loading condition to ensure consistent rendering.
- {showPagination && ( + {!isLoading && showPagination && ( <Pagination hasPages={true} currentPage={currentPage} isLastPage={isLastPage} perPage={itemsPerPage} totalItems={totalItems} onPageSelected={onPageSelected} /> )}
🧹 Nitpick comments (7)
suite-common/wallet-core/src/stake/stakeConstants.ts (1)
16-17
: Fix typo in constant name.The constant name has a typo: "ENPOINT" should be "ENDPOINT".
-export const EVERSTAKE_REWARDS_SOLANA_ENPOINT = +export const EVERSTAKE_REWARDS_SOLANA_ENDPOINT =packages/suite-desktop-core/src/config.ts (1)
31-31
: Fix typo in comment.The comment has a typo: "enpoint" should be "endpoint".
- 'stake-sync-api.everstake.one', // staking rewards enpoint for Solana + 'stake-sync-api.everstake.one', // staking rewards endpoint for Solanasuite-common/wallet-core/src/stake/stakeTypes.ts (1)
97-106
: Consider improving type definitions for better type safety.
- Follow TypeScript naming conventions by using camelCase instead of snake_case
- Consider using number type for
amount
if calculations are needed- Consider using Date or timestamp type for
time
export type StakeAccountRewards = { height: number; epoch: number; validator: string; authority: string; - stake_account: string; + stakeAccount: string; - amount: string; + amount: number; currency: string; - time: string; + time: number; // Unix timestamp };suite-common/wallet-core/src/stake/stakeThunks.ts (1)
110-143
: Consider enhancing error handling and adding pagination support.The current implementation could be improved by:
- Adding pagination parameters to handle large datasets
- Implementing a retry mechanism for failed requests
- Adding more specific error handling
export const fetchEverstakeRewards = createThunk< { rewards: StakeRewardsByAccount }, { symbol: SupportedSolanaNetworkSymbols; endpointType: EverstakeRewardsEndpointType; address: string; + page?: number; + limit?: number; }, { rejectValue: string } >( `${STAKE_MODULE}/fetchEverstakeRewardsData`, async (params, { fulfillWithValue, rejectWithValue }) => { - const { address } = params; + const { address, page = 1, limit = 10 } = params; + const MAX_RETRIES = 3; + let retries = 0; try { - const response = await fetch(`${EVERSTAKE_REWARDS_SOLANA_ENPOINT}/${address}`, { - method: 'POST', - }); + while (retries < MAX_RETRIES) { + try { + const response = await fetch( + `${EVERSTAKE_REWARDS_SOLANA_ENPOINT}/${address}?page=${page}&limit=${limit}`, + { method: 'GET' } + ); + if (!response.ok) { + throw new Error(`HTTP error! status: ${response.status}`); + } + const data = await response.json(); + return fulfillWithValue({ + rewards: { + [address]: data, + }, + }); + } catch (error) { + retries++; + if (retries === MAX_RETRIES) throw error; + await new Promise(resolve => setTimeout(resolve, 1000 * retries)); + } + } }packages/suite/src/hooks/wallet/useSolanaRewards.ts (1)
68-70
: Memoize derived values to prevent unnecessary re-renders.Values derived from state should be memoized to optimize performance.
-const totalItems = selectedAccountRewards?.length ?? 0; -const showPagination = totalItems > itemsPerPage; -const isLastPage = stopIndex >= totalItems; +const totalItems = useMemo(() => selectedAccountRewards?.length ?? 0, [selectedAccountRewards]); +const showPagination = useMemo(() => totalItems > itemsPerPage, [totalItems, itemsPerPage]); +const isLastPage = useMemo(() => stopIndex >= totalItems, [stopIndex, totalItems]);packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx (1)
95-165
: Extract reward item into a separate component.Consider extracting the reward item rendering logic into a separate component to improve code organization and reusability. This would also make the main component more maintainable.
Example structure:
interface RewardItemProps { reward: Reward; account: Account; } const RewardItem = ({ reward, account }: RewardItemProps) => ( <React.Fragment> <Row> <ColDate> <FormattedDate value={reward?.time ?? undefined} day="numeric" month="long" year="numeric" /> </ColDate> </Row> <Card> {/* ... rest of the reward item JSX ... */} </Card> </React.Fragment> );Then use it in the map:
{slicedRewards?.map(reward => ( <RewardItem key={reward.epoch} reward={reward} account={account} /> ))}suite-common/wallet-core/src/stake/stakeReducer.ts (1)
132-174
: Extract common state patterns into helper functions.The state initialization and update patterns are repeated across multiple reducers. Consider extracting these patterns into helper functions to reduce code duplication.
Example implementation:
const createInitialState = (data = {}) => ({ error: false, isLoading: true, lastSuccessfulFetchTimestamp: 0 as Timestamp, data, }); const createSuccessState = (data: unknown) => ({ error: false, isLoading: false, lastSuccessfulFetchTimestamp: Date.now() as Timestamp, data, }); const createErrorState = () => ({ error: true, isLoading: false, lastSuccessfulFetchTimestamp: 0 as Timestamp, data: {}, });Then use these helpers in the reducer:
.addCase(fetchEverstakeAssetData.pending, (state, action) => { const { symbol, endpointType } = action.meta.arg; if (!state.data[symbol]?.[endpointType]) { state.data[symbol] = { ...state.data[symbol], getAssets: createInitialState(), }; } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/suite-desktop-core/src/config.ts
(1 hunks)packages/suite/src/hooks/wallet/useSolanaRewards.ts
(1 hunks)packages/suite/src/support/messages.ts
(2 hunks)packages/suite/src/views/wallet/staking/components/SolStakingDashboard/SolStakingDashboard.tsx
(2 hunks)packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsEmpty.tsx
(1 hunks)packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx
(1 hunks)suite-common/wallet-core/src/stake/stakeConstants.ts
(1 hunks)suite-common/wallet-core/src/stake/stakeReducer.ts
(4 hunks)suite-common/wallet-core/src/stake/stakeSelectors.ts
(1 hunks)suite-common/wallet-core/src/stake/stakeThunks.ts
(2 hunks)suite-common/wallet-core/src/stake/stakeTypes.ts
(2 hunks)
🔇 Additional comments (7)
packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsEmpty.tsx (1)
1-18
: LGTM!The component is well-structured, follows React best practices, and properly handles translations.
suite-common/wallet-core/src/stake/stakeSelectors.ts (1)
51-57
: LGTM!The selector follows Redux best practices and maintains consistency with other selectors in the file.
suite-common/wallet-core/src/stake/stakeTypes.ts (1)
28-30
: LGTM!The enum is well-defined and follows TypeScript naming conventions.
suite-common/wallet-core/src/stake/stakeThunks.ts (1)
124-126
: Verify if POST method without body is intended.The endpoint is called with POST method but no request body is provided. This is unusual and might indicate a mistake.
✅ Verification successful
🌐 Web query:
What is the standard HTTP method for retrieving data without modifying server state?
Length of output: 4788
Action Required: Assess the Use of POST for Data Retrieval
The web query confirms that for purely read operations, the standard and recommended HTTP method is GET. Using a POST request without a request body is unusual for data retrieval. Please verify with backend requirements whether a POST is truly necessary here or if altering the method to GET—or adding a request body if POST is required for specific reasons—would be more appropriate.
- File: suite-common/wallet-core/src/stake/stakeThunks.ts (Lines 124-126)
- Observation: A POST request is used without a request body for fetching rewards.
- Recommendation: Confirm if this behavior is intentional. If the endpoint is meant to only retrieve data, consider switching to GET to align with standard HTTP practices.
packages/suite/src/views/wallet/staking/components/SolStakingDashboard/SolStakingDashboard.tsx (1)
69-69
: LGTM!The RewardsList component is well-integrated into the dashboard layout.
packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx (1)
70-83
: Consider removing the refresh button.Since rewards are updated once every three days, a manual refresh button might not provide much value and could potentially confuse users about when new data is available.
- actions={ - <Column> - <Tooltip - maxWidth={250} - content={<Translation id="TR_STAKE_REFRESH_REWARDS_TOOLTIP" />} - > - <IconButton - icon="arrowClockwise" - variant="tertiary" - size="small" - onClick={() => fetchRewards(account)} - /> - </Tooltip> - </Column> - }packages/suite/src/support/messages.ts (1)
5213-5220
: LGTM: Message keys for Solana staking rewardsThe added message keys provide good coverage for the staking rewards functionality, including tooltips, empty states, and wait time messages. The translations are clear and informative.
Also applies to: 8670-8687
961a180
to
12227d0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx (1)
55-60
: Consider adding smooth scrolling behavior.The current scroll implementation could be enhanced for better user experience.
- sectionRef.current.scrollIntoView(); + sectionRef.current.scrollIntoView({ behavior: 'smooth' });suite-common/wallet-core/src/stake/stakeTypes.ts (1)
97-106
: Convert snake_case to camelCase for consistency with TypeScript conventions.The field names should follow TypeScript naming conventions.
export type StakeAccountRewards = { height: number; epoch: number; validator: string; authority: string; - stake_account: string; + stakeAccount: string; amount: string; currency: string; time: string; };suite-common/wallet-core/src/stake/stakeReducer.ts (2)
133-145
: Optimize state update logic.The state update can be more concise using object spread operator.
- if (!state.data[symbol]?.[endpointType]) { - state.data[symbol] = { - ...state.data[symbol], - getAssets: { - error: false, - isLoading: true, - lastSuccessfulFetchTimestamp: 0 as Timestamp, - data: {}, - }, - }; - } + state.data[symbol] = { + ...state.data[symbol], + [endpointType]: state.data[symbol]?.[endpointType] ?? { + error: false, + isLoading: true, + lastSuccessfulFetchTimestamp: 0 as Timestamp, + data: {}, + }, + };
175-218
: Reduce code duplication in reducer cases.The reducer cases for different endpoints follow the same pattern. Consider extracting common logic into a helper function.
+const updateEndpointState = ( + state: StakeState, + symbol: NetworkSymbol, + endpointType: string, + update: { + error: boolean; + isLoading: boolean; + lastSuccessfulFetchTimestamp: Timestamp; + data: any; + } +) => { + if (!state.data[symbol]) { + state.data[symbol] = {}; + } + state.data[symbol] = { + ...state.data[symbol], + [endpointType]: update, + }; +}; // Use in reducer cases: -.addCase(fetchEverstakeRewards.pending, (state, action) => { - const { symbol, endpointType } = action.meta.arg; - - if (!state.data[symbol]?.[endpointType]) { - state.data[symbol] = { - ...state.data[symbol], - stakingRewards: { - error: false, - isLoading: true, - lastSuccessfulFetchTimestamp: 0 as Timestamp, - data: {}, - }, - }; - } -}) +.addCase(fetchEverstakeRewards.pending, (state, action) => { + const { symbol, endpointType } = action.meta.arg; + updateEndpointState(state, symbol, endpointType, { + error: false, + isLoading: true, + lastSuccessfulFetchTimestamp: 0 as Timestamp, + data: {}, + }); +})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/suite-desktop-core/src/config.ts
(1 hunks)packages/suite/src/hooks/wallet/useSolanaRewards.ts
(1 hunks)packages/suite/src/support/messages.ts
(2 hunks)packages/suite/src/views/wallet/staking/components/SolStakingDashboard/SolStakingDashboard.tsx
(2 hunks)packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsEmpty.tsx
(1 hunks)packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx
(1 hunks)suite-common/wallet-core/src/stake/stakeConstants.ts
(1 hunks)suite-common/wallet-core/src/stake/stakeReducer.ts
(4 hunks)suite-common/wallet-core/src/stake/stakeSelectors.ts
(1 hunks)suite-common/wallet-core/src/stake/stakeThunks.ts
(2 hunks)suite-common/wallet-core/src/stake/stakeTypes.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- suite-common/wallet-core/src/stake/stakeConstants.ts
- packages/suite/src/views/wallet/staking/components/SolStakingDashboard/SolStakingDashboard.tsx
- packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsEmpty.tsx
- packages/suite/src/hooks/wallet/useSolanaRewards.ts
- packages/suite-desktop-core/src/config.ts
- suite-common/wallet-core/src/stake/stakeSelectors.ts
- packages/suite/src/support/messages.ts
🔇 Additional comments (8)
packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx (4)
1-36
: LGTM! Well-organized imports and clean type definitions.The imports are logically grouped and the props interface is properly typed.
38-52
: LGTM! Clean hook usage and state management.The component properly utilizes the custom hook and follows React best practices.
70-83
: Remove the refresh button.As discussed in previous reviews, since rewards are updated once every three days, this refresh button is unnecessary.
87-168
: LGTM! Well-structured reward list implementation.The implementation handles loading states properly and presents rewards in a clear, organized manner.
suite-common/wallet-core/src/stake/stakeTypes.ts (2)
28-30
: LGTM!The new endpoint type follows the existing pattern and is well-defined.
108-110
: LGTM!The type provides a clear structure for mapping addresses to their rewards.
suite-common/wallet-core/src/stake/stakeThunks.ts (1)
110-143
: 🛠️ Refactor suggestionEnhance error handling and API request implementation.
Several improvements are needed for robustness:
- POST request should include appropriate headers and body
- Error response should be parsed for detailed error messages
- Consider implementing retry mechanism for transient failures
- Consider rate limiting to prevent API abuse
export const fetchEverstakeRewards = createThunk< { rewards: StakeRewardsByAccount }, { symbol: SupportedSolanaNetworkSymbols; endpointType: EverstakeRewardsEndpointType; address: string; }, { rejectValue: string } >( `${STAKE_MODULE}/fetchEverstakeRewardsData`, async (params, { fulfillWithValue, rejectWithValue }) => { const { address } = params; try { const response = await fetch(`${EVERSTAKE_REWARDS_SOLANA_ENPOINT}/${address}`, { method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ address }), }); if (!response.ok) { - throw Error(response.statusText); + const errorData = await response.json().catch(() => ({ message: response.statusText })); + throw Error(errorData.message || response.statusText); } const data = await response.json(); return fulfillWithValue({ rewards: { [address]: data, }, }); } catch (error) { - return rejectWithValue(error.toString()); + return rejectWithValue(error instanceof Error ? error.message : error.toString()); } }, );suite-common/wallet-core/src/stake/stakeReducer.ts (1)
39-44
: LGTM!The new state structure follows the existing pattern and is well-defined.
...e/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx
Outdated
Show resolved
Hide resolved
98018c9
to
493fb5c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
suite-common/wallet-core/src/stake/stakeTypes.ts (1)
97-106
: Consider adding JSDoc comments to document the type.The
StakeAccountRewards
type would benefit from documentation explaining each field's purpose and format.+/** + * Represents a reward entry for a staking account. + * @property height - The block height at which the reward was earned. + * @property epoch - The epoch number during which the reward was earned. + * @property validator - The validator's address. + * @property authority - The authority's address. + * @property stake_account - The staking account's address. + * @property amount - The reward amount in the smallest unit of the currency. + * @property currency - The currency symbol. + * @property time - The timestamp in ISO 8601 format. + */ export type StakeAccountRewards = { height: number; epoch: number; validator: string; authority: string; stake_account: string; amount: string; currency: string; time: string; };packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx (1)
28-50
: Consider memoizing the callback function.The
onPageSelected
function is recreated on every render. Consider usinguseCallback
to optimize performance.- const onPageSelected = (page: number) => { + const onPageSelected = useCallback((page: number) => { setSelectedPage(page); if (sectionRef.current) { sectionRef.current.scrollIntoView(); } - }; + }, [setSelectedPage]);suite-common/wallet-core/src/stake/stakeThunks.ts (1)
110-143
: Consider adding request body validation and error details.The thunk could benefit from:
- Request body validation to ensure the address is valid.
- More detailed error handling to provide specific error messages.
export const fetchEverstakeRewards = createThunk< { rewards: StakeRewardsByAccount }, { symbol: SupportedSolanaNetworkSymbols; endpointType: EverstakeRewardsEndpointType; address: string; }, { rejectValue: string } >( `${STAKE_MODULE}/fetchEverstakeRewardsData`, async (params, { fulfillWithValue, rejectWithValue }) => { const { address } = params; + + if (!address || typeof address !== 'string' || address.length === 0) { + return rejectWithValue('Invalid address provided'); + } try { const response = await fetch(`${EVERSTAKE_REWARDS_SOLANA_ENPOINT}/${address}`, { method: 'POST', }); if (!response.ok) { - throw Error(response.statusText); + const errorData = await response.json().catch(() => ({})); + throw Error( + `Failed to fetch rewards: ${response.status} ${response.statusText}${ + errorData.message ? ` - ${errorData.message}` : '' + }`, + ); } const data = await response.json(); return fulfillWithValue({ rewards: { [address]: data, }, }); } catch (error) { return rejectWithValue(error.toString()); } }, );suite-common/wallet-core/src/stake/stakeReducer.ts (1)
175-189
: Consider consolidating duplicate reducer logic.The reducer cases for
fetchEverstakeRewards
follow the same pattern as other fetch actions. Consider extracting the common logic into a reusable helper function.+const handleFetchState = <T>( + state: StakeState, + symbol: NetworkSymbol, + endpointType: string, + status: 'pending' | 'fulfilled' | 'rejected', + payload?: T, +) => { + if (!state.data[symbol]?.[endpointType]) { + state.data[symbol] = { + ...state.data[symbol], + [endpointType]: { + error: status === 'rejected', + isLoading: status === 'pending', + lastSuccessfulFetchTimestamp: status === 'fulfilled' ? Date.now() as Timestamp : 0, + data: status === 'fulfilled' ? payload : {}, + }, + }; + } +}; builder // ... other cases ... .addCase(fetchEverstakeRewards.pending, (state, action) => { const { symbol, endpointType } = action.meta.arg; - if (!state.data[symbol]?.[endpointType]) { - state.data[symbol] = { - ...state.data[symbol], - stakingRewards: { - error: false, - isLoading: true, - lastSuccessfulFetchTimestamp: 0 as Timestamp, - data: {}, - }, - }; - } + handleFetchState(state, symbol, endpointType, 'pending'); }) .addCase(fetchEverstakeRewards.fulfilled, (state, action) => { const { symbol, endpointType } = action.meta.arg; - const data = state.data[symbol]; - if (data?.[endpointType]) { - data[endpointType] = { - error: false, - isLoading: false, - lastSuccessfulFetchTimestamp: Date.now() as Timestamp, - data: action.payload, - }; - } + handleFetchState(state, symbol, endpointType, 'fulfilled', action.payload); }) .addCase(fetchEverstakeRewards.rejected, (state, action) => { const { symbol, endpointType } = action.meta.arg; - const data = state.data[symbol]; - if (data?.[endpointType]) { - data[endpointType] = { - error: true, - isLoading: false, - lastSuccessfulFetchTimestamp: 0 as Timestamp, - data: {}, - }; - } + handleFetchState(state, symbol, endpointType, 'rejected'); });Also applies to: 190-204, 205-218
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/suite/src/hooks/wallet/useSolanaRewards.ts
(1 hunks)packages/suite/src/support/messages.ts
(2 hunks)packages/suite/src/views/wallet/staking/components/SolStakingDashboard/SolStakingDashboard.tsx
(2 hunks)packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsEmpty.tsx
(1 hunks)packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx
(1 hunks)suite-common/wallet-core/src/stake/stakeReducer.ts
(4 hunks)suite-common/wallet-core/src/stake/stakeThunks.ts
(2 hunks)suite-common/wallet-core/src/stake/stakeTypes.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/suite/src/views/wallet/staking/components/SolStakingDashboard/SolStakingDashboard.tsx
- packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsEmpty.tsx
- packages/suite/src/hooks/wallet/useSolanaRewards.ts
- packages/suite/src/support/messages.ts
🔇 Additional comments (7)
suite-common/wallet-core/src/stake/stakeTypes.ts (2)
28-30
: LGTM!The enum is well-defined and follows the pattern established by other endpoint type enums in the file.
108-110
: LGTM!The type is well-defined and provides a clear mapping between addresses and their rewards.
packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx (4)
24-26
: LGTM!The props interface is well-defined and follows TypeScript best practices.
51-53
: LGTM!The early return pattern with the empty state is well-implemented.
61-67
: LGTM!The loading state with skeleton placeholders provides a good user experience.
144-153
: LGTM!The pagination implementation follows the previous review feedback by checking
isLoading
state.suite-common/wallet-core/src/stake/stakeReducer.ts (1)
39-44
: LGTM!The state interface is well-defined and follows the pattern established by other data types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/suite/src/hooks/wallet/useSolanaRewards.ts
(1 hunks)suite-common/wallet-core/src/stake/stakeThunks.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/suite/src/hooks/wallet/useSolanaRewards.ts
🔇 Additional comments (3)
suite-common/wallet-core/src/stake/stakeThunks.ts (3)
14-14
: LGTM!The new imports and types are properly organized and follow the project's conventions.
Also applies to: 21-22
110-119
: Verify the necessity of thesymbol
parameter.The
symbol
parameter is accepted but not used in the function. If it's not needed, consider removing it to avoid confusion.
147-195
: Verify periodic refresh requirements for rewards data.The new rewards thunk is not included in the periodic check mechanism. Please verify if this is intentional or if rewards data should be periodically refreshed like other staking data.
If periodic refresh is needed, consider updating the
initStakeDataThunk
to include rewards data:const solPromises = createPromises( solanaBasedNetworksWithStaking, EverstakeAssetEndpointType, ); +// Add rewards promises if periodic refresh is needed +const rewardsPromises = solanaBasedNetworksWithStaking.map(symbol => { + // Get addresses for the network + const addresses = selectAddressesForNetwork(getState(), symbol); + return addresses.map(address => + dispatch(fetchEverstakeRewards({ symbol, endpointType: 'rewards', address })) + ); +}).flat(); -return Promise.all([...ethPromises, ...solPromises]); +return Promise.all([...ethPromises, ...solPromises, ...rewardsPromises]);
try { | ||
const response = await fetch(`${EVERSTAKE_REWARDS_SOLANA_ENPOINT}/${address}`, { | ||
method: 'POST', | ||
signal, | ||
}); | ||
|
||
if (!response.ok) { | ||
throw Error(response.statusText); | ||
} | ||
|
||
const data = await response.json(); | ||
|
||
return fulfillWithValue({ | ||
rewards: { | ||
[address]: data, | ||
}, | ||
}); | ||
} catch (error) { | ||
return rejectWithValue(error.toString()); | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance error handling and request validation.
Several improvements can be made to the implementation:
- The POST request is missing a request body despite using POST method.
- The response data structure is not validated.
- Error handling could be more specific to help with debugging.
Consider applying these improvements:
try {
const response = await fetch(`${EVERSTAKE_REWARDS_SOLANA_ENPOINT}/${address}`, {
method: 'POST',
+ headers: {
+ 'Content-Type': 'application/json',
+ },
+ body: JSON.stringify({}), // Add empty body or required parameters
signal,
});
if (!response.ok) {
- throw Error(response.statusText);
+ throw new Error(`Failed to fetch rewards: ${response.status} ${response.statusText}`);
}
const data = await response.json();
+
+ // Validate response data structure
+ if (!Array.isArray(data)) {
+ throw new Error('Invalid response: rewards data should be an array');
+ }
return fulfillWithValue({
rewards: {
[address]: data,
},
});
} catch (error) {
- return rejectWithValue(error.toString());
+ return rejectWithValue(
+ error instanceof Error ? error.message : 'Unknown error occurred'
+ );
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
try { | |
const response = await fetch(`${EVERSTAKE_REWARDS_SOLANA_ENPOINT}/${address}`, { | |
method: 'POST', | |
signal, | |
}); | |
if (!response.ok) { | |
throw Error(response.statusText); | |
} | |
const data = await response.json(); | |
return fulfillWithValue({ | |
rewards: { | |
[address]: data, | |
}, | |
}); | |
} catch (error) { | |
return rejectWithValue(error.toString()); | |
} | |
}, | |
try { | |
const response = await fetch(`${EVERSTAKE_REWARDS_SOLANA_ENPOINT}/${address}`, { | |
method: 'POST', | |
headers: { | |
'Content-Type': 'application/json', | |
}, | |
body: JSON.stringify({}), // Add empty body or required parameters | |
signal, | |
}); | |
if (!response.ok) { | |
throw new Error(`Failed to fetch rewards: ${response.status} ${response.statusText}`); | |
} | |
const data = await response.json(); | |
// Validate response data structure | |
if (!Array.isArray(data)) { | |
throw new Error('Invalid response: rewards data should be an array'); | |
} | |
return fulfillWithValue({ | |
rewards: { | |
[address]: data, | |
}, | |
}); | |
} catch (error) { | |
return rejectWithValue( | |
error instanceof Error ? error.message : 'Unknown error occurred' | |
); | |
} | |
}, |
7072660
to
90a7579
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
suite-common/wallet-core/src/stake/stakeReducer.ts (1)
175-218
: Improve type safety and endpoint type handling.The reducer cases for staking rewards could be improved:
- Make the endpoint type check more specific
- Add type safety to state updates
Consider these improvements:
.addCase(fetchEverstakeRewards.pending, (state, action) => { const { symbol, endpointType } = action.meta.arg; - if (!state.data[symbol]?.[endpointType]) { + if (!state.data[symbol]?.stakingRewards) { state.data[symbol] = { ...state.data[symbol], stakingRewards: { error: false, isLoading: true, lastSuccessfulFetchTimestamp: 0 as Timestamp, data: {}, }, }; } }) .addCase(fetchEverstakeRewards.fulfilled, (state, action) => { const { symbol, endpointType } = action.meta.arg; const data = state.data[symbol]; - if (data?.[endpointType]) { + if (data?.stakingRewards) { - data[endpointType] = { + data.stakingRewards = { error: false, isLoading: false, lastSuccessfulFetchTimestamp: Date.now() as Timestamp, data: action.payload, }; } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/suite-desktop-core/src/config.ts
(1 hunks)packages/suite/src/hooks/wallet/useSolanaRewards.ts
(1 hunks)packages/suite/src/support/messages.ts
(2 hunks)packages/suite/src/views/wallet/staking/components/SolStakingDashboard/SolStakingDashboard.tsx
(2 hunks)packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsEmpty.tsx
(1 hunks)packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx
(1 hunks)suite-common/wallet-core/src/stake/stakeConstants.ts
(1 hunks)suite-common/wallet-core/src/stake/stakeReducer.ts
(4 hunks)suite-common/wallet-core/src/stake/stakeSelectors.ts
(1 hunks)suite-common/wallet-core/src/stake/stakeThunks.ts
(2 hunks)suite-common/wallet-core/src/stake/stakeTypes.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- suite-common/wallet-core/src/stake/stakeConstants.ts
- packages/suite-desktop-core/src/config.ts
- packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsEmpty.tsx
- packages/suite/src/views/wallet/staking/components/SolStakingDashboard/SolStakingDashboard.tsx
- suite-common/wallet-core/src/stake/stakeSelectors.ts
- suite-common/wallet-core/src/stake/stakeTypes.ts
- packages/suite/src/views/wallet/staking/components/SolStakingDashboard/components/Rewards/RewardsList.tsx
- packages/suite/src/hooks/wallet/useSolanaRewards.ts
- packages/suite/src/support/messages.ts
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: run-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=other, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: prepare_android_test_app
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
🔇 Additional comments (3)
suite-common/wallet-core/src/stake/stakeThunks.ts (2)
14-14
: LGTM!The imports and type declarations are correctly added for the new Solana staking rewards functionality.
Also applies to: 21-22
110-144
: Enhance error handling and request validation.The implementation needs improvements in several areas:
- The POST request is missing a request body
- The response data structure is not validated
- Error handling could be more specific
Apply these improvements:
try { const response = await fetch(`${EVERSTAKE_REWARDS_SOLANA_ENPOINT}/${address}`, { method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify({}), // Add empty body or required parameters signal, }); if (!response.ok) { - throw Error(response.statusText); + throw new Error(`Failed to fetch rewards: ${response.status} ${response.statusText}`); } const data = await response.json(); + + // Validate response data structure + if (!Array.isArray(data)) { + throw new Error('Invalid response: rewards data should be an array'); + } return fulfillWithValue({ rewards: { [address]: data, }, }); } catch (error) { - return rejectWithValue(error.toString()); + return rejectWithValue( + error instanceof Error ? error.message : 'Unknown error occurred' + ); }suite-common/wallet-core/src/stake/stakeReducer.ts (1)
39-44
: LGTM!The state structure for staking rewards follows the established pattern and includes all necessary fields.
Description
This PR adds rewards to the Solana staking dashboard. Key changes:
Related Issue
Resolve #16597
Screenshots: