feat(suite): solana v2 staking support and fee estimation #16800
Conversation
WalkthroughThe changes refactor dependency management and staking functionality across multiple packages. The dependency on ✨ 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 (
|
packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx
Outdated
Show resolved
Hide resolved
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts
Outdated
Show resolved
Hide resolved
packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx
Outdated
Show resolved
Hide resolved
dev-pvl
force-pushed
the
feat/solana-v2-staking-support
branch
2 times, most recently
from
4d36393 to
22e4cd8
Compare
dev-pvl
requested review from
MiroslavProchazka,
marekrjpolak and
mroz22
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/suite/src/utils/suite/solanaStaking.ts (1)
39-61: Refactor 'createTransactionShimCommon' to avoid code duplicationThe function
createTransactionShimCommonincludes a TODO comment about refactoring to eliminate code duplication. Refactoring this function can improve maintainability and reduce redundancy.Do you want me to help refactor this function to avoid code duplication?
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
145-145: Simplify conditional check using optional chainingConsider simplifying the conditional check at line 145 by using optional chaining to improve readability.
Apply this diff to use optional chaining:
- if (estimatedFee && estimatedFee.payload && 'levels' in estimatedFee.payload) { + if (estimatedFee?.payload && 'levels' in estimatedFee.payload) {🧰 Tools
🪛 Biome (1.9.4)
[error] 145-145: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/blockchain-link/src/workers/utils.ts (1)
28-71: Enhance error handling in getSolanaStakingData.The function correctly handles the new epoch parameter and SDK integration. However, consider adding more specific error handling for edge cases.
export const getSolanaStakingData = async ( descriptor: string, isTestnet: boolean, epoch: number, ) => { const blockchainEnvironment = isTestnet ? 'devnet' : 'mainnet'; // Find the blockchain configuration for the specified chain and environment const blockchainConfig = config.find(c => c.blockchain.name.toLowerCase().includes(`solana ${blockchainEnvironment}`), ); + if (!blockchainConfig) { + throw new Error(`No configuration found for Solana ${blockchainEnvironment}`); + } const serverUrl = blockchainConfig?.blockchain.server[0]; + if (!serverUrl) { + throw new Error('No server URL configured'); + } const network = isTestnet ? Network.Devnet : Network.Mainnet; const solanaClient = new Solana(network, serverUrl);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
package.json(1 hunks)packages/blockchain-link-types/package.json(0 hunks)packages/blockchain-link-types/src/solana.ts(1 hunks)packages/blockchain-link/package.json(1 hunks)packages/blockchain-link/src/workers/solana/index.ts(1 hunks)packages/blockchain-link/src/workers/utils.ts(3 hunks)packages/suite/package.json(0 hunks)packages/suite/src/actions/wallet/stake/stakeFormActions.ts(3 hunks)packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts(8 hunks)packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx(1 hunks)packages/suite/src/support/messages.ts(1 hunks)packages/suite/src/utils/suite/solanaStaking.ts(5 hunks)packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx(2 hunks)suite-common/wallet-constants/src/solanaStakingConstants.ts(1 hunks)suite-common/wallet-utils/package.json(1 hunks)suite-common/wallet-utils/src/solanaStakingUtils.ts(5 hunks)
💤 Files with no reviewable changes (2)
- packages/blockchain-link-types/package.json
- packages/suite/package.json
✅ Files skipped from review due to trivial changes (1)
- packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts
🧰 Additional context used
🪛 Biome (1.9.4)
suite-common/wallet-utils/src/solanaStakingUtils.ts
[error] 138-138: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts
[error] 145-145: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (16)
packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx (1)
39-42: LGTM! Translation key change improves clarity.The change from using a tilde character to a more descriptive translation key
TR_STAKE_APPROXIMATE_DAYSimproves clarity for users.packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx (2)
150-150: LGTM! Network type check follows best practices.The addition of
isEthereumNetworkvariable improves code readability and follows the suggested implementation from past review comments.
228-238: LGTM! Network-specific translation keys improve clarity.The conditional selection of translation keys based on network type provides more accurate messaging for users. The parentheses formatting also improves readability.
packages/suite/src/support/messages.ts (1)
8844-8847: LGTM! New message entry follows i18n best practices.The new message
TR_STAKE_APPROXIMATE_DAYScorrectly implements plural formatting and uses a consistent pattern for approximate day values.packages/suite/src/utils/suite/solanaStaking.ts (1)
125-125:⚠️ Potential issueFix the variable name typo caused by a non-ASCII character
In line 125, the property name
сomputeUnitPriceuses a non-ASCII character for 'c' (Cyrillic\u0441) instead of the standard ASCII 'c'. This can lead to unexpected behavior and hard-to-find bugs.Apply this diff to correct the variable name:
- сomputeUnitPrice: BigInt(estimatedFee.feePerUnit), + computeUnitPrice: BigInt(estimatedFee.feePerUnit),Consider running the following script to check for other instances of non-ASCII characters in identifiers:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Find occurrences of non-ASCII characters in the codebase. grep --color='auto' -nP '[^\x00-\x7F]' -r ./packages/suite/src/utils/suite/Length of output: 1154
Non-ASCII Character Detected in
solanaStaking.tsWe confirmed that the property name on line 125 in
packages/suite/src/utils/suite/solanaStaking.tsuses a non-ASCII Cyrillic character (с) instead of the standard ASCIIc. No other occurrences in relevant TypeScript files were found (the additional output is from binary fixture files).
- Action: Replace
сomputeUnitPricewithcomputeUnitPrice.suite-common/wallet-constants/src/solanaStakingConstants.ts (1)
9-10: Confirm that the hardcoded values for SOL compute units are up to datePlease verify that the constants
SOL_COMPUTE_UNIT_PRICEandSOL_COMPUTE_UNIT_LIMITreflect the current values on the Solana network. Hardcoded values may become outdated if network parameters change.packages/blockchain-link-types/src/solana.ts (1)
73-77: LGTM! Verify type compatibility with Solana staking accounts.The new
SolanaStakingAccounttype definition correctly captures the essential fields for Solana staking accounts.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify that all usages of SolanaStakingAccount in the codebase align with the new type definition # Search for all usages of SolanaStakingAccount rg -A 5 "SolanaStakingAccount" --type tsLength of output: 5022
LGTM – The
SolanaStakingAccounttype is consistently used across the codebase and aligns with the required Solana staking account structure.
- The type is defined with required fields (
statusandrentExemptReserve) and an optionalstakefield.- Searches show that all references in
packages/blockchain-link-types/src/solana.ts,packages/blockchain-link-types/src/common.ts,suite-common/wallet-utils/src/solanaStakingUtils.ts, andsuite-common/wallet-types/src/account.tscorrectly import and use the new type.packages/blockchain-link/src/workers/utils.ts (1)
1-1: LGTM! Migration to Solana-specific SDK.The import change correctly switches to the Solana-specific SDK package.
suite-common/wallet-utils/src/solanaStakingUtils.ts (1)
38-45: LGTM! Clear network configuration.The network configuration is correctly updated to use the new SDK's Network enum.
packages/suite/src/actions/wallet/stake/stakeFormActions.ts (1)
109-166: LGTM! Enhanced fee estimation support.The function correctly integrates fee estimation while maintaining backward compatibility through optional parameters.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify that all callers of composeStakingTransaction handle the new optional parameters # Search for all calls to composeStakingTransaction rg "composeStakingTransaction\(" --type tsLength of output: 262
Fee Estimation Integration Verified
The
composeStakingTransactionfunction now correctly integrates fee estimation with optional parameters, and our search confirms that its callers in both Solana and Ethereum actions remain intact and backward compatible.
- Verified calls in
stakeFormSolanaActions.tsandstakeFormEthereumActions.ts.- Optional parameters (
estimatedFee,customFeeLimit) are handled appropriately.packages/blockchain-link/src/workers/solana/index.ts (3)
300-315: LGTM! Well-implemented epoch caching mechanism.The
getEpochfunction implementation is robust with:
- Proper cache checking
- Deferred promise to handle parallel requests
- Reasonable cache duration (1 hour)
369-374: LGTM! Epoch information is now properly integrated with staking data.The changes correctly fetch and pass the epoch information to
getSolanaStakingData.
375-380: LGTM! Misc object construction is properly updated.The
miscobject now includes bothsolStakingAccountsandsolEpochfields.suite-common/wallet-utils/package.json (1)
15-15: Verify compatibility with the new Solana SDK version.The addition of
@everstake/wallet-sdk-solanav2.0.5 aligns with the PR objectives for Solana v2 staking support.packages/blockchain-link/package.json (1)
75-75: LGTM! Consistent dependency version across packages.The addition of
@everstake/wallet-sdk-solanav2.0.5 matches the version used insuite-common/wallet-utils/package.json.package.json (1)
103-103: LGTM! Resolution cleanup aligns with SDK transition.The removal of
@everstake/wallet-sdk/@solana/web3.jsresolution is appropriate as the project now uses@everstake/wallet-sdk-solana.
|
|
||
| const serializedVersiondeTx = txData.tx.versionedTx.serialize(); | ||
| const signedSerializedTx = Buffer.from(serializedVersiondeTx).toString('hex'); | ||
| txData.tx.txShim.addSignature(address(account.descriptor), signedTx.payload.signature); |
There was a problem hiding this comment.
Verify the use of the 'address' function on 'account.descriptor'
At line 264, address(account.descriptor) is used to add the signature. Since the address import may be invalid or incorrect, ensure that this function correctly processes account.descriptor. If you're aiming to convert the account descriptor into a public key string, consider using new PublicKey(account.descriptor).toBase58() instead.
Apply this diff to correct the function usage:
- txData.tx.txShim.addSignature(address(account.descriptor), signedTx.payload.signature);
+ txData.tx.txShim.addSignature(new PublicKey(account.descriptor).toBase58(), signedTx.payload.signature);📝 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.
| txData.tx.txShim.addSignature(address(account.descriptor), signedTx.payload.signature); | |
| txData.tx.txShim.addSignature(new PublicKey(account.descriptor).toBase58(), signedTx.payload.signature); |
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring: Next stepsTake a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with |
dev-pvl
force-pushed
the
feat/solana-v2-staking-support
branch
from
22e4cd8 to
c19f5e0
Compare
|
@tomasklim conflicts are resolved. Feel free to take a look |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (2)
77-126: Consider refactoring to reduce parameter duplication.The prepare functions share common parameters. Consider extracting a base configuration object to reduce duplication.
+type BaseTxConfig = { + from: string; + path: string; + symbol: NetworkSymbol; + selectedBlockchain: any; + estimatedFee?: Fee[number]; +}; const getTransactionData = async ( formValues: StakeFormState, selectedAccount: SelectedAccountStatus, blockchain: BlockchainNetworks, estimatedFee?: Fee[number], ) => { // ... existing validation code ... + const baseConfig: BaseTxConfig = { + from: account.descriptor, + path: account.path, + symbol: account.symbol, + selectedBlockchain, + estimatedFee, + }; let txData; if (stakeType === 'stake') { - txData = await prepareStakeSolTx({ - from: account.descriptor, - path: account.path, - amount: formValues.outputs[0].amount, - symbol: account.symbol, - selectedBlockchain, - estimatedFee, - }); + txData = await prepareStakeSolTx({ + ...baseConfig, + amount: formValues.outputs[0].amount, + }); } // Similar changes for unstake and claim
128-152: Enhance code safety with optional chaining.The function can be made more concise and safer using optional chaining.
- if (estimatedFee && estimatedFee.payload && 'levels' in estimatedFee.payload) { + if (estimatedFee?.payload && 'levels' in estimatedFee.payload) { const { levels } = estimatedFee.payload; return levels[0]; }🧰 Tools
🪛 Biome (1.9.4)
[error] 145-145: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
package.json(1 hunks)packages/blockchain-link-types/package.json(0 hunks)packages/blockchain-link-types/src/solana.ts(1 hunks)packages/blockchain-link/package.json(1 hunks)packages/blockchain-link/src/workers/solana/index.ts(1 hunks)packages/blockchain-link/src/workers/utils.ts(3 hunks)packages/suite/package.json(0 hunks)packages/suite/src/actions/wallet/stake/stakeFormActions.ts(3 hunks)packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts(8 hunks)packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- packages/blockchain-link-types/package.json
- packages/suite/package.json
🚧 Files skipped from review as they are similar to previous changes (6)
- package.json
- packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts
- packages/blockchain-link/package.json
- packages/blockchain-link-types/src/solana.ts
- packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx
- packages/suite/src/actions/wallet/stake/stakeFormActions.ts
🧰 Additional context used
🧠 Learnings (1)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (2)
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.421Z
Learning: In @solana/web3.js v2, 'address' is the correct import to use for handling Solana addresses, replacing the previously used PublicKey class from v1.
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.421Z
Learning: @solana/web3.js v2 has moved to a modular architecture where functionality is split into separate packages like @solana/addresses, @solana/codecs, @solana/rpc, etc., replacing the monolithic imports from v1.
🪛 Biome (1.9.4)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts
[error] 145-145: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (7)
packages/blockchain-link/src/workers/utils.ts (2)
1-1: LGTM! SDK import updated for Solana v2 support.The import statement has been correctly updated to use the Solana-specific SDK package and includes the necessary utilities for stake account handling.
28-70: LGTM! Enhanced staking data retrieval with epoch support.The function has been improved to:
- Include epoch information for accurate stake state determination
- Filter out undefined accounts
- Extract relevant staking data (rentExemptReserve, stake, status)
packages/blockchain-link/src/workers/solana/index.ts (2)
300-315: LGTM! Efficient epoch caching implementation.The
getEpochfunction is well-implemented with:
- Smart caching to avoid redundant RPC calls
- Proper handling of parallel requests using deferred promise
- Reasonable cache duration (1 hour)
369-380: LGTM! Staking data retrieval enhanced with epoch information.The code correctly:
- Fetches the epoch using the cached getter
- Passes epoch to
getSolanaStakingData- Updates the misc object with both epoch and staking accounts
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (3)
38-75: LGTM! Well-implemented fee estimation logic.The function elegantly handles dynamic fee estimation while maintaining backward compatibility with a fallback fee.
154-184: LGTM! Well-implemented fee estimation workflow.The function efficiently handles fee estimation with proper error handling and optimization (skipping estimation for empty input).
186-267: LGTM! Correctly implements Solana v2 transaction signing.The function properly uses the new v2 transaction signing workflow with
txShim.serializeMessage()and theaddressfunction.
| @@ -6,5 +6,7 @@ export const MAX_SOL_AMOUNT_FOR_STAKING = new BigNumber(10_000_000); | |||
| export const MIN_SOL_FOR_WITHDRAWALS = new BigNumber(0.000005); | |||
| export const MIN_SOL_BALANCE_FOR_STAKING = MIN_SOL_AMOUNT_FOR_STAKING.plus(MIN_SOL_FOR_WITHDRAWALS); | |||
| export const SOL_STAKING_OPERATION_FEE = new BigNumber(70_000); // 0.00007 SOL | |||
| export const SOL_COMPUTE_UNIT_PRICE = 100000; | |||
| export const SOL_COMPUTE_UNIT_LIMIT = 200000; | |||
There was a problem hiding this comment.
is it really necessary to use base limit of 200k units? It seems that around 2k is needed for creating stake account and the other (deactivate/withdraw) use even less units.
What do you think about using staking specific default constants for dummy with reasonable amounts for price and limit?
There was a problem hiding this comment.
these values are taken from solanaUtils in the connect package, where we have similar defaults.
export const dummyPriorityFeesForFeeEstimation: PriorityFees = {
computeUnitPrice: '100000',
computeUnitLimit: '200000',
};
Regarding the base limit of 200k units, I see your point. We can try smth staking specific
There was a problem hiding this comment.
I know it is similar to base there and we should change it there as well. We are actually simulate txn to get CU needed for transfers and it results in much better fees for user and also better landing of txns.
I just suggest to make CU limit lower. This is definitely not a blocker.
|
The SDK with it's dependency on Solana Web3.js is still in the dependency tree of |
ETH_NETWORK_ADDRESSES, which is used in As a solution, we can modify @tomasklim wdyt? |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/blockchain-link-utils/src/blockbook.ts (1)
91-100: Consider making staking addresses configurable and add documentation.The hardcoded Ethereum staking addresses could be difficult to maintain. Consider:
- Moving these addresses to a configuration file.
- Adding documentation explaining the purpose of each address.
- Adding validation to ensure addresses are in the correct format.
Here's a suggested refactor:
+import { isValidEthereumAddress } from './validation'; + +interface EthereumStakingAddresses { + poolInstance: string[]; + withdrawTreasury: string[]; +} + +/** + * Ethereum staking contract addresses + * - poolInstance: Addresses of staking pool contracts + * - withdrawTreasury: Addresses of withdrawal treasury contracts + */ +const validateStakingAddresses = (addresses: EthereumStakingAddresses) => { + const allAddresses = [...addresses.poolInstance, ...addresses.withdrawTreasury]; + if (!allAddresses.every(isValidEthereumAddress)) { + throw new Error('Invalid Ethereum address format'); + } + return addresses; +}; + -const ethereumStakingAddresses = { +const ethereumStakingAddresses = validateStakingAddresses({ poolInstance: [ '0xD523794C879d9eC028960a231F866758e405bE34', '0xAFA848357154a6a624686b348303EF9a13F63264', ], withdrawTreasury: [ '0x19449f0f696703Aa3b1485DfA2d855F33659397a', '0x66cb3AeD024740164EBcF04e292dB09b5B63A2e1', ], -}; +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
packages/blockchain-link-utils/package.json(0 hunks)packages/blockchain-link-utils/src/blockbook.ts(1 hunks)packages/blockchain-link/package.json(1 hunks)packages/suite/package.json(1 hunks)suite-common/wallet-constants/src/solanaStakingConstants.ts(1 hunks)suite-common/wallet-utils/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/blockchain-link-utils/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/blockchain-link/package.json
- packages/suite/package.json
- suite-common/wallet-constants/src/solanaStakingConstants.ts
- suite-common/wallet-utils/package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (1)
packages/blockchain-link-utils/src/blockbook.ts (1)
102-106: LGTM! Clean and efficient implementation.The refactored function is more straightforward and easier to understand. The use of
includesmethod provides efficient lookup, and the destructuring makes the code more readable.
tomasklim
force-pushed
the
feat/solana-v2-staking-support
branch
from
47be609 to
3d7b0f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
suite-common/wallet-utils/src/solanaStakingUtils.ts (1)
137-140:⚠️ Potential issueFix O(n²) complexity in reducer.
The spread operator in the reducer's accumulator causes quadratic time complexity. This issue was previously identified and should be fixed.
const balances: Record<StakeStateType, string> = balanceResults.reduce( - (acc, [status, balance]) => ({ ...acc, [status]: balance }), + (acc, [status, balance]) => { + acc[status] = balance; + return acc; + }, {}, );🧰 Tools
🪛 Biome (1.9.4)
[error] 138-138: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
🧹 Nitpick comments (5)
packages/blockchain-link/src/workers/utils.ts (1)
50-71: Enhance error handling for edge cases.The current implementation might return undefined values in the array before filtering. Consider handling edge cases more explicitly:
- Early return if no stake account data
- Explicit type safety for the return value
return stakingAccounts .map(account => { const stakeAccount = account?.data; - if (!stakeAccount) return; + if (!stakeAccount?.state) return undefined; const stakeState = stakeAccountState(stakeAccount, BigInt(epoch)); - const { state } = account?.data ?? {}; - if (!isStake(state)) return; + if (!isStake(stakeAccount.state)) return undefined; - if (state && 'fields' in state) { + if ('fields' in stakeAccount.state) { - const { fields } = state; + const { fields } = stakeAccount.state; + if (!fields?.[0] || !fields?.[1]?.delegation) return undefined; return { rentExemptReserve: fields[0]?.rentExemptReserve, stake: fields[1]?.delegation?.stake, status: stakeState, }; } + return undefined; }) - .filter(account => account !== undefined); + .filter((account): account is NonNullable<typeof account> => account !== undefined);packages/suite/src/utils/suite/solanaStaking.ts (2)
40-61: Consider adding unit tests for the transaction shim creation.The
createTransactionShimCommonfunction is a critical piece that handles transaction signatures and serialization. While the TODO comment acknowledges code duplication, it's important to ensure this function is thoroughly tested.Would you like me to help generate unit tests for this function to ensure its reliability?
141-151: Consider adding error handling for invalid lamports value.The
lamportsconversion should be wrapped in a try-catch block to handle potential invalid number conversions.- const lamports = networkAmountToSmallestUnit(amount, symbol); + let lamports; + try { + lamports = networkAmountToSmallestUnit(amount, symbol); + } catch (error) { + return { + success: false, + errorMessage: 'Invalid amount format', + }; + }packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (2)
128-152: Consider using optional chaining for cleaner code.The fee estimation response handling can be simplified using optional chaining.
- if (estimatedFee && estimatedFee.payload && 'levels' in estimatedFee.payload) { + if (estimatedFee?.payload?.levels) { const { levels } = estimatedFee.payload; return levels[0]; }🧰 Tools
🪛 Biome (1.9.4)
[error] 145-145: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
204-208: Consider adding validation for estimated fee values.The estimated fee object construction should validate the values to ensure they are non-negative.
const estimatedFee = { - feePerTx: transactionInfo.fee, - feeLimit: transactionInfo.feeLimit, - feePerUnit: transactionInfo.feePerByte, + feePerTx: Math.max(0, transactionInfo.fee), + feeLimit: Math.max(0, transactionInfo.feeLimit), + feePerUnit: Math.max(0, transactionInfo.feePerByte), };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (19)
package.json(1 hunks)packages/blockchain-link-types/package.json(0 hunks)packages/blockchain-link-types/src/solana.ts(1 hunks)packages/blockchain-link-utils/package.json(0 hunks)packages/blockchain-link-utils/src/blockbook.ts(1 hunks)packages/blockchain-link/package.json(1 hunks)packages/blockchain-link/src/workers/solana/index.ts(1 hunks)packages/blockchain-link/src/workers/utils.ts(3 hunks)packages/suite/package.json(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormActions.ts(3 hunks)packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts(8 hunks)packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx(1 hunks)packages/suite/src/support/messages.ts(1 hunks)packages/suite/src/utils/suite/solanaStaking.ts(5 hunks)packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx(2 hunks)suite-common/wallet-constants/src/solanaStakingConstants.ts(1 hunks)suite-common/wallet-utils/package.json(1 hunks)suite-common/wallet-utils/src/solanaStakingUtils.ts(5 hunks)
💤 Files with no reviewable changes (2)
- packages/blockchain-link-utils/package.json
- packages/blockchain-link-types/package.json
🚧 Files skipped from review as they are similar to previous changes (13)
- package.json
- packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx
- packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx
- packages/suite/package.json
- packages/blockchain-link/package.json
- packages/blockchain-link-types/src/solana.ts
- packages/blockchain-link-utils/src/blockbook.ts
- packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts
- packages/blockchain-link/src/workers/solana/index.ts
- suite-common/wallet-constants/src/solanaStakingConstants.ts
- suite-common/wallet-utils/package.json
- packages/suite/src/actions/wallet/stake/stakeFormActions.ts
- packages/suite/src/support/messages.ts
🧰 Additional context used
🧠 Learnings (1)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.421Z
Learning: In @solana/web3.js v2, 'address' is the correct import to use for handling Solana addresses, replacing the previously used PublicKey class from v1.
🪛 Biome (1.9.4)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts
[error] 145-145: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
suite-common/wallet-utils/src/solanaStakingUtils.ts
[error] 138-138: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (12)
packages/blockchain-link/src/workers/utils.ts (2)
1-1: LGTM! Import statement aligns with SDK upgrade.The import statement has been updated to use the new Solana-specific SDK package, which aligns with the PR objectives to upgrade the wallet-sdk for Solana v2 support.
28-32: LGTM! Function signature updated to support epoch-based state calculation.The addition of the
epochparameter is necessary for accurate stake state calculation in Solana v2.suite-common/wallet-utils/src/solanaStakingUtils.ts (4)
1-1: LGTM! Import and type updates align with SDK upgrade.The changes correctly update the imports and types to use the new Solana-specific SDK.
Also applies to: 34-36
40-42: LGTM! Network configuration updated correctly.The network configuration has been updated to use the new Network enum while maintaining the same functionality.
56-59: LGTM! Simplified stake value access.The code has been simplified to directly access the stake value while maintaining null safety.
143-149: LGTM! StakeState enum usage updated correctly.The code has been updated to use PascalCase for StakeState enum values, maintaining type safety and improving code consistency.
packages/suite/src/utils/suite/solanaStaking.ts (3)
1-10: LGTM! Imports are correctly updated for Solana Web3.js v2.The imports are properly updated to use the new modular architecture of Solana Web3.js v2, importing specific functions instead of the monolithic imports from v1.
26-34: LGTM! Well-defined transaction shim types.The
SolanaTxandTransactionShimtypes are well-defined with clear method signatures for handling transaction signatures and serialization.
109-128: LGTM! Well-structured fee estimation implementation.The
PriorityFeestype anddummyPriorityFeesForFeeEstimationprovide a robust fallback mechanism for fee estimation. ThegetStakingParamsfunction properly handles both estimated and default fee scenarios.packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (3)
1-1: LGTM! Correct import for Solana Web3.js v2.The
addressimport from@solana/web3.jsis the correct approach in v2, replacing the previously usedPublicKeyclass from v1.
46-48: LGTM! Robust fee estimation with fallback.The implementation properly handles both estimated and default fees, with a clean fallback to
SOL_STAKING_OPERATION_FEEwhen no estimate is available.Also applies to: 65-74
77-126: LGTM! Well-structured transaction data preparation.The
getTransactionDatafunction effectively encapsulates the logic for preparing transaction data based on stake type, with proper type checking and error handling.
tomasklim
force-pushed
the
feat/solana-v2-staking-support
branch
from
3d7b0f5 to
93c37b1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
suite-common/wallet-utils/src/solanaStakingUtils.ts (2)
56-59: Strengthen null checks in stake balance calculation.The current implementation could be more defensive against null values.
- if (account) { - const delegationStake = account?.stake?.toString(); + if (account?.stake) { + const delegationStake = account.stake.toString();
98-101: Simplify optional chaining in account access.The current implementation can be more concise.
- if (account?.networkType !== 'solana') return []; - - const { solStakingAccounts } = account?.misc ?? {}; - if (!solStakingAccounts) return []; + const solStakingAccounts = account?.misc?.solStakingAccounts; + if (account?.networkType !== 'solana' || !solStakingAccounts) return [];packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (2)
77-126: Simplify control flow with early returns.The function could be more concise by combining conditions and using early returns.
- if (selectedAccount.status !== 'loaded') return; - - const { account } = selectedAccount; - if (account.networkType !== 'solana') return; + if (selectedAccount.status !== 'loaded' || selectedAccount.account.networkType !== 'solana') return; + + const { account } = selectedAccount;
145-149: Simplify fee level access with optional chaining.The conditional check can be simplified using optional chaining.
- if (estimatedFee && estimatedFee.payload && 'levels' in estimatedFee.payload) { - const { levels } = estimatedFee.payload; - - return levels[0]; - } + return estimatedFee?.payload?.levels?.[0];🧰 Tools
🪛 Biome (1.9.4)
[error] 145-145: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/suite/src/utils/suite/solanaStaking.ts (2)
36-61: Consider refactoring to avoid code duplication.The
createTransactionShimCommonfunction is duplicated from the connect package. While the implementation is solid, this duplication should be addressed.Consider:
- Moving the function to a shared utility package
- Refactoring the connect package to expose this function
- Creating a new package for common Solana utilities
130-164: Consider refactoring error handling to reduce duplication.The staking functions (
prepareStakeSolTx,prepareUnstakeSolTx,prepareClaimSolTx) have identical error handling blocks. Consider extracting this to a shared utility function.+ const handleStakingError = (e: Error): PrepareStakeSolTxResponse => { + console.error(e); + return { + success: false, + errorMessage: e.message, + }; + }; export const prepareStakeSolTx = async ({...}: PrepareStakeSolTxParams): Promise<PrepareStakeSolTxResponse> => { try { // ... existing implementation } catch (e) { - console.error(e); - return { - success: false, - errorMessage: e.message, - }; + return handleStakingError(e); } };Also applies to: 166-194, 198-224
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
packages/suite/package.json(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormActions.ts(3 hunks)packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts(8 hunks)packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx(1 hunks)packages/suite/src/support/messages.ts(1 hunks)packages/suite/src/utils/suite/solanaStaking.ts(5 hunks)packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx(2 hunks)scripts/list-outdated-dependencies/trends-dependencies.txt(1 hunks)suite-common/wallet-constants/src/solanaStakingConstants.ts(1 hunks)suite-common/wallet-utils/package.json(1 hunks)suite-common/wallet-utils/src/solanaStakingUtils.ts(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- scripts/list-outdated-dependencies/trends-dependencies.txt
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx
- packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx
- packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts
- packages/suite/package.json
- suite-common/wallet-constants/src/solanaStakingConstants.ts
- suite-common/wallet-utils/package.json
- packages/suite/src/actions/wallet/stake/stakeFormActions.ts
- packages/suite/src/support/messages.ts
🧰 Additional context used
🧠 Learnings (1)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.421Z
Learning: In @solana/web3.js v2, 'address' is the correct import to use for handling Solana addresses, replacing the previously used PublicKey class from v1.
🪛 Biome (1.9.4)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts
[error] 145-145: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
suite-common/wallet-utils/src/solanaStakingUtils.ts
[error] 138-138: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (10)
suite-common/wallet-utils/src/solanaStakingUtils.ts (3)
1-1: LGTM! Import and interface changes align with SDK migration.The changes correctly update the imports and interface to use the new SDK types.
Also applies to: 35-35
40-41: LGTM! Network configuration updated correctly.The solNetworks object correctly uses the new Network enum from the SDK.
137-140: Optimize the balances reduction to avoid O(n²) complexity.The spread operator in the reducer accumulator causes quadratic time complexity.
🧰 Tools
🪛 Biome (1.9.4)
[error] 138-138: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (3)
38-75: LGTM! Fee handling improved with dynamic estimation.The function now correctly handles estimated fees with a fallback to constant fee.
154-184: LGTM! Transaction composition with fee estimation.The function correctly handles fee estimation with proper error handling.
186-267: LGTM! Transaction signing updated for SDK v2.The function correctly uses the new serialization methods and address handling from SDK v2.
packages/suite/src/utils/suite/solanaStaking.ts (4)
1-34: LGTM! Clean import organization and type definitions.The updated imports and type definitions align well with the Solana v2 upgrade objectives. The
TransactionShimtype provides a clear interface for transaction handling.
63-83: LGTM! Clean transaction transformation implementation.The updated
transformTxfunction properly handles the new transaction types and compilation process.
103-107: LGTM! Well-implemented type guard.The type guard provides a clean way to ensure type safety when working with transaction messages.
109-128:❓ Verification inconclusive
Verify the priority fee constants.
The priority fee implementation includes fallback values. Please ensure these values are appropriate for the current Solana network conditions.
🌐 Web query:
What are the recommended compute unit price and limit values for Solana transactions?💡 Result:
To optimize Solana transactions, compute unit limits should be set based on actual transaction needs via simulation, while priority fees (compute unit prices) should adjust dynamically to network congestion. Here's a structured breakdown:
Compute Unit Limit Recommendations
Default: 200,000 CU (often inefficient)
Optimal Approach:
- Simulate transactions using
simulateTransactionRPC to measure actual CU consumption ([2][4][5]).- Add a 10% buffer to the simulated value for safety (e.g., 110,000 CU if simulation shows 100,000 CU used) [4][8].
- Avoid exceeding 1.4 million CU/transaction, as blocks reject over-limit transactions [2][5][8].
Transaction Type Typical CU Range Source Simple SOL transfer 20,000–50,000 [3] NFT mint 100,000–300,000 [3][8] Complex DeFi operation 500,000–1M [5][8]
Compute Unit Price Recommendations
Formula:
Priority Fee = Compute Unit Limit × Compute Unit Price([9]).Dynamic Pricing Strategies
- Low congestion: 0.1–1 micro-lamports/CU ([1][6]).
- Moderate congestion: 1–5 micro-lamports/CU ([1][6]).
- High congestion: 5–10+ micro-lamports/CU ([1][6][7]).
Tools for Optimization:
- Use
getRecentPrioritizationFeesRPC for baseline rates ([1][9]).- Leverage QuickNode’s Priority Fee API for real-time data ([1][5]).
- Monitor metrics via Solana Priority Fee Tracker ([1]).
Best Practices
- Automate simulations to set precise CU limits per transaction type ([4][5]).
- Combine with priority fees using SDKs like QuickNode’s
sendSmartTransactionfor hands-off optimization ([1][5]).- Avoid overpaying: Setting excessively high CU limits wastes fees and risks block rejection ([2][8]).
By tailoring compute budgets to transaction complexity and adjusting priority fees based on network demand, developers achieve ~98% success rates even during congestion ([1][5]).
Citations:
- 1: https://blog.quicknode.com/solana-transaction-efficiency-priority-fees/
- 2: https://www.quicknode.com/docs/solana/transactions
- 3: https://developers.metaplex.com/umi/guides/optimal-transactions-with-compute-units-and-priority-fees
- 4: https://solana.com/developers/guides/advanced/how-to-request-optimal-compute
- 5: https://www.quicknode.com/guides/solana-development/transactions/how-to-optimize-solana-transactions
- 6: https://www.quicknode.com/guides/solana-development/transactions/how-to-use-priority-fees
- 7: https://www.helius.dev/blog/priority-fees-understanding-solanas-transaction-fee-mechanics
- 8: https://solana.com/developers/guides/advanced/how-to-optimize-compute
- 9: https://solana.com/docs/core/fees
- 10: https://docs.anza.xyz/proposals/comprehensive-compute-fees
Action: Verify that the fallback constants reflect current optimal network settings
Based on the latest guidance for Solana transactions:
- Compute Unit Limit: Instead of relying on a fixed value, consider using simulation (with a ~10% buffer) for each transaction type, as recommended.
- Compute Unit Price: Since fees should adjust dynamically to network congestion, confirm that the hardcoded fallback value (SOL_COMPUTE_UNIT_PRICE) falls within the appropriate micro-lamports/CU range for your expected network conditions.
Please review and update the fallback constants in
packages/suite/src/utils/suite/solanaStaking.ts(lines 109–128) if necessary to ensure they align with the current best practices.
tomasklim
force-pushed
the
feat/solana-v2-staking-support
branch
2 times, most recently
from
9852d4c to
b3cf69c
Compare
|
@SocketSecurity ignore npm/@everstake/wallet-sdk-solana@2.0.5 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (4)
46-47: Consider using BigNumber for fee comparison and fallback.The fee fallback mechanism could be improved by using BigNumber for consistency and to prevent potential precision issues when comparing fees.
- const feeInLamports = - estimatedFee?.feePerTx ?? new BigNumber(SOL_STAKING_OPERATION_FEE).toString(); + const feeInLamports = new BigNumber(estimatedFee?.feePerTx ?? SOL_STAKING_OPERATION_FEE).toString();
77-127: Consider refactoring common parameters into a shared object.The function has repeated patterns when calling prepare functions. Consider extracting common parameters into a shared object to improve maintainability and reduce duplication.
+ const commonParams = { + from: account.descriptor, + path: account.path, + symbol: account.symbol, + selectedBlockchain, + estimatedFee, + }; + if (stakeType === 'stake') { - txData = await prepareStakeSolTx({ - from: account.descriptor, - path: account.path, - amount: formValues.outputs[0].amount, - symbol: account.symbol, - selectedBlockchain, - estimatedFee, - }); + txData = await prepareStakeSolTx({ + ...commonParams, + amount: formValues.outputs[0].amount, + }); }
146-150: Use optional chaining for cleaner code.The condition check can be simplified using optional chaining.
- if (estimatedFee && estimatedFee.payload && 'levels' in estimatedFee.payload) { - const { levels } = estimatedFee.payload; - - return levels[0]; - } + return estimatedFee?.payload?.levels?.[0];🧰 Tools
🪛 Biome (1.9.4)
[error] 146-146: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
219-224: Enhance error message for better debugging.The error message "Unknown stake action" is too generic. Consider providing more context about which stake action failed.
dispatch( notificationsActions.addToast({ type: 'sign-tx-error', - error: 'Unknown stake action', + error: `Unknown stake action: ${formValues.stakeType}`, }), );packages/suite/src/utils/suite/solanaStaking.ts (1)
36-61: Track the TODO comment to avoid code duplication.While the implementation is correct, the code is duplicated from the connect package. Consider creating a tracking issue to refactor this in the future.
Would you like me to create a tracking issue for refactoring
createTransactionShimCommonto avoid code duplication with the connect package?packages/blockchain-link/src/workers/solana/utils.ts (1)
21-48: Improve type safety and simplify the mapping logic.The current implementation could benefit from:
- Type definition for the returned object structure.
- Simplified mapping logic to reduce cognitive complexity.
Apply this diff to improve the code:
+ interface StakingData { + rentExemptReserve: bigint; + stake: bigint; + status: string; + } + return stakingAccounts - .map(account => { - const stakeAccount = account?.data; - if (!stakeAccount) return; - - const stakeState = stakeAccountState(stakeAccount, BigInt(epoch)); - - const { state } = account?.data ?? {}; - if (!isStake(state)) return; - - if (state && 'fields' in state) { - const { fields } = state; - - return { - rentExemptReserve: fields[0]?.rentExemptReserve, - stake: fields[1]?.delegation?.stake, - status: stakeState, - }; - } - }) - .filter(account => account !== undefined); + .reduce<StakingData[]>((acc, account) => { + const { data: stakeAccount } = account ?? {}; + const { state } = stakeAccount ?? {}; + + if (stakeAccount && isStake(state) && 'fields' in state) { + const { fields } = state; + acc.push({ + rentExemptReserve: fields[0]?.rentExemptReserve ?? BigInt(0), + stake: fields[1]?.delegation?.stake ?? BigInt(0), + status: stakeAccountState(stakeAccount, BigInt(epoch)), + }); + } + return acc; + }, []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
package.json(1 hunks)packages/blockchain-link-types/package.json(0 hunks)packages/blockchain-link-types/src/solana.ts(1 hunks)packages/blockchain-link-utils/package.json(0 hunks)packages/blockchain-link-utils/src/blockbook.ts(1 hunks)packages/blockchain-link/package.json(1 hunks)packages/blockchain-link/src/workers/solana/index.ts(2 hunks)packages/blockchain-link/src/workers/solana/utils.ts(1 hunks)packages/blockchain-link/src/workers/utils.ts(0 hunks)packages/suite/package.json(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormActions.ts(3 hunks)packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts(8 hunks)packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx(1 hunks)packages/suite/src/support/messages.ts(1 hunks)packages/suite/src/utils/suite/solanaStaking.ts(5 hunks)packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx(2 hunks)scripts/list-outdated-dependencies/trends-dependencies.txt(1 hunks)suite-common/wallet-constants/src/solanaStakingConstants.ts(1 hunks)suite-common/wallet-utils/package.json(1 hunks)suite-common/wallet-utils/src/solanaStakingUtils.ts(5 hunks)
💤 Files with no reviewable changes (3)
- packages/blockchain-link-utils/package.json
- packages/blockchain-link-types/package.json
- packages/blockchain-link/src/workers/utils.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- scripts/list-outdated-dependencies/trends-dependencies.txt
- package.json
- packages/blockchain-link-types/src/solana.ts
- packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx
- packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx
- packages/blockchain-link-utils/src/blockbook.ts
- suite-common/wallet-utils/package.json
- packages/blockchain-link/package.json
- packages/suite/package.json
- packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts
- packages/blockchain-link/src/workers/solana/index.ts
- suite-common/wallet-constants/src/solanaStakingConstants.ts
- packages/suite/src/actions/wallet/stake/stakeFormActions.ts
- packages/suite/src/support/messages.ts
🧰 Additional context used
🧠 Learnings (1)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (2)
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.421Z
Learning: In @solana/web3.js v2, 'address' is the correct import to use for handling Solana addresses, replacing the previously used PublicKey class from v1.
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.421Z
Learning: @solana/web3.js v2 has moved to a modular architecture where functionality is split into separate packages like @solana/addresses, @solana/codecs, @solana/rpc, etc., replacing the monolithic imports from v1.
🪛 Biome (1.9.4)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts
[error] 146-146: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
suite-common/wallet-utils/src/solanaStakingUtils.ts
[error] 138-138: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (9)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
155-185: LGTM! Good error handling and fee estimation logic.The function correctly handles fee estimation only when needed and includes proper error handling.
packages/suite/src/utils/suite/solanaStaking.ts (3)
1-34: LGTM! Clean transition to Solana v2 transaction format.The updated imports and type definitions properly support the new transaction format while maintaining a clean interface through
TransactionShim.
63-83: LGTM! Clean implementation of transaction transformation.The function correctly handles the new transaction format and properly integrates with Solana web3.js utilities.
130-224: LGTM! Robust implementation of fee estimation.The transaction preparation functions correctly implement fee estimation with proper error handling and type checking. The implementation is consistent across stake, unstake, and claim operations.
packages/blockchain-link/src/workers/solana/utils.ts (1)
1-9: LGTM!The import statement is concise, and the function signature is well-defined with proper TypeScript types.
suite-common/wallet-utils/src/solanaStakingUtils.ts (4)
1-1: LGTM!The import statement is concise, and the type updates correctly align with the new SDK package.
Also applies to: 34-36
38-51: LGTM!The network configuration is correctly updated to use the new Network enum while maintaining the original behavior.
53-83: LGTM!The changes improve code readability while maintaining the same functionality.
137-140: Optimize the balances reduction to avoid O(n²) complexity.The spread operator in the reducer accumulator causes quadratic time complexity.
Apply this diff to fix the performance issue:
const balances: Record<StakeStateType, string> = balanceResults.reduce( - (acc, [status, balance]) => ({ ...acc, [status]: balance }), + (acc, [status, balance]) => { + acc[status] = balance; + return acc; + }, {}, );🧰 Tools
🪛 Biome (1.9.4)
[error] 138-138: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
| // Type guard to check if transaction is of type CompilableTransactionMessage | ||
| function isCompilableTransactionMessage( | ||
| tx: TransactionMessageWithBlockhashLifetime | CompilableTransactionMessage, | ||
| ): tx is CompilableTransactionMessage { | ||
| return (tx as CompilableTransactionMessage).feePayer !== undefined; | ||
| } | ||
|
|
||
| type PriorityFees = { | ||
| computeUnitPrice: bigint; | ||
| computeUnitLimit: number; | ||
| }; | ||
|
|
||
| export const dummyPriorityFeesForFeeEstimation: PriorityFees = { | ||
| computeUnitPrice: BigInt(SOL_COMPUTE_UNIT_PRICE), | ||
| computeUnitLimit: SOL_COMPUTE_UNIT_LIMIT, | ||
| }; | ||
|
|
||
| const getStakingParams = (estimatedFee?: Fee[number]) => { | ||
| if (!estimatedFee || !estimatedFee.feePerUnit || !estimatedFee.feeLimit) { | ||
| return dummyPriorityFeesForFeeEstimation; | ||
| } | ||
|
|
||
| return { | ||
| сomputeUnitPrice: BigInt(estimatedFee.feePerUnit), | ||
| computeUnitLimit: Number(estimatedFee.feeLimit), // solana package expects number | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Fix typo in property name.
There's a typo in the property name сomputeUnitPrice (using Cyrillic 'с' instead of Latin 'c').
Apply this diff to fix the typo:
- сomputeUnitPrice: BigInt(estimatedFee.feePerUnit),
+ computeUnitPrice: BigInt(estimatedFee.feePerUnit),📝 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.
| // Type guard to check if transaction is of type CompilableTransactionMessage | |
| function isCompilableTransactionMessage( | |
| tx: TransactionMessageWithBlockhashLifetime | CompilableTransactionMessage, | |
| ): tx is CompilableTransactionMessage { | |
| return (tx as CompilableTransactionMessage).feePayer !== undefined; | |
| } | |
| type PriorityFees = { | |
| computeUnitPrice: bigint; | |
| computeUnitLimit: number; | |
| }; | |
| export const dummyPriorityFeesForFeeEstimation: PriorityFees = { | |
| computeUnitPrice: BigInt(SOL_COMPUTE_UNIT_PRICE), | |
| computeUnitLimit: SOL_COMPUTE_UNIT_LIMIT, | |
| }; | |
| const getStakingParams = (estimatedFee?: Fee[number]) => { | |
| if (!estimatedFee || !estimatedFee.feePerUnit || !estimatedFee.feeLimit) { | |
| return dummyPriorityFeesForFeeEstimation; | |
| } | |
| return { | |
| сomputeUnitPrice: BigInt(estimatedFee.feePerUnit), | |
| computeUnitLimit: Number(estimatedFee.feeLimit), // solana package expects number | |
| }; | |
| }; | |
| // Type guard to check if transaction is of type CompilableTransactionMessage | |
| function isCompilableTransactionMessage( | |
| tx: TransactionMessageWithBlockhashLifetime | CompilableTransactionMessage, | |
| ): tx is CompilableTransactionMessage { | |
| return (tx as CompilableTransactionMessage).feePayer !== undefined; | |
| } | |
| type PriorityFees = { | |
| computeUnitPrice: bigint; | |
| computeUnitLimit: number; | |
| }; | |
| export const dummyPriorityFeesForFeeEstimation: PriorityFees = { | |
| computeUnitPrice: BigInt(SOL_COMPUTE_UNIT_PRICE), | |
| computeUnitLimit: SOL_COMPUTE_UNIT_LIMIT, | |
| }; | |
| const getStakingParams = (estimatedFee?: Fee[number]) => { | |
| if (!estimatedFee || !estimatedFee.feePerUnit || !estimatedFee.feeLimit) { | |
| return dummyPriorityFeesForFeeEstimation; | |
| } | |
| return { | |
| computeUnitPrice: BigInt(estimatedFee.feePerUnit), | |
| computeUnitLimit: Number(estimatedFee.feeLimit), // solana package expects number | |
| }; | |
| }; |
| const blockchainEnvironment = isTestnet ? 'devnet' : 'mainnet'; | ||
|
|
||
| // Find the blockchain configuration for the specified chain and environment | ||
| const blockchainConfig = config.find(c => | ||
| c.blockchain.name.toLowerCase().includes(`solana ${blockchainEnvironment}`), | ||
| ); | ||
| const serverUrl = blockchainConfig?.blockchain.server[0]; | ||
| const network = isTestnet ? Network.Devnet : Network.Mainnet; | ||
|
|
||
| const solanaClient = new Solana(network, serverUrl); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for missing configuration.
The current implementation has potential issues:
- The config search using string inclusion is fragile and could match unintended entries.
- Missing error handling for undefined config or server URL.
Apply this diff to improve error handling:
const blockchainEnvironment = isTestnet ? 'devnet' : 'mainnet';
// Find the blockchain configuration for the specified chain and environment
const blockchainConfig = config.find(c =>
- c.blockchain.name.toLowerCase().includes(`solana ${blockchainEnvironment}`),
+ c.blockchain.name.toLowerCase() === `solana ${blockchainEnvironment}`,
);
+ if (!blockchainConfig) {
+ throw new Error(`Configuration not found for Solana ${blockchainEnvironment}`);
+ }
const serverUrl = blockchainConfig?.blockchain.server[0];
+ if (!serverUrl) {
+ throw new Error(`Server URL not found for Solana ${blockchainEnvironment}`);
+ }
const network = isTestnet ? Network.Devnet : Network.Mainnet;
const solanaClient = new Solana(network, serverUrl);📝 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.
| const blockchainEnvironment = isTestnet ? 'devnet' : 'mainnet'; | |
| // Find the blockchain configuration for the specified chain and environment | |
| const blockchainConfig = config.find(c => | |
| c.blockchain.name.toLowerCase().includes(`solana ${blockchainEnvironment}`), | |
| ); | |
| const serverUrl = blockchainConfig?.blockchain.server[0]; | |
| const network = isTestnet ? Network.Devnet : Network.Mainnet; | |
| const solanaClient = new Solana(network, serverUrl); | |
| const blockchainEnvironment = isTestnet ? 'devnet' : 'mainnet'; | |
| // Find the blockchain configuration for the specified chain and environment | |
| const blockchainConfig = config.find(c => | |
| c.blockchain.name.toLowerCase() === `solana ${blockchainEnvironment}`, | |
| ); | |
| if (!blockchainConfig) { | |
| throw new Error(`Configuration not found for Solana ${blockchainEnvironment}`); | |
| } | |
| const serverUrl = blockchainConfig?.blockchain.server[0]; | |
| if (!serverUrl) { | |
| throw new Error(`Server URL not found for Solana ${blockchainEnvironment}`); | |
| } | |
| const network = isTestnet ? Network.Devnet : Network.Mainnet; | |
| const solanaClient = new Solana(network, serverUrl); |
tomasklim
force-pushed
the
feat/solana-v2-staking-support
branch
from
b3cf69c to
de12fa6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/suite/src/utils/suite/solanaStaking.ts (1)
124-128:⚠️ Potential issueFix typo in property name.
The property name
сomputeUnitPriceuses a Cyrillic 'с' instead of Latin 'c'.Apply this diff to fix the typo:
- сomputeUnitPrice: BigInt(estimatedFee.feePerUnit), + computeUnitPrice: BigInt(estimatedFee.feePerUnit),
🧹 Nitpick comments (4)
suite-common/wallet-utils/src/solanaStakingUtils.ts (2)
97-104: Add type safety to status parameter.The
statusparameter should be typed to ensure only validStakeStatevalues are passed.-export const getSolanaStakingAccountsByStatus = (account: Account, status: string) => { +export const getSolanaStakingAccountsByStatus = (account: Account, status: StakeStateType) => {
143-147: Consider extracting stake state strings as constants.For better maintainability and to avoid typos, consider extracting the stake state strings into named constants.
+const STAKE_STATES = { + ACTIVE: StakeState.Active, + DEACTIVATED: StakeState.Deactivated, + ACTIVATING: StakeState.Activating, + DEACTIVATING: StakeState.Deactivating, +} as const; return { - solStakedBalance: balances[StakeState.Active], - solClaimableBalance: balances[StakeState.Deactivated], - solPendingStakeBalance: balances[StakeState.Activating], - solPendingUnstakeBalance: balances[StakeState.Deactivating], + solStakedBalance: balances[STAKE_STATES.ACTIVE], + solClaimableBalance: balances[STAKE_STATES.DEACTIVATED], + solPendingStakeBalance: balances[STAKE_STATES.ACTIVATING], + solPendingUnstakeBalance: balances[STAKE_STATES.DEACTIVATING],packages/suite/src/utils/suite/solanaStaking.ts (1)
40-61: Consider adding unit tests for the transaction shim.The
createTransactionShimCommonfunction handles critical transaction operations. Given its importance in the Solana staking workflow, it should be thoroughly tested.Would you like me to help generate unit tests for this function?
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
146-153: Use optional chaining for cleaner code.The nested condition checking can be simplified using optional chaining.
Apply this diff to improve readability:
- if (estimatedFee && estimatedFee.payload && 'levels' in estimatedFee.payload) { - const { levels } = estimatedFee.payload; - - return levels[0]; - } - - return undefined; + return estimatedFee?.payload?.levels?.[0];🧰 Tools
🪛 Biome (1.9.4)
[error] 146-146: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (22)
package.json(1 hunks)packages/blockchain-link-types/package.json(0 hunks)packages/blockchain-link-types/src/solana.ts(1 hunks)packages/blockchain-link-utils/package.json(0 hunks)packages/blockchain-link-utils/src/blockbook.ts(1 hunks)packages/blockchain-link/package.json(1 hunks)packages/blockchain-link/src/workers/solana/index.ts(2 hunks)packages/blockchain-link/src/workers/solana/utils.ts(1 hunks)packages/blockchain-link/src/workers/utils.ts(0 hunks)packages/suite/package.json(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormActions.ts(3 hunks)packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts(8 hunks)packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx(1 hunks)packages/suite/src/components/suite/__tests__/NumberInput.test.tsx(2 hunks)packages/suite/src/support/messages.ts(1 hunks)packages/suite/src/utils/suite/solanaStaking.ts(5 hunks)packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx(2 hunks)scripts/list-outdated-dependencies/trends-dependencies.txt(1 hunks)suite-common/wallet-constants/src/solanaStakingConstants.ts(1 hunks)suite-common/wallet-utils/package.json(1 hunks)suite-common/wallet-utils/src/solanaStakingUtils.ts(5 hunks)
💤 Files with no reviewable changes (3)
- packages/blockchain-link-utils/package.json
- packages/blockchain-link-types/package.json
- packages/blockchain-link/src/workers/utils.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- scripts/list-outdated-dependencies/trends-dependencies.txt
- package.json
- packages/blockchain-link-types/src/solana.ts
- packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts
- packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx
- packages/suite/package.json
- packages/blockchain-link-utils/src/blockbook.ts
- suite-common/wallet-constants/src/solanaStakingConstants.ts
- suite-common/wallet-utils/package.json
- packages/blockchain-link/src/workers/solana/utils.ts
- packages/blockchain-link/package.json
- packages/blockchain-link/src/workers/solana/index.ts
- packages/suite/src/actions/wallet/stake/stakeFormActions.ts
- packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx
- packages/suite/src/support/messages.ts
🧰 Additional context used
🧠 Learnings (1)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.421Z
Learning: In @solana/web3.js v2, 'address' is the correct import to use for handling Solana addresses, replacing the previously used PublicKey class from v1.
🪛 Biome (1.9.4)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts
[error] 146-146: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
suite-common/wallet-utils/src/solanaStakingUtils.ts
[error] 138-138: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (7)
suite-common/wallet-utils/src/solanaStakingUtils.ts (4)
1-1: LGTM: Clean SDK upgrade changes!The import changes and type updates correctly align with the migration to
@everstake/wallet-sdk-solana.Also applies to: 34-36
40-41: LGTM: Clean network configuration update!The network configuration correctly uses the new
Networkenum from the SDK.
56-59: LGTM: Simplified balance calculation!The balance calculation is now more straightforward with direct property access.
137-140: Optimize the balances reduction to avoid O(n²) complexity.The spread operator in the reducer accumulator causes quadratic time complexity.
const balances: Record<StakeStateType, string> = balanceResults.reduce( - (acc, [status, balance]) => ({ ...acc, [status]: balance }), + (acc, [status, balance]) => { + acc[status] = balance; + return acc; + }, {}, );🧰 Tools
🪛 Biome (1.9.4)
[error] 138-138: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
packages/suite/src/components/suite/__tests__/NumberInput.test.tsx (1)
62-70: LGTM! Improved test stability by wrapping user events inact().The changes correctly wrap user events in
act()to ensure all state updates are properly flushed before assertions, following React Testing Library best practices.packages/suite/src/utils/suite/solanaStaking.ts (1)
30-34: LGTM! Well-defined transaction shim interface.The
TransactionShiminterface provides a clear contract for transaction handling with methods for signature management and serialization.packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
46-48: LGTM! Robust fee handling with fallback.The code correctly handles fee estimation with a fallback to
SOL_STAKING_OPERATION_FEEwhenestimatedFeeis not available.Also applies to: 65-66
| const estimatedFee = { | ||
| feePerTx: transactionInfo.fee, | ||
| feeLimit: transactionInfo.feeLimit, | ||
| feePerUnit: transactionInfo.feePerByte, | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider adding validation for fee values.
The estimated fee object is constructed without validating the input values. Consider adding checks to ensure the values are valid numbers.
🏁 Script executed:
#!/bin/bash
# Check if there are any fee validation checks in the codebase
rg -A 5 "feePerTx|feeLimit|feePerByte" --type tsLength of output: 68729
Fee Value Validation Needed
The estimated fee object in this file
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (lines 205–209)
is constructed directly from properties of transactionInfo without any explicit validation. In contrast, other parts of the codebase—such as in the blockchain workers and fee UI components (e.g., in Fees/CustomFee.tsx and the fee-related hooks)—apply validation or conversion (e.g., using BigNumber) to ensure fees are valid numbers. To ensure consistency and prevent possible downstream issues, please add appropriate checks to verify that transactionInfo.fee, transactionInfo.feeLimit, and transactionInfo.feePerByte are valid numeric values (and ideally non-negative) before constructing the estimatedFee object.
tomasklim
force-pushed
the
feat/solana-v2-staking-support
branch
from
fee3959 to
15c0089
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
suite-common/wallet-utils/src/solanaStakingUtils.ts (1)
137-140:⚠️ Potential issueOptimize the balances reduction to avoid O(n²) complexity.
The spread operator in the reducer accumulator causes quadratic time complexity.
Apply this diff to optimize the reducer:
const balances: Record<StakeStateType, string> = balanceResults.reduce( - (acc, [status, balance]) => ({ ...acc, [status]: balance }), + (acc, [status, balance]) => { + acc[status] = balance; + return acc; + }, {}, );🧰 Tools
🪛 Biome (1.9.4)
[error] 138-138: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
🧹 Nitpick comments (5)
packages/suite/src/utils/suite/solanaStaking.ts (3)
146-149: Enhance error message for better debugging.The error message "Transaction is not compilable" doesn't provide enough context about why the transaction failed compilation.
Apply this diff to improve the error message:
- throw new Error('Transaction is not compilable'); + throw new Error('Transaction is not compilable: stakeTx does not contain feePayer property');
102-107: Strengthen type guard implementation.The type guard only checks for the
feePayerproperty, which might not be sufficient to guarantee that the transaction is aCompilableTransactionMessage.Apply this diff to make the type guard more robust:
function isCompilableTransactionMessage( tx: TransactionMessageWithBlockhashLifetime | CompilableTransactionMessage, ): tx is CompilableTransactionMessage { - return (tx as CompilableTransactionMessage).feePayer !== undefined; + return ( + 'feePayer' in tx && + 'instructions' in tx && + 'recentBlockhash' in tx + ); }
36-61: Address code duplication in transaction shim creation.The TODO comment indicates that this function is duplicated from the connect package. Consider extracting this common functionality into a shared utility package.
Consider creating a new package (e.g.,
@trezor/solana-utils) to share common Solana-related utilities between packages. This would help avoid code duplication and make maintenance easier.packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (2)
146-150: Simplify optional chaining.The conditional check can be simplified using optional chaining.
Apply this diff to improve the code:
- if (estimatedFee && estimatedFee.payload && 'levels' in estimatedFee.payload) { - const { levels } = estimatedFee.payload; - - return levels[0]; - } + return estimatedFee?.payload?.levels?.[0];🧰 Tools
🪛 Biome (1.9.4)
[error] 146-146: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
77-127: Refactor to reduce code duplication.The function contains similar code patterns for different stake types. Consider refactoring to reduce duplication.
Apply this diff to improve the code structure:
const getTransactionData = async ( formValues: StakeFormState, selectedAccount: SelectedAccountStatus, blockchain: BlockchainNetworks, estimatedFee?: Fee[number], ) => { const { stakeType } = formValues; if (selectedAccount.status !== 'loaded' || selectedAccount.account.networkType !== 'solana') { return; } const { account } = selectedAccount; const selectedBlockchain = blockchain[account.symbol]; + const commonParams = { + from: account.descriptor, + path: account.path, + symbol: account.symbol, + selectedBlockchain, + estimatedFee, + }; - let txData; - if (stakeType === 'stake') { - txData = await prepareStakeSolTx({ - from: account.descriptor, - path: account.path, - amount: formValues.outputs[0].amount, - symbol: account.symbol, - selectedBlockchain, - estimatedFee, - }); - } - - if (stakeType === 'unstake') { - txData = await prepareUnstakeSolTx({ - from: account.descriptor, - path: account.path, - amount: formValues.outputs[0].amount, - symbol: account.symbol, - selectedBlockchain, - estimatedFee, - }); - } - - if (stakeType === 'claim') { - txData = await prepareClaimSolTx({ - from: account.descriptor, - path: account.path, - symbol: account.symbol, - selectedBlockchain, - estimatedFee, - }); - } + const txData = await (stakeType === 'stake' + ? prepareStakeSolTx({ ...commonParams, amount: formValues.outputs[0].amount }) + : stakeType === 'unstake' + ? prepareUnstakeSolTx({ ...commonParams, amount: formValues.outputs[0].amount }) + : stakeType === 'claim' + ? prepareClaimSolTx(commonParams) + : undefined); return txData; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (23)
package.json(1 hunks)packages/blockchain-link-types/package.json(0 hunks)packages/blockchain-link-types/src/solana.ts(1 hunks)packages/blockchain-link-utils/package.json(0 hunks)packages/blockchain-link-utils/src/blockbook.ts(1 hunks)packages/blockchain-link/package.json(1 hunks)packages/blockchain-link/src/workers/solana/index.ts(2 hunks)packages/blockchain-link/src/workers/solana/utils.ts(1 hunks)packages/blockchain-link/src/workers/utils.ts(0 hunks)packages/components/src/components/Tooltip/Tooltip.test.tsx(4 hunks)packages/suite/package.json(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormActions.ts(3 hunks)packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts(8 hunks)packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx(1 hunks)packages/suite/src/components/suite/__tests__/NumberInput.test.tsx(2 hunks)packages/suite/src/support/messages.ts(1 hunks)packages/suite/src/utils/suite/solanaStaking.ts(5 hunks)packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx(2 hunks)scripts/list-outdated-dependencies/trends-dependencies.txt(1 hunks)suite-common/wallet-constants/src/solanaStakingConstants.ts(1 hunks)suite-common/wallet-utils/package.json(1 hunks)suite-common/wallet-utils/src/solanaStakingUtils.ts(5 hunks)
💤 Files with no reviewable changes (3)
- packages/blockchain-link-utils/package.json
- packages/blockchain-link-types/package.json
- packages/blockchain-link/src/workers/utils.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- scripts/list-outdated-dependencies/trends-dependencies.txt
- packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx
- package.json
- packages/suite/package.json
- packages/blockchain-link-utils/src/blockbook.ts
- packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts
- packages/suite/src/components/suite/tests/NumberInput.test.tsx
- packages/blockchain-link-types/src/solana.ts
- packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx
- suite-common/wallet-utils/package.json
- packages/components/src/components/Tooltip/Tooltip.test.tsx
- packages/blockchain-link/src/workers/solana/utils.ts
- packages/blockchain-link/src/workers/solana/index.ts
- suite-common/wallet-constants/src/solanaStakingConstants.ts
- packages/blockchain-link/package.json
- packages/suite/src/actions/wallet/stake/stakeFormActions.ts
- packages/suite/src/support/messages.ts
🧰 Additional context used
🧠 Learnings (1)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (2)
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.421Z
Learning: In @solana/web3.js v2, 'address' is the correct import to use for handling Solana addresses, replacing the previously used PublicKey class from v1.
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.421Z
Learning: @solana/web3.js v2 has moved to a modular architecture where functionality is split into separate packages like @solana/addresses, @solana/codecs, @solana/rpc, etc., replacing the monolithic imports from v1.
🪛 Biome (1.9.4)
suite-common/wallet-utils/src/solanaStakingUtils.ts
[error] 138-138: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts
[error] 146-146: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (7)
suite-common/wallet-utils/src/solanaStakingUtils.ts (5)
1-1: LGTM! Clean migration to the new SDK types.The changes consistently update the imports and types to use the new
@everstake/wallet-sdk-solanapackage, aligning with the PR's objective of upgrading the wallet-sdk.Also applies to: 35-35, 40-41
47-51: LGTM! Network selection logic preserved.The network selection functionality remains intact while properly utilizing the new SDK types.
56-59: LGTM! Simplified stake access logic.The refactored code is more straightforward and maintains proper null checks through optional chaining.
98-103: LGTM! Clean status filtering implementation.The account filtering logic is well-structured with proper null checks and correctly uses the new SDK's
StakeStateenum.Also applies to: 109-109
143-148: LGTM! Clean state-dependent balance mapping.The balance properties are correctly mapped using the new SDK's
StakeStateenum, maintaining type safety and proper boolean calculations.packages/suite/src/utils/suite/solanaStaking.ts (1)
124-128: Fix typo in property name.There's a typo in the property name
сomputeUnitPrice(using Cyrillic 'с' instead of Latin 'c').Apply this diff to fix the typo:
- сomputeUnitPrice: BigInt(estimatedFee.feePerUnit), + computeUnitPrice: BigInt(estimatedFee.feePerUnit),packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
205-209: Add validation for fee values.The estimated fee object is constructed without validating the input values. Consider adding checks to ensure the values are valid numbers.
#!/bin/bash # Check if there are any fee validation checks in the codebase rg -A 5 "feePerTx|feeLimit|feePerByte" --type ts
tomasklim
force-pushed
the
feat/solana-v2-staking-support
branch
from
15c0089 to
7b2c7b4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/blockchain-link/src/workers/solana/utils.ts (1)
10-20:⚠️ Potential issueAdd error handling for missing configuration.
The current implementation has potential issues:
- The config search using string inclusion is fragile and could match unintended entries.
- Missing error handling for undefined config or server URL.
Apply this diff to improve error handling:
const blockchainEnvironment = isTestnet ? 'devnet' : 'mainnet'; // Find the blockchain configuration for the specified chain and environment const blockchainConfig = config.find(c => - c.blockchain.name.toLowerCase().includes(`solana ${blockchainEnvironment}`), + c.blockchain.name.toLowerCase() === `solana ${blockchainEnvironment}`, ); + if (!blockchainConfig) { + throw new Error(`Configuration not found for Solana ${blockchainEnvironment}`); + } const serverUrl = blockchainConfig?.blockchain.server[0]; + if (!serverUrl) { + throw new Error(`Server URL not found for Solana ${blockchainEnvironment}`); + } const network = isTestnet ? Network.Devnet : Network.Mainnet; const solanaClient = new Solana(network, serverUrl);
🧹 Nitpick comments (2)
packages/blockchain-link/src/workers/solana/utils.ts (1)
27-48: Improve robustness and performance of staking account processing.Consider the following improvements:
- Add type guards to make the code more robust
- Simplify nested optional chaining
- Combine map and filter operations for better performance
Apply this diff to improve the code:
- return stakingAccounts - .map(account => { + return stakingAccounts.reduce<Array<{ + rentExemptReserve: bigint; + stake: bigint; + status: string; + }>>((acc, account) => { const stakeAccount = account?.data; - if (!stakeAccount) return; + if (!stakeAccount) return acc; const stakeState = stakeAccountState(stakeAccount, BigInt(epoch)); - const { state } = account?.data ?? {}; + const { state } = stakeAccount; - if (!isStake(state)) return; + if (!isStake(state)) return acc; - if (state && 'fields' in state) { - const { fields } = state; + if ('fields' in state) { + const { fields } = state; + const rentExemptReserve = fields[0]?.rentExemptReserve; + const stake = fields[1]?.delegation?.stake; + + if (!rentExemptReserve || !stake) return acc; - return { + acc.push({ rentExemptReserve: fields[0]?.rentExemptReserve, stake: fields[1]?.delegation?.stake, status: stakeState, - }; + }); } - }) - .filter(account => account !== undefined); + return acc; + }, []);packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
146-150: Improve optional chaining.The condition can be simplified using optional chaining.
Apply this diff to improve the code:
- if (estimatedFee && estimatedFee.payload && 'levels' in estimatedFee.payload) { - const { levels } = estimatedFee.payload; - - return levels[0]; - } + return estimatedFee?.payload?.levels?.[0];🧰 Tools
🪛 Biome (1.9.4)
[error] 146-146: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (23)
package.json(1 hunks)packages/blockchain-link-types/package.json(0 hunks)packages/blockchain-link-types/src/solana.ts(1 hunks)packages/blockchain-link-utils/package.json(0 hunks)packages/blockchain-link-utils/src/blockbook.ts(1 hunks)packages/blockchain-link/package.json(1 hunks)packages/blockchain-link/src/workers/solana/index.ts(2 hunks)packages/blockchain-link/src/workers/solana/utils.ts(1 hunks)packages/blockchain-link/src/workers/utils.ts(0 hunks)packages/components/src/components/Tooltip/Tooltip.test.tsx(4 hunks)packages/suite/package.json(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormActions.ts(3 hunks)packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts(8 hunks)packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx(1 hunks)packages/suite/src/components/suite/__tests__/NumberInput.test.tsx(2 hunks)packages/suite/src/support/messages.ts(1 hunks)packages/suite/src/utils/suite/solanaStaking.ts(5 hunks)packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx(2 hunks)scripts/list-outdated-dependencies/trends-dependencies.txt(1 hunks)suite-common/wallet-constants/src/solanaStakingConstants.ts(1 hunks)suite-common/wallet-utils/package.json(1 hunks)suite-common/wallet-utils/src/solanaStakingUtils.ts(5 hunks)
💤 Files with no reviewable changes (3)
- packages/blockchain-link-types/package.json
- packages/blockchain-link-utils/package.json
- packages/blockchain-link/src/workers/utils.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- scripts/list-outdated-dependencies/trends-dependencies.txt
- packages/components/src/components/Tooltip/Tooltip.test.tsx
- packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx
- package.json
- packages/suite/src/components/suite/tests/NumberInput.test.tsx
- packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx
- packages/blockchain-link-types/src/solana.ts
- suite-common/wallet-constants/src/solanaStakingConstants.ts
- suite-common/wallet-utils/package.json
- packages/suite/package.json
- packages/blockchain-link-utils/src/blockbook.ts
- packages/blockchain-link/src/workers/solana/index.ts
- packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts
- packages/blockchain-link/package.json
- packages/suite/src/actions/wallet/stake/stakeFormActions.ts
- packages/suite/src/support/messages.ts
🧰 Additional context used
🧠 Learnings (1)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.421Z
Learning: In @solana/web3.js v2, 'address' is the correct import to use for handling Solana addresses, replacing the previously used PublicKey class from v1.
🪛 Biome (1.9.4)
suite-common/wallet-utils/src/solanaStakingUtils.ts
[error] 138-138: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts
[error] 146-146: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (5)
packages/blockchain-link/src/workers/solana/utils.ts (2)
1-9: LGTM! Clean imports and well-defined function signature.The imports are focused on the required functionality, and the function signature is clear with appropriate parameter types.
21-26: LGTM! Good error handling for delegation fetching.The code properly handles potential failures in delegation fetching and follows good practices with clean destructuring.
suite-common/wallet-utils/src/solanaStakingUtils.ts (1)
137-140: Optimize the balances reduction to avoid O(n²) complexity.The spread operator in the reducer accumulator causes quadratic time complexity.
Apply this diff to optimize the code:
const balances: Record<StakeStateType, string> = balanceResults.reduce( - (acc, [status, balance]) => ({ ...acc, [status]: balance }), + (acc, [status, balance]) => { + acc[status] = balance; + return acc; + }, {}, );🧰 Tools
🪛 Biome (1.9.4)
[error] 138-138: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
packages/suite/src/utils/suite/solanaStaking.ts (1)
124-127: Fix typo in property name.There's a typo in the property name
сomputeUnitPrice(using Cyrillic 'с' instead of Latin 'c').Apply this diff to fix the typo:
- сomputeUnitPrice: BigInt(estimatedFee.feePerUnit), + computeUnitPrice: BigInt(estimatedFee.feePerUnit),packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
205-209: Add validation for fee values.The estimated fee object is constructed without validating the input values. Consider adding checks to ensure the values are valid numbers.
Apply this diff to add validation:
+ const validateFee = (fee?: string | number) => { + if (!fee) return undefined; + const value = new BigNumber(fee); + return value.isNaN() || value.isLessThan(0) ? undefined : value.toString(); + }; + const estimatedFee = { - feePerTx: transactionInfo.fee, - feeLimit: transactionInfo.feeLimit, - feePerUnit: transactionInfo.feePerByte, + feePerTx: validateFee(transactionInfo.fee), + feeLimit: validateFee(transactionInfo.feeLimit), + feePerUnit: validateFee(transactionInfo.feePerByte), };
tomasklim
force-pushed
the
feat/solana-v2-staking-support
branch
from
7b2c7b4 to
3d8a167
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
suite-common/wallet-utils/src/solanaStakingUtils.ts (1)
137-140:⚠️ Potential issueOptimize the balances reduction to avoid O(n²) complexity.
The spread operator in the reducer accumulator causes quadratic time complexity.
const balances: Record<StakeStateType, string> = balanceResults.reduce( - (acc, [status, balance]) => ({ ...acc, [status]: balance }), + (acc, [status, balance]) => { + acc[status] = balance; + return acc; + }, {}, );🧰 Tools
🪛 Biome (1.9.4)
[error] 138-138: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
packages/suite/src/utils/suite/solanaStaking.ts (1)
124-128:⚠️ Potential issueFix typo in property name.
There's a typo in the property name
сomputeUnitPrice(using Cyrillic 'с' instead of Latin 'c').Apply this diff to fix the typo:
- сomputeUnitPrice: BigInt(estimatedFee.feePerUnit), + computeUnitPrice: BigInt(estimatedFee.feePerUnit),packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
205-209:⚠️ Potential issueFee Value Validation Needed
The estimated fee object is constructed without validating the input values.
Add validation for fee values:
const estimatedFee = { + feePerTx: transactionInfo.fee > 0 ? transactionInfo.fee : undefined, + feeLimit: transactionInfo.feeLimit > 0 ? transactionInfo.feeLimit : undefined, + feePerUnit: transactionInfo.feePerByte > 0 ? transactionInfo.feePerByte : undefined, - feePerTx: transactionInfo.fee, - feeLimit: transactionInfo.feeLimit, - feePerUnit: transactionInfo.feePerByte, };
🧹 Nitpick comments (7)
suite-common/wallet-utils/src/solanaStakingUtils.ts (3)
56-59: Add null check for account parameter.Consider adding a null check for the
accountparameter to prevent potential runtime errors.- const totalAmount = stakingAccounts.reduce((acc, account) => { - if (account?.stake) { - const delegationStake = account.stake?.toString(); + const totalAmount = stakingAccounts.reduce((acc, account) => { + if (!account) return acc; + if (account.stake) { + const delegationStake = account.stake.toString();
85-95: Add input validation for numeric values.Consider validating that
accountBalanceandapyare valid numeric strings before performing calculations.export const calculateSolanaStakingReward = (accountBalance?: string, apy?: string) => { if (!accountBalance || !apy) return '0'; + if (isNaN(Number(accountBalance)) || isNaN(Number(apy))) return '0'; return new BigNumber(accountBalance ?? '')
97-104: Enhance type safety for status parameter.Consider using the StakeState type for the status parameter to prevent invalid status values.
-export const getSolanaStakingAccountsByStatus = (account: Account, status: string) => { +export const getSolanaStakingAccountsByStatus = (account: Account, status: StakeStateType) => {packages/suite/src/utils/suite/solanaStaking.ts (2)
36-61: Consider refactoring to avoid code duplication.The
createTransactionShimCommonfunction is duplicated from the connect package. Consider extracting this common functionality into a shared utility package to maintain DRY principles.
130-164: Enhance error handling in transaction preparation functions.While the transaction preparation functions handle basic errors, they could benefit from more specific error messages and handling of edge cases, such as invalid fee parameters.
Consider enhancing error handling:
try { const solanaClient = selectSolanaWalletSdkNetwork(symbol, selectedBlockchain.url); + if (!solanaClient) { + throw new Error('Failed to initialize Solana client'); + } const lamports = networkAmountToSmallestUnit(amount, symbol); + if (lamports <= 0) { + throw new Error('Invalid amount'); + } + const params = getStakingParams(estimatedFee); + if (params.computeUnitPrice <= 0 || params.computeUnitLimit <= 0) { + throw new Error('Invalid fee parameters'); + }Also applies to: 166-194, 198-224
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (2)
129-153: Improve code style and error handling in fee estimation.The code could benefit from optional chaining and more specific error handling.
Consider these improvements:
async function estimateFee( account: Account, txData?: PrepareStakeSolTxResponse, ): Promise<Fee[number] | undefined> { if (!txData?.success) return undefined; const estimatedFee = await TrezorConnect.blockchainEstimateFee({ coin: account.symbol, request: { specific: { data: txData.tx.txShim.serialize(), isCreatingAccount: false, newTokenAccountProgramName: undefined, }, }, }); - if (estimatedFee && estimatedFee.payload && 'levels' in estimatedFee.payload) { + if (estimatedFee?.payload?.levels?.[0]) { const { levels } = estimatedFee.payload; return levels[0]; } + console.warn('Fee estimation failed:', estimatedFee?.payload?.error); return undefined; }🧰 Tools
🪛 Biome (1.9.4)
[error] 146-146: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
155-185: Enhance error handling in transaction composition.The transaction composition could benefit from more specific error handling and user feedback.
Consider adding specific error handling:
export const composeTransaction = (formValues: StakeFormState, formState: ComposeActionContext) => async (_: Dispatch, getState: GetState) => { const { selectedAccount, blockchain } = getState().wallet; - if (selectedAccount.status !== 'loaded') return; + if (selectedAccount.status !== 'loaded') { + console.warn('Account not loaded'); + return; + } const { account } = selectedAccount; const txData = await getTransactionData(formValues, selectedAccount, blockchain); + if (!txData?.success) { + console.warn('Failed to get transaction data:', txData?.errorMessage); + return; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (23)
package.json(1 hunks)packages/blockchain-link-types/package.json(0 hunks)packages/blockchain-link-types/src/solana.ts(1 hunks)packages/blockchain-link-utils/package.json(0 hunks)packages/blockchain-link-utils/src/blockbook.ts(1 hunks)packages/blockchain-link/package.json(1 hunks)packages/blockchain-link/src/workers/solana/index.ts(2 hunks)packages/blockchain-link/src/workers/solana/utils.ts(1 hunks)packages/blockchain-link/src/workers/utils.ts(0 hunks)packages/components/src/components/Tooltip/Tooltip.test.tsx(4 hunks)packages/suite/package.json(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormActions.ts(3 hunks)packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts(8 hunks)packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx(1 hunks)packages/suite/src/components/suite/__tests__/NumberInput.test.tsx(2 hunks)packages/suite/src/support/messages.ts(1 hunks)packages/suite/src/utils/suite/solanaStaking.ts(5 hunks)packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx(2 hunks)scripts/list-outdated-dependencies/trends-dependencies.txt(1 hunks)suite-common/wallet-constants/src/solanaStakingConstants.ts(1 hunks)suite-common/wallet-utils/package.json(1 hunks)suite-common/wallet-utils/src/solanaStakingUtils.ts(5 hunks)
💤 Files with no reviewable changes (3)
- packages/blockchain-link-types/package.json
- packages/blockchain-link-utils/package.json
- packages/blockchain-link/src/workers/utils.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- scripts/list-outdated-dependencies/trends-dependencies.txt
- suite-common/wallet-utils/package.json
- package.json
- packages/blockchain-link/package.json
- packages/blockchain-link-types/src/solana.ts
- packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx
- packages/components/src/components/Tooltip/Tooltip.test.tsx
- packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts
- suite-common/wallet-constants/src/solanaStakingConstants.ts
- packages/suite/package.json
- packages/blockchain-link/src/workers/solana/utils.ts
- packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx
- packages/suite/src/components/suite/tests/NumberInput.test.tsx
- packages/blockchain-link/src/workers/solana/index.ts
- packages/blockchain-link-utils/src/blockbook.ts
- packages/suite/src/actions/wallet/stake/stakeFormActions.ts
- packages/suite/src/support/messages.ts
🧰 Additional context used
🧠 Learnings (1)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (2)
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.421Z
Learning: In @solana/web3.js v2, 'address' is the correct import to use for handling Solana addresses, replacing the previously used PublicKey class from v1.
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.421Z
Learning: @solana/web3.js v2 has moved to a modular architecture where functionality is split into separate packages like @solana/addresses, @solana/codecs, @solana/rpc, etc., replacing the monolithic imports from v1.
🪛 Biome (1.9.4)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts
[error] 146-146: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
suite-common/wallet-utils/src/solanaStakingUtils.ts
[error] 138-138: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (3)
suite-common/wallet-utils/src/solanaStakingUtils.ts (2)
1-1: LGTM! SDK upgrade changes look good.The changes correctly update the imports and network configurations to use the new SDK types from '@everstake/wallet-sdk-solana'.
Also applies to: 35-35, 40-41
47-51: LGTM! Network selection logic is clean and focused.The function follows the single responsibility principle and properly handles network selection.
packages/suite/src/utils/suite/solanaStaking.ts (1)
1-34: LGTM!The imports and type definitions are well-structured and correctly aligned with Solana v2's modular architecture.
Description
Upgraded wallet-sdk to the latest version
Added support for Solana v2
Implemented dynamic fee estimation for better accuracy
Refactored Solana staking utils to remove unnecessary functions.
Related Issue
Resolve #16365
Screenshots: