feat(suite): add Solana message system feature for stake, unstake and claim #17381
Conversation
WalkthroughThe changes update multiple components to pass the current account’s network symbol into the 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ Context from checks skipped due to timeout of 90000ms (7)
🔇 Additional comments (8)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/UnstakeModal/UnstakeEthForm/UnstakeButton.tsx (1)
12-14: Consider adding error handling for missing selected accountWhile the component expects a selected account to be present (as shown by the type assertion), there's no explicit error handling if the account isn't properly loaded. Consider adding a guard clause to handle this edge case.
const selectedAccount = useSelector( state => state.wallet.selectedAccount, ) as SelectedAccountLoaded; + + // Prevent rendering if no account is selected + if (!selectedAccount || selectedAccount.status !== 'loaded') { + return null; + }suite-common/message-system/src/messageSystemTypes.ts (1)
23-34: Improved architecture for multi-network staking feature supportThis restructuring of the
Featureobject from flat properties to a nested object structure provides better organization and scalability for supporting multiple blockchain networks. The new design allows for cleaner feature references by network type.This approach will make it easier to add support for additional networks in the future without modifying the overall structure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/ClaimModal/ClaimModal.tsx(1 hunks)packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/StakeModal/StakeEthForm/StakeButton.tsx(1 hunks)packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/UnstakeModal/UnstakeEthForm/UnstakeButton.tsx(1 hunks)packages/suite/src/hooks/suite/useMessageSystemStaking.ts(1 hunks)packages/suite/src/views/wallet/staking/components/StakingDashboard/components/ClaimCard.tsx(1 hunks)packages/suite/src/views/wallet/staking/components/StakingDashboard/components/EmptyStakingCard.tsx(1 hunks)suite-common/message-system/src/messageSystemTypes.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- 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=passphrase, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: EAS Update
- GitHub Check: run-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: build-web
- GitHub Check: prepare_android_test_app
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: Analyze with CodeQL (javascript)
🔇 Additional comments (17)
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/ClaimModal/ClaimModal.tsx (1)
25-27: Network-aware message system support added - LGTM!The change properly passes the selected account's network symbol to the
useMessageSystemStakinghook, enabling network-specific claiming functionality. This enhancement aligns with the PR objective to add Solana message system support alongside existing Ethereum functionality.packages/suite/src/views/wallet/staking/components/StakingDashboard/components/EmptyStakingCard.tsx (1)
31-31: Network-aware staking message system enabled - LGTM!The update correctly passes the account symbol to the
useMessageSystemStakinghook, enabling network-specific staking messages and controls. The optional chaining (account?.symbol) appropriately handles cases where the account might be undefined.packages/suite/src/views/wallet/staking/components/StakingDashboard/components/ClaimCard.tsx (1)
20-22: Network-aware claiming message system implemented - LGTM!The update correctly passes the selected account's symbol to the
useMessageSystemStakinghook, enabling network-specific claiming functionality. The optional chaining operator is appropriately used to handle potential undefined values.packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/UnstakeModal/UnstakeEthForm/UnstakeButton.tsx (3)
1-1: Type import added for enhanced type safety - LGTM!The
SelectedAccountLoadedtype import ensures proper typing for the selected account, which improves code safety and IDE support.
5-5: Added Redux selector support - LGTM!Importing
useSelectoralong with the existinguseDevicehook enables access to Redux state for retrieving the currently selected account.
12-17: Network-aware unstaking message system implemented - LGTM!These changes properly integrate the unstaking functionality with the network-specific message system:
- Retrieves the selected account from Redux state
- Casts it to the appropriate type for type safety
- Passes the network symbol to the
useMessageSystemStakinghookThis enhancement fulfills the PR objective to add Solana message system features for unstaking operations.
suite-common/message-system/src/messageSystemTypes.ts (2)
43-48: Well-designed type utility for extracting nested feature valuesThe
ExtractFeatureValues<T>type utility effectively handles recursive extraction of values from nested objects, providing strong type safety while accommodating the new structure.This recursive type approach ensures that all feature values, regardless of nesting level, are properly typed and accessible.
50-50: Updated type definition matches the new feature structureThe
FeatureDomaintype now correctly uses theExtractFeatureValuesutility to extract all possible feature values from the restructured object.This ensures type safety is maintained throughout the application when referencing feature domains.
packages/suite/src/hooks/suite/useMessageSystemStaking.ts (5)
12-17: Good type constraints and network availability checksThe
AvailableNetworkSymbolstype properly restricts the network symbols to only supported networks ('eth' and 'sol'). TheavailableNetworksconstant efficiently aggregates all supported networks from the feature definitions.This approach ensures type safety while maintaining a single source of truth for supported networks by deriving them directly from the Feature object keys.
19-19: Enhanced hook flexibility with optional network parameterThe function signature update appropriately makes the network symbol optional, allowing for backward compatibility while adding support for network-specific behavior.
This design choice supports both legacy code that doesn't pass a network symbol and new code that does.
22-28: Robust network availability checking and feature selectionThe implementation correctly handles the case when a network symbol is provided and checks if it's available before attempting to use it, preventing potential runtime errors.
The type casting to
AvailableNetworkSymbolsis appropriate here since you've already validated that the network is included in the available networks list.
30-38: Improved selector implementation with null safetyThe selectors now properly handle the case when a feature might be undefined, returning undefined instead of potentially causing runtime errors.
This defensive programming approach ensures the hook won't crash when used with unsupported networks.
40-48: Consistent null safety pattern across message content selectorsThe message content selectors follow the same pattern as the disabled state selectors, maintaining consistency throughout the implementation.
This consistent approach makes the code more maintainable and predictable.
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/StakeModal/StakeEthForm/StakeButton.tsx (4)
1-1: Added proper type import for selected accountImporting the
SelectedAccountLoadedtype ensures type safety when working with the selected account.
5-5: Added useSelector hook importThe import has been updated to include the
useSelectorhook, which is needed to access the Redux state.
22-24: Passing network symbol to staking hookThe component now correctly passes the selected account's network symbol to the
useMessageSystemStakinghook, enabling network-specific staking message behavior.This ensures that the staking messages are contextually appropriate for the selected network (Ethereum or Solana).
12-14:❓ Verification inconclusive
Verify type assertion safety
The code retrieves the selected account from Redux and asserts it as
SelectedAccountLoaded. This assumes that a selected account is always loaded when this component is rendered.
🏁 Script executed:
#!/bin/bash # Verify if the StakeButton component is only rendered when a selected account is loaded # Check if there are any conditionals to ensure selected account is loaded before rendering this component rg -A 5 -B 5 "StakeButton" --glob "*.tsx" | grep -i "selectedAccount" # Check if there's any parent component that ensures the account is loaded rg "<StakeButton" -A 2 -B 10 --glob "*.tsx" | grep -A 5 -B 5 -i "selectedAccount"Length of output: 602
Action Required: Confirm Runtime Guard for Selected Account
It appears that our automated search did not reveal any explicit conditional checks (in StakeButton or its parent components) ensuring that a selected account is loaded before rendering this component. The code currently casts the retrieved state directly to
SelectedAccountLoadedwithout verifying if the account is indeed available. Please manually verify that:
- Any parent component or execution path guarantees a loaded selected account before rendering
StakeButton.- If such a guarantee is absent, consider introducing runtime checks or conditional rendering to safely handle cases where no account is loaded.
|
🚀 Expo preview is ready!
|
izmy
force-pushed
the
feat/solana-message-system-staking
branch
from
dcac848 to
625185a
Compare
There was a problem hiding this comment.
LGTM tested with
{
"conditions": [],
"message": {
"id": "12345",
"priority": 93,
"dismissible": false,
"variant": "info",
"category": ["feature"],
"content": {
"en-GB": "123",
"en": "123",
"es": "123",
"cs": "123",
"ru": "123",
"ja": "123",
"hu": "123",
"it": "123",
"fr": "123",
"de": "123",
"tr": "123",
"pt": "123",
"uk": "123"
},
"feature": [
{
"domain": "sol.staking.stake",
"flag": false
}
]
}
},
Description
Created features for
sol.staking.stakesol.staking.unstakesol.staking.claimI also rewrote
useMessageSystemStakinghook in a more generic way.Related Issue
Resolve #17321